Received-SPF: pass (google.com: domain of linux-kernel+bounces-36077-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 23 Jan 2024 16:27:35 -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 v2 3/3] mm/mempolicy: introduce MPOL_WEIGHTED_INTERLEAVE
 for weighted interleaving
Message-ID: <ZbAvR+U+tyLvsh8R@memverge.com>
References: <20240119175730.15484-1-gregory.price@memverge.com>
 <20240119175730.15484-4-gregory.price@memverge.com>
 <87jzo0vjkk.fsf@yhuang6-desk2.ccr.corp.intel.com>
 <Za9GiqsZtcfKXc5m@memverge.com>
 <Za9LnN59SBWwdFdW@memverge.com>
 <87a5owv454.fsf@yhuang6-desk2.ccr.corp.intel.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <87a5owv454.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?9osMhT33FDTSV7W1injJrk7wydqL2ha16DL7sq6V5Bkda5C65V6Hu4z2YQuq?=
 =?us-ascii?Q?/cWctv26+Xk0amJETwQ4t4CP0CfJqQyKF4z4Ov+jlTuNvtAFxVq+5rl6ve0w?=
 =?us-ascii?Q?ruUPQc/XbLxPirEuiBHvSvnuCT6PFzf7E46wULvgJMyqA3huikTK0gwKHV6p?=
 =?us-ascii?Q?A6u5EF8itTHvddgJmgFKKhw7hZMu7kHipKnrY1qoPP3Z/0wH8nszn65Pkom2?=
 =?us-ascii?Q?ym/VOBOCrFb8y+vzqioXeaifAJIFWY4zVHhqwar9fEqJXUjTbPu+KQ8vZnue?=
 =?us-ascii?Q?GbCvv7ET00l2xsoriBT2oRRpuyggwAbxsbPlQePL1R8/Wn/RBGFQCbD8hxnr?=
 =?us-ascii?Q?DvOAPLwy8BjJ6NrBhIC6APYJ2adYnYFMCSmZp77tBtfpqs7Zo61rP2A7wi6O?=
 =?us-ascii?Q?bT1rBo+74tfBW3HmO2kk9oCZESInYiZfLnqR72GY4tdsqtd3J8fya2IGyD/y?=
 =?us-ascii?Q?2j+Lz5PlYEghH9QGaQ5KumW47+zUWAQKZGEKPTyp8KtOpgkhH+MmBZ6+bKXa?=
 =?us-ascii?Q?0YnfDdoFhJPBTgFIokJrCG3QaZ+WToYQCXUgmb6sts6Iv5b9O2y4oQNKbueO?=
 =?us-ascii?Q?q5lipnzfAiWOoggBnRhlQi+q+bZIZPMOnt3VMIPVtyir4Djx/Kd79dtZ3zVL?=
 =?us-ascii?Q?SboXFXS9ynEf72pWeOhxuGdk+kZwDtNhedL4P7WFPeDQltqXdin2XbSZUpYj?=
 =?us-ascii?Q?s2yXWOTNil18KfF1bmbuIwZvK2tMxfw1R4oQ+0rerNi3kl04lL+7p4YjpV2Y?=
 =?us-ascii?Q?bnwWzBlLklHrX3o+gZmbrTcsnih9p8Rsfpa5nkXv6LpH/lxg03+kqkCFwFy3?=
 =?us-ascii?Q?m0DZtsYMq7hPSs0Px3IzEH2ETvMcZ1faGoVs24lqlQTaLHXPTU27/rjCF+Y7?=
 =?us-ascii?Q?0lgk74VW4wkr1HiGeFeNWfsaDKoP3pNsPcIAX6wbqff0/Gm+OEsmQrMdf6WV?=
 =?us-ascii?Q?p4q+23mEWOfilNeeNwWO1fvhfrBt57cO5jukN57fg438xQ8H+ZcU8xn1AB8k?=
 =?us-ascii?Q?qnet48SjS7M7F5Uh67q9HRedLOxdsNTK4mXQUg3HD7oXtFFcgu7ryPt/rXOr?=
 =?us-ascii?Q?rPzIjpqEU7sH773sWUKMgH8xOiCUpuh4eN7fDhkwEFwax6W7dYNmaRzsQ6cw?=
 =?us-ascii?Q?nc9OS/T6Z6r86A6YCdC73Ci0GrStPmxZxeyjEt/kSI7xsxdWaWpZ98+UA+Jj?=
 =?us-ascii?Q?QPbYeFXKqWq3jZ0ojH/dLBFxWeRPwEbgNvupsbfgQhLNk3HpEkBZWiLLXvi+?=
 =?us-ascii?Q?9r295ou8IBzo6/54jWmUqZKKrUC1FdiEgzT4eED0qI8O8T57C7rqENzmC1rV?=
 =?us-ascii?Q?wdEbK5gfcBKdl5JWbQ4yrS7YKJ/4/OGccwPa8YCMMKWEGyS3yJBPDaPiRotQ?=
 =?us-ascii?Q?FqujcnllVTsuVUIC4qoIzWkoT2jziN8/Lbf7mzYcgC8nAQ/nJ4LM9NPxGuG0?=
 =?us-ascii?Q?ZUVu3loctVIb6WdAaHdtRk6eS89b56aQ93ynmEW1W18uSjrA6wNFe65ywQTt?=
 =?us-ascii?Q?sMJR4X7TUCCDv636coW26QaMIX4y+TCbY7yZrqH1j3gK8OHpAerUZNoVwN5I?=
 =?us-ascii?Q?ztCPpsGUENXne++qXqopmd5gKnhDR+oip5oJolGKHWW1xaK5+Ppq2v2G0qSk?=
 =?us-ascii?Q?ZQ=3D=3D?=
