[RFC net-next v1 0/6] nmdesc cleanups and optimisations

[RFC net-next v1 0/6] nmdesc cleanups and optimisations

[Pavel Begunkov]

This series uses newly introduced struct netmem_desc, which represents
common fields b/w netmem types, for optimisations and to start
addressing some of the netmem technical debt.

First, replace __netmem_clear_lsb with netmem_to_nmdesc(). The helper
optimises pp fields accesses, but now we can do the same but cleaner.

The second problem is abundance of places where the user has struct
page / net_iov but still needs to cast it to netmem back and forth
just to be able to use generic helpers. It's not the prettiest
pattern and often can't be optimised. Start introducing netmem_desc
based helpers and using them instead.

There is more work we can do, but these are the patches I want to
pull into zcrx. It's an RFC for now, I'll send it as a pull request
without zcrx bits.

Byungchul Park (1):
  net: replace __netmem_clear_lsb() with netmem_to_nmdesc()

Pavel Begunkov (5):
  net: move pp_page_to_nmdesc()
  net: page_pool: remove page_pool_set_dma_addr()
  net: convert page pool dma helpers to netmem_desc
  net: page_pool: convert refcounting helpers to nmdesc
  io_uring/zcrx: avoid netmem casts with nmdesc

 include/net/netmem.h            | 75 +++++++++++++++------------------
 include/net/page_pool/helpers.h | 41 ++++++++++++++----
 io_uring/zcrx.c                 | 12 +++---
 net/core/devmem.c               |  5 ---
 net/core/netmem_priv.h          | 20 +++------
 net/core/page_pool_priv.h       | 14 +++---
 6 files changed, 82 insertions(+), 85 deletions(-)

-- 
2.49.0

[RFC net-next v1 1/6] net: move pp_page_to_nmdesc()

[Pavel Begunkov]

A preparation patch moving pp_page_to_nmdesc() up in the header file and
no other changes otherwise. It reduces cluttering from the following
patch.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 include/net/netmem.h | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index f7dacc9e75fd..8b639e45cfe2 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -217,6 +217,23 @@ static inline netmem_ref net_iov_to_netmem(struct net_iov *niov)
 	const struct page * :	(__force const netmem_ref)(p),	\
 	struct page * :		(__force netmem_ref)(p)))
 
+/* XXX: How to extract netmem_desc from page must be changed, once
+ * netmem_desc no longer overlays on page and will be allocated through
+ * slab.
+ */
+#define __pp_page_to_nmdesc(p)	(_Generic((p),				\
+	const struct page * :	(const struct netmem_desc *)(p),	\
+	struct page * :		(struct netmem_desc *)(p)))
+
+/* CAUTION: Check if the page is a pp page before calling this helper or
+ * know it's a pp page.
+ */
+#define pp_page_to_nmdesc(p)						\
+({									\
+	DEBUG_NET_WARN_ON_ONCE(!page_pool_page_is_pp(p));		\
+	__pp_page_to_nmdesc(p);						\
+})
+
 /**
  * virt_to_netmem - convert virtual memory pointer to a netmem reference
  * @data: host memory pointer to convert
@@ -285,23 +302,6 @@ static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem)
 	return (struct net_iov *)((__force unsigned long)netmem & ~NET_IOV);
 }
 
-/* XXX: How to extract netmem_desc from page must be changed, once
- * netmem_desc no longer overlays on page and will be allocated through
- * slab.
- */
-#define __pp_page_to_nmdesc(p)	(_Generic((p),				\
-	const struct page * :	(const struct netmem_desc *)(p),	\
-	struct page * :		(struct netmem_desc *)(p)))
-
-/* CAUTION: Check if the page is a pp page before calling this helper or
- * know it's a pp page.
- */
-#define pp_page_to_nmdesc(p)						\
-({									\
-	DEBUG_NET_WARN_ON_ONCE(!page_pool_page_is_pp(p));		\
-	__pp_page_to_nmdesc(p);						\
-})
-
 /**
  * __netmem_get_pp - unsafely get pointer to the &page_pool backing @netmem
  * @netmem: netmem reference to get the pointer from
-- 
2.49.0

[RFC net-next v1 2/6] net: replace __netmem_clear_lsb() with netmem_to_nmdesc()

[Pavel Begunkov]

From: Byungchul Park <byungchul@sk.com>

Now that we have struct netmem_desc, it'd better access the pp fields
via struct netmem_desc rather than struct net_iov.

Introduce netmem_to_nmdesc() for safely converting netmem_ref to
netmem_desc regardless of the type underneath e.i. netmem_desc, net_iov.

While at it, remove __netmem_clear_lsb() and make netmem_to_nmdesc()
used instead.

Suggested-by: Pavel Begunkov <asml.silence@gmail.com>
Signed-off-by: Byungchul Park <byungchul@sk.com>
Reviewed-by: Mina Almasry <almasrymina@google.com>
[pavel: rebased, cleaned up, added comments]
Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 include/net/netmem.h   | 37 +++++++++++++++++++------------------
 net/core/netmem_priv.h | 16 ++++++++--------
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index 8b639e45cfe2..d08797e40a7c 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -282,24 +282,25 @@ static inline struct netmem_desc *__netmem_to_nmdesc(netmem_ref netmem)
 	return (__force struct netmem_desc *)netmem;
 }
 
-/* __netmem_clear_lsb - convert netmem_ref to struct net_iov * for access to
- * common fields.
- * @netmem: netmem reference to extract as net_iov.
+/* netmem_to_nmdesc - convert netmem_ref to struct netmem_desc.
+ * @netmem: netmem reference to get netmem_desc.
  *
- * All the sub types of netmem_ref (page, net_iov) have the same pp, pp_magic,
- * dma_addr, and pp_ref_count fields at the same offsets. Thus, we can access
- * these fields without a type check to make sure that the underlying mem is
- * net_iov or page.
- *
- * The resulting value of this function can only be used to access the fields
- * that are NET_IOV_ASSERT_OFFSET'd. Accessing any other fields will result in
- * undefined behavior.
- *
- * Return: the netmem_ref cast to net_iov* regardless of its underlying type.
+ * Return: the pointer to struct netmem_desc * regardless of its
+ * underlying type.
  */
