Received-SPF: pass (google.com: domain of linux-kernel+bounces-46816-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Wed, 31 Jan 2024 11:35:53 -0500
From: Gregory Price <gregory.price@memverge.com>
To: "Huang, Ying" <ying.huang@intel.com>
Cc: Gregory Price <gourry.memverge@gmail.com>, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-api@vger.kernel.org,
	corbet@lwn.net, akpm@linux-foundation.org, honggyu.kim@sk.com,
	rakie.kim@sk.com, hyeongtak.ji@sk.com, mhocko@kernel.org,
	vtavarespetr@micron.com, jgroves@micron.com,
	ravis.opensrc@micron.com, sthanneeru@micron.com,
	emirakhur@micron.com, Hasan.Maruf@amd.com, seungjun.ha@samsung.com,
	hannes@cmpxchg.org, dan.j.williams@intel.com,
	Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
Subject: Re: [PATCH v4 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE
 for weighted interleaving
Message-ID: <Zbp26WZBkzhpLTLV@memverge.com>
References: <20240130182046.74278-1-gregory.price@memverge.com>
 <20240130182046.74278-4-gregory.price@memverge.com>
 <877cjqgfzz.fsf@yhuang6-desk2.ccr.corp.intel.com>
 <Zbn6FG3346jhrQga@memverge.com>
 <87y1c5g8qw.fsf@yhuang6-desk2.ccr.corp.intel.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87y1c5g8qw.fsf@yhuang6-desk2.ccr.corp.intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?us-ascii?Q?iLHkSBamSyy51NedgAIFl+Ghj4g/cSmMzxhkU+/PJpf+yVwJWAFGKN/PuDfW?=
 =?us-ascii?Q?xd2dQhQxE8Lp2KhTV29Xx4x3Jvt6mf1x1qc/D7DY+KfZ6RmBBt0ze7pbej7x?=
 =?us-ascii?Q?UtRA59T/HIDbRACpH3Kcd+o7PnwI4auRmXwNDmecHg3ffksEWYY4/6V2x2rH?=
 =?us-ascii?Q?4gEHJ6oTtfeYTNe9cYsyVruyxvmdFHJvSsztbDeJ4esofRBAVzWLmNv7ke53?=
 =?us-ascii?Q?SB8WLAHNc8M8m2c5gfYNTrkJ3cclA+3+2HXfkxOM8DppeCy6TflbjmEZrc6l?=
 =?us-ascii?Q?Y1fjs17PzD8PSmDv8q2uWEslnh4SJxt74TSJGxTtYhsY8iIE1VVttFYreVKb?=
 =?us-ascii?Q?fUcdF8DzxB5RLuAvPh8clfl/64soVVnq2/IImgYVg9xPz5qIosrRnpixhI3s?=
 =?us-ascii?Q?oIPvV0R6Rdy5jAfbSpAGZtdl7I1fxpTge+GaJGV74qNOu2LJAgF3cXQPMIcq?=
 =?us-ascii?Q?5qRXYb/JGw4asmQvTieB8K8RlBH3Z+xlf+iSMwFlKCHKizHMjFc8C3m2mQSx?=
 =?us-ascii?Q?7t2uzLb80fZKrrrgA9xhAxaV3KoIEwIntfuLayQZ5EOu/RUsPb9l4jIX4NJW?=
 =?us-ascii?Q?xZeeZljTutkSebTyZ2EhLdt3RvzAD/lp+mX5GyND/MK7o9ivEbhlyjBzwqDE?=
 =?us-ascii?Q?7vAbyyL6E872oydCSQEYE1668Rr3w1HyoR60JXMmmojHixb6XI7uBzLQB0uS?=
 =?us-ascii?Q?qy+gBzKdivMtoIbAvfmfy2h9Gu0JU0IgrHyGOMnTYpIoF49QtQSR0JDvPLBN?=
 =?us-ascii?Q?Rlv0Vv9idaLOJfLvZx7NgGpT6Mj35ab1uglmg2kDALeRtsB1qHsRXrT9n019?=
 =?us-ascii?Q?qtBmYgOEH7aR34BHP8kA2LHePxLPM5xxtjANx42fmEfRj1ez8LZkdOhndjTf?=
 =?us-ascii?Q?YmFPb45yjsqAuN/wYshEQsEkjuxTr9TMsMFQstYmrdS9Pgjv1GaiHLRIQEj1?=
 =?us-ascii?Q?8iNmn4wZ/ZpjNuOe4U4CsRcA7pdReIZ7d4GbITzmcBrjm73veI6AB6uF+DT2?=
 =?us-ascii?Q?Q33VGcAGYlKln/HrJ00BZWe48Ni14OteJ9RY/gpRexI2dnGUk44v68k1Niuz?=
 =?us-ascii?Q?ZwzMYfhSS6+gp68td1YpEizr9axFKpkF7rUYKSb5Rrw3spr9rmpzOKyKdyTW?=
 =?us-ascii?Q?ZRc+UNEnMXS/fuDe2wMj138Os/BRDbzFTo0Y0gvbLOwgIjbPT7e8Ma3vZane?=
 =?us-ascii?Q?7x7J5kA+5CqnYFBRXWJHBNGCNYyOxDBkH/i3hIRpFCb+KixM7kxRn5tel2q7?=
 =?us-ascii?Q?X1lLmRwaMCKmGNkALCoEGxzW8NL1Aka1QPQ0Vf4LDGFHpf/wLAKqZTzu6YsY?=
 =?us-ascii?Q?r7xXfQKeX8MnnsyAwJ0i1TaYxl83QR5gbh4dTGONs1/Q7iANZzX253Oqs2W3?=
 =?us-ascii?Q?Rn7hZ0T7Fz3D71Oo7XULPy/zvUNu/ow2FmZQi0MDk6asy/oo72V2aPpYvWU8?=
 =?us-ascii?Q?9f5hWZbyPGCL5v6XTq12rHH3MBxG9JxrXswzG2zoYLFMWt+Uhe8rJnrm3v5E?=
 =?us-ascii?Q?CjyRM2ZplqsQwCfOcx5GMM/haoW0NMj4HuTZg3b39e04MXckTRO7HSpaquH8?=
 =?us-ascii?Q?xBR0oMJyzBLbQVDpkfsaobPR7znKZOfyK1C09zGYlojZkB4nByxB/xggNVAd?=
 =?us-ascii?Q?Mg=3D=3D?=
X-OriginatorOrg: memverge.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3d1ceb2b-5ad5-4b77-75c0-08dc227ab48a
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR17MB5512.namprd17.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2024 16:35:59.1692
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 5c90cb59-37e7-4c81-9c07-00473d5fb682
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: bfExtsZ3aMZfRmJt1mGuuEDhGT7Cme0mDjcjJyErGagf8jgaf3WtFk1cgHfcK00Kn+2AHWhj03Bl96OBooL6Yb9GO7BSJeNUtqkL4/K763s=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY1PR17MB6878

On Wed, Jan 31, 2024 at 05:19:51PM +0800, Huang, Ying wrote:
> Gregory Price <gregory.price@memverge.com> writes:
> 
> > On Wed, Jan 31, 2024 at 02:43:12PM +0800, Huang, Ying wrote:
> >> Gregory Price <gourry.memverge@gmail.com> writes:
> >> >  
> >> > +static unsigned int weighted_interleave_nodes(struct mempolicy *policy)
> >> > +{
> >> > +	unsigned int node = current->il_prev;
> >> > +
> >> > +	if (!current->il_weight || !node_isset(node, policy->nodes)) {
> >> > +		node = next_node_in(node, policy->nodes);
> >> > +		/* can only happen if nodemask is being rebound */
> >> > +		if (node == MAX_NUMNODES)
> >> > +			return node;
> >> 
> >> I feel a little unsafe to read policy->nodes at same time of writing in
> >> rebound.  Is it better to use a seqlock to guarantee its consistency?
> >> It's unnecessary to be a part of this series though.
> >> 
> >
> > I think this is handled already? It is definitely an explicit race
> > condition that is documented elsewhere:
> >
> > /*
> >  * mpol_rebind_policy - Migrate a policy to a different set of nodes
> >  *
> >  * Per-vma policies are protected by mmap_lock. Allocations using per-task
> >  * policies are protected by task->mems_allowed_seq to prevent a premature
> >  * OOM/allocation failure due to parallel nodemask modification.
> >  */
> 
> Thanks for pointing this out!
> 
> If we use task->mems_allowed_seq reader side in
> weighted_interleave_nodes() we can guarantee the consistency of
> policy->nodes.  That may be not deserved, because it's not a big deal to
> allocate 1 page in a wrong node.
> 
> It makes more sense to do that in
> alloc_pages_bulk_array_weighted_interleave(), because a lot of pages may
> be allocated there.
>

That's probably worth just adding now, I'll do it and squash the style
updates into the branch.  Sorry Andrew, I guess 1 last version is
inbound :]

I'll pick up the reviewed tags along the way.

~Gregory