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

[Vlastimil Babka (SUSE)]

On 9/19/26 06:13, Matthew Wilcox wrote:
> On Wed, Sep 16, 2026 at 03:35:40PM -0700, Andrew Morton wrote:
>> From: Salvatore Dipietro <dipiets@xxxxxxxxx>
>> Subject: mm/page_alloc: avoid direct compaction for costly __GFP_NORETRY allocations
>> Date: Fri, 4 Sep 2026 11:56:28 +0000
>> 
>> Commit 5d8edfb900d5 ("iomap: Copy larger chunks from userspace")
>> introduced high-order folio allocations in the iomap buffered write path. 
>> When memory is fragmented, each failed costly-order allocation enters
>> __alloc_pages_slowpath() which runs direct compaction and
>> drain_all_pages(), causing a 0.38x throughput drop on PostgreSQL pgbench
>> (simple-update) with 1024 clients on a 96-vCPU arm64 system.
>> 
>> The root issue is that direct compaction is too expensive for hot
>> allocation paths that have fallbacks to smaller allocations. 
>> __filemap_get_folio_mpol() already marks higher-order allocations with
>> __GFP_NORETRY | __GFP_NOWARN, signalling that the caller can handle
>> failure.  However, the page allocator still attempts full direct
>> compaction for costly orders with __GFP_NORETRY, which is unnecessarily
>> aggressive when the caller will simply retry at a lower order.
>> 
>> For costly-order allocations with __GFP_NORETRY, clear
>> __GFP_DIRECT_RECLAIM at the very start of the slowpath, before
>> can_direct_reclaim, can_compact and the nofail checks are evaluated.  This
>> makes the entire slowpath treat the request as non-blocking: no direct
>> reclaim, no direct compaction and no drain_all_pages() IPI across every
>> CPU.  kswapd (and in turn kcompactd) is still woken further down for
>> background defragmentation, so compaction keeps working for long-term
>> system health while being removed from the latency-critical direct
>> allocation path.
>> 
>> Allocations that also request __GFP_THISNODE are exempted.  That flag
>> pairing identifies the local-node-first THP attempt issued by
>> alloc_pages_mpol() (mempolicy.c), which relies on direct compaction to
>> form transparent huge pages.
> 
> This patch is still piling hack on hack.  We haven't made a serious
> effort to understand what's going on, we're just adjusting flags until
> things stop sucking.
> 
> What we know:
>  - Doing compaction every time (which is what we currently do)
>    performs badly
>  - Doing compaction once at the requested large size and then not
>    again until the smallest size also performs badly.  So it's not
>    that we're doing too much compaction, it's that we're doing
>    compaction at all.

The very first version [1] mentions the associated drain_all_pages(),
interestingly.

> But we don't know why compaction is performing badly.  For example,
> we could specify MIGRATE_ASYNC or MIGRATE_SYNC_LIGHT if __GFP_NORETRY
> is set.  Just as a reminder, here's how __GFP_NORETRY is documented:

That already happens (for costly orders) in __alloc_pages_slowpath() where
it sets INIT_COMPACT_PRIORITY.

>  * %__GFP_NORETRY: The VM implementation will try only very lightweight
>  * memory direct reclaim to get some memory under memory pressure (thus
>  * it can sleep). It will avoid disruptive actions like OOM killer. The
>  * caller must handle the failure which is quite likely to happen under
>  * heavy memory pressure. The flag is suitable when failure can easily be
>  * handled at small cost, such as reduced throughput.
> 
> So the callers aren't doing anything unreasonable when they say
> __GFP_NORETRY.  It's the page allocator (and apparently the compaction
> side of it) that's not living up to the documented contract.

Note that all these different versions of this patch mention compaction, but
implement a removal of __GFP_DIRECT_RECLAIM, thus suppressing both reclaim
and compaction, in response to __GFP_NORETRY. Thus turning this effectively
to a GFP_NOWAIT allocation. So that's also a violation of the contract, no?

In [1] thread and also later on v2 thread by you [2] it was proposed to make
the caller itself do this downgrade to GFP_NOWAIT. But Christoph and Dave
didn't like that approach (that was conditional to costly order, I don't
think yours had a response), which is understandable for the costly-order
case, but for your case it makes sense that if you don't want any
reclaim+compaction, you tell the allocator GFP_NOWAIT and not __GFP_NORETRY.

> So that's one approach which has not, as far as I can tell, been
> investigated.
> 
> The other thing that icks me about this patch is how complex the
> condition is:
> 
>> +	/*
>> +	 * Costly __GFP_NORETRY callers have a cheap fallback, so don't stall
>> +	 * them in reclaim or compaction. __GFP_THISNODE callers are exempt.
>> +	 */
>> +	if (costly_order && (gfp_mask & __GFP_NORETRY) &&
>> +	    !(gfp_mask & __GFP_THISNODE))
>> +		gfp_mask &= ~__GFP_DIRECT_RECLAIM;
> 
> So basically we're saying that __GFP_NORETRY means don't do compaction
> unless __GFP_THISNODE is set, which is just special pleading.  Surely
> the right answer is to remove __GFP_NORETRY from the one caller which
> needs __GFP_THISNODE?
> 
> 
> A slightly unrelated critique of this patch is that it's far too
> complicated for what it does.  We could achieve the same thing by doing:
> 
>  static inline bool gfp_compaction_allowed(gfp_t gfp_mask)
>  {
> -       return IS_ENABLED(CONFIG_COMPACTION) && (gfp_mask & __GFP_IO);
> +       return IS_ENABLED(CONFIG_COMPACTION) && (gfp_mask & __GFP_IO) &&
> +               (!(gfp_mask & __GFP_NORETRY) || (gfp_mask & __GFP_THISNODE));
>  }
> 
> 
> Something I've been noodling on the past day or so is cleaning up the
> GFP flags for "how hard to reclaim".  We currently have five
> possibilities encoded in four bits:
> 
> 1. No direct reclaim (__GFP_DIRECT_RECLAIM clear)
> 2. Light reclaim (__GFP_DIRECT_RECLAIM | __GFP_NORETRY)
> 3. Normal reclaim (__GFP_DIRECT_RECLAIM)
> 4. Extra reclaim (__GFP_DIRECT_RECLAIM | ___GFP_RETRY_MAYFAIL)
> 5. Reclaim forever (__GFP_DIRECT_RECLAIM | __GFP_NOFAIL)
> 
> Clearly we can save ourselves a GFP flag bit by encoding those five
> options into three bits.  Some of the code that manipulates "how hard to
> reclaim" will need to be adjusted, but it shouldn't be that many places
> to change.
> 
> If we do that, we can insert more options into the mix if they're really
> needed.  Like we could have:
> 
> 0 - No direct reclaim
> 1 - Light reclaim, no compaction
> 2 - Light reclaim with compaction
> 3 - Normal reclaim
> 4 - Extra reclaim
> 5 - Reclaim forever

There's also still (for better or worse) the THP special case of light
compaction with no reclaim.
And the long-standing principle (maybe cargo cult? who knows) that for
non-costly-order allocations (with fallback) the tradeoff of e.g. 2 is more
favorable than 0. But we don't want the callers to have to change flags
depending on the order. So it would have to be a distinct option?

> and we'd still have two extra states in case we need to add more
> flavours.

[1] https://lore.kernel.org/all/20260403193535.9970-1-dipiets@xxxxxxxxx/
[2] https://lore.kernel.org/all/alEz4Chf7Ibyg-ZG@xxxxxxxxxxxxxxxxxxxx/