On Mon, Aug 17, 2026 at 12:46 AM Kiryl Shutsemau <[email protected]> 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.)