X-OriginatorOrg: memverge.com
X-MS-Exchange-CrossTenant-Network-Message-Id: de1dc1d1-f7af-4aec-e6e6-08dc1c5a22ea
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR17MB5512.namprd17.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jan 2024 21:27:43.9741
 (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: HJXAQFCME7akKKtGG+e4MkTWxTJFff70lYwI9qBx352jtMFvcgtKOfZQUOI5BhoGd29DfJpm8d9KfSVGI5ne3OHVexPQEpYuk7jsuMZGvc8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR17MB4374

On Tue, Jan 23, 2024 at 04:35:19PM +0800, Huang, Ying wrote:
> Gregory Price <gregory.price@memverge.com> writes:
> 
> > On Mon, Jan 22, 2024 at 11:54:34PM -0500, Gregory Price wrote:
> >> > 
> >> > Can the above code be simplified as something like below?
> >> > 
> >> >         resume_node = prev_node;
> > ---         resume_weight = 0;
> > +++         resume_weight = weights[node];
> >> >         for (...) {
> >> >                 ...
> >> >         }
> >> > 
> >> 
> >> I'll take another look at it, but this logic is annoying because of the
> >> corner case:  me->il_prev can be NUMA_NO_NODE or an actual numa node.
> >> 
> >
> > After a quick look, as long as no one objects to (me->il_prev) remaining
> > NUMA_NO_NODE
> 
> MAX_NUMNODES-1 ?
> 

When setting a new policy, the il_prev gets set to NUMA_NO_NODE. It's
not harmful and is just (-1), which is functionally the same as
(MAX_NUMNODES-1) for the purpose of iterating the nodemask with
next_node_in(). So it's fine to set (resume_node = me->il_prev)
as discussed.

I have a cleaned up function I'll push when i fix up a few other spots.

> > while having a weight assigned to pol->wil.cur_weight,
> 
> I think that it is OK.
> 
> And, IIUC, pol->wil.cur_weight can be 0, as in
> weighted_interleave_nodes(), if it's 0, it will be assigned to default
> weight for the node.
> 

cur_weight is different than the global weights.  cur_weight tells us
how many pages are remaining to allocate for the current node.

(cur_weight = 0) can happen in two scenarios:
  - initial setting of mempolicy (NUMA_NO_NODE w/ cur_weight=0)
  - weighted_interleave_nodes decrements it down to 0

Now that i'm looking at it - the second condition should not exist, and
we can eliminate it. The logic in weighted_interleave_nodes is actually
annoyingly unclear at the moment, so I'm going to re-factor it a bit to
be more explicit.

~Gregory