Re: [PATCH v2 4/4] mm: hugetlb: fix cpuset violations in gigantic surplus allocations

[Qi Zheng <qi.zheng@linux.dev>]

On 11/5/26 1:15 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] mm: hugetlb: empty nodemask passed in alloc_buddy_hugetlb_folio()
> - [Medium] mm: hugetlb: MPOL_BIND violation in alloc_buddy_hugetlb_folio() on cpuset race
> --
> 
> commit 6e27dfcfc4f6238bea6b6e8639c15188d8d94295
> Author: Qi Zheng <qi.zheng@linux.dev>
> 
> mm: hugetlb: fix cpuset violations in gigantic surplus allocations
> 
> This commit intersects the allocation nodemask in alloc_buddy_hugetlb_folio()
> with cpuset_current_mems_allowed so gigantic surplus hugepage allocations do
> not allocate from nodes outside the current task's cpuset. It also retries
> failed allocations if cpuset_current_mems_allowed changes during the attempt.
> 
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index 6fba92b0f6749..09b64f7c11dd2 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -2181,22 +2181,33 @@ struct folio *alloc_buddy_hugetlb_folio(struct hstate *h,
>>   		gfp_t gfp_mask, struct mempolicy_interpreted *mpoli)
>>   {
>>   	struct folio *folio = NULL;
>> -	nodemask_t *nodemask = mpoli->nodemask;
>> +	nodemask_t alloc_nodemask;
>> +	unsigned int cpuset_mems_cookie;
>> +
>> +retry:
>> +	cpuset_mems_cookie = read_mems_allowed_begin();
>> +	alloc_nodemask = cpuset_current_mems_allowed;
>> +	if (mpoli->mode == MPOL_BIND && mpoli->nodemask &&
>> +	    nodes_intersects(*mpoli->nodemask, alloc_nodemask))
>> +		nodes_and(alloc_nodemask, alloc_nodemask, *mpoli->nodemask);
>>   
>>   	if (mpoli->mode == MPOL_PREFERRED_MANY) {
>>   		gfp_t gfp = gfp_mask & ~(__GFP_DIRECT_RECLAIM | __GFP_NOFAIL);
>>   
>> +		nodes_and(alloc_nodemask, alloc_nodemask, *mpoli->nodemask);
>>   		folio = alloc_surplus_hugetlb_folio(h, gfp, mpoli->nid,
>> -						    nodemask);
>> +						    &alloc_nodemask);
> 
> [Severity: Medium]
> What happens here when an MPOL_PREFERRED_MANY policy's nodemask is disjoint
> from cpuset_current_mems_allowed (for example, after moving a task or
> non-leader thread to a cpuset on different NUMA nodes, since
> mpol_rebind_preferred() does not remap pol->nodes)?
> 
> Unlike the MPOL_BIND branch above, this path does not check whether the
> intersection is non-empty before calling alloc_surplus_hugetlb_folio() with
> &alloc_nodemask. When alloc_nodemask is empty:
> 
> 1. For gigantic hugepages, alloc_surplus_hugetlb_folio() performs a wasted
>     pass through CMA and alloc_contig_frozen_pages() with an empty nodemask.
> 
> 2. For non-gigantic hugepages, alloc_surplus_hugetlb_folio() ->
>     alloc_fresh_hugetlb_folio() -> only_alloc_fresh_hugetlb_folio() ->
>     alloc_buddy_frozen_folio() -> __alloc_frozen_pages_noprof() fails the fast
>     path and enters __alloc_pages_slowpath(). In check_retry_cpuset():
> 
>     mm/page_alloc.c:check_retry_cpuset() {
>     	if (cpusets_enabled() && ac->nodemask &&
>     			!cpuset_nodemask_valid_mems_allowed(ac->nodemask)) {
>     		ac->nodemask = NULL;
>     		return true;
>     	}
>     }
> 
>     Because cpuset_nodemask_valid_mems_allowed() returns false for an empty
>     nodemask, check_retry_cpuset() treats it as a raced MPOL_BIND nodemask,
>     resets ac->nodemask to NULL, and restarts __alloc_pages_slowpath() with
>     __GFP_DIRECT_RECLAIM masked off. That wakes kswapd across all nodes via
>     wake_all_kswapds() with ac->nodemask == NULL, attempts an allocation on
>     non-preferred fallback cpuset nodes at ALLOC_WMARK_MIN before the normal
>     fallback pass at ALLOC_WMARK_LOW with reclaim/compaction runs, and
>     increments HTLB_BUDDY_PGALLOC_FAIL in alloc_buddy_frozen_folio() if that
>     first pass fails.
> 
> Should the first-pass alloc_surplus_hugetlb_folio() call be guarded by
> if (nodes_and(alloc_nodemask, alloc_nodemask, *mpoli->nodemask)) so that an
> empty intersection skips straight to the cpuset fallback pass?

Right. We should skip the preferred-node attempt when no preferred nodes
are allowed by the current cpuset.

> 
>>   
>> -		/* Fallback to all nodes if page==NULL */
>> -		nodemask = NULL;
>> +		/* Fallback to all nodes allowed by the current cpuset */
>> +		alloc_nodemask = cpuset_current_mems_allowed;
>>   	}
>>   
>> -	if (!folio) {
>> +	if (!folio)
>>   		folio = alloc_surplus_hugetlb_folio(h, gfp_mask, mpoli->nid,
>> -						    nodemask);
>> -	}
>> +						    &alloc_nodemask);
>> +
>> +	if (!folio && unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
>> +		goto retry;
> 
> [Severity: Medium]
> Can pre-intersecting alloc_nodemask with cpuset_current_mems_allowed cause
> non-gigantic surplus allocations to violate MPOL_BIND or use a stale nodemask
> when cpuset_current_mems_allowed changes concurrently?

Yes. We should preserve the original policy nodemask and check cpuset
constraints separately in the hugeTLB CMA and contiguous allocators.
This avoids losing policy information or restricting retries to a stale
cpuset snapshot.

The plan is to:

1. Add optional cpuset checks to the contiguous allocator.
2. Add node-selection checks to the HugeTLB CMA allocator and enable
    cpuset enforcement in both paths for HugeTLB surplus allocations.
3. Keep policy fallback at the policy-aware callers and retry failed
    allocations when cpuset changes.

I'd like to address cpuset handling more comprehensively across the
HugeTLB allocation paths, so I'll drop patch 4 from this series and
follow up with a separate patchset.

Thanks,
Qi

>