Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] net: devmem: remove gen_pool from dma-buf allocations
@ 2026-08-31 18:35 Stanislav Fomichev
  2026-08-31 18:35 ` [PATCH net-next 1/2] net: devmem: replace gen_pool with freelist Stanislav Fomichev
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Stanislav Fomichev @ 2026-08-31 18:35 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, horms, sdf, bobbyeshleman,
	almasrymina, linux-kernel

Replace devmem's gen_pool based fixed-size allocator with a binding-level
freelist similar to the one used by io_uring zero-copy receive.

This is motivated by allocation latency observed in the NAPI receive path:

  [ 1036.228913]  ? gen_pool_create+0x90/0x90
  [ 1036.228915]  net_devmem_alloc_dmabuf+0x1f/0x60
  [ 1036.228918]  mp_dmabuf_devmem_alloc_netmems+0x17/0x80
  [ 1036.228920]  mlx5e_post_rx_mpwqes+0xdbe/0xdd0
  [ 1036.228926]  mlx5e_napi_poll+0x113/0x830
  [ 1036.228928]  ? sched_clock+0x5/0x10
  [ 1036.228931]  ? wake_up_process+0x778/0x14b0
  [ 1036.228933]  net_rx_action+0x15d/0x570
  [ 1036.228934]  ? update_rq_clock+0x31/0x240
  [ 1036.228937]  ? __napi_schedule+0x55/0xa0
  [ 1036.228938]  ? mlx5_eq_comp_int+0x137/0x230
  [ 1036.228940]  ? atomic_notifier_call_chain+0x36/0x90
  [ 1036.228943]  ? sched_clock+0x5/0x10
  [ 1036.228944]  ? sched_clock_cpu+0xc/0x170
  [ 1036.228947]  irq_exit_rcu+0x12b/0x370
  [ 1036.228950]  common_interrupt+0x85/0x90

udmabuf can create a very large number of SG entries. In the worst case,
devmem ends up adding one gen_pool chunk for each net_iov allocation
unit backed by those entries. The gen_pool allocation path then has to
traverse a linked list that can become too long for this hot path.

Patch 1 removes the gen_pool and replaces it with a simple freelist of
net_iov pointers protected by the same spin_lock_bh() pattern used by
io_uring zcrx. Patch 2 removes the now-unnecessary chunk owner wrapper by
embedding the net_iov_area directly in the dma-buf binding.

Stanislav Fomichev (2):
  net: devmem: replace gen_pool with freelist
  net: devmem: embed net_iov_area in binding

 net/Kconfig       |   1 -
 net/core/devmem.c | 158 ++++++++++++++++++----------------------------
 net/core/devmem.h |  38 ++++-------
 3 files changed, 72 insertions(+), 125 deletions(-)

-- 
2.53.0-Meta


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH net-next 1/2] net: devmem: replace gen_pool with freelist
  2026-08-31 18:35 [PATCH net-next 0/2] net: devmem: remove gen_pool from dma-buf allocations Stanislav Fomichev
@ 2026-08-31 18:35 ` Stanislav Fomichev
  2026-09-04  2:22   ` Jakub Kicinski
  2026-08-31 18:35 ` [PATCH net-next 2/2] net: devmem: embed net_iov_area in binding Stanislav Fomichev
  2026-08-31 19:16 ` [PATCH net-next 0/2] net: devmem: remove gen_pool from dma-buf allocations Mina Almasry
  2 siblings, 1 reply; 6+ messages in thread
From: Stanislav Fomichev @ 2026-08-31 18:35 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, horms, sdf, bobbyeshleman,
	almasrymina, linux-kernel

devmem only needs fixed-size net_iov allocations for each dma-buf binding.
The gen_pool tracks the same free set indirectly through DMA addresses,
which makes devmem depend on the generic allocator even though the users
are fixed-size net_iov chunks.

Mirror the io_uring zcrx model more closely by keeping a binding-level
freelist protected by spin_lock_bh(). Use a single net_iov_area owner for
the binding, populate each net_iov's DMA address while walking the SG
table, and check at teardown that all net_iovs have returned to the
freelist.

Drop the NET_DEVMEM select of GENERIC_ALLOCATOR now that devmem no longer
calls gen_pool APIs.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 net/Kconfig       |   1 -
 net/core/devmem.c | 172 +++++++++++++++++++++-------------------------
 net/core/devmem.h |  16 ++---
 3 files changed, 85 insertions(+), 104 deletions(-)

diff --git a/net/Kconfig b/net/Kconfig
index e38477393551..76ab44aa439a 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -68,7 +68,6 @@ config SKB_EXTENSIONS
 
 config NET_DEVMEM
 	def_bool y
-	select GENERIC_ALLOCATOR
 	depends on DMA_SHARED_BUFFER
 	depends on PAGE_POOL
 
diff --git a/net/core/devmem.c b/net/core/devmem.c
index f4d60654ce7f..34b3157677e1 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -8,7 +8,6 @@
  */
 
 #include <linux/dma-buf.h>
-#include <linux/genalloc.h>
 #include <linux/mm.h>
 #include <linux/netdevice.h>
 #include <linux/types.h>
@@ -30,23 +29,13 @@ static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1);
 
 static const struct memory_provider_ops dmabuf_devmem_ops;
 
-static void net_devmem_dmabuf_free_chunk_owner(struct gen_pool *genpool,
-					       struct gen_pool_chunk *chunk,
-					       void *not_used)
+static void
+net_devmem_dmabuf_free_chunk_owner(struct dmabuf_genpool_chunk_owner *owner)
 {
-	struct dmabuf_genpool_chunk_owner *owner = chunk->owner;
-
-	kvfree(owner->area.niovs);
-	kfree(owner);
-}
-
-static dma_addr_t net_devmem_get_dma_addr(const struct net_iov *niov)
-{
-	struct dmabuf_genpool_chunk_owner *owner;
-
-	owner = net_devmem_iov_to_chunk_owner(niov);
-	return owner->base_dma_addr +
-	       ((dma_addr_t)net_iov_idx(niov) << owner->binding->niov_shift);
+	if (owner) {
+		kvfree(owner->area.niovs);
+		kfree(owner);
+	}
 }
 
 static void net_devmem_dmabuf_binding_release(struct percpu_ref *ref)
@@ -62,24 +51,18 @@ void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
 {
 	struct net_devmem_dmabuf_binding *binding = container_of(wq, typeof(*binding), unbind_w);
 
-	size_t size, avail;
-
-	gen_pool_for_each_chunk(binding->chunk_pool,
-				net_devmem_dmabuf_free_chunk_owner, NULL);
-
-	size = gen_pool_size(binding->chunk_pool);
-	avail = gen_pool_avail(binding->chunk_pool);
-
-	if (!WARN(size != avail, "can't destroy genpool. size=%zu, avail=%zu",
-		  size, avail))
-		gen_pool_destroy(binding->chunk_pool);
+	WARN(binding->free_count != binding->total_niovs,
+	     "can't destroy dmabuf binding. total=%zu, free=%zu",
+	     binding->total_niovs, binding->free_count);
 
+	net_devmem_dmabuf_free_chunk_owner(binding->chunk_owner);
 	dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt,
 					  binding->direction);
 	dma_buf_detach(binding->dmabuf, binding->attachment);
 	dma_buf_put(binding->dmabuf);
 	xa_destroy(&binding->bound_rxqs);
 	percpu_ref_exit(&binding->ref);
+	kvfree(binding->freelist);
 	kvfree(binding->tx_vec);
 	kfree(binding);
 }
@@ -87,21 +70,16 @@ void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
 struct net_iov *
 net_devmem_alloc_dmabuf(struct net_devmem_dmabuf_binding *binding)
 {
-	struct dmabuf_genpool_chunk_owner *owner;
-	unsigned long dma_addr;
 	struct net_iov *niov;
-	ssize_t offset;
-	ssize_t index;
-
-	dma_addr = gen_pool_alloc_owner(binding->chunk_pool,
-					1UL << binding->niov_shift,
-					(void **)&owner);
-	if (!dma_addr)
+	spin_lock_bh(&binding->freelist_lock);
+	if (unlikely(!binding->free_count)) {
+		spin_unlock_bh(&binding->freelist_lock);
 		return NULL;
+	}
 
-	offset = dma_addr - owner->base_dma_addr;
-	index = offset >> binding->niov_shift;
-	niov = &owner->area.niovs[index];
+	niov = binding->freelist[--binding->free_count];
+	binding->freelist[binding->free_count] = NULL;
+	spin_unlock_bh(&binding->freelist_lock);
 
 	niov->desc.pp_magic = 0;
 	niov->desc.pp = NULL;
@@ -113,14 +91,15 @@ net_devmem_alloc_dmabuf(struct net_devmem_dmabuf_binding *binding)
 void net_devmem_free_dmabuf(struct net_iov *niov)
 {
 	struct net_devmem_dmabuf_binding *binding = net_devmem_iov_binding(niov);
-	unsigned long dma_addr = net_devmem_get_dma_addr(niov);
-	size_t niov_size = 1UL << binding->niov_shift;
 
-	if (WARN_ON(!gen_pool_has_addr(binding->chunk_pool, dma_addr,
-				       niov_size)))
+	spin_lock_bh(&binding->freelist_lock);
+	if (WARN_ON_ONCE(binding->free_count >= binding->total_niovs)) {
+		spin_unlock_bh(&binding->freelist_lock);
 		return;
+	}
 
-	gen_pool_free(binding->chunk_pool, dma_addr, niov_size);
+	binding->freelist[binding->free_count++] = niov;
+	spin_unlock_bh(&binding->freelist_lock);
 }
 
 void net_devmem_unbind_dmabuf(struct net_devmem_dmabuf_binding *binding)
@@ -198,8 +177,11 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 	static u32 id_alloc_next;
 	struct scatterlist *sg;
 	struct dma_buf *dmabuf;
-	unsigned int sg_idx, i;
-	unsigned long virtual;
+	struct dmabuf_genpool_chunk_owner *owner;
+	unsigned int sg_idx;
+	size_t total_niovs;
+	size_t niov_idx;
+	size_t i;
 	int err;
 
 	if (!dma_dev) {
@@ -230,6 +212,7 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 		goto err_free_binding;
 
 	mutex_init(&binding->lock);
+	spin_lock_init(&binding->freelist_lock);
 
 	binding->dmabuf = dmabuf;
 	binding->direction = direction;
@@ -262,20 +245,10 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 			goto err_unmap;
 		}
 	}
-
-	binding->chunk_pool = gen_pool_create(niov_shift,
-					      dev_to_node(&dev->dev));
-	if (!binding->chunk_pool) {
-		err = -ENOMEM;
-		goto err_tx_vec;
-	}
-
-	virtual = 0;
+	total_niovs = 0;
 	for_each_sgtable_dma_sg(binding->sgt, sg, sg_idx) {
 		dma_addr_t dma_addr = sg_dma_address(sg);
-		struct dmabuf_genpool_chunk_owner *owner;
 		size_t len = sg_dma_len(sg);
-		struct net_iov *niov;
 
 		if (!IS_ALIGNED(dma_addr, niov_size) ||
 		    !IS_ALIGNED(len, niov_size)) {
@@ -283,63 +256,74 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 			NL_SET_ERR_MSG_FMT(extack,
 					   "dmabuf sg entry (addr=%pad, len=%zu) not aligned to niov size %zu",
 					   &dma_addr, len, niov_size);
-			goto err_free_chunks;
+			goto err_tx_vec;
 		}
 
-		owner = kzalloc_node(sizeof(*owner), GFP_KERNEL,
-				     dev_to_node(&dev->dev));
-		if (!owner) {
-			err = -ENOMEM;
-			goto err_free_chunks;
-		}
+		total_niovs += len >> niov_shift;
+	}
 
-		owner->area.base_virtual = virtual;
-		owner->base_dma_addr = dma_addr;
-		owner->area.num_niovs = len >> niov_shift;
-		owner->binding = binding;
+	binding->freelist = kvmalloc_array(total_niovs,
+					   sizeof(binding->freelist[0]),
+					   GFP_KERNEL);
+	if (!binding->freelist) {
+		err = -ENOMEM;
+		goto err_tx_vec;
+	}
+	binding->total_niovs = total_niovs;
 
-		err = gen_pool_add_owner(binding->chunk_pool, dma_addr,
-					 dma_addr, len, dev_to_node(&dev->dev),
-					 owner);
-		if (err) {
-			kfree(owner);
-			err = -EINVAL;
-			goto err_free_chunks;
-		}
+	owner = kzalloc_node(sizeof(*owner), GFP_KERNEL,
+			     dev_to_node(&dev->dev));
+	if (!owner) {
+		err = -ENOMEM;
+		goto err_free_freelist;
+	}
 
-		owner->area.niovs = kvmalloc_objs(*owner->area.niovs,
-						  owner->area.num_niovs);
-		if (!owner->area.niovs) {
-			err = -ENOMEM;
-			goto err_free_chunks;
-		}
+	owner->area.num_niovs = total_niovs;
+	owner->binding = binding;
+	owner->area.niovs = kvmalloc_objs(*owner->area.niovs,
+					  owner->area.num_niovs);
+	if (!owner->area.niovs) {
+		err = -ENOMEM;
+		goto err_free_owner;
+	}
+	binding->chunk_owner = owner;
+
+	niov_idx = 0;
+	for_each_sgtable_dma_sg(binding->sgt, sg, sg_idx) {
+		dma_addr_t dma_addr = sg_dma_address(sg);
+		size_t len = sg_dma_len(sg);
+		struct net_iov *niov;
+		size_t nr_niovs = len >> niov_shift;
 
-		for (i = 0; i < owner->area.num_niovs; i++) {
-			niov = &owner->area.niovs[i];
+		for (i = 0; i < nr_niovs; i++, niov_idx++) {
+			niov = &owner->area.niovs[niov_idx];
 			net_iov_init(niov, &owner->area, NET_IOV_DMABUF);
 			page_pool_set_dma_addr_netmem(net_iov_to_netmem(niov),
-						      net_devmem_get_dma_addr(niov));
+						      dma_addr);
 			if (direction == DMA_TO_DEVICE)
-				binding->tx_vec[owner->area.base_virtual / PAGE_SIZE + i] = niov;
+				binding->tx_vec[niov_idx] = niov;
+			binding->freelist[binding->free_count++] = niov;
+			dma_addr += niov_size;
 		}
-
-		virtual += len;
 	}
 
 	err = xa_alloc_cyclic(&net_devmem_dmabuf_bindings, &binding->id,
 			      binding, xa_limit_32b, &id_alloc_next,
 			      GFP_KERNEL);
 	if (err < 0)
-		goto err_free_chunks;
+		goto err_free_chunk_owner;
 
 	list_add(&binding->list, &priv->bindings);
 
 	return binding;
 
-err_free_chunks:
-	gen_pool_for_each_chunk(binding->chunk_pool,
-				net_devmem_dmabuf_free_chunk_owner, NULL);
-	gen_pool_destroy(binding->chunk_pool);
+err_free_chunk_owner:
+	net_devmem_dmabuf_free_chunk_owner(binding->chunk_owner);
+	goto err_free_freelist;
+err_free_owner:
+	kfree(owner);
+err_free_freelist:
+	kvfree(binding->freelist);
 err_tx_vec:
 	kvfree(binding->tx_vec);
 err_unmap:
diff --git a/net/core/devmem.h b/net/core/devmem.h
index 4a293a7d1149..a5ee2d8d9169 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -14,6 +14,7 @@
 #include <net/netdev_netlink.h>
 
 struct netlink_ext_ack;
+struct dmabuf_genpool_chunk_owner;
 
 struct net_devmem_dmabuf_binding {
 	struct dma_buf *dmabuf;
@@ -26,7 +27,7 @@ struct net_devmem_dmabuf_binding {
 	 * dereferenced.
 	 */
 	void *vdev;
-	struct gen_pool *chunk_pool;
+	struct dmabuf_genpool_chunk_owner *chunk_owner;
 	/* Protect dev */
 	struct mutex lock;
 
@@ -57,6 +58,11 @@ struct net_devmem_dmabuf_binding {
 	/* rxq's this binding is active on. */
 	struct xarray bound_rxqs;
 
+	spinlock_t freelist_lock ____cacheline_aligned_in_smp;
+	size_t free_count;
+	size_t total_niovs;
+	struct net_iov **freelist;
+
 	/* ID of this binding. Globally unique to all bindings currently
 	 * active.
 	 */
@@ -77,17 +83,9 @@ struct net_devmem_dmabuf_binding {
 };
 
 #if defined(CONFIG_NET_DEVMEM)
-/* Owner of the dma-buf chunks inserted into the gen pool. Each scatterlist
- * entry from the dmabuf is inserted into the genpool as a chunk, and needs
- * this owner struct to keep track of some metadata necessary to create
- * allocations from this chunk.
- */
 struct dmabuf_genpool_chunk_owner {
 	struct net_iov_area area;
 	struct net_devmem_dmabuf_binding *binding;
-
-	/* dma_addr of the start of the chunk.  */
-	dma_addr_t base_dma_addr;
 };
 
 void __net_devmem_dmabuf_binding_free(struct work_struct *wq);
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net-next 2/2] net: devmem: embed net_iov_area in binding
  2026-08-31 18:35 [PATCH net-next 0/2] net: devmem: remove gen_pool from dma-buf allocations Stanislav Fomichev
  2026-08-31 18:35 ` [PATCH net-next 1/2] net: devmem: replace gen_pool with freelist Stanislav Fomichev
