From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2FE3A37F731 for ; Mon, 29 Jun 2026 10:26:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782728816; cv=none; b=qobUj9y7+qesTNjW4F/oLDscWFzUWX5gYyRg/hsvgVI+34IVsennVf0tTI5Cp4rF4QMo4WmgsavQj95F30mFSyi4/X7g7FLJn1+aM4ciNXvBnj24o7eat5lqJMU4U2w8DcnhPFsI3I1O34af53/tBm3CTu7OG2t1XW0VDZTMKq8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1782728816; c=relaxed/simple; bh=BhPepSDsUzhI/SUgR/pLODKfXPM0zy/zT67ZNkJsx64=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=b5pI/wg6lUMWLGjyYIYNi6B3fi9rmFjIeEnBu4buUMkYJ7Xjt1VI5mAbUTfSk4rMZ0JDjExD4rAuI96yQyhI1uNoE40BIUX5ebGC5LXbpK7t1D/S2SxZT3srtdgfutbtZYHAqPZxO7Y4HJteXLujLHRzIrLJ4Kg/bP1UavKrM88= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=S9QscAoC; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="S9QscAoC" Received: by smtp.kernel.org (Postfix) with ESMTPSA id C96C01F000E9; Mon, 29 Jun 2026 10:26:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1782728814; bh=sCpWp+WP18a51W0Wds2yLSSvymsUKmeoRVPKKR/GJMo=; h=Date:From:To:Cc:Subject:References:In-Reply-To; b=S9QscAoC5Ej/+BLfSY8duB/nQtuinHR2xPD7Xgb/YeVsinPZihIvVCLkjFKZ4NGiR vgbcVGHETJgafBzt1aqIyWGJa3AbheTOXA77+Gsvck0PbXFaSSM81j93W2QkEXL13Z ziS9ThTsTlgxnxp5yyi6fvcXua6U37FGcLBQRzB5xi7TvFcWew0ykN3j6vcQTT41ea 5d0ynRFR5buOX0RZ5F1pmei717y/09wbkb1BZ+riIERZmXsmt8oZiQ5xmVfsGC9iKJ UJP4299OqZ346RPe1e77LN6x5pzN+ium8+mgzmDtxaLlGPUekoof8PHW+jt8J5XjdD 71NNZBZwKLU1A== Date: Mon, 29 Jun 2026 11:26:44 +0100 From: Lorenzo Stoakes To: "David Hildenbrand (Arm)" Cc: Hui Zhu , Andrew Morton , "Liam R. Howlett" , Vlastimil Babka , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , Kairui Song , Qi Zheng , Shakeel Butt , Barry Song , Axel Rasmussen , Yuanchu Xie , Wei Xu , linux-mm@kvack.org, linux-kernel@vger.kernel.org, Hui Zhu Subject: Re: [PATCH v5] mm: assert exclusive nid/zonenum bits at the page/folio access sites Message-ID: References: <20260625071830.996043-1-hui.zhu@linux.dev> <2c4dd46a-4755-4bf5-8f14-2d73eb356e3e@kernel.org> <7efb57b0-7205-440c-8638-2ec5354aacce@kernel.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <7efb57b0-7205-440c-8638-2ec5354aacce@kernel.org> On Thu, Jun 25, 2026 at 02:08:39PM +0200, David Hildenbrand (Arm) wrote: > On 6/25/26 14:07, Lorenzo Stoakes wrote: > > On Thu, Jun 25, 2026 at 01:53:14PM +0200, David Hildenbrand (Arm) wrote: > >> On 6/25/26 09:18, Hui Zhu wrote: > >>> From: Hui Zhu > >>> > >>> KCSAN reports a data race between page_to_nid()/folio_pgdat() reading > >>> page->flags and folio_trylock()/folio_lock() concurrently doing > >>> test_and_set_bit_lock(PG_locked, ...) on the same word, e.g.: > >>> > >>> BUG: KCSAN: data-race in __lruvec_stat_mod_folio / shmem_get_folio_gfp > >>> > >>> The node id and zone id occupy fixed bit-ranges of page->flags that > >>> are set once at page init and never modified afterwards, so they can > >>> never overlap with the low PG_locked/PG_waiters bits touched by the > >>> folio lock path. > >>> > >>> ASSERT_EXCLUSIVE_BITS(mdf.f, ...) inside memdesc_nid()/memdesc_zonenum() > >>> checks a by-value copy of the flags word, not the actual shared > >>> page->flags/folio->flags being modified concurrently, so it doesn't > >>> reliably assert anything about the real race. Move the assertion to > >>> page_to_nid(), folio_nid(), page_zonenum() and folio_zonenum(), where > >>> flags is dereferenced directly from the page/folio. > >>> > >>> On CONFIG_NUMA=n, NODES_MASK is 0 and the old memdesc_nid() body > >>> folded to a constant, so page->flags/folio->flags was never actually > >>> read. ASSERT_EXCLUSIVE_BITS() is a real runtime check that can't be > >>> folded away, so doing it unconditionally would add a pointless read > >>> of page->flags/folio->flags and a check that can never fire. Keep > >>> page_to_nid()/folio_nid() as plain "return 0" static inline stubs > >>> under CONFIG_NUMA=n instead. > >>> > >>> Signed-off-by: Hui Zhu > >>> Acked-by: David Hildenbrand (Arm) > >>> --- > >>> Changelog: > >>> v5: > >>> According to the comments of Sashiko, guard the ASSERT_EXCLUSIVE_BITS() > >>> calls with #ifndef NODE_NOT_IN_PAGE_FLAGS (for nid) and #if > >>> ZONES_WIDTH != 0 (for zonenum). > >>> According to the comments of David, avoid calling > >>> PF_POISONED_CHECK(page) twice in page_to_nid(). > >>> According to the warning of lkp, switch the CONFIG_NUMA=n > >>> page_to_nid()/folio_nid() stubs from macros to static inline functions. > >>> v4: > >>> According to the comments of Andrew and Sashiko, set > >>> page_to_nid()/folio_nid() as static inline stubs returning 0 > >>> under CONFIG_NUMA=n. > >>> v3: > >>> According to the comments of Andrew and Sashiko, move > >>> ASSERT_EXCLUSIVE_BITS out of memdesc_nid()/memdesc_zonenum() > >>> into the page/folio call sites. > >>> v2: > >>> According to the comments of David, remove useless comments and use > >>> ASSERT_EXCLUSIVE_BITS() in memdesc_nid() instead of data_race() in > >>> page_to_nid(). > >>> > >>> include/linux/mm.h | 23 ++++++++++++++++++++++- > >>> include/linux/mmzone.h | 7 ++++++- > >>> 2 files changed, 28 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/include/linux/mm.h b/include/linux/mm.h > >>> index 485df9c2dbdd..772bd1fc6fe7 100644 > >>> --- a/include/linux/mm.h > >>> +++ b/include/linux/mm.h > >>> @@ -2294,15 +2294,36 @@ static inline int memdesc_nid(memdesc_flags_t mdf) > >>> } > >>> #endif > >>> > >>> +#ifdef CONFIG_NUMA > >>> static inline int page_to_nid(const struct page *page) > >>> { > >>> - return memdesc_nid(PF_POISONED_CHECK(page)->flags); > >>> + const struct page *p = PF_POISONED_CHECK(page); > >>> + > >>> +#ifndef NODE_NOT_IN_PAGE_FLAGS > >>> + ASSERT_EXCLUSIVE_BITS(p->flags, NODES_MASK << NODES_PGSHIFT); > >>> +#endif > >>> + return memdesc_nid(p->flags); > >>> } > >>> > >>> static inline int folio_nid(const struct folio *folio) > >>> { > >>> +#ifndef NODE_NOT_IN_PAGE_FLAGS > >>> + ASSERT_EXCLUSIVE_BITS(folio->flags, > >>> + NODES_MASK << NODES_PGSHIFT); > >>> +#endif47 > >> > >> This is getting ugly, really. We're leaking implementation details from > >> memdesc_nid() into folio_nid(). > >> > >> Maybe just turn memdesc_nid() into a macro where we can just do that check > >> internally? Not the best thing in this world, but better than this here. > > > > Could also do: > > > > if (!IS_ENABLED(NODE_NOT_IN_PAGE_FLAGS)) > > ASSERT_EXCLUSIVE_BITS(folio->flags, > > NODES_MASK << NODES_PGSHIFT); > > > > But not sure if it's that much better. > > It's still making an assumption of what the memdesc function we're calling will do. Ack, yeah we should avoid having implicit assumptions as to use here! > > -- > Cheers, > > David Cheers, Lorenzo