Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next] net/mlx5e: bound TX CQ poll softirq residency with a time budget
@ 2026-07-03  1:36 Jose Fernandez (Anthropic)
  2026-07-06 11:48 ` Dragos Tatulea
  0 siblings, 1 reply; 2+ messages in thread
From: Jose Fernandez (Anthropic) @ 2026-07-03  1:36 UTC (permalink / raw)
  To: Saeed Mahameed, Tariq Toukan, Mark Bloch, Leon Romanovsky,
	Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: netdev, linux-rdma, linux-kernel, Jose Fernandez (Anthropic),
	Ben Cressey

Under strict IOMMU invalidation (iommu.strict=1), each per-fragment DMA
unmap in the TX completion path issues a synchronous TLB invalidate and
waits for CMD_SYNC, spinning IRQ-off in the SMMU command queue. Under
cross-CPU command-queue contention this per-unmap cost inflates from
microseconds to hundreds of microseconds. mlx5e_poll_tx_cq()'s per-CQE
budget (128) does not bound time in this regime: one CQE can cover a
multi-WQE batch with many fragments, so a single poll invocation can
accumulate seconds of softirq residency and trip the soft-lockup
watchdog on arm64/SMMU-v3 systems.

Bound the invocation by time: check local_clock() every 8 CQEs against
a budget (default 500us; module parameter tx_cq_time_budget_us,
runtime-writable, 0 disables) and break out of the CQE loop when
exceeded, reporting busy exactly like the existing CQE-budget
exhaustion path so NAPI keeps the poll scheduled. Remaining
completions are delayed by one reschedule, never stranded. The inner
WQE walk is never interrupted mid-CQE (sqcc/dma_fifo_cc accounting).
A new ethtool statistic (tx_time_budget_exit) counts early exits.

Also add cond_resched() in mlx5e_free_txqsq_descs(): the teardown path
walks the same per-fragment unmaps in process context.

Tested on arm64 with SMMU-v3 under strict mode: throughput cost is
within run-to-run variance at every measured load shape; under active
invalidation-storm contention, the bounded poll measures 35-50%
faster than unbounded (bounded polling yields cores back to the
transmit path).

Assisted-by: Claude:unspecified
Signed-off-by: Jose Fernandez (Anthropic) <jose.fernandez@linux.dev>
Reviewed-by: Ben Cressey <ben@cressey.dev>
---
 drivers/net/ethernet/mellanox/mlx5/core/en_stats.c |  5 ++++
 drivers/net/ethernet/mellanox/mlx5/core/en_stats.h |  2 ++
 drivers/net/ethernet/mellanox/mlx5/core/en_tx.c    | 29 +++++++++++++++++++++-
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
index 7f33261ba655..b940280af19d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
@@ -171,6 +171,7 @@ static const struct counter_desc sw_stats_desc[] = {
 	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_cqes) },
 	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_queue_wake) },
 	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_cqe_err) },
+	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_time_budget_exit) },
 	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_xdp_xmit) },
 	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_xdp_mpwqe) },
 	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_xdp_inlnw) },
@@ -426,6 +427,7 @@ static void mlx5e_stats_grp_sw_update_stats_sq(struct mlx5e_sw_stats *s,
 	s->tx_queue_wake            += sq_stats->wake;
 	s->tx_queue_dropped         += sq_stats->dropped;
 	s->tx_cqe_err               += sq_stats->cqe_err;
+	s->tx_time_budget_exit      += sq_stats->time_budget_exit;
 	s->tx_recover               += sq_stats->recover;
 	s->tx_xmit_more             += sq_stats->xmit_more;
 	s->tx_csum_partial_inner    += sq_stats->csum_partial_inner;
@@ -2323,6 +2325,7 @@ static const struct counter_desc sq_stats_desc[] = {
 	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, cqes) },
 	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, wake) },
 	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, cqe_err) },
+	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, time_budget_exit) },
 };
 
 static const struct counter_desc rq_xdpsq_stats_desc[] = {
@@ -2399,6 +2402,7 @@ static const struct counter_desc ptp_sq_stats_desc[] = {
 	{ MLX5E_DECLARE_PTP_TX_STAT(struct mlx5e_sq_stats, cqes) },
 	{ MLX5E_DECLARE_PTP_TX_STAT(struct mlx5e_sq_stats, wake) },
 	{ MLX5E_DECLARE_PTP_TX_STAT(struct mlx5e_sq_stats, cqe_err) },
+	{ MLX5E_DECLARE_PTP_TX_STAT(struct mlx5e_sq_stats, time_budget_exit) },
 };
 
 static const struct counter_desc ptp_ch_stats_desc[] = {
@@ -2476,6 +2480,7 @@ static const struct counter_desc qos_sq_stats_desc[] = {
 	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, cqes) },
 	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, wake) },
 	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, cqe_err) },
+	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, time_budget_exit) },
 };
 
 #define NUM_RQ_STATS			ARRAY_SIZE(rq_stats_desc)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
index 09f155acb461..5ba954f42ccd 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
@@ -187,6 +187,7 @@ struct mlx5e_sw_stats {
 	u64 tx_cqes;
 	u64 tx_queue_wake;
 	u64 tx_cqe_err;
+	u64 tx_time_budget_exit;
 	u64 tx_xdp_xmit;
 	u64 tx_xdp_mpwqe;
 	u64 tx_xdp_inlnw;
@@ -445,6 +446,7 @@ struct mlx5e_sq_stats {
 	u64 cqes ____cacheline_aligned_in_smp;
 	u64 wake;
 	u64 cqe_err;
+	u64 time_budget_exit;
 };
 
 struct mlx5e_xdpsq_stats {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
index 0b5e600e4a6a..994df912b765 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
@@ -43,6 +43,13 @@
 #include "en_accel/macsec.h"
 #include "en/ptp.h"
 #include <net/ipv6.h>
+#include <linux/moduleparam.h>
+#include <linux/sched/clock.h>
+
+static unsigned int mlx5e_tx_cq_time_budget_us = 500;
+module_param_named(tx_cq_time_budget_us, mlx5e_tx_cq_time_budget_us, uint, 0644);
+MODULE_PARM_DESC(tx_cq_time_budget_us,
+		 "Max microseconds one TX CQ poll may spend before yielding (0 = unbounded)");
 
 static void mlx5e_dma_unmap_wqe_err(struct mlx5e_txqsq *sq, u8 num_dma)
 {
@@ -760,9 +767,12 @@ void mlx5e_txqsq_wake(struct mlx5e_txqsq *sq)
 bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
 {
 	struct mlx5e_sq_stats *stats;
+	bool time_exceeded = false;
+	u64 time_budget_end = 0;
 	struct mlx5e_txqsq *sq;
 	struct mlx5_cqe64 *cqe;
 	u32 dma_fifo_cc;
+	u32 budget_us;
 	u32 nbytes;
 	u16 npkts;
 	u16 sqcc;
@@ -790,6 +800,10 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
 	/* avoid dirtying sq cache line every cqe */
 	dma_fifo_cc = sq->dma_fifo_cc;
 
+	budget_us = READ_ONCE(mlx5e_tx_cq_time_budget_us);
+	if (budget_us)
+		time_budget_end = local_clock() + (u64)budget_us * NSEC_PER_USEC;
+
 	i = 0;
 	do {
 		struct mlx5e_tx_wqe_info *wi;
@@ -842,8 +856,19 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
 			stats->cqe_err++;
 		}
 
+		/* Check between CQEs only (sqcc/dma_fifo_cc must advance together). */
+		if (unlikely(time_budget_end && (i & 7) == 7 &&
+			     local_clock() >= time_budget_end)) {
+			time_exceeded = true;
+			i++;
+			break;
+		}
+
 	} while ((++i < MLX5E_TX_CQ_POLL_BUDGET) && (cqe = mlx5_cqwq_get_cqe(&cq->wq)));
 
+	if (unlikely(time_exceeded))
+		stats->time_budget_exit++;
+
 	stats->cqes += i;
 
 	mlx5_cqwq_update_db_record(&cq->wq);
@@ -858,7 +883,7 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
 
 	mlx5e_txqsq_wake(sq);
 
-	return (i == MLX5E_TX_CQ_POLL_BUDGET);
+	return time_exceeded || (i == MLX5E_TX_CQ_POLL_BUDGET);
 }
 
 static void mlx5e_tx_wi_kfree_fifo_skbs(struct mlx5e_txqsq *sq, struct mlx5e_tx_wqe_info *wi)
@@ -879,6 +904,8 @@ void mlx5e_free_txqsq_descs(struct mlx5e_txqsq *sq)
 	dma_fifo_cc = sq->dma_fifo_cc;
 
 	while (sqcc != sq->pc) {
+		cond_resched();
+
 		ci = mlx5_wq_cyc_ctr2ix(&sq->wq, sqcc);
 		wi = &sq->db.wqe_info[ci];
 

---
base-commit: 08bc5b2636afcbadc31bb17243eec094e048bd79
change-id: 20260702-mlx5e-tx-cq-time-budget-02cccf37bf54

Best regards,
--  
Jose Fernandez (Anthropic) <jose.fernandez@linux.dev>


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

* Re: [PATCH net-next] net/mlx5e: bound TX CQ poll softirq residency with a time budget
  2026-07-03  1:36 [PATCH net-next] net/mlx5e: bound TX CQ poll softirq residency with a time budget Jose Fernandez (Anthropic)
@ 2026-07-06 11:48 ` Dragos Tatulea
  0 siblings, 0 replies; 2+ messages in thread
