[PATCH net-next v9 0/8] Split netmem from struct page

[PATCH net-next v9 0/8] Split netmem from struct page

[Byungchul Park]

Hi all,

The MM subsystem is trying to reduce struct page to a single pointer.
See the following link for your information:

   https://kernelnewbies.org/MatthewWilcox/Memdescs/Path

The first step towards that is splitting struct page by its individual
users, as has already been done with folio and slab.  This patchset does
that for page pool.

Matthew Wilcox tried and stopped the same work, you can see in:

   https://lore.kernel.org/linux-mm/20230111042214.907030-1-willy@infradead.org/

I focused on removing the page pool members in struct page this time,
not moving the allocation code of page pool from net to mm.  It can be
done later if needed.

The final patch removing the page pool fields will be posted once all
the converting of page to netmem are done:

   1. converting use of the pp fields in struct page in prueth_swdata.
   2. converting use of the pp fields in struct page in freescale driver.

For our discussion, I'm sharing what the final patch looks like, in this
cover letter.

	Byungchul
--8<--
commit 1847d9890f798456b21ccb27aac7545303048492
Author: Byungchul Park <byungchul@sk.com>
Date:   Wed May 28 20:44:55 2025 +0900

    mm, netmem: remove the page pool members in struct page
    
    Now that all the users of the page pool members in struct page have been
    gone, the members can be removed from struct page.
    
    However, since struct netmem_desc still uses the space in struct page,
    the important offsets should be checked properly, until struct
    netmem_desc has its own instance from slab.
    
    Remove the page pool members in struct page and modify static checkers
    for the offsets.
    
    Signed-off-by: Byungchul Park <byungchul@sk.com>

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 32ba5126e221..db2fe0d0ebbf 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -120,17 +120,6 @@ struct page {
 			 */
 			unsigned long private;
 		};
-		struct {	/* page_pool used by netstack */
-			/**
-			 * @pp_magic: magic value to avoid recycling non
-			 * page_pool allocated pages.
-			 */
-			unsigned long pp_magic;
-			struct page_pool *pp;
-			unsigned long _pp_mapping_pad;
-			unsigned long dma_addr;
-			atomic_long_t pp_ref_count;
-		};
 		struct {	/* Tail pages of compound page */
 			unsigned long compound_head;	/* Bit zero is set */
 		};
diff --git a/include/net/netmem.h b/include/net/netmem.h
index 8f354ae7d5c3..3414f184d018 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -42,11 +42,8 @@ struct netmem_desc {
 	static_assert(offsetof(struct page, pg) == \
 		      offsetof(struct netmem_desc, desc))
 NETMEM_DESC_ASSERT_OFFSET(flags, _flags);
-NETMEM_DESC_ASSERT_OFFSET(pp_magic, pp_magic);
-NETMEM_DESC_ASSERT_OFFSET(pp, pp);
-NETMEM_DESC_ASSERT_OFFSET(_pp_mapping_pad, _pp_mapping_pad);
-NETMEM_DESC_ASSERT_OFFSET(dma_addr, dma_addr);
-NETMEM_DESC_ASSERT_OFFSET(pp_ref_count, pp_ref_count);
+NETMEM_DESC_ASSERT_OFFSET(lru, pp_magic);
+NETMEM_DESC_ASSERT_OFFSET(mapping, _pp_mapping_pad);
 #undef NETMEM_DESC_ASSERT_OFFSET
 
 /*
---
Changes from v8:
	1. Rebase on net-next/main as of Jul 10.
	2. Exclude non-controversial patches that have already been
	   merged to net-next.
	3. Re-add the patches that focus on removing accessing the page
	   pool fields in struct page.
	4. Add utility APIs e.g. casting, to use struct netmem_desc as
	   descriptor, to support __netmem_get_pp() that has started to
	   be used again e.g. by libeth.

Changes from v7 (no actual updates):
	1. Exclude "netmem: introduce struct netmem_desc mirroring
	   struct page" that might be controversial.
	2. Exclude "netmem: introduce a netmem API,
	   virt_to_head_netmem()" since there are no users.

Changes from v6 (no actual updates):
	1. Rebase on net-next/main as of Jun 25.
	2. Supplement a comment describing struct net_iov.
	3. Exclude a controversial patch, "page_pool: access ->pp_magic
	   through struct netmem_desc in page_pool_page_is_pp()".
	4. Exclude "netmem: remove __netmem_get_pp()" since the API
	   started to be used again by libeth.

Changes from v5 (no actual updates):
	1. Rebase on net-next/main as of Jun 20.
	2. Add given 'Reviewed-by's and 'Acked-by's, thanks to all.
	3. Add missing cc's.

Changes from v4:
	1. Add given 'Reviewed-by's, thanks to all.
	2. Exclude potentially controversial patches.

Changes from v3:
	1. Relocates ->owner and ->type of net_iov out of netmem_desc
	   and make them be net_iov specific.
	2. Remove __force when casting struct page to struct netmem_desc.

Changes from v2:
	1. Introduce a netmem API, virt_to_head_netmem(), and use it
	   when it's needed.
	2. Introduce struct netmem_desc as a new struct and union'ed
	   with the existing fields in struct net_iov.
	3. Make page_pool_page_is_pp() access ->pp_magic through struct
	   netmem_desc instead of struct page.
	4. Move netmem alloc APIs from include/net/netmem.h to
	   net/core/netmem_priv.h.
	5. Apply trivial feedbacks, thanks to Mina, Pavel, and Toke.
	6. Add given 'Reviewed-by's, thanks to Mina.

Changes from v1:
	1. Rebase on net-next's main as of May 26.
	2. Check checkpatch.pl, feedbacked by SJ Park.
	3. Add converting of page to netmem in mt76.
	4. Revert 'mlx5: use netmem descriptor and APIs for page pool'
	   since it's on-going by Tariq Toukan.  I will wait for his
	   work to be done.
	5. Revert 'page_pool: use netmem APIs to access page->pp_magic
	   in page_pool_page_is_pp()' since we need more discussion.
	6. Revert 'mm, netmem: remove the page pool members in struct
	   page' since there are some prerequisite works to remove the
	   page pool fields from struct page.  I can submit this patch
	   separatedly later.
	7. Cancel relocating a page pool member in struct page.
	8. Modify static assert for offests and size of struct
	   netmem_desc.

Changes from rfc:
	1. Rebase on net-next's main branch.
	   https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/
	2. Fix a build error reported by kernel test robot.
	   https://lore.kernel.org/all/202505100932.uzAMBW1y-lkp@intel.com/
	3. Add given 'Reviewed-by's, thanks to Mina and Ilias.
	4. Do static_assert() on the size of struct netmem_desc instead
	   of placing place-holder in struct page, feedbacked by
	   Matthew.
	5. Do struct_group_tagged(netmem_desc) on struct net_iov instead
	   of wholly renaming it to strcut netmem_desc, feedbacked by
	   Mina and Pavel.

Byungchul Park (8):
  netmem: introduce struct netmem_desc mirroring struct page
  netmem: introduce utility APIs to use struct netmem_desc
  page_pool: access ->pp_magic through struct netmem_desc in
    page_pool_page_is_pp()
  netmem: use netmem_desc instead of page to access ->pp in
    __netmem_get_pp()
  netmem: introduce a netmem API, virt_to_head_netmem()
  mlx4: use netmem descriptor and APIs for page pool
  netdevsim: use netmem descriptor and APIs for page pool
  mt76: use netmem descriptor and APIs for page pool

 drivers/net/ethernet/mellanox/mlx4/en_rx.c    |  48 ++---
 drivers/net/ethernet/mellanox/mlx4/en_tx.c    |   8 +-
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h  |   4 +-
 drivers/net/netdevsim/netdev.c                |  19 +-
 drivers/net/netdevsim/netdevsim.h             |   2 +-
 drivers/net/wireless/mediatek/mt76/dma.c      |   6 +-
 drivers/net/wireless/mediatek/mt76/mt76.h     |  12 +-
 .../net/wireless/mediatek/mt76/sdio_txrx.c    |  24 +--
 drivers/net/wireless/mediatek/mt76/usb.c      |  10 +-
 include/linux/mm.h                            |  12 --
 include/net/netmem.h                          | 183 +++++++++++++++---
 mm/page_alloc.c                               |   1 +
 12 files changed, 231 insertions(+), 98 deletions(-)


base-commit: c65d34296b2252897e37835d6007bbd01b255742
-- 
2.17.1

[PATCH net-next v9 1/8] netmem: introduce struct netmem_desc mirroring struct page

[Byungchul Park]

To simplify struct page, the page pool members of struct page should be
moved to other, allowing these members to be removed from struct page.

Introduce a network memory descriptor to store the members, struct
netmem_desc, and make it union'ed with the existing fields in struct
net_iov, allowing to organize the fields of struct net_iov.

Signed-off-by: Byungchul Park <byungchul@sk.com>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: Harry Yoo <harry.yoo@oracle.com>
---
 include/net/netmem.h | 116 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 95 insertions(+), 21 deletions(-)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index de1d95f04076..535cf17b9134 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -12,6 +12,50 @@
 #include <linux/mm.h>
 #include <net/net_debug.h>
 
+/* These fields in struct page are used by the page_pool and net stack:
+ *
+ *        struct {
+ *                unsigned long pp_magic;
+ *                struct page_pool *pp;
+ *                unsigned long _pp_mapping_pad;
+ *                unsigned long dma_addr;
+ *                atomic_long_t pp_ref_count;
+ *        };
+ *
+ * We mirror the page_pool fields here so the page_pool can access these
+ * fields without worrying whether the underlying fields belong to a
+ * page or netmem_desc.
+ *
+ * CAUTION: Do not update the fields in netmem_desc without also
+ * updating the anonymous aliasing union in struct net_iov.
+ */
+struct netmem_desc {
+	unsigned long _flags;
+	unsigned long pp_magic;
+	struct page_pool *pp;
+	unsigned long _pp_mapping_pad;
+	unsigned long dma_addr;
+	atomic_long_t pp_ref_count;
+};
+
+#define NETMEM_DESC_ASSERT_OFFSET(pg, desc)        \
+	static_assert(offsetof(struct page, pg) == \
+		      offsetof(struct netmem_desc, desc))
+NETMEM_DESC_ASSERT_OFFSET(flags, _flags);
+NETMEM_DESC_ASSERT_OFFSET(pp_magic, pp_magic);
+NETMEM_DESC_ASSERT_OFFSET(pp, pp);
+NETMEM_DESC_ASSERT_OFFSET(_pp_mapping_pad, _pp_mapping_pad);
+NETMEM_DESC_ASSERT_OFFSET(dma_addr, dma_addr);
+NETMEM_DESC_ASSERT_OFFSET(pp_ref_count, pp_ref_count);
+#undef NETMEM_DESC_ASSERT_OFFSET
+
+/*
+ * Since struct netmem_desc uses the space in struct page, the size
+ * should be checked, until struct netmem_desc has its own instance from
+ * slab, to avoid conflicting with other members within struct page.
+ */
+static_assert(sizeof(struct netmem_desc) <= offsetof(struct page, _refcount));
+
 /* net_iov */
 
 DECLARE_STATIC_KEY_FALSE(page_pool_mem_providers);
@@ -30,13 +74,48 @@ enum net_iov_type {
 	NET_IOV_MAX = ULONG_MAX
 };
 
+/* A memory descriptor representing abstract networking I/O vectors,
+ * generally for non-pages memory that doesn't have its corresponding
+ * struct page and needs to be explicitly allocated through slab.
+ *
+ * net_iovs are allocated and used by networking code, and the size of
+ * the chunk is PAGE_SIZE.
+ *
+ * This memory can be any form of non-struct paged memory.  Examples
+ * include imported dmabuf memory and imported io_uring memory.  See
+ * net_iov_type for all the supported types.
+ *
+ * @pp_magic:	pp field, similar to the one in struct page/struct
+ *		netmem_desc.
+ * @pp:		the pp this net_iov belongs to, if any.
+ * @dma_addr:	the dma addrs of the net_iov. Needed for the network
+ *		card to send/receive this net_iov.
+ * @pp_ref_count: the pp ref count of this net_iov, exactly the same
+ *		usage as struct page/struct netmem_desc.
+ * @owner:	the net_iov_area this net_iov belongs to, if any.
+ * @type:	the type of the memory.  Different types of net_iovs are
+ *		supported.
+ */
 struct net_iov {
-	enum net_iov_type type;
-	unsigned long pp_magic;
-	struct page_pool *pp;
+	union {
+		struct netmem_desc desc;
+
+		/* XXX: The following part should be removed once all
+		 * the references to them are converted so as to be
+		 * accessed via netmem_desc e.g. niov->desc.pp instead
+		 * of niov->pp.
+		 */
+		struct {
+			unsigned long _flags;
+			unsigned long pp_magic;
+			struct page_pool *pp;
+			unsigned long _pp_mapping_pad;
+			unsigned long dma_addr;
+			atomic_long_t pp_ref_count;
+		};
+	};
 	struct net_iov_area *owner;
-	unsigned long dma_addr;
-	atomic_long_t pp_ref_count;
+	enum net_iov_type type;
 };
 
 struct net_iov_area {
@@ -48,27 +127,22 @@ struct net_iov_area {
 	unsigned long base_virtual;
 };
 
-/* These fields in struct page are used by the page_pool and net stack:
+/* net_iov is union'ed with struct netmem_desc mirroring struct page, so
+ * the page_pool can access these fields without worrying whether the
+ * underlying fields are accessed via netmem_desc or directly via
+ * net_iov, until all the references to them are converted so as to be
+ * accessed via netmem_desc e.g. niov->desc.pp instead of niov->pp.
  *
- *        struct {
- *                unsigned long pp_magic;
- *                struct page_pool *pp;
- *                unsigned long _pp_mapping_pad;
- *                unsigned long dma_addr;
- *                atomic_long_t pp_ref_count;
- *        };
- *
- * We mirror the page_pool fields here so the page_pool can access these fields
- * without worrying whether the underlying fields belong to a page or net_iov.
- *
- * The non-net stack fields of struct page are private to the mm stack and must
- * never be mirrored to net_iov.
+ * The non-net stack fields of struct page are private to the mm stack
+ * and must never be mirrored to net_iov.
  */
-#define NET_IOV_ASSERT_OFFSET(pg, iov)             \
-	static_assert(offsetof(struct page, pg) == \
+#define NET_IOV_ASSERT_OFFSET(desc, iov)                    \
+	static_assert(offsetof(struct netmem_desc, desc) == \
 		      offsetof(struct net_iov, iov))
+NET_IOV_ASSERT_OFFSET(_flags, _flags);
 NET_IOV_ASSERT_OFFSET(pp_magic, pp_magic);
 NET_IOV_ASSERT_OFFSET(pp, pp);
+NET_IOV_ASSERT_OFFSET(_pp_mapping_pad, _pp_mapping_pad);
 NET_IOV_ASSERT_OFFSET(dma_addr, dma_addr);
 NET_IOV_ASSERT_OFFSET(pp_ref_count, pp_ref_count);
 #undef NET_IOV_ASSERT_OFFSET
-- 
2.17.1

[PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Byungchul Park]

To eliminate the use of the page pool fields in struct page, the page
pool code should use netmem descriptor and APIs instead.

However, some code e.g. __netmem_to_page() is still used to access the
page pool fields e.g. ->pp via struct page, which should be changed so
as to access them via netmem descriptor, struct netmem_desc instead,
since the fields no longer will be available in struct page.

Introduce utility APIs to make them easy to use struct netmem_desc as
descriptor.  The APIs are:

   1. __netmem_to_nmdesc(), to convert netmem_ref to struct netmem_desc,
      but unsafely without checking if it's net_iov or system memory.

   2. netmem_to_nmdesc(), to convert netmem_ref to struct netmem_desc,
      safely with checking if it's net_iov or system memory.

   3. nmdesc_to_page(), to convert struct netmem_desc to struct page,
      assuming struct netmem_desc overlays on struct page.

   4. page_to_nmdesc(), to convert struct page to struct netmem_desc,
      assuming struct netmem_desc overlays on struct page, allowing only
      head page to be converted.

   5. nmdesc_adress(), to get its virtual address corresponding to the
      struct netmem_desc.

Signed-off-by: Byungchul Park <byungchul@sk.com>
---
 include/net/netmem.h | 41 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index 535cf17b9134..ad9444be229a 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -198,6 +198,32 @@ static inline struct page *netmem_to_page(netmem_ref netmem)
 	return __netmem_to_page(netmem);
 }
 
+/**
+ * __netmem_to_nmdesc - unsafely get pointer to the &netmem_desc backing
+ * @netmem
+ * @netmem: netmem reference to convert
+ *
+ * Unsafe version of netmem_to_nmdesc(). When @netmem is always backed
+ * by system memory, performs faster and generates smaller object code
+ * (no check for the LSB, no WARN). When @netmem points to IOV, provokes
+ * undefined behaviour.
+ *
+ * Return: pointer to the &netmem_desc (garbage if @netmem is not backed
+ * by system memory).
+ */
+static inline struct netmem_desc *__netmem_to_nmdesc(netmem_ref netmem)
+{
+	return (__force struct netmem_desc *)netmem;
+}
+
+static inline struct netmem_desc *netmem_to_nmdesc(netmem_ref netmem)
+{
+	if (WARN_ON_ONCE(netmem_is_net_iov(netmem)))
+		return NULL;
+
+	return __netmem_to_nmdesc(netmem);
+}
+
 static inline struct net_iov *netmem_to_net_iov(netmem_ref netmem)
 {
 	if (netmem_is_net_iov(netmem))
@@ -314,6 +340,21 @@ static inline netmem_ref netmem_compound_head(netmem_ref netmem)
 	return page_to_netmem(compound_head(netmem_to_page(netmem)));
 }
 
+#define nmdesc_to_page(nmdesc)		(_Generic((nmdesc),		\
+	const struct netmem_desc * :	(const struct page *)(nmdesc),	\
+	struct netmem_desc * :		(struct page *)(nmdesc)))
+
+static inline struct netmem_desc *page_to_nmdesc(struct page *page)
+{
+	VM_BUG_ON_PAGE(PageTail(page), page);
+	return (struct netmem_desc *)page;
+}
+
+static inline void *nmdesc_address(struct netmem_desc *nmdesc)
+{
+	return page_address(nmdesc_to_page(nmdesc));
+}
+
 /**
  * __netmem_address - unsafely get pointer to the memory backing @netmem
  * @netmem: netmem reference to get the pointer for
-- 
2.17.1

[PATCH net-next v9 3/8] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[Byungchul Park]

To simplify struct page, the effort to separate its own descriptor from
struct page is required and the work for page pool is on going.

To achieve that, all the code should avoid directly accessing page pool
members of struct page.

Access ->pp_magic through struct netmem_desc instead of directly
accessing it through struct page in page_pool_page_is_pp().  Plus, move
page_pool_page_is_pp() from mm.h to netmem.h to use struct netmem_desc
without header dependency issue.

Signed-off-by: Byungchul Park <byungchul@sk.com>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
Acked-by: Harry Yoo <harry.yoo@oracle.com>
---
 include/linux/mm.h   | 12 ------------
 include/net/netmem.h | 17 +++++++++++++++++
 mm/page_alloc.c      |  1 +
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0ef2ba0c667a..0b7f7f998085 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4172,16 +4172,4 @@ int arch_lock_shadow_stack_status(struct task_struct *t, unsigned long status);
  */
 #define PP_MAGIC_MASK ~(PP_DMA_INDEX_MASK | 0x3UL)
 
-#ifdef CONFIG_PAGE_POOL
-static inline bool page_pool_page_is_pp(struct page *page)
-{
-	return (page->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
-}
-#else
-static inline bool page_pool_page_is_pp(struct page *page)
-{
-	return false;
-}
-#endif
-
 #endif /* _LINUX_MM_H */
diff --git a/include/net/netmem.h b/include/net/netmem.h
index ad9444be229a..11e9de45efcb 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -355,6 +355,23 @@ static inline void *nmdesc_address(struct netmem_desc *nmdesc)
 	return page_address(nmdesc_to_page(nmdesc));
 }
 
+#ifdef CONFIG_PAGE_POOL
+/* XXX: This would better be moved to mm, once mm gets its way to
+ * identify the type of page for page pool.
+ */
+static inline bool page_pool_page_is_pp(struct page *page)
+{
+	struct netmem_desc *desc = page_to_nmdesc(page);
+
+	return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
+}
+#else
+static inline bool page_pool_page_is_pp(struct page *page)
+{
+	return false;
+}
+#endif
+
 /**
  * __netmem_address - unsafely get pointer to the memory backing @netmem
  * @netmem: netmem reference to get the pointer for
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 2ef3c07266b3..cc1d169853e8 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -55,6 +55,7 @@
 #include <linux/delayacct.h>
 #include <linux/cacheinfo.h>
 #include <linux/pgalloc_tag.h>
+#include <net/netmem.h>
 #include <asm/div64.h>
 #include "internal.h"
 #include "shuffle.h"
-- 
2.17.1

[PATCH net-next v9 4/8] netmem: use netmem_desc instead of page to access ->pp in __netmem_get_pp()

[Byungchul Park]

To eliminate the use of the page pool fields in struct page, the page
pool code should use netmem descriptor and APIs instead.

However, __netmem_get_pp() still accesses ->pp via struct page.  So
change it to use struct netmem_desc instead, since ->pp no longer will
be available in struct page.

Signed-off-by: Byungchul Park <byungchul@sk.com>
---
 include/net/netmem.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index 11e9de45efcb..283b4a997fbc 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -306,7 +306,7 @@ static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem)
  */
 static inline struct page_pool *__netmem_get_pp(netmem_ref netmem)
 {
-	return __netmem_to_page(netmem)->pp;
+	return __netmem_to_nmdesc(netmem)->pp;
 }
 
 static inline struct page_pool *netmem_get_pp(netmem_ref netmem)
-- 
2.17.1

[PATCH net-next v9 5/8] netmem: introduce a netmem API, virt_to_head_netmem()

[Byungchul Park]

To eliminate the use of struct page in page pool, the page pool code
should use netmem descriptor and APIs instead.

As part of the work, introduce a netmem API to convert a virtual address
to a head netmem allowing the code to use it rather than the existing
API, virt_to_head_page() for struct page.

Signed-off-by: Byungchul Park <byungchul@sk.com>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
Reviewed-by: Mina Almasry <almasrymina@google.com>
---
 include/net/netmem.h | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index 283b4a997fbc..b92c7f15166a 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -372,6 +372,13 @@ static inline bool page_pool_page_is_pp(struct page *page)
 }
 #endif
 
+static inline netmem_ref virt_to_head_netmem(const void *x)
+{
+	netmem_ref netmem = virt_to_netmem(x);
+
+	return netmem_compound_head(netmem);
+}
+
 /**
  * __netmem_address - unsafely get pointer to the memory backing @netmem
  * @netmem: netmem reference to get the pointer for
-- 
2.17.1

[PATCH net-next v9 6/8] mlx4: use netmem descriptor and APIs for page pool

[Byungchul Park]

To simplify struct page, the effort to separate its own descriptor from
struct page is required and the work for page pool is on going.

Use netmem descriptor and APIs for page pool in mlx4 code.

Signed-off-by: Byungchul Park <byungchul@sk.com>
---
 drivers/net/ethernet/mellanox/mlx4/en_rx.c   | 48 +++++++++++---------
 drivers/net/ethernet/mellanox/mlx4/en_tx.c   |  8 ++--
 drivers/net/ethernet/mellanox/mlx4/mlx4_en.h |  4 +-
 3 files changed, 32 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
index b33285d755b9..7cf0d2dc5011 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
@@ -62,18 +62,18 @@ static int mlx4_en_alloc_frags(struct mlx4_en_priv *priv,
 	int i;
 
 	for (i = 0; i < priv->num_frags; i++, frags++) {
-		if (!frags->page) {
-			frags->page = page_pool_alloc_pages(ring->pp, gfp);
-			if (!frags->page) {
+		if (!frags->netmem) {
+			frags->netmem = page_pool_alloc_netmems(ring->pp, gfp);
+			if (!frags->netmem) {
 				ring->alloc_fail++;
 				return -ENOMEM;
 			}
-			page_pool_fragment_page(frags->page, 1);
+			page_pool_fragment_netmem(frags->netmem, 1);
 			frags->page_offset = priv->rx_headroom;
 
 			ring->rx_alloc_pages++;
 		}
-		dma = page_pool_get_dma_addr(frags->page);
+		dma = page_pool_get_dma_addr_netmem(frags->netmem);
 		rx_desc->data[i].addr = cpu_to_be64(dma + frags->page_offset);
 	}
 	return 0;
@@ -83,10 +83,10 @@ static void mlx4_en_free_frag(const struct mlx4_en_priv *priv,
 			      struct mlx4_en_rx_ring *ring,
 			      struct mlx4_en_rx_alloc *frag)
 {
-	if (frag->page)
-		page_pool_put_full_page(ring->pp, frag->page, false);
+	if (frag->netmem)
+		page_pool_put_full_netmem(ring->pp, frag->netmem, false);
 	/* We need to clear all fields, otherwise a change of priv->log_rx_info
-	 * could lead to see garbage later in frag->page.
+	 * could lead to see garbage later in frag->netmem.
 	 */
 	memset(frag, 0, sizeof(*frag));
 }
