Re: [PATCH v4] mm/page_alloc: avoid direct compaction for costly __GFP_NORETRY allocations

On 9/16/26 17:58, Johannes Weiner wrote:
> On Wed, Sep 16, 2026 at 01:24:31PM +0200, Vlastimil Babka (SUSE) wrote:
>> On 9/11/26 17:59, Johannes Weiner wrote:
>> > The access to
>> > MIGRATE_HIGHATOMIC that it points out is in itself too generous. This
>> > seems like a real but separate bug. It will allow GFP_TRANSHUGE_LIGHT
>> > into the highatomic reserves as well, for example.
>> 
>> I don't follow this part. For ALLOC_HIGHATOMIC you need __GFP_HIGH in
>> alloc_flags_nonblocking(). So GFP_TRANSHUGE_LIGHT won't get the access, no?
> 
> rmqueue_buddy() has this:
> 
> 			/*
> 			 * If the allocation fails, allow OOM handling and
> 			 * order-0 (atomic) allocs access to HIGHATOMIC
> 			 * reserves as failing now is worse than failing a
> 			 * high-order atomic allocation in the future.
> 			 */
> 			if (!page && (alloc_flags & (ALLOC_OOM|ALLOC_NON_BLOCK)))
> 				page = __rmqueue_smallest(zone, order, MIGRATE_HIGHATOMIC);

Ah right, comes from Matt's 281dd25c1a01 ("mm/page_alloc: let GFP_ATOMIC
order-0 allocs access highatomic reserves")

> It says "atomic", but it's checking only ALLOC_NON_BLOCK, which is
> broader than GFP_ATOMIC. alloc_flags_nonblocking();
> 
> 	if (gfp_mask & __GFP_DIRECT_RECLAIM)
> 		return 0;
> 
> 	if (gfp_mask & __GFP_NOMEMALLOC)
> 		return 0;
> 
> 	alloc_flags |= ALLOC_NON_BLOCK;
> 
> 	if (order > 0 && (gfp_mask & __GFP_HIGH))
> 		alloc_flags |= ALLOC_HIGHATOMIC;
> 
> So this can apply to random !direct_reclaim requests, no?

Yes, and I agree it shouldn't.

> I have to correct myself on GFP_TRANSHUGE_LIGHT because it happens to
> include __GFP_NOMEMALLOC, and so won't actually get ALLOC_NON_BLOCK.

At least there's that.

> But what about random GFP_NOWAIT and & ~__GFP_DIRECT_RECLAIM sites?
> Those explicitly don't get watermark exemptions already, and weren't
> the intent of the rmqueue_buddy() exemption above.
> 
> Something like this?
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 12fac9084c48..c79cc7aa4dff 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -3246,7 +3246,8 @@ struct page *rmqueue_buddy(struct zone *preferred_zone, struct zone *zone,
>  			 * reserves as failing now is worse than failing a
>  			 * high-order atomic allocation in the future.
>  			 */
> -			if (!page && (alloc_flags & (ALLOC_OOM|ALLOC_NON_BLOCK)))
> +			if (!page && ((alloc_flags & ALLOC_OOM) ||
> +				      ((alloc_flags & ALLOC_MASK_ATOMIC) == ALLOC_MASK_ATOMIC)))
>  				page = __rmqueue_smallest(zone, order, MIGRATE_HIGHATOMIC);
>  
>  			if (!page) {
> diff --git a/mm/page_alloc.h b/mm/page_alloc.h
> index b9259deddb59..11714ddca254 100644
> --- a/mm/page_alloc.h
> +++ b/mm/page_alloc.h
> @@ -60,6 +60,9 @@
>  /* Flags that allow allocations below the min watermark. */
>  #define ALLOC_RESERVES (ALLOC_NON_BLOCK|ALLOC_MIN_RESERVE|ALLOC_HIGHATOMIC|ALLOC_OOM)
>  
> +/* Flag combination from GFP_ATOMIC */
> +#define ALLOC_MASK_ATOMIC (ALLOC_NON_BLOCK|ALLOC_MIN_RESERVE)
> +
>  /*
>   * Structure for holding the mostly immutable allocation parameters passed
>   * between functions involved in allocations, including the alloc_pages*

Should work.

> 
>> Unless I'm mistaken about that MIGRATE_HIGHATOMIC part, it seems all sashiko
>> concerns can be dismissed and then indeed v4 is the better version.
> 
> It looks like a real issue to me, but one that already exists
> independent of Salvatore's change.

But Salvatore's change (v4, not v5 IIUC) will make it worse because
__GFP_DIRECT_RECLAIM + __GFP_NORETRY costly order opportunistic attempts
with fallback will start eating the highatomic reserves too?

In that case the fix for this should probably be part of the same PR to
Linus and be also stable with Fixes: 281dd25c1a01