@ 2026-08-31 18:35 ` Stanislav Fomichev
  2026-08-31 19:16 ` [PATCH net-next 0/2] net: devmem: remove gen_pool from dma-buf allocations Mina Almasry
  2 siblings, 0 replies; 6+ messages in thread
From: Stanislav Fomichev @ 2026-08-31 18:35 UTC (permalink / raw)
  To: netdev
  Cc: davem, edumazet, kuba, pabeni, horms, sdf, bobbyeshleman,
	almasrymina, linux-kernel

After replacing the gen_pool with a binding-level freelist, devmem no
longer needs a separate chunk owner object. There is only one
net_iov_area for the binding, so store it directly in struct
net_devmem_dmabuf_binding.

Derive the binding from net_iov_owner() with container_of(), matching the
pattern used by io_uring zcrx. This removes the leftover
dmabuf_genpool_chunk_owner wrapper and its allocation/free path.

Signed-off-by: Stanislav Fomichev <sdf@fomichev.me>
---
 net/core/devmem.c | 42 ++++++++++--------------------------------
 net/core/devmem.h | 26 +++++++-------------------
 2 files changed, 17 insertions(+), 51 deletions(-)

diff --git a/net/core/devmem.c b/net/core/devmem.c
index 34b3157677e1..84d6c30516c8 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -29,15 +29,6 @@ static DEFINE_XARRAY_FLAGS(net_devmem_dmabuf_bindings, XA_FLAGS_ALLOC1);
 
 static const struct memory_provider_ops dmabuf_devmem_ops;
 
