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

[Rakie Kim]

Hello Gregory,

Thanks for the series.

On Fri, 28 Aug 2026 21:59:43 -0400 Gregory Price <gourry@xxxxxxxxxx> wrote:

> The interleave node selectors copy pol->nodes onto the stack so the mask
> cannot change while they walk it.  nodemask_t is 128 bytes at
> MAX_NUMNODES=1024, and two of the three run per folio fault.
>
> The copy only buys consistency between the node count and the walk.
> Drop the consistency and just bounds check the walk instead.

I went through both patches. Resolving the sleeping-allocation
problem with SRCU rather than patching the allocation site, and
removing the copies from the fault path along the way, looks like
the right direction to me.

>
> If an empty nodelist or weight is perceived, fall back to numa_node_id(),
> which is what the functions already did when the copy came back empty.
>
> weighted_interleave_nid() counts the nodes as we sum the weights. We use
> that node count to limit the maximum skew a single node can host.
>
> interleave_nid() walks with next_node_in() rather than next_node(), so a
> mask that shrank mid-walk wraps to a node still in the policy.
>
> alloc_pages_bulk_weighted_interleave() derives per-node counts from a
> weight total summed over the mask, so a changing mask can make them exceed
> the request.  Clamp each chunk to the space left in page_array.
>
> A cpuset cookie will not work here: two of these take VMA policies, which
> mpol_rebind_mm() rebinds under mmap_write_lock(), not mems_allowed_seq.
>
> Cost is distribution accuracy during a rebind - but the copy never
> corrected this anyway, it was just a safety mechanism to prevent div/0
> and overrunning the alloc request buffer.
>
> Remove read_once_policy_nodemask(), now unused.
>
> -fstack-usage at MAX_NUMNODES=1024:
>
>   weighted_interleave_nid            184 -> 56
>   interleave_nid                     168 -> 32
>   alloc_pages_bulk_mempolicy_noprof  360 -> 136
>
> Assisted-by: Claude:claude-opus-5
> Signed-off-by: Gregory Price (Meta) <gourry@xxxxxxxxxx>
> ---
>  mm/mempolicy.c | 86 ++++++++++++++++++++++++++++----------------------
>  1 file changed, 49 insertions(+), 37 deletions(-)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c

[...snip...]

> @@ -2675,10 +2676,10 @@ static unsigned long alloc_pages_bulk_weighted_interleave(gfp_t gfp,
>  	if (!nr_pages)
>  		return 0;
>
> -	/* read the nodes onto the stack, retry if done during rebind */
> +	/* count the nodes, retry if a rebind happened during the read */
>  	do {
>  		cpuset_mems_cookie = read_mems_allowed_begin();
> -		nnodes = read_once_policy_nodemask(pol, &nodes);
> +		nnodes = nodes_weight(pol->nodes);
>  	} while (read_mems_allowed_retry(cpuset_mems_cookie));
>
>  	/* if the nodemask has become invalid, we cannot do anything */

[...snip...]

> @@ -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?

	nnodes = 0;
	for_each_node_mask(node, pol->nodes) {
		weight_total += table ? table[node] : 1;
		nnodes++;
	}

[...snip...]

Thanks again for your time.

Rakie Kim