-static inline struct net_iov *__netmem_clear_lsb(netmem_ref netmem)
+static inline struct netmem_desc *netmem_to_nmdesc(netmem_ref netmem)
 {
-	return (struct net_iov *)((__force unsigned long)netmem & ~NET_IOV);
+	void *p = (void *)((__force unsigned long)netmem & ~NET_IOV);
+
+	/* Note: can be replaced with netmem_to_net_iov(), but open coded
+	 * NET_IOV masking gives better code generation.
+	 */
+	if (netmem_is_net_iov(netmem)) {
+		struct net_iov *niov = p;
+
+		return &niov->desc;
+	}
+	return __pp_page_to_nmdesc((struct page *)p);
 }
 
 /**
@@ -320,12 +321,12 @@ static inline struct page_pool *__netmem_get_pp(netmem_ref netmem)
 
 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)
@@ -390,7 +391,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/net/core/netmem_priv.h b/net/core/netmem_priv.h
index cd95394399b4..23175cb2bd86 100644
--- a/net/core/netmem_priv.h
+++ b/net/core/netmem_priv.h
@@ -5,19 +5,19 @@
 
 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);
+	WARN_ON_ONCE(netmem_to_nmdesc(netmem)->pp_magic & PP_DMA_INDEX_MASK);
 
-	__netmem_clear_lsb(netmem)->pp_magic = 0;
+	netmem_to_nmdesc(netmem)->pp_magic = 0;
 }
 
 static inline bool netmem_is_pp(netmem_ref netmem)
@@ -27,13 +27,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 +43,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;
 }
@@ -57,6 +57,6 @@ static inline void netmem_set_dma_index(netmem_ref netmem,
 		return;
 
 	magic = netmem_get_pp_magic(netmem) | (id << PP_DMA_INDEX_SHIFT);
-	__netmem_clear_lsb(netmem)->pp_magic = magic;
+	netmem_to_nmdesc(netmem)->pp_magic = magic;
 }
 #endif
-- 
2.49.0

[RFC net-next v1 3/6] net: page_pool: remove page_pool_set_dma_addr()

[Pavel Begunkov]

page_pool_set_dma_addr() is not used, kill it.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 net/core/page_pool_priv.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/net/core/page_pool_priv.h b/net/core/page_pool_priv.h
index 2fb06d5f6d55..6445e7da5b82 100644
--- a/net/core/page_pool_priv.h
+++ b/net/core/page_pool_priv.h
@@ -32,11 +32,6 @@ page_pool_set_dma_addr_netmem(netmem_ref netmem, dma_addr_t addr)
 	return false;
 }
 
-static inline bool page_pool_set_dma_addr(struct page *page, dma_addr_t addr)
-{
-	return page_pool_set_dma_addr_netmem(page_to_netmem(page), addr);
-}
-
 #if defined(CONFIG_PAGE_POOL)
 void page_pool_set_pp_info(struct page_pool *pool, netmem_ref netmem);
 void page_pool_clear_pp_info(netmem_ref netmem);
-- 
2.49.0

[RFC net-next v1 4/6] net: convert page pool dma helpers to netmem_desc

[Pavel Begunkov]

struct netmem_desc has a clearly defined field that keeps dma_addr. Use
the new type in netmem and page_pool functions and get rid of a bunch of
now unnecessary accessor helpers.

While doing so, extract a helper for getting a dma address out of a
netmem desc, which can be used to optimise paths that already know the
underlying netmem type like memory providers.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 include/net/netmem.h            |  5 -----
 include/net/page_pool/helpers.h | 12 ++++++++++--
 net/core/netmem_priv.h          |  6 ------
 net/core/page_pool_priv.h       |  9 +++++----
 4 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index d08797e40a7c..ca6d5d151acc 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -389,11 +389,6 @@ static inline bool netmem_is_pfmemalloc(netmem_ref netmem)
 	return page_is_pfmemalloc(netmem_to_page(netmem));
 }
 
-static inline unsigned long netmem_get_dma_addr(netmem_ref netmem)
-{
-	return netmem_to_nmdesc(netmem)->dma_addr;
-}
-
 void get_netmem(netmem_ref netmem);
 void put_netmem(netmem_ref netmem);
 
diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index db180626be06..a9774d582933 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -425,9 +425,10 @@ 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(const struct netmem_desc *nmdesc)
 {
-	dma_addr_t ret = netmem_get_dma_addr(netmem);
+	dma_addr_t ret = nmdesc->dma_addr;
 
 	if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA)
 		ret <<= PAGE_SHIFT;
@@ -435,6 +436,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)
+{
+	const 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/net/core/netmem_priv.h b/net/core/netmem_priv.h
index 23175cb2bd86..070faa136305 100644
--- a/net/core/netmem_priv.h
+++ b/net/core/netmem_priv.h
@@ -30,12 +30,6 @@ static inline void netmem_set_pp(netmem_ref netmem, struct page_pool *pool)
 	netmem_to_nmdesc(netmem)->pp = pool;
 }
 
-static inline void netmem_set_dma_addr(netmem_ref netmem,
-				       unsigned long dma_addr)
-{
-	netmem_to_nmdesc(netmem)->dma_addr = dma_addr;
-}
-
 static inline unsigned long netmem_get_dma_index(netmem_ref netmem)
 {
 	unsigned long magic;
diff --git a/net/core/page_pool_priv.h b/net/core/page_pool_priv.h
index 6445e7da5b82..fcf70cbc3e09 100644
--- a/net/core/page_pool_priv.h
+++ b/net/core/page_pool_priv.h
@@ -18,17 +18,18 @@ void page_pool_unlist(struct page_pool *pool);
 static inline bool
 page_pool_set_dma_addr_netmem(netmem_ref netmem, dma_addr_t addr)
 {
+	struct netmem_desc *desc = netmem_to_nmdesc(netmem);
+
 	if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA) {
-		netmem_set_dma_addr(netmem, addr >> PAGE_SHIFT);
+		desc->dma_addr = addr >> PAGE_SHIFT;
 
 		/* We assume page alignment to shave off bottom bits,
 		 * if this "compression" doesn't work we need to drop.
 		 */
