Re: [PATCH 3/4] mm: hugetlb: skip CMA when direct reclaim is disallowed

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

On 11/3/26 4:47 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] mm: hugetlb: hugetlb_cma_alloc_frozen_folio() breaks MPOL_PREFERRED_MANY
> --
> 
> commit c96d52d66d643a77abaa96a17e392b7ab0f36ccd
> Author: Qi Zheng <qi.zheng@linux.dev>
> 
> mm: hugetlb: skip CMA when direct reclaim is disallowed
> 
> This commit updates hugetlb_cma_alloc_frozen_folio() to return NULL when
> gfpflags_allow_blocking(gfp_mask) is false, avoiding direct reclaim inside
> cma_alloc_frozen_compound() when __GFP_DIRECT_RECLAIM was cleared by the
> caller.
> 
>> diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
>> index dde1c866406fb..90f8d718a1f16 100644
>> --- a/mm/hugetlb_cma.c
>> +++ b/mm/hugetlb_cma.c
>> @@ -59,7 +59,7 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int order, gfp_t gfp_mask,
>>   	const nodemask_t *nmask;
>>   	unsigned int cpuset_mems_cookie;
>>   
>> -	if (!hugetlb_cma_size)
>> +	if (!hugetlb_cma_size || !gfpflags_allow_blocking(gfp_mask))
>>   		return NULL;
> 
> [Severity: Medium]
> Does returning NULL from hugetlb_cma_alloc_frozen_folio() whenever
> !gfpflags_allow_blocking(gfp_mask) break MPOL_PREFERRED_MANY for gigantic
> surplus HugeTLB allocations?
> 
> In mm/hugetlb.c, alloc_buddy_hugetlb_folio() implements MPOL_PREFERRED_MANY
> as a two-pass allocation:
> 
> mm/hugetlb.c:alloc_buddy_hugetlb_folio() {
> 	if (mpoli->mode == MPOL_PREFERRED_MANY) {
> 		gfp_t gfp = gfp_mask & ~(__GFP_DIRECT_RECLAIM | __GFP_NOFAIL);
> 
> 		folio = alloc_surplus_hugetlb_folio(h, gfp, mpoli->nid,
> 						    nodemask);
> 
> 		/* Fallback to all nodes if page==NULL */
> 		nodemask = NULL;
> 	}
> 
> 	if (!folio) {
> 		folio = alloc_surplus_hugetlb_folio(h, gfp_mask, mpoli->nid,
> 						    nodemask);
> 	}
> }
> 
> This reaches hugetlb_cma_alloc_frozen_folio() via:
> 
>    alloc_buddy_hugetlb_folio()
>      -> alloc_surplus_hugetlb_folio()
>        -> alloc_fresh_hugetlb_folio()
>          -> only_alloc_fresh_hugetlb_folio()
>            -> alloc_gigantic_frozen_folio()
>              -> hugetlb_cma_alloc_frozen_folio()
> 
> On the first pass, __GFP_DIRECT_RECLAIM is cleared while nodemask points to
> the preferred nodes (pol->nodes), and on the second pass nodemask is cleared
> to NULL. Because hugetlb_cma_alloc_frozen_folio() now returns NULL on the
> first pass, it never sees the MPOL_PREFERRED_MANY nodemask.
> 
> In mm/hugetlb.c:alloc_gigantic_frozen_folio():
> 
> 	folio = hugetlb_cma_alloc_frozen_folio(order, gfp_mask, nid, nodemask);
> 	if (folio)
> 		return folio;
> 
> 	if (hugetlb_cma_exclusive_alloc())
> 		return NULL;
> 
> 	folio = (struct folio *)alloc_contig_frozen_pages(1 << order, gfp_mask,
> 							  nid, nodemask);
> 
> When hugetlb_cma_exclusive_alloc() is false, returning NULL from
> hugetlb_cma_alloc_frozen_folio() on the first pass falls through to
> alloc_contig_frozen_pages() on the preferred nodes, bypassing the dedicated
> hugetlb_cma pool on the preferred nodes in favor of non-CMA memory.
> 
> When hugetlb_cma_exclusive_alloc() is true (or when non-CMA allocation fails
> on the preferred nodes), the first pass returns NULL and the second pass
> invokes hugetlb_cma_alloc_frozen_folio() with nodemask set to NULL. Since
> mm/mempolicy.c:policy_nodemask() leaves *nid as numa_node_id() when
> pol->home_node is NUMA_NO_NODE:
> 
> mm/mempolicy.c:policy_nodemask() {
> 	case MPOL_PREFERRED_MANY:
> 		nodemask = &pol->nodes;
> 		if (pol->home_node != NUMA_NO_NODE)
> 			*nid = pol->home_node;
> 		break;
> }
> 
> hugetlb_cma_alloc_frozen_folio() on the second pass tries
> hugetlb_cma[numa_node_id()] and then iterates cpuset_current_mems_allowed
> from node 0, allocating from non-preferred CMA nodes even when preferred
> nodes in pol->nodes have free CMA pages.
> 
> Prior to commit 9bda131c6093 ("mm: cma: add cma_alloc_frozen{_compound}()"),
> cma_alloc_folio() forwarded gfp_mask down to alloc_contig_frozen_range(),
> where __alloc_contig_verify_gfp_mask() preserved the cleared
> __GFP_DIRECT_RECLAIM bit so CMA on the preferred nodemask was tried on the
> first pass without entering direct reclaim. Would forwarding gfp_mask to

