Re: [PATCH v4 10/16] mm/page-flags: check page/folio->private instead of PG_private

["David Hildenbrand (Arm)" <david@kernel.org>]

On 9/14/26 16:44, Zi Yan wrote:
> On 14 Sep 2026, at 9:26, David Hildenbrand (Arm) wrote:
> 
>> On 9/14/26 04:24, Zi Yan wrote:
>>> After the changes of the prior commits, page/folio->private != NULL is now
>>> equivalent to checking PG_private.
>>>
>>> Stop checking PG_private on pages and folios and use page/folio->private
>>> instead, except swapcache and hugetlb folios, because the former uses a
>>> field (swp_entry_t swap) overlapping with ->private and the latter sets its
>>> flags in ->private. Exclude swapcache and hugetlb when the code is meant to
>>> check PG_private only. PG_swapcache and folio->swap.val cannot be set/clear
>>> as a whole, so excluding swapcache with folio_test_swapcache() is not
>>> reliable. Instead, use folio_test_swapbacked(), since PG_swapbacked is
>>> stable when a folio is added to/removed from swapcache. Add a helper,
>>> folio_has_attached_private(), for this check.
>>>
>>> folio_expected_ref_count() can be called without folio lock, so annotate
>>> folio_test_private() with data_race() to avoid triggering race condition
>>> checks. While at it, annotate folio->mapping too. Add data_race()
>>> annotation for other lockless callers too.
>>
>> Shouldn't we just move the data_race() into folio_has_attached_private() and
>> document there, that it can be called without the folio lock as well? Having the
>> data_race in multiple callers looks odd.
> 
> Not all callers call it without holding a lock. Before the patch series,
> it is fine for lockless reads because PagePrivate() is an atomic operation.
> But after the patch series, lockless reads can cause data tearing of
> folio->private. I think I need to add this to avoid the confusion.
Okay, but we should just keep that complexity internally.

E.g., just teach folio_test_private/PagePrivate about the data_race, because
that's exactly what worked before even with data races.

-- 
Cheers,

David