Re: [PATCH net V2 5/6] net/mlx5e: Fix deadlocks between devlink and netdev instance locks

[Cosmin Ratiu <cratiu@nvidia.com>]

On Thu, 2026-03-05 at 10:33 +0800, Jinjie Ruan wrote:
> 
> 
> On 2026/2/18 15:29, Tariq Toukan wrote:
> > From: Cosmin Ratiu <cratiu@nvidia.com>
> > 
> > In the mentioned "Fixes" commit, various work tasks triggering
> > devlink
> > health reporter recovery were switched to use netdev_trylock to
> > protect
> > against concurrent tear down of the channels being recovered. But
> > this
> > had the side effect of introducing potential deadlocks because of
> > incorrect lock ordering.
> > 
> > The correct lock order is described by the init flow:
> > probe_one -> mlx5_init_one (acquires devlink lock)
> > -> mlx5_init_one_devl_locked -> mlx5_register_device
> > -> mlx5_rescan_drivers_locked -...-> mlx5e_probe -> _mlx5e_probe
> > -> register_netdev (acquires rtnl lock)
> > -> register_netdevice (acquires netdev lock)
> > => devlink lock -> rtnl lock -> netdev lock.
> > 
> > But in the current recovery flow, the order is wrong:
> > mlx5e_tx_err_cqe_work (acquires netdev lock)
> > -> mlx5e_reporter_tx_err_cqe -> mlx5e_health_report
> > -> devlink_health_report (acquires devlink lock => boom!)
> > -> devlink_health_reporter_recover
> > -> mlx5e_tx_reporter_recover -> mlx5e_tx_reporter_recover_from_ctx
> > -> mlx5e_tx_reporter_err_cqe_recover
> > 
> > The same pattern exists in:
> > mlx5e_reporter_rx_timeout
> > mlx5e_reporter_tx_ptpsq_unhealthy
> > mlx5e_reporter_tx_timeout
> > 
> 
> On 7.0 rc2，It seems that similar problems still exist, causing the
> ARM64
> kernel to fail to start on the Kunpeng 920 as below:

Thank you for the report. From it, I can understand that it's something
else:
Task2 (mlx5_init_one_devl_locked -> ... -> mlx5_fw_tracer_init) is
holding the devlink lock and busy executing a firmware command for more
than 120 seconds. There should be no other locks held on that path.
Task1 (mlx5_fw_reporter_err_work -> devlink_health_report) is trying to
acquire the same devlink lock and is stuck on that.
Task3 (mlx5_init -> __pci_register_driver -> ...) is stuck waiting for
all devices to finish probing/registering (and is waiting for task2 to
finish).

I don't know what the problem is with task2 taking so long to execute a
fw command. In order to exclude potential deadlocks, is it possible to
boot up your kernel with CONFIG_LOCKDEP enabled?

Cosmin.

