Re: [PATCH v3 09/14] mm/page-flags: check page/folio->private instead of PG_private

[Zi Yan]

On Tue Sep 8, 2026 at 12:56 PM EDT, David Hildenbrand (Arm) wrote:
> On 9/8/26 04:56, 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.
>> 
>> 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.
>> 
>> folio_set/clear_private() and Set/ClearPagePrivate() become no-ops.
>> PG_private is no longer checked at page free time.
>> 
>> Remove KPF_PRIVATE since PG_private is no longer used.
>> 
>> Assisted-by: Claude:claude-opus-4-8
>> Assisted-by: Codex:gpt-5
>> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
>> To: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>> To: David Hildenbrand <david@xxxxxxxxxx>
>> To: Steven Rostedt <rostedt@xxxxxxxxxxx>
>> To: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
>> To: Lorenzo Stoakes <ljs@xxxxxxxxxx>
>> To: "Matthew Wilcox (Oracle)" <willy@xxxxxxxxxxxxx>
>> To: Jan Kara <jack@xxxxxxx>
>> To: Johannes Weiner <hannes@xxxxxxxxxxx>
>> Cc: "Liam R. Howlett" <liam@xxxxxxxxxxxxx>
>> Cc: Vlastimil Babka <vbabka@xxxxxxxxxx>
>> Cc: Mike Rapoport <rppt@xxxxxxxxxx>
>> Cc: Suren Baghdasaryan <surenb@xxxxxxxxxx>
>> Cc: Michal Hocko <mhocko@xxxxxxxx>
>> Cc: Mathieu Desnoyers <mathieu.desnoyers@xxxxxxxxxxxx>
>> Cc: Zi Yan <ziy@xxxxxxxxxx>
>> Cc: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>
>> Cc: Nico Pache <nico.pache@xxxxxxxxx>
>> Cc: Ryan Roberts <ryan.roberts@xxxxxxx>
>> Cc: Dev Jain <dev.jain@xxxxxxx>
>> Cc: Barry Song <baohua@xxxxxxxxxx>
>> Cc: Lance Yang <lance.yang@xxxxxxxxx>
>> Cc: Usama Arif <usama.arif@xxxxxxxxx>
>> Cc: Matthew Brost <matthew.brost@xxxxxxxxx>
>> Cc: Joshua Hahn <joshua.hahnjy@xxxxxxxxx>
>> Cc: Rakie Kim <rakie.kim@xxxxxx>
>> Cc: Byungchul Park <byungchul@xxxxxx>
>> Cc: Gregory Price <gourry@xxxxxxxxxx>
>> Cc: Ying Huang <ying.huang@xxxxxxxxxxxxxxxxx>
>> Cc: Alistair Popple <apopple@xxxxxxxxxx>
>> Cc: Qi Zheng <qi.zheng@xxxxxxxxx>
>> Cc: Shakeel Butt <shakeel.butt@xxxxxxxxx>
>> Cc: Kairui Song <kasong@xxxxxxxxxxx>
>> Cc: Axel Rasmussen <axelrasmussen@xxxxxxxxxx>
>> Cc: Yuanchu Xie <yuanchu@xxxxxxxxxx>
>> Cc: Wei Xu <weixugc@xxxxxxxxxx>
>> Cc: linux-kernel@xxxxxxxxxxxxxxx
>> Cc: linux-fsdevel@xxxxxxxxxxxxxxx
>> Cc: linux-mm@xxxxxxxxx
>> Cc: linux-trace-kernel@xxxxxxxxxxxxxxx
>> ---
>
> [...]
>
>>  		/* !PageAnon && !swapper_space */
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index 40d3f1b48a74c..9348ebf9de882 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -978,7 +978,8 @@ static void folio_check_dirty_writeback(struct folio *folio,
>>  	*writeback = folio_test_writeback(folio);
>>  
>>  	/* Verify dirty/writeback state if the filesystem supports it */
>> -	if (!folio_test_private(folio))
>> +	if (!(folio_test_private(folio) && !folio_test_swapbacked(folio) &&
>> +	      !folio_test_hugetlb(folio)))
>
> Now that I read that check ... a fourth time? a fifth? I stopped counting :)
>
> Should we have a (in light of folio_attach_private()):
>
> static inline bool folio_has_attached_private(const struct folio *folio)
> {
> 	/* Careful, we might get called on unlocked folios. */
> 	if (!data_race(folio->private))
> 		return false;
> 	/* On some folios ->private is used for different purposes. */
> 	return !folio_test_swapbacked(folio) && !folio_test_hugetlb(folio);
> }
>
> Of course, adding some nice documentation what having attached private means?

Will squash the next one into this one and use
folio_has_attached_private() instead of folio_test_fs_private(), like we
discussed in Patch 10.



-- 
Best Regards,
Yan, Zi