Re: [PATCH v3] mm/huge_memory: prevent losing PG_waiters in __split_folio_to_order()

[Kiryl Shutsemau <kas@kernel.org>]

On Tue, Nov 10, 2026 at 12:31:21PM +0100, David Hildenbrand (Arm) wrote:
> > This is V3 of the patch. Instead of using prep_compound_page_atomic(),
> > PG_head is written before clear_compound_page(), after which new_head
> > is visible to the reference-less writers.
> 
> I'm confused, how is it then valid to have PG_waiters set concurrently while we
> read new_folio->flags.f?

It is not. The comment names the wrong word: PG_waiters can be set
concurrently on the old head, not on new_folio.

CPU1 and CPU2 below are two faults on a migration entry that points at
new_head. Both take this path:

  do_swap_page()
    migration_entry_wait()
      softleaf_entry_wait_on_locked()
        softleaf_to_folio()
          page_folio()           <-- old head or new_head?
        folio_set_waiters()
          set_bit(PG_waiters)    <-- on that folio's flags

Which one page_folio() returns depends on clear_compound_head():

CPU0: __split_folio_to_order()  CPU1: fault           CPU2: fault

                                1. page_folio()
                                   -> old head
2. flags  = new_folio->flags.f
   flags |= folio->flags.f
                                3. set_bit(PG_waiters)
                                   -> folio->flags.f
4. new_folio->flags.f =
           flags | PG_head
   smp_wmb()
5. clear_compound_head()
                                                      6. page_folio()
                                                         -> new_head
                                                      7. set_bit(PG_waiters)
                                                         -> new_folio->flags.f

2 and 3 race on the old head's flags word.

That is Ye Liu's KCSAN report, and it is benign: PG_waiters is not in
the cloned mask. new_folio->flags.f gets PG_waiters only at 7, after
CPU0's last plain write to it at 4.

So only the folio->flags.f read needs data_race().

The new_folio->flags.f read needs nothing; the READ_ONCE() I had there
in my sketch was not needed.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov