Re: [PATCHv7 13/18] mm: Drop fake head checks

[Kiryl Shutsemau]

On Tue, Oct 27, 2026 at 12:00:28PM +0100, Peter Zijlstra wrote:
> On Tue, Oct 27, 2026 at 12:37:13AM +0100, David Hildenbrand (Arm) wrote:
> > On 10/27/26 00:25, Kiryl Shutsemau wrote:
> > > On Mon, Oct 26, 2026 at 06:45:48PM +0100, David Hildenbrand (Arm) wrote:
> > >> On 2/27/26 20:42, Kiryl Shutsemau (Meta) wrote:
> > >>> From: Kiryl Shutsemau <kas@xxxxxxxxxx>
> > >>>
> > >>> With fake head pages eliminated in the previous commit, remove the
> > >>> supporting infrastructure:
> > >>>
> > >>>   - page_fixed_fake_head(): no longer needed to detect fake heads;
> > >>>   - page_is_fake_head(): no longer needed;
> > >>>   - page_count_writable(): no longer needed for RCU protection;
> > >>>   - RCU read_lock in page_ref_add_unless(): no longer needed;
> > >>
> > >> I have an AI report that removing RCU read-lock was wrong.
> > > 
> > > Ughh. Yes, it is a real bug.
> > > 
> > > My reasoning in the commit message was wrong. Nothing stops a
> > > speculative get from writing to one after the remap. On x86 even a
> > > failing cmpxchg faults there.
> > > 
> > > The fix I have in mind is to let the atomic fault.
> > > 
> > > If the update faults, the struct page is a read-only HVO tail with a
> > > zero refcount, so the get just fails. An exception table entry on the
> > > cmpxchg handles it. No RCU on either side, and the fast path stays the
> > > same lock cmpxchg loop.
> > > 
> > > Only x86, riscv and loongarch have HVO. Below is x86 only, to show the
> > > idea; riscv and loongarch need the same primitive in their LL/SC
> > > fetch_add_unless().
> > > 
> > > Lightly tested on x86 by hitting a real HVO tail page.
> > > 
> > > Does it look sane?
> > 
> > I'm sure x86 folks will have an opinion :) . Let me CC them whether something
> > like that would be acceptable.
> 
> This looks quite terrible, doubly so because I have no idea what its
> trying to fix. Please try and justify this insanity.

Fair enough. Here's some context.

HVO (HugeTLB Vmemmap Optimization) frees most of the memmap of a hugetlb
folio by remapping the struct pages after the first vmemmap page to a
shared page, mapped read-only.

Every lockless get of a page reference (e.g. page cache lookup, PFN
scanners) relies on page_ref_add_unless_zero(), which is
atomic_add_unless(&page->_refcount, nr, 0).

The page can be freed and reused between finding it and the get; the
zero refcount is what makes the get fail. The race:

  lockless get                        other CPU
  c = atomic_read(&page->_refcount)
  c != 0
                                      page freed
                                      page allocated as a hugetlb tail
                                      HVO remaps it read-only
  try_cmpxchg(&page->_refcount, &c, c + nr)
    -> write fault on read-only vmemmap

The refcount is zero by then and the compare fails, but LOCK CMPXCHG
writes anyway.

Before v7.1, HVO mapped the tail vmemmap pages to the head vmemmap page
of the same folio. The first struct page in each of them read as the
head, with PG_head and the head's refcount: a fake head. A get on a fake
head of a folio in use always tried the write, not only in a race, so
page_ref_add_unless() checked for fake heads before the atomic.

Back then, the race above was closed by bd225530a4c7: rcu_read_lock()
around the read and the cmpxchg, and synchronize_rcu() in HVO before the
remap. The refcounts are already zero by then, so the get either
finishes its cmpxchg before the remap or reads zero and bails.

Since v7.1, tails encode the head as a mask instead of a pointer, so
tails of the same order are identical and HVO maps them to one page of
real tails, shared by the zone. Fake heads are gone, and every read-only
struct page is a tail with a zero refcount. The race is not gone, but
32c440d67e6c and 01b1d0ffb6f7 dropped the RCU on both sides anyway, on
the wrong belief that it was only there for fake heads. That is what
David reported.

One fix is to put the RCU back. It works, but it means rcu_read_lock()
around every get that goes through page_ref_add_unless_zero(), GUP-fast
included, and synchronize_rcu() on hugetlb allocation and freeing,
including surplus allocation at fault time. So I am exploring other
options.

The fixup is one of them. With the new layout, a fault on that CMPXCHG
can only be a write to an HVO tail, so the refcount is zero and the get
fails, and the fault only happens in the window above. With GCC the loop
compiles to the same code as before; clang adds a SETE/TEST per call
site.

Writing this, I just realized that we have another option -- map the
shared tail page writable. A failing LOCK CMPXCHG writes back the value
it read, and the get never changes a zero refcount, so the race becomes
harmless. The cost is the protection: the page is shared by every
hugetlb folio of that order in the zone, so a stray write to any tail
silently corrupts all of them instead of oopsing.


And yes, I know, HVO is an ugly hack, like most of hugetlb. But it
works, and it gives back about 16MB of memmap for every 1GB hugetlb
page.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov