On Wed, 2026-01-28 at 20:56 -0800, Jakub Kicinski wrote:

On Tue, 27 Jan 2026 10:52:39 +0200 Tariq Toukan wrote:

quoted

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.

quoted

 	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.

quoted

+			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.