Re: [PATCH v4 10/16] mm/page-flags: check page/folio->private instead of PG_private
From: "David Hildenbrand (Arm)" <david@kernel.org>
Date: 2026-09-14 13:26:44
Also in:
linux-fsdevel, linux-mm, lkml
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.
folio_set/clear_private() and Set/ClearPagePrivate() become no-ops. PG_private is no longer checked at page free time.
You can mention here that we will clean this part up next, to remove these helpers entirely. [...]
quoted hunk ↗ jump to hunk
} /* One reference per page table mapping. */diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h index 462e89e055485..08988877331ba 100644 --- a/include/linux/page-flags.h +++ b/include/linux/page-flags.h@@ -577,7 +577,23 @@ FOLIO_FLAG(swapbacked, FOLIO_HEAD_PAGE) * for its own purposes. * - PG_private and PG_private_2 cause release_folio() and co to be invoked */ -PAGEFLAG(Private, private, PF_ANY) + +static __always_inline bool folio_test_private(const struct folio *folio) +{ + return folio->private; +} + +static __always_inline int PagePrivate(const struct page *page) +{ + return !!page->private; +} + +/* no-ops during transition */ +static __always_inline void folio_set_private(struct folio *folio) { } +static __always_inline void folio_clear_private(struct folio *folio) { } +static __always_inline void SetPagePrivate(struct page *page) { } +static __always_inline void ClearPagePrivate(struct page *page) { } + FOLIO_FLAG(private_2, FOLIO_HEAD_PAGE) /* owner_2 can be set on tail pages for anon memory */@@ -1169,7 +1185,7 @@ static __always_inline void __ClearPageAnonExclusive(struct page *page) */ #define PAGE_FLAGS_CHECK_AT_FREE \ (1UL << PG_lru | 1UL << PG_locked | \ - 1UL << PG_private | 1UL << PG_private_2 | \ + 1UL << PG_private_2 | \ 1UL << PG_writeback | 1UL << PG_reserved | \ 1UL << PG_active | \ 1UL << PG_unevictable | __PG_MLOCKED | LRU_GEN_MASK)@@ -1193,8 +1209,28 @@ static __always_inline void __ClearPageAnonExclusive(struct page *page) (0xffUL /* order */ | 1UL << PG_has_hwpoisoned | \ 1UL << PG_large_rmappable | 1UL << PG_partially_mapped) -#define PAGE_FLAGS_PRIVATE \ - (1UL << PG_private | 1UL << PG_private_2) +/** + * folio_has_attached_private - check if the folio has private data attached + * @folio: The folio to check. + * + * Use this in code that may encounter swapcache or hugetlb folios but only + * wants to detect attached private data. Swapcache stores swp_entry_t in + * folio->swap, a union with folio->private, and hugetlb stores its own flags + * in folio->private; both are excluded.
This is an implementation detail that is irrelevant for callers. Move that above the actual checks in the code below.
+ * + * NOTE: For swapcache, folio->swap.val PG_swapcache are not set as a whole, + * so folio_test_swapcache() is not reliable to exclude swapcache. + * Use folio_test_swapbacked() instead, since it remains set when a folio is + * added to/removed from swapcache.
Dito, a caller of this function doesn't care.
+ * + * Return: true if folio->private is set and the folio is neither swapcache + * nor hugetlb.
Can we remove these implementation details here instead and talk about "true if the folio has private data attached" ?
+ */
+static inline bool folio_has_attached_private(const struct folio *folio)
+{
+ return folio_test_private(folio) && !folio_test_swapbacked(folio) &&
+ !folio_test_hugetlb(folio);
+}Apart from that LGTM. -- Cheers, David