From 5cc9049cb9021a46ad5711a946eb3ded47eed0de Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 18 Jan 2023 16:21:04 +0100 Subject: [PATCH 01/12] devlink: remove linecards lock Similar to other devlink objects, convert the linecards list to be protected by devlink instance lock. Alongside with that rename the create/destroy() functions to devl_* to indicate the devlink instance lock needs to be held while calling them. Signed-off-by: Jiri Pirko Reviewed-by: Ido Schimmel Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski --- .../ethernet/mellanox/mlxsw/core_linecards.c | 8 ++-- include/net/devlink.h | 6 +-- net/devlink/core.c | 2 - net/devlink/devl_internal.h | 1 - net/devlink/leftover.c | 40 ++++++------------- 5 files changed, 20 insertions(+), 37 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c index 83d2dc91ba2c..025e0db983fe 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c +++ b/drivers/net/ethernet/mellanox/mlxsw/core_linecards.c @@ -1259,9 +1259,9 @@ static int mlxsw_linecard_init(struct mlxsw_core *mlxsw_core, linecard->linecards = linecards; mutex_init(&linecard->lock); - devlink_linecard = devlink_linecard_create(priv_to_devlink(mlxsw_core), - slot_index, &mlxsw_linecard_ops, - linecard); + devlink_linecard = devl_linecard_create(priv_to_devlink(mlxsw_core), + slot_index, &mlxsw_linecard_ops, + linecard); if (IS_ERR(devlink_linecard)) return PTR_ERR(devlink_linecard); @@ -1285,7 +1285,7 @@ static void mlxsw_linecard_fini(struct mlxsw_core *mlxsw_core, if (linecard->active) mlxsw_linecard_active_clear(linecard); mlxsw_linecard_bdev_del(linecard); - devlink_linecard_destroy(linecard->devlink_linecard); + devl_linecard_destroy(linecard->devlink_linecard); mutex_destroy(&linecard->lock); } diff --git a/include/net/devlink.h b/include/net/devlink.h index 425ecef431b7..d7c9572e5bea 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -1687,9 +1687,9 @@ void devl_rate_nodes_destroy(struct devlink *devlink); void devlink_port_linecard_set(struct devlink_port *devlink_port, struct devlink_linecard *linecard); struct devlink_linecard * -devlink_linecard_create(struct devlink *devlink, unsigned int linecard_index, - const struct devlink_linecard_ops *ops, void *priv); -void devlink_linecard_destroy(struct devlink_linecard *linecard); +devl_linecard_create(struct devlink *devlink, unsigned int linecard_index, + const struct devlink_linecard_ops *ops, void *priv); +void devl_linecard_destroy(struct devlink_linecard *linecard); void devlink_linecard_provision_set(struct devlink_linecard *linecard, const char *type); void devlink_linecard_provision_clear(struct devlink_linecard *linecard); diff --git a/net/devlink/core.c b/net/devlink/core.c index 60beca2df7cc..dfc5b58c0464 100644 --- a/net/devlink/core.c +++ b/net/devlink/core.c @@ -247,7 +247,6 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops, mutex_init(&devlink->lock); lockdep_set_class(&devlink->lock, &devlink->lock_key); mutex_init(&devlink->reporters_lock); - mutex_init(&devlink->linecards_lock); refcount_set(&devlink->refcount, 1); return devlink; @@ -269,7 +268,6 @@ void devlink_free(struct devlink *devlink) { ASSERT_DEVLINK_NOT_REGISTERED(devlink); - mutex_destroy(&devlink->linecards_lock); mutex_destroy(&devlink->reporters_lock); WARN_ON(!list_empty(&devlink->trap_policer_list)); WARN_ON(!list_empty(&devlink->trap_group_list)); diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index e724e4c2a4ff..32f0adc40c18 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -38,7 +38,6 @@ struct devlink { struct list_head trap_group_list; struct list_head trap_policer_list; struct list_head linecard_list; - struct mutex linecards_lock; /* protects linecard_list */ const struct devlink_ops *ops; u64 features; struct xarray snapshot_ids; diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index bf5e0b1c0422..44f308d45697 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -282,13 +282,10 @@ devlink_linecard_get_from_attrs(struct devlink *devlink, struct nlattr **attrs) u32 linecard_index = nla_get_u32(attrs[DEVLINK_ATTR_LINECARD_INDEX]); struct devlink_linecard *linecard; - mutex_lock(&devlink->linecards_lock); linecard = devlink_linecard_get_by_index(devlink, linecard_index); - if (linecard) - refcount_inc(&linecard->refcount); - mutex_unlock(&devlink->linecards_lock); if (!linecard) return ERR_PTR(-ENODEV); + refcount_inc(&linecard->refcount); return linecard; } return ERR_PTR(-EINVAL); @@ -2129,7 +2126,7 @@ static int devlink_nl_cmd_linecard_get_dumpit(struct sk_buff *msg, devlink_dump_for_each_instance_get(msg, state, devlink) { int idx = 0; - mutex_lock(&devlink->linecards_lock); + devl_lock(devlink); if (!devl_is_registered(devlink)) goto next_devlink; @@ -2147,7 +2144,7 @@ static int devlink_nl_cmd_linecard_get_dumpit(struct sk_buff *msg, cb->extack); mutex_unlock(&linecard->state_lock); if (err) { - mutex_unlock(&devlink->linecards_lock); + devl_unlock(devlink); devlink_put(devlink); state->idx = idx; goto out; @@ -2155,7 +2152,7 @@ static int devlink_nl_cmd_linecard_get_dumpit(struct sk_buff *msg, idx++; } next_devlink: - mutex_unlock(&devlink->linecards_lock); + devl_unlock(devlink); devlink_put(devlink); } out: @@ -10223,7 +10220,7 @@ static void devlink_linecard_types_fini(struct devlink_linecard *linecard) } /** - * devlink_linecard_create - Create devlink linecard + * devl_linecard_create - Create devlink linecard * * @devlink: devlink * @linecard_index: driver-specific numerical identifier of the linecard @@ -10236,8 +10233,8 @@ static void devlink_linecard_types_fini(struct devlink_linecard *linecard) * Return: Line card structure or an ERR_PTR() encoded error code. */ struct devlink_linecard * -devlink_linecard_create(struct devlink *devlink, unsigned int linecard_index, - const struct devlink_linecard_ops *ops, void *priv) +devl_linecard_create(struct devlink *devlink, unsigned int linecard_index, + const struct devlink_linecard_ops *ops, void *priv) { struct devlink_linecard *linecard; int err; @@ -10246,17 +10243,12 @@ devlink_linecard_create(struct devlink *devlink, unsigned int linecard_index, !ops->types_count || !ops->types_get)) return ERR_PTR(-EINVAL); - mutex_lock(&devlink->linecards_lock); - if (devlink_linecard_index_exists(devlink, linecard_index)) { - mutex_unlock(&devlink->linecards_lock); + if (devlink_linecard_index_exists(devlink, linecard_index)) return ERR_PTR(-EEXIST); - } linecard = kzalloc(sizeof(*linecard), GFP_KERNEL); - if (!linecard) { - mutex_unlock(&devlink->linecards_lock); + if (!linecard) return ERR_PTR(-ENOMEM); - } linecard->devlink = devlink; linecard->index = linecard_index; @@ -10269,35 +10261,29 @@ devlink_linecard_create(struct devlink *devlink, unsigned int linecard_index, if (err) { mutex_destroy(&linecard->state_lock); kfree(linecard); - mutex_unlock(&devlink->linecards_lock); return ERR_PTR(err); } list_add_tail(&linecard->list, &devlink->linecard_list); refcount_set(&linecard->refcount, 1); - mutex_unlock(&devlink->linecards_lock); devlink_linecard_notify(linecard, DEVLINK_CMD_LINECARD_NEW); return linecard; } -EXPORT_SYMBOL_GPL(devlink_linecard_create); +EXPORT_SYMBOL_GPL(devl_linecard_create); /** - * devlink_linecard_destroy - Destroy devlink linecard + * devl_linecard_destroy - Destroy devlink linecard * * @linecard: devlink linecard */ -void devlink_linecard_destroy(struct devlink_linecard *linecard) +void devl_linecard_destroy(struct devlink_linecard *linecard) { - struct devlink *devlink = linecard->devlink; - devlink_linecard_notify(linecard, DEVLINK_CMD_LINECARD_DEL); - mutex_lock(&devlink->linecards_lock); list_del(&linecard->list); devlink_linecard_types_fini(linecard); - mutex_unlock(&devlink->linecards_lock); devlink_linecard_put(linecard); } -EXPORT_SYMBOL_GPL(devlink_linecard_destroy); +EXPORT_SYMBOL_GPL(devl_linecard_destroy); /** * devlink_linecard_provision_set - Set provisioning on linecard From 3a10173f48aa1bae64baf0fea228c6045996b117 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 18 Jan 2023 16:21:05 +0100 Subject: [PATCH 02/12] devlink: remove linecard reference counting As long as the linecard life time is protected by devlink instance lock, the reference counting is no longer needed. Remove it. Signed-off-by: Jiri Pirko Reviewed-by: Ido Schimmel Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski --- net/devlink/devl_internal.h | 1 - net/devlink/leftover.c | 14 ++------------ net/devlink/netlink.c | 5 ----- 3 files changed, 2 insertions(+), 18 deletions(-) diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index 32f0adc40c18..dd4e2c37cf07 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -193,7 +193,6 @@ struct devlink_linecard; struct devlink_linecard * devlink_linecard_get_from_info(struct devlink *devlink, struct genl_info *info); -void devlink_linecard_put(struct devlink_linecard *linecard); /* Rates */ extern const struct devlink_gen_cmd devl_gen_rate_get; diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index 44f308d45697..f029e1bc68f6 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -37,7 +37,6 @@ struct devlink_linecard { struct list_head list; struct devlink *devlink; unsigned int index; - refcount_t refcount; const struct devlink_linecard_ops *ops; void *priv; enum devlink_linecard_state state; @@ -285,7 +284,6 @@ devlink_linecard_get_from_attrs(struct devlink *devlink, struct nlattr **attrs) linecard = devlink_linecard_get_by_index(devlink, linecard_index); if (!linecard) return ERR_PTR(-ENODEV); - refcount_inc(&linecard->refcount); return linecard; } return ERR_PTR(-EINVAL); @@ -297,14 +295,6 @@ devlink_linecard_get_from_info(struct devlink *devlink, struct genl_info *info) return devlink_linecard_get_from_attrs(devlink, info->attrs); } -void devlink_linecard_put(struct devlink_linecard *linecard) -{ - if (refcount_dec_and_test(&linecard->refcount)) { - mutex_destroy(&linecard->state_lock); - kfree(linecard); - } -} - struct devlink_sb { struct list_head list; unsigned int index; @@ -10265,7 +10255,6 @@ devl_linecard_create(struct devlink *devlink, unsigned int linecard_index, } list_add_tail(&linecard->list, &devlink->linecard_list); - refcount_set(&linecard->refcount, 1); devlink_linecard_notify(linecard, DEVLINK_CMD_LINECARD_NEW); return linecard; } @@ -10281,7 +10270,8 @@ void devl_linecard_destroy(struct devlink_linecard *linecard) devlink_linecard_notify(linecard, DEVLINK_CMD_LINECARD_DEL); list_del(&linecard->list); devlink_linecard_types_fini(linecard); - devlink_linecard_put(linecard); + mutex_destroy(&linecard->state_lock); + kfree(linecard); } EXPORT_SYMBOL_GPL(devl_linecard_destroy); diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c index b5b8ac6db2d1..3f2ab4360f11 100644 --- a/net/devlink/netlink.c +++ b/net/devlink/netlink.c @@ -170,14 +170,9 @@ static int devlink_nl_pre_doit(const struct genl_split_ops *ops, static void devlink_nl_post_doit(const struct genl_split_ops *ops, struct sk_buff *skb, struct genl_info *info) { - struct devlink_linecard *linecard; struct devlink *devlink; devlink = info->user_ptr[0]; - if (ops->internal_flags & DEVLINK_NL_FLAG_NEED_LINECARD) { - linecard = info->user_ptr[1]; - devlink_linecard_put(linecard); - } devl_unlock(devlink); devlink_put(devlink); } From ee75f1fc44dd73f0323118f31cf91f9b6db147fa Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 18 Jan 2023 16:21:06 +0100 Subject: [PATCH 03/12] net/mlx5e: Create separate devlink instance for ethernet auxiliary device The fact that devlink instance lock is held over mlx5 auxiliary devices probe and remove routines brought a need to conditionally take devlink instance lock there. The code is checking a MLX5E_LOCKED_FLOW flag in mlx5 priv struct. This is racy and may lead to access devlink objects without holding instance lock or deadlock. To avoid this, the only lock-wise sane solution is to make the devlink entities created by the auxiliary device independent on the original pci devlink instance. Create devlink instance for the auxiliary device and put the uplink port instance there alongside with the port health reporters. Signed-off-by: Jiri Pirko Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/mellanox/mlx5/core/en.h | 4 ++ .../ethernet/mellanox/mlx5/core/en/devlink.c | 44 ++++++++++++------- .../ethernet/mellanox/mlx5/core/en/devlink.h | 5 ++- .../net/ethernet/mellanox/mlx5/core/en_main.c | 25 ++++++++--- 4 files changed, 55 insertions(+), 23 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h index 82573ac722d1..da58322cbc3a 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h @@ -971,6 +971,10 @@ struct mlx5e_priv { struct dentry *dfs_root; }; +struct mlx5e_dev { + struct mlx5e_priv *priv; +}; + struct mlx5e_rx_handlers { mlx5e_fp_handle_rx_cqe handle_rx_cqe; mlx5e_fp_handle_rx_cqe handle_rx_cqe_mpwqe; diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c index 83adaabf59f5..03ad3b61dfc7 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.c @@ -4,6 +4,29 @@ #include "en/devlink.h" #include "eswitch.h" +static const struct devlink_ops mlx5e_devlink_ops = { +}; + +struct mlx5e_dev *mlx5e_create_devlink(struct device *dev) +{ + struct mlx5e_dev *mlx5e_dev; + struct devlink *devlink; + + devlink = devlink_alloc(&mlx5e_devlink_ops, sizeof(*mlx5e_dev), dev); + if (!devlink) + return ERR_PTR(-ENOMEM); + devlink_register(devlink); + return devlink_priv(devlink); +} + +void mlx5e_destroy_devlink(struct mlx5e_dev *mlx5e_dev) +{ + struct devlink *devlink = priv_to_devlink(mlx5e_dev); + + devlink_unregister(devlink); + devlink_free(devlink); +} + static void mlx5e_devlink_get_port_parent_id(struct mlx5_core_dev *dev, struct netdev_phys_item_id *ppid) { @@ -14,14 +37,14 @@ 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) +int mlx5e_devlink_port_register(struct mlx5e_dev *mlx5e_dev, + struct mlx5e_priv *priv) { - struct devlink *devlink = priv_to_devlink(priv->mdev); + struct devlink *devlink = priv_to_devlink(mlx5e_dev); struct devlink_port_attrs attrs = {}; struct netdev_phys_item_id ppid = {}; struct devlink_port *dl_port; unsigned int dl_port_index; - int ret; if (mlx5_core_is_pf(priv->mdev)) { attrs.flavour = DEVLINK_PORT_FLAVOUR_PHYSICAL; @@ -42,23 +65,12 @@ int mlx5e_devlink_port_register(struct mlx5e_priv *priv) memset(dl_port, 0, sizeof(*dl_port)); devlink_port_attrs_set(dl_port, &attrs); - if (!(priv->mdev->priv.flags & MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW)) - devl_lock(devlink); - ret = devl_port_register(devlink, dl_port, dl_port_index); - if (!(priv->mdev->priv.flags & MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW)) - devl_unlock(devlink); - - return ret; + return devlink_port_register(devlink, dl_port, dl_port_index); } void mlx5e_devlink_port_unregister(struct mlx5e_priv *priv) { struct devlink_port *dl_port = mlx5e_devlink_get_dl_port(priv); - struct devlink *devlink = priv_to_devlink(priv->mdev); - if (!(priv->mdev->priv.flags & MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW)) - devl_lock(devlink); - devl_port_unregister(dl_port); - if (!(priv->mdev->priv.flags & MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW)) - devl_unlock(devlink); + devlink_port_unregister(dl_port); } diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.h b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.h index 4f238d4fff55..19b1d8e9634e 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.h +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/devlink.h @@ -7,7 +7,10 @@ #include #include "en.h" -int mlx5e_devlink_port_register(struct mlx5e_priv *priv); +struct mlx5e_dev *mlx5e_create_devlink(struct device *dev); +void mlx5e_destroy_devlink(struct mlx5e_dev *mlx5e_dev); +int mlx5e_devlink_port_register(struct mlx5e_dev *mlx5e_dev, + struct mlx5e_priv *priv); void mlx5e_devlink_port_unregister(struct mlx5e_priv *priv); static inline struct devlink_port * diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c index 6bb0fdaa5efa..1e0afaa31dd0 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_main.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_main.c @@ -5876,7 +5876,8 @@ void mlx5e_destroy_netdev(struct mlx5e_priv *priv) static int mlx5e_resume(struct auxiliary_device *adev) { struct mlx5_adev *edev = container_of(adev, struct mlx5_adev, adev); - struct mlx5e_priv *priv = auxiliary_get_drvdata(adev); + struct mlx5e_dev *mlx5e_dev = auxiliary_get_drvdata(adev); + struct mlx5e_priv *priv = mlx5e_dev->priv; struct net_device *netdev = priv->netdev; struct mlx5_core_dev *mdev = edev->mdev; int err; @@ -5899,7 +5900,8 @@ static int mlx5e_resume(struct auxiliary_device *adev) static int mlx5e_suspend(struct auxiliary_device *adev, pm_message_t state) { - struct mlx5e_priv *priv = auxiliary_get_drvdata(adev); + struct mlx5e_dev *mlx5e_dev = auxiliary_get_drvdata(adev); + struct mlx5e_priv *priv = mlx5e_dev->priv; struct net_device *netdev = priv->netdev; struct mlx5_core_dev *mdev = priv->mdev; @@ -5917,21 +5919,28 @@ static int mlx5e_probe(struct auxiliary_device *adev, struct mlx5_adev *edev = container_of(adev, struct mlx5_adev, adev); const struct mlx5e_profile *profile = &mlx5e_nic_profile; struct mlx5_core_dev *mdev = edev->mdev; + struct mlx5e_dev *mlx5e_dev; struct net_device *netdev; pm_message_t state = {}; struct mlx5e_priv *priv; int err; + mlx5e_dev = mlx5e_create_devlink(&adev->dev); + if (IS_ERR(mlx5e_dev)) + return PTR_ERR(mlx5e_dev); + auxiliary_set_drvdata(adev, mlx5e_dev); + netdev = mlx5e_create_netdev(mdev, profile); if (!netdev) { mlx5_core_err(mdev, "mlx5e_create_netdev failed\n"); - return -ENOMEM; + err = -ENOMEM; + goto err_devlink_unregister; } mlx5e_build_nic_netdev(netdev); priv = netdev_priv(netdev); - auxiliary_set_drvdata(adev, priv); + mlx5e_dev->priv = priv; priv->profile = profile; priv->ppriv = NULL; @@ -5939,7 +5948,7 @@ static int mlx5e_probe(struct auxiliary_device *adev, priv->dfs_root = debugfs_create_dir("nic", mlx5_debugfs_get_dev_root(priv->mdev)); - err = mlx5e_devlink_port_register(priv); + err = mlx5e_devlink_port_register(mlx5e_dev, priv); if (err) { mlx5_core_err(mdev, "mlx5e_devlink_port_register failed, %d\n", err); goto err_destroy_netdev; @@ -5978,12 +5987,15 @@ static int mlx5e_probe(struct auxiliary_device *adev, err_destroy_netdev: debugfs_remove_recursive(priv->dfs_root); mlx5e_destroy_netdev(priv); +err_devlink_unregister: + mlx5e_destroy_devlink(mlx5e_dev); return err; } static void mlx5e_remove(struct auxiliary_device *adev) { - struct mlx5e_priv *priv = auxiliary_get_drvdata(adev); + struct mlx5e_dev *mlx5e_dev = auxiliary_get_drvdata(adev); + struct mlx5e_priv *priv = mlx5e_dev->priv; pm_message_t state = {}; mlx5e_dcbnl_delete_app(priv); @@ -5993,6 +6005,7 @@ static void mlx5e_remove(struct auxiliary_device *adev) mlx5e_devlink_port_unregister(priv); debugfs_remove_recursive(priv->dfs_root); mlx5e_destroy_netdev(priv); + mlx5e_destroy_devlink(mlx5e_dev); } static const struct auxiliary_device_id mlx5e_id_table[] = { From 65a20c2eb96d698206846b27f9ac5ab05a24d569 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 18 Jan 2023 16:21:07 +0100 Subject: [PATCH 04/12] net/mlx5: Remove MLX5E_LOCKED_FLOW flag The MLX5E_LOCKED_FLOW flag is not checked anywhere now so remove it entirely. Signed-off-by: Jiri Pirko Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/mellanox/mlx5/core/dev.c | 14 ++------------ include/linux/mlx5/driver.h | 4 ---- 2 files changed, 2 insertions(+), 16 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/dev.c b/drivers/net/ethernet/mellanox/mlx5/core/dev.c index 5b6b0b126e52..2b444fb12388 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/dev.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/dev.c @@ -349,7 +349,6 @@ int mlx5_attach_device(struct mlx5_core_dev *dev) devl_assert_locked(priv_to_devlink(dev)); mutex_lock(&mlx5_intf_mutex); priv->flags &= ~MLX5_PRIV_FLAGS_DETACH; - priv->flags |= MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW; for (i = 0; i < ARRAY_SIZE(mlx5_adev_devices); i++) { if (!priv->adev[i]) { bool is_supported = false; @@ -397,7 +396,6 @@ int mlx5_attach_device(struct mlx5_core_dev *dev) break; } } - priv->flags &= ~MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW; mutex_unlock(&mlx5_intf_mutex); return ret; } @@ -412,7 +410,6 @@ void mlx5_detach_device(struct mlx5_core_dev *dev) devl_assert_locked(priv_to_devlink(dev)); mutex_lock(&mlx5_intf_mutex); - priv->flags |= MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW; for (i = ARRAY_SIZE(mlx5_adev_devices) - 1; i >= 0; i--) { if (!priv->adev[i]) continue; @@ -441,7 +438,6 @@ void mlx5_detach_device(struct mlx5_core_dev *dev) del_adev(&priv->adev[i]->adev); priv->adev[i] = NULL; } - priv->flags &= ~MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW; priv->flags |= MLX5_PRIV_FLAGS_DETACH; mutex_unlock(&mlx5_intf_mutex); } @@ -540,22 +536,16 @@ static void delete_drivers(struct mlx5_core_dev *dev) int mlx5_rescan_drivers_locked(struct mlx5_core_dev *dev) { struct mlx5_priv *priv = &dev->priv; - int err = 0; lockdep_assert_held(&mlx5_intf_mutex); if (priv->flags & MLX5_PRIV_FLAGS_DETACH) return 0; - priv->flags |= MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW; delete_drivers(dev); if (priv->flags & MLX5_PRIV_FLAGS_DISABLE_ALL_ADEV) - goto out; + return 0; - err = add_drivers(dev); - -out: - priv->flags &= ~MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW; - return err; + return add_drivers(dev); } bool mlx5_same_hw_devs(struct mlx5_core_dev *dev, struct mlx5_core_dev *peer_dev) diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h index b957b8f22a6b..44167760ff29 100644 --- a/include/linux/mlx5/driver.h +++ b/include/linux/mlx5/driver.h @@ -554,10 +554,6 @@ enum { * creation/deletion on drivers rescan. Unset during device attach. */ MLX5_PRIV_FLAGS_DETACH = 1 << 2, - /* Distinguish between mlx5e_probe/remove called by module init/cleanup - * and called by other flows which can already hold devlink lock - */ - MLX5_PRIV_FLAGS_MLX5E_LOCKED_FLOW = 1 << 3, }; struct mlx5_adev { From dfdfd1305ddecb990566193f2ba8a11bccba4cde Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 18 Jan 2023 16:21:08 +0100 Subject: [PATCH 05/12] devlink: protect health reporter operation with instance lock Similar to other devlink objects, protect the reporters list by devlink instance lock. Alongside add unlocked versions of health reporter create/destroy functions and use them in drivers on call paths where the instance lock is held. Signed-off-by: Jiri Pirko Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski --- drivers/net/ethernet/mellanox/mlxsw/core.c | 8 +- drivers/net/netdevsim/health.c | 20 ++--- include/net/devlink.h | 22 ++++- net/devlink/leftover.c | 99 +++++++++++++++++----- 4 files changed, 110 insertions(+), 39 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c index a0a06e2eff82..33ef726e4d54 100644 --- a/drivers/net/ethernet/mellanox/mlxsw/core.c +++ b/drivers/net/ethernet/mellanox/mlxsw/core.c @@ -2051,8 +2051,8 @@ static int mlxsw_core_health_init(struct mlxsw_core *mlxsw_core) if (!(mlxsw_core->bus->features & MLXSW_BUS_F_TXRX)) return 0; - fw_fatal = devlink_health_reporter_create(devlink, &mlxsw_core_health_fw_fatal_ops, - 0, mlxsw_core); + fw_fatal = devl_health_reporter_create(devlink, &mlxsw_core_health_fw_fatal_ops, + 0, mlxsw_core); if (IS_ERR(fw_fatal)) { dev_err(mlxsw_core->bus_info->dev, "Failed to create fw fatal reporter"); return PTR_ERR(fw_fatal); @@ -2072,7 +2072,7 @@ static int mlxsw_core_health_init(struct mlxsw_core *mlxsw_core) err_fw_fatal_config: mlxsw_core_trap_unregister(mlxsw_core, &mlxsw_core_health_listener, mlxsw_core); err_trap_register: - devlink_health_reporter_destroy(mlxsw_core->health.fw_fatal); + devl_health_reporter_destroy(mlxsw_core->health.fw_fatal); return err; } @@ -2085,7 +2085,7 @@ static void mlxsw_core_health_fini(struct mlxsw_core *mlxsw_core) mlxsw_core_trap_unregister(mlxsw_core, &mlxsw_core_health_listener, mlxsw_core); /* Make sure there is no more event work scheduled */ mlxsw_core_flush_owq(); - devlink_health_reporter_destroy(mlxsw_core->health.fw_fatal); + devl_health_reporter_destroy(mlxsw_core->health.fw_fatal); } static void mlxsw_core_irq_event_handler_init(struct mlxsw_core *mlxsw_core) diff --git a/drivers/net/netdevsim/health.c b/drivers/net/netdevsim/health.c index aa77af4a68df..eb04ed715d2d 100644 --- a/drivers/net/netdevsim/health.c +++ b/drivers/net/netdevsim/health.c @@ -233,16 +233,16 @@ int nsim_dev_health_init(struct nsim_dev *nsim_dev, struct devlink *devlink) int err; health->empty_reporter = - devlink_health_reporter_create(devlink, - &nsim_dev_empty_reporter_ops, - 0, health); + devl_health_reporter_create(devlink, + &nsim_dev_empty_reporter_ops, + 0, health); if (IS_ERR(health->empty_reporter)) return PTR_ERR(health->empty_reporter); health->dummy_reporter = - devlink_health_reporter_create(devlink, - &nsim_dev_dummy_reporter_ops, - 0, health); + devl_health_reporter_create(devlink, + &nsim_dev_dummy_reporter_ops, + 0, health); if (IS_ERR(health->dummy_reporter)) { err = PTR_ERR(health->dummy_reporter); goto err_empty_reporter_destroy; @@ -266,9 +266,9 @@ int nsim_dev_health_init(struct nsim_dev *nsim_dev, struct devlink *devlink) return 0; err_dummy_reporter_destroy: - devlink_health_reporter_destroy(health->dummy_reporter); + devl_health_reporter_destroy(health->dummy_reporter); err_empty_reporter_destroy: - devlink_health_reporter_destroy(health->empty_reporter); + devl_health_reporter_destroy(health->empty_reporter); return err; } @@ -278,6 +278,6 @@ void nsim_dev_health_exit(struct nsim_dev *nsim_dev) debugfs_remove_recursive(health->ddir); kfree(health->recovered_break_msg); - devlink_health_reporter_destroy(health->dummy_reporter); - devlink_health_reporter_destroy(health->empty_reporter); + devl_health_reporter_destroy(health->dummy_reporter); + devl_health_reporter_destroy(health->empty_reporter); } diff --git a/include/net/devlink.h b/include/net/devlink.h index d7c9572e5bea..0d64feaef7cb 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -1865,18 +1865,34 @@ int devlink_fmsg_binary_pair_put(struct devlink_fmsg *fmsg, const char *name, const void *value, u32 value_len); struct devlink_health_reporter * -devlink_health_reporter_create(struct devlink *devlink, - const struct devlink_health_reporter_ops *ops, - u64 graceful_period, void *priv); +devl_port_health_reporter_create(struct devlink_port *port, + const struct devlink_health_reporter_ops *ops, + u64 graceful_period, void *priv); struct devlink_health_reporter * devlink_port_health_reporter_create(struct devlink_port *port, const struct devlink_health_reporter_ops *ops, u64 graceful_period, void *priv); +struct devlink_health_reporter * +devl_health_reporter_create(struct devlink *devlink, + const struct devlink_health_reporter_ops *ops, + u64 graceful_period, void *priv); + +struct devlink_health_reporter * +devlink_health_reporter_create(struct devlink *devlink, + const struct devlink_health_reporter_ops *ops, + u64 graceful_period, void *priv); + +void +devl_health_reporter_destroy(struct devlink_health_reporter *reporter); + void devlink_health_reporter_destroy(struct devlink_health_reporter *reporter); +void +devl_port_health_reporter_destroy(struct devlink_health_reporter *reporter); + void devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter); diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index f029e1bc68f6..0263dc18e72d 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -7337,8 +7337,8 @@ __devlink_health_reporter_create(struct devlink *devlink, } /** - * devlink_port_health_reporter_create - create devlink health reporter for - * specified port instance + * devl_port_health_reporter_create - create devlink health reporter for + * specified port instance * * @port: devlink_port which should contain the new reporter * @ops: ops @@ -7346,12 +7346,13 @@ __devlink_health_reporter_create(struct devlink *devlink, * @priv: priv */ struct devlink_health_reporter * -devlink_port_health_reporter_create(struct devlink_port *port, - const struct devlink_health_reporter_ops *ops, - u64 graceful_period, void *priv) +devl_port_health_reporter_create(struct devlink_port *port, + const struct devlink_health_reporter_ops *ops, + u64 graceful_period, void *priv) { struct devlink_health_reporter *reporter; + devl_assert_locked(port->devlink); mutex_lock(&port->reporters_lock); if (__devlink_health_reporter_find_by_name(&port->reporter_list, &port->reporters_lock, ops->name)) { @@ -7370,10 +7371,26 @@ devlink_port_health_reporter_create(struct devlink_port *port, mutex_unlock(&port->reporters_lock); return reporter; } +EXPORT_SYMBOL_GPL(devl_port_health_reporter_create); + +struct devlink_health_reporter * +devlink_port_health_reporter_create(struct devlink_port *port, + const struct devlink_health_reporter_ops *ops, + u64 graceful_period, void *priv) +{ + struct devlink_health_reporter *reporter; + struct devlink *devlink = port->devlink; + + devl_lock(devlink); + reporter = devl_port_health_reporter_create(port, ops, + graceful_period, priv); + devl_unlock(devlink); + return reporter; +} EXPORT_SYMBOL_GPL(devlink_port_health_reporter_create); /** - * devlink_health_reporter_create - create devlink health reporter + * devl_health_reporter_create - create devlink health reporter * * @devlink: devlink * @ops: ops @@ -7381,12 +7398,13 @@ EXPORT_SYMBOL_GPL(devlink_port_health_reporter_create); * @priv: priv */ struct devlink_health_reporter * -devlink_health_reporter_create(struct devlink *devlink, - const struct devlink_health_reporter_ops *ops, - u64 graceful_period, void *priv) +devl_health_reporter_create(struct devlink *devlink, + const struct devlink_health_reporter_ops *ops, + u64 graceful_period, void *priv) { struct devlink_health_reporter *reporter; + devl_assert_locked(devlink); mutex_lock(&devlink->reporters_lock); if (devlink_health_reporter_find_by_name(devlink, ops->name)) { reporter = ERR_PTR(-EEXIST); @@ -7403,6 +7421,21 @@ devlink_health_reporter_create(struct devlink *devlink, mutex_unlock(&devlink->reporters_lock); return reporter; } +EXPORT_SYMBOL_GPL(devl_health_reporter_create); + +struct devlink_health_reporter * +devlink_health_reporter_create(struct devlink *devlink, + const struct devlink_health_reporter_ops *ops, + u64 graceful_period, void *priv) +{ + struct devlink_health_reporter *reporter; + + devl_lock(devlink); + reporter = devl_health_reporter_create(devlink, ops, + graceful_period, priv); + devl_unlock(devlink); + return reporter; +} EXPORT_SYMBOL_GPL(devlink_health_reporter_create); static void @@ -7429,35 +7462,61 @@ __devlink_health_reporter_destroy(struct devlink_health_reporter *reporter) } /** - * devlink_health_reporter_destroy - destroy devlink health reporter + * devl_health_reporter_destroy - destroy devlink health reporter * * @reporter: devlink health reporter to destroy */ void -devlink_health_reporter_destroy(struct devlink_health_reporter *reporter) +devl_health_reporter_destroy(struct devlink_health_reporter *reporter) { struct mutex *lock = &reporter->devlink->reporters_lock; + devl_assert_locked(reporter->devlink); + mutex_lock(lock); __devlink_health_reporter_destroy(reporter); mutex_unlock(lock); } +EXPORT_SYMBOL_GPL(devl_health_reporter_destroy); + +void +devlink_health_reporter_destroy(struct devlink_health_reporter *reporter) +{ + struct devlink *devlink = reporter->devlink; + + devl_lock(devlink); + devl_health_reporter_destroy(reporter); + devl_unlock(devlink); +} EXPORT_SYMBOL_GPL(devlink_health_reporter_destroy); /** - * devlink_port_health_reporter_destroy - destroy devlink port health reporter + * devl_port_health_reporter_destroy - destroy devlink port health reporter * * @reporter: devlink health reporter to destroy */ void -devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter) +devl_port_health_reporter_destroy(struct devlink_health_reporter *reporter) { struct mutex *lock = &reporter->devlink_port->reporters_lock; + devl_assert_locked(reporter->devlink); + mutex_lock(lock); __devlink_health_reporter_destroy(reporter); mutex_unlock(lock); } +EXPORT_SYMBOL_GPL(devl_port_health_reporter_destroy); + +void +devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter) +{ + struct devlink *devlink = reporter->devlink; + + devl_lock(devlink); + devl_port_health_reporter_destroy(reporter); + devl_unlock(devlink); +} EXPORT_SYMBOL_GPL(devlink_port_health_reporter_destroy); static int @@ -7805,12 +7864,11 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg, unsigned long port_index; int idx = 0; + devl_lock(devlink); + if (!devl_is_registered(devlink)) + goto next_devlink; + mutex_lock(&devlink->reporters_lock); - if (!devl_is_registered(devlink)) { - mutex_unlock(&devlink->reporters_lock); - devlink_put(devlink); - continue; - } list_for_each_entry(reporter, &devlink->reporter_list, list) { @@ -7824,6 +7882,7 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg, NLM_F_MULTI); if (err) { mutex_unlock(&devlink->reporters_lock); + devl_unlock(devlink); devlink_put(devlink); state->idx = idx; goto out; @@ -7832,10 +7891,6 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg, } mutex_unlock(&devlink->reporters_lock); - devl_lock(devlink); - if (!devl_is_registered(devlink)) - goto next_devlink; - xa_for_each(&devlink->ports, port_index, port) { mutex_lock(&port->reporters_lock); list_for_each_entry(reporter, &port->reporter_list, list) { From 1dea3b4e4c52f4bed64d1c527d548e82ccaea15a Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 18 Jan 2023 16:21:09 +0100 Subject: [PATCH 06/12] devlink: remove reporters_lock Similar to other devlink objects, rely on devlink instance lock and remove object specific reporters_lock. Signed-off-by: Jiri Pirko Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski --- include/net/devlink.h | 1 - net/devlink/core.c | 2 -- net/devlink/devl_internal.h | 1 - net/devlink/leftover.c | 53 +++++++------------------------------ 4 files changed, 9 insertions(+), 48 deletions(-) diff --git a/include/net/devlink.h b/include/net/devlink.h index 0d64feaef7cb..d9ea76bea36e 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -146,7 +146,6 @@ struct devlink_port { initialized:1; struct delayed_work type_warn_dw; struct list_head reporter_list; - struct mutex reporters_lock; /* Protects reporter_list */ struct devlink_rate *devlink_rate; struct devlink_linecard *linecard; diff --git a/net/devlink/core.c b/net/devlink/core.c index dfc5b58c0464..6c0e2fc57e45 100644 --- a/net/devlink/core.c +++ b/net/devlink/core.c @@ -246,7 +246,6 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops, lockdep_register_key(&devlink->lock_key); mutex_init(&devlink->lock); lockdep_set_class(&devlink->lock, &devlink->lock_key); - mutex_init(&devlink->reporters_lock); refcount_set(&devlink->refcount, 1); return devlink; @@ -268,7 +267,6 @@ void devlink_free(struct devlink *devlink) { ASSERT_DEVLINK_NOT_REGISTERED(devlink); - mutex_destroy(&devlink->reporters_lock); WARN_ON(!list_empty(&devlink->trap_policer_list)); WARN_ON(!list_empty(&devlink->trap_group_list)); WARN_ON(!list_empty(&devlink->trap_list)); diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index dd4e2c37cf07..bdb83014b4b5 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -32,7 +32,6 @@ struct devlink { struct list_head param_list; struct list_head region_list; struct list_head reporter_list; - struct mutex reporters_lock; /* protects reporter_list */ struct devlink_dpipe_headers *dpipe_headers; struct list_head trap_list; struct list_head trap_group_list; diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index 0263dc18e72d..548e4a379e48 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -7281,12 +7281,10 @@ EXPORT_SYMBOL_GPL(devlink_health_reporter_priv); static struct devlink_health_reporter * __devlink_health_reporter_find_by_name(struct list_head *reporter_list, - struct mutex *list_lock, const char *reporter_name) { struct devlink_health_reporter *reporter; - lockdep_assert_held(list_lock); list_for_each_entry(reporter, reporter_list, list) if (!strcmp(reporter->ops->name, reporter_name)) return reporter; @@ -7298,7 +7296,6 @@ devlink_health_reporter_find_by_name(struct devlink *devlink, const char *reporter_name) { return __devlink_health_reporter_find_by_name(&devlink->reporter_list, - &devlink->reporters_lock, reporter_name); } @@ -7307,7 +7304,6 @@ devlink_port_health_reporter_find_by_name(struct devlink_port *devlink_port, const char *reporter_name) { return __devlink_health_reporter_find_by_name(&devlink_port->reporter_list, - &devlink_port->reporters_lock, reporter_name); } @@ -7353,22 +7349,18 @@ devl_port_health_reporter_create(struct devlink_port *port, struct devlink_health_reporter *reporter; devl_assert_locked(port->devlink); - mutex_lock(&port->reporters_lock); + if (__devlink_health_reporter_find_by_name(&port->reporter_list, - &port->reporters_lock, ops->name)) { - reporter = ERR_PTR(-EEXIST); - goto unlock; - } + ops->name)) + return ERR_PTR(-EEXIST); reporter = __devlink_health_reporter_create(port->devlink, ops, graceful_period, priv); if (IS_ERR(reporter)) - goto unlock; + return reporter; reporter->devlink_port = port; list_add_tail(&reporter->list, &port->reporter_list); -unlock: - mutex_unlock(&port->reporters_lock); return reporter; } EXPORT_SYMBOL_GPL(devl_port_health_reporter_create); @@ -7405,20 +7397,16 @@ devl_health_reporter_create(struct devlink *devlink, struct devlink_health_reporter *reporter; devl_assert_locked(devlink); - mutex_lock(&devlink->reporters_lock); - if (devlink_health_reporter_find_by_name(devlink, ops->name)) { - reporter = ERR_PTR(-EEXIST); - goto unlock; - } + + if (devlink_health_reporter_find_by_name(devlink, ops->name)) + return ERR_PTR(-EEXIST); reporter = __devlink_health_reporter_create(devlink, ops, graceful_period, priv); if (IS_ERR(reporter)) - goto unlock; + return reporter; list_add_tail(&reporter->list, &devlink->reporter_list); -unlock: - mutex_unlock(&devlink->reporters_lock); return reporter; } EXPORT_SYMBOL_GPL(devl_health_reporter_create); @@ -7469,13 +7457,9 @@ __devlink_health_reporter_destroy(struct devlink_health_reporter *reporter) void devl_health_reporter_destroy(struct devlink_health_reporter *reporter) { - struct mutex *lock = &reporter->devlink->reporters_lock; - devl_assert_locked(reporter->devlink); - mutex_lock(lock); __devlink_health_reporter_destroy(reporter); - mutex_unlock(lock); } EXPORT_SYMBOL_GPL(devl_health_reporter_destroy); @@ -7498,13 +7482,9 @@ EXPORT_SYMBOL_GPL(devlink_health_reporter_destroy); void devl_port_health_reporter_destroy(struct devlink_health_reporter *reporter) { - struct mutex *lock = &reporter->devlink_port->reporters_lock; - devl_assert_locked(reporter->devlink); - mutex_lock(lock); __devlink_health_reporter_destroy(reporter); - mutex_unlock(lock); } EXPORT_SYMBOL_GPL(devl_port_health_reporter_destroy); @@ -7758,17 +7738,13 @@ devlink_health_reporter_get_from_attrs(struct devlink *devlink, reporter_name = nla_data(attrs[DEVLINK_ATTR_HEALTH_REPORTER_NAME]); devlink_port = devlink_port_get_from_attrs(devlink, attrs); if (IS_ERR(devlink_port)) { - mutex_lock(&devlink->reporters_lock); reporter = devlink_health_reporter_find_by_name(devlink, reporter_name); if (reporter) refcount_inc(&reporter->refcount); - mutex_unlock(&devlink->reporters_lock); } else { - mutex_lock(&devlink_port->reporters_lock); reporter = devlink_port_health_reporter_find_by_name(devlink_port, reporter_name); if (reporter) refcount_inc(&reporter->refcount); - mutex_unlock(&devlink_port->reporters_lock); } return reporter; @@ -7868,8 +7844,6 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg, if (!devl_is_registered(devlink)) goto next_devlink; - mutex_lock(&devlink->reporters_lock); - list_for_each_entry(reporter, &devlink->reporter_list, list) { if (idx < state->idx) { @@ -7881,7 +7855,6 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq, NLM_F_MULTI); if (err) { - mutex_unlock(&devlink->reporters_lock); devl_unlock(devlink); devlink_put(devlink); state->idx = idx; @@ -7889,10 +7862,8 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg, } idx++; } - mutex_unlock(&devlink->reporters_lock); xa_for_each(&devlink->ports, port_index, port) { - mutex_lock(&port->reporters_lock); list_for_each_entry(reporter, &port->reporter_list, list) { if (idx < state->idx) { idx++; @@ -7904,7 +7875,6 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg, NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq, NLM_F_MULTI); if (err) { - mutex_unlock(&port->reporters_lock); devl_unlock(devlink); devlink_put(devlink); state->idx = idx; @@ -7912,7 +7882,6 @@ devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg, } idx++; } - mutex_unlock(&port->reporters_lock); } next_devlink: devl_unlock(devlink); @@ -9633,12 +9602,9 @@ int devl_port_register(struct 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); err = xa_insert(&devlink->ports, port_index, devlink_port, GFP_KERNEL); - if (err) { - mutex_destroy(&devlink_port->reporters_lock); + if (err) return err; - } INIT_DELAYED_WORK(&devlink_port->type_warn_dw, &devlink_port_type_warn); devlink_port_type_warn_schedule(devlink_port); @@ -9689,7 +9655,6 @@ void devl_port_unregister(struct devlink_port *devlink_port) devlink_port_notify(devlink_port, DEVLINK_CMD_PORT_DEL); xa_erase(&devlink_port->devlink->ports, devlink_port->index); WARN_ON(!list_empty(&devlink_port->reporter_list)); - mutex_destroy(&devlink_port->reporters_lock); devlink_port->registered = false; } EXPORT_SYMBOL_GPL(devl_port_unregister); From 9f167327efecc3977deff0c852760e3759b0c2a7 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 18 Jan 2023 16:21:10 +0100 Subject: [PATCH 07/12] devlink: remove devl*_port_health_reporter_destroy() Remove port-specific health reporter destroy function as it is currently the same as the instance one so no longer needed. Inline __devlink_health_reporter_destroy() as it is no longer called from multiple places. Signed-off-by: Jiri Pirko Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski --- .../mellanox/mlx5/core/en/reporter_rx.c | 2 +- .../mellanox/mlx5/core/en/reporter_tx.c | 2 +- include/net/devlink.h | 6 ---- net/devlink/leftover.c | 35 ++----------------- 4 files changed, 4 insertions(+), 41 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c index 1ae15b8536a8..95edab4a1732 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_rx.c @@ -754,6 +754,6 @@ void mlx5e_reporter_rx_destroy(struct mlx5e_priv *priv) if (!priv->rx_reporter) return; - devlink_port_health_reporter_destroy(priv->rx_reporter); + devlink_health_reporter_destroy(priv->rx_reporter); priv->rx_reporter = NULL; } diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c index 60bc5b577ab9..b195dbbf6c90 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/reporter_tx.c @@ -609,6 +609,6 @@ void mlx5e_reporter_tx_destroy(struct mlx5e_priv *priv) if (!priv->tx_reporter) return; - devlink_port_health_reporter_destroy(priv->tx_reporter); + devlink_health_reporter_destroy(priv->tx_reporter); priv->tx_reporter = NULL; } diff --git a/include/net/devlink.h b/include/net/devlink.h index d9ea76bea36e..608a0c198be8 100644 --- a/include/net/devlink.h +++ b/include/net/devlink.h @@ -1889,12 +1889,6 @@ devl_health_reporter_destroy(struct devlink_health_reporter *reporter); void devlink_health_reporter_destroy(struct devlink_health_reporter *reporter); -void -devl_port_health_reporter_destroy(struct devlink_health_reporter *reporter); - -void -devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter); - void * devlink_health_reporter_priv(struct devlink_health_reporter *reporter); int devlink_health_report(struct devlink_health_reporter *reporter, diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index 548e4a379e48..d4e8c285b255 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -7442,13 +7442,6 @@ devlink_health_reporter_put(struct devlink_health_reporter *reporter) devlink_health_reporter_free(reporter); } -static void -__devlink_health_reporter_destroy(struct devlink_health_reporter *reporter) -{ - list_del(&reporter->list); - devlink_health_reporter_put(reporter); -} - /** * devl_health_reporter_destroy - destroy devlink health reporter * @@ -7459,7 +7452,8 @@ devl_health_reporter_destroy(struct devlink_health_reporter *reporter) { devl_assert_locked(reporter->devlink); - __devlink_health_reporter_destroy(reporter); + list_del(&reporter->list); + devlink_health_reporter_put(reporter); } EXPORT_SYMBOL_GPL(devl_health_reporter_destroy); @@ -7474,31 +7468,6 @@ devlink_health_reporter_destroy(struct devlink_health_reporter *reporter) } EXPORT_SYMBOL_GPL(devlink_health_reporter_destroy); -/** - * devl_port_health_reporter_destroy - destroy devlink port health reporter - * - * @reporter: devlink health reporter to destroy - */ -void -devl_port_health_reporter_destroy(struct devlink_health_reporter *reporter) -{ - devl_assert_locked(reporter->devlink); - - __devlink_health_reporter_destroy(reporter); -} -EXPORT_SYMBOL_GPL(devl_port_health_reporter_destroy); - -void -devlink_port_health_reporter_destroy(struct devlink_health_reporter *reporter) -{ - struct devlink *devlink = reporter->devlink; - - devl_lock(devlink); - devl_port_health_reporter_destroy(reporter); - devl_unlock(devlink); -} -EXPORT_SYMBOL_GPL(devlink_port_health_reporter_destroy); - static int devlink_nl_health_reporter_fill(struct sk_buff *msg, struct devlink_health_reporter *reporter, From e994a75fb7f9dcdb53ff5c33859531141de30899 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 18 Jan 2023 16:21:11 +0100 Subject: [PATCH 08/12] devlink: remove reporter reference counting As long as the reporter life time is protected by devlink instance lock, the reference counting is no longer needed. Remove it. Signed-off-by: Jiri Pirko Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski --- net/devlink/leftover.c | 113 +++++++++++------------------------------ 1 file changed, 30 insertions(+), 83 deletions(-) diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index d4e8c285b255..114baecebaeb 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -7269,7 +7269,6 @@ struct devlink_health_reporter { u64 error_count; u64 recovery_count; u64 last_recovery_ts; - refcount_t refcount; }; void * @@ -7328,7 +7327,6 @@ __devlink_health_reporter_create(struct devlink *devlink, reporter->auto_recover = !!ops->recover; reporter->auto_dump = !!ops->dump; mutex_init(&reporter->dump_lock); - refcount_set(&reporter->refcount, 1); return reporter; } @@ -7435,13 +7433,6 @@ devlink_health_reporter_free(struct devlink_health_reporter *reporter) kfree(reporter); } -static void -devlink_health_reporter_put(struct devlink_health_reporter *reporter) -{ - if (refcount_dec_and_test(&reporter->refcount)) - devlink_health_reporter_free(reporter); -} - /** * devl_health_reporter_destroy - destroy devlink health reporter * @@ -7453,7 +7444,7 @@ devl_health_reporter_destroy(struct devlink_health_reporter *reporter) devl_assert_locked(reporter->devlink); list_del(&reporter->list); - devlink_health_reporter_put(reporter); + devlink_health_reporter_free(reporter); } EXPORT_SYMBOL_GPL(devl_health_reporter_destroy); @@ -7697,7 +7688,6 @@ static struct devlink_health_reporter * devlink_health_reporter_get_from_attrs(struct devlink *devlink, struct nlattr **attrs) { - struct devlink_health_reporter *reporter; struct devlink_port *devlink_port; char *reporter_name; @@ -7706,17 +7696,12 @@ devlink_health_reporter_get_from_attrs(struct devlink *devlink, reporter_name = nla_data(attrs[DEVLINK_ATTR_HEALTH_REPORTER_NAME]); devlink_port = devlink_port_get_from_attrs(devlink, attrs); - if (IS_ERR(devlink_port)) { - reporter = devlink_health_reporter_find_by_name(devlink, reporter_name); - if (reporter) - refcount_inc(&reporter->refcount); - } else { - reporter = devlink_port_health_reporter_find_by_name(devlink_port, reporter_name); - if (reporter) - refcount_inc(&reporter->refcount); - } - - return reporter; + if (IS_ERR(devlink_port)) + return devlink_health_reporter_find_by_name(devlink, + reporter_name); + else + return devlink_port_health_reporter_find_by_name(devlink_port, + reporter_name); } static struct devlink_health_reporter * @@ -7775,10 +7760,8 @@ static int devlink_nl_cmd_health_reporter_get_doit(struct sk_buff *skb, return -EINVAL; msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL); - if (!msg) { - err = -ENOMEM; - goto out; - } + if (!msg) + return -ENOMEM; err = devlink_nl_health_reporter_fill(msg, reporter, DEVLINK_CMD_HEALTH_REPORTER_GET, @@ -7786,13 +7769,10 @@ static int devlink_nl_cmd_health_reporter_get_doit(struct sk_buff *skb, 0); if (err) { nlmsg_free(msg); - goto out; + return err; } - err = genlmsg_reply(msg, info); -out: - devlink_health_reporter_put(reporter); - return err; + return genlmsg_reply(msg, info); } static int @@ -7866,7 +7846,6 @@ devlink_nl_cmd_health_reporter_set_doit(struct sk_buff *skb, { struct devlink *devlink = info->user_ptr[0]; struct devlink_health_reporter *reporter; - int err; reporter = devlink_health_reporter_get_from_info(devlink, info); if (!reporter) @@ -7874,15 +7853,12 @@ devlink_nl_cmd_health_reporter_set_doit(struct sk_buff *skb, if (!reporter->ops->recover && (info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD] || - info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER])) { - err = -EOPNOTSUPP; - goto out; - } + info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_RECOVER])) + return -EOPNOTSUPP; + if (!reporter->ops->dump && - info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_DUMP]) { - err = -EOPNOTSUPP; - goto out; - } + info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_DUMP]) + return -EOPNOTSUPP; if (info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD]) reporter->graceful_period = @@ -7896,11 +7872,7 @@ devlink_nl_cmd_health_reporter_set_doit(struct sk_buff *skb, reporter->auto_dump = nla_get_u8(info->attrs[DEVLINK_ATTR_HEALTH_REPORTER_AUTO_DUMP]); - devlink_health_reporter_put(reporter); return 0; -out: - devlink_health_reporter_put(reporter); - return err; } static int devlink_nl_cmd_health_reporter_recover_doit(struct sk_buff *skb, @@ -7908,16 +7880,12 @@ static int devlink_nl_cmd_health_reporter_recover_doit(struct sk_buff *skb, { struct devlink *devlink = info->user_ptr[0]; struct devlink_health_reporter *reporter; - int err; reporter = devlink_health_reporter_get_from_info(devlink, info); if (!reporter) return -EINVAL; - err = devlink_health_reporter_recover(reporter, NULL, info->extack); - - devlink_health_reporter_put(reporter); - return err; + return devlink_health_reporter_recover(reporter, NULL, info->extack); } static int devlink_nl_cmd_health_reporter_diagnose_doit(struct sk_buff *skb, @@ -7932,36 +7900,27 @@ static int devlink_nl_cmd_health_reporter_diagnose_doit(struct sk_buff *skb, if (!reporter) return -EINVAL; - if (!reporter->ops->diagnose) { - devlink_health_reporter_put(reporter); + if (!reporter->ops->diagnose) return -EOPNOTSUPP; - } fmsg = devlink_fmsg_alloc(); - if (!fmsg) { - devlink_health_reporter_put(reporter); + if (!fmsg) return -ENOMEM; - } err = devlink_fmsg_obj_nest_start(fmsg); if (err) - goto out; + return err; err = reporter->ops->diagnose(reporter, fmsg, info->extack); if (err) - goto out; + return err; err = devlink_fmsg_obj_nest_end(fmsg); if (err) - goto out; + return err; - err = devlink_fmsg_snd(fmsg, info, - DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE, 0); - -out: - devlink_fmsg_free(fmsg); - devlink_health_reporter_put(reporter); - return err; + return devlink_fmsg_snd(fmsg, info, + DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE, 0); } static int @@ -7976,10 +7935,9 @@ devlink_nl_cmd_health_reporter_dump_get_dumpit(struct sk_buff *skb, if (!reporter) return -EINVAL; - if (!reporter->ops->dump) { - err = -EOPNOTSUPP; - goto out; - } + if (!reporter->ops->dump) + return -EOPNOTSUPP; + mutex_lock(&reporter->dump_lock); if (!state->idx) { err = devlink_health_do_dump(reporter, NULL, cb->extack); @@ -7997,8 +7955,6 @@ devlink_nl_cmd_health_reporter_dump_get_dumpit(struct sk_buff *skb, DEVLINK_CMD_HEALTH_REPORTER_DUMP_GET); unlock: mutex_unlock(&reporter->dump_lock); -out: - devlink_health_reporter_put(reporter); return err; } @@ -8013,15 +7969,12 @@ devlink_nl_cmd_health_reporter_dump_clear_doit(struct sk_buff *skb, if (!reporter) return -EINVAL; - if (!reporter->ops->dump) { - devlink_health_reporter_put(reporter); + if (!reporter->ops->dump) return -EOPNOTSUPP; - } mutex_lock(&reporter->dump_lock); devlink_health_dump_clear(reporter); mutex_unlock(&reporter->dump_lock); - devlink_health_reporter_put(reporter); return 0; } @@ -8030,21 +7983,15 @@ static int devlink_nl_cmd_health_reporter_test_doit(struct sk_buff *skb, { struct devlink *devlink = info->user_ptr[0]; struct devlink_health_reporter *reporter; - int err; reporter = devlink_health_reporter_get_from_info(devlink, info); if (!reporter) return -EINVAL; - if (!reporter->ops->test) { - devlink_health_reporter_put(reporter); + if (!reporter->ops->test) return -EOPNOTSUPP; - } - err = reporter->ops->test(reporter, info->extack); - - devlink_health_reporter_put(reporter); - return err; + return reporter->ops->test(reporter, info->extack); } struct devlink_stats { From 2557396808d916189ec5812f64e2b08bfb905807 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 18 Jan 2023 16:21:12 +0100 Subject: [PATCH 09/12] devlink: convert linecards dump to devlink_nl_instance_iter_dump() Benefit from recently introduced instance iteration and convert linecards .dumpit generic netlink callback to use it. Signed-off-by: Jiri Pirko Reviewed-by: Ido Schimmel Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski --- net/devlink/devl_internal.h | 1 + net/devlink/leftover.c | 64 ++++++++++++++++--------------------- net/devlink/netlink.c | 1 + 3 files changed, 30 insertions(+), 36 deletions(-) diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index bdb83014b4b5..d8f8e4bff5b4 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -167,6 +167,7 @@ extern const struct devlink_gen_cmd devl_gen_info; extern const struct devlink_gen_cmd devl_gen_trap; extern const struct devlink_gen_cmd devl_gen_trap_group; extern const struct devlink_gen_cmd devl_gen_trap_policer; +extern const struct devlink_gen_cmd devl_gen_linecard; /* Ports */ int devlink_port_netdevice_event(struct notifier_block *nb, diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index 114baecebaeb..685845a2fc78 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -2105,50 +2105,42 @@ static int devlink_nl_cmd_linecard_get_doit(struct sk_buff *skb, return genlmsg_reply(msg, info); } -static int devlink_nl_cmd_linecard_get_dumpit(struct sk_buff *msg, - struct netlink_callback *cb) +static int devlink_nl_cmd_linecard_get_dump_one(struct sk_buff *msg, + struct devlink *devlink, + struct netlink_callback *cb) { struct devlink_nl_dump_state *state = devlink_dump_state(cb); struct devlink_linecard *linecard; - struct devlink *devlink; - int err; + int idx = 0; + int err = 0; - devlink_dump_for_each_instance_get(msg, state, devlink) { - int idx = 0; - - devl_lock(devlink); - if (!devl_is_registered(devlink)) - goto next_devlink; - - list_for_each_entry(linecard, &devlink->linecard_list, list) { - if (idx < state->idx) { - idx++; - continue; - } - mutex_lock(&linecard->state_lock); - err = devlink_nl_linecard_fill(msg, devlink, linecard, - DEVLINK_CMD_LINECARD_NEW, - NETLINK_CB(cb->skb).portid, - cb->nlh->nlmsg_seq, - NLM_F_MULTI, - cb->extack); - mutex_unlock(&linecard->state_lock); - if (err) { - devl_unlock(devlink); - devlink_put(devlink); - state->idx = idx; - goto out; - } + list_for_each_entry(linecard, &devlink->linecard_list, list) { + if (idx < state->idx) { idx++; + continue; } -next_devlink: - devl_unlock(devlink); - devlink_put(devlink); + mutex_lock(&linecard->state_lock); + err = devlink_nl_linecard_fill(msg, devlink, linecard, + DEVLINK_CMD_LINECARD_NEW, + NETLINK_CB(cb->skb).portid, + cb->nlh->nlmsg_seq, + NLM_F_MULTI, + cb->extack); + mutex_unlock(&linecard->state_lock); + if (err) { + state->idx = idx; + break; + } + idx++; } -out: - return msg->len; + + return err; } +const struct devlink_gen_cmd devl_gen_linecard = { + .dump_one = devlink_nl_cmd_linecard_get_dump_one, +}; + static struct devlink_linecard_type * devlink_linecard_type_lookup(struct devlink_linecard *linecard, const char *type) @@ -9010,7 +9002,7 @@ const struct genl_small_ops devlink_nl_ops[56] = { { .cmd = DEVLINK_CMD_LINECARD_GET, .doit = devlink_nl_cmd_linecard_get_doit, - .dumpit = devlink_nl_cmd_linecard_get_dumpit, + .dumpit = devlink_nl_instance_iter_dump, .internal_flags = DEVLINK_NL_FLAG_NEED_LINECARD, /* can be retrieved by unprivileged users */ }, diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c index 3f2ab4360f11..b18e216e09b0 100644 --- a/net/devlink/netlink.c +++ b/net/devlink/netlink.c @@ -191,6 +191,7 @@ static const struct devlink_gen_cmd *devl_gen_cmds[] = { [DEVLINK_CMD_TRAP_GET] = &devl_gen_trap, [DEVLINK_CMD_TRAP_GROUP_GET] = &devl_gen_trap_group, [DEVLINK_CMD_TRAP_POLICER_GET] = &devl_gen_trap_policer, + [DEVLINK_CMD_LINECARD_GET] = &devl_gen_linecard, [DEVLINK_CMD_SELFTESTS_GET] = &devl_gen_selftests, }; From 19be51a93d9986edcfecae010920ab1d5714479b Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 18 Jan 2023 16:21:13 +0100 Subject: [PATCH 10/12] devlink: convert reporters dump to devlink_nl_instance_iter_dump() Benefit from recently introduced instance iteration and convert reporters .dumpit generic netlink callback to use it. Signed-off-by: Jiri Pirko Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski --- net/devlink/devl_internal.h | 1 + net/devlink/leftover.c | 87 ++++++++++++++++--------------------- net/devlink/netlink.c | 1 + 3 files changed, 40 insertions(+), 49 deletions(-) diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index d8f8e4bff5b4..10975e4ea2f4 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -164,6 +164,7 @@ extern const struct devlink_gen_cmd devl_gen_selftests; extern const struct devlink_gen_cmd devl_gen_param; extern const struct devlink_gen_cmd devl_gen_region; extern const struct devlink_gen_cmd devl_gen_info; +extern const struct devlink_gen_cmd devl_gen_health_reporter; extern const struct devlink_gen_cmd devl_gen_trap; extern const struct devlink_gen_cmd devl_gen_trap_group; extern const struct devlink_gen_cmd devl_gen_trap_policer; diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c index 685845a2fc78..8eab95cae917 100644 --- a/net/devlink/leftover.c +++ b/net/devlink/leftover.c @@ -7768,70 +7768,59 @@ static int devlink_nl_cmd_health_reporter_get_doit(struct sk_buff *skb, } static int -devlink_nl_cmd_health_reporter_get_dumpit(struct sk_buff *msg, - struct netlink_callback *cb) +devlink_nl_cmd_health_reporter_get_dump_one(struct sk_buff *msg, + struct devlink *devlink, + struct netlink_callback *cb) { struct devlink_nl_dump_state *state = devlink_dump_state(cb); - struct devlink *devlink; + struct devlink_health_reporter *reporter; + struct devlink_port *port; + unsigned long port_index; + int idx = 0; int err; - devlink_dump_for_each_instance_get(msg, state, devlink) { - struct devlink_health_reporter *reporter; - struct devlink_port *port; - unsigned long port_index; - int idx = 0; - - devl_lock(devlink); - if (!devl_is_registered(devlink)) - goto next_devlink; - - list_for_each_entry(reporter, &devlink->reporter_list, - list) { + list_for_each_entry(reporter, &devlink->reporter_list, list) { + if (idx < state->idx) { + idx++; + continue; + } + err = devlink_nl_health_reporter_fill(msg, reporter, + DEVLINK_CMD_HEALTH_REPORTER_GET, + NETLINK_CB(cb->skb).portid, + cb->nlh->nlmsg_seq, + NLM_F_MULTI); + if (err) { + state->idx = idx; + return err; + } + idx++; + } + xa_for_each(&devlink->ports, port_index, port) { + list_for_each_entry(reporter, &port->reporter_list, list) { if (idx < state->idx) { idx++; continue; } - err = devlink_nl_health_reporter_fill( - msg, reporter, DEVLINK_CMD_HEALTH_REPORTER_GET, - NETLINK_CB(cb->skb).portid, cb->nlh->nlmsg_seq, - NLM_F_MULTI); + err = devlink_nl_health_reporter_fill(msg, reporter, + DEVLINK_CMD_HEALTH_REPORTER_GET, + NETLINK_CB(cb->skb).portid, + cb->nlh->nlmsg_seq, + NLM_F_MULTI); if (err) { - devl_unlock(devlink); - devlink_put(devlink); state->idx = idx; - goto out; + return err; } idx++; } - - xa_for_each(&devlink->ports, port_index, port) { - list_for_each_entry(reporter, &port->reporter_list, list) { - if (idx < state->idx) { - idx++; - continue; - } - err = devlink_nl_health_reporter_fill( - msg, reporter, - DEVLINK_CMD_HEALTH_REPORTER_GET, - NETLINK_CB(cb->skb).portid, - cb->nlh->nlmsg_seq, NLM_F_MULTI); - if (err) { - devl_unlock(devlink); - devlink_put(devlink); - state->idx = idx; - goto out; - } - idx++; - } - } -next_devlink: - devl_unlock(devlink); - devlink_put(devlink); } -out: - return msg->len; + + return 0; } +const struct devlink_gen_cmd devl_gen_health_reporter = { + .dump_one = devlink_nl_cmd_health_reporter_get_dump_one, +}; + static int devlink_nl_cmd_health_reporter_set_doit(struct sk_buff *skb, struct genl_info *info) @@ -9193,7 +9182,7 @@ const struct genl_small_ops devlink_nl_ops[56] = { .cmd = DEVLINK_CMD_HEALTH_REPORTER_GET, .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, + .dumpit = devlink_nl_instance_iter_dump, .internal_flags = DEVLINK_NL_FLAG_NEED_DEVLINK_OR_PORT, /* can be retrieved by unprivileged users */ }, diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c index b18e216e09b0..d4539c1aedea 100644 --- a/net/devlink/netlink.c +++ b/net/devlink/netlink.c @@ -187,6 +187,7 @@ static const struct devlink_gen_cmd *devl_gen_cmds[] = { [DEVLINK_CMD_PARAM_GET] = &devl_gen_param, [DEVLINK_CMD_REGION_GET] = &devl_gen_region, [DEVLINK_CMD_INFO_GET] = &devl_gen_info, + [DEVLINK_CMD_HEALTH_REPORTER_GET] = &devl_gen_health_reporter, [DEVLINK_CMD_RATE_GET] = &devl_gen_rate_get, [DEVLINK_CMD_TRAP_GET] = &devl_gen_trap, [DEVLINK_CMD_TRAP_GROUP_GET] = &devl_gen_trap_group, From 543753d9e22e3943e2c31c4b5241afafb55dd52a Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 18 Jan 2023 16:21:14 +0100 Subject: [PATCH 11/12] devlink: remove devlink_dump_for_each_instance_get() helper devlink_dump_for_each_instance_get() is currently called from a single place in netlink.c. As there is no need to use this helper anywhere else in the future, remove it and call devlinks_xa_find_get() directly from while loop in devlink_nl_instance_iter_dump(). Also remove redundant idx clear on loop end as it is already done in devlink_nl_instance_iter_dump(). Signed-off-by: Jiri Pirko Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski --- net/devlink/devl_internal.h | 11 ----------- net/devlink/netlink.c | 5 ++++- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index 10975e4ea2f4..75752f8c4a26 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -123,17 +123,6 @@ struct devlink_gen_cmd { struct netlink_callback *cb); }; -/* Iterate over registered devlink instances for devlink dump. - * devlink_put() needs to be called for each iterated devlink pointer - * in loop body in order to release the reference. - * Note: this is NOT a generic iterator, it makes assumptions about the use - * of @state and can only be used once per dumpit implementation. - */ -#define devlink_dump_for_each_instance_get(msg, state, devlink) \ - for (; (devlink = devlinks_xa_find_get(sock_net(msg->sk), \ - &state->instance)); \ - state->instance++, state->idx = 0) - extern const struct genl_small_ops devlink_nl_ops[56]; struct devlink * diff --git a/net/devlink/netlink.c b/net/devlink/netlink.c index d4539c1aedea..3f44633af01c 100644 --- a/net/devlink/netlink.c +++ b/net/devlink/netlink.c @@ -207,7 +207,8 @@ int devlink_nl_instance_iter_dump(struct sk_buff *msg, cmd = devl_gen_cmds[info->op.cmd]; - devlink_dump_for_each_instance_get(msg, state, devlink) { + while ((devlink = devlinks_xa_find_get(sock_net(msg->sk), + &state->instance))) { devl_lock(devlink); if (devl_is_registered(devlink)) @@ -221,6 +222,8 @@ int devlink_nl_instance_iter_dump(struct sk_buff *msg, if (err) break; + state->instance++; + /* restart sub-object walk for the next instance */ state->idx = 0; } From 63ba54a52c417a0c632a5f85a2b912b8a2320358 Mon Sep 17 00:00:00 2001 From: Jiri Pirko Date: Wed, 18 Jan 2023 16:21:15 +0100 Subject: [PATCH 12/12] devlink: add instance lock assertion in devl_is_registered() After region and linecard lock removals, this helper is always supposed to be called with instance lock held. So put the assertion here and remove the comment which is no longer accurate. Signed-off-by: Jiri Pirko Reviewed-by: Jacob Keller Signed-off-by: Jakub Kicinski --- net/devlink/devl_internal.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h index 75752f8c4a26..1aa1a9549549 100644 --- a/net/devlink/devl_internal.h +++ b/net/devlink/devl_internal.h @@ -85,9 +85,7 @@ struct devlink *devlinks_xa_find_get(struct net *net, unsigned long *indexp); static inline bool devl_is_registered(struct devlink *devlink) { - /* To prevent races the caller must hold the instance lock - * or another lock taken during unregistration. - */ + devl_assert_locked(devlink); return xa_get_mark(&devlinks, devlink->index, DEVLINK_REGISTERED); }