Thanks for catching this. You're right: in CMA-only mode, this check
skips the preferred-node attempt entirely. The fallback can then
allocate from a non-preferred node even when the preferred nodes have
usable CMA memory.

> cma_alloc_frozen_compound() (in mm/cma.c) preserve both the non-reclaiming
> first pass and the MPOL_PREFERRED_MANY nodemask?

Make sense.

I'll replace the check with GFP-mask propagation through
cma_alloc_frozen_compound(). This restores the preferred-node CMA
attempt while preserving the caller's __GFP_DIRECT_RECLAIM setting.

Like the following:

diff --git a/include/linux/cma.h b/include/linux/cma.h
index 8555d38a97b1..caedd59bde03 100644
--- a/include/linux/cma.h
+++ b/include/linux/cma.h
@@ -53,7 +53,8 @@ extern bool cma_release(struct cma *cma, const struct 
page *pages, unsigned long

  struct page *cma_alloc_frozen(struct cma *cma, unsigned long count,
  		unsigned int align, bool no_warn);
-struct page *cma_alloc_frozen_compound(struct cma *cma, unsigned int 
order);
+struct page *cma_alloc_frozen_compound(struct cma *cma, unsigned int order,
+				       gfp_t gfp);
  bool cma_release_frozen(struct cma *cma, const struct page *pages,
  		unsigned long count);

diff --git a/mm/cma.c b/mm/cma.c
index a10ea37a261d..c2605967d77a 100644
--- a/mm/cma.c
+++ b/mm/cma.c
@@ -929,9 +929,10 @@ struct page *cma_alloc_frozen(struct cma *cma, 
unsigned long count,
  	return __cma_alloc_frozen(cma, count, align, gfp);
  }

-struct page *cma_alloc_frozen_compound(struct cma *cma, unsigned int order)
+struct page *cma_alloc_frozen_compound(struct cma *cma, unsigned int order,
+				       gfp_t gfp)
  {
-	gfp_t gfp = GFP_KERNEL | __GFP_COMP | __GFP_NOWARN;
+	gfp |= __GFP_COMP | __GFP_NOWARN;

  	return __cma_alloc_frozen(cma, 1 << order, order, gfp);
  }
diff --git a/mm/hugetlb_cma.c b/mm/hugetlb_cma.c
index 90f8d718a1f1..250b5c10d954 100644
--- a/mm/hugetlb_cma.c
+++ b/mm/hugetlb_cma.c
@@ -59,7 +59,7 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int 
order, gfp_t gfp_mask,
  	const nodemask_t *nmask;
  	unsigned int cpuset_mems_cookie;

-	if (!hugetlb_cma_size || !gfpflags_allow_blocking(gfp_mask))
+	if (!hugetlb_cma_size)
  		return NULL;

  	if (current->flags & PF_MEMALLOC_PIN)
@@ -74,14 +74,16 @@ struct folio *hugetlb_cma_alloc_frozen_folio(int 
order, gfp_t gfp_mask,
  	}

  	if (hugetlb_cma[nid] && node_isset(nid, *nmask))
-		page = cma_alloc_frozen_compound(hugetlb_cma[nid], order);
+		page = cma_alloc_frozen_compound(hugetlb_cma[nid], order,
+						 gfp_mask);

  	if (!page && !(gfp_mask & __GFP_THISNODE)) {
  		for_each_node_mask(node, *nmask) {
  			if (node == nid || !hugetlb_cma[node])
  				continue;

-			page = cma_alloc_frozen_compound(hugetlb_cma[node], order);
+			page = cma_alloc_frozen_compound(hugetlb_cma[node], order,
+							 gfp_mask);
  			if (page)
  				break;
  		}

Thanks,
Qi

>