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

From: Gregory Price

Date: Wed Sep 02 2026 - 11:10:44 EST


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