-static void
-net_devmem_dmabuf_free_chunk_owner(struct dmabuf_genpool_chunk_owner *owner)
-{
-	if (owner) {
-		kvfree(owner->area.niovs);
-		kfree(owner);
-	}
-}
-
 static void net_devmem_dmabuf_binding_release(struct percpu_ref *ref)
 {
 	struct net_devmem_dmabuf_binding *binding =
@@ -55,7 +46,7 @@ void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
 	     "can't destroy dmabuf binding. total=%zu, free=%zu",
 	     binding->total_niovs, binding->free_count);
 
-	net_devmem_dmabuf_free_chunk_owner(binding->chunk_owner);
+	kvfree(binding->area.niovs);
 	dma_buf_unmap_attachment_unlocked(binding->attachment, binding->sgt,
 					  binding->direction);
 	dma_buf_detach(binding->dmabuf, binding->attachment);
@@ -177,7 +168,6 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 	static u32 id_alloc_next;
 	struct scatterlist *sg;
 	struct dma_buf *dmabuf;
-	struct dmabuf_genpool_chunk_owner *owner;
 	unsigned int sg_idx;
 	size_t total_niovs;
 	size_t niov_idx;
@@ -271,23 +261,14 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 	}
 	binding->total_niovs = total_niovs;
 
