mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-10-29 23:53:32 +00:00
devlink: Rely on driver eswitch thread safety instead of devlink
devlink_nl_cmd_eswitch_set_doit() doesn't hold devlink->lock mutex while invoking driver callback. This is likely due to eswitch mode setting involves adding/remove devlink ports, health reporters or other devlink objects for a devlink device. So it is driver responsiblity to ensure thread safe eswitch state transition happening via either sriov legacy enablement or via devlink eswitch set callback. Therefore, get() callback should also be invoked without holding devlink->lock mutex. Vendor driver can use same internal lock which it uses during eswitch mode set() callback. This makes get() and set() implimentation symmetric in devlink core and in vendor drivers. Hence, remove holding devlink->lock mutex during eswitch get() callback. Failing to do so results into below deadlock scenario when mlx5_core driver is improved to handle eswitch mode set critical section invoked by devlink and sriov sysfs interface in subsequent patch. devlink_nl_cmd_eswitch_set_doit() mlx5_eswitch_mode_set() mutex_lock(esw->mode_lock) <- Lock A [...] register_devlink_port() mutex_lock(&devlink->lock); <- lock B mutex_lock(&devlink->lock); <- lock B devlink_nl_cmd_eswitch_get_doit() mlx5_eswitch_mode_get() mutex_lock(esw->mode_lock) <- Lock A In subsequent patch, mlx5_core driver uses its internal lock during get() and set() eswitch callbacks. Other drivers have been inspected which returns either constant during get operations or reads the value from already allocated structure. Hence it is safe to remove the lock in get( ) callback and let vendor driver handle it. Reviewed-by: Jiri Pirko <jiri@mellanox.com> Reviewed-by: Mark Bloch <markb@mellanox.com> Signed-off-by: Parav Pandit <parav@mellanox.com> Signed-off-by: Saeed Mahameed <saeedm@mellanox.com>
This commit is contained in:
parent
f999b706b7
commit
98fed6eb9b
1 changed files with 2 additions and 1 deletions
|
@ -6187,7 +6187,8 @@ static const struct genl_ops devlink_nl_ops[] = {
|
|||
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
|
||||
.doit = devlink_nl_cmd_eswitch_get_doit,
|
||||
.flags = GENL_ADMIN_PERM,
|
||||
.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK,
|
||||
.internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK |
|
||||
DEVLINK_NL_FLAG_NO_LOCK,
|
||||
},
|
||||
{
|
||||
.cmd = DEVLINK_CMD_ESWITCH_SET,
|
||||
|
|
Loading…
Reference in a new issue