* [PATCH net-next 0/3] xsk: introduce atomic for cq in generic
@ 2025-11-24 8:08 Jason Xing
2025-11-24 8:08 ` [PATCH net-next 1/3] xsk: add atomic cached_prod for copy mode Jason Xing
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Jason Xing @ 2025-11-24 8:08 UTC (permalink / raw)
To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson,
maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk,
john.fastabend
Cc: bpf, netdev, Jason Xing
From: Jason Xing <kernelxing@tencent.com>
In the hot path (that is __xsk_generic_xmit()), playing with spin lock
is time consuming. So this series replaces spin lock with atomic
operations to get better performance.
Jason Xing (3):
xsk: add atomic cached_prod for copy mode
xsk: add the atomic parameter around cq in generic path
xsk: convert cq from spin lock protection into atomic operations
include/net/xsk_buff_pool.h | 5 -----
net/xdp/xsk.c | 16 ++++------------
net/xdp/xsk_buff_pool.c | 1 -
net/xdp/xsk_queue.h | 37 ++++++++++++++++++++++++-------------
4 files changed, 28 insertions(+), 31 deletions(-)
--
2.41.3
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net-next 1/3] xsk: add atomic cached_prod for copy mode 2025-11-24 8:08 [PATCH net-next 0/3] xsk: introduce atomic for cq in generic Jason Xing @ 2025-11-24 8:08 ` Jason Xing 2025-11-24 8:08 ` [PATCH net-next 2/3] xsk: add the atomic parameter around cq in generic path Jason Xing 2025-11-24 8:08 ` [PATCH net-next 3/3] xsk: convert cq from spin lock protection into atomic operations Jason Xing 2 siblings, 0 replies; 6+ messages in thread From: Jason Xing @ 2025-11-24 8:08 UTC (permalink / raw) To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson, maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk, john.fastabend Cc: bpf, netdev, Jason Xing From: Jason Xing <kernelxing@tencent.com> Add a union member for completion queue only in copy mode for now. The purpose is to replace the cq_cached_prod_lock with atomic operation to improve performance. Note that completion queue in zerocopy mode doesn't need to be converted because the whole process is lockless. Signed-off-by: Jason Xing <kernelxing@tencent.com> --- net/xdp/xsk_queue.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h index 1eb8d9f8b104..44cc01555c0b 100644 --- a/net/xdp/xsk_queue.h +++ b/net/xdp/xsk_queue.h @@ -40,7 +40,11 @@ struct xdp_umem_ring { struct xsk_queue { u32 ring_mask; u32 nentries; - u32 cached_prod; + union { + u32 cached_prod; + /* Used for cq in copy mode only */ + atomic_t cached_prod_atomic; + }; u32 cached_cons; struct xdp_ring *ring; u64 invalid_descs; -- 2.41.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net-next 2/3] xsk: add the atomic parameter around cq in generic path 2025-11-24 8:08 [PATCH net-next 0/3] xsk: introduce atomic for cq in generic Jason Xing 2025-11-24 8:08 ` [PATCH net-next 1/3] xsk: add atomic cached_prod for copy mode Jason Xing @ 2025-11-24 8:08 ` Jason Xing 2025-11-24 19:10 ` Maciej Fijalkowski 2025-11-24 8:08 ` [PATCH net-next 3/3] xsk: convert cq from spin lock protection into atomic operations Jason Xing 2 siblings, 1 reply; 6+ messages in thread From: Jason Xing @ 2025-11-24 8:08 UTC (permalink / raw) To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson, maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk, john.fastabend Cc: bpf, netdev, Jason Xing From: Jason Xing <kernelxing@tencent.com> No functional changes here. Add a new parameter as a prep to help completion queue in copy mode convert into atomic type in the rest of this series. The patch also keeps the unified interface. Signed-off-by: Jason Xing <kernelxing@tencent.com> --- net/xdp/xsk.c | 8 ++++---- net/xdp/xsk_queue.h | 31 +++++++++++++++++++------------ 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c index bcfd400e9cf8..4e95b894f218 100644 --- a/net/xdp/xsk.c +++ b/net/xdp/xsk.c @@ -276,7 +276,7 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len) xs->rx_dropped++; return -ENOMEM; } - if (xskq_prod_nb_free(xs->rx, num_desc) < num_desc) { + if (xskq_prod_nb_free(xs->rx, num_desc, false) < num_desc) { xs->rx_queue_full++; return -ENOBUFS; } @@ -519,7 +519,7 @@ u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts) * packets. This avoids having to implement any buffering in * the Tx path. */ - nb_pkts = xskq_prod_nb_free(pool->cq, nb_pkts); + nb_pkts = xskq_prod_nb_free(pool->cq, nb_pkts, false); if (!nb_pkts) goto out; @@ -551,7 +551,7 @@ static int xsk_cq_reserve_locked(struct xsk_buff_pool *pool) int ret; spin_lock(&pool->cq_cached_prod_lock); - ret = xskq_prod_reserve(pool->cq); + ret = xskq_prod_reserve(pool->cq, false); spin_unlock(&pool->cq_cached_prod_lock); return ret; @@ -588,7 +588,7 @@ static void xsk_cq_submit_addr_locked(struct xsk_buff_pool *pool, static void xsk_cq_cancel_locked(struct xsk_buff_pool *pool, u32 n) { spin_lock(&pool->cq_cached_prod_lock); - xskq_prod_cancel_n(pool->cq, n); + xskq_prod_cancel_n(pool->cq, n, false); spin_unlock(&pool->cq_cached_prod_lock); } diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h index 44cc01555c0b..7b4d9b954584 100644 --- a/net/xdp/xsk_queue.h +++ b/net/xdp/xsk_queue.h @@ -378,37 +378,44 @@ static inline u32 xskq_get_prod(struct xsk_queue *q) return READ_ONCE(q->ring->producer); } -static inline u32 xskq_prod_nb_free(struct xsk_queue *q, u32 max) +static inline u32 xskq_prod_nb_free(struct xsk_queue *q, u32 max, bool atomic) { - u32 free_entries = q->nentries - (q->cached_prod - q->cached_cons); + u32 cached_prod = atomic ? atomic_read(&q->cached_prod_atomic) : q->cached_prod; + u32 free_entries = q->nentries - (cached_prod - q->cached_cons); if (free_entries >= max) return max; /* Refresh the local tail pointer */ q->cached_cons = READ_ONCE(q->ring->consumer); - free_entries = q->nentries - (q->cached_prod - q->cached_cons); + free_entries = q->nentries - (cached_prod - q->cached_cons); return free_entries >= max ? max : free_entries; } -static inline bool xskq_prod_is_full(struct xsk_queue *q) +static inline bool xskq_prod_is_full(struct xsk_queue *q, bool atomic) { - return xskq_prod_nb_free(q, 1) ? false : true; + return xskq_prod_nb_free(q, 1, atomic) ? false : true; } -static inline void xskq_prod_cancel_n(struct xsk_queue *q, u32 cnt) +static inline void xskq_prod_cancel_n(struct xsk_queue *q, u32 cnt, bool atomic) { - q->cached_prod -= cnt; + if (atomic) + atomic_sub(cnt, &q->cached_prod_atomic); + else + q->cached_prod -= cnt; } -static inline int xskq_prod_reserve(struct xsk_queue *q) +static inline int xskq_prod_reserve(struct xsk_queue *q, bool atomic) { - if (xskq_prod_is_full(q)) + if (xskq_prod_is_full(q, atomic)) return -ENOSPC; /* A, matches D */ - q->cached_prod++; + if (atomic) + atomic_inc(&q->cached_prod_atomic); + else + q->cached_prod++; return 0; } @@ -416,7 +423,7 @@ static inline int xskq_prod_reserve_addr(struct xsk_queue *q, u64 addr) { struct xdp_umem_ring *ring = (struct xdp_umem_ring *)q->ring; - if (xskq_prod_is_full(q)) + if (xskq_prod_is_full(q, false)) return -ENOSPC; /* A, matches D */ @@ -450,7 +457,7 @@ static inline int xskq_prod_reserve_desc(struct xsk_queue *q, struct xdp_rxtx_ring *ring = (struct xdp_rxtx_ring *)q->ring; u32 idx; - if (xskq_prod_is_full(q)) + if (xskq_prod_is_full(q, false)) return -ENOBUFS; /* A, matches D */ -- 2.41.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/3] xsk: add the atomic parameter around cq in generic path 2025-11-24 8:08 ` [PATCH net-next 2/3] xsk: add the atomic parameter around cq in generic path Jason Xing @ 2025-11-24 19:10 ` Maciej Fijalkowski 2025-11-24 23:43 ` Jason Xing 0 siblings, 1 reply; 6+ messages in thread From: Maciej Fijalkowski @ 2025-11-24 19:10 UTC (permalink / raw) To: Jason Xing Cc: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson, jonathan.lemon, sdf, ast, daniel, hawk, john.fastabend, bpf, netdev, Jason Xing On Mon, Nov 24, 2025 at 04:08:57PM +0800, Jason Xing wrote: > From: Jason Xing <kernelxing@tencent.com> > > No functional changes here. Add a new parameter as a prep to help > completion queue in copy mode convert into atomic type in the rest of > this series. The patch also keeps the unified interface. Jason, anything used in ZC should not get a penalty from changes developed to improve copy mode. I'd rather suggest separate functions rather than branches within shared routines. > > Signed-off-by: Jason Xing <kernelxing@tencent.com> > --- > net/xdp/xsk.c | 8 ++++---- > net/xdp/xsk_queue.h | 31 +++++++++++++++++++------------ > 2 files changed, 23 insertions(+), 16 deletions(-) > > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c > index bcfd400e9cf8..4e95b894f218 100644 > --- a/net/xdp/xsk.c > +++ b/net/xdp/xsk.c > @@ -276,7 +276,7 @@ static int __xsk_rcv(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len) > xs->rx_dropped++; > return -ENOMEM; > } > - if (xskq_prod_nb_free(xs->rx, num_desc) < num_desc) { > + if (xskq_prod_nb_free(xs->rx, num_desc, false) < num_desc) { > xs->rx_queue_full++; > return -ENOBUFS; > } > @@ -519,7 +519,7 @@ u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts) > * packets. This avoids having to implement any buffering in > * the Tx path. > */ > - nb_pkts = xskq_prod_nb_free(pool->cq, nb_pkts); > + nb_pkts = xskq_prod_nb_free(pool->cq, nb_pkts, false); > if (!nb_pkts) > goto out; > > @@ -551,7 +551,7 @@ static int xsk_cq_reserve_locked(struct xsk_buff_pool *pool) > int ret; > > spin_lock(&pool->cq_cached_prod_lock); > - ret = xskq_prod_reserve(pool->cq); > + ret = xskq_prod_reserve(pool->cq, false); > spin_unlock(&pool->cq_cached_prod_lock); > > return ret; > @@ -588,7 +588,7 @@ static void xsk_cq_submit_addr_locked(struct xsk_buff_pool *pool, > static void xsk_cq_cancel_locked(struct xsk_buff_pool *pool, u32 n) > { > spin_lock(&pool->cq_cached_prod_lock); > - xskq_prod_cancel_n(pool->cq, n); > + xskq_prod_cancel_n(pool->cq, n, false); > spin_unlock(&pool->cq_cached_prod_lock); > } > > diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h > index 44cc01555c0b..7b4d9b954584 100644 > --- a/net/xdp/xsk_queue.h > +++ b/net/xdp/xsk_queue.h > @@ -378,37 +378,44 @@ static inline u32 xskq_get_prod(struct xsk_queue *q) > return READ_ONCE(q->ring->producer); > } > > -static inline u32 xskq_prod_nb_free(struct xsk_queue *q, u32 max) > +static inline u32 xskq_prod_nb_free(struct xsk_queue *q, u32 max, bool atomic) > { > - u32 free_entries = q->nentries - (q->cached_prod - q->cached_cons); > + u32 cached_prod = atomic ? atomic_read(&q->cached_prod_atomic) : q->cached_prod; > + u32 free_entries = q->nentries - (cached_prod - q->cached_cons); > > if (free_entries >= max) > return max; > > /* Refresh the local tail pointer */ > q->cached_cons = READ_ONCE(q->ring->consumer); > - free_entries = q->nentries - (q->cached_prod - q->cached_cons); > + free_entries = q->nentries - (cached_prod - q->cached_cons); > > return free_entries >= max ? max : free_entries; > } > > -static inline bool xskq_prod_is_full(struct xsk_queue *q) > +static inline bool xskq_prod_is_full(struct xsk_queue *q, bool atomic) > { > - return xskq_prod_nb_free(q, 1) ? false : true; > + return xskq_prod_nb_free(q, 1, atomic) ? false : true; > } > > -static inline void xskq_prod_cancel_n(struct xsk_queue *q, u32 cnt) > +static inline void xskq_prod_cancel_n(struct xsk_queue *q, u32 cnt, bool atomic) > { > - q->cached_prod -= cnt; > + if (atomic) > + atomic_sub(cnt, &q->cached_prod_atomic); > + else > + q->cached_prod -= cnt; > } > > -static inline int xskq_prod_reserve(struct xsk_queue *q) > +static inline int xskq_prod_reserve(struct xsk_queue *q, bool atomic) > { > - if (xskq_prod_is_full(q)) > + if (xskq_prod_is_full(q, atomic)) > return -ENOSPC; > > /* A, matches D */ > - q->cached_prod++; > + if (atomic) > + atomic_inc(&q->cached_prod_atomic); > + else > + q->cached_prod++; > return 0; > } > > @@ -416,7 +423,7 @@ static inline int xskq_prod_reserve_addr(struct xsk_queue *q, u64 addr) > { > struct xdp_umem_ring *ring = (struct xdp_umem_ring *)q->ring; > > - if (xskq_prod_is_full(q)) > + if (xskq_prod_is_full(q, false)) > return -ENOSPC; > > /* A, matches D */ > @@ -450,7 +457,7 @@ static inline int xskq_prod_reserve_desc(struct xsk_queue *q, > struct xdp_rxtx_ring *ring = (struct xdp_rxtx_ring *)q->ring; > u32 idx; > > - if (xskq_prod_is_full(q)) > + if (xskq_prod_is_full(q, false)) > return -ENOBUFS; > > /* A, matches D */ > -- > 2.41.3 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net-next 2/3] xsk: add the atomic parameter around cq in generic path 2025-11-24 19:10 ` Maciej Fijalkowski @ 2025-11-24 23:43 ` Jason Xing 0 siblings, 0 replies; 6+ messages in thread From: Jason Xing @ 2025-11-24 23:43 UTC (permalink / raw) To: Maciej Fijalkowski Cc: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson, jonathan.lemon, sdf, ast, daniel, hawk, john.fastabend, bpf, netdev, Jason Xing On Tue, Nov 25, 2025 at 3:10 AM Maciej Fijalkowski <maciej.fijalkowski@intel.com> wrote: > > On Mon, Nov 24, 2025 at 04:08:57PM +0800, Jason Xing wrote: > > From: Jason Xing <kernelxing@tencent.com> > > > > No functional changes here. Add a new parameter as a prep to help > > completion queue in copy mode convert into atomic type in the rest of > > this series. The patch also keeps the unified interface. > > Jason, > > anything used in ZC should not get a penalty from changes developed to > improve copy mode. I'd rather suggest separate functions rather than > branches within shared routines. Sure thing:) Will change that. Thanks, Jason ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next 3/3] xsk: convert cq from spin lock protection into atomic operations 2025-11-24 8:08 [PATCH net-next 0/3] xsk: introduce atomic for cq in generic Jason Xing 2025-11-24 8:08 ` [PATCH net-next 1/3] xsk: add atomic cached_prod for copy mode Jason Xing 2025-11-24 8:08 ` [PATCH net-next 2/3] xsk: add the atomic parameter around cq in generic path Jason Xing @ 2025-11-24 8:08 ` Jason Xing 2 siblings, 0 replies; 6+ messages in thread From: Jason Xing @ 2025-11-24 8:08 UTC (permalink / raw) To: davem, edumazet, kuba, pabeni, bjorn, magnus.karlsson, maciej.fijalkowski, jonathan.lemon, sdf, ast, daniel, hawk, john.fastabend Cc: bpf, netdev, Jason Xing From: Jason Xing <kernelxing@tencent.com> Now it's time to convert cq in generic path into atomic operations to achieve a higher performance number. I managed to see it improve around 5% over different platforms. Signed-off-by: Jason Xing <kernelxing@tencent.com> --- include/net/xsk_buff_pool.h | 5 ----- net/xdp/xsk.c | 12 ++---------- net/xdp/xsk_buff_pool.c | 1 - 3 files changed, 2 insertions(+), 16 deletions(-) diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h index 92a2358c6ce3..0b1abdb99c9e 100644 --- a/include/net/xsk_buff_pool.h +++ b/include/net/xsk_buff_pool.h @@ -90,11 +90,6 @@ struct xsk_buff_pool { * destructor callback. */ spinlock_t cq_prod_lock; - /* Mutual exclusion of the completion ring in the SKB mode. - * Protect: when sockets share a single cq when the same netdev - * and queue id is shared. - */ - spinlock_t cq_cached_prod_lock; struct xdp_buff_xsk *free_heads[]; }; diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c index 4e95b894f218..6b99a7eeb952 100644 --- a/net/xdp/xsk.c +++ b/net/xdp/xsk.c @@ -548,13 +548,7 @@ static int xsk_wakeup(struct xdp_sock *xs, u8 flags) static int xsk_cq_reserve_locked(struct xsk_buff_pool *pool) { - int ret; - - spin_lock(&pool->cq_cached_prod_lock); - ret = xskq_prod_reserve(pool->cq, false); - spin_unlock(&pool->cq_cached_prod_lock); - - return ret; + return xskq_prod_reserve(pool->cq, true); } static void xsk_cq_submit_addr_locked(struct xsk_buff_pool *pool, @@ -587,9 +581,7 @@ static void xsk_cq_submit_addr_locked(struct xsk_buff_pool *pool, static void xsk_cq_cancel_locked(struct xsk_buff_pool *pool, u32 n) { - spin_lock(&pool->cq_cached_prod_lock); - xskq_prod_cancel_n(pool->cq, n, false); - spin_unlock(&pool->cq_cached_prod_lock); + xskq_prod_cancel_n(pool->cq, n, true); } static void xsk_inc_num_desc(struct sk_buff *skb) diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c index 51526034c42a..9539f121b290 100644 --- a/net/xdp/xsk_buff_pool.c +++ b/net/xdp/xsk_buff_pool.c @@ -91,7 +91,6 @@ struct xsk_buff_pool *xp_create_and_assign_umem(struct xdp_sock *xs, INIT_LIST_HEAD(&pool->xsk_tx_list); spin_lock_init(&pool->xsk_tx_list_lock); spin_lock_init(&pool->cq_prod_lock); - spin_lock_init(&pool->cq_cached_prod_lock); refcount_set(&pool->users, 1); pool->fq = xs->fq_tmp; -- 2.41.3 ^ permalink raw reply related [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-11-24 23:44 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-24 8:08 [PATCH net-next 0/3] xsk: introduce atomic for cq in generic Jason Xing 2025-11-24 8:08 ` [PATCH net-next 1/3] xsk: add atomic cached_prod for copy mode Jason Xing 2025-11-24 8:08 ` [PATCH net-next 2/3] xsk: add the atomic parameter around cq in generic path Jason Xing 2025-11-24 19:10 ` Maciej Fijalkowski 2025-11-24 23:43 ` Jason Xing 2025-11-24 8:08 ` [PATCH net-next 3/3] xsk: convert cq from spin lock protection into atomic operations Jason Xing
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).