-		return addr != (dma_addr_t)netmem_get_dma_addr(netmem)
-				       << PAGE_SHIFT;
+		return addr != desc->dma_addr << PAGE_SHIFT;
 	}
 
-	netmem_set_dma_addr(netmem, addr);
+	desc->dma_addr = addr;
 	return false;
 }
 
-- 
2.49.0

[RFC net-next v1 5/6] net: page_pool: convert refcounting helpers to nmdesc

[Pavel Begunkov]

Use netmem descriptors for the basic buffer refcounting helpers and use
them to implement all other variants. This way netmem type aware helpers
can avoid intermediate netmem casting and bit masking/unmasking.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 include/net/netmem.h            |  5 -----
 include/net/page_pool/helpers.h | 29 ++++++++++++++++++++++-------
 net/core/devmem.c               |  5 -----
 3 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/include/net/netmem.h b/include/net/netmem.h
index ca6d5d151acc..7b5f1427f272 100644
--- a/include/net/netmem.h
+++ b/include/net/netmem.h
@@ -324,11 +324,6 @@ static inline struct page_pool *netmem_get_pp(netmem_ref netmem)
 	return netmem_to_nmdesc(netmem)->pp;
 }
 
-static inline atomic_long_t *netmem_get_pp_ref_count_ref(netmem_ref netmem)
-{
-	return &netmem_to_nmdesc(netmem)->pp_ref_count;
-}
-
 static inline bool netmem_is_pref_nid(netmem_ref netmem, int pref_nid)
 {
 	/* NUMA node preference only makes sense if we're allocating
diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
index a9774d582933..bc54040186d9 100644
--- a/include/net/page_pool/helpers.h
+++ b/include/net/page_pool/helpers.h
@@ -234,9 +234,14 @@ page_pool_get_dma_dir(const struct page_pool *pool)
 	return pool->p.dma_dir;
 }
 
+static inline void page_pool_fragment_nmdesc(struct netmem_desc *desc, long nr)
+{
+	atomic_long_set(&desc->pp_ref_count, nr);
+}
+
 static inline void page_pool_fragment_netmem(netmem_ref netmem, long nr)
 {
-	atomic_long_set(netmem_get_pp_ref_count_ref(netmem), nr);
+	page_pool_fragment_nmdesc(netmem_to_nmdesc(netmem), nr);
 }
 
 /**
@@ -259,12 +264,12 @@ static inline void page_pool_fragment_netmem(netmem_ref netmem, long nr)
  */
 static inline void page_pool_fragment_page(struct page *page, long nr)
 {
-	page_pool_fragment_netmem(page_to_netmem(page), nr);
+	page_pool_fragment_nmdesc(pp_page_to_nmdesc(page), nr);
 }
 
-static inline long page_pool_unref_netmem(netmem_ref netmem, long nr)
+static inline long page_pool_unref_nmdesc(struct netmem_desc *desc, long nr)
 {
-	atomic_long_t *pp_ref_count = netmem_get_pp_ref_count_ref(netmem);
+	atomic_long_t *pp_ref_count = &desc->pp_ref_count;
 	long ret;
 
 	/* If nr == pp_ref_count then we have cleared all remaining
@@ -307,19 +312,29 @@ static inline long page_pool_unref_netmem(netmem_ref netmem, long nr)
 	return ret;
 }
 
+static inline long page_pool_unref_netmem(netmem_ref netmem, long nr)
+{
+	return page_pool_unref_nmdesc(netmem_to_nmdesc(netmem), nr);
+}
+
 static inline long page_pool_unref_page(struct page *page, long nr)
 {
-	return page_pool_unref_netmem(page_to_netmem(page), nr);
+	return page_pool_unref_nmdesc(pp_page_to_nmdesc(page), nr);
+}
+
+static inline void page_pool_ref_nmdesc(struct netmem_desc *desc)
+{
+	atomic_long_inc(&desc->pp_ref_count);
 }
 
 static inline void page_pool_ref_netmem(netmem_ref netmem)
 {
-	atomic_long_inc(netmem_get_pp_ref_count_ref(netmem));
+	page_pool_ref_nmdesc(netmem_to_nmdesc(netmem));
 }
 
 static inline void page_pool_ref_page(struct page *page)
 {
-	page_pool_ref_netmem(page_to_netmem(page));
+	page_pool_ref_nmdesc(pp_page_to_nmdesc(page));
 }
 
 static inline bool page_pool_unref_and_test(netmem_ref netmem)
diff --git a/net/core/devmem.c b/net/core/devmem.c
index 24c591ab38ae..e084dad11506 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -440,14 +440,9 @@ void mp_dmabuf_devmem_destroy(struct page_pool *pool)
 
 bool mp_dmabuf_devmem_release_page(struct page_pool *pool, netmem_ref netmem)
 {
-	long refcount = atomic_long_read(netmem_get_pp_ref_count_ref(netmem));
-
 	if (WARN_ON_ONCE(!netmem_is_net_iov(netmem)))
 		return false;
 
-	if (WARN_ON_ONCE(refcount != 1))
-		return false;
-
 	page_pool_clear_pp_info(netmem);
 
 	net_devmem_free_dmabuf(netmem_to_net_iov(netmem));
-- 
2.49.0

[RFC net-next v1 6/6] io_uring/zcrx: avoid netmem casts with nmdesc

[Pavel Begunkov]

There is a bunch of hot path places where zcrx casts a net_iov to a
netmem just to pass it to a generic helper, which will immediately
remove NET_IOV from it. It's messy, and compilers can't completely
optimise it. Use newly introduced netmem_desc based helpers to avoid the
overhead.

Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
---
 io_uring/zcrx.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/io_uring/zcrx.c b/io_uring/zcrx.c
index e5ff49f3425e..9c733a490122 100644
--- a/io_uring/zcrx.c
+++ b/io_uring/zcrx.c
@@ -301,7 +301,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
@@ -752,7 +752,6 @@ static void io_zcrx_ring_refill(struct page_pool *pp,
 {
 	unsigned int mask = ifq->rq_entries - 1;
 	unsigned int entries;
-	netmem_ref netmem;
 
 	spin_lock_bh(&ifq->rq_lock);
 
@@ -784,8 +783,7 @@ static void io_zcrx_ring_refill(struct page_pool *pp,
 		if (!io_zcrx_put_niov_uref(niov))
 			continue;
 
-		netmem = net_iov_to_netmem(niov);
-		if (page_pool_unref_netmem(netmem, 1) != 0)
+		if (page_pool_unref_nmdesc(&niov->desc, 1) != 0)
 			continue;
 
 		if (unlikely(niov->pp != pp)) {
@@ -794,7 +792,7 @@ static void io_zcrx_ring_refill(struct page_pool *pp,
 		}
 
 		io_zcrx_sync_for_device(pp, niov);
-		net_mp_netmem_place_in_cache(pp, netmem);
+		net_mp_netmem_place_in_cache(pp, net_iov_to_netmem(niov));
 	} while (--entries);
 
 	smp_store_release(&ifq->rq_ring->head, ifq->cached_rq_head);
@@ -950,7 +948,7 @@ static struct net_iov *io_zcrx_alloc_fallback(struct io_zcrx_area *area)
 	spin_unlock_bh(&area->freelist_lock);
 
 	if (niov)
-		page_pool_fragment_netmem(net_iov_to_netmem(niov), 1);
+		page_pool_fragment_nmdesc(&niov->desc, 1);
 	return niov;
 }
 
@@ -1070,7 +1068,7 @@ static int io_zcrx_recv_frag(struct io_kiocb *req, struct io_zcrx_ifq *ifq,
 	 * Prevent it from being recycled while user is accessing it.
 	 * It has to be done before grabbing a user reference.
 	 */
-	page_pool_ref_netmem(net_iov_to_netmem(niov));
+	page_pool_ref_nmdesc(&niov->desc);
 	io_zcrx_get_niov_uref(niov);
 	return len;
 }
-- 
2.49.0

Re: [RFC net-next v1 1/6] net: move pp_page_to_nmdesc()

[Mina Almasry]

On Mon, Aug 11, 2025 at 9:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> A preparation patch moving pp_page_to_nmdesc() up in the header file and
> no other changes otherwise. It reduces cluttering from the following
> patch.
>
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>


Reviewed-by: Mina Almasry <almasrymina@google.com>

-- 
Thanks,
Mina

Re: [RFC net-next v1 3/6] net: page_pool: remove page_pool_set_dma_addr()

[Mina Almasry]

On Mon, Aug 11, 2025 at 9:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> page_pool_set_dma_addr() is not used, kill it.
>
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>

Reviewed-by: Mina Almasry <almasrymina@google.com>

-- 
Thanks,
Mina

Re: [RFC net-next v1 4/6] net: convert page pool dma helpers to netmem_desc

[Mina Almasry]

On Mon, Aug 11, 2025 at 9:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> struct netmem_desc has a clearly defined field that keeps dma_addr. Use
> the new type in netmem and page_pool functions and get rid of a bunch of
> now unnecessary accessor helpers.
>
> While doing so, extract a helper for getting a dma address out of a
> netmem desc, which can be used to optimise paths that already know the
> underlying netmem type like memory providers.
>
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  include/net/netmem.h            |  5 -----
>  include/net/page_pool/helpers.h | 12 ++++++++++--
>  net/core/netmem_priv.h          |  6 ------
>  net/core/page_pool_priv.h       |  9 +++++----
>  4 files changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index d08797e40a7c..ca6d5d151acc 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -389,11 +389,6 @@ static inline bool netmem_is_pfmemalloc(netmem_ref netmem)
>         return page_is_pfmemalloc(netmem_to_page(netmem));
>  }
>
> -static inline unsigned long netmem_get_dma_addr(netmem_ref netmem)
> -{
> -       return netmem_to_nmdesc(netmem)->dma_addr;
> -}
> -
>  void get_netmem(netmem_ref netmem);
>  void put_netmem(netmem_ref netmem);
>
> diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
> index db180626be06..a9774d582933 100644
> --- a/include/net/page_pool/helpers.h
> +++ b/include/net/page_pool/helpers.h
> @@ -425,9 +425,10 @@ 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(const struct netmem_desc *nmdesc)
>  {
> -       dma_addr_t ret = netmem_get_dma_addr(netmem);
> +       dma_addr_t ret = nmdesc->dma_addr;
>
>         if (PAGE_POOL_32BIT_ARCH_WITH_64BIT_DMA)
>                 ret <<= PAGE_SHIFT;
> @@ -435,6 +436,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)
> +{
> +       const struct netmem_desc *desc = netmem_to_nmdesc(netmem);
> +
> +       return page_pool_get_dma_addr_nmdesc(desc);
> +}
> +

