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

[Jose A. Perez de Azpillaga]

On 11/3/26 9:01 AM, Qi Zheng wrote:
> From: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
> 
> 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@xxxxxxxxxxxxxxx
> Reported-by: Lance Yang <lance.yang@xxxxxxxxx>
> Signed-off-by: Qi Zheng <zhengqi.arch@xxxxxxxxxxxxx>
> ---
>  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:

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

>  
>  	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;
>  }


-- 
cheers,
jose a. p-a