From: Dragos Tatulea @ 2026-07-06 11:48 UTC (permalink / raw)
  To: Jose Fernandez (Anthropic), Saeed Mahameed, Tariq Toukan,
	Mark Bloch, Leon Romanovsky, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: netdev, linux-rdma, linux-kernel, Ben Cressey

Hi Jose,

Thanks for your patch.

On 03.07.26 03:36, Jose Fernandez (Anthropic) wrote:
> Under strict IOMMU invalidation (iommu.strict=1), each per-fragment DMA
> unmap in the TX completion path issues a synchronous TLB invalidate and
> waits for CMD_SYNC, spinning IRQ-off in the SMMU command queue. Under
> cross-CPU command-queue contention this per-unmap cost inflates from
> microseconds to hundreds of microseconds. mlx5e_poll_tx_cq()'s per-CQE
> budget (128) does not bound time in this regime: one CQE can cover a
> multi-WQE batch with many fragments, so a single poll invocation can
> accumulate seconds of softirq residency and trip the soft-lockup
> watchdog on arm64/SMMU-v3 systems.
> 
This is interesting. I can think of 2 cases here which can overlap:

1) Packets are sent with xmit_more which trigger this many WQEs per
  CQE scenario.

2) TSO packets can have many fragments so even a single WQE will have
   more than one DMA unmap. For the default GSO size of 64K each TSO
   skb will have 2 fragments on your platform. If the GSO is bumped to
   256K then you usually get ~5 fragments.

