From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 13A9BC282D1 for ; Thu, 6 Mar 2025 03:37:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 31123280004; Wed, 5 Mar 2025 22:37:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 299DB280003; Wed, 5 Mar 2025 22:37:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 13BE8280004; Wed, 5 Mar 2025 22:37:38 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id E66CF280003 for ; Wed, 5 Mar 2025 22:37:37 -0500 (EST) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id AEFCE160F25 for ; Thu, 6 Mar 2025 03:37:37 +0000 (UTC) X-FDA: 83189716554.18.23B7756 Received: from out-181.mta0.migadu.com (out-181.mta0.migadu.com [91.218.175.181]) by imf23.hostedemail.com (Postfix) with ESMTP id C79DC140008 for ; Thu, 6 Mar 2025 03:37:35 +0000 (UTC) Authentication-Results: imf23.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b="pRRKwj/E"; spf=pass (imf23.hostedemail.com: domain of shakeel.butt@linux.dev designates 91.218.175.181 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev; dmarc=pass (policy=none) header.from=linux.dev ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1741232256; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=GNLoFOWU5jJQo91E4c4IafI+3MUS/RR+ykWtiwhiEyQ=; b=v0qaKGtapCK1dC6Gg5yvAnZm+JsbsMAegGW7q3N0bM7Yd+ptIhdkzCVF2B7XrxlP5KjLyi JITMJYP+UrrrdlveRBSZVG4SgCr64pKyCz2kSvDwru4CLAlNIJNiJt4ZLkfxJqwK8MA8W/ ubFNtXOYs5XYDdE6aT5Fhf6TcyoZHJ0= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1741232256; a=rsa-sha256; cv=none; b=6MWCROymBuLqV3PuOxHm+Lzl+NbJVBNXqbCeIOeoYX+NjxWcLKl2HuH+G/8gnU688oplKQ zaEx/upkRQnn+1l2BaB7d6YTMo96nLMyWdDDUV2gJxZv5865+lVAekyt2mXx5qWntKAe1S SxvVDaT0cYL+jTnjNIanmwjcasPezv8= ARC-Authentication-Results: i=1; imf23.hostedemail.com; dkim=pass header.d=linux.dev header.s=key1 header.b="pRRKwj/E"; spf=pass (imf23.hostedemail.com: domain of shakeel.butt@linux.dev designates 91.218.175.181 as permitted sender) smtp.mailfrom=shakeel.butt@linux.dev; dmarc=pass (policy=none) header.from=linux.dev Date: Wed, 5 Mar 2025 19:37:28 -0800 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1741232253; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=GNLoFOWU5jJQo91E4c4IafI+3MUS/RR+ykWtiwhiEyQ=; b=pRRKwj/Ezs38ddcadgvSq9Rt+fjo4Xa2sZqVlY80b5oY9cDtdK0f8WNuPsyG3MaxenqaU1 jcTu1SckckQCzWqcqptvWdyGRuwKtgsbNLgsiJ7DadkdCU9uP/77nmzu4e0jIfEN5Zpfco HDF6+lCQfP8mfdWeYqUIzAhrs9tKKFQ= X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Shakeel Butt To: SeongJae Park Cc: "Liam R. Howlett" , Andrew Morton , David Hildenbrand , Lorenzo Stoakes , Vlastimil Babka , kernel-team@meta.com, linux-kernel@vger.kernel.org, linux-mm@kvack.org Subject: Re: [RFC PATCH 05/16] mm/madvise: define and use madvise_behavior struct for madvise_do_behavior() Message-ID: References: <20250305235632.137169-1-sj@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250305235632.137169-1-sj@kernel.org> X-Migadu-Flow: FLOW_OUT X-Stat-Signature: xrafwy34fawiksz4d79q5ba3kwhsd66x X-Rspamd-Queue-Id: C79DC140008 X-Rspam-User: X-Rspamd-Server: rspam01 X-HE-Tag: 1741232255-430168 X-HE-Meta: U2FsdGVkX19TEA4oRs25B69Wmy1yMmEJPNu42osYQPa8cVbLsuiJgHB2atBGu5dAQCzJ06V6om1zCb0gFJsamfTg3JxJhV4adx7GaqB9sCvvy7Ax1Xzk/QGIx7e/m/VWmqhst+YEFKh4iNQA9xw7cpN4jaHVK/PqBgRF1VocKnhQ/phEnWdIxANiJoGlGPPFaBZ/qZi8u063WwwHO3EHayaZW7VhZuHqBI1tv6w2LciK4nZmhwv93mGCDWJd2KXwyHlIoZ6r8tJk4AWvwgmCNipJ/MDpI1y4FG/1K8H3OgQg8ZUWDyU6X8J/G2dY+KFulVrXa0+lnLblgqTHgltYSpsM+h85LtdmK8Sb5A2d0S4485bt0H9TTjsxrDKytrzFrYwZllSM0h6fRsP7VBbCLWiemZmP+32S66qlgSwtZs2k986UAaCPf1Dmvto/eWfIeSo9HvH4y/givU3x6VnhOfaRZ1StyqfOrtc95LN/s7t5rHQoTNSWGPylMz/ZTyZvPURxKsiaba+ljPcavMyLiVILnRN3UEm+JBrEc7LcmZ3QcOiH2IXffUzitmeGUEHUrrj1er7uxqC6GzV3JhmPLl1OKYeMK3x3amE55BXIg42u/pkxcuJNdVON35N2ir8SLZVQY6KbuXyLSxYsRVv4qwGUdAmEwr5VUzzVMDTDHxJ0IUDQdD3gHE+uDUxZmVWGT/fStRnQFrCfFYjP+62RUNqxWOfN2jwjGUzAMEVvgB1U3unPwBqmxHPCrBWwQgf2jfVnjlIDUixk72JC2v0A4+IKBiiUdjRczygWPUDH3V7GjKkj7S6w/cAynImKtlI3DgGanGkTJLAjLWbmEOegrrEFwNZbHe/0GUp9DIrIfxyv4cLxQGw5MzZj2ajzMGyjfD63VR6/lpvtMbvLQU6hevutPQDx9R6LkmiOy1GfL5OdhwMEDg425WtenuTEGG+hUl7kuu5wTVIOveW1UAn TSndMv1q Il9iFH/fI22w0R2+qBD+DDqqQlBBST8dQMUrKadLsfH7nOc9vWUUWhbk4dIpN/JaZ0e+9KnhGKiarK7yAjtHwWiqqpQWNStVAP4iRBsZs7WXwnnFu+11sjjrzkBdg6EhC036dilkSd/3x/TkC9NNu3D7hMXW1tH97MqBVjcirJLOqLgMvN2JS/alcH+9mJzt/ql5LPgdpH0B9swr4UryCtC05uiu4oBzXf8en X-Bogosity: Ham, tests=bogofilter, spamicity=0.000004, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, Mar 05, 2025 at 03:56:32PM -0800, SeongJae Park wrote: > On Wed, 5 Mar 2025 13:40:17 -0800 Shakeel Butt wrote: > > > > > +struct madvise_walk_param { > > + int behavior; > > + struct anon_vma_name *anon_name; > > +}; > > Only madvise_vma_behavior() will use 'behavior' field. And only > madvise_vma_anon_name() will use 'anon_name' field. But I will have to assume > both function _might_ use both fields when reading the madvise_walk_vmas() > function code. That doesn't make my humble code reading that simple and > straightforward. > > Also populating and passing a data structure that has data that would not > really be used seems not very efficient to me. > This is not a new pattern. You can find a lot of examples in kernel where a struct encapsulates multiple fields and its pointer is passed around rather than those fields (or subset of them). You can check out zap_details, vm_fault, fs_parameter. If some fields are mutually exclusive you can have union in the struct. Also I am not sure what do you mean by "not efficient" here. Inefficient in what sense? Unused memory or something else? > [...] > > @@ -1666,8 +1674,10 @@ static bool is_memory_populate(int behavior) > > } > > > > static int madvise_do_behavior(struct mm_struct *mm, > > - unsigned long start, size_t len_in, int behavior) > > + unsigned long start, size_t len_in, > > + struct madvise_walk_param *arg) > > { > > + int behavior = arg->behavior; > > struct blk_plug plug; > > unsigned long end; > > int error; > > @@ -1681,7 +1691,7 @@ static int madvise_do_behavior(struct mm_struct *mm, > > if (is_memory_populate(behavior)) > > error = madvise_populate(mm, start, end, behavior); > > 'arg' is for madvise_walk_vmas() visit function, but we're using it as a > capsule for passing an information that will be used for madvise_do_behavior(). > This also seems not very straightforward to my humble perspective. Here we can keep behavior as parameter to madvise_walk_vmas() and define struct madvise_walk_param inside it with the passed behavior. Anyways this is a very common pattern in kernel. > > I have no strong opinion and maybe my humble taste is too peculiar. But, if > this is not a blocker for tlb flushes batcing, I'd like to suggest keep this > part as is for now, and revisit for more code cleanup later. What do you > think, Shakeel? > Squashing patches 5 to 8 into one is the main request from me. My other suggestion you can ignore but let's see what other says.