Re: [PATCH 2/2] mm/mempolicy: stop copying the nodemask in the interleave paths

[Rakie Kim]

On Wed, 2 Sep 2026 10:52:45 -0400 Gregory Price <gourry@xxxxxxxxxx> wrote:

> I don't think this actually fixes anything?
>
> But basically the proposal is to widen the SRCU() window further to
> include the cpuset cookie entirely.
>
> e.g.
>
> SRCU() {
> 	cpuset_cookie() {
> 		for_each_node_mask(node, pol->nodes)
> 			weight_total += ...
> 			nnodes++;
> 	}
>
> 	/* ... snip - single node quick-exit ... */
>
>         /* ... actual multi-node bulk allocation ... */
> 	for_each_node_mask(node, pol->nodes) {
> 		nr_allocated = __alloc_pages_bulk(gfp, node, ...);
>
> 		/*
>                  * At this point, due to a torn read from pol->nodes
> 		 * we can visit a node that wasn't present previously
> 		 * or we can skip a node that was present previously.
> 		 *
> 		 * In either case, weight_total is the wrong value for
> 		 * the set of nodes being walked anyway - we are going
> 		 * to skew in the distribution no matter what.
> 		 */
> 	}
> }
>
> I'm not sure widening the SRCU window is worth it here, it doesn't
> actually buy us anything.
>
> Also we'd be calculating the weight total every time even when there's a
> scenario where we quick-exit because the entire allocation fits in the
> first node in the mask.

Sorry, I did not explain that well. I was not suggesting moving the
sum into the cookie loop or widening the SRCU section - only adding
the counter to the sum loop where it already is:

	/* calculate total, detect system default usage */
	nnodes = 0;
	for_each_node_mask(node, pol->nodes) {
		weight_total += table ? table[node] : 1;
		nnodes++;
	}

The order of the function does not change, so the quick-exit path
still returns before this loop runs - the sum is not computed in
that case either way.

What I had in mind is the gap between the two reads. Say the policy
starts with two nodes, every weight is 10, and 100 pages are
requested:

	/* the mask is {0,1} here */
	do {
		cpuset_mems_cookie = read_mems_allowed_begin();
		nnodes = nodes_weight(pol->nodes);	/* nnodes = 2 */
	} while (read_mems_allowed_retry(cpuset_mems_cookie));

	/* a rebind grows the mask to {0,1,2,3} at this point */

	/* calculate total, detect system default usage */
	for_each_node_mask(node, pol->nodes)
		weight_total += ...;			/* 10 * 4 = 40 */

	rounds = rem_pages / weight_total;		/* 100 / 40 = 2 */

	for (i = 0; i < nnodes; i++)			/* bounded by 2 */
		...

The distribution is planned from a total of 40, which spreads the
100 pages over four nodes, but the walk is still bounded by the two
nodes counted earlier. It places 60 pages and returns, and the
caller allocates the remaining 40 one page at a time. The mask does
not have to change again during the walk for this to happen.

You are right that a torn read during the walk still skews the
distribution, and this does not change that. It only removes the
case where the bound and the total start out inconsistent. If that
is not worth the extra line, I am fine either way.

Thanks for looking at it.

Rakie Kim