* [PATCH net-next 1/9] net/mlx5: Remove devl_unlock from mlx5_eswtich_mode_callback_enter
2022-07-11 8:13 [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Saeed Mahameed
@ 2022-07-11 8:14 ` Saeed Mahameed
2022-07-11 8:14 ` [PATCH net-next 2/9] net/mlx5: Use devl_ API for rate nodes destroy Saeed Mahameed
` (9 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Saeed Mahameed @ 2022-07-11 8:14 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
Cc: Saeed Mahameed, netdev, Moshe Shemesh, Leon Romanovsky
From: Moshe Shemesh <moshe@nvidia.com>
The function mlx5_eswtich_mode_callback_enter() was added as a temporary
workaround once devlink instance lock was added to devlink eswitch
callbacks. However, code review and testing show that all the callbacks
part to eswitch_mode_set don't take devlink instance lock in any flow
and so unlocking devlink instance lock while entering these functions is
not needed.
Remove devl_lock from mlx5_eswtich_mode_callback_enter() and devl_unlock
from mlx5_eswtich_mode_callback_exit(). Also remove the functions
mlx5_eswtich_mode_callback_enter()/exit() as they are not needed any
more. The callback eswitch_mode_set will be treated separately in the
following patches.
Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
.../mellanox/mlx5/core/eswitch_offloads.c | 43 +++++--------------
1 file changed, 11 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index e224ec7005a6..3bd843e6d66a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -3342,27 +3342,6 @@ static int esw_inline_mode_to_devlink(u8 mlx5_mode, u8 *mode)
return 0;
}
-/* FIXME: devl_unlock() followed by devl_lock() inside driver callback
- * is never correct and prone to races. It's a transitional workaround,
- * never repeat this pattern.
- *
- * This code MUST be fixed before removing devlink_mutex as it is safe
- * to do only because of that mutex.
- */
-static void mlx5_eswtich_mode_callback_enter(struct devlink *devlink,
- struct mlx5_eswitch *esw)
-{
- devl_unlock(devlink);
- down_write(&esw->mode_lock);
-}
-
-static void mlx5_eswtich_mode_callback_exit(struct devlink *devlink,
- struct mlx5_eswitch *esw)
-{
- up_write(&esw->mode_lock);
- devl_lock(devlink);
-}
-
int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
struct netlink_ext_ack *extack)
{
@@ -3431,9 +3410,9 @@ int mlx5_devlink_eswitch_mode_get(struct devlink *devlink, u16 *mode)
if (IS_ERR(esw))
return PTR_ERR(esw);
- mlx5_eswtich_mode_callback_enter(devlink, esw);
+ down_write(&esw->mode_lock);
err = esw_mode_to_devlink(esw->mode, mode);
- mlx5_eswtich_mode_callback_exit(devlink, esw);
+ up_write(&esw->mode_lock);
return err;
}
@@ -3480,7 +3459,7 @@ int mlx5_devlink_eswitch_inline_mode_set(struct devlink *devlink, u8 mode,
if (IS_ERR(esw))
return PTR_ERR(esw);
- mlx5_eswtich_mode_callback_enter(devlink, esw);
+ down_write(&esw->mode_lock);
switch (MLX5_CAP_ETH(dev, wqe_inline_mode)) {
case MLX5_CAP_INLINE_MODE_NOT_REQUIRED:
@@ -3514,11 +3493,11 @@ int mlx5_devlink_eswitch_inline_mode_set(struct devlink *devlink, u8 mode,
goto out;
esw->offloads.inline_mode = mlx5_mode;
- mlx5_eswtich_mode_callback_exit(devlink, esw);
+ up_write(&esw->mode_lock);
return 0;
out:
- mlx5_eswtich_mode_callback_exit(devlink, esw);
+ up_write(&esw->mode_lock);
return err;
}
@@ -3531,9 +3510,9 @@ int mlx5_devlink_eswitch_inline_mode_get(struct devlink *devlink, u8 *mode)
if (IS_ERR(esw))
return PTR_ERR(esw);
- mlx5_eswtich_mode_callback_enter(devlink, esw);
+ down_write(&esw->mode_lock);
err = esw_inline_mode_to_devlink(esw->offloads.inline_mode, mode);
- mlx5_eswtich_mode_callback_exit(devlink, esw);
+ up_write(&esw->mode_lock);
return err;
}
@@ -3549,7 +3528,7 @@ int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
if (IS_ERR(esw))
return PTR_ERR(esw);
- mlx5_eswtich_mode_callback_enter(devlink, esw);
+ down_write(&esw->mode_lock);
if (encap != DEVLINK_ESWITCH_ENCAP_MODE_NONE &&
(!MLX5_CAP_ESW_FLOWTABLE_FDB(dev, reformat) ||
@@ -3592,7 +3571,7 @@ int mlx5_devlink_eswitch_encap_mode_set(struct devlink *devlink,
}
unlock:
- mlx5_eswtich_mode_callback_exit(devlink, esw);
+ up_write(&esw->mode_lock);
return err;
}
@@ -3605,9 +3584,9 @@ int mlx5_devlink_eswitch_encap_mode_get(struct devlink *devlink,
if (IS_ERR(esw))
return PTR_ERR(esw);
- mlx5_eswtich_mode_callback_enter(devlink, esw);
+ down_write(&esw->mode_lock);
*encap = esw->offloads.encap;
- mlx5_eswtich_mode_callback_exit(devlink, esw);
+ up_write(&esw->mode_lock);
return 0;
}
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 2/9] net/mlx5: Use devl_ API for rate nodes destroy
2022-07-11 8:13 [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Saeed Mahameed
2022-07-11 8:14 ` [PATCH net-next 1/9] net/mlx5: Remove devl_unlock from mlx5_eswtich_mode_callback_enter Saeed Mahameed
@ 2022-07-11 8:14 ` Saeed Mahameed
2022-07-11 8:14 ` [PATCH net-next 3/9] devlink: Remove unused function devlink_rate_nodes_destroy Saeed Mahameed
` (8 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Saeed Mahameed @ 2022-07-11 8:14 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
Cc: Saeed Mahameed, netdev, Moshe Shemesh, Leon Romanovsky
From: Moshe Shemesh <moshe@nvidia.com>
Use devl_rate_nodes_destroy() instead of devlink_rate_nodes_destroy().
Add devlink instance lock in the driver paths to this function to have
it locked while calling devl_ API function.
This will be used by the downstream patch to invoke
mlx5_devlink_eswitch_mode_set() with devlink lock held.
Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 14 ++++++++++++--
.../ethernet/mellanox/mlx5/core/eswitch_offloads.c | 2 ++
2 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index b938632f89ff..571114e4878f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -1330,9 +1330,13 @@ int mlx5_eswitch_enable(struct mlx5_eswitch *esw, int num_vfs)
/* When disabling sriov, free driver level resources. */
void mlx5_eswitch_disable_sriov(struct mlx5_eswitch *esw, bool clear_vf)
{
+ struct devlink *devlink;
+
if (!mlx5_esw_allowed(esw))
return;
+ devlink = priv_to_devlink(esw->dev);
+ devl_lock(devlink);
down_write(&esw->mode_lock);
/* If driver is unloaded, this function is called twice by remove_one()
* and mlx5_unload(). Prevent the second call.
@@ -1354,13 +1358,14 @@ void mlx5_eswitch_disable_sriov(struct mlx5_eswitch *esw, bool clear_vf)
struct devlink *devlink = priv_to_devlink(esw->dev);
esw_offloads_del_send_to_vport_meta_rules(esw);
- devlink_rate_nodes_destroy(devlink);
+ devl_rate_nodes_destroy(devlink);
}
esw->esw_funcs.num_vfs = 0;
unlock:
up_write(&esw->mode_lock);
+ devl_unlock(devlink);
}
/* Free resources for corresponding eswitch mode. It is called by devlink
@@ -1389,18 +1394,23 @@ void mlx5_eswitch_disable_locked(struct mlx5_eswitch *esw)
mlx5_esw_acls_ns_cleanup(esw);
if (esw->mode == MLX5_ESWITCH_OFFLOADS)
- devlink_rate_nodes_destroy(devlink);
+ devl_rate_nodes_destroy(devlink);
}
void mlx5_eswitch_disable(struct mlx5_eswitch *esw)
{
+ struct devlink *devlink;
+
if (!mlx5_esw_allowed(esw))
return;
mlx5_lag_disable_change(esw->dev);
+ devlink = priv_to_devlink(esw->dev);
+ devl_lock(devlink);
down_write(&esw->mode_lock);
mlx5_eswitch_disable_locked(esw);
up_write(&esw->mode_lock);
+ devl_unlock(devlink);
mlx5_lag_enable_change(esw->dev);
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 3bd843e6d66a..f1640e4cb719 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -3377,7 +3377,9 @@ int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
if (cur_mlx5_mode == mlx5_mode)
goto unlock;
+ devl_lock(devlink);
mlx5_eswitch_disable_locked(esw);
+ devl_unlock(devlink);
if (mode == DEVLINK_ESWITCH_MODE_SWITCHDEV) {
if (mlx5_devlink_trap_get_num_active(esw->dev)) {
NL_SET_ERR_MSG_MOD(extack,
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 3/9] devlink: Remove unused function devlink_rate_nodes_destroy
2022-07-11 8:13 [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Saeed Mahameed
2022-07-11 8:14 ` [PATCH net-next 1/9] net/mlx5: Remove devl_unlock from mlx5_eswtich_mode_callback_enter Saeed Mahameed
2022-07-11 8:14 ` [PATCH net-next 2/9] net/mlx5: Use devl_ API for rate nodes destroy Saeed Mahameed
@ 2022-07-11 8:14 ` Saeed Mahameed
2022-07-11 8:14 ` [PATCH net-next 4/9] net/mlx5: Use devl_ API in mlx5_esw_offloads_devlink_port_register Saeed Mahameed
` (7 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Saeed Mahameed @ 2022-07-11 8:14 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
Cc: Saeed Mahameed, netdev, Moshe Shemesh, Leon Romanovsky
From: Moshe Shemesh <moshe@nvidia.com>
The previous patch removed the last usage of the function
devlink_rate_nodes_destroy(). Thus, remove this function from devlink
API.
Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
include/net/devlink.h | 1 -
net/core/devlink.c | 18 ------------------
2 files changed, 19 deletions(-)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 2a2a2a0c93f7..0e163cc87d45 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1571,7 +1571,6 @@ void devlink_port_attrs_pci_sf_set(struct devlink_port *devlink_port,
bool external);
int devlink_rate_leaf_create(struct devlink_port *port, void *priv);
void devlink_rate_leaf_destroy(struct devlink_port *devlink_port);
-void devlink_rate_nodes_destroy(struct devlink *devlink);
void devlink_port_linecard_set(struct devlink_port *devlink_port,
struct devlink_linecard *linecard);
struct devlink_linecard *
diff --git a/net/core/devlink.c b/net/core/devlink.c
index db61f3a341cb..1588e2246234 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -10095,24 +10095,6 @@ void devl_rate_nodes_destroy(struct devlink *devlink)
}
EXPORT_SYMBOL_GPL(devl_rate_nodes_destroy);
-/**
- * devlink_rate_nodes_destroy - destroy all devlink rate nodes on device
- *
- * @devlink: devlink instance
- *
- * Unset parent for all rate objects and destroy all rate nodes
- * on specified device.
- *
- * Context: Takes and release devlink->lock <mutex>.
- */
-void devlink_rate_nodes_destroy(struct devlink *devlink)
-{
- mutex_lock(&devlink->lock);
- devl_rate_nodes_destroy(devlink);
- mutex_unlock(&devlink->lock);
-}
-EXPORT_SYMBOL_GPL(devlink_rate_nodes_destroy);
-
/**
* devlink_port_linecard_set - Link port with a linecard
*
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 4/9] net/mlx5: Use devl_ API in mlx5_esw_offloads_devlink_port_register
2022-07-11 8:13 [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Saeed Mahameed
` (2 preceding siblings ...)
2022-07-11 8:14 ` [PATCH net-next 3/9] devlink: Remove unused function devlink_rate_nodes_destroy Saeed Mahameed
@ 2022-07-11 8:14 ` Saeed Mahameed
2022-07-11 8:14 ` [PATCH net-next 5/9] net/mlx5: Use devl_ API in mlx5_esw_devlink_sf_port_register Saeed Mahameed
` (6 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Saeed Mahameed @ 2022-07-11 8:14 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
Cc: Saeed Mahameed, netdev, Moshe Shemesh, Leon Romanovsky
From: Moshe Shemesh <moshe@nvidia.com>
The function mlx5_esw_offloads_devlink_port_register() calls
devlink_port_register() and devlink_rate_leaf_create(). Use devl_ API to
call devl_port_register() and devl_rate_leaf_create() accordingly and
add devlink instance lock in driver paths to this function.
Similarly, use devl_ API to call devl_port_unregister() and
devl_rate_leaf_destroy() in mlx5_esw_offloads_devlink_port_unregister()
and ensure locking devlink instance lock on the paths to this function
too.
This will be used by the downstream patch to invoke
mlx5_devlink_eswitch_mode_set() with devlink lock held.
Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/esw/devlink_port.c | 10 +++++-----
drivers/net/ethernet/mellanox/mlx5/core/eswitch.c | 4 ++++
.../net/ethernet/mellanox/mlx5/core/eswitch_offloads.c | 10 ++++++++++
3 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
index 7f9b96d9537e..a8f7618831f5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
@@ -87,11 +87,11 @@ int mlx5_esw_offloads_devlink_port_register(struct mlx5_eswitch *esw, u16 vport_
devlink = priv_to_devlink(dev);
dl_port_index = mlx5_esw_vport_to_devlink_port_index(dev, vport_num);
- err = devlink_port_register(devlink, dl_port, dl_port_index);
+ err = devl_port_register(devlink, dl_port, dl_port_index);
if (err)
goto reg_err;
- err = devlink_rate_leaf_create(dl_port, vport);
+ err = devl_rate_leaf_create(dl_port, vport);
if (err)
goto rate_err;
@@ -99,7 +99,7 @@ int mlx5_esw_offloads_devlink_port_register(struct mlx5_eswitch *esw, u16 vport_
return 0;
rate_err:
- devlink_port_unregister(dl_port);
+ devl_port_unregister(dl_port);
reg_err:
mlx5_esw_dl_port_free(dl_port);
return err;
@@ -118,10 +118,10 @@ void mlx5_esw_offloads_devlink_port_unregister(struct mlx5_eswitch *esw, u16 vpo
if (vport->dl_port->devlink_rate) {
mlx5_esw_qos_vport_update_group(esw, vport, NULL, NULL);
- devlink_rate_leaf_destroy(vport->dl_port);
+ devl_rate_leaf_destroy(vport->dl_port);
}
- devlink_port_unregister(vport->dl_port);
+ devl_port_unregister(vport->dl_port);
mlx5_esw_dl_port_free(vport->dl_port);
vport->dl_port = NULL;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
index 571114e4878f..b95f75431882 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch.c
@@ -1296,6 +1296,7 @@ int mlx5_eswitch_enable_locked(struct mlx5_eswitch *esw, int num_vfs)
*/
int mlx5_eswitch_enable(struct mlx5_eswitch *esw, int num_vfs)
{
+ struct devlink *devlink;
bool toggle_lag;
int ret;
@@ -1307,6 +1308,8 @@ int mlx5_eswitch_enable(struct mlx5_eswitch *esw, int num_vfs)
if (toggle_lag)
mlx5_lag_disable_change(esw->dev);
+ devlink = priv_to_devlink(esw->dev);
+ devl_lock(devlink);
down_write(&esw->mode_lock);
if (!mlx5_esw_is_fdb_created(esw)) {
ret = mlx5_eswitch_enable_locked(esw, num_vfs);
@@ -1320,6 +1323,7 @@ int mlx5_eswitch_enable(struct mlx5_eswitch *esw, int num_vfs)
esw->esw_funcs.num_vfs = num_vfs;
}
up_write(&esw->mode_lock);
+ devl_unlock(devlink);
if (toggle_lag)
mlx5_lag_enable_change(esw->dev);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index f1640e4cb719..1bfbc88f513f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -2177,8 +2177,10 @@ static int esw_create_restore_table(struct mlx5_eswitch *esw)
static int esw_offloads_start(struct mlx5_eswitch *esw,
struct netlink_ext_ack *extack)
{
+ struct devlink *devlink = priv_to_devlink(esw->dev);
int err, err1;
+ devl_lock(devlink);
esw->mode = MLX5_ESWITCH_OFFLOADS;
err = mlx5_eswitch_enable_locked(esw, esw->dev->priv.sriov.num_vfs);
if (err) {
@@ -2200,6 +2202,7 @@ static int esw_offloads_start(struct mlx5_eswitch *esw,
"Inline mode is different between vports");
}
}
+ devl_unlock(devlink);
return err;
}
@@ -3064,6 +3067,7 @@ static void esw_offloads_steering_cleanup(struct mlx5_eswitch *esw)
static void
esw_vfs_changed_event_handler(struct mlx5_eswitch *esw, const u32 *out)
{
+ struct devlink *devlink;
bool host_pf_disabled;
u16 new_num_vfs;
@@ -3075,6 +3079,8 @@ esw_vfs_changed_event_handler(struct mlx5_eswitch *esw, const u32 *out)
if (new_num_vfs == esw->esw_funcs.num_vfs || host_pf_disabled)
return;
+ devlink = priv_to_devlink(esw->dev);
+ devl_lock(devlink);
/* Number of VFs can only change from "0 to x" or "x to 0". */
if (esw->esw_funcs.num_vfs > 0) {
mlx5_eswitch_unload_vf_vports(esw, esw->esw_funcs.num_vfs);
@@ -3087,6 +3093,7 @@ esw_vfs_changed_event_handler(struct mlx5_eswitch *esw, const u32 *out)
return;
}
esw->esw_funcs.num_vfs = new_num_vfs;
+ devl_unlock(devlink);
}
static void esw_functions_changed_event_handler(struct work_struct *work)
@@ -3236,8 +3243,10 @@ int esw_offloads_enable(struct mlx5_eswitch *esw)
static int esw_offloads_stop(struct mlx5_eswitch *esw,
struct netlink_ext_ack *extack)
{
+ struct devlink *devlink = priv_to_devlink(esw->dev);
int err, err1;
+ devl_lock(devlink);
esw->mode = MLX5_ESWITCH_LEGACY;
err = mlx5_eswitch_enable_locked(esw, MLX5_ESWITCH_IGNORE_NUM_VFS);
if (err) {
@@ -3249,6 +3258,7 @@ static int esw_offloads_stop(struct mlx5_eswitch *esw,
"Failed setting eswitch back to offloads");
}
}
+ devl_unlock(devlink);
return err;
}
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 5/9] net/mlx5: Use devl_ API in mlx5_esw_devlink_sf_port_register
2022-07-11 8:13 [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Saeed Mahameed
` (3 preceding siblings ...)
2022-07-11 8:14 ` [PATCH net-next 4/9] net/mlx5: Use devl_ API in mlx5_esw_offloads_devlink_port_register Saeed Mahameed
@ 2022-07-11 8:14 ` Saeed Mahameed
2022-07-11 8:14 ` [PATCH net-next 6/9] devlink: Remove unused functions devlink_rate_leaf_create/destroy Saeed Mahameed
` (5 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Saeed Mahameed @ 2022-07-11 8:14 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
Cc: Saeed Mahameed, netdev, Moshe Shemesh, Leon Romanovsky
From: Moshe Shemesh <moshe@nvidia.com>
The function mlx5_esw_devlink_sf_port_register() calls
devlink_port_register() and devlink_rate_leaf_create(). Use devl_ API to
call devl_port_register() and devl_rate_leaf_create() accordingly and
add devlink instance lock in driver paths to this function.
Similarly, use devl_ API to call devl_port_unregister() and
devl_rate_leaf_destroy() in mlx5_esw_devlink_sf_port_unregister() and
ensure locking devlink instance lock on all the paths to this function
too.
This will be used by the downstream patch to invoke
mlx5_devlink_eswitch_mode_set() with devlink lock held.
Note this patch is taking devlink lock on mlx5_devlink_sf_port_new/del()
which are devlink callbacks for port_new/del(). We will take these locks
off once these callbacks will be locked by devlink too.
Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/esw/devlink_port.c | 10 +++++-----
drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c | 4 ++++
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
index a8f7618831f5..9bc7be95db54 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
@@ -156,11 +156,11 @@ int mlx5_esw_devlink_sf_port_register(struct mlx5_eswitch *esw, struct devlink_p
devlink_port_attrs_pci_sf_set(dl_port, controller, pfnum, sfnum, !!controller);
devlink = priv_to_devlink(dev);
dl_port_index = mlx5_esw_vport_to_devlink_port_index(dev, vport_num);
- err = devlink_port_register(devlink, dl_port, dl_port_index);
+ err = devl_port_register(devlink, dl_port, dl_port_index);
if (err)
return err;
- err = devlink_rate_leaf_create(dl_port, vport);
+ err = devl_rate_leaf_create(dl_port, vport);
if (err)
goto rate_err;
@@ -168,7 +168,7 @@ int mlx5_esw_devlink_sf_port_register(struct mlx5_eswitch *esw, struct devlink_p
return 0;
rate_err:
- devlink_port_unregister(dl_port);
+ devl_port_unregister(dl_port);
return err;
}
@@ -182,9 +182,9 @@ void mlx5_esw_devlink_sf_port_unregister(struct mlx5_eswitch *esw, u16 vport_num
if (vport->dl_port->devlink_rate) {
mlx5_esw_qos_vport_update_group(esw, vport, NULL, NULL);
- devlink_rate_leaf_destroy(vport->dl_port);
+ devl_rate_leaf_destroy(vport->dl_port);
}
- devlink_port_unregister(vport->dl_port);
+ devl_port_unregister(vport->dl_port);
vport->dl_port = NULL;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c
index 7d955a4d9f14..2068c22ff367 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c
@@ -355,7 +355,9 @@ int mlx5_devlink_sf_port_new(struct devlink *devlink,
"Port add is only supported in eswitch switchdev mode or SF ports are disabled.");
return -EOPNOTSUPP;
}
+ devl_lock(devlink);
err = mlx5_sf_add(dev, table, new_attr, extack, new_port_index);
+ devl_unlock(devlink);
mlx5_sf_table_put(table);
return err;
}
@@ -400,7 +402,9 @@ int mlx5_devlink_sf_port_del(struct devlink *devlink, unsigned int port_index,
goto sf_err;
}
+ devl_lock(devlink);
mlx5_esw_offloads_sf_vport_disable(esw, sf->hw_fn_id);
+ devl_unlock(devlink);
mlx5_sf_id_erase(table, sf);
mutex_lock(&table->sf_state_lock);
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 6/9] devlink: Remove unused functions devlink_rate_leaf_create/destroy
2022-07-11 8:13 [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Saeed Mahameed
` (4 preceding siblings ...)
2022-07-11 8:14 ` [PATCH net-next 5/9] net/mlx5: Use devl_ API in mlx5_esw_devlink_sf_port_register Saeed Mahameed
@ 2022-07-11 8:14 ` Saeed Mahameed
2022-07-11 8:14 ` [PATCH net-next 7/9] net/mlx5: Use devl_ API in mlx5e_devlink_port_register Saeed Mahameed
` (4 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Saeed Mahameed @ 2022-07-11 8:14 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
Cc: Saeed Mahameed, netdev, Moshe Shemesh, Leon Romanovsky
From: Moshe Shemesh <moshe@nvidia.com>
The previous patch removed the last usage of the functions
devlink_rate_leaf_create() and devlink_rate_nodes_destroy(). Thus,
remove these function from devlink API.
Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
include/net/devlink.h | 2 --
net/core/devlink.c | 42 +++++++-----------------------------------
2 files changed, 7 insertions(+), 37 deletions(-)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 0e163cc87d45..5150deb67fab 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1569,8 +1569,6 @@ void devlink_port_attrs_pci_vf_set(struct devlink_port *devlink_port, u32 contro
void devlink_port_attrs_pci_sf_set(struct devlink_port *devlink_port,
u32 controller, u16 pf, u32 sf,
bool external);
-int devlink_rate_leaf_create(struct devlink_port *port, void *priv);
-void devlink_rate_leaf_destroy(struct devlink_port *devlink_port);
void devlink_port_linecard_set(struct devlink_port *devlink_port,
struct devlink_linecard *linecard);
struct devlink_linecard *
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 1588e2246234..970e5c2a52bd 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -10006,20 +10006,13 @@ int devl_rate_leaf_create(struct devlink_port *devlink_port, void *priv)
}
EXPORT_SYMBOL_GPL(devl_rate_leaf_create);
-int
-devlink_rate_leaf_create(struct devlink_port *devlink_port, void *priv)
-{
- struct devlink *devlink = devlink_port->devlink;
- int ret;
-
- mutex_lock(&devlink->lock);
- ret = devl_rate_leaf_create(devlink_port, priv);
- mutex_unlock(&devlink->lock);
-
- return ret;
-}
-EXPORT_SYMBOL_GPL(devlink_rate_leaf_create);
-
+/**
+ * devl_rate_leaf_destroy - destroy devlink rate leaf
+ *
+ * @devlink_port: devlink port linked to the rate object
+ *
+ * Destroy the devlink rate object of type leaf on provided @devlink_port.
+ */
void devl_rate_leaf_destroy(struct devlink_port *devlink_port)
{
struct devlink_rate *devlink_rate = devlink_port->devlink_rate;
@@ -10037,27 +10030,6 @@ void devl_rate_leaf_destroy(struct devlink_port *devlink_port)
}
EXPORT_SYMBOL_GPL(devl_rate_leaf_destroy);
-/**
- * devlink_rate_leaf_destroy - destroy devlink rate leaf
- *
- * @devlink_port: devlink port linked to the rate object
- *
- * Context: Takes and release devlink->lock <mutex>.
- */
-void devlink_rate_leaf_destroy(struct devlink_port *devlink_port)
-{
- struct devlink_rate *devlink_rate = devlink_port->devlink_rate;
- struct devlink *devlink = devlink_port->devlink;
-
- if (!devlink_rate)
- return;
-
- mutex_lock(&devlink->lock);
- devl_rate_leaf_destroy(devlink_port);
- mutex_unlock(&devlink->lock);
-}
-EXPORT_SYMBOL_GPL(devlink_rate_leaf_destroy);
-
/**
* devl_rate_nodes_destroy - destroy all devlink rate nodes on device
* @devlink: devlink instance
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 7/9] net/mlx5: Use devl_ API in mlx5e_devlink_port_register
2022-07-11 8:13 [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Saeed Mahameed
` (5 preceding siblings ...)
2022-07-11 8:14 ` [PATCH net-next 6/9] devlink: Remove unused functions devlink_rate_leaf_create/destroy Saeed Mahameed
@ 2022-07-11 8:14 ` Saeed Mahameed
2022-07-11 8:14 ` [PATCH net-next 8/9] net/mlx5: Remove devl_unlock from mlx5_devlink_eswitch_mode_set Saeed Mahameed
` (3 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Saeed Mahameed @ 2022-07-11 8:14 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
Cc: Saeed Mahameed, netdev, Moshe Shemesh
From: Moshe Shemesh <moshe@nvidia.com>
As part of the flows invoked by mlx5_devlink_eswitch_mode_set() get to
mlx5_rescan_drivers_locked() which can call mlx5e_probe()/mlx5e_remove
and register/unregister mlx5e driver ports accordingly. This can lead to
deadlock once mlx5_devlink_eswitch_mode_set() will use devlink lock.
Use devl_port_register/unregister() instead of
devlink_port_register/unregister() and add devlink instance locks in the
driver paths to this function to have it locked while calling devl_ API
function.
If remove or probe were called by module init or module cleanup flows,
need to lock devlink just before calling devl_port_register(), otherwise
it is called by attach/detach or register/unregister flow and we can
have the flow locked. Added flag to distinguish between these cases.
This will be used by the downstream patch to invoke
mlx5_devlink_eswitch_mode_set() with devlink locked.
Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/dev.c | 29 +++++++++++++++++--
.../ethernet/mellanox/mlx5/core/en/devlink.c | 16 ++++++++--
.../mellanox/mlx5/core/eswitch_offloads.c | 2 ++
include/linux/mlx5/driver.h | 4 +++
4 files changed, 47 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dev.c b/drivers/net/ethernet/mellanox/mlx5/core/dev.c
index 50422b56a64d..ccf2068d2e79 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/dev.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/dev.c
@@ -335,13 +335,16 @@ static void del_adev(struct auxiliary_device *adev)
int mlx5_attach_device(struct mlx5_core_dev *dev)
{
+ struct devlink *devlink = priv_to_devlink(dev);
struct mlx5_priv *priv = &dev->priv;
struct auxiliary_device *adev;
struct auxiliary_driver *adrv;
int ret = 0, i;
+ devl_lock(devlink);
mutex_lock(&mlx5_intf_mutex);
priv->flags &= ~MLX5_PRIV_FLAGS_DETACH;
+ priv->flags |= MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW;
for (i = 0; i < ARRAY_SIZE(mlx5_adev_devices); i++) {
if (!priv->adev[i]) {
bool is_supported = false;
@@ -389,19 +392,24 @@ int mlx5_attach_device(struct mlx5_core_dev *dev)
break;
}
}
+ priv->flags &= ~MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW;
mutex_unlock(&mlx5_intf_mutex);
+ devl_unlock(devlink);
return ret;
}
void mlx5_detach_device(struct mlx5_core_dev *dev)
{
+ struct devlink *devlink = priv_to_devlink(dev);
struct mlx5_priv *priv = &dev->priv;
struct auxiliary_device *adev;
struct auxiliary_driver *adrv;
pm_message_t pm = {};
int i;
+ devl_lock(devlink);
mutex_lock(&mlx5_intf_mutex);
+ priv->flags |= MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW;
for (i = ARRAY_SIZE(mlx5_adev_devices) - 1; i >= 0; i--) {
if (!priv->adev[i])
continue;
@@ -430,18 +438,24 @@ void mlx5_detach_device(struct mlx5_core_dev *dev)
del_adev(&priv->adev[i]->adev);
priv->adev[i] = NULL;
}
+ priv->flags &= ~MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW;
priv->flags |= MLX5_PRIV_FLAGS_DETACH;
mutex_unlock(&mlx5_intf_mutex);
+ devl_unlock(devlink);
}
int mlx5_register_device(struct mlx5_core_dev *dev)
{
+ struct devlink *devlink;
int ret;
+ devlink = priv_to_devlink(dev);
+ devl_lock(devlink);
mutex_lock(&mlx5_intf_mutex);
dev->priv.flags &= ~MLX5_PRIV_FLAGS_DISABLE_ALL_ADEV;
ret = mlx5_rescan_drivers_locked(dev);
mutex_unlock(&mlx5_intf_mutex);
+ devl_unlock(devlink);
if (ret)
mlx5_unregister_device(dev);
@@ -450,10 +464,15 @@ int mlx5_register_device(struct mlx5_core_dev *dev)
void mlx5_unregister_device(struct mlx5_core_dev *dev)
{
+ struct devlink *devlink;
+
+ devlink = priv_to_devlink(dev);
+ devl_lock(devlink);
mutex_lock(&mlx5_intf_mutex);
dev->priv.flags = MLX5_PRIV_FLAGS_DISABLE_ALL_ADEV;
mlx5_rescan_drivers_locked(dev);
mutex_unlock(&mlx5_intf_mutex);
+ devl_unlock(devlink);
}
static int add_drivers(struct mlx5_core_dev *dev)
@@ -526,16 +545,22 @@ static void delete_drivers(struct mlx5_core_dev *dev)
int mlx5_rescan_drivers_locked(struct mlx5_core_dev *dev)
{
struct mlx5_priv *priv = &dev->priv;
+ int err = 0;
lockdep_assert_held(&mlx5_intf_mutex);
if (priv->flags & MLX5_PRIV_FLAGS_DETACH)
return 0;
+ priv->flags |= MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW;
delete_drivers(dev);
if (priv->flags & MLX5_PRIV_FLAGS_DISABLE_ALL_ADEV)
- return 0;
+ goto out;
+
+ err = add_drivers(dev);
- return add_drivers(dev);
+out:
+ priv->flags &= ~MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW;
+ return err;
}
bool mlx5_same_hw_devs(struct mlx5_core_dev *dev, struct mlx5_core_dev *peer_dev)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
index ae52e7f38306..b69f9d10ccbd 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
@@ -21,6 +21,7 @@ int mlx5e_devlink_port_register(struct mlx5e_priv *priv)
struct netdev_phys_item_id ppid = {};
struct devlink_port *dl_port;
unsigned int dl_port_index;
+ int ret;
if (mlx5_core_is_pf(priv->mdev)) {
attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
@@ -41,7 +42,13 @@ int mlx5e_devlink_port_register(struct mlx5e_priv *priv)
memset(dl_port, 0, sizeof(*dl_port));
devlink_port_attrs_set(dl_port, &attrs);
- return devlink_port_register(devlink, dl_port, dl_port_index);
+ if (!(priv->mdev->priv.flags & MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW))
+ devl_lock(devlink);
+ ret = devl_port_register(devlink, dl_port, dl_port_index);
+ if (!(priv->mdev->priv.flags & MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW))
+ devl_unlock(devlink);
+
+ return ret;
}
void mlx5e_devlink_port_type_eth_set(struct mlx5e_priv *priv)
@@ -54,8 +61,13 @@ void mlx5e_devlink_port_type_eth_set(struct mlx5e_priv *priv)
void mlx5e_devlink_port_unregister(struct mlx5e_priv *priv)
{
struct devlink_port *dl_port = mlx5e_devlink_get_dl_port(priv);
+ struct devlink *devlink = priv_to_devlink(priv->mdev);
- devlink_port_unregister(dl_port);
+ if (!(priv->mdev->priv.flags & MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW))
+ devl_lock(devlink);
+ devl_port_unregister(dl_port);
+ if (!(priv->mdev->priv.flags & MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW))
+ devl_unlock(devlink);
}
struct devlink_port *mlx5e_get_devlink_port(struct net_device *dev)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index 1bfbc88f513f..ccda3a0a2594 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -3400,7 +3400,9 @@ int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
err = esw_offloads_start(esw, extack);
} else if (mode == DEVLINK_ESWITCH_MODE_LEGACY) {
err = esw_offloads_stop(esw, extack);
+ devl_lock(devlink);
mlx5_rescan_drivers(esw->dev);
+ devl_unlock(devlink);
} else {
err = -EINVAL;
}
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 76d7661e3e63..bd882884b23c 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -551,6 +551,10 @@ enum {
* creation/deletion on drivers rescan. Unset during device attach.
*/
MLX5_PRIV_FLAGS_DETACH = 1 << 2,
+ /* Distinguish between mlx5e_probe/remove called by module init/cleanup
+ * and called by other flows which can already hold devlink lock
+ */
+ MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW = 1 << 3,
};
struct mlx5_adev {
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 8/9] net/mlx5: Remove devl_unlock from mlx5_devlink_eswitch_mode_set
2022-07-11 8:13 [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Saeed Mahameed
` (6 preceding siblings ...)
2022-07-11 8:14 ` [PATCH net-next 7/9] net/mlx5: Use devl_ API in mlx5e_devlink_port_register Saeed Mahameed
@ 2022-07-11 8:14 ` Saeed Mahameed
2022-07-11 8:14 ` [PATCH net-next 9/9] devlink: Hold the instance lock in port_new / port_del callbacks Saeed Mahameed
` (2 subsequent siblings)
10 siblings, 0 replies; 12+ messages in thread
From: Saeed Mahameed @ 2022-07-11 8:14 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
Cc: Saeed Mahameed, netdev, Moshe Shemesh, Leon Romanovsky
From: Moshe Shemesh <moshe@nvidia.com>
The callback mlx5_devlink_eswitch_mode_set() had unlocked devlink as a
temporary workaround once devlink instance lock was added to devlink
eswitch callbacks. Now that all flows triggered by this function
that took devlink lock are using devl_ API and all parallel paths are
locked we can remove this workaround.
Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
.../mellanox/mlx5/core/eswitch_offloads.c | 20 -------------------
1 file changed, 20 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
index ccda3a0a2594..d3da52e3fc67 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/eswitch_offloads.c
@@ -2177,10 +2177,8 @@ static int esw_create_restore_table(struct mlx5_eswitch *esw)
static int esw_offloads_start(struct mlx5_eswitch *esw,
struct netlink_ext_ack *extack)
{
- struct devlink *devlink = priv_to_devlink(esw->dev);
int err, err1;
- devl_lock(devlink);
esw->mode = MLX5_ESWITCH_OFFLOADS;
err = mlx5_eswitch_enable_locked(esw, esw->dev->priv.sriov.num_vfs);
if (err) {
@@ -2202,7 +2200,6 @@ static int esw_offloads_start(struct mlx5_eswitch *esw,
"Inline mode is different between vports");
}
}
- devl_unlock(devlink);
return err;
}
@@ -3243,10 +3240,8 @@ int esw_offloads_enable(struct mlx5_eswitch *esw)
static int esw_offloads_stop(struct mlx5_eswitch *esw,
struct netlink_ext_ack *extack)
{
- struct devlink *devlink = priv_to_devlink(esw->dev);
int err, err1;
- devl_lock(devlink);
esw->mode = MLX5_ESWITCH_LEGACY;
err = mlx5_eswitch_enable_locked(esw, MLX5_ESWITCH_IGNORE_NUM_VFS);
if (err) {
@@ -3258,7 +3253,6 @@ static int esw_offloads_stop(struct mlx5_eswitch *esw,
"Failed setting eswitch back to offloads");
}
}
- devl_unlock(devlink);
return err;
}
@@ -3366,15 +3360,6 @@ int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
if (esw_mode_from_devlink(mode, &mlx5_mode))
return -EINVAL;
- /* FIXME: devl_unlock() followed by devl_lock() inside driver callback
- * is never correct and prone to races. It's a transitional workaround,
- * never repeat this pattern.
- *
- * This code MUST be fixed before removing devlink_mutex as it is safe
- * to do only because of that mutex.
- */
- devl_unlock(devlink);
-
mlx5_lag_disable_change(esw->dev);
err = mlx5_esw_try_lock(esw);
if (err < 0) {
@@ -3387,9 +3372,7 @@ int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
if (cur_mlx5_mode == mlx5_mode)
goto unlock;
- devl_lock(devlink);
mlx5_eswitch_disable_locked(esw);
- devl_unlock(devlink);
if (mode == DEVLINK_ESWITCH_MODE_SWITCHDEV) {
if (mlx5_devlink_trap_get_num_active(esw->dev)) {
NL_SET_ERR_MSG_MOD(extack,
@@ -3400,9 +3383,7 @@ int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
err = esw_offloads_start(esw, extack);
} else if (mode == DEVLINK_ESWITCH_MODE_LEGACY) {
err = esw_offloads_stop(esw, extack);
- devl_lock(devlink);
mlx5_rescan_drivers(esw->dev);
- devl_unlock(devlink);
} else {
err = -EINVAL;
}
@@ -3411,7 +3392,6 @@ int mlx5_devlink_eswitch_mode_set(struct devlink *devlink, u16 mode,
mlx5_esw_unlock(esw);
enable_lag:
mlx5_lag_enable_change(esw->dev);
- devl_lock(devlink);
return err;
}
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* [PATCH net-next 9/9] devlink: Hold the instance lock in port_new / port_del callbacks
2022-07-11 8:13 [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Saeed Mahameed
` (7 preceding siblings ...)
2022-07-11 8:14 ` [PATCH net-next 8/9] net/mlx5: Remove devl_unlock from mlx5_devlink_eswitch_mode_set Saeed Mahameed
@ 2022-07-11 8:14 ` Saeed Mahameed
2022-07-12 3:03 ` [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Jakub Kicinski
2022-07-12 8:50 ` patchwork-bot+netdevbpf
10 siblings, 0 replies; 12+ messages in thread
From: Saeed Mahameed @ 2022-07-11 8:14 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Paolo Abeni, Eric Dumazet
Cc: Saeed Mahameed, netdev, Moshe Shemesh, Leon Romanovsky
From: Moshe Shemesh <moshe@nvidia.com>
Let the core take the devlink instance lock around port_new and port_del
callbacks and remove the now redundant locking in the only driver that
currently use them.
Signed-off-by: Moshe Shemesh <moshe@nvidia.com>
Reviewed-by: Leon Romanovsky <leonro@nvidia.com>
Signed-off-by: Saeed Mahameed <saeedm@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c | 4 ----
net/core/devlink.c | 6 +-----
2 files changed, 1 insertion(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c
index 2068c22ff367..7d955a4d9f14 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/sf/devlink.c
@@ -355,9 +355,7 @@ int mlx5_devlink_sf_port_new(struct devlink *devlink,
"Port add is only supported in eswitch switchdev mode or SF ports are disabled.");
return -EOPNOTSUPP;
}
- devl_lock(devlink);
err = mlx5_sf_add(dev, table, new_attr, extack, new_port_index);
- devl_unlock(devlink);
mlx5_sf_table_put(table);
return err;
}
@@ -402,9 +400,7 @@ int mlx5_devlink_sf_port_del(struct devlink *devlink, unsigned int port_index,
goto sf_err;
}
- devl_lock(devlink);
mlx5_esw_offloads_sf_vport_disable(esw, sf->hw_fn_id);
- devl_unlock(devlink);
mlx5_sf_id_erase(table, sf);
mutex_lock(&table->sf_state_lock);
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 970e5c2a52bd..e206cc90bec5 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -1712,7 +1712,7 @@ static int devlink_port_new_notifiy(struct devlink *devlink,
if (!msg)
return -ENOMEM;
- mutex_lock(&devlink->lock);
+ lockdep_assert_held(&devlink->lock);
devlink_port = devlink_port_get_by_index(devlink, port_index);
if (!devlink_port) {
err = -ENODEV;
@@ -1725,11 +1725,9 @@ static int devlink_port_new_notifiy(struct devlink *devlink,
goto out;
err = genlmsg_reply(msg, info);
- mutex_unlock(&devlink->lock);
return err;
out:
- mutex_unlock(&devlink->lock);
nlmsg_free(msg);
return err;
}
@@ -9067,13 +9065,11 @@ static const struct genl_small_ops devlink_nl_ops[] = {
.cmd = DEVLINK_CMD_PORT_NEW,
.doit = devlink_nl_cmd_port_new_doit,
.flags = GENL_ADMIN_PERM,
- .internal_flags = DEVLINK_NL_FLAG_NO_LOCK,
},
{
.cmd = DEVLINK_CMD_PORT_DEL,
.doit = devlink_nl_cmd_port_del_doit,
.flags = GENL_ADMIN_PERM,
- .internal_flags = DEVLINK_NL_FLAG_NO_LOCK,
},
{
.cmd = DEVLINK_CMD_LINECARD_GET,
--
2.36.1
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [PATCH net-next 0/9] mlx5 devlink mutex removal part 1
2022-07-11 8:13 [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Saeed Mahameed
` (8 preceding siblings ...)
2022-07-11 8:14 ` [PATCH net-next 9/9] devlink: Hold the instance lock in port_new / port_del callbacks Saeed Mahameed
@ 2022-07-12 3:03 ` Jakub Kicinski
2022-07-12 8:50 ` patchwork-bot+netdevbpf
10 siblings, 0 replies; 12+ messages in thread
From: Jakub Kicinski @ 2022-07-12 3:03 UTC (permalink / raw)
To: Saeed Mahameed
Cc: David S. Miller, Paolo Abeni, Eric Dumazet, Saeed Mahameed,
netdev
On Mon, 11 Jul 2022 01:13:59 -0700 Saeed Mahameed wrote:
> 1) Fix devlink lock in mlx5 devlink eswitch callbacks
>
> Following the commit 14e426bf1a4d "devlink: hold the instance lock
> during eswitch_mode callbacks" which takes devlink instance lock for all
> devlink eswitch callbacks and adds a temporary workaround, this patchset
> removes the workaround, replaces devlink API functions by devl_ API
> where called from mlx5 driver eswitch callbacks flows and adds devlink
> instance lock in other driver's path that leads to these functions.
> While moving to devl_ API the patchset removes part of the devlink API
> functions which mlx5 was the last one to use and so not used by any
> driver now.
>
> The patchset also remove DEVLINK_NL_FLAG_NO_LOCK flag from the callbacks
> of port_new/port which are called only from mlx5 driver and the already
> locked by the patchset as parallel paths to the eswitch callbacks using
> devl_ API functions.
>
> This patchset will be followed by another patchset that will remove
> DEVLINK_NL_FLAG_NO_LOCK flag from devlink reload and devlink health
> callbacks. Thus we will have all devlink callbacks locked and it will
> pave the way to remove devlink mutex.
Acked-by: Jakub Kicinski <kuba@kernel.org>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [PATCH net-next 0/9] mlx5 devlink mutex removal part 1
2022-07-11 8:13 [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Saeed Mahameed
` (9 preceding siblings ...)
2022-07-12 3:03 ` [PATCH net-next 0/9] mlx5 devlink mutex removal part 1 Jakub Kicinski
@ 2022-07-12 8:50 ` patchwork-bot+netdevbpf
10 siblings, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-07-12 8:50 UTC (permalink / raw)
To: Saeed Mahameed; +Cc: davem, kuba, pabeni, edumazet, saeedm, netdev
Hello:
This series was applied to netdev/net-next.git (master)
by Paolo Abeni <pabeni@redhat.com>:
On Mon, 11 Jul 2022 01:13:59 -0700 you wrote:
> From: Saeed Mahameed <saeedm@nvidia.com>
>
> Moshe Shemesh Says:
> ===================
> 1) Fix devlink lock in mlx5 devlink eswitch callbacks
>
> Following the commit 14e426bf1a4d "devlink: hold the instance lock
> during eswitch_mode callbacks" which takes devlink instance lock for all
> devlink eswitch callbacks and adds a temporary workaround, this patchset
> removes the workaround, replaces devlink API functions by devl_ API
> where called from mlx5 driver eswitch callbacks flows and adds devlink
> instance lock in other driver's path that leads to these functions.
> While moving to devl_ API the patchset removes part of the devlink API
> functions which mlx5 was the last one to use and so not used by any
> driver now.
>
> [...]
Here is the summary with links:
- [net-next,1/9] net/mlx5: Remove devl_unlock from mlx5_eswtich_mode_callback_enter
https://git.kernel.org/netdev/net-next/c/367dfa121205
- [net-next,2/9] net/mlx5: Use devl_ API for rate nodes destroy
https://git.kernel.org/netdev/net-next/c/03f9c47d0f79
- [net-next,3/9] devlink: Remove unused function devlink_rate_nodes_destroy
https://git.kernel.org/netdev/net-next/c/868232f5cd38
- [net-next,4/9] net/mlx5: Use devl_ API in mlx5_esw_offloads_devlink_port_register
https://git.kernel.org/netdev/net-next/c/f1bc646c9a06
- [net-next,5/9] net/mlx5: Use devl_ API in mlx5_esw_devlink_sf_port_register
https://git.kernel.org/netdev/net-next/c/da212bd29d7f
- [net-next,6/9] devlink: Remove unused functions devlink_rate_leaf_create/destroy
https://git.kernel.org/netdev/net-next/c/df539fc62b06
- [net-next,7/9] net/mlx5: Use devl_ API in mlx5e_devlink_port_register
https://git.kernel.org/netdev/net-next/c/7b19119f4c7d
- [net-next,8/9] net/mlx5: Remove devl_unlock from mlx5_devlink_eswitch_mode_set
https://git.kernel.org/netdev/net-next/c/973598d46ede
- [net-next,9/9] devlink: Hold the instance lock in port_new / port_del callbacks
https://git.kernel.org/netdev/net-next/c/f0680ef0f949
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 12+ messages in thread