Let's first understand if this is a case of 1) (small packets sent
with xmit_more) or 2) (many TSO packets) or 1+2) (many TSO packets sent
with xmit_more).

If xmit_more is used then you can actually tune the BQL to avoid this
behavior which seems to wreak havoc on your configuration.

> Bound the invocation by time: check local_clock() every 8 CQEs against
> a budget (default 500us; module parameter tx_cq_time_budget_us,
> runtime-writable, 0 disables) and break out of the CQE loop when
> exceeded, reporting busy exactly like the existing CQE-budget
> exhaustion path so NAPI keeps the poll scheduled. Remaining
> completions are delayed by one reschedule, never stranded. The inner
> WQE walk is never interrupted mid-CQE (sqcc/dma_fifo_cc accounting).
> A new ethtool statistic (tx_time_budget_exit) counts early exits.
> 
> Also add cond_resched() in mlx5e_free_txqsq_descs(): the teardown path
> walks the same per-fragment unmaps in process context.
> 
> Tested on arm64 with SMMU-v3 under strict mode: throughput cost is
> within run-to-run variance at every measured load shape; under active
> invalidation-storm contention, the bounded poll measures 35-50%
> faster than unbounded (bounded polling yields cores back to the
> transmit path).
> 
The bounded poll is faster because it is interrupted earlier or is
it faster because it results in less contention on the IOMMU?

