Re: [RFC PATCH 19/57] mm/collapse: install a PMD leaf as the terminal layer

On Mon, Aug 17, 2026 at 12:46 AM Kiryl Shutsemau [off-list ref] wrote:

Fill in the PMD install.  Under the pmd lock, with the pte ptl nested
inside it: verify, detach the table with pmdp_collapse_flush(), deposit a
fresh one and map the leaf.

That is one atomic section, so no pmd_none() window ever exists: faults

What do you mean by "atomic" and "no pmd_none() window ever exists"?
Is that supposed to be with respect to a subset of readers?

While the PMD table spinlock is held, you clear the PMD entry
(pmdp_collapse_flush) and set it to a new value
(map_anon_folio_pmd_nopf). But codepaths that walk page tables don't
take that spinlock unless they already know they're in a THP case.

zap_pmd_range() does not lock the PMD table before checking
pmd_none(), and if that is true, it skips over the PMD. I think this
means that THP collapse can race with MADV_DONTNEED or zap_vma_range()
such that the zap wrongly has no effect?

stay held down at pte level by the migration entries throughout.  It is

But you don't have a migration entry at the PMD level, right?

what lets PMD collapse run under mmap_read like everything else here.

Two things force that nesting, which is the one the tree already uses to

I'm lost, what is "the tree"?

reinstall a table.  A racing zap of a frozen entry takes the pte ptl, so
the verify has to hold it.  And the table must not come apart between
verify and detach, which is the pmd lock's job.

Nothing leaves the section early, aborts included.  An abort only
restores PTEs and would need no pmd-level exclusion of its own, except
that its pte pointer came from pte_offset_map_rw_nolock(), whose caller
must establish that the pmd is stable.

The deposited table is the freshly allocated one, never the table just
detached.  A deposited table has to be quiescent, because whoever
withdraws it frees it immediately with nothing to hold a lockless walker
off first, and a table that has never been reachable is quiescent by
construction.

The detached one is not: GUP-fast and RCU pte walks that read the old PMD
may still be inside it, and on broadcast-TLBI architectures the flush
expels nobody.  Quiescing it would need an IPI, which has nowhere to go
here -- outside the pmd lock it opens the pmd_none() window this design
does not have, inside it is a broadcast under a spinlock.  So the
detached table goes to pte_free_defer(), which holds the free until those
walkers finish.  One transient table page per PMD collapse is the cost.

No anon_vma_lock_write() is taken, unlike the mechanism being replaced:

 - rmap walks on the sources are unreachable, their refcounts frozen and
   their folio locks held from freeze to putback;

To be clear, we can concurrently rmap-walk into the PMD, right?
Because we might be looking at a different folio which was created in
the parent process or something like that? And so a
page_vma_mapped_walk() without PVMW_SYNC might look at stale PTEs?
(Which may or may not be fine, but would not be what this commit
message claims.)

 - non-rmap pte walkers see migration entries;
 - pmd-level observers see either the old table or the leaf, never an
   intermediate;

(or pmd_none(), see above)

+       old_pmd = pmdp_collapse_flush(vma, cand->addr, pmd);
+       old_table = pmd_pgtable(old_pmd);

[...]

+       pgtable_trans_huge_deposit(mm, pmd, cand->deposit);
+       map_anon_folio_pmd_nopf(cand->new_folio, pmd, vma, cand->addr);

[...]

+out_unlock:
+       if (pte_ptl != pmd_ptl)
+               spin_unlock(pte_ptl);
+       pte_unmap(pte);
+       spin_unlock(pmd_ptl);
+
+       /* The deposit balanced the detached table, so the count is already right */
+       if (old_table)
+               pte_free_defer(mm, old_table);

What does old_table contain at this point - migration entries? I
wonder if we should zero all the PTEs inside it before dropping its
spinlock and scheduling its freeing, so that a concurrent
pte_offset_map_rw_nolock()+spin_lock() wouldn't see dangling migration
entries. (I'm not sure if we actually have a codepath that does that,
but even if not, this seems like it might cause problems later.)