Re: [PATCH net V2 2/4] net/mlx5: Fix deadlock between devlink lock and esw->wq

[Cosmin Ratiu]

On Wed, 2026-01-28 at 20:56 -0800, Jakub Kicinski wrote:
> On Tue, 27 Jan 2026 10:52:39 +0200 Tariq Toukan wrote:
> > esw_functions_changed_event_handler ->
> > esw_vfs_changed_event_handler is
> > called from the esw->work_queue and acquires the devlink lock.
> > 
> > Changing the esw mode is done via .eswitch_mode_set (acquires
> > devlink
> > lock in the devlink_nl_pre_doit call) ->
> > mlx5_devlink_eswitch_mode_set
> > -> mlx5_eswitch_disable_locked ->
> > mlx5_eswitch_event_handler_unregister
> > -> flush_workqueue.  
> 
> This is quite an ugly hack, is there no way to avoid the flush and
> let 
> the work discover that what it was supposed to do is no longer
> needed?

Not possible, unfortunately. I stared at it for quite a while. The wq
is flushed because the esw is being unconfigured, which removes data
structs the work handler uses. Flushing the work is required, otherwise
we'll run into worse issues.

> 
> >  	devlink = priv_to_devlink(esw->dev);
> > -	devl_lock(devlink);
> > +	/* Repeatedly try to grab the lock with a delay while this
> > work is
> > +	 * still relevant.
> > +	 * This allows a concurrent
> > mlx5_eswitch_event_handler_unregister
> > +	 * (holding the devlink lock) to flush the wq without
> > deadlocking.
> > +	 */
> > +	while (!devl_trylock(devlink)) {
> > +		if (!esw->esw_funcs.notifier_enabled)
> 
> Technically READ_ONCE/WRITE_ONCE is required on this.

Will fix.


> > +			return;
> > +		schedule_timeout_interruptible(msecs_to_jiffies(10
> > ));
> 
> Why _interruptible(), you're not handling the return value.
> If somehow this thread gets a signal pending we'll turn this
> loop into a busy poll which doesn't seem ideal?


Didn't pay attention to this possibility. Sorry, will fix.

Cosmin.