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

[Johannes Weiner <hannes@cmpxchg.org>]

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

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?

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.

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*

> 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.