mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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.

  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®