[PATCH] mm/huge_memory: use READ_ONCE() when cloning folio flags during split

[PATCH] mm/huge_memory: use READ_ONCE() when cloning folio flags during split

[Ye Liu]

From: Ye Liu <liuye@kylinos.cn>

The head folio's flags are read as a whole word to clone specific bits
into tail folios during a THP split.  This plain read can race with
concurrent atomic flag modifications such as folio_set_waiters()
(PG_waiters), as reported by KCSAN:

  BUG: KCSAN: data-race in __split_huge_page_tail / migration_entry_wait_on_locked

Signed-off-by: Ye Liu <liuye@kylinos.cn>
---
 mm/huge_memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index bd44d58e8684..aac91c972ced 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3731,7 +3731,7 @@ static void __split_folio_to_order(struct folio *folio, int old_order,
 		 * PG_anon_exclusive (-> PG_mappedtodisk) for these here.
 		 */
 		new_folio->flags.f &= ~PAGE_FLAGS_CHECK_AT_PREP;
-		new_folio->flags.f |= (folio->flags.f &
+		new_folio->flags.f |= (READ_ONCE(folio->flags.f) &
 				((1L << PG_referenced) |
 				 (1L << PG_swapbacked) |
 				 (1L << PG_swapcache) |

---
base-commit: 6d0933ae9d46674a6ea63a87e9e9ab4e662670c9
change-id: 20261029-data_race_huge_mem-42f3a6005c43

Best regards,
-- 
Ye Liu <ye.liu@linux.dev>

Re: [PATCH] mm/huge_memory: use READ_ONCE() when cloning folio flags during split

[Barry Song]

On Thu, Oct 29, 2026 at 3:41 PM Ye Liu <ye.liu@linux.dev> wrote:
>
> From: Ye Liu <liuye@kylinos.cn>
>
> The head folio's flags are read as a whole word to clone specific bits
> into tail folios during a THP split.  This plain read can race with
> concurrent atomic flag modifications such as folio_set_waiters()
> (PG_waiters), as reported by KCSAN:
>
>   BUG: KCSAN: data-race in __split_huge_page_tail / migration_entry_wait_on_locked
>
> Signed-off-by: Ye Liu <liuye@kylinos.cn>
> ---

I assume the race is harmless, but silencing KCSAN is still worthwhile.

Reviewed-by: Barry Song <baohua@kernel.org>

Re: [PATCH] mm/huge_memory: use READ_ONCE() when cloning folio flags during split

[David Hildenbrand (Arm)]

On 10/29/26 10:08, Barry Song wrote:
> On Thu, Oct 29, 2026 at 3:41 PM Ye Liu <ye.liu@linux.dev> wrote:
>>
>> From: Ye Liu <liuye@kylinos.cn>
>>
>> The head folio's flags are read as a whole word to clone specific bits
>> into tail folios during a THP split.  This plain read can race with
>> concurrent atomic flag modifications such as folio_set_waiters()
>> (PG_waiters), as reported by KCSAN:
>>
>>   BUG: KCSAN: data-race in __split_huge_page_tail / migration_entry_wait_on_locked
>>
>> Signed-off-by: Ye Liu <liuye@kylinos.cn>
>> ---
> 
> I assume the race is harmless, but silencing KCSAN is still worthwhile.

Isn't there a way we could lose the PG_waiters flag?

-- 
Cheers,

David

Re: [PATCH] mm/huge_memory: use READ_ONCE() when cloning folio flags during split

[Zi Yan]

On Thu Oct 29, 2026 at 6:47 AM EDT, David Hildenbrand (Arm) wrote:
> On 10/29/26 10:08, Barry Song wrote:
>> On Thu, Oct 29, 2026 at 3:41 PM Ye Liu <ye.liu@linux.dev> wrote:
>>>
>>> From: Ye Liu <liuye@kylinos.cn>
>>>
>>> The head folio's flags are read as a whole word to clone specific bits
>>> into tail folios during a THP split.  This plain read can race with
>>> concurrent atomic flag modifications such as folio_set_waiters()
>>> (PG_waiters), as reported by KCSAN:
>>>
>>>   BUG: KCSAN: data-race in __split_huge_page_tail / migration_entry_wait_on_locked
>>>
>>> Signed-off-by: Ye Liu <liuye@kylinos.cn>
>>> ---
>> 
>> I assume the race is harmless, but silencing KCSAN is still worthwhile.

data_race() would work too.

>
> Isn't there a way we could lose the PG_waiters flag?

The PG_waiters on the original folio/the original head page is not lost,
but there is a chance PG_waiters is lost in the new head, because
prep_compound_page() uses __SetPageHead() and can ignore the atomic set
of PG_waiters.

Any one reaches softleaf_entry_wait_on_locked() via a PTE migration
entry using migration_entry_wait() can cause the PG_waiters loss,
because softleaf_to_folio() in softleaf_entry_wait_on_locked() uses
page_folio() and clear_compound_head() in the split code makes
page_folio() to return the new head. Then PG_waiters is atomically
writtne to the new head's flag but prep_compound_page()'s __SetPageHead()
can miss overwrite the PG_waiters.

Code interleaving is shown below:



● CPU0: __split_folio_to_order()                 CPU1: do_swap_page()
  =====================================================================================
  1. clear_compound_head(new_head)
     new_head->compound_info = 0
     (page_folio(new_head) == new_head now)

                                                 2. migration_entry_wait()
                                                      softleaf_to_folio(entry)
                                                        page_folio(new_head) -> new_head
                                                        (not the old head any more)

  3. prep_compound_page(new_head, new_order)
       __SetPageHead(new_head)
         tmp = new_head->flags.f          <-- load; PG_waiters == 0 in tmp

                                                 4. folio_set_waiters(new_head)
                                                      set_bit(PG_waiters, &new_head->flags.f)
                                                      enqueue on waitqueue, sleep

  5.     new_head->flags.f = tmp | PG_head <-- store; overwrites step 4,
                                               PG_waiters == 0 again

  Later, folio_unlock(new_folio) sees no PG_waiters, skips folio_wake_bit(), CPU1 never wakes.


In addition to do_swap_page(), hmm_vma_handle_pte() and move_pages_pte()
also uses migration_entry_wait() and have the same PG_waiters loss
issue.

Solutions can be:

1. Use atomic SetPageHead() in prep_compound_page().
2. Do __SetPageHead() before clear_compound_head() and some additional
   memory barrier complication LLM tells me to do.

3. (Not good) Set PG_waiters for all after-split folios to mask the issue
4. (Worse) softleaf_entry_wait_on_locked() needs to grab a refcount on
   the folio.
5. (Worst) softleaf_to_folio() should wait until the folio is unfrozen.


-- 
Best Regards,
Yan, Zi

Re: [PATCH] mm/huge_memory: use READ_ONCE() when cloning folio flags during split

[David Hildenbrand (Arm)]

On 10/29/26 15:31, Zi Yan wrote:
> On Thu Oct 29, 2026 at 6:47 AM EDT, David Hildenbrand (Arm) wrote:
>> On 10/29/26 10:08, Barry Song wrote:
>>>
>>> I assume the race is harmless, but silencing KCSAN is still worthwhile.
> 
> data_race() would work too.
> 
>>
>> Isn't there a way we could lose the PG_waiters flag?
> 
> The PG_waiters on the original folio/the original head page is not lost,

Ah, so it's not on arbitrary tail pages, just the old head.

> but there is a chance PG_waiters is lost in the new head, because
> prep_compound_page() uses __SetPageHead() and can ignore the atomic set
> of PG_waiters.

Fun.
...>
> Solutions can be:
> 
> 1. Use atomic SetPageHead() in prep_compound_page().

That sounds like the right thing to do?

-- 
Cheers,

David