@@ -440,29 +440,33 @@ static int mlx4_en_complete_rx_desc(struct mlx4_en_priv *priv,
 	unsigned int truesize = 0;
 	bool release = true;
 	int nr, frag_size;
-	struct page *page;
+	netmem_ref netmem;
 	dma_addr_t dma;
 
 	/* Collect used fragments while replacing them in the HW descriptors */
 	for (nr = 0;; frags++) {
 		frag_size = min_t(int, length, frag_info->frag_size);
 
-		page = frags->page;
-		if (unlikely(!page))
+		netmem = frags->netmem;
+		if (unlikely(!netmem))
 			goto fail;
 
-		dma = page_pool_get_dma_addr(page);
+		dma = page_pool_get_dma_addr_netmem(netmem);
 		dma_sync_single_range_for_cpu(priv->ddev, dma, frags->page_offset,
 					      frag_size, priv->dma_dir);
 
-		__skb_fill_page_desc(skb, nr, page, frags->page_offset,
-				     frag_size);
+		__skb_fill_netmem_desc(skb, nr, netmem, frags->page_offset,
+				       frag_size);
 
 		truesize += frag_info->frag_stride;
 		if (frag_info->frag_stride == PAGE_SIZE / 2) {
+			struct page *page = netmem_to_page(netmem);
+			atomic_long_t *pp_ref_count =
+				netmem_get_pp_ref_count_ref(netmem);
+
 			frags->page_offset ^= PAGE_SIZE / 2;
 			release = page_count(page) != 1 ||
-				  atomic_long_read(&page->pp_ref_count) != 1 ||
+				  atomic_long_read(pp_ref_count) != 1 ||
 				  page_is_pfmemalloc(page) ||
 				  page_to_nid(page) != numa_mem_id();
 		} else if (!priv->rx_headroom) {
@@ -476,9 +480,9 @@ static int mlx4_en_complete_rx_desc(struct mlx4_en_priv *priv,
 			release = frags->page_offset + frag_info->frag_size > PAGE_SIZE;
 		}
 		if (release) {
-			frags->page = NULL;
+			frags->netmem = 0;
 		} else {
-			page_pool_ref_page(page);
+			page_pool_ref_netmem(netmem);
 		}
 
 		nr++;
@@ -719,7 +723,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 		int nr;
 
 		frags = ring->rx_info + (index << priv->log_rx_info);
-		va = page_address(frags[0].page) + frags[0].page_offset;
+		va = netmem_address(frags[0].netmem) + frags[0].page_offset;
 		net_prefetchw(va);
 		/*
 		 * make sure we read the CQE after we read the ownership bit
@@ -748,7 +752,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 			/* Get pointer to first fragment since we haven't
 			 * skb yet and cast it to ethhdr struct
 			 */
-			dma = page_pool_get_dma_addr(frags[0].page);
+			dma = page_pool_get_dma_addr_netmem(frags[0].netmem);
 			dma += frags[0].page_offset;
 			dma_sync_single_for_cpu(priv->ddev, dma, sizeof(*ethh),
 						DMA_FROM_DEVICE);
@@ -788,7 +792,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 			void *orig_data;
 			u32 act;
 
-			dma = page_pool_get_dma_addr(frags[0].page);
+			dma = page_pool_get_dma_addr_netmem(frags[0].netmem);
 			dma += frags[0].page_offset;
 			dma_sync_single_for_cpu(priv->ddev, dma,
 						priv->frag_info[0].frag_size,
@@ -818,7 +822,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 				if (likely(!xdp_do_redirect(dev, &mxbuf.xdp, xdp_prog))) {
 					ring->xdp_redirect++;
 					xdp_redir_flush = true;
-					frags[0].page = NULL;
+					frags[0].netmem = 0;
 					goto next;
 				}
 				ring->xdp_redirect_fail++;
@@ -828,7 +832,7 @@ int mlx4_en_process_rx_cq(struct net_device *dev, struct mlx4_en_cq *cq, int bud
 				if (likely(!mlx4_en_xmit_frame(ring, frags, priv,
 							length, cq_ring,
 							&doorbell_pending))) {
-					frags[0].page = NULL;
+					frags[0].netmem = 0;
 					goto next;
 				}
 				trace_xdp_exception(dev, xdp_prog, act);
diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
index 87f35bcbeff8..b564a953da09 100644
--- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c
@@ -354,7 +354,7 @@ u32 mlx4_en_recycle_tx_desc(struct mlx4_en_priv *priv,
 	struct page_pool *pool = ring->recycle_ring->pp;
 
 	/* Note that napi_mode = 0 means ndo_close() path, not budget = 0 */
-	page_pool_put_full_page(pool, tx_info->page, !!napi_mode);
+	page_pool_put_full_netmem(pool, tx_info->netmem, !!napi_mode);
 
 	return tx_info->nr_txbb;
 }
@@ -1191,10 +1191,10 @@ netdev_tx_t mlx4_en_xmit_frame(struct mlx4_en_rx_ring *rx_ring,
 	tx_desc = ring->buf + (index << LOG_TXBB_SIZE);
 	data = &tx_desc->data;
 
-	dma = page_pool_get_dma_addr(frame->page);
+	dma = page_pool_get_dma_addr_netmem(frame->netmem);
 
-	tx_info->page = frame->page;
-	frame->page = NULL;
+	tx_info->netmem = frame->netmem;
+	frame->netmem = 0;
 	tx_info->map0_dma = dma;
 	tx_info->nr_bytes = max_t(unsigned int, length, ETH_ZLEN);
 
diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
index ad0d91a75184..3ef9a0a1f783 100644
--- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
+++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h
@@ -213,7 +213,7 @@ enum cq_type {
 struct mlx4_en_tx_info {
 	union {
 		struct sk_buff *skb;
-		struct page *page;
+		netmem_ref netmem;
 	};
 	dma_addr_t	map0_dma;
 	u32		map0_byte_count;
@@ -246,7 +246,7 @@ struct mlx4_en_tx_desc {
 #define MLX4_EN_CX3_HIGH_ID	0x1005
 
 struct mlx4_en_rx_alloc {
-	struct page	*page;
+	netmem_ref	netmem;
 	u32		page_offset;
 };
 
-- 
2.17.1

[PATCH net-next v9 7/8] netdevsim: use netmem descriptor and APIs for page pool

[Byungchul Park]

To simplify struct page, the effort to separate its own descriptor from
struct page is required and the work for page pool is on going.

Use netmem descriptor and APIs for page pool in netdevsim code.

Signed-off-by: Byungchul Park <byungchul@sk.com>
---
 drivers/net/netdevsim/netdev.c    | 19 ++++++++++---------
 drivers/net/netdevsim/netdevsim.h |  2 +-
 2 files changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
index e36d3e846c2d..ba19870524c5 100644
--- a/drivers/net/netdevsim/netdev.c
+++ b/drivers/net/netdevsim/netdev.c
@@ -812,7 +812,7 @@ nsim_pp_hold_read(struct file *file, char __user *data,
 	struct netdevsim *ns = file->private_data;
 	char buf[3] = "n\n";
 
-	if (ns->page)
+	if (ns->netmem)
 		buf[0] = 'y';
 
 	return simple_read_from_buffer(data, count, ppos, buf, 2);
@@ -832,18 +832,19 @@ nsim_pp_hold_write(struct file *file, const char __user *data,
 
 	rtnl_lock();
 	ret = count;
-	if (val == !!ns->page)
+	if (val == !!ns->netmem)
 		goto exit;
 
 	if (!netif_running(ns->netdev) && val) {
 		ret = -ENETDOWN;
 	} else if (val) {
-		ns->page = page_pool_dev_alloc_pages(ns->rq[0]->page_pool);
-		if (!ns->page)
+		ns->netmem = page_pool_alloc_netmems(ns->rq[0]->page_pool,
+						     GFP_ATOMIC | __GFP_NOWARN);
+		if (!ns->netmem)
 			ret = -ENOMEM;
 	} else {
-		page_pool_put_full_page(ns->page->pp, ns->page, false);
-		ns->page = NULL;
+		page_pool_put_full_netmem(netmem_get_pp(ns->netmem), ns->netmem, false);
+		ns->netmem = 0;
 	}
 
 exit:
@@ -1068,9 +1069,9 @@ void nsim_destroy(struct netdevsim *ns)
 		nsim_exit_netdevsim(ns);
 
 	/* Put this intentionally late to exercise the orphaning path */
-	if (ns->page) {
-		page_pool_put_full_page(ns->page->pp, ns->page, false);
-		ns->page = NULL;
+	if (ns->netmem) {
+		page_pool_put_full_netmem(netmem_get_pp(ns->netmem), ns->netmem, false);
+		ns->netmem = 0;
 	}
 
 	free_netdev(dev);
diff --git a/drivers/net/netdevsim/netdevsim.h b/drivers/net/netdevsim/netdevsim.h
index 809dd29fc5fe..129e005ef577 100644
--- a/drivers/net/netdevsim/netdevsim.h
+++ b/drivers/net/netdevsim/netdevsim.h
@@ -132,7 +132,7 @@ struct netdevsim {
 		struct debugfs_u32_array dfs_ports[2];
 	} udp_ports;
 
-	struct page *page;
+	netmem_ref netmem;
 	struct dentry *pp_dfs;
 	struct dentry *qr_dfs;
 
-- 
2.17.1

[PATCH net-next v9 8/8] mt76: use netmem descriptor and APIs for page pool

[Byungchul Park]

To simplify struct page, the effort to separate its own descriptor from
struct page is required and the work for page pool is on going.

Use netmem descriptor and APIs for page pool in mt76 code.

Signed-off-by: Byungchul Park <byungchul@sk.com>
Reviewed-by: Mina Almasry <almasrymina@google.com>
---
 drivers/net/wireless/mediatek/mt76/dma.c      |  6 ++---
 drivers/net/wireless/mediatek/mt76/mt76.h     | 12 +++++-----
 .../net/wireless/mediatek/mt76/sdio_txrx.c    | 24 +++++++++----------
 drivers/net/wireless/mediatek/mt76/usb.c      | 10 ++++----
 4 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/dma.c b/drivers/net/wireless/mediatek/mt76/dma.c
index 35b4ec91979e..41b529b95877 100644
--- a/drivers/net/wireless/mediatek/mt76/dma.c
+++ b/drivers/net/wireless/mediatek/mt76/dma.c
@@ -820,10 +820,10 @@ mt76_add_fragment(struct mt76_dev *dev, struct mt76_queue *q, void *data,
 	int nr_frags = shinfo->nr_frags;
 
 	if (nr_frags < ARRAY_SIZE(shinfo->frags)) {
-		struct page *page = virt_to_head_page(data);
-		int offset = data - page_address(page) + q->buf_offset;
+		netmem_ref netmem = virt_to_head_netmem(data);
+		int offset = data - netmem_address(netmem) + q->buf_offset;
 
-		skb_add_rx_frag(skb, nr_frags, page, offset, len, q->buf_size);
+		skb_add_rx_frag_netmem(skb, nr_frags, netmem, offset, len, q->buf_size);
 	} else {
 		mt76_put_page_pool_buf(data, allow_direct);
 	}
diff --git a/drivers/net/wireless/mediatek/mt76/mt76.h b/drivers/net/wireless/mediatek/mt76/mt76.h
index 14927a92f9d1..5fbc15a8cb06 100644
--- a/drivers/net/wireless/mediatek/mt76/mt76.h
+++ b/drivers/net/wireless/mediatek/mt76/mt76.h
@@ -1796,21 +1796,21 @@ int mt76_rx_token_consume(struct mt76_dev *dev, void *ptr,
 int mt76_create_page_pool(struct mt76_dev *dev, struct mt76_queue *q);
 static inline void mt76_put_page_pool_buf(void *buf, bool allow_direct)
 {
-	struct page *page = virt_to_head_page(buf);
+	netmem_ref netmem = virt_to_head_netmem(buf);
 
-	page_pool_put_full_page(page->pp, page, allow_direct);
+	page_pool_put_full_netmem(netmem_get_pp(netmem), netmem, allow_direct);
 }
 
 static inline void *
 mt76_get_page_pool_buf(struct mt76_queue *q, u32 *offset, u32 size)
 {
-	struct page *page;
+	netmem_ref netmem;
 
-	page = page_pool_dev_alloc_frag(q->page_pool, offset, size);
-	if (!page)
+	netmem = page_pool_dev_alloc_netmem(q->page_pool, offset, &size);
+	if (!netmem)
 		return NULL;
 
-	return page_address(page) + *offset;
+	return netmem_address(netmem) + *offset;
 }
 
 static inline void mt76_set_tx_blocked(struct mt76_dev *dev, bool blocked)
diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
index 0a927a7313a6..b1d89b6f663d 100644
--- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
+++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
@@ -68,14 +68,14 @@ mt76s_build_rx_skb(void *data, int data_len, int buf_len)
 
 	skb_put_data(skb, data, len);
 	if (data_len > len) {
-		struct page *page;
+		netmem_ref netmem;
 
 		data += len;
-		page = virt_to_head_page(data);
-		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
-				page, data - page_address(page),
-				data_len - len, buf_len);
-		get_page(page);
+		netmem = virt_to_head_netmem(data);
+		skb_add_rx_frag_netmem(skb, skb_shinfo(skb)->nr_frags,
+				       netmem, data - netmem_address(netmem),
+				       data_len - len, buf_len);
+		get_netmem(netmem);
 	}
 
 	return skb;
@@ -88,7 +88,7 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
 	struct mt76_queue *q = &dev->q_rx[qid];
 	struct mt76_sdio *sdio = &dev->sdio;
 	int len = 0, err, i;
-	struct page *page;
+	netmem_ref netmem;
 	u8 *buf, *end;
 
 	for (i = 0; i < intr->rx.num[qid]; i++)
@@ -100,11 +100,11 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
 	if (len > sdio->func->cur_blksize)
 		len = roundup(len, sdio->func->cur_blksize);
 
-	page = __dev_alloc_pages(GFP_KERNEL, get_order(len));
-	if (!page)
+	netmem = page_to_netmem(__dev_alloc_pages(GFP_KERNEL, get_order(len)));
+	if (!netmem)
 		return -ENOMEM;
 
-	buf = page_address(page);
+	buf = netmem_address(netmem);
 
 	sdio_claim_host(sdio->func);
 	err = sdio_readsb(sdio->func, buf, MCR_WRDR(qid), len);
@@ -112,7 +112,7 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
 
 	if (err < 0) {
 		dev_err(dev->dev, "sdio read data failed:%d\n", err);
-		put_page(page);
+		put_netmem(netmem);
 		return err;
 	}
 
@@ -140,7 +140,7 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
 		}
 		buf += round_up(len + 4, 4);
 	}
-	put_page(page);
+	put_netmem(netmem);
 
 	spin_lock_bh(&q->lock);
 	q->head = (q->head + i) % q->ndesc;
diff --git a/drivers/net/wireless/mediatek/mt76/usb.c b/drivers/net/wireless/mediatek/mt76/usb.c
index f9e67b8c3b3c..1ea80c87a839 100644
--- a/drivers/net/wireless/mediatek/mt76/usb.c
+++ b/drivers/net/wireless/mediatek/mt76/usb.c
@@ -478,7 +478,7 @@ mt76u_build_rx_skb(struct mt76_dev *dev, void *data,
 
 	head_room = drv_flags & MT_DRV_RX_DMA_HDR ? 0 : MT_DMA_HDR_LEN;
 	if (SKB_WITH_OVERHEAD(buf_size) < head_room + len) {
-		struct page *page;
+		netmem_ref netmem;
 
 		/* slow path, not enough space for data and
 		 * skb_shared_info
@@ -489,10 +489,10 @@ mt76u_build_rx_skb(struct mt76_dev *dev, void *data,
 
 		skb_put_data(skb, data + head_room, MT_SKB_HEAD_LEN);
 		data += head_room + MT_SKB_HEAD_LEN;
-		page = virt_to_head_page(data);
-		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
-				page, data - page_address(page),
-				len - MT_SKB_HEAD_LEN, buf_size);
+		netmem = virt_to_head_netmem(data);
+		skb_add_rx_frag_netmem(skb, skb_shinfo(skb)->nr_frags,
+				       netmem, data - netmem_address(netmem),
+				       len - MT_SKB_HEAD_LEN, buf_size);
 
 		return skb;
 	}
-- 
2.17.1

Re: [PATCH net-next v9 0/8] Split netmem from struct page

[Byungchul Park]

On Thu, Jul 10, 2025 at 05:27:59PM +0900, Byungchul Park wrote:
> Hi all,
> 
> The MM subsystem is trying to reduce struct page to a single pointer.
> See the following link for your information:
> 
>    https://kernelnewbies.org/MatthewWilcox/Memdescs/Path
> 
> The first step towards that is splitting struct page by its individual
> users, as has already been done with folio and slab.  This patchset does
> that for page pool.
> 
> Matthew Wilcox tried and stopped the same work, you can see in:
> 
>    https://lore.kernel.org/linux-mm/20230111042214.907030-1-willy@infradead.org/
> 
> I focused on removing the page pool members in struct page this time,
> not moving the allocation code of page pool from net to mm.  It can be
> done later if needed.
> 
> The final patch removing the page pool fields will be posted once all
> the converting of page to netmem are done:
> 
>    1. converting use of the pp fields in struct page in prueth_swdata.
>    2. converting use of the pp fields in struct page in freescale driver.

     3. converting use of the pp fields in strcut page in libeth.

	Byungchul

> For our discussion, I'm sharing what the final patch looks like, in this
> cover letter.
> 
> 	Byungchul
> --8<--
> commit 1847d9890f798456b21ccb27aac7545303048492
> Author: Byungchul Park <byungchul@sk.com>
> Date:   Wed May 28 20:44:55 2025 +0900
> 
>     mm, netmem: remove the page pool members in struct page
>     
>     Now that all the users of the page pool members in struct page have been
>     gone, the members can be removed from struct page.
>     
>     However, since struct netmem_desc still uses the space in struct page,
>     the important offsets should be checked properly, until struct
>     netmem_desc has its own instance from slab.
>     
>     Remove the page pool members in struct page and modify static checkers
>     for the offsets.
>     
>     Signed-off-by: Byungchul Park <byungchul@sk.com>
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 32ba5126e221..db2fe0d0ebbf 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -120,17 +120,6 @@ struct page {
>  			 */
>  			unsigned long private;
>  		};
> -		struct {	/* page_pool used by netstack */
> -			/**
> -			 * @pp_magic: magic value to avoid recycling non
> -			 * page_pool allocated pages.
> -			 */
> -			unsigned long pp_magic;
> -			struct page_pool *pp;
> -			unsigned long _pp_mapping_pad;
> -			unsigned long dma_addr;
> -			atomic_long_t pp_ref_count;
> -		};
>  		struct {	/* Tail pages of compound page */
>  			unsigned long compound_head;	/* Bit zero is set */
>  		};
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 8f354ae7d5c3..3414f184d018 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -42,11 +42,8 @@ struct netmem_desc {
>  	static_assert(offsetof(struct page, pg) == \
>  		      offsetof(struct netmem_desc, desc))
>  NETMEM_DESC_ASSERT_OFFSET(flags, _flags);
> -NETMEM_DESC_ASSERT_OFFSET(pp_magic, pp_magic);
> -NETMEM_DESC_ASSERT_OFFSET(pp, pp);
> -NETMEM_DESC_ASSERT_OFFSET(_pp_mapping_pad, _pp_mapping_pad);
> -NETMEM_DESC_ASSERT_OFFSET(dma_addr, dma_addr);
> -NETMEM_DESC_ASSERT_OFFSET(pp_ref_count, pp_ref_count);
> +NETMEM_DESC_ASSERT_OFFSET(lru, pp_magic);
> +NETMEM_DESC_ASSERT_OFFSET(mapping, _pp_mapping_pad);
>  #undef NETMEM_DESC_ASSERT_OFFSET
>  
>  /*
> ---
> Changes from v8:
> 	1. Rebase on net-next/main as of Jul 10.
> 	2. Exclude non-controversial patches that have already been
> 	   merged to net-next.
> 	3. Re-add the patches that focus on removing accessing the page
> 	   pool fields in struct page.
> 	4. Add utility APIs e.g. casting, to use struct netmem_desc as
> 	   descriptor, to support __netmem_get_pp() that has started to
> 	   be used again e.g. by libeth.
> 
> Changes from v7 (no actual updates):
> 	1. Exclude "netmem: introduce struct netmem_desc mirroring
> 	   struct page" that might be controversial.
> 	2. Exclude "netmem: introduce a netmem API,
> 	   virt_to_head_netmem()" since there are no users.
> 
> Changes from v6 (no actual updates):
> 	1. Rebase on net-next/main as of Jun 25.
> 	2. Supplement a comment describing struct net_iov.
> 	3. Exclude a controversial patch, "page_pool: access ->pp_magic
> 	   through struct netmem_desc in page_pool_page_is_pp()".
> 	4. Exclude "netmem: remove __netmem_get_pp()" since the API
> 	   started to be used again by libeth.
> 
> Changes from v5 (no actual updates):
> 	1. Rebase on net-next/main as of Jun 20.
> 	2. Add given 'Reviewed-by's and 'Acked-by's, thanks to all.
> 	3. Add missing cc's.
> 
> Changes from v4:
> 	1. Add given 'Reviewed-by's, thanks to all.
> 	2. Exclude potentially controversial patches.
> 
> Changes from v3:
> 	1. Relocates ->owner and ->type of net_iov out of netmem_desc
> 	   and make them be net_iov specific.
> 	2. Remove __force when casting struct page to struct netmem_desc.
> 
> Changes from v2:
> 	1. Introduce a netmem API, virt_to_head_netmem(), and use it
> 	   when it's needed.
> 	2. Introduce struct netmem_desc as a new struct and union'ed
> 	   with the existing fields in struct net_iov.
> 	3. Make page_pool_page_is_pp() access ->pp_magic through struct
> 	   netmem_desc instead of struct page.
> 	4. Move netmem alloc APIs from include/net/netmem.h to
> 	   net/core/netmem_priv.h.
> 	5. Apply trivial feedbacks, thanks to Mina, Pavel, and Toke.
> 	6. Add given 'Reviewed-by's, thanks to Mina.
> 
> Changes from v1:
> 	1. Rebase on net-next's main as of May 26.
> 	2. Check checkpatch.pl, feedbacked by SJ Park.
> 	3. Add converting of page to netmem in mt76.
> 	4. Revert 'mlx5: use netmem descriptor and APIs for page pool'
> 	   since it's on-going by Tariq Toukan.  I will wait for his
> 	   work to be done.
> 	5. Revert 'page_pool: use netmem APIs to access page->pp_magic
> 	   in page_pool_page_is_pp()' since we need more discussion.
> 	6. Revert 'mm, netmem: remove the page pool members in struct
> 	   page' since there are some prerequisite works to remove the
> 	   page pool fields from struct page.  I can submit this patch
> 	   separatedly later.
> 	7. Cancel relocating a page pool member in struct page.
> 	8. Modify static assert for offests and size of struct
> 	   netmem_desc.
> 
> Changes from rfc:
> 	1. Rebase on net-next's main branch.
> 	   https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/
> 	2. Fix a build error reported by kernel test robot.
> 	   https://lore.kernel.org/all/202505100932.uzAMBW1y-lkp@intel.com/
> 	3. Add given 'Reviewed-by's, thanks to Mina and Ilias.
> 	4. Do static_assert() on the size of struct netmem_desc instead
> 	   of placing place-holder in struct page, feedbacked by
> 	   Matthew.
> 	5. Do struct_group_tagged(netmem_desc) on struct net_iov instead
> 	   of wholly renaming it to strcut netmem_desc, feedbacked by
> 	   Mina and Pavel.
> 
> Byungchul Park (8):
>   netmem: introduce struct netmem_desc mirroring struct page
>   netmem: introduce utility APIs to use struct netmem_desc
>   page_pool: access ->pp_magic through struct netmem_desc in
>     page_pool_page_is_pp()
>   netmem: use netmem_desc instead of page to access ->pp in
>     __netmem_get_pp()
>   netmem: introduce a netmem API, virt_to_head_netmem()
>   mlx4: use netmem descriptor and APIs for page pool
>   netdevsim: use netmem descriptor and APIs for page pool
>   mt76: use netmem descriptor and APIs for page pool
> 
>  drivers/net/ethernet/mellanox/mlx4/en_rx.c    |  48 ++---
>  drivers/net/ethernet/mellanox/mlx4/en_tx.c    |   8 +-
>  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h  |   4 +-
>  drivers/net/netdevsim/netdev.c                |  19 +-
>  drivers/net/netdevsim/netdevsim.h             |   2 +-
>  drivers/net/wireless/mediatek/mt76/dma.c      |   6 +-
>  drivers/net/wireless/mediatek/mt76/mt76.h     |  12 +-
>  .../net/wireless/mediatek/mt76/sdio_txrx.c    |  24 +--
>  drivers/net/wireless/mediatek/mt76/usb.c      |  10 +-
>  include/linux/mm.h                            |  12 --
>  include/net/netmem.h                          | 183 +++++++++++++++---
>  mm/page_alloc.c                               |   1 +
>  12 files changed, 231 insertions(+), 98 deletions(-)
> 
> 
> base-commit: c65d34296b2252897e37835d6007bbd01b255742
> -- 
> 2.17.1

Re: [PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Mina Almasry]

On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
>
> To eliminate the use of the page pool fields in struct page, the page
> pool code should use netmem descriptor and APIs instead.
>
> However, some code e.g. __netmem_to_page() is still used to access the
> page pool fields e.g. ->pp via struct page, which should be changed so
> as to access them via netmem descriptor, struct netmem_desc instead,
> since the fields no longer will be available in struct page.
>
> Introduce utility APIs to make them easy to use struct netmem_desc as
> descriptor.  The APIs are:
>
>    1. __netmem_to_nmdesc(), to convert netmem_ref to struct netmem_desc,
>       but unsafely without checking if it's net_iov or system memory.
>
>    2. netmem_to_nmdesc(), to convert netmem_ref to struct netmem_desc,
>       safely with checking if it's net_iov or system memory.
>
>    3. nmdesc_to_page(), to convert struct netmem_desc to struct page,
>       assuming struct netmem_desc overlays on struct page.
>
>    4. page_to_nmdesc(), to convert struct page to struct netmem_desc,
>       assuming struct netmem_desc overlays on struct page, allowing only
>       head page to be converted.
>
>    5. nmdesc_adress(), to get its virtual address corresponding to the
>       struct netmem_desc.
>
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> ---
>  include/net/netmem.h | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
>
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 535cf17b9134..ad9444be229a 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -198,6 +198,32 @@ static inline struct page *netmem_to_page(netmem_ref netmem)
>         return __netmem_to_page(netmem);
>  }
>
> +/**
> + * __netmem_to_nmdesc - unsafely get pointer to the &netmem_desc backing
> + * @netmem
> + * @netmem: netmem reference to convert
> + *
> + * Unsafe version of netmem_to_nmdesc(). When @netmem is always backed
> + * by system memory, performs faster and generates smaller object code
> + * (no check for the LSB, no WARN). When @netmem points to IOV, provokes
> + * undefined behaviour.
> + *
> + * Return: pointer to the &netmem_desc (garbage if @netmem is not backed
> + * by system memory).
> + */
> +static inline struct netmem_desc *__netmem_to_nmdesc(netmem_ref netmem)
> +{
> +       return (__force struct netmem_desc *)netmem;
> +}
> +

Does a netmem_desc represent the pp fields shared between struct page
and struct net_iov, or does netmem_desc represent paged kernel memory?
If the former, I don't think we need a safe and unsafe version of this
helper, since netmem_ref always has netmem_desc fields underneath. If
the latter, then this helper should not exist at all. We should not
allow casting netmem_ref to a netmem_desc without first checking if
it's a net_iov.

To be honest the cover letter should come up with a detailed
explanation of (a) what are the current types (b) what are the new
types (c) what are the relationships between the types, so these
questions stop coming up.

> +static inline struct netmem_desc *netmem_to_nmdesc(netmem_ref netmem)
> +{
> +       if (WARN_ON_ONCE(netmem_is_net_iov(netmem)))
> +               return NULL;
> +
> +       return __netmem_to_nmdesc(netmem);
> +}
> +
>  static inline struct net_iov *netmem_to_net_iov(netmem_ref netmem)
>  {
>         if (netmem_is_net_iov(netmem))
> @@ -314,6 +340,21 @@ static inline netmem_ref netmem_compound_head(netmem_ref netmem)
>         return page_to_netmem(compound_head(netmem_to_page(netmem)));
>  }
>
> +#define nmdesc_to_page(nmdesc)         (_Generic((nmdesc),             \
> +       const struct netmem_desc * :    (const struct page *)(nmdesc),  \
> +       struct netmem_desc * :          (struct page *)(nmdesc)))
> +
> +static inline struct netmem_desc *page_to_nmdesc(struct page *page)
> +{
> +       VM_BUG_ON_PAGE(PageTail(page), page);
> +       return (struct netmem_desc *)page;
> +}
> +

It's not safe to cast a page to netmem_desc, without first checking if
it's a pp page or not, otherwise you may be casting random non-pp
pages to netmem_desc...

> +static inline void *nmdesc_address(struct netmem_desc *nmdesc)
> +{
> +       return page_address(nmdesc_to_page(nmdesc));
> +}
> +
>  /**

Introduce helpers in the same patch that uses them please. Having to
cross reference your series to see if there are any callers to this
(and the callers are correct) is an unnecessary burden to the
reviewers.

-- 
Thanks,
Mina

Re: [PATCH net-next v9 3/8] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[Mina Almasry]

On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
>
> To simplify struct page, the effort to separate its own descriptor from
> struct page is required and the work for page pool is on going.
>
> To achieve that, all the code should avoid directly accessing page pool
> members of struct page.
>
> Access ->pp_magic through struct netmem_desc instead of directly
> accessing it through struct page in page_pool_page_is_pp().  Plus, move
> page_pool_page_is_pp() from mm.h to netmem.h to use struct netmem_desc
> without header dependency issue.
>
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Reviewed-by: Mina Almasry <almasrymina@google.com>
> Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
> Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> Acked-by: Harry Yoo <harry.yoo@oracle.com>
> ---
>  include/linux/mm.h   | 12 ------------
>  include/net/netmem.h | 17 +++++++++++++++++
>  mm/page_alloc.c      |  1 +
>  3 files changed, 18 insertions(+), 12 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0ef2ba0c667a..0b7f7f998085 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4172,16 +4172,4 @@ int arch_lock_shadow_stack_status(struct task_struct *t, unsigned long status);
>   */
>  #define PP_MAGIC_MASK ~(PP_DMA_INDEX_MASK | 0x3UL)
>
> -#ifdef CONFIG_PAGE_POOL
> -static inline bool page_pool_page_is_pp(struct page *page)
> -{
> -       return (page->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> -}
> -#else
> -static inline bool page_pool_page_is_pp(struct page *page)
> -{
> -       return false;
> -}
> -#endif
> -
>  #endif /* _LINUX_MM_H */
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index ad9444be229a..11e9de45efcb 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -355,6 +355,23 @@ static inline void *nmdesc_address(struct netmem_desc *nmdesc)
>         return page_address(nmdesc_to_page(nmdesc));
>  }
>
> +#ifdef CONFIG_PAGE_POOL
> +/* XXX: This would better be moved to mm, once mm gets its way to
> + * identify the type of page for page pool.
> + */
> +static inline bool page_pool_page_is_pp(struct page *page)
> +{
> +       struct netmem_desc *desc = page_to_nmdesc(page);
> +
> +       return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> +}

pages can be pp pages (where they have pp fields inside of them) or
non-pp pages (where they don't have pp fields inside them, because
they were never allocated from the page_pool).

Casting a page to a netmem_desc, and then checking if the page was a
pp page doesn't makes sense to me on a fundamental level. The
netmem_desc is only valid if the page was a pp page in the first
place. Maybe page_to_nmdesc should reject the cast if the page is not
a pp page or something.

-- 
Thanks,
Mina

Re: [PATCH net-next v9 4/8] netmem: use netmem_desc instead of page to access ->pp in __netmem_get_pp()

[Mina Almasry]

On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
>
> To eliminate the use of the page pool fields in struct page, the page
> pool code should use netmem descriptor and APIs instead.
>
> However, __netmem_get_pp() still accesses ->pp via struct page.  So
> change it to use struct netmem_desc instead, since ->pp no longer will
> be available in struct page.
>
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> ---
>  include/net/netmem.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 11e9de45efcb..283b4a997fbc 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -306,7 +306,7 @@ static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem)
>   */
>  static inline struct page_pool *__netmem_get_pp(netmem_ref netmem)
>  {
> -       return __netmem_to_page(netmem)->pp;
> +       return __netmem_to_nmdesc(netmem)->pp;
>  }
>

__netmem_to_nmdesc should introduced with this patch.

But also, I wonder why not modify all the callsites of
__netmem_to_page to the new __netmem_to_nmdesc and delete the
__nemem_to_page helper?


-- 
Thanks,
Mina

Re: [PATCH net-next v9 7/8] netdevsim: use netmem descriptor and APIs for page pool

[Mina Almasry]

On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
>
> To simplify struct page, the effort to separate its own descriptor from
> struct page is required and the work for page pool is on going.
>
> Use netmem descriptor and APIs for page pool in netdevsim code.
>
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> ---
>  drivers/net/netdevsim/netdev.c    | 19 ++++++++++---------
>  drivers/net/netdevsim/netdevsim.h |  2 +-
>  2 files changed, 11 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/netdevsim/netdev.c b/drivers/net/netdevsim/netdev.c
> index e36d3e846c2d..ba19870524c5 100644
> --- a/drivers/net/netdevsim/netdev.c
> +++ b/drivers/net/netdevsim/netdev.c
> @@ -812,7 +812,7 @@ nsim_pp_hold_read(struct file *file, char __user *data,
>         struct netdevsim *ns = file->private_data;
>         char buf[3] = "n\n";
>
> -       if (ns->page)
> +       if (ns->netmem)
>                 buf[0] = 'y';
>
>         return simple_read_from_buffer(data, count, ppos, buf, 2);
> @@ -832,18 +832,19 @@ nsim_pp_hold_write(struct file *file, const char __user *data,
>
>         rtnl_lock();
>         ret = count;
> -       if (val == !!ns->page)
> +       if (val == !!ns->netmem)
>                 goto exit;
>
>         if (!netif_running(ns->netdev) && val) {
>                 ret = -ENETDOWN;
>         } else if (val) {
> -               ns->page = page_pool_dev_alloc_pages(ns->rq[0]->page_pool);
> -               if (!ns->page)
> +               ns->netmem = page_pool_alloc_netmems(ns->rq[0]->page_pool,
> +                                                    GFP_ATOMIC | __GFP_NOWARN);

Add page_pool_dev_alloc_netmems helper.


-- 
Thanks,
Mina

Re: [PATCH net-next v9 5/8] netmem: introduce a netmem API, virt_to_head_netmem()

[Mina Almasry]

On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
>
> To eliminate the use of struct page in page pool, the page pool code
> should use netmem descriptor and APIs instead.
>
> As part of the work, introduce a netmem API to convert a virtual address
> to a head netmem allowing the code to use it rather than the existing
> API, virt_to_head_page() for struct page.
>
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
> Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
> Reviewed-by: Mina Almasry <almasrymina@google.com>
> ---
>  include/net/netmem.h | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 283b4a997fbc..b92c7f15166a 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -372,6 +372,13 @@ static inline bool page_pool_page_is_pp(struct page *page)
>  }
>  #endif
>
> +static inline netmem_ref virt_to_head_netmem(const void *x)
> +{
> +       netmem_ref netmem = virt_to_netmem(x);
> +
> +       return netmem_compound_head(netmem);
> +}
> +

Squash with the first user of this helper please.


-- 
Thanks,
Mina

Re: [PATCH net-next v9 6/8] mlx4: use netmem descriptor and APIs for page pool

[Mina Almasry]

On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
>
> To simplify struct page, the effort to separate its own descriptor from
> struct page is required and the work for page pool is on going.
>
> Use netmem descriptor and APIs for page pool in mlx4 code.
>
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> ---
>  drivers/net/ethernet/mellanox/mlx4/en_rx.c   | 48 +++++++++++---------
>  drivers/net/ethernet/mellanox/mlx4/en_tx.c   |  8 ++--
>  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h |  4 +-
>  3 files changed, 32 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> index b33285d755b9..7cf0d2dc5011 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> @@ -62,18 +62,18 @@ static int mlx4_en_alloc_frags(struct mlx4_en_priv *priv,
>         int i;
>
>         for (i = 0; i < priv->num_frags; i++, frags++) {
> -               if (!frags->page) {
> -                       frags->page = page_pool_alloc_pages(ring->pp, gfp);
> -                       if (!frags->page) {
> +               if (!frags->netmem) {
> +                       frags->netmem = page_pool_alloc_netmems(ring->pp, gfp);
> +                       if (!frags->netmem) {
>                                 ring->alloc_fail++;
>                                 return -ENOMEM;
>                         }
> -                       page_pool_fragment_page(frags->page, 1);
> +                       page_pool_fragment_netmem(frags->netmem, 1);
>                         frags->page_offset = priv->rx_headroom;
>
>                         ring->rx_alloc_pages++;
>                 }
> -               dma = page_pool_get_dma_addr(frags->page);
> +               dma = page_pool_get_dma_addr_netmem(frags->netmem);
>                 rx_desc->data[i].addr = cpu_to_be64(dma + frags->page_offset);
>         }
>         return 0;
> @@ -83,10 +83,10 @@ static void mlx4_en_free_frag(const struct mlx4_en_priv *priv,
>                               struct mlx4_en_rx_ring *ring,
>                               struct mlx4_en_rx_alloc *frag)
>  {
> -       if (frag->page)
> -               page_pool_put_full_page(ring->pp, frag->page, false);
> +       if (frag->netmem)
> +               page_pool_put_full_netmem(ring->pp, frag->netmem, false);
>         /* We need to clear all fields, otherwise a change of priv->log_rx_info
> -        * could lead to see garbage later in frag->page.
> +        * could lead to see garbage later in frag->netmem.
>          */
>         memset(frag, 0, sizeof(*frag));
>  }
> @@ -440,29 +440,33 @@ static int mlx4_en_complete_rx_desc(struct mlx4_en_priv *priv,
>         unsigned int truesize = 0;
>         bool release = true;
>         int nr, frag_size;
> -       struct page *page;
> +       netmem_ref netmem;
>         dma_addr_t dma;
>
>         /* Collect used fragments while replacing them in the HW descriptors */
>         for (nr = 0;; frags++) {
>                 frag_size = min_t(int, length, frag_info->frag_size);
>
> -               page = frags->page;
> -               if (unlikely(!page))
> +               netmem = frags->netmem;
> +               if (unlikely(!netmem))
>                         goto fail;
>
> -               dma = page_pool_get_dma_addr(page);
> +               dma = page_pool_get_dma_addr_netmem(netmem);
>                 dma_sync_single_range_for_cpu(priv->ddev, dma, frags->page_offset,
>                                               frag_size, priv->dma_dir);
>
> -               __skb_fill_page_desc(skb, nr, page, frags->page_offset,
> -                                    frag_size);
> +               __skb_fill_netmem_desc(skb, nr, netmem, frags->page_offset,
> +                                      frag_size);
>
>                 truesize += frag_info->frag_stride;
>                 if (frag_info->frag_stride == PAGE_SIZE / 2) {
> +                       struct page *page = netmem_to_page(netmem);

This cast is not safe, try to use the netmem type directly.

-- 
Thanks,
Mina

Re: [PATCH net-next v9 0/8] Split netmem from struct page

[Mina Almasry]

On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
>
> Hi all,
>
> The MM subsystem is trying to reduce struct page to a single pointer.
> See the following link for your information:
>
>    https://kernelnewbies.org/MatthewWilcox/Memdescs/Path
>
> The first step towards that is splitting struct page by its individual
> users, as has already been done with folio and slab.  This patchset does
> that for page pool.
>
> Matthew Wilcox tried and stopped the same work, you can see in:
>
>    https://lore.kernel.org/linux-mm/20230111042214.907030-1-willy@infradead.org/
>
> I focused on removing the page pool members in struct page this time,
> not moving the allocation code of page pool from net to mm.  It can be
> done later if needed.
>
> The final patch removing the page pool fields will be posted once all
> the converting of page to netmem are done:
>
>    1. converting use of the pp fields in struct page in prueth_swdata.
>    2. converting use of the pp fields in struct page in freescale driver.
>
> For our discussion, I'm sharing what the final patch looks like, in this
> cover letter.
>
>         Byungchul
> --8<--
> commit 1847d9890f798456b21ccb27aac7545303048492
> Author: Byungchul Park <byungchul@sk.com>
> Date:   Wed May 28 20:44:55 2025 +0900
>
>     mm, netmem: remove the page pool members in struct page
>
>     Now that all the users of the page pool members in struct page have been
>     gone, the members can be removed from struct page.
>
>     However, since struct netmem_desc still uses the space in struct page,
>     the important offsets should be checked properly, until struct
>     netmem_desc has its own instance from slab.
>
>     Remove the page pool members in struct page and modify static checkers
>     for the offsets.
>
>     Signed-off-by: Byungchul Park <byungchul@sk.com>
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 32ba5126e221..db2fe0d0ebbf 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -120,17 +120,6 @@ struct page {
>                          */
>                         unsigned long private;
>                 };
> -               struct {        /* page_pool used by netstack */
> -                       /**
> -                        * @pp_magic: magic value to avoid recycling non
> -                        * page_pool allocated pages.
> -                        */
> -                       unsigned long pp_magic;
> -                       struct page_pool *pp;
> -                       unsigned long _pp_mapping_pad;
> -                       unsigned long dma_addr;
> -                       atomic_long_t pp_ref_count;
> -               };
>                 struct {        /* Tail pages of compound page */
>                         unsigned long compound_head;    /* Bit zero is set */
>                 };
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 8f354ae7d5c3..3414f184d018 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -42,11 +42,8 @@ struct netmem_desc {
>         static_assert(offsetof(struct page, pg) == \
>                       offsetof(struct netmem_desc, desc))
>  NETMEM_DESC_ASSERT_OFFSET(flags, _flags);
> -NETMEM_DESC_ASSERT_OFFSET(pp_magic, pp_magic);
> -NETMEM_DESC_ASSERT_OFFSET(pp, pp);
> -NETMEM_DESC_ASSERT_OFFSET(_pp_mapping_pad, _pp_mapping_pad);
> -NETMEM_DESC_ASSERT_OFFSET(dma_addr, dma_addr);
> -NETMEM_DESC_ASSERT_OFFSET(pp_ref_count, pp_ref_count);
> +NETMEM_DESC_ASSERT_OFFSET(lru, pp_magic);
> +NETMEM_DESC_ASSERT_OFFSET(mapping, _pp_mapping_pad);
>  #undef NETMEM_DESC_ASSERT_OFFSET
>
>  /*


Can you remove the above patch/diff from the cover letter?

-- 
Thanks,
Mina

Re: [PATCH net-next v9 0/8] Split netmem from struct page

[Byungchul Park]

On Thu, Jul 10, 2025 at 11:35:33AM -0700, Mina Almasry wrote:
> On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
> >
> > Hi all,
> >
> > The MM subsystem is trying to reduce struct page to a single pointer.
> > See the following link for your information:
> >
> >    https://kernelnewbies.org/MatthewWilcox/Memdescs/Path
> >
> > The first step towards that is splitting struct page by its individual
> > users, as has already been done with folio and slab.  This patchset does
> > that for page pool.
> >
> > Matthew Wilcox tried and stopped the same work, you can see in:
> >
> >    https://lore.kernel.org/linux-mm/20230111042214.907030-1-willy@infradead.org/
> >
> > I focused on removing the page pool members in struct page this time,
> > not moving the allocation code of page pool from net to mm.  It can be
> > done later if needed.
> >
> > The final patch removing the page pool fields will be posted once all
> > the converting of page to netmem are done:
> >
> >    1. converting use of the pp fields in struct page in prueth_swdata.
> >    2. converting use of the pp fields in struct page in freescale driver.
> >
> > For our discussion, I'm sharing what the final patch looks like, in this
> > cover letter.
> >
> >         Byungchul
> > --8<--
> > commit 1847d9890f798456b21ccb27aac7545303048492
> > Author: Byungchul Park <byungchul@sk.com>
> > Date:   Wed May 28 20:44:55 2025 +0900
> >
> >     mm, netmem: remove the page pool members in struct page
> >
> >     Now that all the users of the page pool members in struct page have been
> >     gone, the members can be removed from struct page.
> >
> >     However, since struct netmem_desc still uses the space in struct page,
> >     the important offsets should be checked properly, until struct
> >     netmem_desc has its own instance from slab.
> >
> >     Remove the page pool members in struct page and modify static checkers
> >     for the offsets.
> >
> >     Signed-off-by: Byungchul Park <byungchul@sk.com>
> >
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 32ba5126e221..db2fe0d0ebbf 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -120,17 +120,6 @@ struct page {
> >                          */
> >                         unsigned long private;
> >                 };
> > -               struct {        /* page_pool used by netstack */
> > -                       /**
> > -                        * @pp_magic: magic value to avoid recycling non
> > -                        * page_pool allocated pages.
> > -                        */
> > -                       unsigned long pp_magic;
> > -                       struct page_pool *pp;
> > -                       unsigned long _pp_mapping_pad;
> > -                       unsigned long dma_addr;
> > -                       atomic_long_t pp_ref_count;
> > -               };
> >                 struct {        /* Tail pages of compound page */
> >                         unsigned long compound_head;    /* Bit zero is set */
> >                 };
> > diff --git a/include/net/netmem.h b/include/net/netmem.h
> > index 8f354ae7d5c3..3414f184d018 100644
> > --- a/include/net/netmem.h
> > +++ b/include/net/netmem.h
> > @@ -42,11 +42,8 @@ struct netmem_desc {
> >         static_assert(offsetof(struct page, pg) == \
> >                       offsetof(struct netmem_desc, desc))
> >  NETMEM_DESC_ASSERT_OFFSET(flags, _flags);
> > -NETMEM_DESC_ASSERT_OFFSET(pp_magic, pp_magic);
> > -NETMEM_DESC_ASSERT_OFFSET(pp, pp);
> > -NETMEM_DESC_ASSERT_OFFSET(_pp_mapping_pad, _pp_mapping_pad);
> > -NETMEM_DESC_ASSERT_OFFSET(dma_addr, dma_addr);
> > -NETMEM_DESC_ASSERT_OFFSET(pp_ref_count, pp_ref_count);
> > +NETMEM_DESC_ASSERT_OFFSET(lru, pp_magic);
> > +NETMEM_DESC_ASSERT_OFFSET(mapping, _pp_mapping_pad);
> >  #undef NETMEM_DESC_ASSERT_OFFSET
> >
> >  /*
> 
> 
> Can you remove the above patch/diff from the cover letter?

I added the diff for those who might get lost due to the lack of the
final patch.  However, sure, I will remove it.

	Byungchul

> --
> Thanks,
> Mina

Re: [PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Byungchul Park]

On Thu, Jul 10, 2025 at 11:11:51AM -0700, Mina Almasry wrote:
> 
> On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
> >
> > To eliminate the use of the page pool fields in struct page, the page
> > pool code should use netmem descriptor and APIs instead.
> >
> > However, some code e.g. __netmem_to_page() is still used to access the
> > page pool fields e.g. ->pp via struct page, which should be changed so
> > as to access them via netmem descriptor, struct netmem_desc instead,
> > since the fields no longer will be available in struct page.
> >
> > Introduce utility APIs to make them easy to use struct netmem_desc as
> > descriptor.  The APIs are:
> >
> >    1. __netmem_to_nmdesc(), to convert netmem_ref to struct netmem_desc,
> >       but unsafely without checking if it's net_iov or system memory.
> >
> >    2. netmem_to_nmdesc(), to convert netmem_ref to struct netmem_desc,
> >       safely with checking if it's net_iov or system memory.
> >
> >    3. nmdesc_to_page(), to convert struct netmem_desc to struct page,
> >       assuming struct netmem_desc overlays on struct page.
> >
> >    4. page_to_nmdesc(), to convert struct page to struct netmem_desc,
> >       assuming struct netmem_desc overlays on struct page, allowing only
> >       head page to be converted.
> >
> >    5. nmdesc_adress(), to get its virtual address corresponding to the
> >       struct netmem_desc.
> >
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > ---
> >  include/net/netmem.h | 41 +++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >
> > diff --git a/include/net/netmem.h b/include/net/netmem.h
> > index 535cf17b9134..ad9444be229a 100644
> > --- a/include/net/netmem.h
> > +++ b/include/net/netmem.h
> > @@ -198,6 +198,32 @@ static inline struct page *netmem_to_page(netmem_ref netmem)
> >         return __netmem_to_page(netmem);
> >  }
> >
> > +/**
> > + * __netmem_to_nmdesc - unsafely get pointer to the &netmem_desc backing
> > + * @netmem
> > + * @netmem: netmem reference to convert
> > + *
> > + * Unsafe version of netmem_to_nmdesc(). When @netmem is always backed
> > + * by system memory, performs faster and generates smaller object code
> > + * (no check for the LSB, no WARN). When @netmem points to IOV, provokes
> > + * undefined behaviour.
> > + *
> > + * Return: pointer to the &netmem_desc (garbage if @netmem is not backed
> > + * by system memory).
> > + */
> > +static inline struct netmem_desc *__netmem_to_nmdesc(netmem_ref netmem)
> > +{
> > +       return (__force struct netmem_desc *)netmem;
> > +}
> > +
> 
> Does a netmem_desc represent the pp fields shared between struct page
> and struct net_iov, or does netmem_desc represent paged kernel memory?
> If the former, I don't think we need a safe and unsafe version of this
> helper, since netmem_ref always has netmem_desc fields underneath. If

Ah, right.  I missed the point.  I will narrow down into a single safe
helper for that purpose.

> the latter, then this helper should not exist at all. We should not
> allow casting netmem_ref to a netmem_desc without first checking if
> it's a net_iov.
> 
> To be honest the cover letter should come up with a detailed
> explanation of (a) what are the current types (b) what are the new
> types (c) what are the relationships between the types, so these
> questions stop coming up.

I will.  Thanks for the feedback.

> > +static inline struct netmem_desc *netmem_to_nmdesc(netmem_ref netmem)
> > +{
> > +       if (WARN_ON_ONCE(netmem_is_net_iov(netmem)))
> > +               return NULL;
> > +
> > +       return __netmem_to_nmdesc(netmem);
> > +}
> > +
> >  static inline struct net_iov *netmem_to_net_iov(netmem_ref netmem)
> >  {
> >         if (netmem_is_net_iov(netmem))
> > @@ -314,6 +340,21 @@ static inline netmem_ref netmem_compound_head(netmem_ref netmem)
> >         return page_to_netmem(compound_head(netmem_to_page(netmem)));
> >  }
> >
> > +#define nmdesc_to_page(nmdesc)         (_Generic((nmdesc),             \
> > +       const struct netmem_desc * :    (const struct page *)(nmdesc),  \
> > +       struct netmem_desc * :          (struct page *)(nmdesc)))
> > +
> > +static inline struct netmem_desc *page_to_nmdesc(struct page *page)
> > +{
> > +       VM_BUG_ON_PAGE(PageTail(page), page);
> > +       return (struct netmem_desc *)page;
> > +}
> > +
> 
> It's not safe to cast a page to netmem_desc, without first checking if
> it's a pp page or not, otherwise you may be casting random non-pp
> pages to netmem_desc...

Agree, but page_to_nmdesc() will be used in page_pool_page_is_pp() to
check if it's a pp page or not:

   static inline bool page_pool_page_is_pp(struct page *page)
   {
	struct netmem_desc *desc = page_to_nmdesc(page);

	return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
   }

Hm.. maybe, it'd be better to resore the original code and remove this
page_to_nmdesc() helper.  FYI, the original code was:

   static inline bool page_pool_page_is_pp(struct page *page)
   {
	struct netmem_desc *desc = (struct netmem_desc *)page;

	return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
   }

> > +static inline void *nmdesc_address(struct netmem_desc *nmdesc)
> > +{
> > +       return page_address(nmdesc_to_page(nmdesc));
> > +}
> > +
> >  /**
> 
> Introduce helpers in the same patch that uses them please. Having to
> cross reference your series to see if there are any callers to this
> (and the callers are correct) is an unnecessary burden to the
> reviewers.

I adopted how Matthew Wilcox worked on this[1], but sure, I will.

[1] https://lore.kernel.org/linux-mm/20230111042214.907030-3-willy@infradead.org/

	Byungchul

> --
> Thanks,
> Mina

Re: [PATCH net-next v9 3/8] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[Byungchul Park]

On Thu, Jul 10, 2025 at 11:19:53AM -0700, Mina Almasry wrote:
> On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
> >
> > To simplify struct page, the effort to separate its own descriptor from
> > struct page is required and the work for page pool is on going.
> >
> > To achieve that, all the code should avoid directly accessing page pool
> > members of struct page.
> >
> > Access ->pp_magic through struct netmem_desc instead of directly
> > accessing it through struct page in page_pool_page_is_pp().  Plus, move
> > page_pool_page_is_pp() from mm.h to netmem.h to use struct netmem_desc
> > without header dependency issue.
> >
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
> > Reviewed-by: Mina Almasry <almasrymina@google.com>
> > Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
> > Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> > Acked-by: Harry Yoo <harry.yoo@oracle.com>
> > ---
> >  include/linux/mm.h   | 12 ------------
> >  include/net/netmem.h | 17 +++++++++++++++++
> >  mm/page_alloc.c      |  1 +
> >  3 files changed, 18 insertions(+), 12 deletions(-)
> >
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 0ef2ba0c667a..0b7f7f998085 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -4172,16 +4172,4 @@ int arch_lock_shadow_stack_status(struct task_struct *t, unsigned long status);
> >   */
> >  #define PP_MAGIC_MASK ~(PP_DMA_INDEX_MASK | 0x3UL)
> >
> > -#ifdef CONFIG_PAGE_POOL
> > -static inline bool page_pool_page_is_pp(struct page *page)
> > -{
> > -       return (page->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> > -}
> > -#else
> > -static inline bool page_pool_page_is_pp(struct page *page)
> > -{
> > -       return false;
> > -}
> > -#endif
> > -
> >  #endif /* _LINUX_MM_H */
> > diff --git a/include/net/netmem.h b/include/net/netmem.h
> > index ad9444be229a..11e9de45efcb 100644
> > --- a/include/net/netmem.h
> > +++ b/include/net/netmem.h
> > @@ -355,6 +355,23 @@ static inline void *nmdesc_address(struct netmem_desc *nmdesc)
> >         return page_address(nmdesc_to_page(nmdesc));
> >  }
> >
> > +#ifdef CONFIG_PAGE_POOL
> > +/* XXX: This would better be moved to mm, once mm gets its way to
> > + * identify the type of page for page pool.
> > + */
> > +static inline bool page_pool_page_is_pp(struct page *page)
> > +{
> > +       struct netmem_desc *desc = page_to_nmdesc(page);
> > +
> > +       return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> > +}
> 
> pages can be pp pages (where they have pp fields inside of them) or
> non-pp pages (where they don't have pp fields inside them, because
> they were never allocated from the page_pool).
> 
> Casting a page to a netmem_desc, and then checking if the page was a
> pp page doesn't makes sense to me on a fundamental level. The
> netmem_desc is only valid if the page was a pp page in the first
> place. Maybe page_to_nmdesc should reject the cast if the page is not
> a pp page or something.

Right, as you already know, the current mainline code already has the
same problem but we've been using the werid way so far, in other words,
mm code is checking if it's a pp page or not by using ->pp_magic, but
it's ->lur, ->buddy_list, or ->pcp_list if it's not a pp page.

Both the mainline code and this patch can make sense *only if* it's
actually a pp page.  It's unevitable until mm provides a way to identify
the type of page for page pool.  Thoughts?

	Byungchul

> --
> Thanks,
> Mina

Re: [PATCH net-next v9 4/8] netmem: use netmem_desc instead of page to access ->pp in __netmem_get_pp()

[Byungchul Park]

On Thu, Jul 10, 2025 at 11:25:12AM -0700, Mina Almasry wrote:
> On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
> >
> > To eliminate the use of the page pool fields in struct page, the page
> > pool code should use netmem descriptor and APIs instead.
> >
> > However, __netmem_get_pp() still accesses ->pp via struct page.  So
> > change it to use struct netmem_desc instead, since ->pp no longer will
> > be available in struct page.
> >
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > ---
> >  include/net/netmem.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/net/netmem.h b/include/net/netmem.h
> > index 11e9de45efcb..283b4a997fbc 100644
> > --- a/include/net/netmem.h
> > +++ b/include/net/netmem.h
> > @@ -306,7 +306,7 @@ static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem)
> >   */
> >  static inline struct page_pool *__netmem_get_pp(netmem_ref netmem)
> >  {
> > -       return __netmem_to_page(netmem)->pp;
> > +       return __netmem_to_nmdesc(netmem)->pp;
> >  }
> >
> 
> __netmem_to_nmdesc should introduced with this patch.

Okay.

> But also, I wonder why not modify all the callsites of
> __netmem_to_page to the new __netmem_to_nmdesc and delete the
> __nemem_to_page helper?

It'd be better.  I will.  Thanks.

	Byungchul
> 
> 
> --
> Thanks,
> Mina

Re: [PATCH net-next v9 6/8] mlx4: use netmem descriptor and APIs for page pool

[Byungchul Park]

On Thu, Jul 10, 2025 at 11:29:35AM -0700, Mina Almasry wrote:
> On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
> >
> > To simplify struct page, the effort to separate its own descriptor from
> > struct page is required and the work for page pool is on going.
> >
> > Use netmem descriptor and APIs for page pool in mlx4 code.
> >
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > ---
> >  drivers/net/ethernet/mellanox/mlx4/en_rx.c   | 48 +++++++++++---------
> >  drivers/net/ethernet/mellanox/mlx4/en_tx.c   |  8 ++--
> >  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h |  4 +-
> >  3 files changed, 32 insertions(+), 28 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> > index b33285d755b9..7cf0d2dc5011 100644
> > --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> > +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> > @@ -62,18 +62,18 @@ static int mlx4_en_alloc_frags(struct mlx4_en_priv *priv,
> >         int i;
> >
> >         for (i = 0; i < priv->num_frags; i++, frags++) {
> > -               if (!frags->page) {
> > -                       frags->page = page_pool_alloc_pages(ring->pp, gfp);
> > -                       if (!frags->page) {
> > +               if (!frags->netmem) {
> > +                       frags->netmem = page_pool_alloc_netmems(ring->pp, gfp);
> > +                       if (!frags->netmem) {
> >                                 ring->alloc_fail++;
> >                                 return -ENOMEM;
> >                         }
> > -                       page_pool_fragment_page(frags->page, 1);
> > +                       page_pool_fragment_netmem(frags->netmem, 1);
> >                         frags->page_offset = priv->rx_headroom;
> >
> >                         ring->rx_alloc_pages++;
> >                 }
> > -               dma = page_pool_get_dma_addr(frags->page);
> > +               dma = page_pool_get_dma_addr_netmem(frags->netmem);
> >                 rx_desc->data[i].addr = cpu_to_be64(dma + frags->page_offset);
> >         }
> >         return 0;
> > @@ -83,10 +83,10 @@ static void mlx4_en_free_frag(const struct mlx4_en_priv *priv,
> >                               struct mlx4_en_rx_ring *ring,
> >                               struct mlx4_en_rx_alloc *frag)
> >  {
> > -       if (frag->page)
> > -               page_pool_put_full_page(ring->pp, frag->page, false);
> > +       if (frag->netmem)
> > +               page_pool_put_full_netmem(ring->pp, frag->netmem, false);
> >         /* We need to clear all fields, otherwise a change of priv->log_rx_info
> > -        * could lead to see garbage later in frag->page.
> > +        * could lead to see garbage later in frag->netmem.
> >          */
> >         memset(frag, 0, sizeof(*frag));
> >  }
> > @@ -440,29 +440,33 @@ static int mlx4_en_complete_rx_desc(struct mlx4_en_priv *priv,
> >         unsigned int truesize = 0;
> >         bool release = true;
> >         int nr, frag_size;
> > -       struct page *page;
> > +       netmem_ref netmem;
> >         dma_addr_t dma;
> >
> >         /* Collect used fragments while replacing them in the HW descriptors */
> >         for (nr = 0;; frags++) {
> >                 frag_size = min_t(int, length, frag_info->frag_size);
> >
> > -               page = frags->page;
> > -               if (unlikely(!page))
> > +               netmem = frags->netmem;
> > +               if (unlikely(!netmem))
> >                         goto fail;
> >
> > -               dma = page_pool_get_dma_addr(page);
> > +               dma = page_pool_get_dma_addr_netmem(netmem);
> >                 dma_sync_single_range_for_cpu(priv->ddev, dma, frags->page_offset,
> >                                               frag_size, priv->dma_dir);
> >
> > -               __skb_fill_page_desc(skb, nr, page, frags->page_offset,
> > -                                    frag_size);
> > +               __skb_fill_netmem_desc(skb, nr, netmem, frags->page_offset,
> > +                                      frag_size);
> >
> >                 truesize += frag_info->frag_stride;
> >                 if (frag_info->frag_stride == PAGE_SIZE / 2) {
> > +                       struct page *page = netmem_to_page(netmem);
> 
> This cast is not safe, try to use the netmem type directly.

Can it be net_iov?  It already ensures it's a page-backed netmem.  Why
is that unsafe?

With netmem, page_count() and page_to_nid() cannot be used, but needed.
Or checking 'page == NULL' after the casting works for you?

	Byungchul

> --
> Thanks,
> Mina

Re: [PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Pavel Begunkov]

On 7/10/25 09:28, Byungchul Park wrote:
...> +
>   static inline struct net_iov *netmem_to_net_iov(netmem_ref netmem)
>   {
>   	if (netmem_is_net_iov(netmem))
> @@ -314,6 +340,21 @@ static inline netmem_ref netmem_compound_head(netmem_ref netmem)
>   	return page_to_netmem(compound_head(netmem_to_page(netmem)));
>   }
>   
> +#define nmdesc_to_page(nmdesc)		(_Generic((nmdesc),		\
> +	const struct netmem_desc * :	(const struct page *)(nmdesc),	\
> +	struct netmem_desc * :		(struct page *)(nmdesc)))

Considering that nmdesc is going to be separated from pages and
accessed through indirection, and back reference to the page is
not needed (at least for net/), this helper shouldn't even exist.
And in fact, you don't really use it ...

> +static inline struct netmem_desc *page_to_nmdesc(struct page *page)
> +{
> +	VM_BUG_ON_PAGE(PageTail(page), page);
> +	return (struct netmem_desc *)page;
> +}
> +
> +static inline void *nmdesc_address(struct netmem_desc *nmdesc)
> +{
> +	return page_address(nmdesc_to_page(nmdesc));
> +}

... That's the only caller, and nmdesc_address() is not used, so
just nuke both of them. This helper doesn't even make sense.

Please avoid introducing functions that you don't use as a general
rule.

> +
>   /**
>    * __netmem_address - unsafely get pointer to the memory backing @netmem
>    * @netmem: netmem reference to get the pointer for

-- 
Pavel Begunkov

Re: [PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Pavel Begunkov]

On 7/10/25 19:11, Mina Almasry wrote:
> On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
>>
>> To eliminate the use of the page pool fields in struct page, the page
>> pool code should use netmem descriptor and APIs instead.
>>
>> However, some code e.g. __netmem_to_page() is still used to access the
>> page pool fields e.g. ->pp via struct page, which should be changed so
>> as to access them via netmem descriptor, struct netmem_desc instead,
>> since the fields no longer will be available in struct page.
>>
>> Introduce utility APIs to make them easy to use struct netmem_desc as
>> descriptor.  The APIs are:
>>
>>     1. __netmem_to_nmdesc(), to convert netmem_ref to struct netmem_desc,
>>        but unsafely without checking if it's net_iov or system memory.
>>
>>     2. netmem_to_nmdesc(), to convert netmem_ref to struct netmem_desc,
>>        safely with checking if it's net_iov or system memory.
>>
>>     3. nmdesc_to_page(), to convert struct netmem_desc to struct page,
>>        assuming struct netmem_desc overlays on struct page.
>>
>>     4. page_to_nmdesc(), to convert struct page to struct netmem_desc,
>>        assuming struct netmem_desc overlays on struct page, allowing only
>>        head page to be converted.
>>
>>     5. nmdesc_adress(), to get its virtual address corresponding to the
>>        struct netmem_desc.
>>
>> Signed-off-by: Byungchul Park <byungchul@sk.com>
>> ---
>>   include/net/netmem.h | 41 +++++++++++++++++++++++++++++++++++++++++
>>   1 file changed, 41 insertions(+)
>>
>> diff --git a/include/net/netmem.h b/include/net/netmem.h
>> index 535cf17b9134..ad9444be229a 100644
>> --- a/include/net/netmem.h
>> +++ b/include/net/netmem.h
>> @@ -198,6 +198,32 @@ static inline struct page *netmem_to_page(netmem_ref netmem)
>>          return __netmem_to_page(netmem);
>>   }
>>
>> +/**
>> + * __netmem_to_nmdesc - unsafely get pointer to the &netmem_desc backing
>> + * @netmem
>> + * @netmem: netmem reference to convert
>> + *
>> + * Unsafe version of netmem_to_nmdesc(). When @netmem is always backed
>> + * by system memory, performs faster and generates smaller object code
>> + * (no check for the LSB, no WARN). When @netmem points to IOV, provokes
>> + * undefined behaviour.
>> + *
>> + * Return: pointer to the &netmem_desc (garbage if @netmem is not backed
>> + * by system memory).
>> + */
>> +static inline struct netmem_desc *__netmem_to_nmdesc(netmem_ref netmem)
>> +{
>> +       return (__force struct netmem_desc *)netmem;
>> +}
>> +
> 
> Does a netmem_desc represent the pp fields shared between struct page
> and struct net_iov, or does netmem_desc represent paged kernel memory?
> If the former, I don't think we need a safe and unsafe version of this
> helper, since netmem_ref always has netmem_desc fields underneath. If
> the latter, then this helper should not exist at all. We should not
> allow casting netmem_ref to a netmem_desc without first checking if
> it's a net_iov.

+1, and...

> 
> To be honest the cover letter should come up with a detailed
> explanation of (a) what are the current types (b) what are the new
> types (c) what are the relationships between the types, so these
> questions stop coming up.
> 
>> +static inline struct netmem_desc *netmem_to_nmdesc(netmem_ref netmem)
>> +{
>> +       if (WARN_ON_ONCE(netmem_is_net_iov(netmem)))
>> +               return NULL;

... specifically this function should work with net_iov.

-- 
Pavel Begunkov

Re: [PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Pavel Begunkov]

On 7/11/25 02:02, Byungchul Park wrote:
> On Thu, Jul 10, 2025 at 11:11:51AM -0700, Mina Almasry wrote:
...>>> +#define nmdesc_to_page(nmdesc)         (_Generic((nmdesc),             \
>>> +       const struct netmem_desc * :    (const struct page *)(nmdesc),  \
>>> +       struct netmem_desc * :          (struct page *)(nmdesc)))
>>> +
>>> +static inline struct netmem_desc *page_to_nmdesc(struct page *page)
>>> +{
>>> +       VM_BUG_ON_PAGE(PageTail(page), page);
>>> +       return (struct netmem_desc *)page;
>>> +}
>>> +
>>
>> It's not safe to cast a page to netmem_desc, without first checking if
>> it's a pp page or not, otherwise you may be casting random non-pp
>> pages to netmem_desc...

I'd suggest to rename it to sth like pp_page_to_nmdesc() and add:

DEBUG_NET_WARN_ON_ONCE(!page_pool_page_is_pp(page));

...

> Agree, but page_to_nmdesc() will be used in page_pool_page_is_pp() to
> check if it's a pp page or not:
> 
>     static inline bool page_pool_page_is_pp(struct page *page)
>     {
> 	struct netmem_desc *desc = page_to_nmdesc(page);
> 
> 	return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
>     }
> 
> Hm.. maybe, it'd be better to resore the original code and remove this
> page_to_nmdesc() helper.  FYI, the original code was:
> 
>     static inline bool page_pool_page_is_pp(struct page *page)
>     {
> 	struct netmem_desc *desc = (struct netmem_desc *)page;
> 
> 	return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
>     }

... And use this version. It's supposed to be temporary anyway.
It'd be great to add a build check that the page still has space
to alias with for now.

-- 
Pavel Begunkov

Re: [PATCH net-next v9 3/8] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[Pavel Begunkov]

On 7/11/25 02:14, Byungchul Park wrote:
...>>> +#ifdef CONFIG_PAGE_POOL
>>> +/* XXX: This would better be moved to mm, once mm gets its way to
>>> + * identify the type of page for page pool.
>>> + */
>>> +static inline bool page_pool_page_is_pp(struct page *page)
>>> +{
>>> +       struct netmem_desc *desc = page_to_nmdesc(page);
>>> +
>>> +       return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
>>> +}
>>
>> pages can be pp pages (where they have pp fields inside of them) or
>> non-pp pages (where they don't have pp fields inside them, because
>> they were never allocated from the page_pool).
>>
>> Casting a page to a netmem_desc, and then checking if the page was a
>> pp page doesn't makes sense to me on a fundamental level. The
>> netmem_desc is only valid if the page was a pp page in the first
>> place. Maybe page_to_nmdesc should reject the cast if the page is not
>> a pp page or something.
> 
> Right, as you already know, the current mainline code already has the
> same problem but we've been using the werid way so far, in other words,
> mm code is checking if it's a pp page or not by using ->pp_magic, but
> it's ->lur, ->buddy_list, or ->pcp_list if it's not a pp page.
> 
> Both the mainline code and this patch can make sense *only if* it's
> actually a pp page.  It's unevitable until mm provides a way to identify
> the type of page for page pool.  Thoughts?
Question to mm folks, can we add a new PGTY for page pool and use
that to filter page pool originated pages? Like in the incomplete
and untested diff below?


commit 8fc2347fb3ff4a3fc7929c70a5a21e1128935d4a
Author: Pavel Begunkov <asml.silence@gmail.com>
Date:   Sat Jul 12 14:29:52 2025 +0100

     net/mm: use PGTY for tracking page pool pages
     
     Currently, we use page->pp_magic to determine whether a page belongs to
     a page pool. It's not ideal as the field is aliased with other page
     types, and thus needs to to rely on elaborated rules to work. Add a new
     page type for page pool.

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0ef2ba0c667a..975a013f1f17 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -4175,7 +4175,7 @@ int arch_lock_shadow_stack_status(struct task_struct *t, unsigned long status);
  #ifdef CONFIG_PAGE_POOL
  static inline bool page_pool_page_is_pp(struct page *page)
  {
-	return (page->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
+	return PageNetpp(page);
  }
  #else
  static inline bool page_pool_page_is_pp(struct page *page)
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 4fe5ee67535b..9bd1dfded2fc 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -957,6 +957,7 @@ enum pagetype {
  	PGTY_zsmalloc		= 0xf6,
  	PGTY_unaccepted		= 0xf7,
  	PGTY_large_kmalloc	= 0xf8,
+	PGTY_netpp		= 0xf9,
  
  	PGTY_mapcount_underflow = 0xff
  };
@@ -1101,6 +1102,11 @@ PAGE_TYPE_OPS(Zsmalloc, zsmalloc, zsmalloc)
  PAGE_TYPE_OPS(Unaccepted, unaccepted, unaccepted)
  FOLIO_TYPE_OPS(large_kmalloc, large_kmalloc)
  
+/*
+ * Marks page_pool allocated pages
+ */
+PAGE_TYPE_OPS(Netpp, netpp, netpp)
+
  /**
   * PageHuge - Determine if the page belongs to hugetlbfs
   * @page: The page to test.
diff --git a/include/net/netmem.h b/include/net/netmem.h
index de1d95f04076..20f5dbb08149 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -113,6 +113,8 @@ static inline bool netmem_is_net_iov(const netmem_ref netmem)
   */
  static inline struct page *__netmem_to_page(netmem_ref netmem)
  {
+	DEBUG_NET_WARN_ON_ONCE(netmem_is_net_iov(netmem));
+
  	return (__force struct page *)netmem;
  }
  
diff --git a/net/core/netmem_priv.h b/net/core/netmem_priv.h
index cd95394399b4..e38c64da1a78 100644
--- a/net/core/netmem_priv.h
+++ b/net/core/netmem_priv.h
@@ -13,16 +13,11 @@ static inline void netmem_or_pp_magic(netmem_ref netmem, unsigned long pp_magic)
  	__netmem_clear_lsb(netmem)->pp_magic |= pp_magic;
  }
  
-static inline void netmem_clear_pp_magic(netmem_ref netmem)
-{
-	WARN_ON_ONCE(__netmem_clear_lsb(netmem)->pp_magic & PP_DMA_INDEX_MASK);
-
-	__netmem_clear_lsb(netmem)->pp_magic = 0;
-}
-
  static inline bool netmem_is_pp(netmem_ref netmem)
  {
-	return (netmem_get_pp_magic(netmem) & PP_MAGIC_MASK) == PP_SIGNATURE;
+	if (netmem_is_net_iov(netmem))
+		return true;
+	return page_pool_page_is_pp(netmem_to_page(netmem));
  }
  
  static inline void netmem_set_pp(netmem_ref netmem, struct page_pool *pool)
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index 05e2e22a8f7c..52120e2912a6 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -371,6 +371,13 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
  }
  EXPORT_SYMBOL(page_pool_create);
  
+static void page_pool_set_page_pp_info(struct page_pool *pool,
+				       struct page *page)
+{
+	__SetPageNetpp(page);
+	page_pool_set_pp_info(page_to_netmem(page));
+}
+
  static void page_pool_return_netmem(struct page_pool *pool, netmem_ref netmem);
  
  static noinline netmem_ref page_pool_refill_alloc_cache(struct page_pool *pool)
@@ -534,7 +541,7 @@ static struct page *__page_pool_alloc_page_order(struct page_pool *pool,
  	}
  
  	alloc_stat_inc(pool, slow_high_order);
-	page_pool_set_pp_info(pool, page_to_netmem(page));
+	page_pool_set_page_pp_info(pool, page);
  
  	/* Track how many pages are held 'in-flight' */
  	pool->pages_state_hold_cnt++;
@@ -579,7 +586,7 @@ static noinline netmem_ref __page_pool_alloc_netmems_slow(struct page_pool *pool
  			continue;
  		}
  
-		page_pool_set_pp_info(pool, netmem);
+		page_pool_set_page_pp_info(pool, __netmem_to_page(netmem));
  		pool->alloc.cache[pool->alloc.count++] = netmem;
  		/* Track how many pages are held 'in-flight' */
  		pool->pages_state_hold_cnt++;
@@ -654,7 +661,6 @@ s32 page_pool_inflight(const struct page_pool *pool, bool strict)
  void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)
  {
  	netmem_set_pp(netmem, pool);
-	netmem_or_pp_magic(netmem, PP_SIGNATURE);
  
  	/* Ensuring all pages have been split into one fragment initially:
  	 * page_pool_set_pp_info() is only called once for every page when it
@@ -669,7 +675,6 @@ void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)
  
  void page_pool_clear_pp_info(netmem_ref netmem)
  {
-	netmem_clear_pp_magic(netmem);
  	netmem_set_pp(netmem, NULL);
  }
  
@@ -730,8 +735,11 @@ static void page_pool_return_netmem(struct page_pool *pool, netmem_ref netmem)
  	trace_page_pool_state_release(pool, netmem, count);
  
  	if (put) {
+		struct page *page = netmem_to_page(netmem);
+
  		page_pool_clear_pp_info(netmem);
-		put_page(netmem_to_page(netmem));
+		__ClearPageNetpp(page);
+		put_page(page);
  	}
  	/* An optimization would be to call __free_pages(page, pool->p.order)
  	 * knowing page is not part of page-cache (thus avoiding a

-- 
Pavel Begunkov

Re: [PATCH net-next v9 8/8] mt76: use netmem descriptor and APIs for page pool

[Pavel Begunkov]

On 7/10/25 09:28, Byungchul Park wrote:
> To simplify struct page, the effort to separate its own descriptor from
> struct page is required and the work for page pool is on going.
> 
> Use netmem descriptor and APIs for page pool in mt76 code.
> 
> Signed-off-by: Byungchul Park <byungchul@sk.com>
> Reviewed-by: Mina Almasry <almasrymina@google.com>
> ---
...>   static inline void mt76_set_tx_blocked(struct mt76_dev *dev, bool blocked)
> diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> index 0a927a7313a6..b1d89b6f663d 100644
> --- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> +++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> @@ -68,14 +68,14 @@ mt76s_build_rx_skb(void *data, int data_len, int buf_len)
>   
>   	skb_put_data(skb, data, len);
>   	if (data_len > len) {
> -		struct page *page;
> +		netmem_ref netmem;
>   
>   		data += len;
> -		page = virt_to_head_page(data);
> -		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
> -				page, data - page_address(page),
> -				data_len - len, buf_len);
> -		get_page(page);
> +		netmem = virt_to_head_netmem(data);
> +		skb_add_rx_frag_netmem(skb, skb_shinfo(skb)->nr_frags,
> +				       netmem, data - netmem_address(netmem),
> +				       data_len - len, buf_len);
> +		get_netmem(netmem);
>   	}
>   
>   	return skb;
> @@ -88,7 +88,7 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
>   	struct mt76_queue *q = &dev->q_rx[qid];
>   	struct mt76_sdio *sdio = &dev->sdio;
>   	int len = 0, err, i;
> -	struct page *page;
> +	netmem_ref netmem;
>   	u8 *buf, *end;
>   
>   	for (i = 0; i < intr->rx.num[qid]; i++)
> @@ -100,11 +100,11 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
>   	if (len > sdio->func->cur_blksize)
>   		len = roundup(len, sdio->func->cur_blksize);
>   
> -	page = __dev_alloc_pages(GFP_KERNEL, get_order(len));
> -	if (!page)
> +	netmem = page_to_netmem(__dev_alloc_pages(GFP_KERNEL, get_order(len)));
> +	if (!netmem)
>   		return -ENOMEM;
>   
> -	buf = page_address(page);
> +	buf = netmem_address(netmem);

We shouldn't just blindly convert everything to netmem just for the purpose
of creating a type casting hell. It's allocating a page, and continues to
use it as a page, e.g. netmem_address() will fail otherwise. So just leave
it to be a page, and convert it to netmem and the very last moment when
the api expects a netmem. There are likely many chunks like that.

>   
>   	sdio_claim_host(sdio->func);
>   	err = sdio_readsb(sdio->func, buf, MCR_WRDR(qid), len);
> @@ -112,7 +112,7 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
>   
>   	if (err < 0) {
>   		dev_err(dev->dev, "sdio read data failed:%d\n", err);
> -		put_page(page);
> +		put_netmem(netmem);
>   		return err;
>   	}
>   
> @@ -140,7 +140,7 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
>   		}
>   		buf += round_up(len + 4, 4);
>   	}
> -	put_page(page);
> +	put_netmem(netmem);
>   
>   	spin_lock_bh(&q->lock);
>   	q->head = (q->head + i) % q->ndesc;
-- 
Pavel Begunkov

Re: [PATCH net-next v9 1/8] netmem: introduce struct netmem_desc mirroring struct page

[Pavel Begunkov]

On 7/10/25 09:28, Byungchul Park wrote:
> To simplify struct page, the page pool members of struct page should be
> moved to other, allowing these members to be removed from struct page.
> 
> Introduce a network memory descriptor to store the members, struct
> netmem_desc, and make it union'ed with the existing fields in struct
> net_iov, allowing to organize the fields of struct net_iov.

FWIW, regardless of memdesc business, I think it'd be great to have
this patch, as it'll help with some of the netmem casting ugliness and
shed some cycles as well. For example, we have a bunch of
niov -> netmem -> niov casts in various places.

-- 
Pavel Begunkov

Re: [PATCH net-next v9 3/8] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[David Hildenbrand]

On 12.07.25 15:58, Pavel Begunkov wrote:
> On 7/11/25 02:14, Byungchul Park wrote:
> ...>>> +#ifdef CONFIG_PAGE_POOL
>>>> +/* XXX: This would better be moved to mm, once mm gets its way to
>>>> + * identify the type of page for page pool.
>>>> + */
>>>> +static inline bool page_pool_page_is_pp(struct page *page)
>>>> +{
>>>> +       struct netmem_desc *desc = page_to_nmdesc(page);
>>>> +
>>>> +       return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
>>>> +}
>>>
>>> pages can be pp pages (where they have pp fields inside of them) or
>>> non-pp pages (where they don't have pp fields inside them, because
>>> they were never allocated from the page_pool).
>>>
>>> Casting a page to a netmem_desc, and then checking if the page was a
>>> pp page doesn't makes sense to me on a fundamental level. The
>>> netmem_desc is only valid if the page was a pp page in the first
>>> place. Maybe page_to_nmdesc should reject the cast if the page is not
>>> a pp page or something.
>>
>> Right, as you already know, the current mainline code already has the
>> same problem but we've been using the werid way so far, in other words,
>> mm code is checking if it's a pp page or not by using ->pp_magic, but
>> it's ->lur, ->buddy_list, or ->pcp_list if it's not a pp page.
>>
>> Both the mainline code and this patch can make sense *only if* it's
>> actually a pp page.  It's unevitable until mm provides a way to identify
>> the type of page for page pool.  Thoughts?
> Question to mm folks, can we add a new PGTY for page pool and use
> that to filter page pool originated pages? Like in the incomplete
> and untested diff below?

https://lore.kernel.org/all/77c6a6dd-0e03-4b81-a9c7-eaecaa4ebc0b@redhat.com/

We then want to do (on top of mm/mm-unstable)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index fa09154a799c6..cb90d6a3fd9d9 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1042,7 +1042,6 @@ static inline bool page_expected_state(struct page 
*page,
  #ifdef CONFIG_MEMCG
                         page->memcg_data |
  #endif
-                       page_pool_page_is_pp(page) |
                         (page->flags & check_flags)))
                 return false;

@@ -1069,8 +1068,6 @@ static const char *page_bad_reason(struct page 
*page, unsigned long flags)
         if (unlikely(page->memcg_data))
                 bad_reason = "page still charged to cgroup";
  #endif
-       if (unlikely(page_pool_page_is_pp(page)))
-               bad_reason = "page_pool leak";
         return bad_reason;
  }

@@ -1379,9 +1376,11 @@ __always_inline bool free_pages_prepare(struct 
page *page,
                 mod_mthp_stat(order, MTHP_STAT_NR_ANON, -1);
                 folio->mapping = NULL;
         }
-       if (unlikely(page_has_type(page)))
+       if (unlikely(page_has_type(page))) {
+               WARN_ON_ONCE(PageNetpp(page));
                 /* Reset the page_type (which overlays _mapcount) */
                 page->page_type = UINT_MAX;
+       }

         if (is_check_pages_enabled()) {
                 if (free_page_is_bad(page))


> 
> 
> commit 8fc2347fb3ff4a3fc7929c70a5a21e1128935d4a
> Author: Pavel Begunkov <asml.silence@gmail.com>
> Date:   Sat Jul 12 14:29:52 2025 +0100
> 
>       net/mm: use PGTY for tracking page pool pages
>       
>       Currently, we use page->pp_magic to determine whether a page belongs to
>       a page pool. It's not ideal as the field is aliased with other page
>       types, and thus needs to to rely on elaborated rules to work. Add a new
>       page type for page pool.
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0ef2ba0c667a..975a013f1f17 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4175,7 +4175,7 @@ int arch_lock_shadow_stack_status(struct task_struct *t, unsigned long status);
>    #ifdef CONFIG_PAGE_POOL
>    static inline bool page_pool_page_is_pp(struct page *page)
>    {
> -	return (page->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> +	return PageNetpp(page);
>    }
>    #else
>    static inline bool page_pool_page_is_pp(struct page *page)
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 4fe5ee67535b..9bd1dfded2fc 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -957,6 +957,7 @@ enum pagetype {
>    	PGTY_zsmalloc		= 0xf6,
>    	PGTY_unaccepted		= 0xf7,
>    	PGTY_large_kmalloc	= 0xf8,
> +	PGTY_netpp		= 0xf9,
>    
>    	PGTY_mapcount_underflow = 0xff
>    };
> @@ -1101,6 +1102,11 @@ PAGE_TYPE_OPS(Zsmalloc, zsmalloc, zsmalloc)
>    PAGE_TYPE_OPS(Unaccepted, unaccepted, unaccepted)
>    FOLIO_TYPE_OPS(large_kmalloc, large_kmalloc)
>    
> +/*
> + * Marks page_pool allocated pages
> + */
> +PAGE_TYPE_OPS(Netpp, netpp, netpp)
> +
>    /**
>     * PageHuge - Determine if the page belongs to hugetlbfs
>     * @page: The page to test.
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index de1d95f04076..20f5dbb08149 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -113,6 +113,8 @@ static inline bool netmem_is_net_iov(const netmem_ref netmem)
>     */
>    static inline struct page *__netmem_to_page(netmem_ref netmem)
>    {
> +	DEBUG_NET_WARN_ON_ONCE(netmem_is_net_iov(netmem));
> +
>    	return (__force struct page *)netmem;
>    }
>    
> diff --git a/net/core/netmem_priv.h b/net/core/netmem_priv.h
> index cd95394399b4..e38c64da1a78 100644
> --- a/net/core/netmem_priv.h
> +++ b/net/core/netmem_priv.h
> @@ -13,16 +13,11 @@ static inline void netmem_or_pp_magic(netmem_ref netmem, unsigned long pp_magic)
>    	__netmem_clear_lsb(netmem)->pp_magic |= pp_magic;
>    }
>    
> -static inline void netmem_clear_pp_magic(netmem_ref netmem)
> -{
> -	WARN_ON_ONCE(__netmem_clear_lsb(netmem)->pp_magic & PP_DMA_INDEX_MASK);
> -
> -	__netmem_clear_lsb(netmem)->pp_magic = 0;
> -}
> -
>    static inline bool netmem_is_pp(netmem_ref netmem)
>    {
> -	return (netmem_get_pp_magic(netmem) & PP_MAGIC_MASK) == PP_SIGNATURE;
> +	if (netmem_is_net_iov(netmem))
> +		return true;
> +	return page_pool_page_is_pp(netmem_to_page(netmem));
>    }
>    
>    static inline void netmem_set_pp(netmem_ref netmem, struct page_pool *pool)
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 05e2e22a8f7c..52120e2912a6 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -371,6 +371,13 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
>    }
>    EXPORT_SYMBOL(page_pool_create);
>    
> +static void page_pool_set_page_pp_info(struct page_pool *pool,
> +				       struct page *page)
> +{
> +	__SetPageNetpp(page);
> +	page_pool_set_pp_info(page_to_netmem(page));
> +}
> +
>    static void page_pool_return_netmem(struct page_pool *pool, netmem_ref netmem);
>    
>    static noinline netmem_ref page_pool_refill_alloc_cache(struct page_pool *pool)
> @@ -534,7 +541,7 @@ static struct page *__page_pool_alloc_page_order(struct page_pool *pool,
>    	}
>    
>    	alloc_stat_inc(pool, slow_high_order);
> -	page_pool_set_pp_info(pool, page_to_netmem(page));
> +	page_pool_set_page_pp_info(pool, page);
>    
>    	/* Track how many pages are held 'in-flight' */
>    	pool->pages_state_hold_cnt++;
> @@ -579,7 +586,7 @@ static noinline netmem_ref __page_pool_alloc_netmems_slow(struct page_pool *pool
>    			continue;
>    		}
>    
> -		page_pool_set_pp_info(pool, netmem);
> +		page_pool_set_page_pp_info(pool, __netmem_to_page(netmem));
>    		pool->alloc.cache[pool->alloc.count++] = netmem;
>    		/* Track how many pages are held 'in-flight' */
>    		pool->pages_state_hold_cnt++;
> @@ -654,7 +661,6 @@ s32 page_pool_inflight(const struct page_pool *pool, bool strict)
>    void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)
>    {
>    	netmem_set_pp(netmem, pool);
> -	netmem_or_pp_magic(netmem, PP_SIGNATURE);
>    
>    	/* Ensuring all pages have been split into one fragment initially:
>    	 * page_pool_set_pp_info() is only called once for every page when it
> @@ -669,7 +675,6 @@ void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)
>    
>    void page_pool_clear_pp_info(netmem_ref netmem)
>    {
> -	netmem_clear_pp_magic(netmem);
>    	netmem_set_pp(netmem, NULL);
>    }
>    
> @@ -730,8 +735,11 @@ static void page_pool_return_netmem(struct page_pool *pool, netmem_ref netmem)
>    	trace_page_pool_state_release(pool, netmem, count);
>    
>    	if (put) {
> +		struct page *page = netmem_to_page(netmem);
> +
>    		page_pool_clear_pp_info(netmem);
> -		put_page(netmem_to_page(netmem));
> +		__ClearPageNetpp(page);
> +		put_page(page);
>    	}
>    	/* An optimization would be to call __free_pages(page, pool->p.order)
>    	 * knowing page is not part of page-cache (thus avoiding a
> 


-- 
Cheers,

David / dhildenb

Re: [PATCH net-next v9 3/8] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[Pavel Begunkov]

On 7/12/25 15:52, David Hildenbrand wrote:
> On 12.07.25 15:58, Pavel Begunkov wrote:
>> On 7/11/25 02:14, Byungchul Park wrote:
>> ...>>> +#ifdef CONFIG_PAGE_POOL
>>>>> +/* XXX: This would better be moved to mm, once mm gets its way to
>>>>> + * identify the type of page for page pool.
>>>>> + */
>>>>> +static inline bool page_pool_page_is_pp(struct page *page)
>>>>> +{
>>>>> +       struct netmem_desc *desc = page_to_nmdesc(page);
>>>>> +
>>>>> +       return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
>>>>> +}
>>>>
>>>> pages can be pp pages (where they have pp fields inside of them) or
>>>> non-pp pages (where they don't have pp fields inside them, because
>>>> they were never allocated from the page_pool).
>>>>
>>>> Casting a page to a netmem_desc, and then checking if the page was a
>>>> pp page doesn't makes sense to me on a fundamental level. The
>>>> netmem_desc is only valid if the page was a pp page in the first
>>>> place. Maybe page_to_nmdesc should reject the cast if the page is not
>>>> a pp page or something.
>>>
>>> Right, as you already know, the current mainline code already has the
>>> same problem but we've been using the werid way so far, in other words,
>>> mm code is checking if it's a pp page or not by using ->pp_magic, but
>>> it's ->lur, ->buddy_list, or ->pcp_list if it's not a pp page.
>>>
>>> Both the mainline code and this patch can make sense *only if* it's
>>> actually a pp page.  It's unevitable until mm provides a way to identify
>>> the type of page for page pool.  Thoughts?
>> Question to mm folks, can we add a new PGTY for page pool and use
>> that to filter page pool originated pages? Like in the incomplete
>> and untested diff below?
> 
> https://lore.kernel.org/all/77c6a6dd-0e03-4b81-a9c7-eaecaa4ebc0b@redhat.com/

Great, then it'll be the right thing to do here. I somehow missed
the post, will add your suggested-by.

-- 
Pavel Begunkov

Re: [PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Byungchul Park]

On Sat, Jul 12, 2025 at 12:59:34PM +0100, Pavel Begunkov wrote:
> On 7/10/25 09:28, Byungchul Park wrote:
> ...> +
> >   static inline struct net_iov *netmem_to_net_iov(netmem_ref netmem)
> >   {
> >       if (netmem_is_net_iov(netmem))
> > @@ -314,6 +340,21 @@ static inline netmem_ref netmem_compound_head(netmem_ref netmem)
> >       return page_to_netmem(compound_head(netmem_to_page(netmem)));
> >   }
> > 
> > +#define nmdesc_to_page(nmdesc)               (_Generic((nmdesc),             \
> > +     const struct netmem_desc * :    (const struct page *)(nmdesc),  \
> > +     struct netmem_desc * :          (struct page *)(nmdesc)))
> 
> Considering that nmdesc is going to be separated from pages and
> accessed through indirection, and back reference to the page is
> not needed (at least for net/), this helper shouldn't even exist.
> And in fact, you don't really use it ...
> > +static inline struct netmem_desc *page_to_nmdesc(struct page *page)
> > +{
> > +     VM_BUG_ON_PAGE(PageTail(page), page);
> > +     return (struct netmem_desc *)page;
> > +}
> > +
> > +static inline void *nmdesc_address(struct netmem_desc *nmdesc)
> > +{
> > +     return page_address(nmdesc_to_page(nmdesc));
> > +}
> 
> ... That's the only caller, and nmdesc_address() is not used, so
> just nuke both of them. This helper doesn't even make sense.
> 
> Please avoid introducing functions that you don't use as a general
> rule.

I'm sorry about making you confused.  I should've included another patch
using the helper like the following.

	Byungchul

---8<---
diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
index cef9dfb877e8..adccc7c8e68f 100644
--- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
+++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
@@ -3266,7 +3266,7 @@ static u32 idpf_rx_hsplit_wa(const struct libeth_fqe *hdr,
 			     struct libeth_fqe *buf, u32 data_len)
 {
 	u32 copy = data_len <= L1_CACHE_BYTES ? data_len : ETH_HLEN;
-	struct page *hdr_page, *buf_page;
+	struct netmem_desc *hdr_nmdesc, *buf_nmdesc;
 	const void *src;
 	void *dst;
 
@@ -3274,10 +3274,10 @@ static u32 idpf_rx_hsplit_wa(const struct libeth_fqe *hdr,
 	    !libeth_rx_sync_for_cpu(buf, copy))
 		return 0;
 
-	hdr_page = __netmem_to_page(hdr->netmem);
-	buf_page = __netmem_to_page(buf->netmem);
-	dst = page_address(hdr_page) + hdr->offset + hdr_page->pp->p.offset;
-	src = page_address(buf_page) + buf->offset + buf_page->pp->p.offset;
+	hdr_nmdesc = __netmem_to_nmdesc(hdr->netmem);
+	buf_nmdesc = __netmem_to_nmdesc(buf->netmem);
+	dst = nmdesc_address(hdr_nmdesc) + hdr->offset + hdr_nmdesc->pp->p.offset;
+	src = nmdesc_address(buf_nmdesc) + buf->offset + buf_nmdesc->pp->p.offset;
 
 	memcpy(dst, src, LARGEST_ALIGN(copy));
 	buf->offset += copy;
--
> > +
> >   /**
> >    * __netmem_address - unsafely get pointer to the memory backing @netmem
> >    * @netmem: netmem reference to get the pointer for
> 
> --
> Pavel Begunkov

Re: [PATCH net-next v9 3/8] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[Byungchul Park]

On Sat, Jul 12, 2025 at 04:09:13PM +0100, Pavel Begunkov wrote:
> On 7/12/25 15:52, David Hildenbrand wrote:
> > On 12.07.25 15:58, Pavel Begunkov wrote:
> > > On 7/11/25 02:14, Byungchul Park wrote:
> > > ...>>> +#ifdef CONFIG_PAGE_POOL
> > > > > > +/* XXX: This would better be moved to mm, once mm gets its way to
> > > > > > + * identify the type of page for page pool.
> > > > > > + */
> > > > > > +static inline bool page_pool_page_is_pp(struct page *page)
> > > > > > +{
> > > > > > +       struct netmem_desc *desc = page_to_nmdesc(page);
> > > > > > +
> > > > > > +       return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> > > > > > +}
> > > > > 
> > > > > pages can be pp pages (where they have pp fields inside of them) or
> > > > > non-pp pages (where they don't have pp fields inside them, because
> > > > > they were never allocated from the page_pool).
> > > > > 
> > > > > Casting a page to a netmem_desc, and then checking if the page was a
> > > > > pp page doesn't makes sense to me on a fundamental level. The
> > > > > netmem_desc is only valid if the page was a pp page in the first
> > > > > place. Maybe page_to_nmdesc should reject the cast if the page is not
> > > > > a pp page or something.
> > > > 
> > > > Right, as you already know, the current mainline code already has the
> > > > same problem but we've been using the werid way so far, in other words,
> > > > mm code is checking if it's a pp page or not by using ->pp_magic, but
> > > > it's ->lur, ->buddy_list, or ->pcp_list if it's not a pp page.
> > > > 
> > > > Both the mainline code and this patch can make sense *only if* it's
> > > > actually a pp page.  It's unevitable until mm provides a way to identify
> > > > the type of page for page pool.  Thoughts?
> > > Question to mm folks, can we add a new PGTY for page pool and use
> > > that to filter page pool originated pages? Like in the incomplete
> > > and untested diff below?
> > 
> > https://lore.kernel.org/all/77c6a6dd-0e03-4b81-a9c7-eaecaa4ebc0b@redhat.com/
> 
> Great, then it'll be the right thing to do here. I somehow missed
> the post, will add your suggested-by.

It'd be the ideal.  I will wait and work on top of your patch then.

	Byungchul
> 
> --
> Pavel Begunkov

Re: [PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Byungchul Park]

On Mon, Jul 14, 2025 at 08:07:52AM +0900, Byungchul Park wrote:
> On Sat, Jul 12, 2025 at 12:59:34PM +0100, Pavel Begunkov wrote:
> > On 7/10/25 09:28, Byungchul Park wrote:
> > ...> +
> > >   static inline struct net_iov *netmem_to_net_iov(netmem_ref netmem)
> > >   {
> > >       if (netmem_is_net_iov(netmem))
> > > @@ -314,6 +340,21 @@ static inline netmem_ref netmem_compound_head(netmem_ref netmem)
> > >       return page_to_netmem(compound_head(netmem_to_page(netmem)));
> > >   }
> > > 
> > > +#define nmdesc_to_page(nmdesc)               (_Generic((nmdesc),             \
> > > +     const struct netmem_desc * :    (const struct page *)(nmdesc),  \
> > > +     struct netmem_desc * :          (struct page *)(nmdesc)))
> > 
> > Considering that nmdesc is going to be separated from pages and
> > accessed through indirection, and back reference to the page is
> > not needed (at least for net/), this helper shouldn't even exist.
> > And in fact, you don't really use it ...
> > > +static inline struct netmem_desc *page_to_nmdesc(struct page *page)
> > > +{
> > > +     VM_BUG_ON_PAGE(PageTail(page), page);
> > > +     return (struct netmem_desc *)page;
> > > +}
> > > +
> > > +static inline void *nmdesc_address(struct netmem_desc *nmdesc)
> > > +{
> > > +     return page_address(nmdesc_to_page(nmdesc));
> > > +}
> > 
> > ... That's the only caller, and nmdesc_address() is not used, so
> > just nuke both of them. This helper doesn't even make sense.
> > 
> > Please avoid introducing functions that you don't use as a general
> > rule.
> 
> I'm sorry about making you confused.  I should've included another patch
> using the helper like the following.

As Mina suggested, I will add this helper along with the patch using it.
	Byungchul

> 	Byungchul
> 
> ---8<---
> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> index cef9dfb877e8..adccc7c8e68f 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> @@ -3266,7 +3266,7 @@ static u32 idpf_rx_hsplit_wa(const struct libeth_fqe *hdr,
>  			     struct libeth_fqe *buf, u32 data_len)
>  {
>  	u32 copy = data_len <= L1_CACHE_BYTES ? data_len : ETH_HLEN;
> -	struct page *hdr_page, *buf_page;
> +	struct netmem_desc *hdr_nmdesc, *buf_nmdesc;
>  	const void *src;
>  	void *dst;
>  
> @@ -3274,10 +3274,10 @@ static u32 idpf_rx_hsplit_wa(const struct libeth_fqe *hdr,
>  	    !libeth_rx_sync_for_cpu(buf, copy))
>  		return 0;
>  
> -	hdr_page = __netmem_to_page(hdr->netmem);
> -	buf_page = __netmem_to_page(buf->netmem);
> -	dst = page_address(hdr_page) + hdr->offset + hdr_page->pp->p.offset;
> -	src = page_address(buf_page) + buf->offset + buf_page->pp->p.offset;
> +	hdr_nmdesc = __netmem_to_nmdesc(hdr->netmem);
> +	buf_nmdesc = __netmem_to_nmdesc(buf->netmem);
> +	dst = nmdesc_address(hdr_nmdesc) + hdr->offset + hdr_nmdesc->pp->p.offset;
> +	src = nmdesc_address(buf_nmdesc) + buf->offset + buf_nmdesc->pp->p.offset;
>  
>  	memcpy(dst, src, LARGEST_ALIGN(copy));
>  	buf->offset += copy;
> --
> > > +
> > >   /**
> > >    * __netmem_address - unsafely get pointer to the memory backing @netmem
> > >    * @netmem: netmem reference to get the pointer for
> > 
> > --
> > Pavel Begunkov

Re: [PATCH net-next v9 8/8] mt76: use netmem descriptor and APIs for page pool

[Byungchul Park]

On Sat, Jul 12, 2025 at 03:22:17PM +0100, Pavel Begunkov wrote:
> On 7/10/25 09:28, Byungchul Park wrote:
> > To simplify struct page, the effort to separate its own descriptor from
> > struct page is required and the work for page pool is on going.
> > 
> > Use netmem descriptor and APIs for page pool in mt76 code.
> > 
> > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > Reviewed-by: Mina Almasry <almasrymina@google.com>
> > ---
> ...>   static inline void mt76_set_tx_blocked(struct mt76_dev *dev, bool blocked)
> > diff --git a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > index 0a927a7313a6..b1d89b6f663d 100644
> > --- a/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > +++ b/drivers/net/wireless/mediatek/mt76/sdio_txrx.c
> > @@ -68,14 +68,14 @@ mt76s_build_rx_skb(void *data, int data_len, int buf_len)
> > 
> >       skb_put_data(skb, data, len);
> >       if (data_len > len) {
> > -             struct page *page;
> > +             netmem_ref netmem;
> > 
> >               data += len;
> > -             page = virt_to_head_page(data);
> > -             skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags,
> > -                             page, data - page_address(page),
> > -                             data_len - len, buf_len);
> > -             get_page(page);
> > +             netmem = virt_to_head_netmem(data);
> > +             skb_add_rx_frag_netmem(skb, skb_shinfo(skb)->nr_frags,
> > +                                    netmem, data - netmem_address(netmem),
> > +                                    data_len - len, buf_len);
> > +             get_netmem(netmem);
> >       }
> > 
> >       return skb;
> > @@ -88,7 +88,7 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
> >       struct mt76_queue *q = &dev->q_rx[qid];
> >       struct mt76_sdio *sdio = &dev->sdio;
> >       int len = 0, err, i;
> > -     struct page *page;
> > +     netmem_ref netmem;
> >       u8 *buf, *end;
> > 
> >       for (i = 0; i < intr->rx.num[qid]; i++)
> > @@ -100,11 +100,11 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
> >       if (len > sdio->func->cur_blksize)
> >               len = roundup(len, sdio->func->cur_blksize);
> > 
> > -     page = __dev_alloc_pages(GFP_KERNEL, get_order(len));
> > -     if (!page)
> > +     netmem = page_to_netmem(__dev_alloc_pages(GFP_KERNEL, get_order(len)));
> > +     if (!netmem)
> >               return -ENOMEM;
> > 
> > -     buf = page_address(page);
> > +     buf = netmem_address(netmem);
> 
> We shouldn't just blindly convert everything to netmem just for the purpose
> of creating a type casting hell. It's allocating a page, and continues to
> use it as a page, e.g. netmem_address() will fail otherwise. So just leave
> it to be a page, and convert it to netmem and the very last moment when
> the api expects a netmem. There are likely many chunks like that.

Thanks for the feedback.

Unon reconsideration, focusing on the conversion between page and
netmem_desc, plus small modification on user side code e.i. driver are
sufficient to achieve my objectives.  I won't change a lot on user side
code like this from the next spin.

	Byungchul

> > 
> >       sdio_claim_host(sdio->func);
> >       err = sdio_readsb(sdio->func, buf, MCR_WRDR(qid), len);
> > @@ -112,7 +112,7 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
> > 
> >       if (err < 0) {
> >               dev_err(dev->dev, "sdio read data failed:%d\n", err);
> > -             put_page(page);
> > +             put_netmem(netmem);
> >               return err;
> >       }
> > 
> > @@ -140,7 +140,7 @@ mt76s_rx_run_queue(struct mt76_dev *dev, enum mt76_rxq_id qid,
> >               }
> >               buf += round_up(len + 4, 4);
> >       }
> > -     put_page(page);
> > +     put_netmem(netmem);
> > 
> >       spin_lock_bh(&q->lock);
> >       q->head = (q->head + i) % q->ndesc;
> --
> Pavel Begunkov

Re: [PATCH net-next v9 1/8] netmem: introduce struct netmem_desc mirroring struct page

[Byungchul Park]

On Sat, Jul 12, 2025 at 03:39:59PM +0100, Pavel Begunkov wrote:
> On 7/10/25 09:28, Byungchul Park wrote:
> > To simplify struct page, the page pool members of struct page should be
> > moved to other, allowing these members to be removed from struct page.
> > 
> > Introduce a network memory descriptor to store the members, struct
> > netmem_desc, and make it union'ed with the existing fields in struct
> > net_iov, allowing to organize the fields of struct net_iov.
> 
> FWIW, regardless of memdesc business, I think it'd be great to have
> this patch, as it'll help with some of the netmem casting ugliness and
> shed some cycles as well. For example, we have a bunch of
> niov -> netmem -> niov casts in various places.

If Jakub agrees with this, I will re-post this as a separate patch so
that works that require this base can go ahead.

	Byungchul
> 
> --
> Pavel Begunkov

Re: [PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Pavel Begunkov]

On 7/14/25 00:07, Byungchul Park wrote:
> On Sat, Jul 12, 2025 at 12:59:34PM +0100, Pavel Begunkov wrote:
>> On 7/10/25 09:28, Byungchul Park wrote:
>> ...> +
>>>    static inline struct net_iov *netmem_to_net_iov(netmem_ref netmem)
>>>    {
>>>        if (netmem_is_net_iov(netmem))
>>> @@ -314,6 +340,21 @@ static inline netmem_ref netmem_compound_head(netmem_ref netmem)
>>>        return page_to_netmem(compound_head(netmem_to_page(netmem)));
>>>    }
>>>
>>> +#define nmdesc_to_page(nmdesc)               (_Generic((nmdesc),             \
>>> +     const struct netmem_desc * :    (const struct page *)(nmdesc),  \
>>> +     struct netmem_desc * :          (struct page *)(nmdesc)))
>>
>> Considering that nmdesc is going to be separated from pages and
>> accessed through indirection, and back reference to the page is
>> not needed (at least for net/), this helper shouldn't even exist.
>> And in fact, you don't really use it ...
>>> +static inline struct netmem_desc *page_to_nmdesc(struct page *page)
>>> +{
>>> +     VM_BUG_ON_PAGE(PageTail(page), page);
>>> +     return (struct netmem_desc *)page;
>>> +}
>>> +
>>> +static inline void *nmdesc_address(struct netmem_desc *nmdesc)
>>> +{
>>> +     return page_address(nmdesc_to_page(nmdesc));
>>> +}
>>
>> ... That's the only caller, and nmdesc_address() is not used, so
>> just nuke both of them. This helper doesn't even make sense.
>>
>> Please avoid introducing functions that you don't use as a general
>> rule.
> 
> I'm sorry about making you confused.  I should've included another patch
> using the helper like the following.

Ah, I see. And still, it's not a great function. There should be
no way to extract a page or a page address from a nmdesc.

For the diff below it's same as with the mt76 patch, it's allocating
a page, expects it to be a page, using it as a page, but for no reason
keeps it wrapped into netmem. It only adds confusion and overhead.
A rule of thumb would be only converting to netmem if the new code
would be able to work with a netmem-wrapped net_iovs.

> diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> index cef9dfb877e8..adccc7c8e68f 100644
> --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> @@ -3266,7 +3266,7 @@ static u32 idpf_rx_hsplit_wa(const struct libeth_fqe *hdr,
>   			     struct libeth_fqe *buf, u32 data_len)
>   {
>   	u32 copy = data_len <= L1_CACHE_BYTES ? data_len : ETH_HLEN;
> -	struct page *hdr_page, *buf_page;
> +	struct netmem_desc *hdr_nmdesc, *buf_nmdesc;
>   	const void *src;
>   	void *dst;
>   
> @@ -3274,10 +3274,10 @@ static u32 idpf_rx_hsplit_wa(const struct libeth_fqe *hdr,
>   	    !libeth_rx_sync_for_cpu(buf, copy))
>   		return 0;
>   
> -	hdr_page = __netmem_to_page(hdr->netmem);
> -	buf_page = __netmem_to_page(buf->netmem);
> -	dst = page_address(hdr_page) + hdr->offset + hdr_page->pp->p.offset;
> -	src = page_address(buf_page) + buf->offset + buf_page->pp->p.offset;
> +	hdr_nmdesc = __netmem_to_nmdesc(hdr->netmem);
> +	buf_nmdesc = __netmem_to_nmdesc(buf->netmem);
> +	dst = nmdesc_address(hdr_nmdesc) + hdr->offset + hdr_nmdesc->pp->p.offset;
> +	src = nmdesc_address(buf_nmdesc) + buf->offset + buf_nmdesc->pp->p.offset;
>   
>   	memcpy(dst, src, LARGEST_ALIGN(copy));
>   	buf->offset += copy;
-- 
Pavel Begunkov

Re: [PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Byungchul Park]

On Mon, Jul 14, 2025 at 10:43:35AM +0100, Pavel Begunkov wrote:
> On 7/14/25 00:07, Byungchul Park wrote:
> > On Sat, Jul 12, 2025 at 12:59:34PM +0100, Pavel Begunkov wrote:
> > > On 7/10/25 09:28, Byungchul Park wrote:
> > > ...> +
> > > >    static inline struct net_iov *netmem_to_net_iov(netmem_ref netmem)
> > > >    {
> > > >        if (netmem_is_net_iov(netmem))
> > > > @@ -314,6 +340,21 @@ static inline netmem_ref netmem_compound_head(netmem_ref netmem)
> > > >        return page_to_netmem(compound_head(netmem_to_page(netmem)));
> > > >    }
> > > > 
> > > > +#define nmdesc_to_page(nmdesc)               (_Generic((nmdesc),             \
> > > > +     const struct netmem_desc * :    (const struct page *)(nmdesc),  \
> > > > +     struct netmem_desc * :          (struct page *)(nmdesc)))
> > > 
> > > Considering that nmdesc is going to be separated from pages and
> > > accessed through indirection, and back reference to the page is
> > > not needed (at least for net/), this helper shouldn't even exist.
> > > And in fact, you don't really use it ...
> > > > +static inline struct netmem_desc *page_to_nmdesc(struct page *page)
> > > > +{
> > > > +     VM_BUG_ON_PAGE(PageTail(page), page);
> > > > +     return (struct netmem_desc *)page;
> > > > +}
> > > > +
> > > > +static inline void *nmdesc_address(struct netmem_desc *nmdesc)
> > > > +{
> > > > +     return page_address(nmdesc_to_page(nmdesc));
> > > > +}
> > > 
> > > ... That's the only caller, and nmdesc_address() is not used, so
> > > just nuke both of them. This helper doesn't even make sense.
> > > 
> > > Please avoid introducing functions that you don't use as a general
> > > rule.
> > 
> > I'm sorry about making you confused.  I should've included another patch
> > using the helper like the following.
> 
> Ah, I see. And still, it's not a great function. There should be
> no way to extract a page or a page address from a nmdesc.
> 
> For the diff below it's same as with the mt76 patch, it's allocating
> a page, expects it to be a page, using it as a page, but for no reason
> keeps it wrapped into netmem. It only adds confusion and overhead.
> A rule of thumb would be only converting to netmem if the new code
> would be able to work with a netmem-wrapped net_iovs.

Thanks.  I'm now working on this job, avoiding your concern.

By the way, am I supposed to wait for you to complete the work about
extracting type from page e.g. page pool (or bump) type?

	Byungchul

> > diff --git a/drivers/net/ethernet/intel/idpf/idpf_txrx.c b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> > index cef9dfb877e8..adccc7c8e68f 100644
> > --- a/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> > +++ b/drivers/net/ethernet/intel/idpf/idpf_txrx.c
> > @@ -3266,7 +3266,7 @@ static u32 idpf_rx_hsplit_wa(const struct libeth_fqe *hdr,
> >                            struct libeth_fqe *buf, u32 data_len)
> >   {
> >       u32 copy = data_len <= L1_CACHE_BYTES ? data_len : ETH_HLEN;
> > -     struct page *hdr_page, *buf_page;
> > +     struct netmem_desc *hdr_nmdesc, *buf_nmdesc;
> >       const void *src;
> >       void *dst;
> > 
> > @@ -3274,10 +3274,10 @@ static u32 idpf_rx_hsplit_wa(const struct libeth_fqe *hdr,
> >           !libeth_rx_sync_for_cpu(buf, copy))
> >               return 0;
> > 
> > -     hdr_page = __netmem_to_page(hdr->netmem);
> > -     buf_page = __netmem_to_page(buf->netmem);
> > -     dst = page_address(hdr_page) + hdr->offset + hdr_page->pp->p.offset;
> > -     src = page_address(buf_page) + buf->offset + buf_page->pp->p.offset;
> > +     hdr_nmdesc = __netmem_to_nmdesc(hdr->netmem);
> > +     buf_nmdesc = __netmem_to_nmdesc(buf->netmem);
> > +     dst = nmdesc_address(hdr_nmdesc) + hdr->offset + hdr_nmdesc->pp->p.offset;
> > +     src = nmdesc_address(buf_nmdesc) + buf->offset + buf_nmdesc->pp->p.offset;
> > 
> >       memcpy(dst, src, LARGEST_ALIGN(copy));
> >       buf->offset += copy;
> --
> Pavel Begunkov

Re: [PATCH net-next v9 1/8] netmem: introduce struct netmem_desc mirroring struct page

[Pavel Begunkov]

On 7/14/25 05:23, Byungchul Park wrote:
> On Sat, Jul 12, 2025 at 03:39:59PM +0100, Pavel Begunkov wrote:
>> On 7/10/25 09:28, Byungchul Park wrote:
>>> To simplify struct page, the page pool members of struct page should be
>>> moved to other, allowing these members to be removed from struct page.
>>>
>>> Introduce a network memory descriptor to store the members, struct
>>> netmem_desc, and make it union'ed with the existing fields in struct
>>> net_iov, allowing to organize the fields of struct net_iov.
>>
>> FWIW, regardless of memdesc business, I think it'd be great to have
>> this patch, as it'll help with some of the netmem casting ugliness and
>> shed some cycles as well. For example, we have a bunch of
>> niov -> netmem -> niov casts in various places.
> 
> If Jakub agrees with this, I will re-post this as a separate patch so
> that works that require this base can go ahead.

I think it'd be a good idea. It's needed to clean up netmem handling,
and I'll convert io_uring and get rid of the union in niov.

The diff below should give a rough idea of what I want to use it for.
It kills __netmem_clear_lsb() to avoid casting struct page * to niov.
And saves some masking for zcrx, see page_pool_get_dma_addr_nmdesc(),
and there are more places like that.


diff --git a/include/net/netmem.h b/include/net/netmem.h
index 535cf17b9134..41f3a3fd6b6c 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -247,6 +247,8 @@ static inline unsigned long netmem_pfn_trace(netmem_ref netmem)
  	return page_to_pfn(netmem_to_page(netmem));
  }
  
+#define pp_page_to_nmdesc(page)	((struct netmem_desc *)(page))
+
  /* __netmem_clear_lsb - convert netmem_ref to struct net_iov * for access to
   * common fields.
   * @netmem: netmem reference to extract as net_iov.
@@ -262,11 +264,18 @@ static inline unsigned long netmem_pfn_trace(netmem_ref netmem)
   *
   * Return: the netmem_ref cast to net_iov* regardless of its underlying type.
   */
-static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem)
+static inline struct net_iov *__netmem_to_niov(netmem_ref netmem)
  {
  	return (struct net_iov *)((__force unsigned long)netmem & ~NET_IOV);
  }
  
+static inline struct netmem_desc *netmem_to_nmdesc(netmem_ref netmem)
+{
+	if (netmem_is_net_iov(netmem))
+		return &__netmem_to_niov(netmem)->desc;
+	return pp_page_to_nmdesc(__netmem_to_page(netmem));
+}
+
  /**
   * __netmem_get_pp - unsafely get pointer to the &page_pool backing @netmem
   * @netmem: netmem reference to get the pointer from
@@ -280,17 +289,17 @@ static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem)
   */
  static inline struct page_pool *__netmem_get_pp(netmem_ref netmem)
  {
-	return __netmem_to_page(netmem)->pp;
+	return pp_page_to_nmdesc(__netmem_to_page(netmem))->pp;
  }
  
  static inline struct page_pool *netmem_get_pp(netmem_ref netmem)
  {
-	return __netmem_clear_lsb(netmem)->pp;
+	return netmem_to_nmdesc(netmem)->pp;
  }
  
  static inline atomic_long_t *netmem_get_pp_ref_count_ref(netmem_ref netmem)
  {
-	return &__netmem_clear_lsb(netmem)->pp_ref_count;
+	return &netmem_to_nmdesc(netmem)->pp_ref_count;
  }
  
  static inline bool netmem_is_pref_nid(netmem_ref netmem, int pref_nid)
@@ -355,7 +364,7 @@ static inline bool netmem_is_pfmemalloc(netmem_ref netmem)
  
  static inline unsigned long netmem_get_dma_addr(netmem_ref netmem)
  {
-	return __netmem_clear_lsb(netmem)->dma_addr;
+	return netmem_to_nmdesc(netmem)->dma_addr;
  }
  
  void get_netmem(netmem_ref netmem);
diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index db180626be06..002858f3bcb3 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -425,9 +425,9 @@ static inline void page_pool_free_va(struct page_pool *pool, void *va,
  	page_pool_put_page(pool, virt_to_head_page(va), -1, allow_direct);
  }
  
-static inline dma_addr_t page_pool_get_dma_addr_netmem(netmem_ref netmem)
+static inline dma_addr_t page_pool_get_dma_addr_nmdesc(struct netmem_desc *desc)
  {
-	dma_addr_t ret = netmem_get_dma_addr(netmem);
+	dma_addr_t ret = desc->dma_addr;
  
  	if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA)
  		ret <<= PAGE_SHIFT;
@@ -435,6 +435,13 @@ static inline dma_addr_t page_pool_get_dma_addr_netmem(netmem_ref netmem)
  	return ret;
  }
  
+static inline dma_addr_t page_pool_get_dma_addr_netmem(netmem_ref netmem)
+{
+	struct netmem_desc *desc = netmem_to_nmdesc(netmem);
+
+	return page_pool_get_dma_addr_nmdesc(desc);
+}
+
  /**
   * page_pool_get_dma_addr() - Retrieve the stored DMA address.
   * @page:	page allocated from a page pool
diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
index 085eeed8cd50..2e80692d9ee1 100644
--- a/io_uring/zcrx.c
+++ b/io_uring/zcrx.c
@@ -290,7 +290,7 @@ static void io_zcrx_sync_for_device(const struct page_pool *pool,
  	if (!dma_dev_need_sync(pool->p.dev))
  		return;
  
-	dma_addr = page_pool_get_dma_addr_netmem(net_iov_to_netmem(niov));
+	dma_addr = page_pool_get_dma_addr_nmdesc(&niov->desc);
  	__dma_sync_single_for_device(pool->p.dev, dma_addr + pool->p.offset,
  				     PAGE_SIZE, pool->p.dma_dir);
  #endif
diff --git a/net/core/netmem_priv.h b/net/core/netmem_priv.h
index cd95394399b4..97d4beda9174 100644
--- a/net/core/netmem_priv.h
+++ b/net/core/netmem_priv.h
@@ -5,19 +5,21 @@
  
  static inline unsigned long netmem_get_pp_magic(netmem_ref netmem)
  {
-	return __netmem_clear_lsb(netmem)->pp_magic & ~PP_DMA_INDEX_MASK;
+	return netmem_to_nmdesc(netmem)->pp_magic & ~PP_DMA_INDEX_MASK;
  }
  
  static inline void netmem_or_pp_magic(netmem_ref netmem, unsigned long pp_magic)
  {
-	__netmem_clear_lsb(netmem)->pp_magic |= pp_magic;
+	netmem_to_nmdesc(netmem)->pp_magic |= pp_magic;
  }
  
  static inline void netmem_clear_pp_magic(netmem_ref netmem)
  {
-	WARN_ON_ONCE(__netmem_clear_lsb(netmem)->pp_magic & PP_DMA_INDEX_MASK);
+	struct netmem_desc *desc = netmem_to_nmdesc(netmem);
  
-	__netmem_clear_lsb(netmem)->pp_magic = 0;
+	WARN_ON_ONCE(desc->pp_magic & PP_DMA_INDEX_MASK);
+
+	desc->pp_magic = 0;
  }
  
  static inline bool netmem_is_pp(netmem_ref netmem)
@@ -27,13 +29,13 @@ static inline bool netmem_is_pp(netmem_ref netmem)
  
  static inline void netmem_set_pp(netmem_ref netmem, struct page_pool *pool)
  {
-	__netmem_clear_lsb(netmem)->pp = pool;
+	netmem_to_nmdesc(netmem)->pp = pool;
  }
  
  static inline void netmem_set_dma_addr(netmem_ref netmem,
  				       unsigned long dma_addr)
  {
-	__netmem_clear_lsb(netmem)->dma_addr = dma_addr;
+	netmem_to_nmdesc(netmem)->dma_addr = dma_addr;
  }
  
  static inline unsigned long netmem_get_dma_index(netmem_ref netmem)
@@ -43,7 +45,7 @@ static inline unsigned long netmem_get_dma_index(netmem_ref netmem)
  	if (WARN_ON_ONCE(netmem_is_net_iov(netmem)))
  		return 0;
  
-	magic = __netmem_clear_lsb(netmem)->pp_magic;
+	magic = netmem_to_nmdesc(netmem)->pp_magic;
  
  	return (magic & PP_DMA_INDEX_MASK) >> PP_DMA_INDEX_SHIFT;
  }
@@ -51,12 +53,12 @@ static inline unsigned long netmem_get_dma_index(netmem_ref netmem)
  static inline void netmem_set_dma_index(netmem_ref netmem,
  					unsigned long id)
  {
-	unsigned long magic;
+	struct netmem_desc *desc;
  
  	if (WARN_ON_ONCE(netmem_is_net_iov(netmem)))
  		return;
  
-	magic = netmem_get_pp_magic(netmem) | (id << PP_DMA_INDEX_SHIFT);
-	__netmem_clear_lsb(netmem)->pp_magic = magic;
+	desc = netmem_to_nmdesc(netmem);
+	desc->pp_magic |= id << PP_DMA_INDEX_SHIFT;
  }
  #endif

Re: [PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Pavel Begunkov]

On 7/14/25 11:05, Byungchul Park wrote:
> On Mon, Jul 14, 2025 at 10:43:35AM +0100, Pavel Begunkov wrote:
>> On 7/14/25 00:07, Byungchul Park wrote:
>>> On Sat, Jul 12, 2025 at 12:59:34PM +0100, Pavel Begunkov wrote:
>>>> On 7/10/25 09:28, Byungchul Park wrote:
>>>> ...> +
>>>>>     static inline struct net_iov *netmem_to_net_iov(netmem_ref netmem)
>>>>>     {
>>>>>         if (netmem_is_net_iov(netmem))
>>>>> @@ -314,6 +340,21 @@ static inline netmem_ref netmem_compound_head(netmem_ref netmem)
>>>>>         return page_to_netmem(compound_head(netmem_to_page(netmem)));
>>>>>     }
>>>>>
>>>>> +#define nmdesc_to_page(nmdesc)               (_Generic((nmdesc),             \
>>>>> +     const struct netmem_desc * :    (const struct page *)(nmdesc),  \
>>>>> +     struct netmem_desc * :          (struct page *)(nmdesc)))
>>>>
>>>> Considering that nmdesc is going to be separated from pages and
>>>> accessed through indirection, and back reference to the page is
>>>> not needed (at least for net/), this helper shouldn't even exist.
>>>> And in fact, you don't really use it ...
>>>>> +static inline struct netmem_desc *page_to_nmdesc(struct page *page)
>>>>> +{
>>>>> +     VM_BUG_ON_PAGE(PageTail(page), page);
>>>>> +     return (struct netmem_desc *)page;
>>>>> +}
>>>>> +
>>>>> +static inline void *nmdesc_address(struct netmem_desc *nmdesc)
>>>>> +{
>>>>> +     return page_address(nmdesc_to_page(nmdesc));
>>>>> +}
>>>>
>>>> ... That's the only caller, and nmdesc_address() is not used, so
>>>> just nuke both of them. This helper doesn't even make sense.
>>>>
>>>> Please avoid introducing functions that you don't use as a general
>>>> rule.
>>>
>>> I'm sorry about making you confused.  I should've included another patch
>>> using the helper like the following.
>>
>> Ah, I see. And still, it's not a great function. There should be
>> no way to extract a page or a page address from a nmdesc.
>>
>> For the diff below it's same as with the mt76 patch, it's allocating
>> a page, expects it to be a page, using it as a page, but for no reason
>> keeps it wrapped into netmem. It only adds confusion and overhead.
>> A rule of thumb would be only converting to netmem if the new code
>> would be able to work with a netmem-wrapped net_iovs.
> 
> Thanks.  I'm now working on this job, avoiding your concern.
> 
> By the way, am I supposed to wait for you to complete the work about
> extracting type from page e.g. page pool (or bump) type?

1/8 doesn't depend on it, if you're sending it separately. As for
the rest, it might need to wait for the PGTY change, which is more
likely to be for 6.18

-- 
Pavel Begunkov

Re: [PATCH net-next v9 1/8] netmem: introduce struct netmem_desc mirroring struct page

[Byungchul Park]

On Mon, Jul 14, 2025 at 12:30:12PM +0100, Pavel Begunkov wrote:
> On 7/14/25 05:23, Byungchul Park wrote:
> > On Sat, Jul 12, 2025 at 03:39:59PM +0100, Pavel Begunkov wrote:
> > > On 7/10/25 09:28, Byungchul Park wrote:
> > > > To simplify struct page, the page pool members of struct page should be
> > > > moved to other, allowing these members to be removed from struct page.
> > > > 
> > > > Introduce a network memory descriptor to store the members, struct
> > > > netmem_desc, and make it union'ed with the existing fields in struct
> > > > net_iov, allowing to organize the fields of struct net_iov.
> > > 
> > > FWIW, regardless of memdesc business, I think it'd be great to have
> > > this patch, as it'll help with some of the netmem casting ugliness and
> > > shed some cycles as well. For example, we have a bunch of
> > > niov -> netmem -> niov casts in various places.
> > 
> > If Jakub agrees with this, I will re-post this as a separate patch so
> > that works that require this base can go ahead.
> 
> I think it'd be a good idea. It's needed to clean up netmem handling,
> and I'll convert io_uring and get rid of the union in niov.
> 
> The diff below should give a rough idea of what I want to use it for.
> It kills __netmem_clear_lsb() to avoid casting struct page * to niov.
> And saves some masking for zcrx, see page_pool_get_dma_addr_nmdesc(),
> and there are more places like that.
> 
> 
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 535cf17b9134..41f3a3fd6b6c 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -247,6 +247,8 @@ static inline unsigned long netmem_pfn_trace(netmem_ref netmem)
>        return page_to_pfn(netmem_to_page(netmem));
>  }
> 
> +#define pp_page_to_nmdesc(page)        ((struct netmem_desc *)(page))
> +
>  /* __netmem_clear_lsb - convert netmem_ref to struct net_iov * for access to
>   * common fields.
>   * @netmem: netmem reference to extract as net_iov.
> @@ -262,11 +264,18 @@ static inline unsigned long netmem_pfn_trace(netmem_ref netmem)
>   *
>   * Return: the netmem_ref cast to net_iov* regardless of its underlying type.
>   */
> -static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem)
> +static inline struct net_iov *__netmem_to_niov(netmem_ref netmem)
>  {
>        return (struct net_iov *)((__force unsigned long)netmem & ~NET_IOV);
>  }
> 
> +static inline struct netmem_desc *netmem_to_nmdesc(netmem_ref netmem)

I removed netmem_to_nmdesc() and its users, while I was working for v10.
However, You can add it if needed for your clean up.

I think I should share v10 now, to share the decision I made.

	Byungchul

> +{
> +       if (netmem_is_net_iov(netmem))
> +               return &__netmem_to_niov(netmem)->desc;
> +       return pp_page_to_nmdesc(__netmem_to_page(netmem));
> +}
> +
>  /**
>   * __netmem_get_pp - unsafely get pointer to the &page_pool backing @netmem
>   * @netmem: netmem reference to get the pointer from
> @@ -280,17 +289,17 @@ static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem)
>   */
>  static inline struct page_pool *__netmem_get_pp(netmem_ref netmem)
>  {
> -       return __netmem_to_page(netmem)->pp;
> +       return pp_page_to_nmdesc(__netmem_to_page(netmem))->pp;
>  }
> 
>  static inline struct page_pool *netmem_get_pp(netmem_ref netmem)
>  {
> -       return __netmem_clear_lsb(netmem)->pp;
> +       return netmem_to_nmdesc(netmem)->pp;
>  }
> 
>  static inline atomic_long_t *netmem_get_pp_ref_count_ref(netmem_ref netmem)
>  {
> -       return &__netmem_clear_lsb(netmem)->pp_ref_count;
> +       return &netmem_to_nmdesc(netmem)->pp_ref_count;
>  }
> 
>  static inline bool netmem_is_pref_nid(netmem_ref netmem, int pref_nid)
> @@ -355,7 +364,7 @@ static inline bool netmem_is_pfmemalloc(netmem_ref netmem)
> 
>  static inline unsigned long netmem_get_dma_addr(netmem_ref netmem)
>  {
> -       return __netmem_clear_lsb(netmem)->dma_addr;
> +       return netmem_to_nmdesc(netmem)->dma_addr;
>  }
> 
>  void get_netmem(netmem_ref netmem);
> diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
> index db180626be06..002858f3bcb3 100644
> --- a/include/net/page_pool/helpers.h
> +++ b/include/net/page_pool/helpers.h
> @@ -425,9 +425,9 @@ static inline void page_pool_free_va(struct page_pool *pool, void *va,
>        page_pool_put_page(pool, virt_to_head_page(va), -1, allow_direct);
>  }
> 
> -static inline dma_addr_t page_pool_get_dma_addr_netmem(netmem_ref netmem)
> +static inline dma_addr_t page_pool_get_dma_addr_nmdesc(struct netmem_desc *desc)
>  {
> -       dma_addr_t ret = netmem_get_dma_addr(netmem);
> +       dma_addr_t ret = desc->dma_addr;
> 
>        if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA)
>                ret <<= PAGE_SHIFT;
> @@ -435,6 +435,13 @@ static inline dma_addr_t page_pool_get_dma_addr_netmem(netmem_ref netmem)
>        return ret;
>  }
> 
> +static inline dma_addr_t page_pool_get_dma_addr_netmem(netmem_ref netmem)
> +{
> +       struct netmem_desc *desc = netmem_to_nmdesc(netmem);
> +
> +       return page_pool_get_dma_addr_nmdesc(desc);
> +}
> +
>  /**
>   * page_pool_get_dma_addr() - Retrieve the stored DMA address.
>   * @page:     page allocated from a page pool
> diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
> index 085eeed8cd50..2e80692d9ee1 100644
> --- a/io_uring/zcrx.c
> +++ b/io_uring/zcrx.c
> @@ -290,7 +290,7 @@ static void io_zcrx_sync_for_device(const struct page_pool *pool,
>        if (!dma_dev_need_sync(pool->p.dev))
>                return;
> 
> -       dma_addr = page_pool_get_dma_addr_netmem(net_iov_to_netmem(niov));
> +       dma_addr = page_pool_get_dma_addr_nmdesc(&niov->desc);
>        __dma_sync_single_for_device(pool->p.dev, dma_addr + pool->p.offset,
>                                     PAGE_SIZE, pool->p.dma_dir);
>  #endif
> diff --git a/net/core/netmem_priv.h b/net/core/netmem_priv.h
> index cd95394399b4..97d4beda9174 100644
> --- a/net/core/netmem_priv.h
> +++ b/net/core/netmem_priv.h
> @@ -5,19 +5,21 @@
> 
>  static inline unsigned long netmem_get_pp_magic(netmem_ref netmem)
>  {
> -       return __netmem_clear_lsb(netmem)->pp_magic & ~PP_DMA_INDEX_MASK;
> +       return netmem_to_nmdesc(netmem)->pp_magic & ~PP_DMA_INDEX_MASK;
>  }
> 
>  static inline void netmem_or_pp_magic(netmem_ref netmem, unsigned long pp_magic)
>  {
> -       __netmem_clear_lsb(netmem)->pp_magic |= pp_magic;
> +       netmem_to_nmdesc(netmem)->pp_magic |= pp_magic;
>  }
> 
>  static inline void netmem_clear_pp_magic(netmem_ref netmem)
>  {
> -       WARN_ON_ONCE(__netmem_clear_lsb(netmem)->pp_magic & PP_DMA_INDEX_MASK);
> +       struct netmem_desc *desc = netmem_to_nmdesc(netmem);
> 
> -       __netmem_clear_lsb(netmem)->pp_magic = 0;
> +       WARN_ON_ONCE(desc->pp_magic & PP_DMA_INDEX_MASK);
> +
> +       desc->pp_magic = 0;
>  }
> 
>  static inline bool netmem_is_pp(netmem_ref netmem)
> @@ -27,13 +29,13 @@ static inline bool netmem_is_pp(netmem_ref netmem)
> 
>  static inline void netmem_set_pp(netmem_ref netmem, struct page_pool *pool)
>  {
> -       __netmem_clear_lsb(netmem)->pp = pool;
> +       netmem_to_nmdesc(netmem)->pp = pool;
>  }
> 
>  static inline void netmem_set_dma_addr(netmem_ref netmem,
>                                       unsigned long dma_addr)
>  {
> -       __netmem_clear_lsb(netmem)->dma_addr = dma_addr;
> +       netmem_to_nmdesc(netmem)->dma_addr = dma_addr;
>  }
> 
>  static inline unsigned long netmem_get_dma_index(netmem_ref netmem)
> @@ -43,7 +45,7 @@ static inline unsigned long netmem_get_dma_index(netmem_ref netmem)
>        if (WARN_ON_ONCE(netmem_is_net_iov(netmem)))
>                return 0;
> 
> -       magic = __netmem_clear_lsb(netmem)->pp_magic;
> +       magic = netmem_to_nmdesc(netmem)->pp_magic;
> 
>        return (magic & PP_DMA_INDEX_MASK) >> PP_DMA_INDEX_SHIFT;
>  }
> @@ -51,12 +53,12 @@ static inline unsigned long netmem_get_dma_index(netmem_ref netmem)
>  static inline void netmem_set_dma_index(netmem_ref netmem,
>                                        unsigned long id)
>  {
> -       unsigned long magic;
> +       struct netmem_desc *desc;
> 
>        if (WARN_ON_ONCE(netmem_is_net_iov(netmem)))
>                return;
> 
> -       magic = netmem_get_pp_magic(netmem) | (id << PP_DMA_INDEX_SHIFT);
> -       __netmem_clear_lsb(netmem)->pp_magic = magic;
> +       desc = netmem_to_nmdesc(netmem);
> +       desc->pp_magic |= id << PP_DMA_INDEX_SHIFT;
>  }
>  #endif

Re: [PATCH net-next v9 2/8] netmem: introduce utility APIs to use struct netmem_desc

[Byungchul Park]

On Mon, Jul 14, 2025 at 12:45:17PM +0100, Pavel Begunkov wrote:
> On 7/14/25 11:05, Byungchul Park wrote:
> > On Mon, Jul 14, 2025 at 10:43:35AM +0100, Pavel Begunkov wrote:
> > > On 7/14/25 00:07, Byungchul Park wrote:
> > > > On Sat, Jul 12, 2025 at 12:59:34PM +0100, Pavel Begunkov wrote:
> > > > > On 7/10/25 09:28, Byungchul Park wrote:
> > > > > ...> +
> > > > > >     static inline struct net_iov *netmem_to_net_iov(netmem_ref netmem)
> > > > > >     {
> > > > > >         if (netmem_is_net_iov(netmem))
> > > > > > @@ -314,6 +340,21 @@ static inline netmem_ref netmem_compound_head(netmem_ref netmem)
> > > > > >         return page_to_netmem(compound_head(netmem_to_page(netmem)));
> > > > > >     }
> > > > > > 
> > > > > > +#define nmdesc_to_page(nmdesc)               (_Generic((nmdesc),             \
> > > > > > +     const struct netmem_desc * :    (const struct page *)(nmdesc),  \
> > > > > > +     struct netmem_desc * :          (struct page *)(nmdesc)))
> > > > > 
> > > > > Considering that nmdesc is going to be separated from pages and
> > > > > accessed through indirection, and back reference to the page is
> > > > > not needed (at least for net/), this helper shouldn't even exist.
> > > > > And in fact, you don't really use it ...
> > > > > > +static inline struct netmem_desc *page_to_nmdesc(struct page *page)
> > > > > > +{
> > > > > > +     VM_BUG_ON_PAGE(PageTail(page), page);
> > > > > > +     return (struct netmem_desc *)page;
> > > > > > +}
> > > > > > +
> > > > > > +static inline void *nmdesc_address(struct netmem_desc *nmdesc)
> > > > > > +{
> > > > > > +     return page_address(nmdesc_to_page(nmdesc));
> > > > > > +}
> > > > > 
> > > > > ... That's the only caller, and nmdesc_address() is not used, so
> > > > > just nuke both of them. This helper doesn't even make sense.
> > > > > 
> > > > > Please avoid introducing functions that you don't use as a general
> > > > > rule.
> > > > 
> > > > I'm sorry about making you confused.  I should've included another patch
> > > > using the helper like the following.
> > > 
> > > Ah, I see. And still, it's not a great function. There should be
> > > no way to extract a page or a page address from a nmdesc.
> > > 
> > > For the diff below it's same as with the mt76 patch, it's allocating
> > > a page, expects it to be a page, using it as a page, but for no reason
> > > keeps it wrapped into netmem. It only adds confusion and overhead.
> > > A rule of thumb would be only converting to netmem if the new code
> > > would be able to work with a netmem-wrapped net_iovs.
> > 
> > Thanks.  I'm now working on this job, avoiding your concern.
> > 
> > By the way, am I supposed to wait for you to complete the work about
> > extracting type from page e.g. page pool (or bump) type?
> 
> 1/8 doesn't depend on it, if you're sending it separately. As for

Right.

> the rest, it might need to wait for the PGTY change, which is more

Only 3/8 needs to wait for the PGTY change.  The rest can be merged
regardless of it if acceptable.

	Byungchul

> likely to be for 6.18
> 
> --
> Pavel Begunkov

Re: [PATCH net-next v9 6/8] mlx4: use netmem descriptor and APIs for page pool

[Mina Almasry]

On Thu, Jul 10, 2025 at 6:33 PM Byungchul Park <byungchul@sk.com> wrote:
>
> On Thu, Jul 10, 2025 at 11:29:35AM -0700, Mina Almasry wrote:
> > On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
> > >
> > > To simplify struct page, the effort to separate its own descriptor from
> > > struct page is required and the work for page pool is on going.
> > >
> > > Use netmem descriptor and APIs for page pool in mlx4 code.
> > >
> > > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > > ---
> > >  drivers/net/ethernet/mellanox/mlx4/en_rx.c   | 48 +++++++++++---------
> > >  drivers/net/ethernet/mellanox/mlx4/en_tx.c   |  8 ++--
> > >  drivers/net/ethernet/mellanox/mlx4/mlx4_en.h |  4 +-
> > >  3 files changed, 32 insertions(+), 28 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_rx.c b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> > > index b33285d755b9..7cf0d2dc5011 100644
> > > --- a/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> > > +++ b/drivers/net/ethernet/mellanox/mlx4/en_rx.c
> > > @@ -62,18 +62,18 @@ static int mlx4_en_alloc_frags(struct mlx4_en_priv *priv,
> > >         int i;
> > >
> > >         for (i = 0; i < priv->num_frags; i++, frags++) {
> > > -               if (!frags->page) {
> > > -                       frags->page = page_pool_alloc_pages(ring->pp, gfp);
> > > -                       if (!frags->page) {
> > > +               if (!frags->netmem) {
> > > +                       frags->netmem = page_pool_alloc_netmems(ring->pp, gfp);
> > > +                       if (!frags->netmem) {
> > >                                 ring->alloc_fail++;
> > >                                 return -ENOMEM;
> > >                         }
> > > -                       page_pool_fragment_page(frags->page, 1);
> > > +                       page_pool_fragment_netmem(frags->netmem, 1);
> > >                         frags->page_offset = priv->rx_headroom;
> > >
> > >                         ring->rx_alloc_pages++;
> > >                 }
> > > -               dma = page_pool_get_dma_addr(frags->page);
> > > +               dma = page_pool_get_dma_addr_netmem(frags->netmem);
> > >                 rx_desc->data[i].addr = cpu_to_be64(dma + frags->page_offset);
> > >         }
> > >         return 0;
> > > @@ -83,10 +83,10 @@ static void mlx4_en_free_frag(const struct mlx4_en_priv *priv,
> > >                               struct mlx4_en_rx_ring *ring,
> > >                               struct mlx4_en_rx_alloc *frag)
> > >  {
> > > -       if (frag->page)
> > > -               page_pool_put_full_page(ring->pp, frag->page, false);
> > > +       if (frag->netmem)
> > > +               page_pool_put_full_netmem(ring->pp, frag->netmem, false);
> > >         /* We need to clear all fields, otherwise a change of priv->log_rx_info
> > > -        * could lead to see garbage later in frag->page.
> > > +        * could lead to see garbage later in frag->netmem.
> > >          */
> > >         memset(frag, 0, sizeof(*frag));
> > >  }
> > > @@ -440,29 +440,33 @@ static int mlx4_en_complete_rx_desc(struct mlx4_en_priv *priv,
> > >         unsigned int truesize = 0;
> > >         bool release = true;
> > >         int nr, frag_size;
> > > -       struct page *page;
> > > +       netmem_ref netmem;
> > >         dma_addr_t dma;
> > >
> > >         /* Collect used fragments while replacing them in the HW descriptors */
> > >         for (nr = 0;; frags++) {
> > >                 frag_size = min_t(int, length, frag_info->frag_size);
> > >
> > > -               page = frags->page;
> > > -               if (unlikely(!page))
> > > +               netmem = frags->netmem;
> > > +               if (unlikely(!netmem))
> > >                         goto fail;
> > >
> > > -               dma = page_pool_get_dma_addr(page);
> > > +               dma = page_pool_get_dma_addr_netmem(netmem);
> > >                 dma_sync_single_range_for_cpu(priv->ddev, dma, frags->page_offset,
> > >                                               frag_size, priv->dma_dir);
> > >
> > > -               __skb_fill_page_desc(skb, nr, page, frags->page_offset,
> > > -                                    frag_size);
> > > +               __skb_fill_netmem_desc(skb, nr, netmem, frags->page_offset,
> > > +                                      frag_size);
> > >
> > >                 truesize += frag_info->frag_stride;
> > >                 if (frag_info->frag_stride == PAGE_SIZE / 2) {
> > > +                       struct page *page = netmem_to_page(netmem);
> >
> > This cast is not safe, try to use the netmem type directly.
>
> Can it be net_iov?  It already ensures it's a page-backed netmem.  Why
> is that unsafe?
>

Precisely because it can be net_iov. The whole point of netmem_ref is
that it's an abstract type that can be something else underneath.
Converting the driver to netmem only to then say "well, the netmem is
just pages, cast it back" is just adding a lot of useless typecasting
and wasted cpu cycles with no benefit. In general netmem_to_page casts
in drivers are heavily discouraged.

> With netmem, page_count() and page_to_nid() cannot be used, but needed.
> Or checking 'page == NULL' after the casting works for you?
>

I'm really hoping that you can read the code and be a bit familiar
with the driver that you're modifying and suggest a solution that
works for us. netmem_to_page() followed by page_count() without a NULL
check crashes the kernel if the netmem is not a page underneath... so
it shouldn't work for anyone.

What may be acceptable here is netmem_count helper and what not like
we have netmem_is_pfmemalloc which handles net_iov correctly.

However, it looks like this driver is trying to check if the netmem is
recyclable, we have a helper like that already in
__page_pool_page_can_be_recycled. Maybe that can be reused somehow.

-- 
Thanks,
Mina

Re: [PATCH net-next v9 3/8] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[Mina Almasry]

On Thu, Jul 10, 2025 at 6:14 PM Byungchul Park <byungchul@sk.com> wrote:
>
> On Thu, Jul 10, 2025 at 11:19:53AM -0700, Mina Almasry wrote:
> > On Thu, Jul 10, 2025 at 1:28 AM Byungchul Park <byungchul@sk.com> wrote:
> > >
> > > To simplify struct page, the effort to separate its own descriptor from
> > > struct page is required and the work for page pool is on going.
> > >
> > > To achieve that, all the code should avoid directly accessing page pool
> > > members of struct page.
> > >
> > > Access ->pp_magic through struct netmem_desc instead of directly
> > > accessing it through struct page in page_pool_page_is_pp().  Plus, move
> > > page_pool_page_is_pp() from mm.h to netmem.h to use struct netmem_desc
> > > without header dependency issue.
> > >
> > > Signed-off-by: Byungchul Park <byungchul@sk.com>
> > > Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
> > > Reviewed-by: Mina Almasry <almasrymina@google.com>
> > > Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>
> > > Reviewed-by: Vlastimil Babka <vbabka@suse.cz>
> > > Acked-by: Harry Yoo <harry.yoo@oracle.com>
> > > ---
> > >  include/linux/mm.h   | 12 ------------
> > >  include/net/netmem.h | 17 +++++++++++++++++
> > >  mm/page_alloc.c      |  1 +
> > >  3 files changed, 18 insertions(+), 12 deletions(-)
> > >
> > > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > > index 0ef2ba0c667a..0b7f7f998085 100644
> > > --- a/include/linux/mm.h
> > > +++ b/include/linux/mm.h
> > > @@ -4172,16 +4172,4 @@ int arch_lock_shadow_stack_status(struct task_struct *t, unsigned long status);
> > >   */
> > >  #define PP_MAGIC_MASK ~(PP_DMA_INDEX_MASK | 0x3UL)
> > >
> > > -#ifdef CONFIG_PAGE_POOL
> > > -static inline bool page_pool_page_is_pp(struct page *page)
> > > -{
> > > -       return (page->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> > > -}
> > > -#else
> > > -static inline bool page_pool_page_is_pp(struct page *page)
> > > -{
> > > -       return false;
> > > -}
> > > -#endif
> > > -
> > >  #endif /* _LINUX_MM_H */
> > > diff --git a/include/net/netmem.h b/include/net/netmem.h
> > > index ad9444be229a..11e9de45efcb 100644
> > > --- a/include/net/netmem.h
> > > +++ b/include/net/netmem.h
> > > @@ -355,6 +355,23 @@ static inline void *nmdesc_address(struct netmem_desc *nmdesc)
> > >         return page_address(nmdesc_to_page(nmdesc));
> > >  }
> > >
> > > +#ifdef CONFIG_PAGE_POOL
> > > +/* XXX: This would better be moved to mm, once mm gets its way to
> > > + * identify the type of page for page pool.
> > > + */
> > > +static inline bool page_pool_page_is_pp(struct page *page)
> > > +{
> > > +       struct netmem_desc *desc = page_to_nmdesc(page);
> > > +
> > > +       return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> > > +}
> >
> > pages can be pp pages (where they have pp fields inside of them) or
> > non-pp pages (where they don't have pp fields inside them, because
> > they were never allocated from the page_pool).
> >
> > Casting a page to a netmem_desc, and then checking if the page was a
> > pp page doesn't makes sense to me on a fundamental level. The
> > netmem_desc is only valid if the page was a pp page in the first
> > place. Maybe page_to_nmdesc should reject the cast if the page is not
> > a pp page or something.
>
> Right, as you already know, the current mainline code already has the
> same problem but we've been using the werid way so far, in other words,
> mm code is checking if it's a pp page or not by using ->pp_magic, but
> it's ->lur, ->buddy_list, or ->pcp_list if it's not a pp page.
>
> Both the mainline code and this patch can make sense *only if* it's
> actually a pp page.  It's unevitable until mm provides a way to identify
> the type of page for page pool.  Thoughts?

I don't see mainline having a problem. Mainline checks that the page
is a pp page via the magic before using any of the pp fields. This is
because a page* can be a pp page or a non-pp page.

With netmem_desc, having a netmem_desc* should imply that the
underlying memory is a pp page. Having a netmem_desc* that is not
valid because the pp_magic is not correct complicates the code for no
reason. Every user of netmem_desc has to check pp_magic before
actually using the fields. page_to_nmdesc should just refuse to return
a netmem_desc* if the page is not a pp page.

Also, this patch has my Reviewed-by, even though I honestly don't see
it as acceptable and I clearly have feedback (and Pavel seems too?).

__please__, when you make significant changes to a patch, you have to
reset the Reviewed-by tags.

-- 
Thanks,
Mina

Re: [PATCH net-next v9 1/8] netmem: introduce struct netmem_desc mirroring struct page

[Mina Almasry]

On Mon, Jul 14, 2025 at 4:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> On 7/14/25 05:23, Byungchul Park wrote:
> > On Sat, Jul 12, 2025 at 03:39:59PM +0100, Pavel Begunkov wrote:
> >> On 7/10/25 09:28, Byungchul Park wrote:
> >>> To simplify struct page, the page pool members of struct page should be
> >>> moved to other, allowing these members to be removed from struct page.
> >>>
> >>> Introduce a network memory descriptor to store the members, struct
> >>> netmem_desc, and make it union'ed with the existing fields in struct
> >>> net_iov, allowing to organize the fields of struct net_iov.
> >>
> >> FWIW, regardless of memdesc business, I think it'd be great to have
> >> this patch, as it'll help with some of the netmem casting ugliness and
> >> shed some cycles as well. For example, we have a bunch of
> >> niov -> netmem -> niov casts in various places.
> >
> > If Jakub agrees with this, I will re-post this as a separate patch so
> > that works that require this base can go ahead.
>
> I think it'd be a good idea. It's needed to clean up netmem handling,
> and I'll convert io_uring and get rid of the union in niov.
>
> The diff below should give a rough idea of what I want to use it for.
> It kills __netmem_clear_lsb() to avoid casting struct page * to niov.
> And saves some masking for zcrx, see page_pool_get_dma_addr_nmdesc(),
> and there are more places like that.
>
>
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index 535cf17b9134..41f3a3fd6b6c 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -247,6 +247,8 @@ static inline unsigned long netmem_pfn_trace(netmem_ref netmem)
>         return page_to_pfn(netmem_to_page(netmem));
>   }
>
> +#define pp_page_to_nmdesc(page)        ((struct netmem_desc *)(page))
> +
>   /* __netmem_clear_lsb - convert netmem_ref to struct net_iov * for access to
>    * common fields.
>    * @netmem: netmem reference to extract as net_iov.
> @@ -262,11 +264,18 @@ static inline unsigned long netmem_pfn_trace(netmem_ref netmem)
>    *
>    * Return: the netmem_ref cast to net_iov* regardless of its underlying type.
>    */
> -static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem)
> +static inline struct net_iov *__netmem_to_niov(netmem_ref netmem)
>   {
>         return (struct net_iov *)((__force unsigned long)netmem & ~NET_IOV);
>   }
>
> +static inline struct netmem_desc *netmem_to_nmdesc(netmem_ref netmem)
> +{
> +       if (netmem_is_net_iov(netmem))
> +               return &__netmem_to_niov(netmem)->desc;
> +       return pp_page_to_nmdesc(__netmem_to_page(netmem));
> +}
> +

I think instead of netmem_to_nmdesc, you want __netmem_clear_lsb to
return a netmem_desc instead of net_iov.

__netmem_clear_lsb returning a net_iov was always a bit of a hack. The
return value of __netmem_clear_lsb is clearly not a net_iov, but we
needed to access the pp fields, and net_iov encapsulates the pp
fields.

-- 
Thanks,
Mina

Re: [PATCH net-next v9 3/8] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[Pavel Begunkov]

On 7/14/25 20:09, Mina Almasry wrote:
> On Thu, Jul 10, 2025 at 6:14 PM Byungchul Park <byungchul@sk.com> wrote:
...>> Both the mainline code and this patch can make sense *only if* it's
>> actually a pp page.  It's unevitable until mm provides a way to identify
>> the type of page for page pool.  Thoughts?
> 
> I don't see mainline having a problem. Mainline checks that the page
> is a pp page via the magic before using any of the pp fields. This is
> because a page* can be a pp page or a non-pp page.
> 
> With netmem_desc, having a netmem_desc* should imply that the
> underlying memory is a pp page. Having a netmem_desc* that is not
> valid because the pp_magic is not correct complicates the code for no
> reason. Every user of netmem_desc has to check pp_magic before
> actually using the fields. page_to_nmdesc should just refuse to return
> a netmem_desc* if the page is not a pp page.
> 
> Also, this patch has my Reviewed-by, even though I honestly don't see
> it as acceptable and I clearly have feedback (and Pavel seems too?).

I was fine with it as a transitory solution, but there is nothing
to argue about anymore since mm already got a nice way to type
check pages and we can use that.
  > __please__, when you make significant changes to a patch, you have to
> reset the Reviewed-by tags.
> 

-- 
Pavel Begunkov

Re: [PATCH net-next v9 1/8] netmem: introduce struct netmem_desc mirroring struct page

[Pavel Begunkov]

On 7/14/25 20:17, Mina Almasry wrote:
> On Mon, Jul 14, 2025 at 4:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
...>> diff --git a/include/net/netmem.h b/include/net/netmem.h
>> index 535cf17b9134..41f3a3fd6b6c 100644
>> --- a/include/net/netmem.h
>> +++ b/include/net/netmem.h
>> @@ -247,6 +247,8 @@ static inline unsigned long netmem_pfn_trace(netmem_ref netmem)
>>          return page_to_pfn(netmem_to_page(netmem));
>>    }
>>
>> +#define pp_page_to_nmdesc(page)        ((struct netmem_desc *)(page))
>> +
>>    /* __netmem_clear_lsb - convert netmem_ref to struct net_iov * for access to
>>     * common fields.
>>     * @netmem: netmem reference to extract as net_iov.
>> @@ -262,11 +264,18 @@ static inline unsigned long netmem_pfn_trace(netmem_ref netmem)
>>     *
>>     * Return: the netmem_ref cast to net_iov* regardless of its underlying type.
>>     */
>> -static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem)
>> +static inline struct net_iov *__netmem_to_niov(netmem_ref netmem)
>>    {
>>          return (struct net_iov *)((__force unsigned long)netmem & ~NET_IOV);
>>    }
>>
>> +static inline struct netmem_desc *netmem_to_nmdesc(netmem_ref netmem)
>> +{
>> +       if (netmem_is_net_iov(netmem))
>> +               return &__netmem_to_niov(netmem)->desc;
>> +       return pp_page_to_nmdesc(__netmem_to_page(netmem));
>> +}
>> +
> 
> I think instead of netmem_to_nmdesc, you want __netmem_clear_lsb to
> return a netmem_desc instead of net_iov.

Same thing, the diff just renames __netmem_clear_lsb -> netmem_to_nmdesc
on top.

> __netmem_clear_lsb returning a net_iov was always a bit of a hack. The
> return value of __netmem_clear_lsb is clearly not a net_iov, but we
> needed to access the pp fields, and net_iov encapsulates the pp
> fields.

Right, and netmem_desc nicely solves that. I remember suggesting such
a common type during review to the initial netmem / niov patches as well.

-- 
Pavel Begunkov

Re: [PATCH net-next v9 3/8] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[Byungchul Park]

On Sat, Jul 12, 2025 at 02:58:14PM +0100, Pavel Begunkov wrote:
> On 7/11/25 02:14, Byungchul Park wrote:
> ...>>> +#ifdef CONFIG_PAGE_POOL
> > > > +/* XXX: This would better be moved to mm, once mm gets its way to
> > > > + * identify the type of page for page pool.
> > > > + */
> > > > +static inline bool page_pool_page_is_pp(struct page *page)
> > > > +{
> > > > +       struct netmem_desc *desc = page_to_nmdesc(page);
> > > > +
> > > > +       return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> > > > +}
> > > 
> > > pages can be pp pages (where they have pp fields inside of them) or
> > > non-pp pages (where they don't have pp fields inside them, because
> > > they were never allocated from the page_pool).
> > > 
> > > Casting a page to a netmem_desc, and then checking if the page was a
> > > pp page doesn't makes sense to me on a fundamental level. The
> > > netmem_desc is only valid if the page was a pp page in the first
> > > place. Maybe page_to_nmdesc should reject the cast if the page is not
> > > a pp page or something.
> > 
> > Right, as you already know, the current mainline code already has the
> > same problem but we've been using the werid way so far, in other words,
> > mm code is checking if it's a pp page or not by using ->pp_magic, but
> > it's ->lur, ->buddy_list, or ->pcp_list if it's not a pp page.
> > 
> > Both the mainline code and this patch can make sense *only if* it's
> > actually a pp page.  It's unevitable until mm provides a way to identify
> > the type of page for page pool.  Thoughts?
> Question to mm folks, can we add a new PGTY for page pool and use
> that to filter page pool originated pages? Like in the incomplete
> and untested diff below?
> 
> 
> commit 8fc2347fb3ff4a3fc7929c70a5a21e1128935d4a
> Author: Pavel Begunkov <asml.silence@gmail.com>
> Date:   Sat Jul 12 14:29:52 2025 +0100
> 
>     net/mm: use PGTY for tracking page pool pages
> 
>     Currently, we use page->pp_magic to determine whether a page belongs to
>     a page pool. It's not ideal as the field is aliased with other page
>     types, and thus needs to to rely on elaborated rules to work. Add a new
>     page type for page pool.

Hi Pavel,

I need this work to be done to remove ->pp_magic in struct page.  Will
you let me work on this work?  Or can you please refine and post this
work?  If you let me, I will go for this as a separate patch from this
series.

	Byungchul

> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0ef2ba0c667a..975a013f1f17 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -4175,7 +4175,7 @@ int arch_lock_shadow_stack_status(struct task_struct *t, unsigned long status);
>  #ifdef CONFIG_PAGE_POOL
>  static inline bool page_pool_page_is_pp(struct page *page)
>  {
> -       return (page->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> +       return PageNetpp(page);
>  }
>  #else
>  static inline bool page_pool_page_is_pp(struct page *page)
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 4fe5ee67535b..9bd1dfded2fc 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -957,6 +957,7 @@ enum pagetype {
>        PGTY_zsmalloc           = 0xf6,
>        PGTY_unaccepted         = 0xf7,
>        PGTY_large_kmalloc      = 0xf8,
> +       PGTY_netpp              = 0xf9,
> 
>        PGTY_mapcount_underflow = 0xff
>  };
> @@ -1101,6 +1102,11 @@ PAGE_TYPE_OPS(Zsmalloc, zsmalloc, zsmalloc)
>  PAGE_TYPE_OPS(Unaccepted, unaccepted, unaccepted)
>  FOLIO_TYPE_OPS(large_kmalloc, large_kmalloc)
> 
> +/*
> + * Marks page_pool allocated pages
> + */
> +PAGE_TYPE_OPS(Netpp, netpp, netpp)
> +
>  /**
>   * PageHuge - Determine if the page belongs to hugetlbfs
>   * @page: The page to test.
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index de1d95f04076..20f5dbb08149 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -113,6 +113,8 @@ static inline bool netmem_is_net_iov(const netmem_ref netmem)
>   */
>  static inline struct page *__netmem_to_page(netmem_ref netmem)
>  {
> +       DEBUG_NET_WARN_ON_ONCE(netmem_is_net_iov(netmem));
> +
>        return (__force struct page *)netmem;
>  }
> 
> diff --git a/net/core/netmem_priv.h b/net/core/netmem_priv.h
> index cd95394399b4..e38c64da1a78 100644
> --- a/net/core/netmem_priv.h
> +++ b/net/core/netmem_priv.h
> @@ -13,16 +13,11 @@ static inline void netmem_or_pp_magic(netmem_ref netmem, unsigned long pp_magic)
>        __netmem_clear_lsb(netmem)->pp_magic |= pp_magic;
>  }
> 
> -static inline void netmem_clear_pp_magic(netmem_ref netmem)
> -{
> -       WARN_ON_ONCE(__netmem_clear_lsb(netmem)->pp_magic & PP_DMA_INDEX_MASK);
> -
> -       __netmem_clear_lsb(netmem)->pp_magic = 0;
> -}
> -
>  static inline bool netmem_is_pp(netmem_ref netmem)
>  {
> -       return (netmem_get_pp_magic(netmem) & PP_MAGIC_MASK) == PP_SIGNATURE;
> +       if (netmem_is_net_iov(netmem))
> +               return true;
> +       return page_pool_page_is_pp(netmem_to_page(netmem));
>  }
> 
>  static inline void netmem_set_pp(netmem_ref netmem, struct page_pool *pool)
> diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> index 05e2e22a8f7c..52120e2912a6 100644
> --- a/net/core/page_pool.c
> +++ b/net/core/page_pool.c
> @@ -371,6 +371,13 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
>  }
>  EXPORT_SYMBOL(page_pool_create);
> 
> +static void page_pool_set_page_pp_info(struct page_pool *pool,
> +                                      struct page *page)
> +{
> +       __SetPageNetpp(page);
> +       page_pool_set_pp_info(page_to_netmem(page));
> +}
> +
>  static void page_pool_return_netmem(struct page_pool *pool, netmem_ref netmem);
> 
>  static noinline netmem_ref page_pool_refill_alloc_cache(struct page_pool *pool)
> @@ -534,7 +541,7 @@ static struct page *__page_pool_alloc_page_order(struct page_pool *pool,
>        }
> 
>        alloc_stat_inc(pool, slow_high_order);
> -       page_pool_set_pp_info(pool, page_to_netmem(page));
> +       page_pool_set_page_pp_info(pool, page);
> 
>        /* Track how many pages are held 'in-flight' */
>        pool->pages_state_hold_cnt++;
> @@ -579,7 +586,7 @@ static noinline netmem_ref __page_pool_alloc_netmems_slow(struct page_pool *pool
>                        continue;
>                }
> 
> -               page_pool_set_pp_info(pool, netmem);
> +               page_pool_set_page_pp_info(pool, __netmem_to_page(netmem));
>                pool->alloc.cache[pool->alloc.count++] = netmem;
>                /* Track how many pages are held 'in-flight' */
>                pool->pages_state_hold_cnt++;
> @@ -654,7 +661,6 @@ s32 page_pool_inflight(const struct page_pool *pool, bool strict)
>  void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)
>  {
>        netmem_set_pp(netmem, pool);
> -       netmem_or_pp_magic(netmem, PP_SIGNATURE);
> 
>        /* Ensuring all pages have been split into one fragment initially:
>         * page_pool_set_pp_info() is only called once for every page when it
> @@ -669,7 +675,6 @@ void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)
> 
>  void page_pool_clear_pp_info(netmem_ref netmem)
>  {
> -       netmem_clear_pp_magic(netmem);
>        netmem_set_pp(netmem, NULL);
>  }
> 
> @@ -730,8 +735,11 @@ static void page_pool_return_netmem(struct page_pool *pool, netmem_ref netmem)
>        trace_page_pool_state_release(pool, netmem, count);
> 
>        if (put) {
> +               struct page *page = netmem_to_page(netmem);
> +
>                page_pool_clear_pp_info(netmem);
> -               put_page(netmem_to_page(netmem));
> +               __ClearPageNetpp(page);
> +               put_page(page);
>        }
>        /* An optimization would be to call __free_pages(page, pool->p.order)
>         * knowing page is not part of page-cache (thus avoiding a
> 
> --
> Pavel Begunkov

Re: [PATCH net-next v9 3/8] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[Byungchul Park]

On Thu, Jul 17, 2025 at 12:08:58PM +0900, Byungchul Park wrote:
> On Sat, Jul 12, 2025 at 02:58:14PM +0100, Pavel Begunkov wrote:
> > On 7/11/25 02:14, Byungchul Park wrote:
> > ...>>> +#ifdef CONFIG_PAGE_POOL
> > > > > +/* XXX: This would better be moved to mm, once mm gets its way to
> > > > > + * identify the type of page for page pool.
> > > > > + */
> > > > > +static inline bool page_pool_page_is_pp(struct page *page)
> > > > > +{
> > > > > +       struct netmem_desc *desc = page_to_nmdesc(page);
> > > > > +
> > > > > +       return (desc->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> > > > > +}
> > > > 
> > > > pages can be pp pages (where they have pp fields inside of them) or
> > > > non-pp pages (where they don't have pp fields inside them, because
> > > > they were never allocated from the page_pool).
> > > > 
> > > > Casting a page to a netmem_desc, and then checking if the page was a
> > > > pp page doesn't makes sense to me on a fundamental level. The
> > > > netmem_desc is only valid if the page was a pp page in the first
> > > > place. Maybe page_to_nmdesc should reject the cast if the page is not
> > > > a pp page or something.
> > > 
> > > Right, as you already know, the current mainline code already has the
> > > same problem but we've been using the werid way so far, in other words,
> > > mm code is checking if it's a pp page or not by using ->pp_magic, but
> > > it's ->lur, ->buddy_list, or ->pcp_list if it's not a pp page.
> > > 
> > > Both the mainline code and this patch can make sense *only if* it's
> > > actually a pp page.  It's unevitable until mm provides a way to identify
> > > the type of page for page pool.  Thoughts?
> > Question to mm folks, can we add a new PGTY for page pool and use
> > that to filter page pool originated pages? Like in the incomplete
> > and untested diff below?
> > 
> > 
> > commit 8fc2347fb3ff4a3fc7929c70a5a21e1128935d4a
> > Author: Pavel Begunkov <asml.silence@gmail.com>
> > Date:   Sat Jul 12 14:29:52 2025 +0100
> > 
> >     net/mm: use PGTY for tracking page pool pages
> > 
> >     Currently, we use page->pp_magic to determine whether a page belongs to
> >     a page pool. It's not ideal as the field is aliased with other page
> >     types, and thus needs to to rely on elaborated rules to work. Add a new
> >     page type for page pool.
> 
> Hi Pavel,
> 
> I need this work to be done to remove ->pp_magic in struct page.  Will
> you let me work on this work?  Or can you please refine and post this

No response I got.  Thus, I started.  I hope you understand.

	Byungchul

> work?  If you let me, I will go for this as a separate patch from this
> series.
> 
> 	Byungchul
> 
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 0ef2ba0c667a..975a013f1f17 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -4175,7 +4175,7 @@ int arch_lock_shadow_stack_status(struct task_struct *t, unsigned long status);
> >  #ifdef CONFIG_PAGE_POOL
> >  static inline bool page_pool_page_is_pp(struct page *page)
> >  {
> > -       return (page->pp_magic & PP_MAGIC_MASK) == PP_SIGNATURE;
> > +       return PageNetpp(page);
> >  }
> >  #else
> >  static inline bool page_pool_page_is_pp(struct page *page)
> > diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> > index 4fe5ee67535b..9bd1dfded2fc 100644
> > --- a/include/linux/page-flags.h
> > +++ b/include/linux/page-flags.h
> > @@ -957,6 +957,7 @@ enum pagetype {
> >        PGTY_zsmalloc           = 0xf6,
> >        PGTY_unaccepted         = 0xf7,
> >        PGTY_large_kmalloc      = 0xf8,
> > +       PGTY_netpp              = 0xf9,
> > 
> >        PGTY_mapcount_underflow = 0xff
> >  };
> > @@ -1101,6 +1102,11 @@ PAGE_TYPE_OPS(Zsmalloc, zsmalloc, zsmalloc)
> >  PAGE_TYPE_OPS(Unaccepted, unaccepted, unaccepted)
> >  FOLIO_TYPE_OPS(large_kmalloc, large_kmalloc)
> > 
> > +/*
> > + * Marks page_pool allocated pages
> > + */
> > +PAGE_TYPE_OPS(Netpp, netpp, netpp)
> > +
> >  /**
> >   * PageHuge - Determine if the page belongs to hugetlbfs
> >   * @page: The page to test.
> > diff --git a/include/net/netmem.h b/include/net/netmem.h
> > index de1d95f04076..20f5dbb08149 100644
> > --- a/include/net/netmem.h
> > +++ b/include/net/netmem.h
> > @@ -113,6 +113,8 @@ static inline bool netmem_is_net_iov(const netmem_ref netmem)
> >   */
> >  static inline struct page *__netmem_to_page(netmem_ref netmem)
> >  {
> > +       DEBUG_NET_WARN_ON_ONCE(netmem_is_net_iov(netmem));
> > +
> >        return (__force struct page *)netmem;
> >  }
> > 
> > diff --git a/net/core/netmem_priv.h b/net/core/netmem_priv.h
> > index cd95394399b4..e38c64da1a78 100644
> > --- a/net/core/netmem_priv.h
> > +++ b/net/core/netmem_priv.h
> > @@ -13,16 +13,11 @@ static inline void netmem_or_pp_magic(netmem_ref netmem, unsigned long pp_magic)
> >        __netmem_clear_lsb(netmem)->pp_magic |= pp_magic;
> >  }
> > 
> > -static inline void netmem_clear_pp_magic(netmem_ref netmem)
> > -{
> > -       WARN_ON_ONCE(__netmem_clear_lsb(netmem)->pp_magic & PP_DMA_INDEX_MASK);
> > -
> > -       __netmem_clear_lsb(netmem)->pp_magic = 0;
> > -}
> > -
> >  static inline bool netmem_is_pp(netmem_ref netmem)
> >  {
> > -       return (netmem_get_pp_magic(netmem) & PP_MAGIC_MASK) == PP_SIGNATURE;
> > +       if (netmem_is_net_iov(netmem))
> > +               return true;
> > +       return page_pool_page_is_pp(netmem_to_page(netmem));
> >  }
> > 
> >  static inline void netmem_set_pp(netmem_ref netmem, struct page_pool *pool)
> > diff --git a/net/core/page_pool.c b/net/core/page_pool.c
> > index 05e2e22a8f7c..52120e2912a6 100644
> > --- a/net/core/page_pool.c
> > +++ b/net/core/page_pool.c
> > @@ -371,6 +371,13 @@ struct page_pool *page_pool_create(const struct page_pool_params *params)
> >  }
> >  EXPORT_SYMBOL(page_pool_create);
> > 
> > +static void page_pool_set_page_pp_info(struct page_pool *pool,
> > +                                      struct page *page)
> > +{
> > +       __SetPageNetpp(page);
> > +       page_pool_set_pp_info(page_to_netmem(page));
> > +}
> > +
> >  static void page_pool_return_netmem(struct page_pool *pool, netmem_ref netmem);
> > 
> >  static noinline netmem_ref page_pool_refill_alloc_cache(struct page_pool *pool)
> > @@ -534,7 +541,7 @@ static struct page *__page_pool_alloc_page_order(struct page_pool *pool,
> >        }
> > 
> >        alloc_stat_inc(pool, slow_high_order);
> > -       page_pool_set_pp_info(pool, page_to_netmem(page));
> > +       page_pool_set_page_pp_info(pool, page);
> > 
> >        /* Track how many pages are held 'in-flight' */
> >        pool->pages_state_hold_cnt++;
> > @@ -579,7 +586,7 @@ static noinline netmem_ref __page_pool_alloc_netmems_slow(struct page_pool *pool
> >                        continue;
> >                }
> > 
> > -               page_pool_set_pp_info(pool, netmem);
> > +               page_pool_set_page_pp_info(pool, __netmem_to_page(netmem));
> >                pool->alloc.cache[pool->alloc.count++] = netmem;
> >                /* Track how many pages are held 'in-flight' */
> >                pool->pages_state_hold_cnt++;
> > @@ -654,7 +661,6 @@ s32 page_pool_inflight(const struct page_pool *pool, bool strict)
> >  void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)
> >  {
> >        netmem_set_pp(netmem, pool);
> > -       netmem_or_pp_magic(netmem, PP_SIGNATURE);
> > 
> >        /* Ensuring all pages have been split into one fragment initially:
> >         * page_pool_set_pp_info() is only called once for every page when it
> > @@ -669,7 +675,6 @@ void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem)
> > 
> >  void page_pool_clear_pp_info(netmem_ref netmem)
> >  {
> > -       netmem_clear_pp_magic(netmem);
> >        netmem_set_pp(netmem, NULL);
> >  }
> > 
> > @@ -730,8 +735,11 @@ static void page_pool_return_netmem(struct page_pool *pool, netmem_ref netmem)
> >        trace_page_pool_state_release(pool, netmem, count);
> > 
> >        if (put) {
> > +               struct page *page = netmem_to_page(netmem);
> > +
> >                page_pool_clear_pp_info(netmem);
> > -               put_page(netmem_to_page(netmem));
> > +               __ClearPageNetpp(page);
> > +               put_page(page);
> >        }
> >        /* An optimization would be to call __free_pages(page, pool->p.order)
> >         * knowing page is not part of page-cache (thus avoiding a
> > 
> > --
> > Pavel Begunkov

Re: [PATCH net-next v9 3/8] page_pool: access ->pp_magic through struct netmem_desc in page_pool_page_is_pp()

[Pavel Begunkov]

On 7/22/25 02:23, Byungchul Park wrote:
> On Thu, Jul 17, 2025 at 12:08:58PM +0900, Byungchul Park wrote:
>> On Sat, Jul 12, 2025 at 02:58:14PM +0100, Pavel Begunkov wrote:
>>> On 7/11/25 02:14, Byungchul Park wrote:
...>>> commit 8fc2347fb3ff4a3fc7929c70a5a21e1128935d4a
>>> Author: Pavel Begunkov <asml.silence@gmail.com>
>>> Date:   Sat Jul 12 14:29:52 2025 +0100
>>>
>>>      net/mm: use PGTY for tracking page pool pages
>>>
>>>      Currently, we use page->pp_magic to determine whether a page belongs to
>>>      a page pool. It's not ideal as the field is aliased with other page
>>>      types, and thus needs to to rely on elaborated rules to work. Add a new
>>>      page type for page pool.
>>
>> Hi Pavel,
>>
>> I need this work to be done to remove ->pp_magic in struct page.  Will
>> you let me work on this work?  Or can you please refine and post this
> 
> No response I got.  Thus, I started.  I hope you understand.
Missed the first message and then got busy. Anyway, sure, go ahead.

-- 
Pavel Begunkov