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

[Gregory Price <gourry@gourry.net>]

On Wed, Sep 02, 2026 at 06:00:44PM +0900, Rakie Kim wrote:
> 
> > @@ -2712,9 +2713,13 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp,
> >  	table = state ? state->iw_table : NULL;
> >
> >  	/* calculate total, detect system default usage */
> > -	for_each_node_mask(node, nodes)
> > +	for_each_node_mask(node, pol->nodes)
> >  		weight_total += table ? table[node] : 1;
> 
> I have a minor comment on this part. After this change, everything
> else reads pol->nodes fresh at the point of use - the weight sum
> and the walk both look at the current mask. Only nnodes is still
> the count from this earlier read. If the mask changes in between,
> the loop bound no longer matches the mask the loop is actually
> walking, so the walk can stop short of the pages the weight total
> planned for. Would it be better to count the nodes in the loop
> that sums the weights, the way weighted_interleave_nid() does it
> in this patch?
>

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.

> 	nnodes = 0;
> 	for_each_node_mask(node, pol->nodes) {
> 		weight_total += table ? table[node] : 1;
> 		nnodes++;
> 	}
> 
> [...snip...]
> 
> Thanks again for your time.
> 
> Rakie Kim