From: Johannes Weiner <hannes@cmpxchg.org>
To: Kiryl Shutsemau <kirill@shutemov.name>
Cc: Harry Yoo <harry@kernel.org>, Vlastimil Babka <vbabka@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
David Hildenbrand <david@kernel.org>,
Suren Baghdasaryan <surenb@google.com>,
Michal Hocko <mhocko@suse.com>,
Brendan Jackman <brendan.jackman@linux.dev>,
Zi Yan <ziy@nvidia.com>, Shakeel Butt <shakeel.butt@linux.dev>,
Usama Arif <usama.arif@linux.dev>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
stable@vger.kernel.org, kernel-team@meta.com
Subject: Re: [PATCH] mm: page_alloc: make defrag_mode retries follow the promoted order
Date: Tue, 6 Oct 2026 10:10:50 +0200 [thread overview]
Message-ID: <20261006081050.GA234057@cmpxchg.org> (raw)
In-Reply-To: <ar5rOHWwc_iPJF6C@thinkstation>
On Fri, Oct 02, 2026 at 10:59:32AM +0100, Kiryl Shutsemau wrote:
> On Wed, Sep 30, 2026 at 10:06:38AM -0400, Johannes Weiner wrote:
> > > > > @@ -4127,6 +4127,31 @@ __alloc_pages_may_oom(gfp_t gfp_mask, unsigned int order,
> > > > > return page;
> > > > > }
> > > > >
> > > > > +/*
> > > > > + * If fallbacks are not permitted (defrag_mode), we either need to
> > > > > + * reclaim space in a block of matching type, or clear out an entire
> > > > > + * block to allow __rmqueue_claim() to convert.
> > > > > + *
> > > > > + * Reclaim by itself is primarily freeing space in movable blocks,
> > > > > + * since that's where the LRU pages live. So this works for movable
> > > > > + * requests, but not for others.
> > > > > + *
> > > > > + * For those, promote the order of reclaim and compaction to help make
> > > > > + * blocks, instead of spinning in reclaim alone unproductively. Retry
> > > > > + * decisions based on the outcome of that work - reclaim progress and
> > > > > + * compaction results - must account for the promotion as well, see
> > > > > + * should_reclaim_retry() and should_compact_retry().
> > > > > + */
> > > > > +static inline unsigned int nofrag_promote_order(unsigned int order,
> > > > > + unsigned int alloc_flags,
> > > > > + const struct alloc_context *ac)
> > > > > +{
> > > > > + if ((alloc_flags & ALLOC_NOFRAGMENT) && ac->migratetype != MIGRATE_MOVABLE)
> > > > > + return max(order, pageblock_order);
> > > > > +
> > > > > + return order;
> > > > > +}
> > > >
> > > > I think we should start distinguishing order and compact/reclaim_order
> > > > in __alloc_pages_slowpath(). Silently overriding it makes it harder to
> > > > follow and easy to make a mistake.
> > >
> > > Agreed, four callers recomputing the same thing is asking for a
> > > mismatch.
> > >
> > > I would rather not grow this patch, it has to go to stable.
> > >
> > > I will look into a cleanup on top: __alloc_pages_slowpath() computes the
> > > promoted order once per iteration and passes it to direct
> > > reclaim/compaction and the two retry helpers next to the request order,
> > > so the helpers stop knowing about defrag_mode.
> >
> > +1
> >
> > All they really need to know is the split into requested order vs
> > production order (reclaim_order, compaction_order).
>
> Will fold it into the fix itself. The cleanup changes the same places as
> the fix. It makes zero sense to keep it separate.
>
> > > > > @@ -4299,7 +4316,7 @@ should_compact_retry(gfp_t gfp_mask, struct alloc_context *ac, int order,
> > > > > /*
> > > > > * Compaction failed. Retry with increasing priority.
> > > > > */
> > > > > - min_priority = (order > PAGE_ALLOC_COSTLY_ORDER) ?
> > > > > + min_priority = (compact_order > PAGE_ALLOC_COSTLY_ORDER) ?
> > > > > MIN_COMPACT_COSTLY_PRIORITY : MIN_COMPACT_PRIORITY;
> > > >
> > > > This would change how hard we try to compact with defrag_mode in direct
> > > > compaction as it won't try compaction with MIN_COMPACT_PRIORITY anymore.
> > > >
> > > > It doesn't make much sense to change that as part of this fix?
> > >
> > > It is a choice between compacting harder and falling back, which
> > > fragments a block.
> > >
> > > It is a judgement call on what defrag_mode means.
> > >
> > > It would also mean that order-0 allocation request promoted to pageblock
> > > can trigger SYNC_FULL compaction. I cannot say I understand the
> > > implications. Will give it a try with the reproducer.
> > >
> > > Johannes, Vlastimil, any comments here?
> >
> > I would leave this one with requested order unless the reproducer
> > disagrees.
>
> I built a reproducer around what we saw in production, which was oomd
> killing the workload on sustained PSI, so PSI and the number of times
> ALLOC_NOFRAGMENT gets dropped are the headline numbers. 32G VM, 8 CPUs:
>
> - fill memory with 4K anon pages in a cgroup, memory.low = fill + 1G,
> so page cache beyond that stays reclaimable;
>
> - pin one page in every pageblock with io_uring registered buffers[1],
> except one block in 50 (2%), so whole blocks can be made but only
> from a few places. Without the pins rc5 does not storm in 180s:
> compaction makes 130-210 blocks per run and every retry loop ends in
> one. The storm needs blocks to be hard to produce, and the pin
> fraction is the knob for that;
>
> - free one page in eight so ~4G sit scattered in movable blocks;
>
> - turn on defrag_mode and run 8 build-like workers on btrfs for 180s:
> write 1-64K files, read earlier ones back, unlink 20%, fdatasync
> every 50 creates.
>
> Kernels: v7.3-rc5; the fix as posted; the fix with the priority floor
> and the COMPACT_SUCCESS retry limit on the requested order. Three runs
> each, mean ± stddev, 2% producible:
>
> rc5 posted requested
> ops/s 4751 ± 684 5181 ± 18 5206 ± 84
> PSI some, mean % 28.7 ± 11.5 22.0 ± 0 22.0 ± 0
> PSI some, peak avg10 42.8 ± 30.7 24.9 ± 0.3 25.0 ± 0.8
> s with some avg10 > 50 3.3 ± 5.8 0 0
> give-ups (NOFRAG off) 0 222 ± 58 277 ± 199
> movable blocks lost 641 ± 13 654 ± 10 652 ± 14
> whole blocks claimed 90 ± 8 81 ± 10 76 ± 8
>
> rc5 stormed in one of its three runs: 3.85M order-9 reclaim runs in
> 180s, PSI at 42% with ten seconds above 50, workers that would not die
> on SIGKILL. The other two were quiet, which matches a workload that
> OOMs regularly rather than always. The fixed kernels never stormed.
Ack. Nice. Thanks for testing it out.
> Between the two floors there is no difference I can measure here: same
> PSI, same throughput, same give-ups, same movable blocks lost, same
> whole blocks produced.
>
> The promoted path is a small part of what the allocator does in this
> workload, 16-22k order-9 reclaim runs against a million order-0 ones for
> page cache, and a few hundred give-ups in 180s.
>
> > There is a risk of defrag_mode self defeating over time by raising the
> > bar for fallbacks but not high enough. Every fallback we let through
> > will make it harder down the line to compact towards that higher bar.
>
> Same movable blocks lost and same whole blocks produced in all three
> kernels, so in this workload the extra SYNC_FULL passes neither produce
> blocks nor save any.
Ok that's good to know.
Without counter indication, I would prefer to keep the tighter
guarantees. ISTR this mattered on some of my ext4 tests in the past
with the buffer locking.
> > There is also a non-zero risk of connecting order-0 request contexts
> > to SYNC compaction which they haven't done before 7e8756d7ad22. But
> > you traced the problem to retrying, not sync compaction itself.
>
> That shows up only when I take the I/O out: tmpfs, every block pinned,
> 1M empty files. Then every exhausted order-0 refill does a whole-zone
> SYNC_FULL pass before it falls back, PSI some runs at 39% against 20%,
> and the churn takes 1.3-2.8x as long, five runs each.
That seems acceptable for the no-hope worst-case behavior.
> What do you prefer here? I don't have strong preference either way.
>
> [1] One thing the pins made me notice. A FOLL_LONGTERM pin migrates
> the page first only for ZONE_MOVABLE, CMA and isolated blocks, see
> folio_is_longterm_pinnable(); a page in a MIGRATE_MOVABLE block in
> ZONE_NORMAL is pinned where it sits, and the block can never be
> made whole for as long as the pin lives. The migration target in
> gup already uses GFP_USER without __GFP_MOVABLE, so a moved page
> lands in a non-movable block.
+1
> Should defrag_mode treat MIGRATE_MOVABLE like ZONE_MOVABLE there and
> move the page out at pin time? Ideally we might want to move it back
> on unpin, but it can be done by compaction too.
Should it even be specific to defrag_mode? I suppose without it, the
poisoning from fallbacks would dominate by a landslide under
pressure. But these pins can mess with compactability long before
becoming capacity-bound.
next prev parent reply other threads:[~2026-10-06 8:10 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-29 17:45 Kiryl Shutsemau
2026-09-29 18:39 ` Harry Yoo
2026-09-30 13:32 ` Kiryl Shutsemau
2026-09-30 14:06 ` Johannes Weiner
2026-10-02 9:59 ` Kiryl Shutsemau
2026-10-06 8:10 ` Johannes Weiner [this message]
2026-10-06 9:13 ` Kiryl Shutsemau
2026-09-29 19:58 ` Andrew Morton
2026-09-30 12:35 ` Kiryl Shutsemau
2026-09-30 20:02 ` Andrew Morton
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=20261006081050.GA234057@cmpxchg.org \
--to=hannes@cmpxchg.org \
--cc=akpm@linux-foundation.org \
--cc=brendan.jackman@linux.dev \
--cc=david@kernel.org \
--cc=harry@kernel.org \
--cc=kernel-team@meta.com \
--cc=kirill@shutemov.name \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=shakeel.butt@linux.dev \
--cc=stable@vger.kernel.org \
--cc=surenb@google.com \
--cc=usama.arif@linux.dev \
--cc=vbabka@kernel.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®