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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <d52714243592921c08175aa742f32ae56e4f6651.camel@nvidia.com>
Date: Thu, 29 Jan 2026 10:33:40 +0000
From: Cosmin Ratiu <cratiu@...dia.com>
To: Tariq Toukan <tariqt@...dia.com>, "kuba@...nel.org" <kuba@...nel.org>
CC: "andrew+netdev@...n.ch" <andrew+netdev@...n.ch>,
	"linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
	"davem@...emloft.net" <davem@...emloft.net>, Gal Pressman <gal@...dia.com>,
	Moshe Shemesh <moshe@...dia.com>, Mark Bloch <mbloch@...dia.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"pabeni@...hat.com" <pabeni@...hat.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "edumazet@...gle.com" <edumazet@...gle.com>, Saeed
 Mahameed <saeedm@...dia.com>, "horms@...nel.org" <horms@...nel.org>,
	"leon@...nel.org" <leon@...nel.org>
Subject: Re: [PATCH net V2 2/4] net/mlx5: Fix deadlock between devlink lock
 and esw->wq

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.