netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup
@ 2023-01-18 15:21 Jiri Pirko
  2023-01-18 15:21 ` [patch net-next v5 01/12] devlink: remove linecards lock Jiri Pirko
                   ` (12 more replies)
  0 siblings, 13 replies; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

This patchset does not change functionality.

Patches 1-2 remove linecards lock and reference counting, converting
them to be protected by devlink instance lock as the rest of
the objects.

Patches 3-4 fix the mlx5 auxiliary device devlink locking scheme whis is
needed for proper reporters lock conversion done in the following
patches.

Patches 5-8 remove reporters locks and reference counting, converting
them to be protected by devlink instance lock as the rest of
the objects.

Patches 9 and 10 convert linecards and reporters dumpit callbacks to
recently introduced devlink_nl_instance_iter_dump() infra.

Patch 11 removes no longer needed devlink_dump_for_each_instance_get()
helper.

The last patch adds assertion to devl_is_registered() as dependency on
other locks is removed.

---
v4->v5:
- fixed mlx5 locking issues
v3->v4:
- patch #1 was removed from the set and will be sent as a part of
  another patchset
v2->v3:
- see individual patches for changelog, mainly original patch #4 was
  split into 3 patches for easier review
v1->v2:
- patch 7 bits were unsquashed to patch 8

Jiri Pirko (12):
  devlink: remove linecards lock
  devlink: remove linecard reference counting
  net/mlx5e: Create separate devlink instance for ethernet auxiliary
    device
  net/mlx5: Remove MLX5E_LOCKED_FLOW flag
  devlink: protect health reporter operation with instance lock
  devlink: remove reporters_lock
  devlink: remove devl*_port_health_reporter_destroy()
  devlink: remove reporter reference counting
  devlink: convert linecards dump to devlink_nl_instance_iter_dump()
  devlink: convert reporters dump to devlink_nl_instance_iter_dump()
  devlink: remove devlink_dump_for_each_instance_get() helper
  devlink: add instance lock assertion in devl_is_registered()

 drivers/net/ethernet/mellanox/mlx5/core/dev.c |  14 +-
 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 +-
 .../mellanox/mlx5/core/en/reporter_rx.c       |   2 +-
 .../mellanox/mlx5/core/en/reporter_tx.c       |   2 +-
 .../net/ethernet/mellanox/mlx5/core/en_main.c |  25 +-
 drivers/net/ethernet/mellanox/mlxsw/core.c    |   8 +-
 .../ethernet/mellanox/mlxsw/core_linecards.c  |   8 +-
 drivers/net/netdevsim/health.c                |  20 +-
 include/linux/mlx5/driver.h                   |   4 -
 include/net/devlink.h                         |  27 +-
 net/devlink/core.c                            |   4 -
 net/devlink/devl_internal.h                   |  20 +-
 net/devlink/leftover.c                        | 442 +++++++-----------
 net/devlink/netlink.c                         |  12 +-
 16 files changed, 272 insertions(+), 369 deletions(-)

-- 
2.39.0


^ permalink raw reply	[flat|nested] 26+ messages in thread

* [patch net-next v5 01/12] devlink: remove linecards lock
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
@ 2023-01-18 15:21 ` Jiri Pirko
  2023-01-18 21:10   ` Jacob Keller
  2023-01-18 15:21 ` [patch net-next v5 02/12] devlink: remove linecard reference counting Jiri Pirko
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

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 <jiri@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
v2->v3: fixed typo in patch description
---
 .../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                        | 41 +++++++------------
 5 files changed, 21 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..6ba1baab80d3 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,13 @@ 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 +10262,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
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [patch net-next v5 02/12] devlink: remove linecard reference counting
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
  2023-01-18 15:21 ` [patch net-next v5 01/12] devlink: remove linecards lock Jiri Pirko
