* [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