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: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-04 11:56 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-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-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-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®