@ 2023-01-18 15:21 ` Jiri Pirko
  2023-01-18 21:13   ` Jacob Keller
  2023-01-18 15:21 ` [patch net-next v5 03/12] net/mlx5e: Create separate devlink instance for ethernet auxiliary device Jiri Pirko
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

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 <jiri@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
v2->v3:
- removed devlink_linecard_put() prototype from devl_internal.h
- fixed typo in patch description
---
 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 6ba1baab80d3..c92bc04bc25c 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;
@@ -10266,7 +10256,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;
 }
@@ -10282,7 +10271,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);
 }
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [patch net-next v5 03/12] net/mlx5e: Create separate devlink instance for ethernet auxiliary device
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
  2023-01-18 15:21 ` [patch net-next v5 01/12] devlink: remove linecards lock Jiri Pirko
  2023-01-18 15:21 ` [patch net-next v5 02/12] devlink: remove linecard reference counting Jiri Pirko
@ 2023-01-18 15:21 ` Jiri Pirko
  2023-01-18 21:16   ` Jacob Keller
  2023-01-18 15:21 ` [patch net-next v5 04/12] net/mlx5: Remove MLX5E_LOCKED_FLOW flag Jiri Pirko
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

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 <jiri@nvidia.com>
---
v4->v5:
- new patch
---
 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 <net/devlink.h>
 #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[] = {
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [patch net-next v5 04/12] net/mlx5: Remove MLX5E_LOCKED_FLOW flag
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
                   ` (2 preceding siblings ...)
  2023-01-18 15:21 ` [patch net-next v5 03/12] net/mlx5e: Create separate devlink instance for ethernet auxiliary device Jiri Pirko
