Re: [RFC PATCH 00/57] mm/collapse: rebuild collapse on migration primitives

On 8/17/26 00:45, Kiryl Shutsemau wrote:

From: "Kiryl Shutsemau (Meta)" <kas@kernel.org>

Yes, I know, this is a lot of changes. But I'm happy with the overall state
of the patchset and the only reason I tag it as RFC is that it is tricky
to get 57 patches upstream.

I wanted to give a view of the end state first. I will suggest a possible
way to split it below.

I would appreciate any feedback.

Replying here on the overall design first before reading into the other
discussions (and process related topics).

TL;DR
=====

This replaces khugepaged's anonymous collapse with an engine that
can collapse sub-PMD ranges. It is built around migration entries and
frozen folios instead of heavy locking and isolation, aiming for better
scalability and less disruption to the workload being collapsed.

I recall us discussing something around using some PTE/PMD markers (e.g.,
migration entries) in the past.

One thing that needed care is handling concurrent MADV_DONTNEED + faultin after
dropping relevant locks.

Why
===

mTHP collapse landed in khugepaged in 7.2 and I was glad to see it.  We
at Meta run arm64 with 64K base pages, where a PMD is 512M: PMD-order THP
is of limited use at that size, and mTHP is exactly what we want.

Right, as the first step, we decided to go for the simpler and minimally
intrusive approach of using the existing mechanism that always operates on PMD
ranges, keeping using the existing pmdp_collapse_flush()-based mechanism and
locking in place.

That's why anything more elaborate will require significantly more LOC :)

It turned out not to help us.

khugepaged only ever looks at PMD-aligned windows, and it is not an easy
limitation to lift.

I recall we discussed some simpler way to make this work with VMAs that don't
fully span PMDs: I think write-locking VMAs (+ rmap) that cover the PMD was
discussed as a low-hanging fruit, such that other page table walkers would not
suddenly stumble over the temporarily removed page table.

So the mmap_write_lock() + vma write-locks prevents concurrent mmap+page faults
and the rmap locks prevent concurrent rmap walks.

There were discussions on the impact when a PMD spans many VMAs, and I think one
conclusion was that such scenarios are likely not worth considering (e.g., 512
VMAs in a single PMD, all with different rmap locks; your workload sucks already).

Fixing the alignment is a one-line change, but what it feeds assumes the
PMD everywhere that matters: collapse_huge_page() clears and flushes the
whole PMD whatever order it is collapsing, installs a PMD leaf because
that is the only thing it can produce, and keeps everyone out with
mmap_write_lock, anon_vma_lock_write() and an IPI broadcast while it
does.

Which is why hugepage_vma_revalidate() demands that the VMA span the
whole PMD even for an mTHP order -- "we'd need to lock all VMAs in the
PMD range to support this", as the comment there puts it.  A PMD-granular
operation is only safe when one VMA owns the PMD, and that is exactly the
restriction in the way.  The alignment is the symptom; the PMD is the
design.

I disagree with "A PMD-granular operation is only safe when one VMA owns the
PMD". It's safe when all page table walkers can be stopped (see above).

So both roots have to go.

Design
======

The old mechanism holds the address space still because it has nothing
else stopping the sources from moving under the copy.  The new engine
makes the sources themselves inert instead, with the two barriers
migration already uses, raised in that order:

  1. migration entries replace the source PTEs.  Faults and GUP-slow
     now wait on the source folio's lock, which is taken before the
     first entry becomes visible.
  2. the source folio's refcount is frozen to its expected value.
     GUP-fast, pfn walkers, reclaim, compaction and memory-failure all
     fail folio_try_get() and back off.

Between the two, nothing can reach a source, so the copy runs with no
lock held at all -- and the address space is left alone while it does.

Right. Concurrent MADV_DONTNEED can zap migration PTEs and other faults even
re-fault fresh anon folios. So that must be detected before replacing migration
entries again I guess.

What that removes from every collapse path:

  mmap_write_lock              -> mmap_read
  anon_vma_lock_write()        -> nothing: an rmap walk needs the folio
                                  locked, and the engine holds that lock
                                  from freeze to putback
  tlb_remove_table_sync_one()  -> nothing: one ranged flush per round
  LRU isolation                -> nothing: sources are inert in place

Not sure how you handle PMD collapse. I recall problems with migration entries
on the PMD level for non-folio things (we discussed something along these lines
also in the past).

Working in windows rather than whole PMDs takes care of the other root.
A sub-PMD window is collapsed under the page table lock, so a collapse
disturbs only the window it collapses, and each candidate is validated

I recall us discussing that holding the PT lock for a longer collapse operation
(especially on 64k) is problematic. But I don't get all the details from your
description here.

