Re: [PATCH 4/4] mm: hugetlb: respect cpuset restrictions for surplus allocations

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

On 11/3/26 8:21 PM, Jose A. Perez de Azpillaga wrote:
> On 11/3/26 9:01 AM, Qi Zheng wrote:
>> From: Qi Zheng <zhengqi.arch@bytedance.com>
>>
>> Gigantic surplus allocations can escape the faulting task's cpuset.
>> CMA and the contiguous allocator trust the supplied nodemask without
>> checking the current cpuset.
>>
>> For example, a shared HugeTLB VMA has MPOL_BIND={0,1}, and T is a
>> non-leader thread. With surplus allocation enabled and no eligible free
>> HugeTLB folio, the following sequence can occur:
>>
>>    CPU0 (cgroup v1 migration)       CPU1 (thread T)
>>    -------------------------       ---------------
>>    cpuset_attach_task(..., T)
>>      T->mems_allowed = {0}
>>      /* Non-leader: skip
>>         mpol_rebind_mm(). */
>>                                    /* Fault; VMA policy is still {0,1}. */
>>                                    policy_nodemask()
>>                                      /* {0,1} intersects {0}. */
>>                                      nodemask = {0,1}
>>                                    hugetlb_cma_alloc_frozen_folio()
>>                                      /* node0 fails; node1 succeeds. */
>>
>> The 1 GiB folio is allocated on node1 despite T being restricted to node0.
>>
>> Intersect the interpreted policy with the current cpuset before surplus
>> allocation, preserving MPOL_BIND and MPOL_PREFERRED_MANY fallback rules.
>> Apply the restriction at the policy-aware entry point to preserve
>> per-node pool management semantics.
>>
>> If the cpuset changes during allocation, rebuild the mask and revalidate
>> any allocated folio against the current constraints. Keep it if it is
>> still eligible: freeing an HVO-optimized folio can defer returning its
>> range to CMA, so discarding a usable folio can cause a needless allocation
>> failure. Only release the folio and allocate again if it is no longer
>> eligible.
>>
>> Fixes: eb02f14c4a2b ("mm/hugetlb: allow overcommitting gigantic hugepages")
>> Cc: stable@vger.kernel.org
>> Reported-by: Lance Yang <lance.yang@linux.dev>
>> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
>> ---
>>   mm/hugetlb.c | 38 +++++++++++++++++++++++++++++++-------
>>   1 file changed, 31 insertions(+), 7 deletions(-)
>>
>> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
>> index ce9c8e57b8e5c..6fd6e58ace869 100644
>> --- a/mm/hugetlb.c
>> +++ b/mm/hugetlb.c
>> @@ -2181,22 +2181,46 @@ 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 (folio) {
>> +		if (unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
>> +			goto retry;
>> +
>> +		if (node_isset(folio_nid(folio), alloc_nodemask))
>> +			return folio;
>> +
>> +		/* Balance free_huge_folio()'s decrement before caller accounting */
>> +		lruvec_stat_mod_folio(folio, NR_HUGETLB, pages_per_huge_page(h));
>> +		free_huge_folio(folio);
>> +		folio = NULL;
>> +	}
> 
> the page allocator only retries when the allocation fails, why does this
> need to drop a folio that raced with a cpuset change? retrying only on
> !folio would make this whole block go away, with the check at the end as:

You're right. I overthought the cpuset race and added unnecessary
revalidation of successful allocations.

> 
>          if (!folio &&
>              unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
>                  goto retry;

I'll follow your suggestion and retry only failed allocations, and
update the commit message accordingly.

Thanks,
Qi

> 
>>   
>>   	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);
>>   
>> -		/* 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 (unlikely(read_mems_allowed_retry(cpuset_mems_cookie)))
>> +		goto retry;
>>   
>>   	return folio;
>>   }
> 
>