* [RFC PATCH 00/16] Allow parallel devlink execution
@ 2021-11-08 17:05 Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 01/16] devlink: Remove misleading internal_flags from health reporter dump Leon Romanovsky
` (15 more replies)
0 siblings, 16 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
Jakub, this is RFC, not fully tested and not reviewed yet.
I'm sending it to show that first we need to fix internal devlink
locking before even thinking of exporting it to the drivers.
-------------------------------------------------------------------
Hi,
This is final piece of devlink locking puzzle, where I remove global
mutex lock (devlink_mutex) and reduce scope of devlink->lock. The last
change allows us to make sure that devlink commands are called while
devlink_reload is executed and protected at the same time.
All of the changes mentioned above are limited to devlink.c without any
need to update drivers or expose internal details of devlink implementation.
The diffstat is misleading here, because of initial cleanup of overly
engineered interfaces.
BTW, I have one extra patch that documents devlink locking.
Thanks
Leon Romanovsky (16):
devlink: Remove misleading internal_flags from health reporter dump
devlink: Delete useless checks of holding devlink lock
devlink: Simplify devlink resources unregister call
devlink: Clean registration of devlink port
devlink: Be explicit with devlink port protection
devlink: Reshuffle resource registration logic
devlink: Inline sb related functions
devlink: Protect resource list with specific lock
devlink: Protect all traps lists with specialized lock
devlink: Separate region list protection to be done with specialized
lock
devlink: Protect all rate operations with specialized lock
devlink: Protect all sb operations with specialized lock
devlink: Convert dpipe to use dpipe_lock
devlink: Require devlink lock during device reload
devlink: Use xarray locking mechanism instead big devlink lock
devlink: Open devlink to parallel operations
.../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 7 +-
.../freescale/dpaa2/dpaa2-eth-devlink.c | 7 +-
drivers/net/ethernet/intel/ice/ice_devlink.c | 18 +-
.../marvell/prestera/prestera_devlink.c | 7 +-
drivers/net/ethernet/mellanox/mlx4/main.c | 4 +-
.../ethernet/mellanox/mlx5/core/en/devlink.c | 5 +-
.../ethernet/mellanox/mlx5/core/en/devlink.h | 2 +-
.../net/ethernet/mellanox/mlx5/core/en_main.c | 7 +-
.../mellanox/mlx5/core/esw/devlink_port.c | 9 +-
drivers/net/ethernet/mellanox/mlxsw/core.c | 15 +-
.../net/ethernet/mellanox/mlxsw/spectrum.c | 4 +-
drivers/net/ethernet/mscc/ocelot_net.c | 4 +-
.../net/ethernet/netronome/nfp/nfp_devlink.c | 4 +-
.../ethernet/pensando/ionic/ionic_devlink.c | 8 +-
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 14 +-
drivers/net/netdevsim/dev.c | 11 +-
include/net/devlink.h | 9 +-
net/core/devlink.c | 938 ++++++++++--------
net/dsa/dsa.c | 2 +-
net/dsa/dsa2.c | 9 +-
20 files changed, 561 insertions(+), 523 deletions(-)
--
2.33.1
^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH 01/16] devlink: Remove misleading internal_flags from health reporter dump
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 02/16] devlink: Delete useless checks of holding devlink lock Leon Romanovsky
` (14 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET command doesn't have .doit callback
and has no use in internal_flags at all. Remove this misleading assignment.
Fixes: e44ef4e4516c ("devlink: Hang reporter's dump method on a dumpit cb")
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 2d8abe88c673..fb9e60da9a77 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -8836,8 +8836,6 @@ static const struct genl_small_ops devlink_nl_ops[] = {
GENL_DONT_VALIDATE_DUMP_STRICT,
.dumpit = devlink_nl_cmd_health_reporter_dump_get_dumpit,
.flags = GENL_ADMIN_PERM,
- .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
- DEVLINK_NL_FLAG_NO_LOCK,
},
{
.cmd = DEVLINK_CMD_HEALTH_REPORTER_DUMP_CLEAR,
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 02/16] devlink: Delete useless checks of holding devlink lock
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 01/16] devlink: Remove misleading internal_flags from health reporter dump Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 03/16] devlink: Simplify devlink resources unregister call Leon Romanovsky
` (13 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
The snapshot API is fully protected by devlink->lock and these internal
functions are not exported directly to the code outside of the devlink.c.
This makes the checks of holding devlink lock as completely redundant.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 12 ------------
1 file changed, 12 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index fb9e60da9a77..4df241a62a44 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -5221,8 +5221,6 @@ static int __devlink_snapshot_id_increment(struct devlink *devlink, u32 id)
unsigned long count;
void *p;
- lockdep_assert_held(&devlink->lock);
-
p = xa_load(&devlink->snapshot_ids, id);
if (WARN_ON(!p))
return -EINVAL;
@@ -5257,8 +5255,6 @@ static void __devlink_snapshot_id_decrement(struct devlink *devlink, u32 id)
unsigned long count;
void *p;
- lockdep_assert_held(&devlink->lock);
-
p = xa_load(&devlink->snapshot_ids, id);
if (WARN_ON(!p))
return;
@@ -5296,8 +5292,6 @@ static void __devlink_snapshot_id_decrement(struct devlink *devlink, u32 id)
*/
static int __devlink_snapshot_id_insert(struct devlink *devlink, u32 id)
{
- lockdep_assert_held(&devlink->lock);
-
if (xa_load(&devlink->snapshot_ids, id))
return -EEXIST;
@@ -5323,8 +5317,6 @@ static int __devlink_snapshot_id_insert(struct devlink *devlink, u32 id)
*/
static int __devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id)
{
- lockdep_assert_held(&devlink->lock);
-
return xa_alloc(&devlink->snapshot_ids, id, xa_mk_value(1),
xa_limit_32b, GFP_KERNEL);
}
@@ -5351,8 +5343,6 @@ __devlink_region_snapshot_create(struct devlink_region *region,
struct devlink_snapshot *snapshot;
int err;
- lockdep_assert_held(&devlink->lock);
-
/* check if region can hold one more snapshot */
if (region->cur_snapshots == region->max_snapshots)
return -ENOSPC;
@@ -5389,8 +5379,6 @@ static void devlink_region_snapshot_del(struct devlink_region *region,
{
struct devlink *devlink = region->devlink;
- lockdep_assert_held(&devlink->lock);
-
devlink_nl_region_notify(region, snapshot, DEVLINK_CMD_REGION_DEL);
region->cur_snapshots--;
list_del(&snapshot->list);
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 03/16] devlink: Simplify devlink resources unregister call
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 01/16] devlink: Remove misleading internal_flags from health reporter dump Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 02/16] devlink: Delete useless checks of holding devlink lock Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 04/16] devlink: Clean registration of devlink port Leon Romanovsky
` (12 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
The devlink_resources_unregister() used second parameter as an
entry point for the recursive removal of devlink resources. None
of external to devlink users needed to use this field, so lat's
remove it.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
drivers/net/ethernet/mellanox/mlxsw/core.c | 8 ++---
.../net/ethernet/mellanox/mlxsw/spectrum.c | 4 +--
drivers/net/netdevsim/dev.c | 4 +--
include/net/devlink.h | 3 +-
net/core/devlink.c | 34 +++++++++++--------
net/dsa/dsa.c | 2 +-
6 files changed, 29 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 3fd3812b8f31..0d1f08bbf631 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -160,7 +160,7 @@ static void mlxsw_ports_fini(struct mlxsw_core *mlxsw_core, bool reload)
devlink_resource_occ_get_unregister(devlink, MLXSW_CORE_RESOURCE_PORTS);
if (!reload)
- devlink_resources_unregister(priv_to_devlink(mlxsw_core), NULL);
+ devlink_resources_unregister(priv_to_devlink(mlxsw_core));
kfree(mlxsw_core->ports);
}
@@ -2033,7 +2033,7 @@ __mlxsw_core_bus_device_register(const struct mlxsw_bus_info *mlxsw_bus_info,
mlxsw_ports_fini(mlxsw_core, reload);
err_ports_init:
if (!reload)
- devlink_resources_unregister(devlink, NULL);
+ devlink_resources_unregister(devlink);
err_register_resources:
mlxsw_bus->fini(bus_priv);
err_bus_init:
@@ -2099,7 +2099,7 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core,
kfree(mlxsw_core->lag.mapping);
mlxsw_ports_fini(mlxsw_core, reload);
if (!reload)
- devlink_resources_unregister(devlink, NULL);
+ devlink_resources_unregister(devlink);
mlxsw_core->bus->fini(mlxsw_core->bus_priv);
if (!reload)
devlink_free(devlink);
@@ -2108,7 +2108,7 @@ void mlxsw_core_bus_device_unregister(struct mlxsw_core *mlxsw_core,
reload_fail_deinit:
mlxsw_core_params_unregister(mlxsw_core);
- devlink_resources_unregister(devlink, NULL);
+ devlink_resources_unregister(devlink);
devlink_free(devlink);
}
EXPORT_SYMBOL(mlxsw_core_bus_device_unregister);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 5925db386b1b..6a5887aacbc0 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3336,7 +3336,7 @@ static int mlxsw_sp1_resources_register(struct mlxsw_core *mlxsw_core)
err_policer_resources_register:
err_resources_counter_register:
err_resources_span_register:
- devlink_resources_unregister(priv_to_devlink(mlxsw_core), NULL);
+ devlink_resources_unregister(priv_to_devlink(mlxsw_core));
return err;
}
@@ -3370,7 +3370,7 @@ static int mlxsw_sp2_resources_register(struct mlxsw_core *mlxsw_core)
err_policer_resources_register:
err_resources_counter_register:
err_resources_span_register:
- devlink_resources_unregister(priv_to_devlink(mlxsw_core), NULL);
+ devlink_resources_unregister(priv_to_devlink(mlxsw_core));
return err;
}
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 6c906deca71c..4a8ca4896d9d 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -1531,7 +1531,7 @@ int nsim_dev_probe(struct nsim_bus_dev *nsim_bus_dev)
devlink_params_unregister(devlink, nsim_devlink_params,
ARRAY_SIZE(nsim_devlink_params));
err_dl_unregister:
- devlink_resources_unregister(devlink, NULL);
+ devlink_resources_unregister(devlink);
err_devlink_free:
devlink_free(devlink);
return err;
@@ -1571,7 +1571,7 @@ void nsim_dev_remove(struct nsim_bus_dev *nsim_bus_dev)
nsim_dev_debugfs_exit(nsim_dev);
devlink_params_unregister(devlink, nsim_devlink_params,
ARRAY_SIZE(nsim_devlink_params));
- devlink_resources_unregister(devlink, NULL);
+ devlink_resources_unregister(devlink);
devlink_free(devlink);
}
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 1b1317d378de..a888aa13e219 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1567,8 +1567,7 @@ int devlink_resource_register(struct devlink *devlink,
u64 resource_id,
u64 parent_resource_id,
const struct devlink_resource_size_params *size_params);
-void devlink_resources_unregister(struct devlink *devlink,
- struct devlink_resource *resource);
+void devlink_resources_unregister(struct devlink *devlink);
int devlink_resource_size_get(struct devlink *devlink,
u64 resource_id,
u64 *p_resource_size);
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 4df241a62a44..56a1ee65bce5 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -9887,34 +9887,38 @@ int devlink_resource_register(struct devlink *devlink,
}
EXPORT_SYMBOL_GPL(devlink_resource_register);
+static void devlink_resource_unregister(struct devlink *devlink,
+ struct devlink_resource *resource)
+{
+ struct devlink_resource *tmp, *child_resource;
+
+ list_for_each_entry_safe(child_resource, tmp, &resource->resource_list,
+ list) {
+ devlink_resource_unregister(devlink, child_resource);
+ list_del(&child_resource->list);
+ kfree(child_resource);
+ }
+}
+
/**
* devlink_resources_unregister - free all resources
*
* @devlink: devlink
- * @resource: resource
*/
-void devlink_resources_unregister(struct devlink *devlink,
- struct devlink_resource *resource)
+void devlink_resources_unregister(struct devlink *devlink)
{
struct devlink_resource *tmp, *child_resource;
- struct list_head *resource_list;
-
- if (resource)
- resource_list = &resource->resource_list;
- else
- resource_list = &devlink->resource_list;
- if (!resource)
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->lock);
- list_for_each_entry_safe(child_resource, tmp, resource_list, list) {
- devlink_resources_unregister(devlink, child_resource);
+ list_for_each_entry_safe(child_resource, tmp, &devlink->resource_list,
+ list) {
+ devlink_resource_unregister(devlink, child_resource);
list_del(&child_resource->list);
kfree(child_resource);
}
- if (!resource)
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->lock);
}
EXPORT_SYMBOL_GPL(devlink_resources_unregister);
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index ea5169e671ae..d9d0d227092c 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -406,7 +406,7 @@ EXPORT_SYMBOL_GPL(dsa_devlink_resource_register);
void dsa_devlink_resources_unregister(struct dsa_switch *ds)
{
- devlink_resources_unregister(ds->devlink, NULL);
+ devlink_resources_unregister(ds->devlink);
}
EXPORT_SYMBOL_GPL(dsa_devlink_resources_unregister);
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 04/16] devlink: Clean registration of devlink port
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (2 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 03/16] devlink: Simplify devlink resources unregister call Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 05/16] devlink: Be explicit with devlink port protection Leon Romanovsky
` (11 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
devlink_port_register() is in-kernel API and as such can't really fail
as long as driver author didn't make a mistake by providing already existing
port index. Instead of relying on various error prints from the driver,
convert the existence check to be WARN_ON(), so such a mistake will be
caught easier.
As an outcome of this conversion, it was made clear that this function
should be void and devlink->rwsem was intended to protect addition to
port_list.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
.../net/ethernet/broadcom/bnxt/bnxt_devlink.c | 7 +---
.../freescale/dpaa2/dpaa2-eth-devlink.c | 7 +---
drivers/net/ethernet/intel/ice/ice_devlink.c | 18 ++---------
.../marvell/prestera/prestera_devlink.c | 7 +---
drivers/net/ethernet/mellanox/mlx4/main.c | 4 +--
.../ethernet/mellanox/mlx5/core/en/devlink.c | 5 ++-
.../ethernet/mellanox/mlx5/core/en/devlink.h | 2 +-
.../net/ethernet/mellanox/mlx5/core/en_main.c | 7 +---
.../mellanox/mlx5/core/esw/devlink_port.c | 9 ++----
drivers/net/ethernet/mellanox/mlxsw/core.c | 7 ++--
drivers/net/ethernet/mscc/ocelot_net.c | 4 +--
.../net/ethernet/netronome/nfp/nfp_devlink.c | 4 +--
.../ethernet/pensando/ionic/ionic_devlink.c | 8 +----
drivers/net/ethernet/ti/am65-cpsw-nuss.c | 14 +-------
drivers/net/netdevsim/dev.c | 7 ++--
include/net/devlink.h | 6 ++--
net/core/devlink.c | 32 ++++++++-----------
net/dsa/dsa2.c | 9 ++----
18 files changed, 41 insertions(+), 116 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index ce790e9b45c3..048027955cef 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -1209,11 +1209,7 @@ int bnxt_dl_register(struct bnxt *bp)
memcpy(attrs.switch_id.id, bp->dsn, sizeof(bp->dsn));
attrs.switch_id.id_len = sizeof(bp->dsn);
devlink_port_attrs_set(&bp->dl_port, &attrs);
- rc = devlink_port_register(dl, &bp->dl_port, bp->pf.port_id);
- if (rc) {
- netdev_err(bp->dev, "devlink_port_register failed\n");
- goto err_dl_free;
- }
+ devlink_port_register(dl, &bp->dl_port, bp->pf.port_id);
rc = bnxt_dl_params_register(bp);
if (rc)
@@ -1225,7 +1221,6 @@ int bnxt_dl_register(struct bnxt *bp)
err_dl_port_unreg:
devlink_port_unregister(&bp->dl_port);
-err_dl_free:
devlink_free(dl);
return rc;
}
diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
index 7fefe1574b6a..d19423780e59 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-eth-devlink.c
@@ -226,17 +226,12 @@ int dpaa2_eth_dl_port_add(struct dpaa2_eth_priv *priv)
{
struct devlink_port *devlink_port = &priv->devlink_port;
struct devlink_port_attrs attrs = {};
- int err;
attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
devlink_port_attrs_set(devlink_port, &attrs);
- err = devlink_port_register(priv->devlink, devlink_port, 0);
- if (err)
- return err;
-
+ devlink_port_register(priv->devlink, devlink_port, 0);
devlink_port_type_eth_set(devlink_port, priv->net_dev);
-
return 0;
}
diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index b9bd9f9472f6..fb5d0f4ade4e 100644
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -499,7 +499,6 @@ int ice_devlink_create_pf_port(struct ice_pf *pf)
struct devlink *devlink;
struct ice_vsi *vsi;
struct device *dev;
- int err;
dev = ice_pf_to_dev(pf);
@@ -514,13 +513,7 @@ int ice_devlink_create_pf_port(struct ice_pf *pf)
devlink_port_attrs_set(devlink_port, &attrs);
devlink = priv_to_devlink(pf);
- err = devlink_port_register(devlink, devlink_port, vsi->idx);
- if (err) {
- dev_err(dev, "Failed to create devlink port for PF %d, error %d\n",
- pf->hw.pf_id, err);
- return err;
- }
-
+ devlink_port_register(devlink, devlink_port, vsi->idx);
return 0;
}
@@ -556,7 +549,6 @@ int ice_devlink_create_vf_port(struct ice_vf *vf)
struct ice_vsi *vsi;
struct device *dev;
struct ice_pf *pf;
- int err;
pf = vf->pf;
dev = ice_pf_to_dev(pf);
@@ -570,13 +562,7 @@ int ice_devlink_create_vf_port(struct ice_vf *vf)
devlink_port_attrs_set(devlink_port, &attrs);
devlink = priv_to_devlink(pf);
- err = devlink_port_register(devlink, devlink_port, vsi->idx);
- if (err) {
- dev_err(dev, "Failed to create devlink port for VF %d, error %d\n",
- vf->vf_id, err);
- return err;
- }
-
+ devlink_port_register(devlink, devlink_port, vsi->idx);
return 0;
}
diff --git a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
index 06279cd6da67..b211b19460eb 100644
--- a/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
+++ b/drivers/net/ethernet/marvell/prestera/prestera_devlink.c
@@ -431,12 +431,7 @@ int prestera_devlink_port_register(struct prestera_port *port)
devlink_port_attrs_set(&port->dl_port, &attrs);
- err = devlink_port_register(dl, &port->dl_port, port->fp_id);
- if (err) {
- dev_err(prestera_dev(sw), "devlink_port_register failed: %d\n", err);
- return err;
- }
-
+ devlink_port_register(dl, &port->dl_port, port->fp_id);
return 0;
}
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index b187c210d4d6..c88a586ecd8d 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3033,9 +3033,7 @@ static int mlx4_init_port_info(struct mlx4_dev *dev, int port)
struct mlx4_port_info *info = &mlx4_priv(dev)->port[port];
int err;
- err = devlink_port_register(devlink, &info->devlink_port, port);
- if (err)
- return err;
+ devlink_port_register(devlink, &info->devlink_port, port);
/* Ethernet and IB drivers will normally set the port type,
* but if they are not built set the type now to prevent
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
index ae52e7f38306..76326858db22 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c
@@ -14,7 +14,7 @@ mlx5e_devlink_get_port_parent_id(struct mlx5_core_dev *dev, struct netdev_phys_i
memcpy(ppid->id, &parent_id, sizeof(parent_id));
}
-int mlx5e_devlink_port_register(struct mlx5e_priv *priv)
+void mlx5e_devlink_port_register(struct mlx5e_priv *priv)
{
struct devlink *devlink = priv_to_devlink(priv->mdev);
struct devlink_port_attrs attrs = {};
@@ -40,8 +40,7 @@ int mlx5e_devlink_port_register(struct mlx5e_priv *priv)
dl_port = mlx5e_devlink_get_dl_port(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);
+ devlink_port_register(devlink, dl_port, dl_port_index);
}
void mlx5e_devlink_port_type_eth_set(struct mlx5e_priv *priv)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.h b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.h
index 10b50feb9883..04ada8624367 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.h
@@ -7,7 +7,7 @@
#include <net/devlink.h>
#include "en.h"
-int mlx5e_devlink_port_register(struct mlx5e_priv *priv);
+void mlx5e_devlink_port_register(struct mlx5e_priv *priv);
void mlx5e_devlink_port_unregister(struct mlx5e_priv *priv);
void mlx5e_devlink_port_type_eth_set(struct mlx5e_priv *priv);
struct devlink_port *mlx5e_get_devlink_port(struct net_device *dev);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
index 65571593ec5c..385533dce1bb 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c
@@ -5461,11 +5461,7 @@ static int mlx5e_probe(struct auxiliary_device *adev,
priv->profile = profile;
priv->ppriv = NULL;
- err = mlx5e_devlink_port_register(priv);
- if (err) {
- mlx5_core_err(mdev, "mlx5e_devlink_port_register failed, %d\n", err);
- goto err_destroy_netdev;
- }
+ mlx5e_devlink_port_register(priv);
err = profile->init(mdev, netdev);
if (err) {
@@ -5497,7 +5493,6 @@ static int mlx5e_probe(struct auxiliary_device *adev,
profile->cleanup(priv);
err_devlink_cleanup:
mlx5e_devlink_port_unregister(priv);
-err_destroy_netdev:
mlx5e_destroy_netdev(priv);
return err;
}
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..d1550f661644 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/esw/devlink_port.c
@@ -87,9 +87,7 @@ 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);
- if (err)
- goto reg_err;
+ devlink_port_register(devlink, dl_port, dl_port_index);
err = devlink_rate_leaf_create(dl_port, vport);
if (err)
@@ -100,7 +98,6 @@ int mlx5_esw_offloads_devlink_port_register(struct mlx5_eswitch *esw, u16 vport_
rate_err:
devlink_port_unregister(dl_port);
-reg_err:
mlx5_esw_dl_port_free(dl_port);
return err;
}
@@ -156,9 +153,7 @@ 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);
- if (err)
- return err;
+ devlink_port_register(devlink, dl_port, dl_port_index);
err = devlink_rate_leaf_create(dl_port, vport);
if (err)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 0d1f08bbf631..7684a86c1745 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -2760,7 +2760,6 @@ static int __mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
&mlxsw_core->ports[local_port];
struct devlink_port *devlink_port = &mlxsw_core_port->devlink_port;
struct devlink_port_attrs attrs = {};
- int err;
attrs.split = split;
attrs.lanes = lanes;
@@ -2772,10 +2771,8 @@ static int __mlxsw_core_port_init(struct mlxsw_core *mlxsw_core, u8 local_port,
attrs.switch_id.id_len = switch_id_len;
mlxsw_core_port->local_port = local_port;
devlink_port_attrs_set(devlink_port, &attrs);
- err = devlink_port_register(devlink, devlink_port, local_port);
- if (err)
- memset(mlxsw_core_port, 0, sizeof(*mlxsw_core_port));
- return err;
+ devlink_port_register(devlink, devlink_port, local_port);
+ return 0;
}
static void __mlxsw_core_port_fini(struct mlxsw_core *mlxsw_core, u8 local_port)
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index eaeba60b1bba..fc6db586a7ea 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -172,8 +172,8 @@ int ocelot_port_devlink_init(struct ocelot *ocelot, int port,
attrs.flavour = flavour;
devlink_port_attrs_set(dlp, &attrs);
-
- return devlink_port_register(dl, dlp, port);
+ devlink_port_register(dl, dlp, port);
+ return 0;
}
void ocelot_port_devlink_teardown(struct ocelot *ocelot, int port)
diff --git a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
index bea978df7713..ed53462ac9c2 100644
--- a/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
+++ b/drivers/net/ethernet/netronome/nfp/nfp_devlink.c
@@ -374,8 +374,8 @@ int nfp_devlink_port_register(struct nfp_app *app, struct nfp_port *port)
devlink_port_attrs_set(&port->dl_port, &attrs);
devlink = priv_to_devlink(app->pf);
-
- return devlink_port_register(devlink, &port->dl_port, port->eth_id);
+ devlink_port_register(devlink, &port->dl_port, port->eth_id);
+ return 0;
}
void nfp_devlink_port_unregister(struct nfp_port *port)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
index 4297ed9024c0..f8c80856ca02 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_devlink.c
@@ -80,16 +80,10 @@ int ionic_devlink_register(struct ionic *ionic)
{
struct devlink *dl = priv_to_devlink(ionic);
struct devlink_port_attrs attrs = {};
- int err;
attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL;
devlink_port_attrs_set(&ionic->dl_port, &attrs);
- err = devlink_port_register(dl, &ionic->dl_port, 0);
- if (err) {
- dev_err(ionic->dev, "devlink_port_register failed: %d\n", err);
- return err;
- }
-
+ devlink_port_register(dl, &ionic->dl_port, 0);
devlink_port_type_eth_set(&ionic->dl_port, ionic->lif->netdev);
devlink_register(dl);
return 0;
diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
index c092cb61416a..2e3cc08da998 100644
--- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
+++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
@@ -2453,24 +2453,12 @@ static int am65_cpsw_nuss_register_devlink(struct am65_cpsw_common *common)
memcpy(attrs.switch_id.id, common->switch_id, attrs.switch_id.id_len);
devlink_port_attrs_set(dl_port, &attrs);
- ret = devlink_port_register(common->devlink, dl_port, port->port_id);
- if (ret) {
- dev_err(dev, "devlink_port reg fail for port %d, ret:%d\n",
- port->port_id, ret);
- goto dl_port_unreg;
- }
+ devlink_port_register(common->devlink, dl_port, port->port_id);
devlink_port_type_eth_set(dl_port, port->ndev);
}
devlink_register(common->devlink);
return ret;
-dl_port_unreg:
- for (i = i - 1; i >= 1; i--) {
- port = am65_common_get_port(common, i);
- dl_port = &port->devlink_port;
-
- devlink_port_unregister(dl_port);
- }
dl_unreg:
devlink_free(common->devlink);
return ret;
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 4a8ca4896d9d..a0152cfb2197 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -1298,10 +1298,8 @@ static int __nsim_dev_port_add(struct nsim_dev *nsim_dev, enum nsim_dev_port_typ
memcpy(attrs.switch_id.id, nsim_dev->switch_id.id, nsim_dev->switch_id.id_len);
attrs.switch_id.id_len = nsim_dev->switch_id.id_len;
devlink_port_attrs_set(devlink_port, &attrs);
- err = devlink_port_register(priv_to_devlink(nsim_dev), devlink_port,
- nsim_dev_port->port_index);
- if (err)
- goto err_port_free;
+ devlink_port_register(priv_to_devlink(nsim_dev), devlink_port,
+ nsim_dev_port->port_index);
err = nsim_dev_port_debugfs_init(nsim_dev, nsim_dev_port);
if (err)
@@ -1331,7 +1329,6 @@ static int __nsim_dev_port_add(struct nsim_dev *nsim_dev, enum nsim_dev_port_typ
nsim_dev_port_debugfs_exit(nsim_dev_port);
err_dl_port_unregister:
devlink_port_unregister(devlink_port);
-err_port_free:
kfree(nsim_dev_port);
return err;
}
diff --git a/include/net/devlink.h b/include/net/devlink.h
index a888aa13e219..313553f73963 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1511,9 +1511,9 @@ void devlink_set_features(struct devlink *devlink, u64 features);
void devlink_register(struct devlink *devlink);
void devlink_unregister(struct devlink *devlink);
void devlink_free(struct devlink *devlink);
-int devlink_port_register(struct devlink *devlink,
- struct devlink_port *devlink_port,
- unsigned int port_index);
+void devlink_port_register(struct devlink *devlink,
+ struct devlink_port *devlink_port,
+ unsigned int port_index);
void devlink_port_unregister(struct devlink_port *devlink_port);
void devlink_port_type_eth_set(struct devlink_port *devlink_port,
struct net_device *netdev);
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 56a1ee65bce5..b41ab8751635 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -237,12 +237,6 @@ static struct devlink_port *devlink_port_get_by_index(struct devlink *devlink,
return NULL;
}
-static bool devlink_port_index_exists(struct devlink *devlink,
- unsigned int port_index)
-{
- return devlink_port_get_by_index(devlink, port_index);
-}
-
static struct devlink_port *devlink_port_get_from_attrs(struct devlink *devlink,
struct nlattr **attrs)
{
@@ -9201,30 +9195,28 @@ static void devlink_port_type_warn_cancel(struct devlink_port *devlink_port)
* Note that the caller should take care of zeroing the devlink_port
* structure.
*/
-int devlink_port_register(struct devlink *devlink,
- struct devlink_port *devlink_port,
- unsigned int port_index)
+void devlink_port_register(struct devlink *devlink,
+ struct devlink_port *devlink_port,
+ unsigned int port_index)
{
- mutex_lock(&devlink->lock);
- if (devlink_port_index_exists(devlink, port_index)) {
- mutex_unlock(&devlink->lock);
- return -EEXIST;
- }
-
+ WARN_ON(devlink_port_get_by_index(devlink, port_index));
WARN_ON(devlink_port->devlink);
+
devlink_port->devlink = devlink;
devlink_port->index = port_index;
spin_lock_init(&devlink_port->type_lock);
INIT_LIST_HEAD(&devlink_port->reporter_list);
mutex_init(&devlink_port->reporters_lock);
- list_add_tail(&devlink_port->list, &devlink->port_list);
INIT_LIST_HEAD(&devlink_port->param_list);
INIT_LIST_HEAD(&devlink_port->region_list);
- mutex_unlock(&devlink->lock);
INIT_DELAYED_WORK(&devlink_port->type_warn_dw, &devlink_port_type_warn);
- devlink_port_type_warn_schedule(devlink_port);
+
+ mutex_lock(&devlink->lock);
+ list_add_tail(&devlink_port->list, &devlink->port_list);
+ mutex_unlock(&devlink->lock);
+
devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
- return 0;
+ devlink_port_type_warn_schedule(devlink_port);
}
EXPORT_SYMBOL_GPL(devlink_port_register);
@@ -9239,9 +9231,11 @@ void devlink_port_unregister(struct devlink_port *devlink_port)
devlink_port_type_warn_cancel(devlink_port);
devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_DEL);
+
mutex_lock(&devlink->lock);
list_del(&devlink_port->list);
mutex_unlock(&devlink->lock);
+
WARN_ON(!list_empty(&devlink_port->reporter_list));
WARN_ON(!list_empty(&devlink_port->region_list));
mutex_destroy(&devlink_port->reporters_lock);
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 826957b6442b..29f39f761e05 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -504,7 +504,6 @@ static int dsa_port_devlink_setup(struct dsa_port *dp)
struct devlink *dl = dp->ds->devlink;
const unsigned char *id;
unsigned char len;
- int err;
id = (const unsigned char *)&dst->index;
len = sizeof(dst->index);
@@ -530,12 +529,10 @@ static int dsa_port_devlink_setup(struct dsa_port *dp)
}
devlink_port_attrs_set(dlp, &attrs);
- err = devlink_port_register(dl, dlp, dp->index);
-
- if (!err)
- dp->devlink_port_setup = true;
+ devlink_port_register(dl, dlp, dp->index);
+ dp->devlink_port_setup = true;
- return err;
+ return 0;
}
static void dsa_port_teardown(struct dsa_port *dp)
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 05/16] devlink: Be explicit with devlink port protection
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (3 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 04/16] devlink: Clean registration of devlink port Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 06/16] devlink: Reshuffle resource registration logic Leon Romanovsky
` (10 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
Devlink port flows use devlink instance lock to protect from parallel
addition and deletion from port_list. So instead of using global lock,
let's introduce specific protection lock with much more clear scope
that will protect port_list.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 61 ++++++++++++++++++++++++++++++----------------
1 file changed, 40 insertions(+), 21 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index b41ab8751635..d88e882616bc 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -41,6 +41,7 @@ struct devlink_dev_stats {
struct devlink {
u32 index;
struct list_head port_list;
+ struct mutex port_list_lock; /* protects port_list */
struct list_head rate_list;
struct list_head sb_list;
struct list_head dpipe_table_list;
@@ -228,13 +229,17 @@ static struct devlink *devlink_get_from_attrs(struct net *net,
static struct devlink_port *devlink_port_get_by_index(struct devlink *devlink,
unsigned int port_index)
{
- struct devlink_port *devlink_port;
+ struct devlink_port *devlink_port, *res = NULL;
+ mutex_lock(&devlink->port_list_lock);
list_for_each_entry(devlink_port, &devlink->port_list, list) {
- if (devlink_port->index == port_index)
- return devlink_port;
+ if (devlink_port->index == port_index) {
+ res = devlink_port;
+ break;
+ }
}
- return NULL;
+ mutex_unlock(&devlink->port_list_lock);
+ return res;
}
static struct devlink_port *devlink_port_get_from_attrs(struct devlink *devlink,
@@ -1341,7 +1346,7 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->port_list_lock);
list_for_each_entry(devlink_port, &devlink->port_list, list) {
if (idx < start) {
idx++;
@@ -1353,13 +1358,13 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
cb->nlh->nlmsg_seq,
NLM_F_MULTI, cb->extack);
if (err) {
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->port_list_lock);
devlink_put(devlink);
goto out;
}
idx++;
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->port_list_lock);
retry:
devlink_put(devlink);
}
@@ -2567,8 +2572,9 @@ static int __sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
{
struct devlink_port *devlink_port;
u16 tc_index;
- int err;
+ int err = 0;
+ mutex_lock(&devlink->port_list_lock);
list_for_each_entry(devlink_port, &devlink->port_list, list) {
for (tc_index = 0;
tc_index < devlink_sb->ingress_tc_count; tc_index++) {
@@ -2585,7 +2591,7 @@ static int __sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
portid, seq,
NLM_F_MULTI);
if (err)
- return err;
+ goto out;
(*p_idx)++;
}
for (tc_index = 0;
@@ -2603,11 +2609,13 @@ static int __sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
portid, seq,
NLM_F_MULTI);
if (err)
- return err;
+ goto out;
(*p_idx)++;
}
}
- return 0;
+out:
+ mutex_unlock(&devlink->port_list_lock);
+ return err;
}
static int
@@ -4938,7 +4946,7 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->port_list_lock);
list_for_each_entry(devlink_port, &devlink->port_list, list) {
list_for_each_entry(param_item,
&devlink_port->param_list, list) {
@@ -4956,14 +4964,14 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
if (err == -EOPNOTSUPP) {
err = 0;
} else if (err) {
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->port_list_lock);
devlink_put(devlink);
goto out;
}
idx++;
}
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->port_list_lock);
retry:
devlink_put(devlink);
}
@@ -5481,6 +5489,7 @@ static int devlink_nl_cmd_region_get_devlink_dumpit(struct sk_buff *msg,
(*idx)++;
}
+ mutex_lock(&devlink->port_list_lock);
list_for_each_entry(port, &devlink->port_list, list) {
err = devlink_nl_cmd_region_get_port_dumpit(msg, cb, port, idx,
start);
@@ -5489,6 +5498,7 @@ static int devlink_nl_cmd_region_get_devlink_dumpit(struct sk_buff *msg,
}
out:
+ mutex_unlock(&devlink->port_list_lock);
mutex_unlock(&devlink->lock);
return err;
}
@@ -7254,7 +7264,7 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry_port;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->port_list_lock);
list_for_each_entry(port, &devlink->port_list, list) {
mutex_lock(&port->reporters_lock);
list_for_each_entry(reporter, &port->reporter_list, list) {
@@ -7269,7 +7279,7 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
cb->nlh->nlmsg_seq, NLM_F_MULTI);
if (err) {
mutex_unlock(&port->reporters_lock);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->port_list_lock);
devlink_put(devlink);
goto out;
}
@@ -7277,7 +7287,7 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
}
mutex_unlock(&port->reporters_lock);
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->port_list_lock);
retry_port:
devlink_put(devlink);
}
@@ -8978,7 +8988,10 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
devlink->ops = ops;
xa_init_flags(&devlink->snapshot_ids, XA_FLAGS_ALLOC);
write_pnet(&devlink->_net, net);
+
INIT_LIST_HEAD(&devlink->port_list);
+ mutex_init(&devlink->port_list_lock);
+
INIT_LIST_HEAD(&devlink->rate_list);
INIT_LIST_HEAD(&devlink->sb_list);
INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
@@ -9021,8 +9034,10 @@ static void devlink_notify_register(struct devlink *devlink)
struct devlink_region *region;
devlink_notify(devlink, DEVLINK_CMD_NEW);
+ mutex_lock(&devlink->port_list_lock);
list_for_each_entry(devlink_port, &devlink->port_list, list)
devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
+ mutex_unlock(&devlink->port_list_lock);
list_for_each_entry(policer_item, &devlink->trap_policer_list, list)
devlink_trap_policer_notify(devlink, policer_item,
@@ -9077,8 +9092,11 @@ static void devlink_notify_unregister(struct devlink *devlink)
devlink_trap_policer_notify(devlink, policer_item,
DEVLINK_CMD_TRAP_POLICER_DEL);
+ mutex_lock(&devlink->port_list_lock);
list_for_each_entry_reverse(devlink_port, &devlink->port_list, list)
devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_DEL);
+ mutex_unlock(&devlink->port_list_lock);
+
devlink_notify(devlink, DEVLINK_CMD_DEL);
}
@@ -9129,6 +9147,7 @@ void devlink_free(struct devlink *devlink)
ASSERT_DEVLINK_NOT_REGISTERED(devlink);
mutex_destroy(&devlink->reporters_lock);
+ mutex_destroy(&devlink->port_list_lock);
mutex_destroy(&devlink->lock);
WARN_ON(!list_empty(&devlink->trap_policer_list));
WARN_ON(!list_empty(&devlink->trap_group_list));
@@ -9211,9 +9230,9 @@ void devlink_port_register(struct devlink *devlink,
INIT_LIST_HEAD(&devlink_port->region_list);
INIT_DELAYED_WORK(&devlink_port->type_warn_dw, &devlink_port_type_warn);
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->port_list_lock);
list_add_tail(&devlink_port->list, &devlink->port_list);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->port_list_lock);
devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
devlink_port_type_warn_schedule(devlink_port);
@@ -9232,9 +9251,9 @@ void devlink_port_unregister(struct devlink_port *devlink_port)
devlink_port_type_warn_cancel(devlink_port);
devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_DEL);
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->port_list_lock);
list_del(&devlink_port->list);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->port_list_lock);
WARN_ON(!list_empty(&devlink_port->reporter_list));
WARN_ON(!list_empty(&devlink_port->region_list));
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 06/16] devlink: Reshuffle resource registration logic
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (4 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 05/16] devlink: Be explicit with devlink port protection Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 07/16] devlink: Inline sb related functions Leon Romanovsky
` (9 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
The devlink->lock doesn't need to be held during whole execution of
function, but only when recursive path walk and list addition are
performed. So reshuffle the resource registration logic to allocate
resource and configure it without lock.
As part of this change, complain more louder if driver authors used
already existed resource_id. It is performed outside of the locks as
drivers were supposed to provide unique IDs and such error can't happen.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 30 ++++++++++++------------------
1 file changed, 12 insertions(+), 18 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index d88e882616bc..a2cd27fd767e 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -9850,17 +9850,9 @@ int devlink_resource_register(struct devlink *devlink,
{
struct devlink_resource *resource;
struct list_head *resource_list;
- bool top_hierarchy;
int err = 0;
- top_hierarchy = parent_resource_id == DEVLINK_RESOURCE_ID_PARENT_TOP;
-
- mutex_lock(&devlink->lock);
- resource = devlink_resource_find(devlink, NULL, resource_id);
- if (resource) {
- err = -EINVAL;
- goto out;
- }
+ WARN_ON(devlink_resource_find(devlink, NULL, resource_id));
resource = kzalloc(sizeof(*resource), GFP_KERNEL);
if (!resource) {
@@ -9868,7 +9860,17 @@ int devlink_resource_register(struct devlink *devlink,
goto out;
}
- if (top_hierarchy) {
+ resource->name = resource_name;
+ resource->size = resource_size;
+ resource->size_new = resource_size;
+ resource->id = resource_id;
+ resource->size_valid = true;
+ memcpy(&resource->size_params, size_params,
+ sizeof(resource->size_params));
+ INIT_LIST_HEAD(&resource->resource_list);
+
+ mutex_lock(&devlink->lock);
+ if (parent_resource_id == DEVLINK_RESOURCE_ID_PARENT_TOP) {
resource_list = &devlink->resource_list;
} else {
struct devlink_resource *parent_resource;
@@ -9885,14 +9887,6 @@ int devlink_resource_register(struct devlink *devlink,
}
}
- resource->name = resource_name;
- resource->size = resource_size;
- resource->size_new = resource_size;
- resource->id = resource_id;
- resource->size_valid = true;
- memcpy(&resource->size_params, size_params,
- sizeof(resource->size_params));
- INIT_LIST_HEAD(&resource->resource_list);
list_add_tail(&resource->list, resource_list);
out:
mutex_unlock(&devlink->lock);
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 07/16] devlink: Inline sb related functions
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (5 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 06/16] devlink: Reshuffle resource registration logic Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 08/16] devlink: Protect resource list with specific lock Leon Romanovsky
` (8 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
Remove useless indirection of sb related functions, which called only
once and do nothing except accessing specific struct field.
As part of this cleanup, properly report an programming erro if already
existing sb index was supplied during SB registration.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 110 ++++++++++++++-------------------------------
1 file changed, 33 insertions(+), 77 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index a2cd27fd767e..86db7cf1f3ca 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -364,12 +364,6 @@ static struct devlink_sb *devlink_sb_get_by_index(struct devlink *devlink,
return NULL;
}
-static bool devlink_sb_index_exists(struct devlink *devlink,
- unsigned int sb_index)
-{
- return devlink_sb_get_by_index(devlink, sb_index);
-}
-
static struct devlink_sb *devlink_sb_get_from_attrs(struct devlink *devlink,
struct nlattr **attrs)
{
@@ -385,16 +379,11 @@ static struct devlink_sb *devlink_sb_get_from_attrs(struct devlink *devlink,
return ERR_PTR(-EINVAL);
}
-static struct devlink_sb *devlink_sb_get_from_info(struct devlink *devlink,
- struct genl_info *info)
-{
- return devlink_sb_get_from_attrs(devlink, info->attrs);
-}
-
-static int devlink_sb_pool_index_get_from_attrs(struct devlink_sb *devlink_sb,
- struct nlattr **attrs,
- u16 *p_pool_index)
+static int devlink_sb_pool_index_get_from_info(struct devlink_sb *devlink_sb,
+ struct genl_info *info,
+ u16 *p_pool_index)
{
+ struct nlattr **attrs = info->attrs;
u16 val;
if (!attrs[DEVLINK_ATTR_SB_POOL_INDEX])
@@ -407,18 +396,11 @@ static int devlink_sb_pool_index_get_from_attrs(struct devlink_sb *devlink_sb,
return 0;
}
-static int devlink_sb_pool_index_get_from_info(struct devlink_sb *devlink_sb,
- struct genl_info *info,
- u16 *p_pool_index)
-{
- return devlink_sb_pool_index_get_from_attrs(devlink_sb, info->attrs,
- p_pool_index);
-}
-
static int
-devlink_sb_pool_type_get_from_attrs(struct nlattr **attrs,
- enum devlink_sb_pool_type *p_pool_type)
+devlink_sb_pool_type_get_from_info(struct genl_info *info,
+ enum devlink_sb_pool_type *p_pool_type)
{
+ struct nlattr **attrs = info->attrs;
u8 val;
if (!attrs[DEVLINK_ATTR_SB_POOL_TYPE])
@@ -433,16 +415,10 @@ devlink_sb_pool_type_get_from_attrs(struct nlattr **attrs,
}
static int
-devlink_sb_pool_type_get_from_info(struct genl_info *info,
- enum devlink_sb_pool_type *p_pool_type)
-{
- return devlink_sb_pool_type_get_from_attrs(info->attrs, p_pool_type);
-}
-
-static int
-devlink_sb_th_type_get_from_attrs(struct nlattr **attrs,
- enum devlink_sb_threshold_type *p_th_type)
+devlink_sb_th_type_get_from_info(struct genl_info *info,
+ enum devlink_sb_threshold_type *p_th_type)
{
+ struct nlattr **attrs = info->attrs;
u8 val;
if (!attrs[DEVLINK_ATTR_SB_POOL_THRESHOLD_TYPE])
@@ -457,18 +433,12 @@ devlink_sb_th_type_get_from_attrs(struct nlattr **attrs,
}
static int
-devlink_sb_th_type_get_from_info(struct genl_info *info,
- enum devlink_sb_threshold_type *p_th_type)
-{
- return devlink_sb_th_type_get_from_attrs(info->attrs, p_th_type);
-}
-
-static int
-devlink_sb_tc_index_get_from_attrs(struct devlink_sb *devlink_sb,
- struct nlattr **attrs,
- enum devlink_sb_pool_type pool_type,
- u16 *p_tc_index)
+devlink_sb_tc_index_get_from_info(struct devlink_sb *devlink_sb,
+ struct genl_info *info,
+ enum devlink_sb_pool_type pool_type,
+ u16 *p_tc_index)
{
+ struct nlattr **attrs = info->attrs;
u16 val;
if (!attrs[DEVLINK_ATTR_SB_TC_INDEX])
@@ -485,16 +455,6 @@ devlink_sb_tc_index_get_from_attrs(struct devlink_sb *devlink_sb,
return 0;
}
-static int
-devlink_sb_tc_index_get_from_info(struct devlink_sb *devlink_sb,
- struct genl_info *info,
- enum devlink_sb_pool_type pool_type,
- u16 *p_tc_index)
-{
- return devlink_sb_tc_index_get_from_attrs(devlink_sb, info->attrs,
- pool_type, p_tc_index);
-}
-
struct devlink_region {
struct devlink *devlink;
struct devlink_port *port;
@@ -1975,7 +1935,7 @@ static int devlink_nl_cmd_sb_get_doit(struct sk_buff *skb,
struct sk_buff *msg;
int err;
- devlink_sb = devlink_sb_get_from_info(devlink, info);
+ devlink_sb = devlink_sb_get_from_attrs(devlink, info->attrs);
if (IS_ERR(devlink_sb))
return PTR_ERR(devlink_sb);
@@ -2093,7 +2053,7 @@ static int devlink_nl_cmd_sb_pool_get_doit(struct sk_buff *skb,
u16 pool_index;
int err;
- devlink_sb = devlink_sb_get_from_info(devlink, info);
+ devlink_sb = devlink_sb_get_from_attrs(devlink, info->attrs);
if (IS_ERR(devlink_sb))
return PTR_ERR(devlink_sb);
@@ -2217,7 +2177,7 @@ static int devlink_nl_cmd_sb_pool_set_doit(struct sk_buff *skb,
u32 size;
int err;
- devlink_sb = devlink_sb_get_from_info(devlink, info);
+ devlink_sb = devlink_sb_get_from_attrs(devlink, info->attrs);
if (IS_ERR(devlink_sb))
return PTR_ERR(devlink_sb);
@@ -2308,7 +2268,7 @@ static int devlink_nl_cmd_sb_port_pool_get_doit(struct sk_buff *skb,
u16 pool_index;
int err;
- devlink_sb = devlink_sb_get_from_info(devlink, info);
+ devlink_sb = devlink_sb_get_from_attrs(devlink, info->attrs);
if (IS_ERR(devlink_sb))
return PTR_ERR(devlink_sb);
@@ -2438,7 +2398,7 @@ static int devlink_nl_cmd_sb_port_pool_set_doit(struct sk_buff *skb,
u32 threshold;
int err;
- devlink_sb = devlink_sb_get_from_info(devlink, info);
+ devlink_sb = devlink_sb_get_from_attrs(devlink, info->attrs);
if (IS_ERR(devlink_sb))
return PTR_ERR(devlink_sb);
@@ -2531,7 +2491,7 @@ static int devlink_nl_cmd_sb_tc_pool_bind_get_doit(struct sk_buff *skb,
u16 tc_index;
int err;
- devlink_sb = devlink_sb_get_from_info(devlink, info);
+ devlink_sb = devlink_sb_get_from_attrs(devlink, info->attrs);
if (IS_ERR(devlink_sb))
return PTR_ERR(devlink_sb);
@@ -2695,7 +2655,7 @@ static int devlink_nl_cmd_sb_tc_pool_bind_set_doit(struct sk_buff *skb,
u32 threshold;
int err;
- devlink_sb = devlink_sb_get_from_info(devlink, info);
+ devlink_sb = devlink_sb_get_from_attrs(devlink, info->attrs);
if (IS_ERR(devlink_sb))
return PTR_ERR(devlink_sb);
@@ -2729,7 +2689,7 @@ static int devlink_nl_cmd_sb_occ_snapshot_doit(struct sk_buff *skb,
const struct devlink_ops *ops = devlink->ops;
struct devlink_sb *devlink_sb;
- devlink_sb = devlink_sb_get_from_info(devlink, info);
+ devlink_sb = devlink_sb_get_from_attrs(devlink, info->attrs);
if (IS_ERR(devlink_sb))
return PTR_ERR(devlink_sb);
@@ -2745,7 +2705,7 @@ static int devlink_nl_cmd_sb_occ_max_clear_doit(struct sk_buff *skb,
const struct devlink_ops *ops = devlink->ops;
struct devlink_sb *devlink_sb;
- devlink_sb = devlink_sb_get_from_info(devlink, info);
+ devlink_sb = devlink_sb_get_from_attrs(devlink, info->attrs);
if (IS_ERR(devlink_sb))
return PTR_ERR(devlink_sb);
@@ -9653,29 +9613,24 @@ int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
u16 egress_tc_count)
{
struct devlink_sb *devlink_sb;
- int err = 0;
- mutex_lock(&devlink->lock);
- if (devlink_sb_index_exists(devlink, sb_index)) {
- err = -EEXIST;
- goto unlock;
- }
+ WARN_ON(devlink_sb_get_by_index(devlink, sb_index));
devlink_sb = kzalloc(sizeof(*devlink_sb), GFP_KERNEL);
- if (!devlink_sb) {
- err = -ENOMEM;
- goto unlock;
- }
+ if (!devlink_sb)
+ return -ENOMEM;
+
devlink_sb->index = sb_index;
devlink_sb->size = size;
devlink_sb->ingress_pools_count = ingress_pools_count;
devlink_sb->egress_pools_count = egress_pools_count;
devlink_sb->ingress_tc_count = ingress_tc_count;
devlink_sb->egress_tc_count = egress_tc_count;
+
+ mutex_lock(&devlink->lock);
list_add_tail(&devlink_sb->list, &devlink->sb_list);
-unlock:
mutex_unlock(&devlink->lock);
- return err;
+ return 0;
}
EXPORT_SYMBOL_GPL(devlink_sb_register);
@@ -9683,9 +9638,10 @@ void devlink_sb_unregister(struct devlink *devlink, unsigned int sb_index)
{
struct devlink_sb *devlink_sb;
- mutex_lock(&devlink->lock);
devlink_sb = devlink_sb_get_by_index(devlink, sb_index);
WARN_ON(!devlink_sb);
+
+ mutex_lock(&devlink->lock);
list_del(&devlink_sb->list);
mutex_unlock(&devlink->lock);
kfree(devlink_sb);
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 08/16] devlink: Protect resource list with specific lock
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (6 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 07/16] devlink: Inline sb related functions Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 09/16] devlink: Protect all traps lists with specialized lock Leon Romanovsky
` (7 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
Devlink resource flows relied on devlink instance lock to protect
from parallel addition and deletion from resource_list. So instead
of overloading devlink->lock, let's introduce new lock with much more
clear scope.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 100 +++++++++++++++++++++++++--------------------
1 file changed, 56 insertions(+), 44 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 86db7cf1f3ca..97154219fca2 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -46,6 +46,7 @@ struct devlink {
struct list_head sb_list;
struct list_head dpipe_table_list;
struct list_head resource_list;
+ struct mutex resource_list_lock; /* protects resource_list */
struct list_head param_list;
struct list_head region_list;
struct list_head reporter_list;
@@ -3587,12 +3588,15 @@ devlink_resource_find(struct devlink *devlink,
}
static void
-devlink_resource_validate_children(struct devlink_resource *resource)
+devlink_resource_validate_children(struct devlink *devlink,
+ struct devlink_resource *resource)
{
struct devlink_resource *child_resource;
bool size_valid = true;
u64 parts_size = 0;
+ lockdep_assert_held(&devlink->lock);
+
if (list_empty(&resource->resource_list))
goto out;
@@ -3645,20 +3649,25 @@ static int devlink_nl_cmd_resource_set(struct sk_buff *skb,
return -EINVAL;
resource_id = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_ID]);
+ mutex_lock(&devlink->resource_list_lock);
resource = devlink_resource_find(devlink, NULL, resource_id);
- if (!resource)
- return -EINVAL;
+ if (!resource) {
+ err = -EINVAL;
+ goto out;
+ }
size = nla_get_u64(info->attrs[DEVLINK_ATTR_RESOURCE_SIZE]);
err = devlink_resource_validate_size(resource, size, info->extack);
if (err)
- return err;
+ goto out;
resource->size_new = size;
- devlink_resource_validate_children(resource);
+ devlink_resource_validate_children(devlink, resource);
if (resource->parent)
- devlink_resource_validate_children(resource->parent);
- return 0;
+ devlink_resource_validate_children(devlink, resource->parent);
+out:
+ mutex_unlock(&devlink->resource_list_lock);
+ return err;
}
static int
@@ -3742,31 +3751,36 @@ static int devlink_resource_put(struct devlink *devlink, struct sk_buff *skb,
return -EMSGSIZE;
}
-static int devlink_resource_fill(struct genl_info *info,
- enum devlink_command cmd, int flags)
+static int devlink_nl_cmd_resource_dump(struct sk_buff *sk,
+ struct genl_info *info)
{
struct devlink *devlink = info->user_ptr[0];
struct devlink_resource *resource;
struct nlattr *resources_attr;
struct sk_buff *skb = NULL;
+ int err = -EOPNOTSUPP;
struct nlmsghdr *nlh;
bool incomplete;
void *hdr;
int i;
- int err;
+
+ mutex_lock(&devlink->resource_list_lock);
+ if (list_empty(&devlink->resource_list))
+ goto out;
resource = list_first_entry(&devlink->resource_list,
struct devlink_resource, list);
start_again:
err = devlink_dpipe_send_and_alloc_skb(&skb, info);
if (err)
- return err;
+ goto out;
hdr = genlmsg_put(skb, info->snd_portid, info->snd_seq,
- &devlink_nl_family, NLM_F_MULTI, cmd);
+ &devlink_nl_family, NLM_F_MULTI,
+ DEVLINK_CMD_RESOURCE_DUMP);
if (!hdr) {
- nlmsg_free(skb);
- return -EMSGSIZE;
+ err = -EMSGSIZE;
+ goto err_resource_put;
}
if (devlink_nl_put_handle(skb, devlink))
@@ -3793,9 +3807,10 @@ static int devlink_resource_fill(struct genl_info *info,
genlmsg_end(skb, hdr);
if (incomplete)
goto start_again;
+ mutex_unlock(&devlink->resource_list_lock);
send_done:
- nlh = nlmsg_put(skb, info->snd_portid, info->snd_seq,
- NLMSG_DONE, 0, flags | NLM_F_MULTI);
+ nlh = nlmsg_put(skb, info->snd_portid, info->snd_seq, NLMSG_DONE, 0,
+ NLM_F_MULTI);
if (!nlh) {
err = devlink_dpipe_send_and_alloc_skb(&skb, info);
if (err)
@@ -3808,28 +3823,20 @@ static int devlink_resource_fill(struct genl_info *info,
err = -EMSGSIZE;
err_resource_put:
nlmsg_free(skb);
+out:
+ mutex_unlock(&devlink->resource_list_lock);
return err;
}
-static int devlink_nl_cmd_resource_dump(struct sk_buff *skb,
- struct genl_info *info)
-{
- struct devlink *devlink = info->user_ptr[0];
-
- if (list_empty(&devlink->resource_list))
- return -EOPNOTSUPP;
-
- return devlink_resource_fill(info, DEVLINK_CMD_RESOURCE_DUMP, 0);
-}
-
static int
devlink_resources_validate(struct devlink *devlink,
- struct devlink_resource *resource,
- struct genl_info *info)
+ struct devlink_resource *resource)
{
struct list_head *resource_list;
int err = 0;
+ lockdep_assert_held(&devlink->lock);
+
if (resource)
resource_list = &resource->resource_list;
else
@@ -3838,9 +3845,10 @@ devlink_resources_validate(struct devlink *devlink,
list_for_each_entry(resource, resource_list, list) {
if (!resource->size_valid)
return -EINVAL;
- err = devlink_resources_validate(devlink, resource, info);
+
+ err = devlink_resources_validate(devlink, resource);
if (err)
- return err;
+ break;
}
return err;
}
@@ -4064,7 +4072,9 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
if (!(devlink->features & DEVLINK_F_RELOAD))
return -EOPNOTSUPP;
- err = devlink_resources_validate(devlink, NULL, info);
+ mutex_lock(&devlink->resource_list_lock);
+ err = devlink_resources_validate(devlink, NULL);
+ mutex_unlock(&devlink->resource_list_lock);
if (err) {
NL_SET_ERR_MSG_MOD(info->extack, "resources size validation failed");
return err;
@@ -8955,7 +8965,10 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
INIT_LIST_HEAD(&devlink->rate_list);
INIT_LIST_HEAD(&devlink->sb_list);
INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
+
INIT_LIST_HEAD(&devlink->resource_list);
+ mutex_init(&devlink->resource_list_lock);
+
INIT_LIST_HEAD(&devlink->param_list);
INIT_LIST_HEAD(&devlink->region_list);
INIT_LIST_HEAD(&devlink->reporter_list);
@@ -9108,6 +9121,7 @@ void devlink_free(struct devlink *devlink)
mutex_destroy(&devlink->reporters_lock);
mutex_destroy(&devlink->port_list_lock);
+ mutex_destroy(&devlink->resource_list_lock);
mutex_destroy(&devlink->lock);
WARN_ON(!list_empty(&devlink->trap_policer_list));
WARN_ON(!list_empty(&devlink->trap_group_list));
@@ -9825,7 +9839,7 @@ int devlink_resource_register(struct devlink *devlink,
sizeof(resource->size_params));
INIT_LIST_HEAD(&resource->resource_list);
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->resource_list_lock);
if (parent_resource_id == DEVLINK_RESOURCE_ID_PARENT_TOP) {
resource_list = &devlink->resource_list;
} else {
@@ -9845,7 +9859,7 @@ int devlink_resource_register(struct devlink *devlink,
list_add_tail(&resource->list, resource_list);
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
return err;
}
EXPORT_SYMBOL_GPL(devlink_resource_register);
@@ -9872,16 +9886,14 @@ void devlink_resources_unregister(struct devlink *devlink)
{
struct devlink_resource *tmp, *child_resource;
- mutex_lock(&devlink->lock);
-
+ mutex_lock(&devlink->resource_list_lock);
list_for_each_entry_safe(child_resource, tmp, &devlink->resource_list,
list) {
devlink_resource_unregister(devlink, child_resource);
list_del(&child_resource->list);
kfree(child_resource);
}
-
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
}
EXPORT_SYMBOL_GPL(devlink_resources_unregister);
@@ -9899,7 +9911,7 @@ int devlink_resource_size_get(struct devlink *devlink,
struct devlink_resource *resource;
int err = 0;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->resource_list_lock);
resource = devlink_resource_find(devlink, NULL, resource_id);
if (!resource) {
err = -EINVAL;
@@ -9908,7 +9920,7 @@ int devlink_resource_size_get(struct devlink *devlink,
*p_resource_size = resource->size_new;
resource->size = resource->size_new;
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
return err;
}
EXPORT_SYMBOL_GPL(devlink_resource_size_get);
@@ -9959,7 +9971,7 @@ void devlink_resource_occ_get_register(struct devlink *devlink,
{
struct devlink_resource *resource;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->resource_list_lock);
resource = devlink_resource_find(devlink, NULL, resource_id);
if (WARN_ON(!resource))
goto out;
@@ -9968,7 +9980,7 @@ void devlink_resource_occ_get_register(struct devlink *devlink,
resource->occ_get = occ_get;
resource->occ_get_priv = occ_get_priv;
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
}
EXPORT_SYMBOL_GPL(devlink_resource_occ_get_register);
@@ -9983,7 +9995,7 @@ void devlink_resource_occ_get_unregister(struct devlink *devlink,
{
struct devlink_resource *resource;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->resource_list_lock);
resource = devlink_resource_find(devlink, NULL, resource_id);
if (WARN_ON(!resource))
goto out;
@@ -9992,7 +10004,7 @@ void devlink_resource_occ_get_unregister(struct devlink *devlink,
resource->occ_get = NULL;
resource->occ_get_priv = NULL;
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->resource_list_lock);
}
EXPORT_SYMBOL_GPL(devlink_resource_occ_get_unregister);
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 09/16] devlink: Protect all traps lists with specialized lock
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (7 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 08/16] devlink: Protect resource list with specific lock Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 10/16] devlink: Separate region list protection to be done " Leon Romanovsky
` (6 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
Separate traps related list protection from main devlink instance lock
to rely on specialized lock that will protect traps, traps groups and
policies.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 154 ++++++++++++++++++++++++++++++---------------
1 file changed, 102 insertions(+), 52 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 97154219fca2..e89eaaba653d 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -55,6 +55,7 @@ struct devlink {
struct list_head trap_list;
struct list_head trap_group_list;
struct list_head trap_policer_list;
+ struct mutex traps_lock; /* protects all traps activities */
const struct devlink_ops *ops;
u64 features;
struct xarray snapshot_ids;
@@ -7534,6 +7535,8 @@ devlink_trap_item_lookup(struct devlink *devlink, const char *name)
{
struct devlink_trap_item *trap_item;
+ lockdep_assert_held(&devlink->traps_lock);
+
list_for_each_entry(trap_item, &devlink->trap_list, list) {
if (!strcmp(trap_item->trap->name, name))
return trap_item;
@@ -7753,21 +7756,25 @@ static int devlink_nl_cmd_trap_get_doit(struct sk_buff *skb,
struct netlink_ext_ack *extack = info->extack;
struct devlink *devlink = info->user_ptr[0];
struct devlink_trap_item *trap_item;
+ int err = -EOPNOTSUPP;
struct sk_buff *msg;
- int err;
+ mutex_lock(&devlink->traps_lock);
if (list_empty(&devlink->trap_list))
- return -EOPNOTSUPP;
+ goto out;
trap_item = devlink_trap_item_get_from_info(devlink, info);
if (!trap_item) {
NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap");
- return -ENOENT;
+ err = -ENOENT;
+ goto out;
}
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
+ if (!msg) {
+ err = -ENOMEM;
+ goto out;
+ }
err = devlink_nl_trap_fill(msg, devlink, trap_item,
DEVLINK_CMD_TRAP_NEW, info->snd_portid,
@@ -7775,10 +7782,13 @@ static int devlink_nl_cmd_trap_get_doit(struct sk_buff *skb,
if (err)
goto err_trap_fill;
+ mutex_unlock(&devlink->traps_lock);
return genlmsg_reply(msg, info);
err_trap_fill:
nlmsg_free(msg);
+out:
+ mutex_unlock(&devlink->traps_lock);
return err;
}
@@ -7800,7 +7810,7 @@ static int devlink_nl_cmd_trap_get_dumpit(struct sk_buff *msg,
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->traps_lock);
list_for_each_entry(trap_item, &devlink->trap_list, list) {
if (idx < start) {
idx++;
@@ -7812,13 +7822,13 @@ static int devlink_nl_cmd_trap_get_dumpit(struct sk_buff *msg,
cb->nlh->nlmsg_seq,
NLM_F_MULTI);
if (err) {
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
devlink_put(devlink);
goto out;
}
idx++;
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
retry:
devlink_put(devlink);
}
@@ -7878,17 +7888,23 @@ static int devlink_nl_cmd_trap_set_doit(struct sk_buff *skb,
struct netlink_ext_ack *extack = info->extack;
struct devlink *devlink = info->user_ptr[0];
struct devlink_trap_item *trap_item;
+ int err = -EOPNOTSUPP;
+ mutex_lock(&devlink->traps_lock);
if (list_empty(&devlink->trap_list))
- return -EOPNOTSUPP;
+ goto out;
trap_item = devlink_trap_item_get_from_info(devlink, info);
if (!trap_item) {
NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap");
- return -ENOENT;
+ err = -ENOENT;
+ goto out;
}
- return devlink_trap_action_set(devlink, trap_item, info);
+ err = devlink_trap_action_set(devlink, trap_item, info);
+out:
+ mutex_unlock(&devlink->traps_lock);
+ return err;
}
static struct devlink_trap_group_item *
@@ -7978,21 +7994,25 @@ static int devlink_nl_cmd_trap_group_get_doit(struct sk_buff *skb,
struct netlink_ext_ack *extack = info->extack;
struct devlink *devlink = info->user_ptr[0];
struct devlink_trap_group_item *group_item;
+ int err = -EOPNOTSUPP;
struct sk_buff *msg;
- int err;
+ mutex_lock(&devlink->traps_lock);
if (list_empty(&devlink->trap_group_list))
- return -EOPNOTSUPP;
+ goto out;
group_item = devlink_trap_group_item_get_from_info(devlink, info);
if (!group_item) {
NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap group");
- return -ENOENT;
+ err = -ENOENT;
+ goto out;
}
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
+ if (!msg) {
+ err = -ENOMEM;
+ goto out;
+ }
err = devlink_nl_trap_group_fill(msg, devlink, group_item,
DEVLINK_CMD_TRAP_GROUP_NEW,
@@ -8000,10 +8020,13 @@ static int devlink_nl_cmd_trap_group_get_doit(struct sk_buff *skb,
if (err)
goto err_trap_group_fill;
+ mutex_unlock(&devlink->traps_lock);
return genlmsg_reply(msg, info);
err_trap_group_fill:
nlmsg_free(msg);
+out:
+ mutex_unlock(&devlink->traps_lock);
return err;
}
@@ -8027,7 +8050,7 @@ static int devlink_nl_cmd_trap_group_get_dumpit(struct sk_buff *msg,
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->traps_lock);
list_for_each_entry(group_item, &devlink->trap_group_list,
list) {
if (idx < start) {
@@ -8040,13 +8063,13 @@ static int devlink_nl_cmd_trap_group_get_dumpit(struct sk_buff *msg,
cb->nlh->nlmsg_seq,
NLM_F_MULTI);
if (err) {
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
devlink_put(devlink);
goto out;
}
idx++;
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
retry:
devlink_put(devlink);
}
@@ -8067,6 +8090,8 @@ __devlink_trap_group_action_set(struct devlink *devlink,
struct devlink_trap_item *trap_item;
int err;
+ lockdep_assert_held(&devlink->traps_lock);
+
if (devlink->ops->trap_group_action_set) {
err = devlink->ops->trap_group_action_set(devlink, group_item->group,
trap_action, extack);
@@ -8171,31 +8196,35 @@ static int devlink_nl_cmd_trap_group_set_doit(struct sk_buff *skb,
struct devlink *devlink = info->user_ptr[0];
struct devlink_trap_group_item *group_item;
bool modified = false;
- int err;
+ int err = -EOPNOTSUPP;
+ mutex_lock(&devlink->traps_lock);
if (list_empty(&devlink->trap_group_list))
- return -EOPNOTSUPP;
+ goto out;
group_item = devlink_trap_group_item_get_from_info(devlink, info);
if (!group_item) {
NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap group");
- return -ENOENT;
+ err = -ENOENT;
+ goto out;
}
err = devlink_trap_group_action_set(devlink, group_item, info,
&modified);
if (err)
- return err;
+ goto out;
err = devlink_trap_group_set(devlink, group_item, info);
if (err)
goto err_trap_group_set;
-
+ mutex_unlock(&devlink->traps_lock);
return 0;
err_trap_group_set:
if (modified)
NL_SET_ERR_MSG_MOD(extack, "Trap group set failed, but some changes were committed already");
+out:
+ mutex_unlock(&devlink->traps_lock);
return err;
}
@@ -8292,21 +8321,25 @@ static int devlink_nl_cmd_trap_policer_get_doit(struct sk_buff *skb,
struct devlink_trap_policer_item *policer_item;
struct netlink_ext_ack *extack = info->extack;
struct devlink *devlink = info->user_ptr[0];
+ int err = -EOPNOTSUPP;
struct sk_buff *msg;
- int err;
+ mutex_lock(&devlink->traps_lock);
if (list_empty(&devlink->trap_policer_list))
- return -EOPNOTSUPP;
+ goto out;
policer_item = devlink_trap_policer_item_get_from_info(devlink, info);
if (!policer_item) {
NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap policer");
- return -ENOENT;
+ err = -ENOENT;
+ goto out;
}
msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
- if (!msg)
- return -ENOMEM;
+ if (!msg) {
+ err = -ENOMEM;
+ goto out;
+ }
err = devlink_nl_trap_policer_fill(msg, devlink, policer_item,
DEVLINK_CMD_TRAP_POLICER_NEW,
@@ -8314,10 +8347,13 @@ static int devlink_nl_cmd_trap_policer_get_doit(struct sk_buff *skb,
if (err)
goto err_trap_policer_fill;
+ mutex_unlock(&devlink->traps_lock);
return genlmsg_reply(msg, info);
err_trap_policer_fill:
nlmsg_free(msg);
+out:
+ mutex_unlock(&devlink->traps_lock);
return err;
}
@@ -8341,7 +8377,7 @@ static int devlink_nl_cmd_trap_policer_get_dumpit(struct sk_buff *msg,
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->traps_lock);
list_for_each_entry(policer_item, &devlink->trap_policer_list,
list) {
if (idx < start) {
@@ -8354,13 +8390,13 @@ static int devlink_nl_cmd_trap_policer_get_dumpit(struct sk_buff *msg,
cb->nlh->nlmsg_seq,
NLM_F_MULTI);
if (err) {
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
devlink_put(devlink);
goto out;
}
idx++;
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
retry:
devlink_put(devlink);
}
@@ -8427,20 +8463,26 @@ static int devlink_nl_cmd_trap_policer_set_doit(struct sk_buff *skb,
struct devlink_trap_policer_item *policer_item;
struct netlink_ext_ack *extack = info->extack;
struct devlink *devlink = info->user_ptr[0];
-
- if (list_empty(&devlink->trap_policer_list))
- return -EOPNOTSUPP;
+ int err = -EOPNOTSUPP;
if (!devlink->ops->trap_policer_set)
return -EOPNOTSUPP;
+ mutex_lock(&devlink->traps_lock);
+ if (list_empty(&devlink->trap_policer_list))
+ goto out;
+
policer_item = devlink_trap_policer_item_get_from_info(devlink, info);
if (!policer_item) {
NL_SET_ERR_MSG_MOD(extack, "Device did not register this trap policer");
- return -ENOENT;
+ err = -ENOENT;
+ goto out;
}
- return devlink_trap_policer_set(devlink, policer_item, info);
+ err = devlink_trap_policer_set(devlink, policer_item, info);
+out:
+ mutex_unlock(&devlink->traps_lock);
+ return err;
}
static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
@@ -8972,9 +9014,12 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
INIT_LIST_HEAD(&devlink->param_list);
INIT_LIST_HEAD(&devlink->region_list);
INIT_LIST_HEAD(&devlink->reporter_list);
+
INIT_LIST_HEAD(&devlink->trap_list);
INIT_LIST_HEAD(&devlink->trap_group_list);
INIT_LIST_HEAD(&devlink->trap_policer_list);
+ mutex_init(&devlink->traps_lock);
+
mutex_init(&devlink->lock);
mutex_init(&devlink->reporters_lock);
refcount_set(&devlink->refcount, 1);
@@ -9012,6 +9057,7 @@ static void devlink_notify_register(struct devlink *devlink)
devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_NEW);
mutex_unlock(&devlink->port_list_lock);
+ mutex_lock(&devlink->traps_lock);
list_for_each_entry(policer_item, &devlink->trap_policer_list, list)
devlink_trap_policer_notify(devlink, policer_item,
DEVLINK_CMD_TRAP_POLICER_NEW);
@@ -9022,6 +9068,7 @@ static void devlink_notify_register(struct devlink *devlink)
list_for_each_entry(trap_item, &devlink->trap_list, list)
devlink_trap_notify(devlink, trap_item, DEVLINK_CMD_TRAP_NEW);
+ mutex_unlock(&devlink->traps_lock);
list_for_each_entry(rate_node, &devlink->rate_list, list)
devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW);
@@ -9054,6 +9101,7 @@ static void devlink_notify_unregister(struct devlink *devlink)
list_for_each_entry_reverse(rate_node, &devlink->rate_list, list)
devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_DEL);
+ mutex_lock(&devlink->traps_lock);
list_for_each_entry_reverse(trap_item, &devlink->trap_list, list)
devlink_trap_notify(devlink, trap_item, DEVLINK_CMD_TRAP_DEL);
@@ -9064,6 +9112,7 @@ static void devlink_notify_unregister(struct devlink *devlink)
list)
devlink_trap_policer_notify(devlink, policer_item,
DEVLINK_CMD_TRAP_POLICER_DEL);
+ mutex_unlock(&devlink->traps_lock);
mutex_lock(&devlink->port_list_lock);
list_for_each_entry_reverse(devlink_port, &devlink->port_list, list)
@@ -9122,6 +9171,7 @@ void devlink_free(struct devlink *devlink)
mutex_destroy(&devlink->reporters_lock);
mutex_destroy(&devlink->port_list_lock);
mutex_destroy(&devlink->resource_list_lock);
+ mutex_destroy(&devlink->traps_lock);
mutex_destroy(&devlink->lock);
WARN_ON(!list_empty(&devlink->trap_policer_list));
WARN_ON(!list_empty(&devlink->trap_group_list));
@@ -10796,7 +10846,7 @@ int devlink_traps_register(struct devlink *devlink,
if (!devlink->ops->trap_init || !devlink->ops->trap_action_set)
return -EINVAL;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->traps_lock);
for (i = 0; i < traps_count; i++) {
const struct devlink_trap *trap = &traps[i];
@@ -10808,7 +10858,7 @@ int devlink_traps_register(struct devlink *devlink,
if (err)
goto err_trap_register;
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
return 0;
@@ -10816,7 +10866,7 @@ int devlink_traps_register(struct devlink *devlink,
err_trap_verify:
for (i--; i >= 0; i--)
devlink_trap_unregister(devlink, &traps[i]);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
return err;
}
EXPORT_SYMBOL_GPL(devlink_traps_register);
@@ -10833,7 +10883,7 @@ void devlink_traps_unregister(struct devlink *devlink,
{
int i;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->traps_lock);
/* Make sure we do not have any packets in-flight while unregistering
* traps by disabling all of them and waiting for a grace period.
*/
@@ -10842,7 +10892,7 @@ void devlink_traps_unregister(struct devlink *devlink,
synchronize_rcu();
for (i = traps_count - 1; i >= 0; i--)
devlink_trap_unregister(devlink, &traps[i]);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
}
EXPORT_SYMBOL_GPL(devlink_traps_unregister);
@@ -11014,7 +11064,7 @@ int devlink_trap_groups_register(struct devlink *devlink,
{
int i, err;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->traps_lock);
for (i = 0; i < groups_count; i++) {
const struct devlink_trap_group *group = &groups[i];
@@ -11026,7 +11076,7 @@ int devlink_trap_groups_register(struct devlink *devlink,
if (err)
goto err_trap_group_register;
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
return 0;
@@ -11034,7 +11084,7 @@ int devlink_trap_groups_register(struct devlink *devlink,
err_trap_group_verify:
for (i--; i >= 0; i--)
devlink_trap_group_unregister(devlink, &groups[i]);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
return err;
}
EXPORT_SYMBOL_GPL(devlink_trap_groups_register);
@@ -11051,10 +11101,10 @@ void devlink_trap_groups_unregister(struct devlink *devlink,
{
int i;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->traps_lock);
for (i = groups_count - 1; i >= 0; i--)
devlink_trap_group_unregister(devlink, &groups[i]);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
}
EXPORT_SYMBOL_GPL(devlink_trap_groups_unregister);
@@ -11154,7 +11204,7 @@ devlink_trap_policers_register(struct devlink *devlink,
{
int i, err;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->traps_lock);
for (i = 0; i < policers_count; i++) {
const struct devlink_trap_policer *policer = &policers[i];
@@ -11169,7 +11219,7 @@ devlink_trap_policers_register(struct devlink *devlink,
if (err)
goto err_trap_policer_register;
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
return 0;
@@ -11177,7 +11227,7 @@ devlink_trap_policers_register(struct devlink *devlink,
err_trap_policer_verify:
for (i--; i >= 0; i--)
devlink_trap_policer_unregister(devlink, &policers[i]);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
return err;
}
EXPORT_SYMBOL_GPL(devlink_trap_policers_register);
@@ -11195,10 +11245,10 @@ devlink_trap_policers_unregister(struct devlink *devlink,
{
int i;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->traps_lock);
for (i = policers_count - 1; i >= 0; i--)
devlink_trap_policer_unregister(devlink, &policers[i]);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->traps_lock);
}
EXPORT_SYMBOL_GPL(devlink_trap_policers_unregister);
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 10/16] devlink: Separate region list protection to be done with specialized lock
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (8 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 09/16] devlink: Protect all traps lists with specialized lock Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 11/16] devlink: Protect all rate operations " Leon Romanovsky
` (5 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
Reduce scope of devlink->lock and use dedicated lock to protect region_list.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 66 +++++++++++++++++++++++++++-------------------
1 file changed, 39 insertions(+), 27 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index e89eaaba653d..52a1255a0917 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -49,6 +49,7 @@ struct devlink {
struct mutex resource_list_lock; /* protects resource_list */
struct list_head param_list;
struct list_head region_list;
+ struct mutex region_list_lock; /* protects region_list, including port */
struct list_head reporter_list;
struct mutex reporters_lock; /* protects reporter_list */
struct devlink_dpipe_headers *dpipe_headers;
@@ -5412,25 +5413,28 @@ static int devlink_nl_cmd_region_get_port_dumpit(struct sk_buff *msg,
int *idx,
int start)
{
+ struct devlink *devlink = port->devlink;
struct devlink_region *region;
int err = 0;
+ mutex_lock(&devlink->region_list_lock);
list_for_each_entry(region, &port->region_list, list) {
if (*idx < start) {
(*idx)++;
continue;
}
- err = devlink_nl_region_fill(msg, port->devlink,
+ err = devlink_nl_region_fill(msg, devlink,
DEVLINK_CMD_REGION_GET,
NETLINK_CB(cb->skb).portid,
- cb->nlh->nlmsg_seq,
- NLM_F_MULTI, region);
+ cb->nlh->nlmsg_seq, NLM_F_MULTI,
+ region);
if (err)
goto out;
(*idx)++;
}
out:
+ mutex_unlock(&devlink->region_list_lock);
return err;
}
@@ -5444,7 +5448,7 @@ static int devlink_nl_cmd_region_get_devlink_dumpit(struct sk_buff *msg,
struct devlink_port *port;
int err = 0;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->region_list_lock);
list_for_each_entry(region, &devlink->region_list, list) {
if (*idx < start) {
(*idx)++;
@@ -5455,22 +5459,22 @@ static int devlink_nl_cmd_region_get_devlink_dumpit(struct sk_buff *msg,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq,
NLM_F_MULTI, region);
- if (err)
- goto out;
+ if (err) {
+ mutex_unlock(&devlink->region_list_lock);
+ return err;
+ }
(*idx)++;
}
+ mutex_unlock(&devlink->region_list_lock);
mutex_lock(&devlink->port_list_lock);
list_for_each_entry(port, &devlink->port_list, list) {
err = devlink_nl_cmd_region_get_port_dumpit(msg, cb, port, idx,
start);
if (err)
- goto out;
+ break;
}
-
-out:
mutex_unlock(&devlink->port_list_lock);
- mutex_unlock(&devlink->lock);
return err;
}
@@ -5760,7 +5764,7 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
goto out_dev;
}
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->region_list_lock);
if (!attrs[DEVLINK_ATTR_REGION_NAME] ||
!attrs[DEVLINK_ATTR_REGION_SNAPSHOT_ID]) {
@@ -5856,7 +5860,7 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
nla_nest_end(skb, chunks_attr);
genlmsg_end(skb, hdr);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->region_list_lock);
devlink_put(devlink);
mutex_unlock(&devlink_mutex);
@@ -5865,7 +5869,7 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
nla_put_failure:
genlmsg_cancel(skb, hdr);
out_unlock:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->region_list_lock);
devlink_put(devlink);
out_dev:
mutex_unlock(&devlink_mutex);
@@ -9012,7 +9016,10 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
mutex_init(&devlink->resource_list_lock);
INIT_LIST_HEAD(&devlink->param_list);
+
INIT_LIST_HEAD(&devlink->region_list);
+ mutex_init(&devlink->region_list_lock);
+
INIT_LIST_HEAD(&devlink->reporter_list);
INIT_LIST_HEAD(&devlink->trap_list);
@@ -9073,8 +9080,10 @@ static void devlink_notify_register(struct devlink *devlink)
list_for_each_entry(rate_node, &devlink->rate_list, list)
devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW);
+ mutex_lock(&devlink->region_list_lock);
list_for_each_entry(region, &devlink->region_list, list)
devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_NEW);
+ mutex_unlock(&devlink->region_list_lock);
list_for_each_entry(param_item, &devlink->param_list, list)
devlink_param_notify(devlink, 0, param_item,
@@ -9095,8 +9104,10 @@ static void devlink_notify_unregister(struct devlink *devlink)
devlink_param_notify(devlink, 0, param_item,
DEVLINK_CMD_PARAM_DEL);
+ mutex_lock(&devlink->region_list_lock);
list_for_each_entry_reverse(region, &devlink->region_list, list)
devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_DEL);
+ mutex_unlock(&devlink->region_list_lock);
list_for_each_entry_reverse(rate_node, &devlink->rate_list, list)
devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_DEL);
@@ -9172,6 +9183,7 @@ void devlink_free(struct devlink *devlink)
mutex_destroy(&devlink->port_list_lock);
mutex_destroy(&devlink->resource_list_lock);
mutex_destroy(&devlink->traps_lock);
+ mutex_destroy(&devlink->region_list_lock);
mutex_destroy(&devlink->lock);
WARN_ON(!list_empty(&devlink->trap_policer_list));
WARN_ON(!list_empty(&devlink->trap_group_list));
@@ -10293,7 +10305,7 @@ devlink_region_create(struct devlink *devlink,
if (WARN_ON(!ops) || WARN_ON(!ops->destructor))
return ERR_PTR(-EINVAL);
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->region_list_lock);
if (devlink_region_get_by_name(devlink, ops->name)) {
err = -EEXIST;
@@ -10314,11 +10326,11 @@ devlink_region_create(struct devlink *devlink,
list_add_tail(®ion->list, &devlink->region_list);
devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_NEW);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->region_list_lock);
return region;
unlock:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->region_list_lock);
return ERR_PTR(err);
}
EXPORT_SYMBOL_GPL(devlink_region_create);
@@ -10343,7 +10355,7 @@ devlink_port_region_create(struct devlink_port *port,
if (WARN_ON(!ops) || WARN_ON(!ops->destructor))
return ERR_PTR(-EINVAL);
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->region_list_lock);
if (devlink_port_region_get_by_name(port, ops->name)) {
err = -EEXIST;
@@ -10365,11 +10377,11 @@ devlink_port_region_create(struct devlink_port *port,
list_add_tail(®ion->list, &port->region_list);
devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_NEW);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->region_list_lock);
return region;
unlock:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->region_list_lock);
return ERR_PTR(err);
}
EXPORT_SYMBOL_GPL(devlink_port_region_create);
@@ -10384,7 +10396,7 @@ void devlink_region_destroy(struct devlink_region *region)
struct devlink *devlink = region->devlink;
struct devlink_snapshot *snapshot, *ts;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->region_list_lock);
/* Free all snapshots of region */
list_for_each_entry_safe(snapshot, ts, ®ion->snapshot_list, list)
@@ -10393,7 +10405,7 @@ void devlink_region_destroy(struct devlink_region *region)
list_del(®ion->list);
devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_DEL);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->region_list_lock);
kfree(region);
}
EXPORT_SYMBOL_GPL(devlink_region_destroy);
@@ -10417,9 +10429,9 @@ int devlink_region_snapshot_id_get(struct devlink *devlink, u32 *id)
{
int err;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->region_list_lock);
err = __devlink_region_snapshot_id_get(devlink, id);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->region_list_lock);
return err;
}
@@ -10437,9 +10449,9 @@ EXPORT_SYMBOL_GPL(devlink_region_snapshot_id_get);
*/
void devlink_region_snapshot_id_put(struct devlink *devlink, u32 id)
{
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->region_list_lock);
__devlink_snapshot_id_decrement(devlink, id);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->region_list_lock);
}
EXPORT_SYMBOL_GPL(devlink_region_snapshot_id_put);
@@ -10461,9 +10473,9 @@ int devlink_region_snapshot_create(struct devlink_region *region,
struct devlink *devlink = region->devlink;
int err;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->region_list_lock);
err = __devlink_region_snapshot_create(region, data, snapshot_id);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->region_list_lock);
return err;
}
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 11/16] devlink: Protect all rate operations with specialized lock
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (9 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 10/16] devlink: Separate region list protection to be done " Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 12/16] devlink: Protect all sb " Leon Romanovsky
` (4 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
Separate rate related list protection from main devlink instance lock
to rely on specialized lock.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 60 +++++++++++++++++++++++++++-------------------
1 file changed, 36 insertions(+), 24 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 52a1255a0917..008826bc108d 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -43,6 +43,7 @@ struct devlink {
struct list_head port_list;
struct mutex port_list_lock; /* protects port_list */
struct list_head rate_list;
+ struct mutex rate_list_lock; /* protects rate_list */
struct list_head sb_list;
struct list_head dpipe_table_list;
struct list_head resource_list;
@@ -1143,7 +1144,7 @@ static int devlink_nl_cmd_rate_get_dumpit(struct sk_buff *msg,
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->rate_list_lock);
list_for_each_entry(devlink_rate, &devlink->rate_list, list) {
enum devlink_command cmd = DEVLINK_CMD_RATE_NEW;
u32 id = NETLINK_CB(cb->skb).portid;
@@ -1156,13 +1157,13 @@ static int devlink_nl_cmd_rate_get_dumpit(struct sk_buff *msg,
cb->nlh->nlmsg_seq,
NLM_F_MULTI, NULL);
if (err) {
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->rate_list_lock);
devlink_put(devlink);
goto out;
}
idx++;
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->rate_list_lock);
retry:
devlink_put(devlink);
}
@@ -1829,15 +1830,21 @@ static int devlink_nl_cmd_rate_new_doit(struct sk_buff *skb,
if (!devlink_rate_set_ops_supported(ops, info, DEVLINK_RATE_TYPE_NODE))
return -EOPNOTSUPP;
+ mutex_lock(&devlink->rate_list_lock);
rate_node = devlink_rate_node_get_from_attrs(devlink, info->attrs);
- if (!IS_ERR(rate_node))
- return -EEXIST;
- else if (rate_node == ERR_PTR(-EINVAL))
- return -EINVAL;
+ if (!IS_ERR(rate_node)) {
+ err = -EEXIST;
+ goto out;
+ } else if (rate_node == ERR_PTR(-EINVAL)) {
+ err = -EINVAL;
+ goto out;
+ }
rate_node = kzalloc(sizeof(*rate_node), GFP_KERNEL);
- if (!rate_node)
- return -ENOMEM;
+ if (!rate_node) {
+ err = -ENOMEM;
+ goto out;
+ }
rate_node->devlink = devlink;
rate_node->type = DEVLINK_RATE_TYPE_NODE;
@@ -1858,6 +1865,7 @@ static int devlink_nl_cmd_rate_new_doit(struct sk_buff *skb,
refcount_set(&rate_node->refcnt, 1);
list_add(&rate_node->list, &devlink->rate_list);
devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW);
+ mutex_unlock(&devlink->rate_list_lock);
return 0;
err_rate_set:
@@ -1866,6 +1874,8 @@ static int devlink_nl_cmd_rate_new_doit(struct sk_buff *skb,
kfree(rate_node->name);
err_strdup:
kfree(rate_node);
+out:
+ mutex_unlock(&devlink->rate_list_lock);
return err;
}
@@ -2800,14 +2810,14 @@ static int devlink_rate_nodes_check(struct devlink *devlink, u16 mode,
struct devlink_rate *devlink_rate;
/* Take the lock to sync with devlink_rate_nodes_destroy() */
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->rate_list_lock);
list_for_each_entry(devlink_rate, &devlink->rate_list, list)
if (devlink_rate_is_node(devlink_rate)) {
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->rate_list_lock);
NL_SET_ERR_MSG_MOD(extack, "Rate node(s) exists.");
return -EBUSY;
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->rate_list_lock);
return 0;
}
@@ -9009,6 +9019,8 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
mutex_init(&devlink->port_list_lock);
INIT_LIST_HEAD(&devlink->rate_list);
+ mutex_init(&devlink->rate_list_lock);
+
INIT_LIST_HEAD(&devlink->sb_list);
INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
@@ -9077,8 +9089,10 @@ static void devlink_notify_register(struct devlink *devlink)
devlink_trap_notify(devlink, trap_item, DEVLINK_CMD_TRAP_NEW);
mutex_unlock(&devlink->traps_lock);
+ mutex_lock(&devlink->rate_list_lock);
list_for_each_entry(rate_node, &devlink->rate_list, list)
devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_NEW);
+ mutex_unlock(&devlink->rate_list_lock);
mutex_lock(&devlink->region_list_lock);
list_for_each_entry(region, &devlink->region_list, list)
@@ -9109,8 +9123,10 @@ static void devlink_notify_unregister(struct devlink *devlink)
devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_DEL);
mutex_unlock(&devlink->region_list_lock);
+ mutex_lock(&devlink->rate_list_lock);
list_for_each_entry_reverse(rate_node, &devlink->rate_list, list)
devlink_rate_notify(rate_node, DEVLINK_CMD_RATE_DEL);
+ mutex_unlock(&devlink->rate_list_lock);
mutex_lock(&devlink->traps_lock);
list_for_each_entry_reverse(trap_item, &devlink->trap_list, list)
@@ -9184,6 +9200,7 @@ void devlink_free(struct devlink *devlink)
mutex_destroy(&devlink->resource_list_lock);
mutex_destroy(&devlink->traps_lock);
mutex_destroy(&devlink->region_list_lock);
+ mutex_destroy(&devlink->rate_list_lock);
mutex_destroy(&devlink->lock);
WARN_ON(!list_empty(&devlink->trap_policer_list));
WARN_ON(!list_empty(&devlink->trap_group_list));
@@ -9522,8 +9539,6 @@ EXPORT_SYMBOL_GPL(devlink_port_attrs_pci_sf_set);
* Create devlink rate object of type leaf on provided @devlink_port.
* Throws call trace if @devlink_port already has a devlink rate object.
*
- * Context: Takes and release devlink->lock <mutex>.
- *
* Return: -ENOMEM if failed to allocate rate object, 0 otherwise.
*/
int
@@ -9536,16 +9551,17 @@ devlink_rate_leaf_create(struct devlink_port *devlink_port, void *priv)
if (!devlink_rate)
return -ENOMEM;
- mutex_lock(&devlink->lock);
WARN_ON(devlink_port->devlink_rate);
devlink_rate->type = DEVLINK_RATE_TYPE_LEAF;
devlink_rate->devlink = devlink;
devlink_rate->devlink_port = devlink_port;
devlink_rate->priv = priv;
+
+ mutex_lock(&devlink->rate_list_lock);
list_add_tail(&devlink_rate->list, &devlink->rate_list);
devlink_port->devlink_rate = devlink_rate;
devlink_rate_notify(devlink_rate, DEVLINK_CMD_RATE_NEW);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->rate_list_lock);
return 0;
}
@@ -9555,8 +9571,6 @@ EXPORT_SYMBOL_GPL(devlink_rate_leaf_create);
* 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)
{
@@ -9566,13 +9580,13 @@ void devlink_rate_leaf_destroy(struct devlink_port *devlink_port)
if (!devlink_rate)
return;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->rate_list_lock);
devlink_rate_notify(devlink_rate, DEVLINK_CMD_RATE_DEL);
if (devlink_rate->parent)
refcount_dec(&devlink_rate->parent->refcnt);
list_del(&devlink_rate->list);
devlink_port->devlink_rate = NULL;
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->rate_list_lock);
kfree(devlink_rate);
}
EXPORT_SYMBOL_GPL(devlink_rate_leaf_destroy);
@@ -9584,15 +9598,13 @@ EXPORT_SYMBOL_GPL(devlink_rate_leaf_destroy);
*
* 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)
{
static struct devlink_rate *devlink_rate, *tmp;
const struct devlink_ops *ops = devlink->ops;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->rate_list_lock);
list_for_each_entry(devlink_rate, &devlink->rate_list, list) {
if (!devlink_rate->parent)
continue;
@@ -9613,7 +9625,7 @@ void devlink_rate_nodes_destroy(struct devlink *devlink)
kfree(devlink_rate);
}
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->rate_list_lock);
}
EXPORT_SYMBOL_GPL(devlink_rate_nodes_destroy);
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 12/16] devlink: Protect all sb operations with specialized lock
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (10 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 11/16] devlink: Protect all rate operations " Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 13/16] devlink: Convert dpipe to use dpipe_lock Leon Romanovsky
` (3 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
Separate sb related list protection from main devlink instance lock
to rely on specialized lock.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 36 ++++++++++++++++++++----------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 008826bc108d..19f1802f1e5d 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -45,6 +45,7 @@ struct devlink {
struct list_head rate_list;
struct mutex rate_list_lock; /* protects rate_list */
struct list_head sb_list;
+ struct mutex sb_list_lock; /* protects sb_list */
struct list_head dpipe_table_list;
struct list_head resource_list;
struct mutex resource_list_lock; /* protects resource_list */
@@ -1985,7 +1986,7 @@ static int devlink_nl_cmd_sb_get_dumpit(struct sk_buff *msg,
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->sb_list_lock);
list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
if (idx < start) {
idx++;
@@ -1997,13 +1998,13 @@ static int devlink_nl_cmd_sb_get_dumpit(struct sk_buff *msg,
cb->nlh->nlmsg_seq,
NLM_F_MULTI);
if (err) {
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->sb_list_lock);
devlink_put(devlink);
goto out;
}
idx++;
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->sb_list_lock);
retry:
devlink_put(devlink);
}
@@ -2138,7 +2139,7 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
!devlink->ops->sb_pool_get)
goto retry;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->sb_list_lock);
list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
err = __sb_pool_get_dumpit(msg, start, &idx, devlink,
devlink_sb,
@@ -2147,12 +2148,12 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
if (err == -EOPNOTSUPP) {
err = 0;
} else if (err) {
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->sb_list_lock);
devlink_put(devlink);
goto out;
}
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->sb_list_lock);
retry:
devlink_put(devlink);
}
@@ -2359,7 +2360,7 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
!devlink->ops->sb_port_pool_get)
goto retry;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->sb_list_lock);
list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
err = __sb_port_pool_get_dumpit(msg, start, &idx,
devlink, devlink_sb,
@@ -2368,12 +2369,12 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
if (err == -EOPNOTSUPP) {
err = 0;
} else if (err) {
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->sb_list_lock);
devlink_put(devlink);
goto out;
}
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->sb_list_lock);
retry:
devlink_put(devlink);
}
@@ -2611,7 +2612,7 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
!devlink->ops->sb_tc_pool_bind_get)
goto retry;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->sb_list_lock);
list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
err = __sb_tc_pool_bind_get_dumpit(msg, start, &idx,
devlink,
@@ -2621,12 +2622,12 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
if (err == -EOPNOTSUPP) {
err = 0;
} else if (err) {
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->sb_list_lock);
devlink_put(devlink);
goto out;
}
}
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->sb_list_lock);
retry:
devlink_put(devlink);
}
@@ -9022,6 +9023,8 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
mutex_init(&devlink->rate_list_lock);
INIT_LIST_HEAD(&devlink->sb_list);
+ mutex_init(&devlink->sb_list_lock);
+
INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
INIT_LIST_HEAD(&devlink->resource_list);
@@ -9201,6 +9204,7 @@ void devlink_free(struct devlink *devlink)
mutex_destroy(&devlink->traps_lock);
mutex_destroy(&devlink->region_list_lock);
mutex_destroy(&devlink->rate_list_lock);
+ mutex_destroy(&devlink->sb_list_lock);
mutex_destroy(&devlink->lock);
WARN_ON(!list_empty(&devlink->trap_policer_list));
WARN_ON(!list_empty(&devlink->trap_group_list));
@@ -9715,9 +9719,9 @@ int devlink_sb_register(struct devlink *devlink, unsigned int sb_index,
devlink_sb->ingress_tc_count = ingress_tc_count;
devlink_sb->egress_tc_count = egress_tc_count;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->sb_list_lock);
list_add_tail(&devlink_sb->list, &devlink->sb_list);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->sb_list_lock);
return 0;
}
EXPORT_SYMBOL_GPL(devlink_sb_register);
@@ -9729,9 +9733,9 @@ void devlink_sb_unregister(struct devlink *devlink, unsigned int sb_index)
devlink_sb = devlink_sb_get_by_index(devlink, sb_index);
WARN_ON(!devlink_sb);
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->sb_list_lock);
list_del(&devlink_sb->list);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->sb_list_lock);
kfree(devlink_sb);
}
EXPORT_SYMBOL_GPL(devlink_sb_unregister);
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 13/16] devlink: Convert dpipe to use dpipe_lock
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (11 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 12/16] devlink: Protect all sb " Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 14/16] devlink: Require devlink lock during device reload Leon Romanovsky
` (2 subsequent siblings)
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
Separate dpipe related list protection from main devlink instance lock
to rely on specialized lock.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 67 +++++++++++++++++++++++++++++-----------------
1 file changed, 43 insertions(+), 24 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 19f1802f1e5d..60af6a3fc130 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -47,6 +47,7 @@ struct devlink {
struct list_head sb_list;
struct mutex sb_list_lock; /* protects sb_list */
struct list_head dpipe_table_list;
+ struct mutex dpipe_lock; /* protects dpipe_table_list */
struct list_head resource_list;
struct mutex resource_list_lock; /* protects resource_list */
struct list_head param_list;
@@ -3105,13 +3106,16 @@ static int devlink_nl_cmd_dpipe_table_get(struct sk_buff *skb,
{
struct devlink *devlink = info->user_ptr[0];
const char *table_name = NULL;
+ int ret;
if (info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME])
table_name = nla_data(info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME]);
- return devlink_dpipe_tables_fill(info, DEVLINK_CMD_DPIPE_TABLE_GET, 0,
- &devlink->dpipe_table_list,
- table_name);
+ mutex_lock(&devlink->dpipe_lock);
+ ret = devlink_dpipe_tables_fill(info, DEVLINK_CMD_DPIPE_TABLE_GET, 0,
+ &devlink->dpipe_table_list, table_name);
+ mutex_unlock(&devlink->dpipe_lock);
+ return ret;
}
static int devlink_dpipe_value_put(struct sk_buff *skb,
@@ -3266,7 +3270,7 @@ devlink_dpipe_table_find(struct list_head *dpipe_tables,
{
struct devlink_dpipe_table *table;
list_for_each_entry_rcu(table, dpipe_tables, list,
- lockdep_is_held(&devlink->lock)) {
+ lockdep_is_held(&devlink->dpipe_lock)) {
if (!strcmp(table->name, table_name))
return table;
}
@@ -3379,21 +3383,31 @@ static int devlink_nl_cmd_dpipe_entries_get(struct sk_buff *skb,
struct devlink *devlink = info->user_ptr[0];
struct devlink_dpipe_table *table;
const char *table_name;
+ int ret;
if (!info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME])
return -EINVAL;
table_name = nla_data(info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME]);
+ mutex_lock(&devlink->dpipe_lock);
table = devlink_dpipe_table_find(&devlink->dpipe_table_list,
table_name, devlink);
- if (!table)
- return -EINVAL;
+ if (!table) {
+ ret = -EINVAL;
+ goto out;
+ }
- if (!table->table_ops->entries_dump)
- return -EINVAL;
+ if (!table->table_ops->entries_dump) {
+ ret = -EINVAL;
+ goto out;
+ }
- return devlink_dpipe_entries_fill(info, DEVLINK_CMD_DPIPE_ENTRIES_GET,
- 0, table);
+ ret = devlink_dpipe_entries_fill(info, DEVLINK_CMD_DPIPE_ENTRIES_GET, 0,
+ table);
+
+out:
+ mutex_unlock(&devlink->dpipe_lock);
+ return ret;
}
static int devlink_dpipe_fields_put(struct sk_buff *skb,
@@ -3563,6 +3577,7 @@ static int devlink_nl_cmd_dpipe_table_counters_set(struct sk_buff *skb,
struct devlink *devlink = info->user_ptr[0];
const char *table_name;
bool counters_enable;
+ int ret;
if (!info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME] ||
!info->attrs[DEVLINK_ATTR_DPIPE_TABLE_COUNTERS_ENABLED])
@@ -3571,8 +3586,11 @@ static int devlink_nl_cmd_dpipe_table_counters_set(struct sk_buff *skb,
table_name = nla_data(info->attrs[DEVLINK_ATTR_DPIPE_TABLE_NAME]);
counters_enable = !!nla_get_u8(info->attrs[DEVLINK_ATTR_DPIPE_TABLE_COUNTERS_ENABLED]);
- return devlink_dpipe_table_counters_set(devlink, table_name,
- counters_enable);
+ mutex_lock(&devlink->dpipe_lock);
+ ret = devlink_dpipe_table_counters_set(devlink, table_name,
+ counters_enable);
+ mutex_unlock(&devlink->dpipe_lock);
+ return ret;
}
static struct devlink_resource *
@@ -9026,6 +9044,7 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
mutex_init(&devlink->sb_list_lock);
INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
+ mutex_init(&devlink->dpipe_lock);
INIT_LIST_HEAD(&devlink->resource_list);
mutex_init(&devlink->resource_list_lock);
@@ -9205,6 +9224,7 @@ void devlink_free(struct devlink *devlink)
mutex_destroy(&devlink->region_list_lock);
mutex_destroy(&devlink->rate_list_lock);
mutex_destroy(&devlink->sb_list_lock);
+ mutex_destroy(&devlink->dpipe_lock);
mutex_destroy(&devlink->lock);
WARN_ON(!list_empty(&devlink->trap_policer_list));
WARN_ON(!list_empty(&devlink->trap_group_list));
@@ -9751,9 +9771,9 @@ EXPORT_SYMBOL_GPL(devlink_sb_unregister);
int devlink_dpipe_headers_register(struct devlink *devlink,
struct devlink_dpipe_headers *dpipe_headers)
{
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->dpipe_lock);
devlink->dpipe_headers = dpipe_headers;
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->dpipe_lock);
return 0;
}
EXPORT_SYMBOL_GPL(devlink_dpipe_headers_register);
@@ -9767,9 +9787,9 @@ EXPORT_SYMBOL_GPL(devlink_dpipe_headers_register);
*/
void devlink_dpipe_headers_unregister(struct devlink *devlink)
{
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->dpipe_lock);
devlink->dpipe_headers = NULL;
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->dpipe_lock);
}
EXPORT_SYMBOL_GPL(devlink_dpipe_headers_unregister);
@@ -9824,8 +9844,7 @@ int devlink_dpipe_table_register(struct devlink *devlink,
if (WARN_ON(!table_ops->size_get))
return -EINVAL;
- mutex_lock(&devlink->lock);
-
+ mutex_lock(&devlink->dpipe_lock);
if (devlink_dpipe_table_find(&devlink->dpipe_table_list, table_name,
devlink)) {
err = -EEXIST;
@@ -9845,7 +9864,7 @@ int devlink_dpipe_table_register(struct devlink *devlink,
list_add_tail_rcu(&table->list, &devlink->dpipe_table_list);
unlock:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->dpipe_lock);
return err;
}
EXPORT_SYMBOL_GPL(devlink_dpipe_table_register);
@@ -9861,17 +9880,17 @@ void devlink_dpipe_table_unregister(struct devlink *devlink,
{
struct devlink_dpipe_table *table;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->dpipe_lock);
table = devlink_dpipe_table_find(&devlink->dpipe_table_list,
table_name, devlink);
if (!table)
goto unlock;
list_del_rcu(&table->list);
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->dpipe_lock);
kfree_rcu(table, rcu);
return;
unlock:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->dpipe_lock);
}
EXPORT_SYMBOL_GPL(devlink_dpipe_table_unregister);
@@ -10018,7 +10037,7 @@ int devlink_dpipe_table_resource_set(struct devlink *devlink,
struct devlink_dpipe_table *table;
int err = 0;
- mutex_lock(&devlink->lock);
+ mutex_lock(&devlink->dpipe_lock);
table = devlink_dpipe_table_find(&devlink->dpipe_table_list,
table_name, devlink);
if (!table) {
@@ -10029,7 +10048,7 @@ int devlink_dpipe_table_resource_set(struct devlink *devlink,
table->resource_units = resource_units;
table->resource_valid = true;
out:
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->dpipe_lock);
return err;
}
EXPORT_SYMBOL_GPL(devlink_dpipe_table_resource_set);
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 14/16] devlink: Require devlink lock during device reload
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (12 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 13/16] devlink: Convert dpipe to use dpipe_lock Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 15/16] devlink: Use xarray locking mechanism instead big devlink lock Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 16/16] devlink: Open devlink to parallel operations Leon Romanovsky
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
Devlink reload was implemented as a special command which does _SET_
operation, but doesn't take devlink->lock, while recursive devlink
calls that were part of .reload_up()/.reload_down() sequence took it.
This fragile flow was possible due to holding a big devlink lock
(devlink_mutex), which effectively stopped all devlink activities,
even unrelated to reloaded devlink.
So let's make sure that devlink reload behaves as other commands.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 60af6a3fc130..32d274bfd049 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -8771,7 +8771,6 @@ static const struct genl_small_ops devlink_nl_ops[] = {
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = devlink_nl_cmd_reload,
.flags = GENL_ADMIN_PERM,
- .internal_flags = DEVLINK_NL_FLAG_NO_LOCK,
},
{
.cmd = DEVLINK_CMD_PARAM_GET,
@@ -11462,6 +11461,7 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
if (!net_eq(devlink_net(devlink), net))
goto retry;
+ mutex_lock(&devlink->lock);
WARN_ON(!(devlink->features & DEVLINK_F_RELOAD));
err = devlink_reload(devlink, &init_net,
DEVLINK_RELOAD_ACTION_DRIVER_REINIT,
@@ -11469,6 +11469,7 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
&actions_performed, NULL);
if (err && err != -EOPNOTSUPP)
pr_warn("Failed to reload devlink instance into init_net\n");
+ mutex_unlock(&devlink->lock);
retry:
devlink_put(devlink);
}
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 15/16] devlink: Use xarray locking mechanism instead big devlink lock
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (13 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 14/16] devlink: Require devlink lock during device reload Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 16/16] devlink: Open devlink to parallel operations Leon Romanovsky
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
The conversion to XArray together with devlink reference counting
allows us reuse the following locking pattern:
xa_lock()
xa_for_each() {
devlink_try_get()
xa_unlock()
....
xa_lock()
}
This pattern gives us a way to run any commands between xa_unlock() and
xa_lock() without big devlink mutex, while making sure that devlink instance
won't be released.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 256 ++++++++++++++++++++++++++-------------------
1 file changed, 150 insertions(+), 106 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 32d274bfd049..147f606cac41 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -175,14 +175,6 @@ static DEFINE_XARRAY_FLAGS(devlinks, XA_FLAGS_ALLOC);
#define ASSERT_DEVLINK_NOT_REGISTERED(d) \
WARN_ON_ONCE(xa_get_mark(&devlinks, (d)->index, DEVLINK_REGISTERED))
-/* devlink_mutex
- *
- * An overall lock guarding every operation coming from userspace.
- * It also guards devlink devices list and it is taken when
- * driver registers/unregisters it.
- */
-static DEFINE_MUTEX(devlink_mutex);
-
struct net *devlink_net(const struct devlink *devlink)
{
return read_pnet(&devlink->_net);
@@ -215,8 +207,7 @@ static struct devlink *devlink_get_from_attrs(struct net *net,
busname = nla_data(attrs[DEVLINK_ATTR_BUS_NAME]);
devname = nla_data(attrs[DEVLINK_ATTR_DEV_NAME]);
- lockdep_assert_held(&devlink_mutex);
-
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (strcmp(devlink->dev->bus->name, busname) == 0 &&
strcmp(dev_name(devlink->dev), devname) == 0 &&
@@ -228,6 +219,7 @@ static struct devlink *devlink_get_from_attrs(struct net *net,
if (!found || !devlink_try_get(devlink))
devlink = ERR_PTR(-ENODEV);
+ xa_unlock(&devlinks);
return devlink;
}
@@ -524,12 +516,6 @@ devlink_region_snapshot_get_by_id(struct devlink_region *region, u32 id)
#define DEVLINK_NL_FLAG_NEED_RATE BIT(2)
#define DEVLINK_NL_FLAG_NEED_RATE_NODE BIT(3)
-/* The per devlink instance lock is taken by default in the pre-doit
- * operation, yet several commands do not require this. The global
- * devlink lock is taken and protects from disruption by user-calls.
- */
-#define DEVLINK_NL_FLAG_NO_LOCK BIT(4)
-
static int devlink_nl_pre_doit(const struct genl_ops *ops,
struct sk_buff *skb, struct genl_info *info)
{
@@ -537,14 +523,11 @@ static int devlink_nl_pre_doit(const struct genl_ops *ops,
struct devlink *devlink;
int err;
- mutex_lock(&devlink_mutex);
devlink = devlink_get_from_attrs(genl_info_net(info), info->attrs);
- if (IS_ERR(devlink)) {
- mutex_unlock(&devlink_mutex);
+ if (IS_ERR(devlink))
return PTR_ERR(devlink);
- }
- if (~ops->internal_flags & DEVLINK_NL_FLAG_NO_LOCK)
- mutex_lock(&devlink->lock);
+
+ mutex_lock(&devlink->lock);
info->user_ptr[0] = devlink;
if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_PORT) {
devlink_port = devlink_port_get_from_info(devlink, info);
@@ -579,23 +562,18 @@ static int devlink_nl_pre_doit(const struct genl_ops *ops,
return 0;
unlock:
- if (~ops->internal_flags & DEVLINK_NL_FLAG_NO_LOCK)
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->lock);
devlink_put(devlink);
- mutex_unlock(&devlink_mutex);
return err;
}
static void devlink_nl_post_doit(const struct genl_ops *ops,
struct sk_buff *skb, struct genl_info *info)
{
- struct devlink *devlink;
+ struct devlink *devlink = info->user_ptr[0];
- devlink = info->user_ptr[0];
- if (~ops->internal_flags & DEVLINK_NL_FLAG_NO_LOCK)
- mutex_unlock(&devlink->lock);
+ mutex_unlock(&devlink->lock);
devlink_put(devlink);
- mutex_unlock(&devlink_mutex);
}
static struct genl_family devlink_nl_family;
@@ -1138,14 +1116,16 @@ static int devlink_nl_cmd_rate_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->rate_list_lock);
list_for_each_entry(devlink_rate, &devlink->rate_list, list) {
enum devlink_command cmd = DEVLINK_CMD_RATE_NEW;
@@ -1160,17 +1140,20 @@ static int devlink_nl_cmd_rate_get_dumpit(struct sk_buff *msg,
NLM_F_MULTI, NULL);
if (err) {
mutex_unlock(&devlink->rate_list_lock);
+ mutex_unlock(&devlink->lock);
devlink_put(devlink);
goto out;
}
idx++;
}
mutex_unlock(&devlink->rate_list_lock);
+ mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
if (err != -EMSGSIZE)
return err;
@@ -1241,32 +1224,37 @@ static int devlink_nl_cmd_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
- if (!net_eq(devlink_net(devlink), sock_net(msg->sk))) {
- devlink_put(devlink);
- continue;
- }
+ xa_unlock(&devlinks);
+ if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
+ goto retry;
if (idx < start) {
idx++;
- devlink_put(devlink);
- continue;
+ goto retry;
}
+ mutex_lock(&devlink->lock);
err = devlink_nl_fill(msg, devlink, DEVLINK_CMD_NEW,
NETLINK_CB(cb->skb).portid,
cb->nlh->nlmsg_seq, NLM_F_MULTI);
- devlink_put(devlink);
- if (err)
+ mutex_unlock(&devlink->lock);
+ if (err) {
+ xa_lock(&devlinks);
+ devlink_put(devlink);
goto out;
+ }
idx++;
+retry:
+ xa_lock(&devlinks);
+ devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
cb->args[0] = idx;
return msg->len;
@@ -1304,14 +1292,16 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->port_list_lock);
list_for_each_entry(devlink_port, &devlink->port_list, list) {
if (idx < start) {
@@ -1325,17 +1315,21 @@ static int devlink_nl_cmd_port_get_dumpit(struct sk_buff *msg,
NLM_F_MULTI, cb->extack);
if (err) {
mutex_unlock(&devlink->port_list_lock);
+ mutex_unlock(&devlink->lock);
+ xa_lock(&devlinks);
devlink_put(devlink);
goto out;
}
idx++;
}
mutex_unlock(&devlink->port_list_lock);
+ mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
cb->args[0] = idx;
return msg->len;
@@ -1979,14 +1973,16 @@ static int devlink_nl_cmd_sb_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->sb_list_lock);
list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
if (idx < start) {
@@ -2000,17 +1996,21 @@ static int devlink_nl_cmd_sb_get_dumpit(struct sk_buff *msg,
NLM_F_MULTI);
if (err) {
mutex_unlock(&devlink->sb_list_lock);
+ mutex_unlock(&devlink->lock);
+ xa_lock(&devlinks);
devlink_put(devlink);
goto out;
}
idx++;
}
mutex_unlock(&devlink->sb_list_lock);
+ mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
cb->args[0] = idx;
return msg->len;
@@ -2131,15 +2131,17 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
!devlink->ops->sb_pool_get)
goto retry;
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->sb_list_lock);
list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
err = __sb_pool_get_dumpit(msg, start, &idx, devlink,
@@ -2150,16 +2152,20 @@ static int devlink_nl_cmd_sb_pool_get_dumpit(struct sk_buff *msg,
err = 0;
} else if (err) {
mutex_unlock(&devlink->sb_list_lock);
+ mutex_unlock(&devlink->lock);
+ xa_lock(&devlinks);
devlink_put(devlink);
goto out;
}
}
mutex_unlock(&devlink->sb_list_lock);
+ mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
if (err != -EMSGSIZE)
return err;
@@ -2352,15 +2358,17 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
!devlink->ops->sb_port_pool_get)
goto retry;
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->sb_list_lock);
list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
err = __sb_port_pool_get_dumpit(msg, start, &idx,
@@ -2371,16 +2379,20 @@ static int devlink_nl_cmd_sb_port_pool_get_dumpit(struct sk_buff *msg,
err = 0;
} else if (err) {
mutex_unlock(&devlink->sb_list_lock);
+ mutex_unlock(&devlink->lock);
+ xa_lock(&devlinks);
devlink_put(devlink);
goto out;
}
}
mutex_unlock(&devlink->sb_list_lock);
+ mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
if (err != -EMSGSIZE)
return err;
@@ -2604,15 +2616,17 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)) ||
!devlink->ops->sb_tc_pool_bind_get)
goto retry;
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->sb_list_lock);
list_for_each_entry(devlink_sb, &devlink->sb_list, list) {
err = __sb_tc_pool_bind_get_dumpit(msg, start, &idx,
@@ -2624,16 +2638,20 @@ devlink_nl_cmd_sb_tc_pool_bind_get_dumpit(struct sk_buff *msg,
err = 0;
} else if (err) {
mutex_unlock(&devlink->sb_list_lock);
+ mutex_unlock(&devlink->lock);
+ xa_lock(&devlinks);
devlink_put(devlink);
goto out;
}
}
mutex_unlock(&devlink->sb_list_lock);
+ mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
if (err != -EMSGSIZE)
return err;
@@ -4704,11 +4722,12 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -4727,6 +4746,7 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
err = 0;
} else if (err) {
mutex_unlock(&devlink->lock);
+ xa_lock(&devlinks);
devlink_put(devlink);
goto out;
}
@@ -4734,10 +4754,11 @@ static int devlink_nl_cmd_param_get_dumpit(struct sk_buff *msg,
}
mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
if (err != -EMSGSIZE)
return err;
@@ -4939,14 +4960,16 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->port_list_lock);
list_for_each_entry(devlink_port, &devlink->port_list, list) {
list_for_each_entry(param_item,
@@ -4966,6 +4989,8 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
err = 0;
} else if (err) {
mutex_unlock(&devlink->port_list_lock);
+ mutex_unlock(&devlink->lock);
+ xa_lock(&devlinks);
devlink_put(devlink);
goto out;
}
@@ -4973,11 +4998,13 @@ static int devlink_nl_cmd_port_param_get_dumpit(struct sk_buff *msg,
}
}
mutex_unlock(&devlink->port_list_lock);
+ mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
if (err != -EMSGSIZE)
return err;
@@ -5516,23 +5543,27 @@ static int devlink_nl_cmd_region_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
+ mutex_lock(&devlink->lock);
err = devlink_nl_cmd_region_get_devlink_dumpit(msg, cb, devlink,
&idx, start);
+ mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
if (err)
goto out;
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
cb->args[0] = idx;
return msg->len;
}
@@ -5786,13 +5817,11 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
start_offset = *((u64 *)&cb->args[0]);
- mutex_lock(&devlink_mutex);
devlink = devlink_get_from_attrs(sock_net(cb->skb->sk), attrs);
- if (IS_ERR(devlink)) {
- err = PTR_ERR(devlink);
- goto out_dev;
- }
+ if (IS_ERR(devlink))
+ return PTR_ERR(devlink);
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->region_list_lock);
if (!attrs[DEVLINK_ATTR_REGION_NAME] ||
@@ -5890,8 +5919,8 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
nla_nest_end(skb, chunks_attr);
genlmsg_end(skb, hdr);
mutex_unlock(&devlink->region_list_lock);
+ mutex_unlock(&devlink->lock);
devlink_put(devlink);
- mutex_unlock(&devlink_mutex);
return skb->len;
@@ -5899,9 +5928,8 @@ static int devlink_nl_cmd_region_read_dumpit(struct sk_buff *skb,
genlmsg_cancel(skb, hdr);
out_unlock:
mutex_unlock(&devlink->region_list_lock);
+ mutex_unlock(&devlink->lock);
devlink_put(devlink);
-out_dev:
- mutex_unlock(&devlink_mutex);
return err;
}
@@ -6050,11 +6078,12 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err = 0;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
@@ -6070,15 +6099,17 @@ static int devlink_nl_cmd_info_get_dumpit(struct sk_buff *msg,
if (err == -EOPNOTSUPP)
err = 0;
else if (err) {
+ xa_lock(&devlinks);
devlink_put(devlink);
break;
}
inc:
idx++;
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
if (err != -EMSGSIZE)
return err;
@@ -7153,18 +7184,15 @@ devlink_health_reporter_get_from_cb(struct netlink_callback *cb)
struct nlattr **attrs = info->attrs;
struct devlink *devlink;
- mutex_lock(&devlink_mutex);
devlink = devlink_get_from_attrs(sock_net(cb->skb->sk), attrs);
if (IS_ERR(devlink))
- goto unlock;
+ return NULL;
+ mutex_lock(&devlink->lock);
reporter = devlink_health_reporter_get_from_attrs(devlink, attrs);
+ mutex_unlock(&devlink->lock);
devlink_put(devlink);
- mutex_unlock(&devlink_mutex);
return reporter;
-unlock:
- mutex_unlock(&devlink_mutex);
- return NULL;
}
void
@@ -7230,14 +7258,16 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry_rep;
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->reporters_lock);
list_for_each_entry(reporter, &devlink->reporter_list,
list) {
@@ -7251,13 +7281,17 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
NLM_F_MULTI);
if (err) {
mutex_unlock(&devlink->reporters_lock);
+ mutex_unlock(&devlink->lock);
+ xa_lock(&devlinks);
devlink_put(devlink);
goto out;
}
idx++;
}
mutex_unlock(&devlink->reporters_lock);
+ mutex_unlock(&devlink->lock);
retry_rep:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
@@ -7265,9 +7299,11 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry_port;
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->port_list_lock);
list_for_each_entry(port, &devlink->port_list, list) {
mutex_lock(&port->reporters_lock);
@@ -7284,6 +7320,8 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
if (err) {
mutex_unlock(&port->reporters_lock);
mutex_unlock(&devlink->port_list_lock);
+ mutex_unlock(&devlink->lock);
+ xa_lock(&devlinks);
devlink_put(devlink);
goto out;
}
@@ -7292,11 +7330,13 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg,
mutex_unlock(&port->reporters_lock);
}
mutex_unlock(&devlink->port_list_lock);
+ mutex_unlock(&devlink->lock);
retry_port:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
cb->args[0] = idx;
return msg->len;
@@ -7835,14 +7875,16 @@ static int devlink_nl_cmd_trap_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->traps_lock);
list_for_each_entry(trap_item, &devlink->trap_list, list) {
if (idx < start) {
@@ -7856,17 +7898,21 @@ static int devlink_nl_cmd_trap_get_dumpit(struct sk_buff *msg,
NLM_F_MULTI);
if (err) {
mutex_unlock(&devlink->traps_lock);
+ mutex_unlock(&devlink->lock);
+ xa_lock(&devlinks);
devlink_put(devlink);
goto out;
}
idx++;
}
mutex_unlock(&devlink->traps_lock);
+ mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
cb->args[0] = idx;
return msg->len;
@@ -8075,14 +8121,16 @@ static int devlink_nl_cmd_trap_group_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->traps_lock);
list_for_each_entry(group_item, &devlink->trap_group_list,
list) {
@@ -8097,17 +8145,21 @@ static int devlink_nl_cmd_trap_group_get_dumpit(struct sk_buff *msg,
NLM_F_MULTI);
if (err) {
mutex_unlock(&devlink->traps_lock);
+ mutex_unlock(&devlink->lock);
+ xa_lock(&devlinks);
devlink_put(devlink);
goto out;
}
idx++;
}
mutex_unlock(&devlink->traps_lock);
+ mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
cb->args[0] = idx;
return msg->len;
@@ -8402,14 +8454,16 @@ static int devlink_nl_cmd_trap_policer_get_dumpit(struct sk_buff *msg,
int idx = 0;
int err;
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), sock_net(msg->sk)))
goto retry;
+ mutex_lock(&devlink->lock);
mutex_lock(&devlink->traps_lock);
list_for_each_entry(policer_item, &devlink->trap_policer_list,
list) {
@@ -8424,17 +8478,21 @@ static int devlink_nl_cmd_trap_policer_get_dumpit(struct sk_buff *msg,
NLM_F_MULTI);
if (err) {
mutex_unlock(&devlink->traps_lock);
+ mutex_unlock(&devlink->lock);
+ xa_lock(&devlinks);
devlink_put(devlink);
goto out;
}
idx++;
}
mutex_unlock(&devlink->traps_lock);
+ mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
out:
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
cb->args[0] = idx;
return msg->len;
@@ -8633,26 +8691,22 @@ static const struct genl_small_ops devlink_nl_ops[] = {
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = devlink_nl_cmd_port_split_doit,
.flags = GENL_ADMIN_PERM,
- .internal_flags = DEVLINK_NL_FLAG_NO_LOCK,
},
{
.cmd = DEVLINK_CMD_PORT_UNSPLIT,
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = devlink_nl_cmd_port_unsplit_doit,
.flags = GENL_ADMIN_PERM,
- .internal_flags = DEVLINK_NL_FLAG_NO_LOCK,
},
{
.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_SB_GET,
@@ -8721,14 +8775,12 @@ static const struct genl_small_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_NO_LOCK,
},
{
.cmd = DEVLINK_CMD_ESWITCH_SET,
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = devlink_nl_cmd_eswitch_set_doit,
.flags = GENL_ADMIN_PERM,
- .internal_flags = DEVLINK_NL_FLAG_NO_LOCK,
},
{
.cmd = DEVLINK_CMD_DPIPE_TABLE_GET,
@@ -8838,8 +8890,7 @@ static const struct genl_small_ops devlink_nl_ops[] = {
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = devlink_nl_cmd_health_reporter_get_doit,
.dumpit = devlink_nl_cmd_health_reporter_get_dumpit,
- .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
- DEVLINK_NL_FLAG_NO_LOCK,
+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT,
/* can be retrieved by unprivileged users */
},
{
@@ -8847,24 +8898,21 @@ static const struct genl_small_ops devlink_nl_ops[] = {
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = devlink_nl_cmd_health_reporter_set_doit,
.flags = GENL_ADMIN_PERM,
- .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
- DEVLINK_NL_FLAG_NO_LOCK,
+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT,
},
{
.cmd = DEVLINK_CMD_HEALTH_REPORTER_RECOVER,
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = devlink_nl_cmd_health_reporter_recover_doit,
.flags = GENL_ADMIN_PERM,
- .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
- DEVLINK_NL_FLAG_NO_LOCK,
+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT,
},
{
.cmd = DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE,
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = devlink_nl_cmd_health_reporter_diagnose_doit,
.flags = GENL_ADMIN_PERM,
- .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
- DEVLINK_NL_FLAG_NO_LOCK,
+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT,
},
{
.cmd = DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET,
@@ -8878,16 +8926,14 @@ static const struct genl_small_ops devlink_nl_ops[] = {
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = devlink_nl_cmd_health_reporter_dump_clear_doit,
.flags = GENL_ADMIN_PERM,
- .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
- DEVLINK_NL_FLAG_NO_LOCK,
+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT,
},
{
.cmd = DEVLINK_CMD_HEALTH_REPORTER_TEST,
.validate = GENL_DONT_VALIDATE_STRICT | GENL_DONT_VALIDATE_DUMP,
.doit = devlink_nl_cmd_health_reporter_test_doit,
.flags = GENL_ADMIN_PERM,
- .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT |
- DEVLINK_NL_FLAG_NO_LOCK,
+ .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT,
},
{
.cmd = DEVLINK_CMD_FLASH_UPDATE,
@@ -9180,10 +9226,8 @@ void devlink_register(struct devlink *devlink)
ASSERT_DEVLINK_NOT_REGISTERED(devlink);
/* Make sure that we are in .probe() routine */
- mutex_lock(&devlink_mutex);
xa_set_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
devlink_notify_register(devlink);
- mutex_unlock(&devlink_mutex);
}
EXPORT_SYMBOL_GPL(devlink_register);
@@ -9200,10 +9244,8 @@ void devlink_unregister(struct devlink *devlink)
devlink_put(devlink);
wait_for_completion(&devlink->comp);
- mutex_lock(&devlink_mutex);
devlink_notify_unregister(devlink);
xa_clear_mark(&devlinks, devlink->index, DEVLINK_REGISTERED);
- mutex_unlock(&devlink_mutex);
}
EXPORT_SYMBOL_GPL(devlink_unregister);
@@ -11453,11 +11495,12 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
/* In case network namespace is getting destroyed, reload
* all devlink instances from this namespace into init_net.
*/
- mutex_lock(&devlink_mutex);
+ xa_lock(&devlinks);
xa_for_each_marked(&devlinks, index, devlink, DEVLINK_REGISTERED) {
if (!devlink_try_get(devlink))
continue;
+ xa_unlock(&devlinks);
if (!net_eq(devlink_net(devlink), net))
goto retry;
@@ -11471,9 +11514,10 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
pr_warn("Failed to reload devlink instance into init_net\n");
mutex_unlock(&devlink->lock);
retry:
+ xa_lock(&devlinks);
devlink_put(devlink);
}
- mutex_unlock(&devlink_mutex);
+ xa_unlock(&devlinks);
}
static struct pernet_operations devlink_pernet_ops __net_initdata = {
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [RFC PATCH 16/16] devlink: Open devlink to parallel operations
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
` (14 preceding siblings ...)
2021-11-08 17:05 ` [RFC PATCH 15/16] devlink: Use xarray locking mechanism instead big devlink lock Leon Romanovsky
@ 2021-11-08 17:05 ` Leon Romanovsky
15 siblings, 0 replies; 17+ messages in thread
From: Leon Romanovsky @ 2021-11-08 17:05 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski
Cc: Leon Romanovsky, Ido Schimmel, Jiri Pirko, netdev
From: Leon Romanovsky <leonro@nvidia.com>
Allow parallel execution of devlink ops.
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
net/core/devlink.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 147f606cac41..0c118002ef3c 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -8989,6 +8989,7 @@ static struct genl_family devlink_nl_family __ro_after_init = {
.n_small_ops = ARRAY_SIZE(devlink_nl_ops),
.mcgrps = devlink_nl_mcgrps,
.n_mcgrps = ARRAY_SIZE(devlink_nl_mcgrps),
+ .parallel_ops = true,
};
static bool devlink_reload_actions_valid(const struct devlink_ops *ops)
--
2.33.1
^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2021-11-08 17:06 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-08 17:05 [RFC PATCH 00/16] Allow parallel devlink execution Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 01/16] devlink: Remove misleading internal_flags from health reporter dump Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 02/16] devlink: Delete useless checks of holding devlink lock Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 03/16] devlink: Simplify devlink resources unregister call Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 04/16] devlink: Clean registration of devlink port Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 05/16] devlink: Be explicit with devlink port protection Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 06/16] devlink: Reshuffle resource registration logic Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 07/16] devlink: Inline sb related functions Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 08/16] devlink: Protect resource list with specific lock Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 09/16] devlink: Protect all traps lists with specialized lock Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 10/16] devlink: Separate region list protection to be done " Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 11/16] devlink: Protect all rate operations " Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 12/16] devlink: Protect all sb " Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 13/16] devlink: Convert dpipe to use dpipe_lock Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 14/16] devlink: Require devlink lock during device reload Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 15/16] devlink: Use xarray locking mechanism instead big devlink lock Leon Romanovsky
2021-11-08 17:05 ` [RFC PATCH 16/16] devlink: Open devlink to parallel operations Leon Romanovsky
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.