@ 2023-01-18 15:21 ` Jiri Pirko
  2023-01-18 21:16   ` Jacob Keller
  2023-01-18 15:21 ` [patch net-next v5 05/12] devlink: protect health reporter operation with instance lock Jiri Pirko
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

The MLX5E_LOCKED_FLOW flag is not checked anywhere now so remove it
entirely.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v4->v5:
- new patch
---
 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;
-
-	err = add_drivers(dev);
+		return 0;
 
-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 {
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [patch net-next v5 05/12] devlink: protect health reporter operation with instance lock
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
                   ` (3 preceding siblings ...)
  2023-01-18 15:21 ` [patch net-next v5 04/12] net/mlx5: Remove MLX5E_LOCKED_FLOW flag Jiri Pirko
@ 2023-01-18 15:21 ` Jiri Pirko
  2023-01-18 21:17   ` Jacob Keller
  2023-01-18 15:21 ` [patch net-next v5 06/12] devlink: remove reporters_lock Jiri Pirko
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

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 <jiri@nvidia.com>
---
v4->v5:
- changed mlx5 bits due to changed locking scheme
- added locked versions of port reporter create/destroy functions
v2->v3:
- split from v2 patch #4 - "devlink: remove reporters_lock", no change
---
 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 c92bc04bc25c..0fc374140e6a 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) {
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [patch net-next v5 06/12] devlink: remove reporters_lock
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
                   ` (4 preceding siblings ...)
  2023-01-18 15:21 ` [patch net-next v5 05/12] devlink: protect health reporter operation with instance lock Jiri Pirko
@ 2023-01-18 15:21 ` Jiri Pirko
  2023-01-18 21:18   ` Jacob Keller
  2023-01-18 15:21 ` [patch net-next v5 07/12] devlink: remove devl*_port_health_reporter_destroy() Jiri Pirko
                   ` (6 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

Similar to other devlink objects, rely on devlink instance lock
and remove object specific reporters_lock.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v4->v5:
- rebased on top of the mutex destroy fix
v2->v3:
- split from v2 patch #4 - "devlink: remove reporters_lock", no change
---
 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 0fc374140e6a..29e2351ee752 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);
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [patch net-next v5 07/12] devlink: remove devl*_port_health_reporter_destroy()
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
                   ` (5 preceding siblings ...)
  2023-01-18 15:21 ` [patch net-next v5 06/12] devlink: remove reporters_lock Jiri Pirko
@ 2023-01-18 15:21 ` Jiri Pirko
  2023-01-18 21:19   ` Jacob Keller
  2023-01-18 15:21 ` [patch net-next v5 08/12] devlink: remove reporter reference counting Jiri Pirko
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

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 <jiri@nvidia.com>
---
v4->v5:
- changed mlx5 bits a bit due to changed locking scheme
- added removal of locked port reporter destroy function
v2->v3:
- split from v2 patch #4 - "devlink: remove reporters_lock", no change
---
 .../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 29e2351ee752..a56dd70a10e0 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,
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [patch net-next v5 08/12] devlink: remove reporter reference counting
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
                   ` (6 preceding siblings ...)
  2023-01-18 15:21 ` [patch net-next v5 07/12] devlink: remove devl*_port_health_reporter_destroy() Jiri Pirko
@ 2023-01-18 15:21 ` Jiri Pirko
  2023-01-18 21:20   ` Jacob Keller
  2023-01-18 15:21 ` [patch net-next v5 09/12] devlink: convert linecards dump to devlink_nl_instance_iter_dump() Jiri Pirko
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

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 <jiri@nvidia.com>
---
v2->v3
- fixed typo in patch description
---
 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 a56dd70a10e0..70eebf4a61c8 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;
-
-	err = devlink_fmsg_snd(fmsg, info,
-			       DEVLINK_CMD_HEALTH_REPORTER_DIAGNOSE, 0);
+		return err;
 
-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 {
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [patch net-next v5 09/12] devlink: convert linecards dump to devlink_nl_instance_iter_dump()
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
                   ` (7 preceding siblings ...)
  2023-01-18 15:21 ` [patch net-next v5 08/12] devlink: remove reporter reference counting Jiri Pirko
@ 2023-01-18 15:21 ` Jiri Pirko
  2023-01-18 21:21   ` Jacob Keller
  2023-01-18 15:21 ` [patch net-next v5 10/12] devlink: convert reporters " Jiri Pirko
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

Benefit from recently introduced instance iteration and convert
linecards .dumpit generic netlink callback to use it.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
---
 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 70eebf4a61c8..8bb4c2710688 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;
-
-	devlink_dump_for_each_instance_get(msg, state, devlink) {
-		int idx = 0;
-
-		devl_lock(devlink);
-		if (!devl_is_registered(devlink))
-			goto next_devlink;
+	int idx = 0;
+	int err = 0;
 
-		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,
 };
 
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [patch net-next v5 10/12] devlink: convert reporters dump to devlink_nl_instance_iter_dump()
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
                   ` (8 preceding siblings ...)
  2023-01-18 15:21 ` [patch net-next v5 09/12] devlink: convert linecards dump to devlink_nl_instance_iter_dump() Jiri Pirko
@ 2023-01-18 15:21 ` Jiri Pirko
  2023-01-18 21:22   ` Jacob Keller
  2023-01-18 15:21 ` [patch net-next v5 11/12] devlink: remove devlink_dump_for_each_instance_get() helper Jiri Pirko
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

Benefit from recently introduced instance iteration and convert
reporters .dumpit generic netlink callback to use it.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v1->v2:
- unsquashed the next patch (devlink: remove
  devlink_dump_for_each_instance_get() helper) from this one
---
 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 8bb4c2710688..2825995704c4 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,
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [patch net-next v5 11/12] devlink: remove devlink_dump_for_each_instance_get() helper
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
                   ` (9 preceding siblings ...)
  2023-01-18 15:21 ` [patch net-next v5 10/12] devlink: convert reporters " Jiri Pirko
@ 2023-01-18 15:21 ` Jiri Pirko
  2023-01-18 21:24   ` Jacob Keller
  2023-01-18 15:21 ` [patch net-next v5 12/12] devlink: add instance lock assertion in devl_is_registered() Jiri Pirko
  2023-01-20  3:20 ` [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup patchwork-bot+netdevbpf
  12 siblings, 1 reply; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

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 <jiri@nvidia.com>
---
v1->v2: new patch, unsquashed from the previous one
---
 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;
 	}
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [patch net-next v5 12/12] devlink: add instance lock assertion in devl_is_registered()
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
                   ` (10 preceding siblings ...)
  2023-01-18 15:21 ` [patch net-next v5 11/12] devlink: remove devlink_dump_for_each_instance_get() helper Jiri Pirko
@ 2023-01-18 15:21 ` Jiri Pirko
  2023-01-18 21:25   ` Jacob Keller
  2023-01-20  3:20 ` [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup patchwork-bot+netdevbpf
  12 siblings, 1 reply; 26+ messages in thread
From: Jiri Pirko @ 2023-01-18 15:21 UTC (permalink / raw)
  To: netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, jacob.e.keller, gal

From: Jiri Pirko <jiri@nvidia.com>

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 <jiri@nvidia.com>
---
 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);
 }
 
-- 
2.39.0


^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 01/12] devlink: remove linecards lock
  2023-01-18 15:21 ` [patch net-next v5 01/12] devlink: remove linecards lock Jiri Pirko
@ 2023-01-18 21:10   ` Jacob Keller
  0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2023-01-18 21:10 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, gal



On 1/18/2023 7:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> 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 <jiri@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 02/12] devlink: remove linecard reference counting
  2023-01-18 15:21 ` [patch net-next v5 02/12] devlink: remove linecard reference counting Jiri Pirko
@ 2023-01-18 21:13   ` Jacob Keller
  0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2023-01-18 21:13 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, gal



On 1/18/2023 7:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> As long as the linecard life time is protected by devlink instance
> lock, the reference counting is no longer needed. Remove it.
> 
Makes sense.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>
> ---
> v2->v3:
> - removed devlink_linecard_put() prototype from devl_internal.h
> - fixed typo in patch description
> ---
>  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 6ba1baab80d3..c92bc04bc25c 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;
> @@ -10266,7 +10256,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;
>  }
> @@ -10282,7 +10271,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);
>  }

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 03/12] net/mlx5e: Create separate devlink instance for ethernet auxiliary device
  2023-01-18 15:21 ` [patch net-next v5 03/12] net/mlx5e: Create separate devlink instance for ethernet auxiliary device Jiri Pirko
@ 2023-01-18 21:16   ` Jacob Keller
  0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2023-01-18 21:16 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, gal



On 1/18/2023 7:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> 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.
> 

So this would make the port appear independent of the main PCI devlink.
I think that's ok, they are a separate driver and managing the
connection would be really difficult.

Ok.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 04/12] net/mlx5: Remove MLX5E_LOCKED_FLOW flag
  2023-01-18 15:21 ` [patch net-next v5 04/12] net/mlx5: Remove MLX5E_LOCKED_FLOW flag Jiri Pirko
@ 2023-01-18 21:16   ` Jacob Keller
  0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2023-01-18 21:16 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, gal



On 1/18/2023 7:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> The MLX5E_LOCKED_FLOW flag is not checked anywhere now so remove it
> entirely.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>

Makes sense.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 05/12] devlink: protect health reporter operation with instance lock
  2023-01-18 15:21 ` [patch net-next v5 05/12] devlink: protect health reporter operation with instance lock Jiri Pirko
@ 2023-01-18 21:17   ` Jacob Keller
  0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2023-01-18 21:17 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, gal



On 1/18/2023 7:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> 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.
> 

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 06/12] devlink: remove reporters_lock
  2023-01-18 15:21 ` [patch net-next v5 06/12] devlink: remove reporters_lock Jiri Pirko
@ 2023-01-18 21:18   ` Jacob Keller
  0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2023-01-18 21:18 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, gal



On 1/18/2023 7:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Similar to other devlink objects, rely on devlink instance lock
> and remove object specific reporters_lock.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 07/12] devlink: remove devl*_port_health_reporter_destroy()
  2023-01-18 15:21 ` [patch net-next v5 07/12] devlink: remove devl*_port_health_reporter_destroy() Jiri Pirko
@ 2023-01-18 21:19   ` Jacob Keller
  0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2023-01-18 21:19 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, gal



On 1/18/2023 7:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> 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.
> 

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 08/12] devlink: remove reporter reference counting
  2023-01-18 15:21 ` [patch net-next v5 08/12] devlink: remove reporter reference counting Jiri Pirko
@ 2023-01-18 21:20   ` Jacob Keller
  0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2023-01-18 21:20 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, gal



On 1/18/2023 7:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> 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 <jiri@nvidia.com>

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 09/12] devlink: convert linecards dump to devlink_nl_instance_iter_dump()
  2023-01-18 15:21 ` [patch net-next v5 09/12] devlink: convert linecards dump to devlink_nl_instance_iter_dump() Jiri Pirko
@ 2023-01-18 21:21   ` Jacob Keller
  0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2023-01-18 21:21 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, gal



On 1/18/2023 7:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Benefit from recently introduced instance iteration and convert
> linecards .dumpit generic netlink callback to use it.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Reviewed-by: Ido Schimmel <idosch@nvidia.com>

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 10/12] devlink: convert reporters dump to devlink_nl_instance_iter_dump()
  2023-01-18 15:21 ` [patch net-next v5 10/12] devlink: convert reporters " Jiri Pirko
@ 2023-01-18 21:22   ` Jacob Keller
  0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2023-01-18 21:22 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, gal



On 1/18/2023 7:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Benefit from recently introduced instance iteration and convert
> reporters .dumpit generic netlink callback to use it.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 11/12] devlink: remove devlink_dump_for_each_instance_get() helper
  2023-01-18 15:21 ` [patch net-next v5 11/12] devlink: remove devlink_dump_for_each_instance_get() helper Jiri Pirko
@ 2023-01-18 21:24   ` Jacob Keller
  0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2023-01-18 21:24 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, gal



On 1/18/2023 7:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> 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 <jiri@nvidia.com>

Nice cleanup considering that the function has a comment about how its
not a generic iterator.. Less chance of accidentally misusing it in the
future.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 12/12] devlink: add instance lock assertion in devl_is_registered()
  2023-01-18 15:21 ` [patch net-next v5 12/12] devlink: add instance lock assertion in devl_is_registered() Jiri Pirko
@ 2023-01-18 21:25   ` Jacob Keller
  0 siblings, 0 replies; 26+ messages in thread
From: Jacob Keller @ 2023-01-18 21:25 UTC (permalink / raw)
  To: Jiri Pirko, netdev
  Cc: davem, kuba, pabeni, edumazet, michael.chan, yisen.zhuang,
	salil.mehta, jesse.brandeburg, anthony.l.nguyen, tariqt, saeedm,
	leon, idosch, petrm, mailhol.vincent, gal



On 1/18/2023 7:21 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> 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 <jiri@nvidia.com>

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup
  2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
                   ` (11 preceding siblings ...)
  2023-01-18 15:21 ` [patch net-next v5 12/12] devlink: add instance lock assertion in devl_is_registered() Jiri Pirko
@ 2023-01-20  3:20 ` patchwork-bot+netdevbpf
  12 siblings, 0 replies; 26+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-01-20  3:20 UTC (permalink / raw)
  To: Jiri Pirko
  Cc: netdev, davem, kuba, pabeni, edumazet, michael.chan,
	yisen.zhuang, salil.mehta, jesse.brandeburg, anthony.l.nguyen,
	tariqt, saeedm, leon, idosch, petrm, mailhol.vincent,
	jacob.e.keller, gal

Hello:

This series was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 18 Jan 2023 16:21:03 +0100 you wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> This patchset does not change functionality.
> 
> Patches 1-2 remove linecards lock and reference counting, converting
> them to be protected by devlink instance lock as the rest of
> the objects.
> 
> [...]

Here is the summary with links:
  - [net-next,v5,01/12] devlink: remove linecards lock
    (no matching commit)
  - [net-next,v5,02/12] devlink: remove linecard reference counting
    https://git.kernel.org/netdev/net-next/c/3a10173f48aa
  - [net-next,v5,03/12] net/mlx5e: Create separate devlink instance for ethernet auxiliary device
    https://git.kernel.org/netdev/net-next/c/ee75f1fc44dd
  - [net-next,v5,04/12] net/mlx5: Remove MLX5E_LOCKED_FLOW flag
    https://git.kernel.org/netdev/net-next/c/65a20c2eb96d
  - [net-next,v5,05/12] devlink: protect health reporter operation with instance lock
    https://git.kernel.org/netdev/net-next/c/dfdfd1305dde
  - [net-next,v5,06/12] devlink: remove reporters_lock
    https://git.kernel.org/netdev/net-next/c/1dea3b4e4c52
  - [net-next,v5,07/12] devlink: remove devl*_port_health_reporter_destroy()
    https://git.kernel.org/netdev/net-next/c/9f167327efec
  - [net-next,v5,08/12] devlink: remove reporter reference counting
    https://git.kernel.org/netdev/net-next/c/e994a75fb7f9
  - [net-next,v5,09/12] devlink: convert linecards dump to devlink_nl_instance_iter_dump()
    https://git.kernel.org/netdev/net-next/c/2557396808d9
  - [net-next,v5,10/12] devlink: convert reporters dump to devlink_nl_instance_iter_dump()
    https://git.kernel.org/netdev/net-next/c/19be51a93d99
  - [net-next,v5,11/12] devlink: remove devlink_dump_for_each_instance_get() helper
    https://git.kernel.org/netdev/net-next/c/543753d9e22e
  - [net-next,v5,12/12] devlink: add instance lock assertion in devl_is_registered()
    https://git.kernel.org/netdev/net-next/c/63ba54a52c41

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2023-01-20  4:45 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-18 15:21 [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup Jiri Pirko
2023-01-18 15:21 ` [patch net-next v5 01/12] devlink: remove linecards lock Jiri Pirko
2023-01-18 21:10   ` Jacob Keller
2023-01-18 15:21 ` [patch net-next v5 02/12] devlink: remove linecard reference counting Jiri Pirko
2023-01-18 21:13   ` Jacob Keller
2023-01-18 15:21 ` [patch net-next v5 03/12] net/mlx5e: Create separate devlink instance for ethernet auxiliary device Jiri Pirko
2023-01-18 21:16   ` Jacob Keller
2023-01-18 15:21 ` [patch net-next v5 04/12] net/mlx5: Remove MLX5E_LOCKED_FLOW flag Jiri Pirko
2023-01-18 21:16   ` Jacob Keller
2023-01-18 15:21 ` [patch net-next v5 05/12] devlink: protect health reporter operation with instance lock Jiri Pirko
2023-01-18 21:17   ` Jacob Keller
2023-01-18 15:21 ` [patch net-next v5 06/12] devlink: remove reporters_lock Jiri Pirko
2023-01-18 21:18   ` Jacob Keller
2023-01-18 15:21 ` [patch net-next v5 07/12] devlink: remove devl*_port_health_reporter_destroy() Jiri Pirko
2023-01-18 21:19   ` Jacob Keller
2023-01-18 15:21 ` [patch net-next v5 08/12] devlink: remove reporter reference counting Jiri Pirko
2023-01-18 21:20   ` Jacob Keller
2023-01-18 15:21 ` [patch net-next v5 09/12] devlink: convert linecards dump to devlink_nl_instance_iter_dump() Jiri Pirko
2023-01-18 21:21   ` Jacob Keller
2023-01-18 15:21 ` [patch net-next v5 10/12] devlink: convert reporters " Jiri Pirko
2023-01-18 21:22   ` Jacob Keller
2023-01-18 15:21 ` [patch net-next v5 11/12] devlink: remove devlink_dump_for_each_instance_get() helper Jiri Pirko
2023-01-18 21:24   ` Jacob Keller
2023-01-18 15:21 ` [patch net-next v5 12/12] devlink: add instance lock assertion in devl_is_registered() Jiri Pirko
2023-01-18 21:25   ` Jacob Keller
2023-01-20  3:20 ` [patch net-next v5 00/12] devlink: linecard and reporters locking cleanup patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).