summaryrefslogtreecommitdiffstats
path: root/net/core
diff options
context:
space:
mode:
authorParav Pandit <parav@mellanox.com>2020-02-23 19:06:56 -0600
committerSaeed Mahameed <saeedm@mellanox.com>2020-03-25 23:19:18 -0700
commit98fed6eb9b17d4edb1d57b5f51c63b77686aaf1d (patch)
treea78b6e6c806f4b99e0f08d237ac8591e09cef22c /net/core
parentf999b706b7ab5dae45a3ee5c2b3bc2b47c11b0c5 (diff)
downloadlinux-98fed6eb9b17d4edb1d57b5f51c63b77686aaf1d.tar.bz2
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>
Diffstat (limited to 'net/core')
-rw-r--r--net/core/devlink.c3
1 files changed, 2 insertions, 1 deletions
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 73bb8fbe3393..a9036af7e002 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -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,