Re: [PATCH] mm: avoid KCSAN false positive in page_to_nid()

[David Hildenbrand (Arm)]

On 6/23/26 09:41, Hui Zhu wrote:
> From: Hui Zhu <zhuhui@xxxxxxxxxx>
> 
> KCSAN reports a data race between page_to_nid() reading page->flags
> and folio_trylock()/folio_lock() doing test_and_set_bit_lock(PG_locked,
> ...) on the same word from another CPU, e.g.:
> 
>   BUG: KCSAN: data-race in __lruvec_stat_mod_folio / shmem_get_folio_gfp
> 
> The node id occupies a fixed, high bit-range of page->flags that is
> set once when the page is initialized and never modified afterwards,
> so it can never overlap with the low PG_locked/PG_waiters bits touched
> by the folio lock path. The race is therefore harmless: page_to_nid()
> always returns a consistent value regardless of how the read
> interleaves with the lock bit ops.
> 
> Wrap the flags read with data_race() to tell KCSAN this race is
> intentional and benign, consistent with how page->page_type is
> already annotated for similar packed-field accesses.
> 
> Signed-off-by: Hui Zhu <zhuhui@xxxxxxxxxx>
> ---
>  include/linux/mm.h | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 485df9c2dbdd..122d3b39369f 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2296,7 +2296,14 @@ static inline int memdesc_nid(memdesc_flags_t mdf)
>  
>  static inline int page_to_nid(const struct page *page)
>  {
> -	return memdesc_nid(PF_POISONED_CHECK(page)->flags);
> +	/*
> +	 * The node id occupies a fixed high bit-range of page->flags
> +	 * that is set once at page init and never changed afterwards.
> +	 * It cannot overlap with the low PG_locked/PG_waiters bits
> +	 * that folio_lock()/folio_unlock() concurrently update, so
> +	 * this data race is benign.
> +	 */

Do we really need this excessive comment?

> +	return memdesc_nid(data_race(PF_POISONED_CHECK(page)->flags));

In memdesc_zonenum() we use ASSERT_EXCLUSIVE_BITS.

Can we do the same here inside memdesc_nid?

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 69daeeab7fe8f..76d3bb54be844 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2290,6 +2290,7 @@ int memdesc_nid(memdesc_flags_t mdf);
 #else
 static inline int memdesc_nid(memdesc_flags_t mdf)
 {
+       ASSERT_EXCLUSIVE_BITS(mdf.f, NODES_MASK << NODES_PGSHIFT);
        return (mdf.f >> NODES_PGSHIFT) & NODES_MASK;
 }
 #endif


-- 
Cheers,

David