at its own order -- a window need only fit its own VMA.  A PMD-order
candidate still has to own the whole PMD, which is the old rule kept
where it is still needed.

Candidates are carried through the passes a batch at a time rather than
one window at a time, so a round pays for its flush and its lock
acquisitions once.

Now I am starting to feel that there are too many changes packed in a single
series :)

With the barriers holding the sources still, which read lock the engine
takes stops being part of the design.  A round works inside a single
VMA, so patches 43-49 switch it from mmap_read to per-VMA locking: an
mmap_write elsewhere in the mm then stops waiting for a collapse that
has nothing to do with it.  That block is the only part of the series
that needs per-VMA locking to be unconditional, and it is a separate
dependency (see below); everything before it runs under mmap_read and
does not care.

Patch 7 sketches the engine as a comment naming every pass, what lock it
takes and what it may sleep on; the details are there rather than here.

What falls out beyond the lock diet:

 - mTHP collapse in VMAs smaller than a PMD, which is the arm64 case
   above: a 2M VMA on an arm64/64K machine collapses nothing today at
   any order, and collapses to mTHP here.
 - Hole and zeropage population at every order, so partially populated
   windows collapse to mTHP under the same max_ptes_none policy as PMD.
 - Sources come in spans -- any stretch of consecutive PTEs mapping
   consecutive pages of one folio -- so partially mapped and scrambled
   compound sources (the PTE-mapped-THP re-collapse class) work at
   every order.
 - A table that cannot become one huge page still yields the largest
   windows inside it, where before a single disqualified PTE gave up
   the whole table.

Reading the series
==================

57 patches is a lot to land on a list.  They go in blocks:

  1-6    helpers and shared state: pte_folio(), pte_none_or_zero(),
         mm/collapse.h, and the policy that replaces asking whether
         khugepaged started a collapse
  7-8    the engine's shape: entry points, a call-tree comment naming
         every pass, and the scan filled in
  9-23   the collapse half, top down: the round frame, then each pass
         in turn, then selection and the retry store

I fail to parse this sentence.

  24     per-candidate tracing, before the switch takes the old
         tracepoints away
  25-28  the switch: point the anon path at the engine, widen coverage
         to sub-PMD VMAs, delete the mechanism it replaces
  29-35  move what is left of collapse out of khugepaged.c, and
         MADV_COLLAPSE into madvise.c
  36-42  tracing: the engine's own events and trace header
  43-49  per-VMA locking, and the mm reference that makes it safe
  50-56  selftests for what the engine can now do
  57     MAINTAINERS

The two patches worth reading first if you read nothing else are 7 (the
design, as a comment naming the whole call tree) and 16 (the freeze,
which is where the safety argument lives).

A possible split, if that helps:

  1-2    two mm helpers, pte_folio() and pte_none_or_zero().  Both
         convert callers outside collapse and are useful on their own
  3-27   the engine and the switch-over.  This is the smallest unit
         that does anything: stop earlier and the tree carries an
         engine nothing calls
  28     remove the mechanism the engine replaces
  29-42  moving what is left of collapse out of khugepaged.c, and the
         engine's own tracepoints
  43-49  per-VMA locking
  50-57  selftests and MAINTAINERS

Keeping the removal separate leaves both engines in the tree with only
the new one reachable, so the switch can be reverted on its own if
something turns up.  The old mechanism is already carried that way for
three patches inside the series, so this costs nothing but 975 lines of
unreferenced code until 28 lands.  That safety net only lasts until the
blocks after it land, though: once collapse has moved out of
khugepaged.c and the locking has changed, reverting the switch no longer
gives back a working old engine.

[...]

Performance
===========

Measuring khugepaged is awkward.  It is a background daemon, so what
matters is what a workload feels while it runs, not what the daemon
reports about itself -- and the usual coverage instrument is no help
below the PMD: smaps AnonHugePages only counts PMD-order folios, so it
reads zero however much mTHP has been collapsed.

So I wrote "perf bench mem usemem" for this.  It touches a region while
khugepaged works on it and reports the workload's own latency
percentiles and throughput, against per-size counters that can see
sub-PMD folios.  The branch is above; it is unposted and not a
dependency.

Something more realistic might be running some workload in a VM whereby the VM
is getting collapsed by khugepaged.

[...]

There may be a way out -- a PMD migration entry over the table during
the window, so the CPU never caches a walk to shoot down -- but that
means teaching every pmd-level walker a new kind of entry, and I have
not tried it.

I think we discussed that in the past and it's absolutely nasty.

[...]

Size
====

mm/ grows by 1915 lines net: 4475 added against 2560 deleted.

That's quite a lot for something that reads like a cleanup at first.


Okay, let me read the other discussions.

-- 
Cheers,

David