Re: [PATCH v5 00/17] Remove PG_private by using page/folio->private checks instead

[Andrew Morton <akpm@linux-foundation.org>]

On Sun, 20 Sep 2026 22:27:56 -0400 Zi Yan <ziy@nvidia.com> wrote:

> Hi all,
> 
> This patchset removes PG_private to make space for upcoming PG_folio for
> identifying pages from a folio (more details in Note below). Instead of
> checking PG_private, all code is changed to check page/folio->private !=
> NULL instead.

Thanks, I updated mm-unstable to this version.

> Changes in v5:
> 1. replaced md patches (patch 13 and 14 in v4) with Matthew Wilcox's
>    version (see Matthew's replies to v4).
> 2. used data_race() inside folio_test_private() and PagePrivate(), so that
>    the new versions can be used without KCSAN warnings while not holding
>    folio lock like before.
> 3. moved folio_has_attached_private() implementation detail comment next to
>    the code.

Here's how v5 altered mm.git:


 drivers/md/md-bitmap.c         |   17 ++++++++---------
 fs/buffer.c                    |    8 --------
 include/linux/buffer_head.h    |    2 +-
 include/linux/mm.h             |    3 +--
 include/linux/page-flags.h     |   30 +++++++++++++++++++-----------
 include/trace/events/pagemap.h |    3 +--
 mm/huge_memory.c               |    3 +--
 mm/page-writeback.c            |    3 +--
 8 files changed, 32 insertions(+), 37 deletions(-)

--- a/drivers/md/md-bitmap.c~b
+++ a/drivers/md/md-bitmap.c
@@ -516,7 +516,8 @@ static void end_bitmap_write(struct bio
 
 static void write_file_page(struct bitmap *bitmap, struct page *page, int wait)
 {
-	struct buffer_head *bh = (struct buffer_head *)page_private(page);
+	struct folio *folio = page_folio(page);
+	struct buffer_head *bh = folio_buffers(folio);
 
 	while (bh && bh->b_blocknr) {
 		atomic_inc(&bitmap->pending_writes);
@@ -533,18 +534,15 @@ static void write_file_page(struct bitma
 
 static void free_buffers(struct page *page)
 {
-	struct buffer_head *bh = (struct buffer_head *)page_private(page);
-
-	if (!bh)
-		return;
+	struct folio *folio = page_folio(page);
+	struct buffer_head *bh = folio_detach_private(folio);
 
 	while (bh) {
 		struct buffer_head *next = bh->b_this_page;
 		free_buffer_head(bh);
 		bh = next;
 	}
-	detach_page_private(page);
-	put_page(page);
+	folio_put(folio);
 }
 
 /* read a page from a file.
@@ -559,6 +557,7 @@ static int read_file_page(struct file *f
 {
 	int ret = 0;
 	struct inode *inode = file_inode(file);
+	struct folio *folio = page_folio(page);
 	struct buffer_head *bh;
 	sector_t block, blk_cur;
 	unsigned long blocksize = i_blocksize(inode);
@@ -566,12 +565,12 @@ static int read_file_page(struct file *f
 	pr_debug("read bitmap file (%dB @ %llu)\n", (int)PAGE_SIZE,
 		 (unsigned long long)index << PAGE_SHIFT);
 
-	bh = alloc_page_buffers(page, blocksize);
+	bh = folio_alloc_buffers(folio, blocksize, GFP_NOFS | __GFP_ACCOUNT);
 	if (!bh) {
 		ret = -ENOMEM;
 		goto out;
 	}
-	attach_page_private(page, bh);
+	folio_attach_private(folio, bh);
 	blk_cur = index << (PAGE_SHIFT - inode->i_blkbits);
 	while (bh) {
 		block = blk_cur;
--- a/fs/buffer.c~b
+++ a/fs/buffer.c
@@ -773,14 +773,6 @@ no_grow:
 }
 EXPORT_SYMBOL_GPL(folio_alloc_buffers);
 
-struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size)
-{
-	gfp_t gfp = GFP_NOFS | __GFP_ACCOUNT;
-
-	return folio_alloc_buffers(page_folio(page), size, gfp);
-}
-EXPORT_SYMBOL_GPL(alloc_page_buffers);
-
 static inline void link_dev_buffers(struct folio *folio,
 		struct buffer_head *head)
 {
--- a/include/linux/buffer_head.h~b
+++ a/include/linux/buffer_head.h
@@ -175,6 +175,7 @@ static inline unsigned long bh_offset(co
 	return (unsigned long)(bh)->b_data & (page_size(bh->b_page) - 1);
 }
 
+/* If we *know* folio->private refers to buffer_heads */
 #define folio_buffers(folio)		folio_get_private(folio)
 
 void buffer_check_dirty_writeback(struct folio *folio,
@@ -191,7 +192,6 @@ void folio_set_bh(struct buffer_head *bh
 		  unsigned long offset);
 struct buffer_head *folio_alloc_buffers(struct folio *folio, unsigned long size,
 					gfp_t gfp);
-struct buffer_head *alloc_page_buffers(struct page *page, unsigned long size);
 struct buffer_head *create_empty_buffers(struct folio *folio,
 		unsigned long blocksize, unsigned long b_state);
 void end_buffer_read_sync(struct buffer_head *bh, int uptodate);
--- a/include/linux/mm.h~b
+++ a/include/linux/mm.h
@@ -3052,9 +3052,8 @@ static inline int folio_expected_ref_cou
 		ref_count += !!data_race(folio->mapping) << order;
 		/*
 		 * One reference from filesystem private data.
-		 * Use data_race() since folio might not be locked.
 		 */
-		ref_count += data_race(folio_has_attached_private(folio));
+		ref_count += folio_has_attached_private(folio);
 	}
 
 	/* One reference per page table mapping. */
--- a/include/linux/page-flags.h~b
+++ a/include/linux/page-flags.h
@@ -576,7 +576,12 @@ FOLIO_FLAG(swapbacked, FOLIO_HEAD_PAGE)
 
 static __always_inline bool folio_test_private(const struct folio *folio)
 {
-	return folio->private;
+	/*
+	 * data_race() is added for readers without holding the folio lock.
+	 * Only the NULL/non-NULL answer is used and both are valid while
+	 * private is being attached or detached, so the race is benign.
+	 */
+	return data_race(folio->private);
 }
 
 FOLIO_FLAG(private_2, FOLIO_HEAD_PAGE)
@@ -1199,20 +1204,23 @@ static __always_inline void __ClearPageA
  * @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.
- *
- * 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.
+ * wants to detect attached private data.
  *
- * Return: true if folio->private is set and the folio is neither swapcache
- * nor hugetlb.
+ * Return: true if the folio has private data attached.
  */
 static inline bool folio_has_attached_private(const struct folio *folio)
 {
+	/*
+	 * 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.
+	 *
+	 * 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.
+	 */
+
 	return folio_test_private(folio) && !folio_test_swapbacked(folio) &&
 	       !folio_test_hugetlb(folio);
 }
--- a/include/trace/events/pagemap.h~b
+++ a/include/trace/events/pagemap.h
@@ -22,8 +22,7 @@
 	(folio_test_swapcache(folio)	? PAGEMAP_SWAPCACHE  : 0) | \
 	(folio_test_swapbacked(folio)	? PAGEMAP_SWAPBACKED : 0) | \
 	(folio_test_mappedtodisk(folio)	? PAGEMAP_MAPPEDDISK : 0) | \
-	/* data_race() is used to read attached private locklessly */ \
-	(data_race(folio_has_attached_private(folio))	? PAGEMAP_BUFFERS    : 0) \
+	(folio_has_attached_private(folio)	? PAGEMAP_BUFFERS    : 0) \
 	)
 
 TRACE_EVENT(mm_lru_insertion,
--- a/mm/huge_memory.c~b
+++ a/mm/huge_memory.c
@@ -4845,9 +4845,8 @@ static int split_huge_pages_pid(int pid,
 		 * For folios with private, split_huge_page_to_list_to_order()
 		 * will try to drop it before split and then check if the folio
 		 * can be split or not. So skip the check here.
-		 * data_race() is used to read attached private locklessly.
 		 */
-		if (!data_race(folio_has_attached_private(folio)) &&
+		if (!folio_has_attached_private(folio) &&
 		    folio_expected_ref_count(folio) != folio_ref_count(folio))
 			goto next;
 
--- a/mm/page-writeback.c~b
+++ a/mm/page-writeback.c
@@ -2705,8 +2705,7 @@ bool filemap_dirty_folio(struct address_
 	if (folio_test_set_dirty(folio))
 		return false;
 
-	/* data_race() is used to read attached private locklessly */
-	__folio_mark_dirty(folio, mapping, !data_race(folio_has_attached_private(folio)));
+	__folio_mark_dirty(folio, mapping, !folio_has_attached_private(folio));
 
 	if (mapping->host) {
 		/* !PageAnon && !swapper_space */
_