On Tue, Aug 25, 2026 at 06:52:17PM +0200, Jann Horn wrote: > 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?
Yes, the race with MADV_DONTNEED you point out is real. Thanks for flagging this. The idea I want to explore is installing a pmd_mkinvalid() entry rather than none in pmdp_collapse_flush(), so walkers have to serialize on the pmd lock. But it was a long day, I am not sure if the idea is sane. > > 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? Right. They are all at pte level, in the table being detached. What keeps a fault out during the install is the pmd lock: pmd_install() takes it before it can put a table back. I will fix wording once the pmd_none() question above is settled. > > 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"? Upstream. try_collapse_pte_mapped_thp() and retract_page_tables() nest the pte ptl inside the pmd lock the same way. I will update wording here. > > - 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.) Again, wording should be better here. A walk can reach the range, but it cannot act on a source: that takes the folio lock, which the round holds from freeze to putback. > > + /* 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? Yes. I will clear it under the ptl before it goes. Not for the pattern you name, though: a pte_offset_map_rw_nolock() caller owes a pmd_same() recheck and page_vma_mapped_walk() does it. The exposure is the plain pte_offset_map() callers -- pagewalk.c, memory-failure.c, swap_state.c and a dozen more -- none of which rechecks the pmd. -- Kiryl Shutsemau / Kirill A. Shutemov
