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

[Moshe Shemesh <moshe@nvidia.com>]

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");
>