Skip to content

Commit aed763a

Browse files
Cosmin Ratiukuba-moo
authored andcommitted
net/mlx5: Fix deadlock between devlink lock and esw->wq
esw->work_queue executes esw_functions_changed_event_handler -> esw_vfs_changed_event_handler and acquires the devlink lock. .eswitch_mode_set (acquires devlink lock in devlink_nl_pre_doit) -> mlx5_devlink_eswitch_mode_set -> mlx5_eswitch_disable_locked -> mlx5_eswitch_event_handler_unregister -> flush_workqueue deadlocks when esw_vfs_changed_event_handler executes. Fix that by no longer flushing the work to avoid the deadlock, and using a generation counter to keep track of work relevance. This avoids an old handler manipulating an esw that has undergone one or more mode changes: - the counter is incremented in mlx5_eswitch_event_handler_unregister. - the counter is read and passed to the ephemeral mlx5_host_work struct. - the work handler takes the devlink lock and bails out if the current generation is different than the one it was scheduled to operate on. - mlx5_eswitch_cleanup does the final draining before destroying the wq. No longer flushing the workqueue has the side effect of maybe no longer cancelling pending vport_change_handler work items, but that's ok since those are disabled elsewhere: - mlx5_eswitch_disable_locked disables the vport eq notifier. - mlx5_esw_vport_disable disarms the HW EQ notification and marks vport->enabled under state_lock to false to prevent pending vport handler from doing anything. - mlx5_eswitch_cleanup destroys the workqueue and makes sure all events are disabled/finished. Fixes: f1bc646 ("net/mlx5: Use devl_ API in mlx5_esw_offloads_devlink_port_register") Signed-off-by: Cosmin Ratiu <[email protected]> Reviewed-by: Moshe Shemesh <[email protected]> Reviewed-by: Dragos Tatulea <[email protected]> Reviewed-by: Simon Horman <[email protected]> Signed-off-by: Tariq Toukan <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 55f854d commit aed763a

3 files changed

Lines changed: 19 additions & 8 deletions

File tree

drivers/net/ethernet/mellanox/mlx5/core/eswitch.c

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,10 +1072,11 @@ static void mlx5_eswitch_event_handler_register(struct mlx5_eswitch *esw)
10721072

10731073
static void mlx5_eswitch_event_handler_unregister(struct mlx5_eswitch *esw)
10741074
{
1075-
if (esw->mode == MLX5_ESWITCH_OFFLOADS && mlx5_eswitch_is_funcs_handler(esw->dev))
1075+
if (esw->mode == MLX5_ESWITCH_OFFLOADS &&
1076+
mlx5_eswitch_is_funcs_handler(esw->dev)) {
10761077
mlx5_eq_notifier_unregister(esw->dev, &esw->esw_funcs.nb);
1077-
1078-
flush_workqueue(esw->work_queue);
1078+
atomic_inc(&esw->esw_funcs.generation);
1079+
}
10791080
}
10801081

10811082
static void mlx5_eswitch_clear_vf_vports_info(struct mlx5_eswitch *esw)

drivers/net/ethernet/mellanox/mlx5/core/eswitch.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,10 +335,12 @@ struct esw_mc_addr { /* SRIOV only */
335335
struct mlx5_host_work {
336336
struct work_struct work;
337337
struct mlx5_eswitch *esw;
338+
int work_gen;
338339
};
339340

340341
struct mlx5_esw_functions {
341342
struct mlx5_nb nb;
343+
atomic_t generation;
342344
bool host_funcs_disabled;
343345
u16 num_vfs;
344346
u16 num_ec_vfs;

drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3582,22 +3582,28 @@ static void esw_offloads_steering_cleanup(struct mlx5_eswitch *esw)
35823582
}
35833583

35843584
static void
3585-
esw_vfs_changed_event_handler(struct mlx5_eswitch *esw, const u32 *out)
3585+
esw_vfs_changed_event_handler(struct mlx5_eswitch *esw, int work_gen,
3586+
const u32 *out)
35863587
{
35873588
struct devlink *devlink;
35883589
bool host_pf_disabled;
35893590
u16 new_num_vfs;
35903591

3592+
devlink = priv_to_devlink(esw->dev);
3593+
devl_lock(devlink);
3594+
3595+
/* Stale work from one or more mode changes ago. Bail out. */
3596+
if (work_gen != atomic_read(&esw->esw_funcs.generation))
3597+
goto unlock;
3598+
35913599
new_num_vfs = MLX5_GET(query_esw_functions_out, out,
35923600
host_params_context.host_num_of_vfs);
35933601
host_pf_disabled = MLX5_GET(query_esw_functions_out, out,
35943602
host_params_context.host_pf_disabled);
35953603

35963604
if (new_num_vfs == esw->esw_funcs.num_vfs || host_pf_disabled)
3597-
return;
3605+
goto unlock;
35983606

3599-
devlink = priv_to_devlink(esw->dev);
3600-
devl_lock(devlink);
36013607
/* Number of VFs can only change from "0 to x" or "x to 0". */
36023608
if (esw->esw_funcs.num_vfs > 0) {
36033609
mlx5_eswitch_unload_vf_vports(esw, esw->esw_funcs.num_vfs);
@@ -3612,6 +3618,7 @@ esw_vfs_changed_event_handler(struct mlx5_eswitch *esw, const u32 *out)
36123618
}
36133619
}
36143620
esw->esw_funcs.num_vfs = new_num_vfs;
3621+
unlock:
36153622
devl_unlock(devlink);
36163623
}
36173624

@@ -3628,7 +3635,7 @@ static void esw_functions_changed_event_handler(struct work_struct *work)
36283635
if (IS_ERR(out))
36293636
goto out;
36303637

3631-
esw_vfs_changed_event_handler(esw, out);
3638+
esw_vfs_changed_event_handler(esw, host_work->work_gen, out);
36323639
kvfree(out);
36333640
out:
36343641
kfree(host_work);
@@ -3648,6 +3655,7 @@ int mlx5_esw_funcs_changed_handler(struct notifier_block *nb, unsigned long type
36483655
esw = container_of(esw_funcs, struct mlx5_eswitch, esw_funcs);
36493656

36503657
host_work->esw = esw;
3658+
host_work->work_gen = atomic_read(&esw_funcs->generation);
36513659

36523660
INIT_WORK(&host_work->work, esw_functions_changed_event_handler);
36533661
queue_work(esw->work_queue, &host_work->work);

0 commit comments

Comments
 (0)