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

[Johannes Weiner <hannes@cmpxchg.org>]

On Thu, Sep 10, 2026 at 11:46:02AM +0000, Salvatore Dipietro wrote:
> 
> On Sat, 05 Sep 2026 17:42:39 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > Is there anything particularly unusual about this test case?
> 
> It is a stock pgbench simple-update PostgreSQL workload on a large
> instance (96 vCPUs), using standard PostgreSQL settings and with no huge
> pages assigned to the database. We deliberately overprovision the
> pgbench clients: 1024 clients over 96 threads. That keeps enough writers
> in the buffered write path concurrently to hit the costly-order
> allocation failure path continuously. The memory fragmentation comes from 
> page tables: PostgreSQL spawns a new process per client, and those page
> tables consume ~40% of memory, which significantly limits the page cache
> and the free memory available.
> 
> 
> > > Results (average of 3 runs, TPS):
> > >
> > >   Config                   Avg TPS      % vs Baseline
> > >   baseline (no patch)       59,408       -
> > >   With this patch          155,409      +161.6%
> >
> > Is this back to pre-5d8edfb900d5 performance?
> 
> Yes - fully recovered. Here is the summary, same host and workload
> throughout, average of 3 runs:
> 
>   Config                                        Avg TPS   % vs baseline
>   AL2023 stock 6.1 kernel                       136,942            n/a
>   v7.3-rc1 baseline (no patch)                   59,408             -
>   v7.3-rc1, 5d8edfb900d5 behaviour reverted     151,184        +154.5%
>   v7.3-rc1 + v4                                 155,409        +161.6%
> 
> The 6.1 row predates 5d8edfb900d5 entirely. A different kernel version,
> so not directly comparable, but it shows the performance this workload
> used to get on this host.
> 
> A literal "git revert 5d8edfb900d5" does not apply to v7.3-rc1 -
> iomap_write_iter() has been rewritten since - so the reverted row is a
> one-line behavioural revert, forcing the write loop back to copying at
> most PAGE_SIZE per iteration:
> 
>   -	size_t chunk = mapping_max_folio_size(mapping);
>   +	size_t chunk = PAGE_SIZE;
> 
> which is what the pre-5d8edfb900d5 loop computed. That makes
> iomap_get_folio() pass no order hint, so the path issues only order-0
> allocations.
> 
> 
> > AI review asked a few serious-looking questions:
> > 	https://sashiko.dev/#/patchset/20260904115629.3993331-1-dipiets@amazon.it
> 
> Thanks for pointing that out. To address them, we can have something
> like the patch below. Performance results are still similar to v4. Happy
> to submit a formal v5 patch with it if you would like.

The feedback looks misleading to me.

ALLOC_NON_BLOCKING itself is fine, it usually only means something in
conjunction with other flags, like __GFP_HIGH. 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.

The rt issue seems made up too. We're talking about costly_order &&
__GFP_NORETRY allocations. Why would they need access to reserves?

>   Config                                        Avg TPS   % vs baseline
>   v7.3-rc1 baseline (no patch)                   59,408             -
>   v7.3-rc1 + v4                                 155,409        +161.6%
>   v7.3-rc1 + proposed patch                     161,994        +172.7%
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 12fac9084c48..be8b0d72a2db 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -4784,10 +4784,22 @@ static inline struct page *
>  __alloc_pages_slowpath(gfp_t gfp_mask, unsigned int order,
>  						struct alloc_context *ac)
>  {
> -	bool can_direct_reclaim = gfp_mask & __GFP_DIRECT_RECLAIM;
> +	const bool costly_order = order > PAGE_ALLOC_COSTLY_ORDER;
> +	/*
> +	 * Costly __GFP_NORETRY callers have a cheap fallback to a lower order,
> +	 * so don't stall them in direct reclaim or direct compaction.  Exempt
> +	 * __GFP_THISNODE (the THP attempt from alloc_pages_mpol() needs direct
> +	 * compaction) and __GFP_NOFAIL (must not be made to fail).  Don't
> +	 * clear __GFP_DIRECT_RECLAIM from gfp_mask instead: that would also
> +	 * change the alloc_flags derived by alloc_flags_slowpath().
> +	 */
> +	const bool costly_noretry = costly_order &&
> +		(gfp_mask & __GFP_NORETRY) &&
> +		!(gfp_mask & (__GFP_THISNODE | __GFP_NOFAIL));
> +	bool can_direct_reclaim = !costly_noretry &&
> +		(gfp_mask & __GFP_DIRECT_RECLAIM);

IMO this is worse because now there is a disconnect between these
flags and the gfp mask again. This sets us up for subtle bugs.

I much preferred v4.