nit: this wrapper feels very unnecessary. The _nmdesc variant has only
one call site from page_pool_get_dma_addr_netmem. I'd really prefer we
don't have the _nmdesc variant.

But, minor issue, so reviewed-by anyway.

Reviewed-by: Mina Almasry <almasrymina@google.com>

Re: [RFC net-next v1 5/6] net: page_pool: convert refcounting helpers to nmdesc

[Mina Almasry]

On Mon, Aug 11, 2025 at 9:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> Use netmem descriptors for the basic buffer refcounting helpers and use
> them to implement all other variants. This way netmem type aware helpers
> can avoid intermediate netmem casting and bit masking/unmasking.
>
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>  include/net/netmem.h            |  5 -----
>  include/net/page_pool/helpers.h | 29 ++++++++++++++++++++++-------
>  net/core/devmem.c               |  5 -----
>  3 files changed, 22 insertions(+), 17 deletions(-)
>
> diff --git a/include/net/netmem.h b/include/net/netmem.h
> index ca6d5d151acc..7b5f1427f272 100644
> --- a/include/net/netmem.h
> +++ b/include/net/netmem.h
> @@ -324,11 +324,6 @@ static inline struct page_pool *netmem_get_pp(netmem_ref netmem)
>         return netmem_to_nmdesc(netmem)->pp;
>  }
>
> -static inline atomic_long_t *netmem_get_pp_ref_count_ref(netmem_ref netmem)
> -{
> -       return &netmem_to_nmdesc(netmem)->pp_ref_count;
> -}
> -
>  static inline bool netmem_is_pref_nid(netmem_ref netmem, int pref_nid)
>  {
>         /* NUMA node preference only makes sense if we're allocating
> diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
> index a9774d582933..bc54040186d9 100644
> --- a/include/net/page_pool/helpers.h
> +++ b/include/net/page_pool/helpers.h
> @@ -234,9 +234,14 @@ page_pool_get_dma_dir(const struct page_pool *pool)
>         return pool->p.dma_dir;
>  }
>
> +static inline void page_pool_fragment_nmdesc(struct netmem_desc *desc, long nr)
> +{
> +       atomic_long_set(&desc->pp_ref_count, nr);
> +}
> +
>  static inline void page_pool_fragment_netmem(netmem_ref netmem, long nr)
>  {
> -       atomic_long_set(netmem_get_pp_ref_count_ref(netmem), nr);
> +       page_pool_fragment_nmdesc(netmem_to_nmdesc(netmem), nr);
>  }
>
>  /**
> @@ -259,12 +264,12 @@ static inline void page_pool_fragment_netmem(netmem_ref netmem, long nr)
>   */
>  static inline void page_pool_fragment_page(struct page *page, long nr)
>  {
> -       page_pool_fragment_netmem(page_to_netmem(page), nr);
> +       page_pool_fragment_nmdesc(pp_page_to_nmdesc(page), nr);
>  }
>
> -static inline long page_pool_unref_netmem(netmem_ref netmem, long nr)
> +static inline long page_pool_unref_nmdesc(struct netmem_desc *desc, long nr)
>  {
> -       atomic_long_t *pp_ref_count = netmem_get_pp_ref_count_ref(netmem);
> +       atomic_long_t *pp_ref_count = &desc->pp_ref_count;

nit: I think we can also kill the pp_ref_count local var and use
desc->pp_ref_count directly.

>         long ret;
>
>         /* If nr == pp_ref_count then we have cleared all remaining
> @@ -307,19 +312,29 @@ static inline long page_pool_unref_netmem(netmem_ref netmem, long nr)
>         return ret;
>  }
>
> +static inline long page_pool_unref_netmem(netmem_ref netmem, long nr)
> +{
> +       return page_pool_unref_nmdesc(netmem_to_nmdesc(netmem), nr);
> +}
> +
>  static inline long page_pool_unref_page(struct page *page, long nr)
>  {
> -       return page_pool_unref_netmem(page_to_netmem(page), nr);
> +       return page_pool_unref_nmdesc(pp_page_to_nmdesc(page), nr);
> +}
> +
> +static inline void page_pool_ref_nmdesc(struct netmem_desc *desc)
> +{
> +       atomic_long_inc(&desc->pp_ref_count);
>  }
>
>  static inline void page_pool_ref_netmem(netmem_ref netmem)
>  {
> -       atomic_long_inc(netmem_get_pp_ref_count_ref(netmem));
> +       page_pool_ref_nmdesc(netmem_to_nmdesc(netmem));
>  }
>
>  static inline void page_pool_ref_page(struct page *page)
>  {
> -       page_pool_ref_netmem(page_to_netmem(page));
> +       page_pool_ref_nmdesc(pp_page_to_nmdesc(page));
>  }
>
>  static inline bool page_pool_unref_and_test(netmem_ref netmem)
> diff --git a/net/core/devmem.c b/net/core/devmem.c
> index 24c591ab38ae..e084dad11506 100644
> --- a/net/core/devmem.c
> +++ b/net/core/devmem.c
> @@ -440,14 +440,9 @@ void mp_dmabuf_devmem_destroy(struct page_pool *pool)
>
>  bool mp_dmabuf_devmem_release_page(struct page_pool *pool, netmem_ref netmem)
>  {
> -       long refcount = atomic_long_read(netmem_get_pp_ref_count_ref(netmem));
> -
>         if (WARN_ON_ONCE(!netmem_is_net_iov(netmem)))
>                 return false;
>
> -       if (WARN_ON_ONCE(refcount != 1))
> -               return false;
> -

Rest of the patch looks good to me, but this comes across as a
completely unrelated clean up/change or something? Lets keep the
WARN_ON_ONCE?

-- 
Thanks,
Mina

Re: [RFC net-next v1 6/6] io_uring/zcrx: avoid netmem casts with nmdesc

[Mina Almasry]

On Mon, Aug 11, 2025 at 9:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> There is a bunch of hot path places where zcrx casts a net_iov to a
> netmem just to pass it to a generic helper, which will immediately
> remove NET_IOV from it. It's messy, and compilers can't completely
> optimise it. Use newly introduced netmem_desc based helpers to avoid the
> overhead.
>
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>

Great cleanup actually. I didn't realize how much of a pain the casts were.

Reviewed-by: Mina Almasry <almasrymina@google.com>

-- 
Thanks,
Mina

Re: [RFC net-next v1 4/6] net: convert page pool dma helpers to netmem_desc

[Pavel Begunkov]

On 8/13/25 01:05, Mina Almasry wrote:
...>> diff --git a/include/net/page_pool/helpers.h b/include/net/page_pool/helpers.h
>> index db180626be06..a9774d582933 100644
>> --- a/include/net/page_pool/helpers.h
>> +++ b/include/net/page_pool/helpers.h
...>> +static inline dma_addr_t page_pool_get_dma_addr_netmem(netmem_ref netmem)
>> +{
>> +       const struct netmem_desc *desc = netmem_to_nmdesc(netmem);
>> +
>> +       return page_pool_get_dma_addr_nmdesc(desc);
>> +}
>> +
> 
> nit: this wrapper feels very unnecessary. The _nmdesc variant has only
> one call site from page_pool_get_dma_addr_netmem. I'd really prefer we
> don't have the _nmdesc variant.

It's reused for zcrx in Patch 6. If I want a cast there optimised,
it's either that, or some new get_dma_niov helper, which would still
need to be expressed through sth like page_pool_get_dma_addr_nmdesc()
to avoid duplication.

-- 
Pavel Begunkov

Re: [RFC net-next v1 5/6] net: page_pool: convert refcounting helpers to nmdesc

[Pavel Begunkov]

On 8/13/25 01:14, Mina Almasry wrote:
> On Mon, Aug 11, 2025 at 9:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
...>> -static inline long page_pool_unref_netmem(netmem_ref netmem, long nr)
>> +static inline long page_pool_unref_nmdesc(struct netmem_desc *desc, long nr)
>>   {
>> -       atomic_long_t *pp_ref_count = netmem_get_pp_ref_count_ref(netmem);
>> +       atomic_long_t *pp_ref_count = &desc->pp_ref_count;
> 
> nit: I think we can also kill the pp_ref_count local var and use
> desc->pp_ref_count directly.

I stopped there to save the churn, I'd rather have it on top and outside
of cross tree branches. But I agree in general, and there is more that
we can do as well.

...>>   static inline bool page_pool_unref_and_test(netmem_ref netmem)
>> diff --git a/net/core/devmem.c b/net/core/devmem.c
>> index 24c591ab38ae..e084dad11506 100644
>> --- a/net/core/devmem.c
>> +++ b/net/core/devmem.c
>> @@ -440,14 +440,9 @@ void mp_dmabuf_devmem_destroy(struct page_pool *pool)
>>
>>   bool mp_dmabuf_devmem_release_page(struct page_pool *pool, netmem_ref netmem)
>>   {
>> -       long refcount = atomic_long_read(netmem_get_pp_ref_count_ref(netmem));
>> -
>>          if (WARN_ON_ONCE(!netmem_is_net_iov(netmem)))
>>                  return false;
>>
>> -       if (WARN_ON_ONCE(refcount != 1))
>> -               return false;
>> -
> 
> Rest of the patch looks good to me, but this comes across as a
> completely unrelated clean up/change or something? Lets keep the
> WARN_ON_ONCE?
I was killing netmem_get_pp_ref_count_ref(), which is why it's here.
It checks an assumption that's guaranteed by page pools and shared
with non-mp pools, so not like devmem needs it, and it'd not catch
any recycling problems either. Regardless, I can leave the warning.

-- 
Pavel Begunkov

Re: [RFC net-next v1 5/6] net: page_pool: convert refcounting helpers to nmdesc

[Mina Almasry]

On Wed, Aug 13, 2025 at 2:10 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>
> On 8/13/25 01:14, Mina Almasry wrote:
> > On Mon, Aug 11, 2025 at 9:28 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
> ...>> -static inline long page_pool_unref_netmem(netmem_ref netmem, long nr)
> >> +static inline long page_pool_unref_nmdesc(struct netmem_desc *desc, long nr)
> >>   {
> >> -       atomic_long_t *pp_ref_count = netmem_get_pp_ref_count_ref(netmem);
> >> +       atomic_long_t *pp_ref_count = &desc->pp_ref_count;
> >
> > nit: I think we can also kill the pp_ref_count local var and use
> > desc->pp_ref_count directly.
>
> I stopped there to save the churn, I'd rather have it on top and outside
> of cross tree branches. But I agree in general, and there is more that
> we can do as well.
>
> ...>>   static inline bool page_pool_unref_and_test(netmem_ref netmem)
> >> diff --git a/net/core/devmem.c b/net/core/devmem.c
> >> index 24c591ab38ae..e084dad11506 100644
> >> --- a/net/core/devmem.c
> >> +++ b/net/core/devmem.c
> >> @@ -440,14 +440,9 @@ void mp_dmabuf_devmem_destroy(struct page_pool *pool)
> >>
> >>   bool mp_dmabuf_devmem_release_page(struct page_pool *pool, netmem_ref netmem)
> >>   {
> >> -       long refcount = atomic_long_read(netmem_get_pp_ref_count_ref(netmem));
> >> -
> >>          if (WARN_ON_ONCE(!netmem_is_net_iov(netmem)))
> >>                  return false;
> >>
> >> -       if (WARN_ON_ONCE(refcount != 1))
> >> -               return false;
> >> -
> >
> > Rest of the patch looks good to me, but this comes across as a
> > completely unrelated clean up/change or something? Lets keep the
> > WARN_ON_ONCE?
> I was killing netmem_get_pp_ref_count_ref(), which is why it's here.
> It checks an assumption that's guaranteed by page pools and shared
> with non-mp pools, so not like devmem needs it, and it'd not catch
> any recycling problems either. Regardless, I can leave the warning.
>

Ack. I also agree the WARN_ON_ONCE is not necessary, even the one
above it for the net_iov check is not necessary.

But since devmem was the first memory provider I'm paranoid that I got
something wrong in the general memory provider infra or in the devmem
implementation specifically; I think some paranoid WARN_ON_ONCEs are
justified, maybe. I'd rather debug the warning firing rather than a
very subtle refcounting issue or provider mixup or something at a
later point. I'm still surprised there aren't many bug reports about
any memory providers. They probably aren't used much in production
yet.

I think after 2025 or 2026 LTS it's probably time to clean up these
unnecessary WARN_ONs, but until then, let's keep them in if you don't
mind.

-- 
Thanks,
Mina

Re: [RFC net-next v1 5/6] net: page_pool: convert refcounting helpers to nmdesc

[Pavel Begunkov]

On 8/13/25 17:55, Mina Almasry wrote:
> On Wed, Aug 13, 2025 at 2:10 AM Pavel Begunkov <asml.silence@gmail.com> wrote:
>> ...>>   static inline bool page_pool_unref_and_test(netmem_ref netmem)
>>>> diff --git a/net/core/devmem.c b/net/core/devmem.c
>>>> index 24c591ab38ae..e084dad11506 100644
>>>> --- a/net/core/devmem.c
>>>> +++ b/net/core/devmem.c
>>>> @@ -440,14 +440,9 @@ void mp_dmabuf_devmem_destroy(struct page_pool *pool)
>>>>
>>>>    bool mp_dmabuf_devmem_release_page(struct page_pool *pool, netmem_ref netmem)
>>>>    {
>>>> -       long refcount = atomic_long_read(netmem_get_pp_ref_count_ref(netmem));
>>>> -
>>>>           if (WARN_ON_ONCE(!netmem_is_net_iov(netmem)))
>>>>                   return false;
>>>>
>>>> -       if (WARN_ON_ONCE(refcount != 1))
>>>> -               return false;
>>>> -
>>>
>>> Rest of the patch looks good to me, but this comes across as a
>>> completely unrelated clean up/change or something? Lets keep the
>>> WARN_ON_ONCE?
>> I was killing netmem_get_pp_ref_count_ref(), which is why it's here.
>> It checks an assumption that's guaranteed by page pools and shared
>> with non-mp pools, so not like devmem needs it, and it'd not catch
>> any recycling problems either. Regardless, I can leave the warning.
>>
> 
> Ack. I also agree the WARN_ON_ONCE is not necessary, even the one
> above it for the net_iov check is not necessary.
> 
> But since devmem was the first memory provider I'm paranoid that I got
> something wrong in the general memory provider infra or in the devmem
> implementation specifically; I think some paranoid WARN_ON_ONCEs are
> justified, maybe. I'd rather debug the warning firing rather than a
> very subtle refcounting issue or provider mixup or something at a
> later point. I'm still surprised there aren't many bug reports about
> any memory providers. They probably aren't used much in production
> yet.
> 
> I think after 2025 or 2026 LTS it's probably time to clean up these
> unnecessary WARN_ONs, but until then, let's keep them in if you don't
> mind.

No problem, thanks for the review!

-- 
Pavel Begunkov