Re: [PATCH v4] mm: introduce a new page type for page pool in page type

[Byungchul Park]

On Wed, May 13, 2026 at 11:00:51AM +0200, Dragos Tatulea wrote:
> On 24.02.26 06:13, Byungchul Park wrote:
> > Currently, the condition 'page->pp_magic == PP_SIGNATURE' is used to
> > determine if a page belongs to a page pool.  However, with the planned
> > removal of @pp_magic, we should instead leverage the page_type in struct
> > page, such as PGTY_netpp, for this purpose.
> >
> > Introduce and use the page type APIs e.g. PageNetpp(), __SetPageNetpp(),
> > and __ClearPageNetpp() instead, and remove the existing APIs accessing
> > @pp_magic e.g. page_pool_page_is_pp(), netmem_or_pp_magic(), and
> > netmem_clear_pp_magic().
> >
> > Plus, add @page_type to struct net_iov at the same offset as struct page
> > so as to use the page_type APIs for struct net_iov as well.  While at it,
> > reorder @type and @owner in struct net_iov to avoid a hole and
> > increasing the struct size.
> >
> > This work was inspired by the following link:
> >
> >   https://lore.kernel.org/all/582f41c0-2742-4400-9c81-0d46bf4e8314@xxxxxxxxx/
> >
> > While at it, move the sanity check for page pool to on the free path.
> >
> > Suggested-by: David Hildenbrand <david@xxxxxxxxxx>
> > Co-developed-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
> > Signed-off-by: Pavel Begunkov <asml.silence@xxxxxxxxx>
> > Signed-off-by: Byungchul Park <byungchul@xxxxxx>
> > Acked-by: David Hildenbrand <david@xxxxxxxxxx>
> > Acked-by: Zi Yan <ziy@xxxxxxxxxx>
> > Acked-by: Vlastimil Babka <vbabka@xxxxxxx>
> > Reviewed-by: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
> > ---
> 
> Seems like this patch broke tcp_mmap because
> validate_page_before_insert() returns -EINVAL due
> to a page having a type. Here's the full flow:
> 
> getsockopt(TCP_ZEROCOPY_RECEIVE) returns -EINVAL because of the
> below flow in the kernel:
> 
> tcp_zerocopy_receive()
> -> tcp_zerocopy_vm_insert_batch()
>   -> vm_insert_pages()
>     -> insert_pages()
>       -> insert_page_in_batch_locked()
>         -> validate_page_before_insert() returns -EINVAL
>            because page_has_type(page) is now true.
> 
> The patch below fixes the issue. But is this a valid fix?

Hi,

The problem comes from the fact that page_type and _mapcount are
union'ed but there is a case where these two information should be kept
at the same time.

Why don't we allow these two information can be kept in the 4 bytes at
the same time until Zi Yan's work on _mapcount and page_type will be
done, instead of taking a step back?