> 
> [  242.676635][ T1644] INFO: task kworker/u1280:1:1671 blocked for
> more
> than 120 seconds.
> [  242.682141][ T1644]       Not tainted 7.0.0-rc2+ #3
> [  242.684942][ T1644] "echo 0 >
> /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [  242.690552][ T1644] task:kworker/u1280:1 state:D stack:0    
> pid:1671
>  tgid:1671  ppid:2      task_flags:0x4208060 flags:0x00000010
> [  242.696332][ T1644] Workqueue: mlx5_health0000:97:00.0
> mlx5_fw_reporter_err_work [mlx5_core]
> [  242.702324][ T1644] Call trace:
> [  242.705187][ T1644]  __switch_to+0xdc/0x108 (T)
> [  242.707936][ T1644]  __schedule+0x2a0/0x8a8
> [  242.710647][ T1644]  schedule+0x3c/0xc0
> [  242.713321][ T1644]  schedule_preempt_disabled+0x2c/0x50
> [  242.715875][ T1644]  __mutex_lock.constprop.0+0x344/0x918
> [  242.718421][ T1644]  __mutex_lock_slowpath+0x1c/0x30
> [  242.720885][ T1644]  mutex_lock+0x50/0x68
> [  242.723278][ T1644]  devl_lock+0x1c/0x30
> [  242.725607][ T1644]  devlink_health_report+0x240/0x328
> [  242.727902][ T1644]  mlx5_fw_reporter_err_work+0xa0/0xb0
> [mlx5_core]
> [  242.730333][ T1644]  process_one_work+0x180/0x4f8
> [  242.732687][ T1644]  worker_thread+0x208/0x280
> [  242.734976][ T1644]  kthread+0x128/0x138
> [  242.737217][ T1644]  ret_from_fork+0x10/0x20
> [  242.739599][ T1644] INFO: task kworker/u1280:1:1671 is blocked on
> a
> mutex likely owned by task kworker/240:2:2582.
> [  242.744002][ T1644] task:kworker/240:2   state:D stack:0    
> pid:2582
>  tgid:2582  ppid:2      task_flags:0x4208060 flags:0x00000010
> [  242.748447][ T1644] Workqueue: sync_wq local_pci_probe_callback
> [  242.750654][ T1644] Call trace:
> [  242.752793][ T1644]  __switch_to+0xdc/0x108 (T)
> [  242.754882][ T1644]  __schedule+0x2a0/0x8a8
> [  242.756946][ T1644]  schedule+0x3c/0xc0
> [  242.758951][ T1644]  schedule_timeout+0x80/0x120
> [  242.760903][ T1644]  __wait_for_common+0xc4/0x1d0
> [  242.762796][ T1644]  wait_for_completion_timeout+0x28/0x40
> [  242.764670][ T1644]  wait_func+0x180/0x240 [mlx5_core]
> [  242.766533][ T1644]  mlx5_cmd_invoke+0x244/0x3e0 [mlx5_core]
> [  242.768338][ T1644]  cmd_exec+0x208/0x448 [mlx5_core]
> [  242.770153][ T1644]  mlx5_cmd_do+0x38/0x80 [mlx5_core]
> [  242.771974][ T1644]  mlx5_cmd_exec+0x2c/0x60 [mlx5_core]
> [  242.773848][ T1644]  mlx5_core_create_mkey+0x70/0x120 [mlx5_core]
> [  242.775712][ T1644]  mlx5_fw_tracer_create_mkey+0x114/0x180
> [mlx5_core]
> [  242.777609][ T1644]  mlx5_fw_tracer_init.part.0+0xb0/0x1f0
> [mlx5_core]
> [  242.779495][ T1644]  mlx5_fw_tracer_init+0x24/0x40 [mlx5_core]
> [  242.781380][ T1644]  mlx5_load+0x78/0x360 [mlx5_core]
> [  242.783256][ T1644]  mlx5_init_one_devl_locked+0xd0/0x278
> [mlx5_core]
> [  242.785231][ T1644]  probe_one+0xe0/0x208 [mlx5_core]
> [  242.787159][ T1644]  local_pci_probe+0x48/0xb8
> [  242.789038][ T1644]  local_pci_probe_callback+0x24/0x40
> [  242.790876][ T1644]  process_one_work+0x180/0x4f8
> [  242.792731][ T1644]  worker_thread+0x208/0x280
> [  242.794578][ T1644]  kthread+0x128/0x138
> [  242.796427][ T1644]  ret_from_fork+0x10/0x20
> [  242.798277][ T1644] INFO: task systemd-udevd:2281 blocked for more
> than 120 seconds.
> [  242.801795][ T1644]       Not tainted 7.0.0-rc2+ #3
> [  242.803542][ T1644] "echo 0 >
> /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> [  242.807056][ T1644] task:systemd-udevd   state:D stack:0    
> pid:2281
>  tgid:2281  ppid:2256   task_flags:0x400140 flags:0x00000811
> [  242.810829][ T1644] Call trace:
> [  242.812779][ T1644]  __switch_to+0xdc/0x108 (T)
> [  242.814681][ T1644]  __schedule+0x2a0/0x8a8
> [  242.816609][ T1644]  schedule+0x3c/0xc0
> [  242.818499][ T1644]  schedule_timeout+0x10c/0x120
> [  242.820388][ T1644]  __wait_for_common+0xc4/0x1d0
> [  242.822267][ T1644]  wait_for_completion+0x28/0x40
> [  242.824168][ T1644]  __flush_work+0x7c/0xf8
> [  242.825983][ T1644]  flush_work+0x1c/0x30
> [  242.827816][ T1644]  pci_call_probe+0x174/0x1e0
> [  242.829652][ T1644]  pci_device_probe+0x98/0x108
> [  242.831455][ T1644]  call_driver_probe+0x34/0x158
> [  242.833261][ T1644]  really_probe+0xc0/0x320
> [  242.835082][ T1644]  __driver_probe_device+0x88/0x190
> [  242.836843][ T1644]  driver_probe_device+0x48/0x120
> [  242.838607][ T1644]  __driver_attach+0x138/0x280
> [  242.840355][ T1644]  bus_for_each_dev+0x80/0xe8
> [  242.842095][ T1644]  driver_attach+0x2c/0x40
> [  242.843830][ T1644]  bus_add_driver+0x128/0x258
> [  242.845564][ T1644]  driver_register+0x68/0x138
> [  242.847285][ T1644]  __pci_register_driver+0x4c/0x60
> [  242.849038][ T1644]  mlx5_init+0x7c/0xff8 [mlx5_core]
> [  242.850871][ T1644]  do_one_initcall+0x50/0x498
> [  242.852561][ T1644]  do_init_module+0x60/0x280
> [  242.854220][ T1644]  load_module+0x3d8/0x6a8
> [  242.855856][ T1644]  init_module_from_file+0xe4/0x108
> [  242.857470][ T1644]  idempotent_init_module+0x190/0x290
> [  242.859022][ T1644]  __arm64_sys_finit_module+0x74/0xf8
> [  242.860544][ T1644]  invoke_syscall+0x50/0x120
> [  242.861996][ T1644]  el0_svc_common.constprop.0+0xc8/0xf0
> [  242.863457][ T1644]  do_el0_svc+0x24/0x38
> [  242.864914][ T1644]  el0_svc+0x34/0x170
> [  242.866361][ T1644]  el0t_64_sync_handler+0xa0/0xe8
> [  242.867839][ T1644]  el0t_64_sync+0x190/0x198
> 
> 
> > Fix these by moving the netdev_trylock calls from the work handlers
> > lower in the call stack, in the respective recovery functions,
> > where
> > they are actually necessary.
> > 
> > Fixes: 8f7b00307bf1 ("net/mlx5e: Convert mlx5 netdevs to instance
> > locking")
> > Signed-off-by: Cosmin Ratiu <cratiu@nvidia.com>
> > Reviewed-by: Dragos Tatulea <dtatulea@nvidia.com>
> > Signed-off-by: Tariq Toukan <tariqt@nvidia.com>
> > Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> > ---
> >  .../net/ethernet/mellanox/mlx5/core/en/ptp.c  | 14 -----
> >  .../mellanox/mlx5/core/en/reporter_rx.c       | 13 +++++
> >  .../mellanox/mlx5/core/en/reporter_tx.c       | 52
> > +++++++++++++++++--
> >  .../net/ethernet/mellanox/mlx5/core/en_main.c | 40 --------------
> >  4 files changed, 61 insertions(+), 58 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
> > index 424f8a2728a3..74660e7fe674 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/ptp.c
> > @@ -457,22 +457,8 @@ static void mlx5e_ptpsq_unhealthy_work(struct
> > work_struct *work)
> >  {
> >  	struct mlx5e_ptpsq *ptpsq =
> >  		container_of(work, struct mlx5e_ptpsq,
> > report_unhealthy_work);
> > -	struct mlx5e_txqsq *sq = &ptpsq->txqsq;
> > -
> > -	/* Recovering the PTP SQ means re-enabling NAPI, which
> > requires the
> > -	 * netdev instance lock. However, SQ closing has to wait
> > for this work
> > -	 * task to finish while also holding the same lock. So
> > either get the
> > -	 * lock or find that the SQ is no longer enabled and thus
> > this work is
> > -	 * not relevant anymore.
> > -	 */
> > -	while (!netdev_trylock(sq->netdev)) {
> > -		if (!test_bit(MLX5E_SQ_STATE_ENABLED, &sq->state))
> > -			return;
> > -		msleep(20);
> > -	}
> >  
> >  	mlx5e_reporter_tx_ptpsq_unhealthy(ptpsq);
> > -	netdev_unlock(sq->netdev);
> >  }
> >  
> >  static int mlx5e_ptp_open_txqsq(struct mlx5e_ptp *c, u32 tisn,
> > diff --git
> > a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
> > index 0686fbdd5a05..6efb626b5506 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c
> > @@ -1,6 +1,8 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >  // Copyright (c) 2019 Mellanox Technologies.
> >  
> > +#include <net/netdev_lock.h>
> > +
> >  #include "health.h"
> >  #include "params.h"
> >  #include "txrx.h"
> > @@ -177,6 +179,16 @@ static int
> > mlx5e_rx_reporter_timeout_recover(void *ctx)
> >  	rq = ctx;
> >  	priv = rq->priv;
> >  
> > +	/* Acquire netdev instance lock to synchronize with
> > channel close and
> > +	 * reopen flows. Either successfully obtain the lock, or
> > detect that
> > +	 * channels are closing for another reason, making this
> > work no longer
> > +	 * necessary.
> > +	 */
> > +	while (!netdev_trylock(rq->netdev)) {
> > +		if (!test_bit(MLX5E_STATE_CHANNELS_ACTIVE, &rq-
> > >priv->state))
> > +			return 0;
> > +		msleep(20);
> > +	}
> >  	mutex_lock(&priv->state_lock);
> >  
> >  	eq = rq->cq.mcq.eq;
> > @@ -186,6 +198,7 @@ static int
> > mlx5e_rx_reporter_timeout_recover(void *ctx)
> >  		clear_bit(MLX5E_SQ_STATE_ENABLED, &rq->icosq-
> > >state);
> >  
> >  	mutex_unlock(&priv->state_lock);
> > +	netdev_unlock(rq->netdev);
> >  
> >  	return err;
> >  }
> > diff --git
> > a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
> > index 4adc1adf9897..60ba840e00fa 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c
> > @@ -1,6 +1,8 @@
> >  /* SPDX-License-Identifier: GPL-2.0 */
> >  /* Copyright (c) 2019 Mellanox Technologies. */
> >  
> > +#include <net/netdev_lock.h>
> > +
> >  #include "health.h"
> >  #include "en/ptp.h"
> >  #include "en/devlink.h"
> > @@ -79,6 +81,18 @@ static int
> > mlx5e_tx_reporter_err_cqe_recover(void *ctx)
> >  	if (!test_bit(MLX5E_SQ_STATE_RECOVERING, &sq->state))
> >  		return 0;
> >  
> > +	/* Recovering queues means re-enabling NAPI, which
> > requires the netdev
> > +	 * instance lock. However, SQ closing flows have to wait
> > for work tasks
> > +	 * to finish while also holding the netdev instance lock.
> > So either get
> > +	 * the lock or find that the SQ is no longer enabled and
> > thus this work
> > +	 * is not relevant anymore.
> > +	 */
> > +	while (!netdev_trylock(dev)) {
> > +		if (!test_bit(MLX5E_SQ_STATE_ENABLED, &sq->state))
> > +			return 0;
> > +		msleep(20);
> > +	}
> > +
> >  	err = mlx5_core_query_sq_state(mdev, sq->sqn, &state);
> >  	if (err) {
> >  		netdev_err(dev, "Failed to query SQ 0x%x state.
> > err = %d\n",
> > @@ -114,9 +128,11 @@ static int
> > mlx5e_tx_reporter_err_cqe_recover(void *ctx)
> >  	else
> >  		mlx5e_trigger_napi_sched(sq->cq.napi);
> >  
> > +	netdev_unlock(dev);
> >  	return 0;
> >  out:
> >  	clear_bit(MLX5E_SQ_STATE_RECOVERING, &sq->state);
> > +	netdev_unlock(dev);
> >  	return err;
> >  }
> >  
> > @@ -137,10 +153,24 @@ static int
> > mlx5e_tx_reporter_timeout_recover(void *ctx)
> >  	sq = to_ctx->sq;
> >  	eq = sq->cq.mcq.eq;
> >  	priv = sq->priv;
> > +
> > +	/* Recovering the TX queues implies re-enabling NAPI,
> > which requires
> > +	 * the netdev instance lock.
> > +	 * However, channel closing flows have to wait for this
> > work to finish
> > +	 * while holding the same lock. So either get the lock or
> > find that
> > +	 * channels are being closed for other reason and this
> > work is not
> > +	 * relevant anymore.
> > +	 */
> > +	while (!netdev_trylock(sq->netdev)) {
> > +		if (!test_bit(MLX5E_STATE_CHANNELS_ACTIVE, &priv-
> > >state))
> > +			return 0;
> > +		msleep(20);
> > +	}
> > +
> >  	err = mlx5e_health_channel_eq_recover(sq->netdev, eq, sq-
> > >cq.ch_stats);
> >  	if (!err) {
> >  		to_ctx->status = 0; /* this sq recovered */
> > -		return err;
> > +		goto out;
> >  	}
> >  
> >  	mutex_lock(&priv->state_lock);
> > @@ -148,7 +178,7 @@ static int
> > mlx5e_tx_reporter_timeout_recover(void *ctx)
> >  	mutex_unlock(&priv->state_lock);
> >  	if (!err) {
> >  		to_ctx->status = 1; /* all channels recovered */
> > -		return err;
> > +		goto out;
> >  	}
> >  
> >  	to_ctx->status = err;
> > @@ -156,7 +186,8 @@ static int
> > mlx5e_tx_reporter_timeout_recover(void *ctx)
> >  	netdev_err(priv->netdev,
> >  		   "mlx5e_safe_reopen_channels failed recovering
> > from a tx_timeout, err(%d).\n",
> >  		   err);
> > -
> > +out:
> > +	netdev_unlock(sq->netdev);
> >  	return err;
> >  }
> >  
> > @@ -173,10 +204,22 @@ static int
> > mlx5e_tx_reporter_ptpsq_unhealthy_recover(void *ctx)
> >  		return 0;
> >  
> >  	priv = ptpsq->txqsq.priv;
> > +	netdev = priv->netdev;
> > +
> > +	/* Recovering the PTP SQ means re-enabling NAPI, which
> > requires the
> > +	 * netdev instance lock. However, SQ closing has to wait
> > for this work
> > +	 * task to finish while also holding the same lock. So
> > either get the
> > +	 * lock or find that the SQ is no longer enabled and thus
> > this work is
> > +	 * not relevant anymore.
> > +	 */
> > +	while (!netdev_trylock(netdev)) {
> > +		if (!test_bit(MLX5E_SQ_STATE_ENABLED, &ptpsq-
> > >txqsq.state))
> > +			return 0;
> > +		msleep(20);
> > +	}
> >  
> >  	mutex_lock(&priv->state_lock);
> >  	chs = &priv->channels;
> > -	netdev = priv->netdev;
> >  
> >  	carrier_ok = netif_carrier_ok(netdev);
> >  	netif_carrier_off(netdev);
> > @@ -193,6 +236,7 @@ static int
> > mlx5e_tx_reporter_ptpsq_unhealthy_recover(void *ctx)
> >  		netif_carrier_on(netdev);
> >  
> >  	mutex_unlock(&priv->state_lock);
> > +	netdev_unlock(netdev);
> >  
> >  	return err;
> >  }
> > diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > index 4b8084420816..73f4805feac7 100644
> > --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
> > @@ -631,19 +631,7 @@ static void mlx5e_rq_timeout_work(struct
> > work_struct *timeout_work)
> >  					   struct mlx5e_rq,
> >  					   rx_timeout_work);
> >  
> > -	/* Acquire netdev instance lock to synchronize with
> > channel close and
> > -	 * reopen flows. Either successfully obtain the lock, or
> > detect that
> > -	 * channels are closing for another reason, making this
> > work no longer
> > -	 * necessary.
> > -	 */
> > -	while (!netdev_trylock(rq->netdev)) {
> > -		if (!test_bit(MLX5E_STATE_CHANNELS_ACTIVE, &rq-
> > >priv->state))
> > -			return;
> > -		msleep(20);
> > -	}
> > -
> >  	mlx5e_reporter_rx_timeout(rq);
> > -	netdev_unlock(rq->netdev);
> >  }
> >  
> >  static int mlx5e_alloc_mpwqe_rq_drop_page(struct mlx5e_rq *rq)
> > @@ -1952,20 +1940,7 @@ void mlx5e_tx_err_cqe_work(struct
> > work_struct *recover_work)
> >  	struct mlx5e_txqsq *sq = container_of(recover_work, struct
> > mlx5e_txqsq,
> >  					      recover_work);
> >  
> > -	/* Recovering queues means re-enabling NAPI, which
> > requires the netdev
> > -	 * instance lock. However, SQ closing flows have to wait
> > for work tasks
> > -	 * to finish while also holding the netdev instance lock.
> > So either get
> > -	 * the lock or find that the SQ is no longer enabled and
> > thus this work
> > -	 * is not relevant anymore.
> > -	 */
> > -	while (!netdev_trylock(sq->netdev)) {
> > -		if (!test_bit(MLX5E_SQ_STATE_ENABLED, &sq->state))
> > -			return;
> > -		msleep(20);
> > -	}
> > -
> >  	mlx5e_reporter_tx_err_cqe(sq);
> > -	netdev_unlock(sq->netdev);
> >  }
> >  
> >  static struct dim_cq_moder mlx5e_get_def_tx_moderation(u8
> > cq_period_mode)
> > @@ -5105,19 +5080,6 @@ static void mlx5e_tx_timeout_work(struct
> > work_struct *work)
> >  	struct net_device *netdev = priv->netdev;
> >  	int i;
> >  
> > -	/* Recovering the TX queues implies re-enabling NAPI,
> > which requires
> > -	 * the netdev instance lock.
> > -	 * However, channel closing flows have to wait for this
> > work to finish
> > -	 * while holding the same lock. So either get the lock or
> > find that
> > -	 * channels are being closed for other reason and this
> > work is not
> > -	 * relevant anymore.
> > -	 */
> > -	while (!netdev_trylock(netdev)) {
> > -		if (!test_bit(MLX5E_STATE_CHANNELS_ACTIVE, &priv-
> > >state))
> > -			return;
> > -		msleep(20);
> > -	}
> > -
> >  	for (i = 0; i < netdev->real_num_tx_queues; i++) {
> >  		struct netdev_queue *dev_queue =
> >  			netdev_get_tx_queue(netdev, i);
> > @@ -5130,8 +5092,6 @@ static void mlx5e_tx_timeout_work(struct
> > work_struct *work)
> >  		/* break if tried to reopened channels */
> >  			break;
> >  	}
> > -
> > -	netdev_unlock(netdev);
> >  }
> >  
> >  static void mlx5e_tx_timeout(struct net_device *dev, unsigned int
> > txqueue)
>