-	owner = kzalloc_node(sizeof(*owner), GFP_KERNEL,
-			     dev_to_node(&dev->dev));
-	if (!owner) {
+	binding->area.num_niovs = total_niovs;
+	binding->area.niovs = kvmalloc_objs(*binding->area.niovs,
+					    binding->area.num_niovs);
+	if (!binding->area.niovs) {
 		err = -ENOMEM;
 		goto err_free_freelist;
 	}
 
-	owner->area.num_niovs = total_niovs;
-	owner->binding = binding;
-	owner->area.niovs = kvmalloc_objs(*owner->area.niovs,
-					  owner->area.num_niovs);
-	if (!owner->area.niovs) {
-		err = -ENOMEM;
-		goto err_free_owner;
-	}
-	binding->chunk_owner = owner;
-
 	niov_idx = 0;
 	for_each_sgtable_dma_sg(binding->sgt, sg, sg_idx) {
 		dma_addr_t dma_addr = sg_dma_address(sg);
@@ -296,8 +277,8 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 		size_t nr_niovs = len >> niov_shift;
 
 		for (i = 0; i < nr_niovs; i++, niov_idx++) {
-			niov = &owner->area.niovs[niov_idx];
-			net_iov_init(niov, &owner->area, NET_IOV_DMABUF);
+			niov = &binding->area.niovs[niov_idx];
+			net_iov_init(niov, &binding->area, NET_IOV_DMABUF);
 			page_pool_set_dma_addr_netmem(net_iov_to_netmem(niov),
 						      dma_addr);
 			if (direction == DMA_TO_DEVICE)
@@ -311,17 +292,14 @@ net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
 			      binding, xa_limit_32b, &id_alloc_next,
 			      GFP_KERNEL);
 	if (err < 0)
-		goto err_free_chunk_owner;
+		goto err_free_niovs;
 
 	list_add(&binding->list, &priv->bindings);
 
 	return binding;
 
-err_free_chunk_owner:
-	net_devmem_dmabuf_free_chunk_owner(binding->chunk_owner);
-	goto err_free_freelist;
-err_free_owner:
-	kfree(owner);
+err_free_niovs:
+	kvfree(binding->area.niovs);
 err_free_freelist:
 	kvfree(binding->freelist);
 err_tx_vec:
diff --git a/net/core/devmem.h b/net/core/devmem.h
index a5ee2d8d9169..20a3eb90ea7f 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -14,9 +14,9 @@
 #include <net/netdev_netlink.h>
 
 struct netlink_ext_ack;
-struct dmabuf_genpool_chunk_owner;
 
 struct net_devmem_dmabuf_binding {
+	struct net_iov_area area;
 	struct dma_buf *dmabuf;
 	struct dma_buf_attachment *attachment;
 	struct sg_table *sgt;
@@ -27,7 +27,6 @@ struct net_devmem_dmabuf_binding {
 	 * dereferenced.
 	 */
 	void *vdev;
-	struct dmabuf_genpool_chunk_owner *chunk_owner;
 	/* Protect dev */
 	struct mutex lock;
 
@@ -83,11 +82,6 @@ struct net_devmem_dmabuf_binding {
 };
 
 #if defined(CONFIG_NET_DEVMEM)
-struct dmabuf_genpool_chunk_owner {
-	struct net_iov_area area;
-	struct net_devmem_dmabuf_binding *binding;
-};
-
 void __net_devmem_dmabuf_binding_free(struct work_struct *wq);
 struct net_devmem_dmabuf_binding *
 net_devmem_bind_dmabuf(struct net_device *dev, void *vdev,
@@ -102,18 +96,12 @@ int net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
 				    struct net_devmem_dmabuf_binding *binding,
 				    struct netlink_ext_ack *extack);
 
-static inline struct dmabuf_genpool_chunk_owner *
-net_devmem_iov_to_chunk_owner(const struct net_iov *niov)
-{
-	struct net_iov_area *owner = net_iov_owner(niov);
-
-	return container_of(owner, struct dmabuf_genpool_chunk_owner, area);
-}
-
 static inline struct net_devmem_dmabuf_binding *
 net_devmem_iov_binding(const struct net_iov *niov)
 {
-	return net_devmem_iov_to_chunk_owner(niov)->binding;
+	struct net_iov_area *owner = net_iov_owner(niov);
+
+	return container_of(owner, struct net_devmem_dmabuf_binding, area);
 }
 
 static inline u32 net_devmem_iov_binding_id(const struct net_iov *niov)
@@ -123,11 +111,11 @@ static inline u32 net_devmem_iov_binding_id(const struct net_iov *niov)
 
 static inline unsigned long net_iov_virtual_addr(const struct net_iov *niov)
 {
-	struct dmabuf_genpool_chunk_owner *co =
-		net_devmem_iov_to_chunk_owner(niov);
+	struct net_devmem_dmabuf_binding *binding =
+		net_devmem_iov_binding(niov);
 
 	return net_iov_owner(niov)->base_virtual +
-	       ((unsigned long)net_iov_idx(niov) << co->binding->niov_shift);
+	       ((unsigned long)net_iov_idx(niov) << binding->niov_shift);
 }
 
 static inline bool
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next 0/2] net: devmem: remove gen_pool from dma-buf allocations
  2026-08-31 18:35 [PATCH net-next 0/2] net: devmem: remove gen_pool from dma-buf allocations Stanislav Fomichev
  2026-08-31 18:35 ` [PATCH net-next 1/2] net: devmem: replace gen_pool with freelist Stanislav Fomichev
  2026-08-31 18:35 ` [PATCH net-next 2/2] net: devmem: embed net_iov_area in binding Stanislav Fomichev
@ 2026-08-31 19:16 ` Mina Almasry
  2026-09-02 18:36   ` Stanislav Fomichev
  2 siblings, 1 reply; 6+ messages in thread
From: Mina Almasry @ 2026-08-31 19:16 UTC (permalink / raw)
  To: Stanislav Fomichev, Kaifeng Wang
  Cc: netdev, davem, edumazet, kuba, pabeni, horms, sdf, bobbyeshleman,
	linux-kernel

On Mon, Aug 31, 2026 at 11:35 AM Stanislav Fomichev
<sdf.kernel@gmail.com> wrote:
>
> Replace devmem's gen_pool based fixed-size allocator with a binding-level
> freelist similar to the one used by io_uring zero-copy receive.
>
> This is motivated by allocation latency observed in the NAPI receive path:
>
>   [ 1036.228913]  ? gen_pool_create+0x90/0x90
>   [ 1036.228915]  net_devmem_alloc_dmabuf+0x1f/0x60
>   [ 1036.228918]  mp_dmabuf_devmem_alloc_netmems+0x17/0x80
>   [ 1036.228920]  mlx5e_post_rx_mpwqes+0xdbe/0xdd0
>   [ 1036.228926]  mlx5e_napi_poll+0x113/0x830
>   [ 1036.228928]  ? sched_clock+0x5/0x10
>   [ 1036.228931]  ? wake_up_process+0x778/0x14b0
>   [ 1036.228933]  net_rx_action+0x15d/0x570
>   [ 1036.228934]  ? update_rq_clock+0x31/0x240
>   [ 1036.228937]  ? __napi_schedule+0x55/0xa0
>   [ 1036.228938]  ? mlx5_eq_comp_int+0x137/0x230
>   [ 1036.228940]  ? atomic_notifier_call_chain+0x36/0x90
>   [ 1036.228943]  ? sched_clock+0x5/0x10
>   [ 1036.228944]  ? sched_clock_cpu+0xc/0x170
>   [ 1036.228947]  irq_exit_rcu+0x12b/0x370
>   [ 1036.228950]  common_interrupt+0x85/0x90
>
> udmabuf can create a very large number of SG entries. In the worst case,
> devmem ends up adding one gen_pool chunk for each net_iov allocation
> unit backed by those entries. The gen_pool allocation path then has to
> traverse a linked list that can become too long for this hot path.
>
> Patch 1 removes the gen_pool and replaces it with a simple freelist of
> net_iov pointers protected by the same spin_lock_bh() pattern used by
> io_uring zcrx. Patch 2 removes the now-unnecessary chunk owner wrapper by
> embedding the net_iov_area directly in the dma-buf binding.
>

Oh boy, this is going to be a bit tricky.

I ran into this exact horrible perf bug (sorry for it in the first
place), but my solution was different. My solution [1] was to coalesce
the SG entries that are contigious (and they usually are in practice),
and I got 'acceptable' perf after that. Kaifeng is actually working on
cleaning up my hacky patch up to send it upstream now.

Now I don't know which approach is better. Thinking about the pros and
cons of your approach:

+ your approach is much simpler, and removes gen_pool overheads for a
single queue case. It should be (much?) faster for that case.
- your approach adds a lock and allocations from multiple queues in
parallel will contend on this lock. There should be some value of # of
queues N where your approach starts to completely trash. gen_pool is
lockless so I wouldn't expect it to degrade significantly in the
multi-queue case.

The question for me is what the performance is for a real use case
(NCCL all-to-all for example) over a realistic number of shared queues
(it's 4-8 for me). I need that perf data to be honest before judging
this.

The io_uring zcrx comparision is not completely valid. io_uring zcrx
is built from the ground up to be one-buffer-is-bound-to-one-rx-queue,
and devmem tcp is built from the ground up to be
one-buffer-can-be-bound-to-N-rx-queues.

Are you able to get NCCL all-to-all tests for N=4/8 yourself?
Otherwise please wait for me to backport this to my release kernel and
test it. ETA sometime this week, I hope.

But please no merge without real perf data. This has potential to be
great, but is very risky :(


[1] patch:
commit 7bb6d32e21b6e ("net: devmem: coalesce sg chunks before feeding
into gen_pool")
Author: Mina Almasry <almasrymina@google.com>
Date:   Mon Dec 8 01:44:39 2025 +0000

    net: devmem: coalesce sg chunks before feeding into gen_pool

    On dma_buf_map_attachment drivers typically return an sglist where each
    sg is of size 64KB. When mapping a very large dmabuf (like 4GB or so),
    this results in an sglist of 62500 entries, but usually they all map to
    a contiguous range.

    The current implementation inserts each sg as a separate chunk into the
    gen_pool. This results in much slower gen_pool_alloc and gen_pool_free
    performance due to the gen_pool actually looping through all the chunks
    and trying to free/alloc from each of them.

    There is no reason to insert each individual sg into a gen_pool chunk.
    Instead, detect that the next sg starts at the dma_addr of the current
    chunk and coalesce them.

    This results in a huge improvement in all-to-all devmem collectives, for
    example sweeping 1G and 2G message sizes:

    Before:

           0     0x0         AlltoAll  1073741824       8388608
float    none      -1   289212    3.71    3.60      0 1796277    0.60
  0.58    N/A
           0     0x0         AlltoAll  2147483648      16777216
float    none      -1 4242107    0.51    0.49      0 6301030    0.34
 0.33    N/A

    After:

           0     0x0         AlltoAll  1073741824       8388608
float    none      -1   284860    3.77    3.65      0   252456    4.25
   4.12    N/A
           0     0x0         AlltoAll  2147483648      16777216
float    none      -1   569225    3.77    3.65      0   553717    3.88
   3.76    N/A

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

    TEST=Tested using the all-to-all devmem collective outlined above.

    Reviewed-by: Eric Dumazet <edumazet@google.com>
    Reviewed-by: Kevin Berry <kpberry@google.com>

diff --git a/net/core/devmem.c b/net/core/devmem.c
index 606e556d1935b..07ba32b177390 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -274,6 +274,18 @@ net_devmem_bind_dmabuf(struct net_device *dev,
                size_t len = sg_dma_len(sg);
                struct net_iov *niov;

+               /* Detect contiguous sg's and coalesce them. This improves the
+                * gen_pool allocs/frees with O(GB) dmabufs, because the
+                * gen_pool code attempts 1 allocation/free per chunk in a
+                * list_for_each_entry_rcu.
+                */
+               while (sg_idx < binding->sgt->orig_nents - 1 &&
+                      dma_addr + len == sg_dma_address(sg_next(sg))) {
+                       len += sg_dma_len(sg_next(sg));
+                       sg = sg_next(sg);
+                       sg_idx++;
+               }
+
                owner = kzalloc_node(sizeof(*owner), GFP_KERNEL,
                                     dev_to_node(&dev->dev));
                if (!owner) {
> Stanislav Fomichev (2):
>   net: devmem: replace gen_pool with freelist
>   net: devmem: embed net_iov_area in binding
>
>  net/Kconfig       |   1 -
>  net/core/devmem.c | 158 ++++++++++++++++++----------------------------
>  net/core/devmem.h |  38 ++++-------
>  3 files changed, 72 insertions(+), 125 deletions(-)
>
> --
> 2.53.0-Meta
>


-- 
Thanks,
Mina

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next 0/2] net: devmem: remove gen_pool from dma-buf allocations
  2026-08-31 19:16 ` [PATCH net-next 0/2] net: devmem: remove gen_pool from dma-buf allocations Mina Almasry
@ 2026-09-02 18:36   ` Stanislav Fomichev
  0 siblings, 0 replies; 6+ messages in thread
From: Stanislav Fomichev @ 2026-09-02 18:36 UTC (permalink / raw)
  To: Mina Almasry
  Cc: Kaifeng Wang, netdev, davem, edumazet, kuba, pabeni, horms, sdf,
	bobbyeshleman, linux-kernel

On 08/31, Mina Almasry wrote:
> On Mon, Aug 31, 2026 at 11:35 AM Stanislav Fomichev
> <sdf.kernel@gmail.com> wrote:
> >
> > Replace devmem's gen_pool based fixed-size allocator with a binding-level
> > freelist similar to the one used by io_uring zero-copy receive.
> >
> > This is motivated by allocation latency observed in the NAPI receive path:
> >
> >   [ 1036.228913]  ? gen_pool_create+0x90/0x90
> >   [ 1036.228915]  net_devmem_alloc_dmabuf+0x1f/0x60
> >   [ 1036.228918]  mp_dmabuf_devmem_alloc_netmems+0x17/0x80
> >   [ 1036.228920]  mlx5e_post_rx_mpwqes+0xdbe/0xdd0
> >   [ 1036.228926]  mlx5e_napi_poll+0x113/0x830
> >   [ 1036.228928]  ? sched_clock+0x5/0x10
> >   [ 1036.228931]  ? wake_up_process+0x778/0x14b0
> >   [ 1036.228933]  net_rx_action+0x15d/0x570
> >   [ 1036.228934]  ? update_rq_clock+0x31/0x240
> >   [ 1036.228937]  ? __napi_schedule+0x55/0xa0
> >   [ 1036.228938]  ? mlx5_eq_comp_int+0x137/0x230
> >   [ 1036.228940]  ? atomic_notifier_call_chain+0x36/0x90
> >   [ 1036.228943]  ? sched_clock+0x5/0x10
> >   [ 1036.228944]  ? sched_clock_cpu+0xc/0x170
> >   [ 1036.228947]  irq_exit_rcu+0x12b/0x370
> >   [ 1036.228950]  common_interrupt+0x85/0x90
> >
> > udmabuf can create a very large number of SG entries. In the worst case,
> > devmem ends up adding one gen_pool chunk for each net_iov allocation
> > unit backed by those entries. The gen_pool allocation path then has to
> > traverse a linked list that can become too long for this hot path.
> >
> > Patch 1 removes the gen_pool and replaces it with a simple freelist of
> > net_iov pointers protected by the same spin_lock_bh() pattern used by
> > io_uring zcrx. Patch 2 removes the now-unnecessary chunk owner wrapper by
> > embedding the net_iov_area directly in the dma-buf binding.
> >
> 
> Oh boy, this is going to be a bit tricky.
> 
> I ran into this exact horrible perf bug (sorry for it in the first
> place), but my solution was different. My solution [1] was to coalesce
> the SG entries that are contigious (and they usually are in practice),
> and I got 'acceptable' perf after that. Kaifeng is actually working on
> cleaning up my hacky patch up to send it upstream now.

As you mention, coalescing might happen to work or it might not :-(
I'd like us to have something that's less probabilistic.

> Now I don't know which approach is better. Thinking about the pros and
> cons of your approach:
> 
> + your approach is much simpler, and removes gen_pool overheads for a
> single queue case. It should be (much?) faster for that case.
> - your approach adds a lock and allocations from multiple queues in
> parallel will contend on this lock. There should be some value of # of
> queues N where your approach starts to completely trash. gen_pool is
> lockless so I wouldn't expect it to degrade significantly in the
> multi-queue case.
> 
> The question for me is what the performance is for a real use case
> (NCCL all-to-all for example) over a realistic number of shared queues
> (it's 4-8 for me). I need that perf data to be honest before judging
> this.
> 
> The io_uring zcrx comparision is not completely valid. io_uring zcrx
> is built from the ground up to be one-buffer-is-bound-to-one-rx-queue,
> and devmem tcp is built from the ground up to be
> one-buffer-can-be-bound-to-N-rx-queues.

What if we add batching similar to io_pp_zc_alloc_netmems? So we don't
have to spin lock on every netmem (and move PP_ALLOC_CACHE_REFILL-worth
of chunks). Untested, on top of this series:


diff --git a/net/core/devmem.c b/net/core/devmem.c
index 84d6c30516c8..5a1c996ba515 100644
--- a/net/core/devmem.c
+++ b/net/core/devmem.c
@@ -58,25 +58,25 @@ void __net_devmem_dmabuf_binding_free(struct work_struct *wq)
 	kfree(binding);
 }
 
-struct net_iov *
-net_devmem_alloc_dmabuf(struct net_devmem_dmabuf_binding *binding)
+static unsigned int
+net_devmem_alloc_dmabuf_bulk(struct net_devmem_dmabuf_binding *binding,
+			     netmem_ref *netmems, unsigned int count)
 {
-	struct net_iov *niov;
+	unsigned int i;
+
 	spin_lock_bh(&binding->freelist_lock);
-	if (unlikely(!binding->free_count)) {
-		spin_unlock_bh(&binding->freelist_lock);
-		return NULL;
+
+	count = min_t(size_t, count, binding->free_count);
+	for (i = 0; i < count; i++) {
+		struct net_iov *niov = binding->freelist[--binding->free_count];
+
+		binding->freelist[binding->free_count] = NULL;
+		netmems[i] = net_iov_to_netmem(niov);
 	}
 
-	niov = binding->freelist[--binding->free_count];
-	binding->freelist[binding->free_count] = NULL;
 	spin_unlock_bh(&binding->freelist_lock);
 
-	niov->desc.pp_magic = 0;
-	niov->desc.pp = NULL;
-	atomic_long_set(&niov->desc.pp_ref_count, 0);
-
-	return niov;
+	return count;
 }
 
 void net_devmem_free_dmabuf(struct net_iov *niov)
@@ -433,20 +433,35 @@ int mp_dmabuf_devmem_init(struct page_pool *pool)
 netmem_ref mp_dmabuf_devmem_alloc_netmems(struct page_pool *pool, gfp_t gfp)
 {
 	struct net_devmem_dmabuf_binding *binding = pool->mp_priv;
-	struct net_iov *niov;
-	netmem_ref netmem;
+	netmem_ref *netmems = pool->alloc.cache;
+	unsigned int allocated, i;
+
+	if (WARN_ON_ONCE(pool->alloc.count))
+		return 0;
 
-	niov = net_devmem_alloc_dmabuf(binding);
-	if (!niov)
+	allocated = net_devmem_alloc_dmabuf_bulk(binding, netmems,
+						 PP_ALLOC_CACHE_REFILL);
+	if (unlikely(!allocated))
 		return 0;
 
-	netmem = net_iov_to_netmem(niov);
+	for (i = 0; i < allocated; i++) {
+		struct net_iov *niov = netmem_to_net_iov(netmems[i]);
 
-	page_pool_set_pp_info(pool, netmem);
+		niov->desc.pp_magic = 0;
+		niov->desc.pp = NULL;
+		atomic_long_set(&niov->desc.pp_ref_count, 0);
+
+		page_pool_set_pp_info(pool, netmems[i]);
+
+		pool->pages_state_hold_cnt++;
+		trace_page_pool_state_hold(pool, netmems[i],
+					   pool->pages_state_hold_cnt);
+	}
 
-	pool->pages_state_hold_cnt++;
-	trace_page_pool_state_hold(pool, netmem, pool->pages_state_hold_cnt);
-	return netmem;
+	/* Return the last one, the rest stay in the page_pool cache. */
+	allocated--;
+	pool->alloc.count = allocated;
+	return netmems[allocated];
 }
 
 void mp_dmabuf_devmem_destroy(struct page_pool *pool)
diff --git a/net/core/devmem.h b/net/core/devmem.h
index 20a3eb90ea7f..7195769b8bd1 100644
--- a/net/core/devmem.h
+++ b/net/core/devmem.h
@@ -133,8 +133,6 @@ net_devmem_dmabuf_binding_put(struct net_devmem_dmabuf_binding *binding)
 void net_devmem_get_net_iov(struct net_iov *niov);
 void net_devmem_put_net_iov(struct net_iov *niov);
 
-struct net_iov *
-net_devmem_alloc_dmabuf(struct net_devmem_dmabuf_binding *binding);
 void net_devmem_free_dmabuf(struct net_iov *ppiov);
 
 
@@ -191,12 +189,6 @@ net_devmem_bind_dmabuf_to_queue(struct net_device *dev, u32 rxq_idx,
 	return -EOPNOTSUPP;
 }
 
-static inline struct net_iov *
-net_devmem_alloc_dmabuf(struct net_devmem_dmabuf_binding *binding)
-{
-	return NULL;
-}
-
 static inline void net_devmem_free_dmabuf(struct net_iov *ppiov)
 {
 }


> Are you able to get NCCL all-to-all tests for N=4/8 yourself?
> Otherwise please wait for me to backport this to my release kernel and
> test it. ETA sometime this week, I hope.

I can definitely wait for you to access the perf impact on your side. Wonder
if we need to have a selftest to do that properly in NIPA. Doesn't have to
be a red/green signal, but some number for humans to compare
before-after (specifically this bind one dmabuf to multiple queues and pass
a lot of traffic). I can probably sketch something..

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net-next 1/2] net: devmem: replace gen_pool with freelist
  2026-08-31 18:35 ` [PATCH net-next 1/2] net: devmem: replace gen_pool with freelist Stanislav Fomichev
@ 2026-09-04  2:22   ` Jakub Kicinski
  0 siblings, 0 replies; 6+ messages in thread
From: Jakub Kicinski @ 2026-09-04  2:22 UTC (permalink / raw)
  To: Stanislav Fomichev
  Cc: netdev, davem, edumazet, pabeni, horms, sdf, bobbyeshleman,
	almasrymina, linux-kernel

On Mon, 31 Aug 2026 11:35:28 -0700 Stanislav Fomichev wrote:
>  	struct dma_buf *dmabuf;
> -	unsigned int sg_idx, i;
> -	unsigned long virtual;
> +	struct dmabuf_genpool_chunk_owner *owner;
> +	unsigned int sg_idx;

reverse xmas tree

The idea makes sense, but indeed hard to merge this without any perf
data to support..

I'll mark this as deferred since it's reaching the top of the queue 
for us, please repost when ready.

(BTW for some reason NIPA started showing flakes in qlease around the
time this got into testing branch, but it can't be the reason. Maybe
my split of the GRO test reordered something.)

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-04  2:22 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 18:35 [PATCH net-next 0/2] net: devmem: remove gen_pool from dma-buf allocations Stanislav Fomichev
2026-08-31 18:35 ` [PATCH net-next 1/2] net: devmem: replace gen_pool with freelist Stanislav Fomichev
2026-09-04  2:22   ` Jakub Kicinski
2026-08-31 18:35 ` [PATCH net-next 2/2] net: devmem: embed net_iov_area in binding Stanislav Fomichev
2026-08-31 19:16 ` [PATCH net-next 0/2] net: devmem: remove gen_pool from dma-buf allocations Mina Almasry
2026-09-02 18:36   ` Stanislav Fomichev

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox