[PATCH RFC 0/2] net/mlx5: Cut workqueue traffic of firmware commands and stats

[PATCH RFC 0/2] net/mlx5: Cut workqueue traffic of firmware commands and stats

[Breno Leitao]

While looking for Meta's arm64 fleet wide lockstat, I found that mlx5 is
one of the major culprit for the locks in the fleet, and this is the
motivation for this patchset.

On an arm64 host with a dozen mlx5 functions in switchdev mode, lock
contention profiling showed the pool->lock of the unbound worker pool
that all ordered workqueues share as the hottest workqueue lock.
Monitoring daemons read /proc/net/dev, so every mlx5e netdev keeps
queueing update_stats_work. Each refresh sends one to three firmware
commands, and each command is one more work item, on the ordered
cmd->wq of its function.

Patch 1 submits blocking firmware commands from the caller's context,
as page-queue commands already do, and keeps the workqueue for
callback commands.

Patch 2 brings back the 200 msecs stats refresh interval that the
driver used until 2018, as the minimum time between two on-demand
refreshes.

Tested on a ConnectX-4 Lx host (x86, firmware 14.32) against the base
commit, in legacy mode and in switchdev mode with 8 VFs. With one
/proc/net/dev reader in switchdev mode, the firmware commands of a
13 second run drop from 13.4k to 1.8k, and the mlx5_cmd work items of
32 parallel ethtool -S loops drop from 46.7k to none. Channel
reconfiguration, switchdev enable/disable and the load above ran with
no failed firmware commands and no warnings.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
Breno Leitao (2):
      net/mlx5: Submit blocking commands from the caller's context
      net/mlx5e: Rate limit the HW stats refresh from ndo_get_stats64

 drivers/net/ethernet/mellanox/mlx5/core/cmd.c     | 3 ++-
 drivers/net/ethernet/mellanox/mlx5/core/en.h      | 2 ++
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 9 +++++++++
 3 files changed, 13 insertions(+), 1 deletion(-)
---
base-commit: 4dd16e31135ea8437a720222df34e8745f385d6d
change-id: 20261030-mlx5-cmd-wq-a156836104af

Best regards,
--  
Breno Leitao <leitao@debian.org>

[PATCH RFC 1/2] net/mlx5: Submit blocking commands from the caller's context

[Breno Leitao]

mlx5_cmd_invoke() queues every command that is not a page-queue command
on the ordered cmd->wq, and a blocking caller then sleeps in wait_func()
until the command completes. The work item only takes a command slot,
writes the descriptor and rings the doorbell, so each blocking command
pays for a queue_work(), a worker wakeup and a context switch while its
caller sits idle.

All ordered workqueues share one unbound worker pool, so the command
work items of every mlx5 function also contend on a single pool->lock.
On an arm64 host with a dozen mlx5 functions in switchdev mode, where
monitoring daemons read /proc/net/dev, lock contention profiling put
36% of the pool->lock wait in queue_work() called from cmd_exec(), and
the rest in the workers running the command and stats work items.

Page-queue commands already call cmd_work_handler() directly. Do the
same for every command without a callback. Callback commands keep using
the workqueue, since mlx5_cmd_exec_cb() must not block.

Firmware runs up to max_reg_cmds commands concurrently, so callers could
not rely on the submission order, and the slot semaphore still serves
its waiters in FIFO order.

With 32 parallel ethtool -S loops on a ConnectX-4 Lx, this removes all
46.7k mlx5_cmd work items of the run and cuts kworker context switches
from 50.7k to 3.9k.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
index 84583dc5eb1c05..988518155582a4 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
@@ -1301,7 +1301,8 @@ static int mlx5_cmd_invoke(struct mlx5_core_dev *dev, struct mlx5_cmd_msg *in,
 
 	INIT_DELAYED_WORK(&ent->cb_timeout_work, cb_timeout_handler);
 	INIT_WORK(&ent->work, cmd_work_handler);
-	if (page_queue) {
+	/* A blocking caller waits for the command anyway, submit it inline */
+	if (page_queue || !callback) {
 		cmd_work_handler(&ent->work);
 	} else if (!queue_work(cmd->wq, &ent->work)) {
 		mlx5_core_warn(dev, "failed to queue work\n");

-- 
2.53.0-Meta

Re: [PATCH RFC 1/2] net/mlx5: Submit blocking commands from the caller's context

[Moshe Shemesh]

On 10/30/2026 6:31 PM, Breno Leitao wrote:
> mlx5_cmd_invoke() queues every command that is not a page-queue command
> on the ordered cmd->wq, and a blocking caller then sleeps in wait_func()
> until the command completes. The work item only takes a command slot,
> writes the descriptor and rings the doorbell, so each blocking command
> pays for a queue_work(), a worker wakeup and a context switch while its
> caller sits idle.
> 
> All ordered workqueues share one unbound worker pool, so the command
> work items of every mlx5 function also contend on a single pool->lock.
> On an arm64 host with a dozen mlx5 functions in switchdev mode, where
> monitoring daemons read /proc/net/dev, lock contention profiling put
> 36% of the pool->lock wait in queue_work() called from cmd_exec(), and
> the rest in the workers running the command and stats work items.
> 
> Page-queue commands already call cmd_work_handler() directly. Do the
> same for every command without a callback. Callback commands keep using
> the workqueue, since mlx5_cmd_exec_cb() must not block.
> 
> Firmware runs up to max_reg_cmds commands concurrently, so callers could
> not rely on the submission order, and the slot semaphore still serves
> its waiters in FIFO order.
> 
> With 32 parallel ethtool -S loops on a ConnectX-4 Lx, this removes all
> 46.7k mlx5_cmd work items of the run and cuts kworker context switches
> from 50.7k to 3.9k.
> 
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
>   drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> index 84583dc5eb1c05..988518155582a4 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/cmd.c
> @@ -1301,7 +1301,8 @@ static int mlx5_cmd_invoke(struct mlx5_core_dev *dev, struct mlx5_cmd_msg *in,
>   
>   	INIT_DELAYED_WORK(&ent->cb_timeout_work, cb_timeout_handler);
>   	INIT_WORK(&ent->work, cmd_work_handler);
> -	if (page_queue) {
> +	/* A blocking caller waits for the command anyway, submit it inline */
> +	if (page_queue || !callback) {
>   		cmd_work_handler(&ent->work);
>   	} else if (!queue_work(cmd->wq, &ent->work)) {

Thanks for the RFC.
My main concern with the approach is fairness for callback commands.
They still go through the ordered cmd->wq, so only one of them at a
time waits on cmd->vars.sem, behind every blocking caller that now
queues there directly. A callback command queued before a burst of
blocking commands used to be submitted first. Now the whole burst can
be served ahead of it. I don't think we should go this way while
callback commands are left behind like that.


>   		mlx5_core_warn(dev, "failed to queue work\n");
> 

[PATCH RFC 2/2] net/mlx5e: Rate limit the HW stats refresh from ndo_get_stats64

[Breno Leitao]

mlx5e_get_stats() and mlx5e_rep_get_stats() return the counters cached
by the previous refresh and queue update_stats_work to refresh them in
the background. queue_work() only merges requests while the work is
still pending, so with several readers each netdev keeps refreshing,
and every refresh sends one to three firmware commands. A single read
of /proc/net/dev requests a refresh on every netdev, representors
included.

Until commit ed56c5193ad8 ("net/mlx5e: Update NIC HW stats on demand
only") the driver refreshed these counters every 200 msecs. Bring back
that interval as the minimum time between two on-demand refreshes, and
skip the requests that arrive sooner.

Readers already get the values of the previous refresh, so at worst
they are now up to 200 msecs older. The monitor counter event path
queues the work directly and is not rate limited.

In switchdev mode with 8 VFs, one /proc/net/dev reader at about 75 Hz
caused 13.4k firmware commands in 13 seconds, and 1.8k with this
change. With two readers the count drops from 27.3k to 1.6k.

Signed-off-by: Breno Leitao <leitao@debian.org>
---
 drivers/net/ethernet/mellanox/mlx5/core/en.h      | 2 ++
 drivers/net/ethernet/mellanox/mlx5/core/en_main.c | 9 +++++++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index a3945195f454e3..b66d40cdfdda53 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -151,6 +151,7 @@ struct page_pool;
 #define MLX5E_MAX_NUM_CHANNELS         256
 #define MLX5E_TX_CQ_POLL_BUDGET        128
 #define MLX5E_TX_XSK_POLL_BUDGET       64
+#define MLX5E_UPDATE_STATS_INTERVAL    200 /* msecs */
 #define MLX5E_SQ_RECOVER_MIN_INTERVAL  500 /* msecs */
 
 #define mlx5e_state_dereference(priv, p) \
@@ -928,6 +929,7 @@ struct mlx5e_priv {
 	struct work_struct         set_rx_mode_work;
 	struct work_struct         tx_timeout_work;
 	struct work_struct         update_stats_work;
+	unsigned long              update_stats_jiffies;
 	struct work_struct         monitor_counters_work;
 	struct mlx5_nb             monitor_counters_nb;
 
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index c3c2c5720c6f32..307505e0a827c6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -169,12 +169,21 @@ static void mlx5e_update_stats_work(struct work_struct *work)
 
 void mlx5e_queue_update_stats(struct mlx5e_priv *priv)
 {
+	unsigned long last;
+
 	if (!priv->profile->update_stats)
 		return;
 
 	if (unlikely(test_bit(MLX5E_STATE_DESTROYING, &priv->state)))
 		return;
 
+	/* Stats readers may poll often, limit the rate of FW queries */
+	last = READ_ONCE(priv->update_stats_jiffies);
+	if (time_in_range(jiffies, last,
+			  last + msecs_to_jiffies(MLX5E_UPDATE_STATS_INTERVAL)))
+		return;
+
+	WRITE_ONCE(priv->update_stats_jiffies, jiffies);
 	queue_work(priv->wq, &priv->update_stats_work);
 }
 

-- 
2.53.0-Meta