From: "Vlastimil Babka (SUSE)" <vbabka@kernel.org>
To: Matthew Wilcox <willy@infradead.org>,
Andrew Morton <akpm@linux-foundation.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
Salvatore Dipietro <dipiets@amazon.it>,
abuehaze@amazon.com, alisaidi@amazon.com, blakgeof@amazon.com,
brauner@kernel.org, brendan.jackman@linux.dev, david@redhat.com,
dgc@kernel.org, dipietro.salvatore@gmail.com, djwong@kernel.org,
hch@infradead.org, hch@lst.de, linux-fsdevel@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
linux-xfs@vger.kernel.org, mhocko@suse.com,
ritesh.list@gmail.com, rvvandan@amazon.com,
stable@vger.kernel.org, surenb@google.com, ziy@nvidia.com
Subject: Re: [PATCH v4] mm/page_alloc: avoid direct compaction for costly __GFP_NORETRY allocations
Date: Mon, 21 Sep 2026 11:35:42 +0200 [thread overview]
Message-ID: <6c94405a-48e4-4842-a096-e2fbbbaa862f@kernel.org> (raw)
In-Reply-To: <aq4L29SBCYwvKrw2@casper.infradead.org>
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@amazon.it>
>> 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@amazon.it/
[2] https://lore.kernel.org/all/alEz4Chf7Ibyg-ZG@casper.infradead.org/
next prev parent reply other threads:[~2026-09-21 9:35 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-04 11:56 [PATCH v4] mm/page_alloc: avoid direct compaction for costly __GFP_NORETRY allocations Salvatore Dipietro
2026-09-04 14:11 ` Vlastimil Babka (SUSE)
2026-09-04 15:08 ` Zi Yan
2026-09-07 7:30 ` Vlastimil Babka (SUSE)
2026-09-09 2:33 ` Zi Yan
2026-09-09 8:51 ` Vlastimil Babka (SUSE)
2026-09-04 16:10 ` Johannes Weiner
2026-09-06 0:42 ` Andrew Morton
2026-09-06 23:05 ` Dave Chinner
2026-09-10 11:46 ` Salvatore Dipietro
2026-09-10 22:00 ` Andrew Morton
2026-09-11 14:30 ` Salvatore Dipietro
2026-09-11 15:59 ` Johannes Weiner
2026-09-16 11:24 ` Vlastimil Babka (SUSE)
2026-09-16 15:58 ` Johannes Weiner
2026-09-16 22:34 ` Andrew Morton
2026-09-16 22:35 ` Andrew Morton
2026-09-19 4:13 ` Matthew Wilcox
2026-09-21 9:35 ` Vlastimil Babka (SUSE) [this message]
2026-09-23 9:25 ` Salvatore Dipietro
2026-09-18 7:05 ` Vlastimil Babka (SUSE)
2026-09-18 21:28 ` Andrew Morton
2026-09-22 8:44 ` Vlastimil Babka (SUSE)
2026-09-21 14:37 ` Johannes Weiner
2026-09-21 14:38 ` [PATCH 1/2] mm: page_alloc: do not give all non-blocking requests reserve access Johannes Weiner
2026-09-21 14:54 ` Matthew Wilcox
2026-09-21 15:58 ` Johannes Weiner
2026-09-22 11:52 ` Vlastimil Babka (SUSE)
2026-09-22 13:56 ` Johannes Weiner
2026-09-24 14:34 ` Vlastimil Babka (SUSE)
2026-09-21 14:39 ` [PATCH 2/2] mm: page_alloc: remove ALLOC_NON_BLOCK from ALLOC_RESERVES Johannes Weiner
2026-09-22 12:06 ` Vlastimil Babka (SUSE)
2026-09-22 14:00 ` Johannes Weiner
2026-09-07 5:54 ` [PATCH v4] mm/page_alloc: avoid direct compaction for costly __GFP_NORETRY allocations Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6c94405a-48e4-4842-a096-e2fbbbaa862f@kernel.org \
--to=vbabka@kernel.org \
--cc=abuehaze@amazon.com \
--cc=akpm@linux-foundation.org \
--cc=alisaidi@amazon.com \
--cc=blakgeof@amazon.com \
--cc=brauner@kernel.org \
--cc=brendan.jackman@linux.dev \
--cc=david@redhat.com \
--cc=dgc@kernel.org \
--cc=dipietro.salvatore@gmail.com \
--cc=dipiets@amazon.it \
--cc=djwong@kernel.org \
--cc=hannes@cmpxchg.org \
--cc=hch@infradead.org \
--cc=hch@lst.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-xfs@vger.kernel.org \
--cc=mhocko@suse.com \
--cc=ritesh.list@gmail.com \
--cc=rvvandan@amazon.com \
--cc=stable@vger.kernel.org \
--cc=surenb@google.com \
--cc=willy@infradead.org \
--cc=ziy@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.