It can be more optimized but I suggest the approach I just mentioned:
---
diff --git a/fs/proc/internal.h b/fs/proc/internal.h
index 64dc44832808..e5ec204866dc 100644
--- a/fs/proc/internal.h
+++ b/fs/proc/internal.h
@@ -185,8 +185,7 @@ static inline int folio_precise_page_mapcount(struct folio *folio,
 {
 	int mapcount = atomic_read(&page->_mapcount) + 1;
 
-	if (page_mapcount_is_type(mapcount))
-		mapcount = 0;
+	mapcount = page_mapcount_clear_type(mapcount);
 	if (folio_test_large(folio))
 		mapcount += folio_entire_mapcount(folio);
 
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 8260e28205e9..f45064796313 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1865,8 +1865,7 @@ static inline int folio_mapcount(const struct folio *folio)
 
 	if (likely(!folio_test_large(folio))) {
 		mapcount = atomic_read(&folio->_mapcount) + 1;
-		if (page_mapcount_is_type(mapcount))
-			mapcount = 0;
+		mapcount = page_mapcount_clear_type(mapcount);
 		return mapcount;
 	}
 	return folio_large_mapcount(folio);
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 0e03d816e8b9..f3b0d1fa262d 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -934,9 +934,9 @@ static inline bool page_type_has_type(int page_type)
 }
 
 /* This takes a mapcount which is one more than page->_mapcount */
-static inline bool page_mapcount_is_type(unsigned int mapcount)
+static inline unsigned int page_mapcount_clear_type(unsigned int mapcount)
 {
-	return page_type_has_type(mapcount - 1);
+	return (unsigned int)(((int)(mapcount << 8)) >> 8);
 }
 
 static inline bool page_has_type(const struct page *page)
@@ -953,16 +953,20 @@ static __always_inline void __folio_set_##fname(struct folio *folio)	\
 {									\
 	if (folio_test_##fname(folio))					\
 		return;							\
-	VM_BUG_ON_FOLIO(data_race(folio->page.page_type) != UINT_MAX,	\
+	VM_BUG_ON_FOLIO(page_type_has_type(data_race(folio->page.page_type)), \
 			folio);						\
-	folio->page.page_type = (unsigned int)PGTY_##lname << 24;	\
+	folio->page.page_type &= ~(PGTY_mapcount_underflow << 24);	\
+	folio->page.page_type |= (unsigned int)PGTY_##lname << 24;	\
 }									\
 static __always_inline void __folio_clear_##fname(struct folio *folio)	\
 {									\
-	if (folio->page.page_type == UINT_MAX)				\
+	int mapcount;							\
+									\
+	if (!page_type_has_type(folio->page.page_type))			\
 		return;							\
 	VM_BUG_ON_FOLIO(!folio_test_##fname(folio), folio);		\
-	folio->page.page_type = UINT_MAX;				\
+	mapcount = atomic_read(&folio->page._mapcount);			\
+	folio->page.page_type = page_mapcount_clear_type(mapcount);	\
 }
 
 #define PAGE_TYPE_OPS(uname, lname, fname)				\
@@ -975,15 +979,20 @@ static __always_inline void __SetPage##uname(struct page *page)		\
 {									\
 	if (Page##uname(page))						\
 		return;							\
-	VM_BUG_ON_PAGE(data_race(page->page_type) != UINT_MAX, page);	\
-	page->page_type = (unsigned int)PGTY_##lname << 24;		\
+	VM_BUG_ON_PAGE(page_type_has_type(data_race(page->page_type)),	\
+				page);					\
+	page->page_type &= ~(PGTY_mapcount_underflow << 24);		\
+	page->page_type |= (unsigned int)PGTY_##lname << 24;		\
 }									\
 static __always_inline void __ClearPage##uname(struct page *page)	\
 {									\
-	if (page->page_type == UINT_MAX)				\
+	int mapcount;							\
+									\
+	if (!page_type_has_type(page->page_type))			\
 		return;							\
 	VM_BUG_ON_PAGE(!Page##uname(page), page);			\
-	page->page_type = UINT_MAX;					\
+	mapcount = atomic_read(&page->_mapcount);			\
+	page->page_type = page_mapcount_clear_type(mapcount);		\
 }
 
 /*
diff --git a/mm/debug.c b/mm/debug.c
index 77fa8fe1d641..9a932ded09d4 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -74,8 +74,7 @@ static void __dump_folio(const struct folio *folio, const struct page *page,
 	int mapcount = atomic_read(&page->_mapcount) + 1;
 	char *type = "";
 
-	if (page_mapcount_is_type(mapcount))
-		mapcount = 0;
+	mapcount = page_mapcount_clear_type(mapcount);
 
 	pr_warn("page: refcount:%d mapcount:%d mapping:%p index:%#lx pfn:%#lx\n",
 			folio_ref_count(folio), mapcount, mapping,
---

Thoughts?

	Byungchul

> diff --git a/mm/memory.c b/mm/memory.c
> index ea6568571131..4cb12673f450 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2326,7 +2326,7 @@ static int validate_page_before_insert(struct vm_area_struct *vma,
>                         return -EINVAL;
>                 return 0;
>         }
> -       if (folio_test_anon(folio) || page_has_type(page))
> +       if (folio_test_anon(folio) || (page_has_type(page) && !PageNetpp(page)))
>                 return -EINVAL;
>         flush_dcache_folio(folio);
>         return 0;
> 
> Thanks,
> Dragos
>