> Assisted-by: Claude:unspecified
> Signed-off-by: Jose Fernandez (Anthropic) <jose.fernandez@linux.dev>
> Reviewed-by: Ben Cressey <ben@cressey.dev>
> ---
>  drivers/net/ethernet/mellanox/mlx5/core/en_stats.c |  5 ++++
>  drivers/net/ethernet/mellanox/mlx5/core/en_stats.h |  2 ++
>  drivers/net/ethernet/mellanox/mlx5/core/en_tx.c    | 29 +++++++++++++++++++++-
>  3 files changed, 35 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> index 7f33261ba655..b940280af19d 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.c
> @@ -171,6 +171,7 @@ static const struct counter_desc sw_stats_desc[] = {
>  	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_cqes) },
>  	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_queue_wake) },
>  	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_cqe_err) },
> +	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_time_budget_exit) },
>  	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_xdp_xmit) },
>  	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_xdp_mpwqe) },
>  	{ MLX5E_DECLARE_STAT(struct mlx5e_sw_stats, tx_xdp_inlnw) },
> @@ -426,6 +427,7 @@ static void mlx5e_stats_grp_sw_update_stats_sq(struct mlx5e_sw_stats *s,
>  	s->tx_queue_wake            += sq_stats->wake;
>  	s->tx_queue_dropped         += sq_stats->dropped;
>  	s->tx_cqe_err               += sq_stats->cqe_err;
> +	s->tx_time_budget_exit      += sq_stats->time_budget_exit;
>  	s->tx_recover               += sq_stats->recover;
>  	s->tx_xmit_more             += sq_stats->xmit_more;
>  	s->tx_csum_partial_inner    += sq_stats->csum_partial_inner;
> @@ -2323,6 +2325,7 @@ static const struct counter_desc sq_stats_desc[] = {
>  	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, cqes) },
>  	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, wake) },
>  	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, cqe_err) },
> +	{ MLX5E_DECLARE_TX_STAT(struct mlx5e_sq_stats, time_budget_exit) },
>  };
>  
>  static const struct counter_desc rq_xdpsq_stats_desc[] = {
> @@ -2399,6 +2402,7 @@ static const struct counter_desc ptp_sq_stats_desc[] = {
>  	{ MLX5E_DECLARE_PTP_TX_STAT(struct mlx5e_sq_stats, cqes) },
>  	{ MLX5E_DECLARE_PTP_TX_STAT(struct mlx5e_sq_stats, wake) },
>  	{ MLX5E_DECLARE_PTP_TX_STAT(struct mlx5e_sq_stats, cqe_err) },
> +	{ MLX5E_DECLARE_PTP_TX_STAT(struct mlx5e_sq_stats, time_budget_exit) },
>  };
>  
>  static const struct counter_desc ptp_ch_stats_desc[] = {
> @@ -2476,6 +2480,7 @@ static const struct counter_desc qos_sq_stats_desc[] = {
>  	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, cqes) },
>  	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, wake) },
>  	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, cqe_err) },
> +	{ MLX5E_DECLARE_QOS_TX_STAT(struct mlx5e_sq_stats, time_budget_exit) },
>  };
>  
>  #define NUM_RQ_STATS			ARRAY_SIZE(rq_stats_desc)
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
> index 09f155acb461..5ba954f42ccd 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_stats.h
> @@ -187,6 +187,7 @@ struct mlx5e_sw_stats {
>  	u64 tx_cqes;
>  	u64 tx_queue_wake;
>  	u64 tx_cqe_err;
> +	u64 tx_time_budget_exit;
>  	u64 tx_xdp_xmit;
>  	u64 tx_xdp_mpwqe;
>  	u64 tx_xdp_inlnw;
> @@ -445,6 +446,7 @@ struct mlx5e_sq_stats {
>  	u64 cqes ____cacheline_aligned_in_smp;
>  	u64 wake;
>  	u64 cqe_err;
> +	u64 time_budget_exit;
>  };
>  
>  struct mlx5e_xdpsq_stats {
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
> index 0b5e600e4a6a..994df912b765 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tx.c
> @@ -43,6 +43,13 @@
>  #include "en_accel/macsec.h"
>  #include "en/ptp.h"
>  #include <net/ipv6.h>
> +#include <linux/moduleparam.h>
> +#include <linux/sched/clock.h>
> +
> +static unsigned int mlx5e_tx_cq_time_budget_us = 500;
> +module_param_named(tx_cq_time_budget_us, mlx5e_tx_cq_time_budget_us, uint, 0644);
> +MODULE_PARM_DESC(tx_cq_time_budget_us,
> +		 "Max microseconds one TX CQ poll may spend before yielding (0 = unbounded)");
>  
>  static void mlx5e_dma_unmap_wqe_err(struct mlx5e_txqsq *sq, u8 num_dma)
>  {
> @@ -760,9 +767,12 @@ void mlx5e_txqsq_wake(struct mlx5e_txqsq *sq)
>  bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
>  {
>  	struct mlx5e_sq_stats *stats;
> +	bool time_exceeded = false;
> +	u64 time_budget_end = 0;
>  	struct mlx5e_txqsq *sq;
>  	struct mlx5_cqe64 *cqe;
>  	u32 dma_fifo_cc;
> +	u32 budget_us;
>  	u32 nbytes;
>  	u16 npkts;
>  	u16 sqcc;
> @@ -790,6 +800,10 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
>  	/* avoid dirtying sq cache line every cqe */
>  	dma_fifo_cc = sq->dma_fifo_cc;
>  
> +	budget_us = READ_ONCE(mlx5e_tx_cq_time_budget_us);
> +	if (budget_us)
> +		time_budget_end = local_clock() + (u64)budget_us * NSEC_PER_USEC;
> +
>  	i = 0;
>  	do {
>  		struct mlx5e_tx_wqe_info *wi;
> @@ -842,8 +856,19 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
>  			stats->cqe_err++;
>  		}
>  
> +		/* Check between CQEs only (sqcc/dma_fifo_cc must advance together). */
> +		if (unlikely(time_budget_end && (i & 7) == 7 &&
> +			     local_clock() >= time_budget_end)) {
> +			time_exceeded = true;
> +			i++;
> +			break;
> +		}
> +
>  	} while ((++i < MLX5E_TX_CQ_POLL_BUDGET) && (cqe = mlx5_cqwq_get_cqe(&cq->wq)));
>  
> +	if (unlikely(time_exceeded))
> +		stats->time_budget_exit++;
> +
>  	stats->cqes += i;
>  
>  	mlx5_cqwq_update_db_record(&cq->wq);
> @@ -858,7 +883,7 @@ bool mlx5e_poll_tx_cq(struct mlx5e_cq *cq, int napi_budget)
>  
>  	mlx5e_txqsq_wake(sq);
>  
> -	return (i == MLX5E_TX_CQ_POLL_BUDGET);
> +	return time_exceeded || (i == MLX5E_TX_CQ_POLL_BUDGET);
>  }
>  
This change is too invasive in the driver for all the other cases. We have to figure
another way to go about this issue.

Thanks,
Dragos

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

end of thread, other threads:[~2026-07-06 11:48 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-03  1:36 [PATCH net-next] net/mlx5e: bound TX CQ poll softirq residency with a time budget Jose Fernandez (Anthropic)
2026-07-06 11:48 ` Dragos Tatulea

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