[patch net-next v2 0/7] devlink: params cleanups and devl_param_driverinit_value_get() fix

[patch net-next v2 0/7] devlink: params cleanups and devl_param_driverinit_value_get() fix

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

The primary motivation of this patchset is the patch #6, which fixes an
issue introduced by 075935f0ae0f ("devlink: protect devlink param list
by instance lock") and reported by Kim Phillips <kim.phillips@amd.com>
(https://lore.kernel.org/netdev/719de4f0-76ac-e8b9-38a9-167ae239efc7@amd.com/)
and my colleagues doing mlx5 driver regression testing.

The basis idea is that devl_param_driverinit_value_get() could be
possible to the called without holding devlink intance lock in
most of the cases (all existing ones in the current codebase),
which would fix some mlx5 flows where the lock is not held.

To achieve that, make sure that the param value does not change between
reloads with patch #2.

Also, convert the param list to xarray which removes the worry about
list_head consistency when doing lockless lookup.

The rest of the patches are doing some small related cleanup of things
that poke me in the eye during the work.

---
v1->v2:
- a small bug was fixed in patch #2, the rest of the code stays the same
  so I left review/ack tags attached to them

Jiri Pirko (7):
  devlink: don't use strcpy() to copy param value
  devlink: make sure driver does not read updated driverinit param
    before reload
  devlink: fix the name of value arg of
    devl_param_driverinit_value_get()
  devlink: use xa_for_each_start() helper in
    devlink_nl_cmd_port_get_dump_one()
  devlink: convert param list to xarray
  devlink: allow to call devl_param_driverinit_value_get() without
    holding instance lock
  devlink: add forgotten devlink instance lock assertion to
    devl_param_driverinit_value_set()

 include/net/devlink.h       |   6 +-
 net/devlink/core.c          |   4 +-
 net/devlink/dev.c           |   3 +
 net/devlink/devl_internal.h |   5 +-
 net/devlink/leftover.c      | 139 ++++++++++++++++++++----------------
 5 files changed, 91 insertions(+), 66 deletions(-)

-- 
2.39.0

[patch net-next v2 1/7] devlink: don't use strcpy() to copy param value

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

No need to treat string params any different comparing to other types.
Rely on the struct assign to copy the whole struct, including the
string.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
---
 net/devlink/leftover.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index f05ab093d231..f2f6a2f42864 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -4388,10 +4388,7 @@ static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
 		return -EOPNOTSUPP;
 
 	if (cmode == DEVLINK_PARAM_CMODE_DRIVERINIT) {
-		if (param->type == DEVLINK_PARAM_TYPE_STRING)
-			strcpy(param_item->driverinit_value.vstr, value.vstr);
-		else
-			param_item->driverinit_value = value;
+		param_item->driverinit_value = value;
 		param_item->driverinit_value_valid = true;
 	} else {
 		if (!param->set)
@@ -9656,10 +9653,7 @@ int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
 						      DEVLINK_PARAM_CMODE_DRIVERINIT)))
 		return -EOPNOTSUPP;
 
-	if (param_item->param->type == DEVLINK_PARAM_TYPE_STRING)
-		strcpy(init_val->vstr, param_item->driverinit_value.vstr);
-	else
-		*init_val = param_item->driverinit_value;
+	*init_val = param_item->driverinit_value;
 
 	return 0;
 }
@@ -9690,10 +9684,7 @@ void devl_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
 						      DEVLINK_PARAM_CMODE_DRIVERINIT)))
 		return;
 
-	if (param_item->param->type == DEVLINK_PARAM_TYPE_STRING)
-		strcpy(param_item->driverinit_value.vstr, init_val.vstr);
-	else
-		param_item->driverinit_value = init_val;
+	param_item->driverinit_value = init_val;
 	param_item->driverinit_value_valid = true;
 
 	devlink_param_notify(devlink, 0, param_item, DEVLINK_CMD_PARAM_NEW);
-- 
2.39.0

[patch net-next v2 2/7] devlink: make sure driver does not read updated driverinit param before reload

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

The driverinit param purpose is to serve the driver during init/reload
time to provide a value, either default or set by user.

Make sure that driver does not read value updated by user before the
reload is performed. Hold the new value in a separate struct and switch
it during reload.

Note that this is required to be eventually possible to call
devl_param_driverinit_value_get() without holding instance lock.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
v1->v2:
- extended patch description with a note
- call driverinit_load_new only if action is REINIT
---
 include/net/devlink.h       |  4 ++++
 net/devlink/dev.c           |  3 +++
 net/devlink/devl_internal.h |  3 +++
 net/devlink/leftover.c      | 26 ++++++++++++++++++++++----
 4 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 2e85a5970a32..8ed960345f37 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -489,6 +489,10 @@ struct devlink_param_item {
 	const struct devlink_param *param;
 	union devlink_param_value driverinit_value;
 	bool driverinit_value_valid;
+	union devlink_param_value driverinit_value_new; /* Not reachable
+							 * until reload.
+							 */
+	bool driverinit_value_new_valid;
 };
 
 enum devlink_param_generic_id {
diff --git a/net/devlink/dev.c b/net/devlink/dev.c
index 78d824eda5ec..f995d88b9d04 100644
--- a/net/devlink/dev.c
+++ b/net/devlink/dev.c
@@ -369,6 +369,9 @@ int devlink_reload(struct devlink *devlink, struct net *dest_net,
 	if (dest_net && !net_eq(dest_net, curr_net))
 		devlink_reload_netns_change(devlink, curr_net, dest_net);
 
+	if (action == DEVLINK_RELOAD_ACTION_DRIVER_REINIT)
+		devlink_params_driverinit_load_new(devlink);
+
 	err = devlink->ops->reload_up(devlink, action, limit, actions_performed, extack);
 	devlink_reload_failed_set(devlink, !!err);
 	if (err)
diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index 941174e157d4..5c117e8d4377 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -189,6 +189,9 @@ static inline bool devlink_reload_supported(const struct devlink_ops *ops)
 	return ops->reload_down && ops->reload_up;
 }
 
+/* Params */
+void devlink_params_driverinit_load_new(struct devlink *devlink);
+
 /* Resources */
 struct devlink_resource;
 int devlink_resources_validate(struct devlink *devlink,
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index f2f6a2f42864..61e59556ea03 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -4098,9 +4098,12 @@ static int devlink_nl_param_fill(struct sk_buff *msg, struct devlink *devlink,
 		if (!devlink_param_cmode_is_supported(param, i))
 			continue;
 		if (i == DEVLINK_PARAM_CMODE_DRIVERINIT) {
-			if (!param_item->driverinit_value_valid)
+			if (param_item->driverinit_value_new_valid)
+				param_value[i] = param_item->driverinit_value_new;
+			else if (param_item->driverinit_value_valid)
+				param_value[i] = param_item->driverinit_value;
+			else
 				return -EOPNOTSUPP;
-			param_value[i] = param_item->driverinit_value;
 		} else {
 			ctx.cmode = i;
 			err = devlink_param_get(devlink, param, &ctx);
@@ -4388,8 +4391,8 @@ static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
 		return -EOPNOTSUPP;
 
 	if (cmode == DEVLINK_PARAM_CMODE_DRIVERINIT) {
-		param_item->driverinit_value = value;
-		param_item->driverinit_value_valid = true;
+		param_item->driverinit_value_new = value;
+		param_item->driverinit_value_new_valid = true;
 	} else {
 		if (!param->set)
 			return -EOPNOTSUPP;
@@ -9691,6 +9694,21 @@ void devl_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
 }
 EXPORT_SYMBOL_GPL(devl_param_driverinit_value_set);
 
+void devlink_params_driverinit_load_new(struct devlink *devlink)
+{
+	struct devlink_param_item *param_item;
+
+	list_for_each_entry(param_item, &devlink->param_list, list) {
+		if (!devlink_param_cmode_is_supported(param_item->param,
+						      DEVLINK_PARAM_CMODE_DRIVERINIT) ||
+		    !param_item->driverinit_value_new_valid)
+			continue;
+		param_item->driverinit_value = param_item->driverinit_value_new;
+		param_item->driverinit_value_valid = true;
+		param_item->driverinit_value_new_valid = false;
+	}
+}
+
 /**
  *	devl_param_value_changed - notify devlink on a parameter's value
  *				   change. Should be called by the driver
-- 
2.39.0

[patch net-next v2 3/7] devlink: fix the name of value arg of devl_param_driverinit_value_get()

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Probably due to copy-paste error, the name of the arg is "init_val"
which is misleading, as the pointer is used to point to struct where to
store the current value. Rename it to "val" and change the arg comment
a bit on the way.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
---
 include/net/devlink.h  | 2 +-
 net/devlink/leftover.c | 7 ++++---
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/include/net/devlink.h b/include/net/devlink.h
index 8ed960345f37..6a942e70e451 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1784,7 +1784,7 @@ void devlink_params_unregister(struct devlink *devlink,
 			       const struct devlink_param *params,
 			       size_t params_count);
 int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
-				    union devlink_param_value *init_val);
+				    union devlink_param_value *val);
 void devl_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
 				     union devlink_param_value init_val);
 void devl_param_value_changed(struct devlink *devlink, u32 param_id);
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index 61e59556ea03..dab7326dc3ea 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -9630,13 +9630,14 @@ EXPORT_SYMBOL_GPL(devlink_params_unregister);
  *
  *	@devlink: devlink
  *	@param_id: parameter ID
- *	@init_val: value of parameter in driverinit configuration mode
+ *	@val: pointer to store the value of parameter in driverinit
+ *	      configuration mode
  *
  *	This function should be used by the driver to get driverinit
  *	configuration for initialization after reload command.
  */
 int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
-				    union devlink_param_value *init_val)
+				    union devlink_param_value *val)
 {
 	struct devlink_param_item *param_item;
 
@@ -9656,7 +9657,7 @@ int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
 						      DEVLINK_PARAM_CMODE_DRIVERINIT)))
 		return -EOPNOTSUPP;
 
-	*init_val = param_item->driverinit_value;
+	*val = param_item->driverinit_value;
 
 	return 0;
 }
-- 
2.39.0

[patch net-next v2 4/7] devlink: use xa_for_each_start() helper in devlink_nl_cmd_port_get_dump_one()

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

As xarray has an iterator helper that allows to start from specified
index, use this directly and avoid repeated iteration from 0.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
---
 net/devlink/leftover.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index dab7326dc3ea..c7d9ef832c8c 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -1111,24 +1111,18 @@ devlink_nl_cmd_port_get_dump_one(struct sk_buff *msg, struct devlink *devlink,
 	struct devlink_nl_dump_state *state = devlink_dump_state(cb);
 	struct devlink_port *devlink_port;
 	unsigned long port_index;
-	int idx = 0;
 	int err = 0;
 
-	xa_for_each(&devlink->ports, port_index, devlink_port) {
-		if (idx < state->idx) {
-			idx++;
-			continue;
-		}
+	xa_for_each_start(&devlink->ports, port_index, devlink_port, state->idx) {
 		err = devlink_nl_port_fill(msg, devlink_port,
 					   DEVLINK_CMD_NEW,
 					   NETLINK_CB(cb->skb).portid,
 					   cb->nlh->nlmsg_seq,
 					   NLM_F_MULTI, cb->extack);
 		if (err) {
-			state->idx = idx;
+			state->idx = port_index;
 			break;
 		}
-		idx++;
 	}
 
 	return err;
-- 
2.39.0

[patch net-next v2 5/7] devlink: convert param list to xarray

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Loose the linked list for params and use xarray instead.

Note that this is required to be eventually possible to call
devl_param_driverinit_value_get() without holding instance lock.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
---
v1->v2:
- extended patch description with a note
---
 net/devlink/core.c          |  4 +--
 net/devlink/devl_internal.h |  2 +-
 net/devlink/leftover.c      | 72 ++++++++++++++++++-------------------
 3 files changed, 39 insertions(+), 39 deletions(-)

diff --git a/net/devlink/core.c b/net/devlink/core.c
index a4f47dafb864..777b091ef74d 100644
--- a/net/devlink/core.c
+++ b/net/devlink/core.c
@@ -212,6 +212,7 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
 	devlink->dev = dev;
 	devlink->ops = ops;
 	xa_init_flags(&devlink->ports, XA_FLAGS_ALLOC);
+	xa_init_flags(&devlink->params, XA_FLAGS_ALLOC);
 	xa_init_flags(&devlink->snapshot_ids, XA_FLAGS_ALLOC);
 	write_pnet(&devlink->_net, net);
 	INIT_LIST_HEAD(&devlink->rate_list);
@@ -219,7 +220,6 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
 	INIT_LIST_HEAD(&devlink->sb_list);
 	INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
 	INIT_LIST_HEAD(&devlink->resource_list);
-	INIT_LIST_HEAD(&devlink->param_list);
 	INIT_LIST_HEAD(&devlink->region_list);
 	INIT_LIST_HEAD(&devlink->reporter_list);
 	INIT_LIST_HEAD(&devlink->trap_list);
@@ -255,7 +255,6 @@ void devlink_free(struct devlink *devlink)
 	WARN_ON(!list_empty(&devlink->trap_list));
 	WARN_ON(!list_empty(&devlink->reporter_list));
 	WARN_ON(!list_empty(&devlink->region_list));
-	WARN_ON(!list_empty(&devlink->param_list));
 	WARN_ON(!list_empty(&devlink->resource_list));
 	WARN_ON(!list_empty(&devlink->dpipe_table_list));
 	WARN_ON(!list_empty(&devlink->sb_list));
@@ -264,6 +263,7 @@ void devlink_free(struct devlink *devlink)
 	WARN_ON(!xa_empty(&devlink->ports));
 
 	xa_destroy(&devlink->snapshot_ids);
+	xa_destroy(&devlink->params);
 	xa_destroy(&devlink->ports);
 
 	WARN_ON_ONCE(unregister_netdevice_notifier(&devlink->netdevice_nb));
diff --git a/net/devlink/devl_internal.h b/net/devlink/devl_internal.h
index 5c117e8d4377..2f4820e40d27 100644
--- a/net/devlink/devl_internal.h
+++ b/net/devlink/devl_internal.h
@@ -29,7 +29,7 @@ struct devlink {
 	struct list_head sb_list;
 	struct list_head dpipe_table_list;
 	struct list_head resource_list;
-	struct list_head param_list;
+	struct xarray params;
 	struct list_head region_list;
 	struct list_head reporter_list;
 	struct devlink_dpipe_headers *dpipe_headers;
diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index c7d9ef832c8c..6d3988f4e843 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -3954,26 +3954,22 @@ static int devlink_param_driver_verify(const struct devlink_param *param)
 }
 
 static struct devlink_param_item *
-devlink_param_find_by_name(struct list_head *param_list,
-			   const char *param_name)
+devlink_param_find_by_name(struct xarray *params, const char *param_name)
 {
 	struct devlink_param_item *param_item;
+	unsigned long param_id;
 
-	list_for_each_entry(param_item, param_list, list)
+	xa_for_each(params, param_id, param_item) {
 		if (!strcmp(param_item->param->name, param_name))
 			return param_item;
+	}
 	return NULL;
 }
 
 static struct devlink_param_item *
-devlink_param_find_by_id(struct list_head *param_list, u32 param_id)
+devlink_param_find_by_id(struct xarray *params, u32 param_id)
 {
-	struct devlink_param_item *param_item;
-
-	list_for_each_entry(param_item, param_list, list)
-		if (param_item->param->id == param_id)
-			return param_item;
-	return NULL;
+	return xa_load(params, param_id);
 }
 
 static bool
@@ -4202,14 +4198,10 @@ devlink_nl_cmd_param_get_dump_one(struct sk_buff *msg, struct devlink *devlink,
 {
 	struct devlink_nl_dump_state *state = devlink_dump_state(cb);
 	struct devlink_param_item *param_item;
-	int idx = 0;
+	unsigned long param_id;
 	int err = 0;
 
-	list_for_each_entry(param_item, &devlink->param_list, list) {
-		if (idx < state->idx) {
-			idx++;
-			continue;
-		}
+	xa_for_each_start(&devlink->params, param_id, param_item, state->idx) {
 		err = devlink_nl_param_fill(msg, devlink, 0, param_item,
 					    DEVLINK_CMD_PARAM_GET,
 					    NETLINK_CB(cb->skb).portid,
@@ -4218,10 +4210,9 @@ devlink_nl_cmd_param_get_dump_one(struct sk_buff *msg, struct devlink *devlink,
 		if (err == -EOPNOTSUPP) {
 			err = 0;
 		} else if (err) {
-			state->idx = idx;
+			state->idx = param_id;
 			break;
 		}
-		idx++;
 	}
 
 	return err;
@@ -4307,8 +4298,7 @@ devlink_param_value_get_from_info(const struct devlink_param *param,
 }
 
 static struct devlink_param_item *
-devlink_param_get_from_info(struct list_head *param_list,
-			    struct genl_info *info)
+devlink_param_get_from_info(struct xarray *params, struct genl_info *info)
 {
 	char *param_name;
 
@@ -4316,7 +4306,7 @@ devlink_param_get_from_info(struct list_head *param_list,
 		return NULL;
 
 	param_name = nla_data(info->attrs[DEVLINK_ATTR_PARAM_NAME]);
-	return devlink_param_find_by_name(param_list, param_name);
+	return devlink_param_find_by_name(params, param_name);
 }
 
 static int devlink_nl_cmd_param_get_doit(struct sk_buff *skb,
@@ -4327,7 +4317,7 @@ static int devlink_nl_cmd_param_get_doit(struct sk_buff *skb,
 	struct sk_buff *msg;
 	int err;
 
-	param_item = devlink_param_get_from_info(&devlink->param_list, info);
+	param_item = devlink_param_get_from_info(&devlink->params, info);
 	if (!param_item)
 		return -EINVAL;
 
@@ -4348,7 +4338,7 @@ static int devlink_nl_cmd_param_get_doit(struct sk_buff *skb,
 
 static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
 					   unsigned int port_index,
-					   struct list_head *param_list,
+					   struct xarray *params,
 					   struct genl_info *info,
 					   enum devlink_command cmd)
 {
@@ -4360,7 +4350,7 @@ static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
 	union devlink_param_value value;
 	int err = 0;
 
-	param_item = devlink_param_get_from_info(param_list, info);
+	param_item = devlink_param_get_from_info(params, info);
 	if (!param_item)
 		return -EINVAL;
 	param = param_item->param;
@@ -4406,7 +4396,7 @@ static int devlink_nl_cmd_param_set_doit(struct sk_buff *skb,
 {
 	struct devlink *devlink = info->user_ptr[0];
 
-	return __devlink_nl_cmd_param_set_doit(devlink, 0, &devlink->param_list,
+	return __devlink_nl_cmd_param_set_doit(devlink, 0, &devlink->params,
 					       info, DEVLINK_CMD_PARAM_NEW);
 }
 
@@ -8038,6 +8028,7 @@ void devlink_notify_register(struct devlink *devlink)
 	struct devlink_rate *rate_node;
 	struct devlink_region *region;
 	unsigned long port_index;
+	unsigned long param_id;
 
 	devlink_notify(devlink, DEVLINK_CMD_NEW);
 	list_for_each_entry(linecard, &devlink->linecard_list, list)
@@ -8063,7 +8054,7 @@ void devlink_notify_register(struct devlink *devlink)
 	list_for_each_entry(region, &devlink->region_list, list)
 		devlink_nl_region_notify(region, NULL, DEVLINK_CMD_REGION_NEW);
 
-	list_for_each_entry(param_item, &devlink->param_list, list)
+	xa_for_each(&devlink->params, param_id, param_item)
 		devlink_param_notify(devlink, 0, param_item,
 				     DEVLINK_CMD_PARAM_NEW);
 }
@@ -8078,8 +8069,9 @@ void devlink_notify_unregister(struct devlink *devlink)
 	struct devlink_rate *rate_node;
 	struct devlink_region *region;
 	unsigned long port_index;
+	unsigned long param_id;
 
-	list_for_each_entry_reverse(param_item, &devlink->param_list, list)
+	xa_for_each(&devlink->params, param_id, param_item)
 		devlink_param_notify(devlink, 0, param_item,
 				     DEVLINK_CMD_PARAM_DEL);
 
@@ -9506,9 +9498,10 @@ static int devlink_param_register(struct devlink *devlink,
 				  const struct devlink_param *param)
 {
 	struct devlink_param_item *param_item;
+	int err;
 
 	WARN_ON(devlink_param_verify(param));
-	WARN_ON(devlink_param_find_by_name(&devlink->param_list, param->name));
+	WARN_ON(devlink_param_find_by_name(&devlink->params, param->name));
 
 	if (param->supported_cmodes == BIT(DEVLINK_PARAM_CMODE_DRIVERINIT))
 		WARN_ON(param->get || param->set);
@@ -9521,9 +9514,16 @@ static int devlink_param_register(struct devlink *devlink,
 
 	param_item->param = param;
 
-	list_add_tail(&param_item->list, &devlink->param_list);
+	err = xa_insert(&devlink->params, param->id, param_item, GFP_KERNEL);
+	if (err)
+		goto err_xa_insert;
+
 	devlink_param_notify(devlink, 0, param_item, DEVLINK_CMD_PARAM_NEW);
 	return 0;
+
+err_xa_insert:
+	kfree(param_item);
+	return err;
 }
 
 static void devlink_param_unregister(struct devlink *devlink,
@@ -9531,12 +9531,11 @@ static void devlink_param_unregister(struct devlink *devlink,
 {
 	struct devlink_param_item *param_item;
 
-	param_item =
-		devlink_param_find_by_name(&devlink->param_list, param->name);
+	param_item = devlink_param_find_by_id(&devlink->params, param->id);
 	if (WARN_ON(!param_item))
 		return;
 	devlink_param_notify(devlink, 0, param_item, DEVLINK_CMD_PARAM_DEL);
-	list_del(&param_item->list);
+	xa_erase(&devlink->params, param->id);
 	kfree(param_item);
 }
 
@@ -9640,7 +9639,7 @@ int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
 	if (WARN_ON(!devlink_reload_supported(devlink->ops)))
 		return -EOPNOTSUPP;
 
-	param_item = devlink_param_find_by_id(&devlink->param_list, param_id);
+	param_item = devlink_param_find_by_id(&devlink->params, param_id);
 	if (!param_item)
 		return -EINVAL;
 
@@ -9674,7 +9673,7 @@ void devl_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
 {
 	struct devlink_param_item *param_item;
 
-	param_item = devlink_param_find_by_id(&devlink->param_list, param_id);
+	param_item = devlink_param_find_by_id(&devlink->params, param_id);
 	if (WARN_ON(!param_item))
 		return;
 
@@ -9692,8 +9691,9 @@ EXPORT_SYMBOL_GPL(devl_param_driverinit_value_set);
 void devlink_params_driverinit_load_new(struct devlink *devlink)
 {
 	struct devlink_param_item *param_item;
+	unsigned long param_id;
 
-	list_for_each_entry(param_item, &devlink->param_list, list) {
+	xa_for_each(&devlink->params, param_id, param_item) {
 		if (!devlink_param_cmode_is_supported(param_item->param,
 						      DEVLINK_PARAM_CMODE_DRIVERINIT) ||
 		    !param_item->driverinit_value_new_valid)
@@ -9720,7 +9720,7 @@ void devl_param_value_changed(struct devlink *devlink, u32 param_id)
 {
 	struct devlink_param_item *param_item;
 
-	param_item = devlink_param_find_by_id(&devlink->param_list, param_id);
+	param_item = devlink_param_find_by_id(&devlink->params, param_id);
 	WARN_ON(!param_item);
 
 	devlink_param_notify(devlink, 0, param_item, DEVLINK_CMD_PARAM_NEW);
-- 
2.39.0

[patch net-next v2 6/7] devlink: allow to call devl_param_driverinit_value_get() without holding instance lock

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

If the driver maintains following basic sane behavior, the
devl_param_driverinit_value_get() function could be called without
holding instance lock:

1) Driver ensures a call to devl_param_driverinit_value_get() cannot
   race with registering/unregistering the parameter with
   the same parameter ID.
2) Driver ensures a call to devl_param_driverinit_value_get() cannot
   race with devl_param_driverinit_value_set() call with
   the same parameter ID.
3) Driver ensures a call to devl_param_driverinit_value_get() cannot
   race with reload operation.

By the nature of params usage, these requirements should be
trivially achievable. If the driver for some off reason
is not able to comply, it has to take the devlink->lock while
calling devl_param_driverinit_value_get().

Remove the lock assertion and add comment describing
the locking requirements.

This fixes a splat in mlx5 driver introduced by the commit
referenced in the "Fixes" tag.

Lore: https://lore.kernel.org/netdev/719de4f0-76ac-e8b9-38a9-167ae239efc7@amd.com/
Reported-by: Kim Phillips <kim.phillips@amd.com>
Fixes: 075935f0ae0f ("devlink: protect devlink param list by instance lock")
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
---
 net/devlink/leftover.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index 6d3988f4e843..9f0256c2c323 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -9628,14 +9628,23 @@ EXPORT_SYMBOL_GPL(devlink_params_unregister);
  *
  *	This function should be used by the driver to get driverinit
  *	configuration for initialization after reload command.
+ *
+ *	Note that lockless call of this function relies on the
+ *	driver to maintain following basic sane behavior:
+ *	1) Driver ensures a call to this function cannot race with
+ *	   registering/unregistering the parameter with the same parameter ID.
+ *	2) Driver ensures a call to this function cannot race with
+ *	   devl_param_driverinit_value_set() call with the same parameter ID.
+ *	3) Driver ensures a call to this function cannot race with
+ *	   reload operation.
+ *	If the driver is not able to comply, it has to take the devlink->lock
+ *	while calling this.
  */
 int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
 				    union devlink_param_value *val)
 {
 	struct devlink_param_item *param_item;
 
-	lockdep_assert_held(&devlink->lock);
-
 	if (WARN_ON(!devlink_reload_supported(devlink->ops)))
 		return -EOPNOTSUPP;
 
-- 
2.39.0

[patch net-next v2 7/7] devlink: add forgotten devlink instance lock assertion to devl_param_driverinit_value_set()

[Jiri Pirko]

From: Jiri Pirko <jiri@nvidia.com>

Driver calling devl_param_driverinit_value_set() has to hold devlink
instance lock while doing that. Put an assertion there.

Signed-off-by: Jiri Pirko <jiri@nvidia.com>
Reviewed-by: Simon Horman <simon.horman@corigine.com>
Acked-by: Jakub Kicinski <kuba@kernel.org>
---
 net/devlink/leftover.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
index 9f0256c2c323..38cdbc2039dd 100644
--- a/net/devlink/leftover.c
+++ b/net/devlink/leftover.c
@@ -9682,6 +9682,8 @@ void devl_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
 {
 	struct devlink_param_item *param_item;
 
+	devl_assert_locked(devlink);
+
 	param_item = devlink_param_find_by_id(&devlink->params, param_id);
 	if (WARN_ON(!param_item))
 		return;
-- 
2.39.0

Re: [patch net-next v2 2/7] devlink: make sure driver does not read updated driverinit param before reload

[Simon Horman]

On Fri, Feb 10, 2023 at 11:01:26AM +0100, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> The driverinit param purpose is to serve the driver during init/reload
> time to provide a value, either default or set by user.
> 
> Make sure that driver does not read value updated by user before the
> reload is performed. Hold the new value in a separate struct and switch
> it during reload.
> 
> Note that this is required to be eventually possible to call
> devl_param_driverinit_value_get() without holding instance lock.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> ---
> v1->v2:
> - extended patch description with a note

Thanks!

> - call driverinit_load_new only if action is REINIT

Reviewed-by: Simon Horman <simon.horman@corigine.com>

Re: [patch net-next v2 1/7] devlink: don't use strcpy() to copy param value

[Jacob Keller]

On 2/10/2023 2:01 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> No need to treat string params any different comparing to other types.
> Rely on the struct assign to copy the whole struct, including the
> string.

I suspect the strcpy came from thinking about it as if it stored a
pointer to the string and not as an array struct member.

Makes sense.

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

> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> Acked-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/devlink/leftover.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
> 
> diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
> index f05ab093d231..f2f6a2f42864 100644
> --- a/net/devlink/leftover.c
> +++ b/net/devlink/leftover.c
> @@ -4388,10 +4388,7 @@ static int __devlink_nl_cmd_param_set_doit(struct devlink *devlink,
>  		return -EOPNOTSUPP;
>  
>  	if (cmode == DEVLINK_PARAM_CMODE_DRIVERINIT) {
> -		if (param->type == DEVLINK_PARAM_TYPE_STRING)
> -			strcpy(param_item->driverinit_value.vstr, value.vstr);
> -		else
> -			param_item->driverinit_value = value;
> +		param_item->driverinit_value = value;
>  		param_item->driverinit_value_valid = true;
>  	} else {
>  		if (!param->set)
> @@ -9656,10 +9653,7 @@ int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
>  						      DEVLINK_PARAM_CMODE_DRIVERINIT)))
>  		return -EOPNOTSUPP;
>  
> -	if (param_item->param->type == DEVLINK_PARAM_TYPE_STRING)
> -		strcpy(init_val->vstr, param_item->driverinit_value.vstr);
> -	else
> -		*init_val = param_item->driverinit_value;
> +	*init_val = param_item->driverinit_value;
>  
>  	return 0;
>  }
> @@ -9690,10 +9684,7 @@ void devl_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
>  						      DEVLINK_PARAM_CMODE_DRIVERINIT)))
>  		return;
>  
> -	if (param_item->param->type == DEVLINK_PARAM_TYPE_STRING)
> -		strcpy(param_item->driverinit_value.vstr, init_val.vstr);
> -	else
> -		param_item->driverinit_value = init_val;
> +	param_item->driverinit_value = init_val;
>  	param_item->driverinit_value_valid = true;
>  
>  	devlink_param_notify(devlink, 0, param_item, DEVLINK_CMD_PARAM_NEW);

Re: [patch net-next v2 3/7] devlink: fix the name of value arg of devl_param_driverinit_value_get()

[Jacob Keller]

On 2/10/2023 2:01 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Probably due to copy-paste error, the name of the arg is "init_val"
> which is misleading, as the pointer is used to point to struct where to
> store the current value. Rename it to "val" and change the arg comment
> a bit on the way.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> Acked-by: Jakub Kicinski <kuba@kernel.org>

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

> ---
>  include/net/devlink.h  | 2 +-
>  net/devlink/leftover.c | 7 ++++---
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/devlink.h b/include/net/devlink.h
> index 8ed960345f37..6a942e70e451 100644
> --- a/include/net/devlink.h
> +++ b/include/net/devlink.h
> @@ -1784,7 +1784,7 @@ void devlink_params_unregister(struct devlink *devlink,
>  			       const struct devlink_param *params,
>  			       size_t params_count);
>  int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
> -				    union devlink_param_value *init_val);
> +				    union devlink_param_value *val);
>  void devl_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
>  				     union devlink_param_value init_val);
>  void devl_param_value_changed(struct devlink *devlink, u32 param_id);
> diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
> index 61e59556ea03..dab7326dc3ea 100644
> --- a/net/devlink/leftover.c
> +++ b/net/devlink/leftover.c
> @@ -9630,13 +9630,14 @@ EXPORT_SYMBOL_GPL(devlink_params_unregister);
>   *
>   *	@devlink: devlink
>   *	@param_id: parameter ID
> - *	@init_val: value of parameter in driverinit configuration mode
> + *	@val: pointer to store the value of parameter in driverinit
> + *	      configuration mode
>   *
>   *	This function should be used by the driver to get driverinit
>   *	configuration for initialization after reload command.
>   */
>  int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
> -				    union devlink_param_value *init_val)
> +				    union devlink_param_value *val)
>  {
>  	struct devlink_param_item *param_item;
>  
> @@ -9656,7 +9657,7 @@ int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
>  						      DEVLINK_PARAM_CMODE_DRIVERINIT)))
>  		return -EOPNOTSUPP;
>  
> -	*init_val = param_item->driverinit_value;
> +	*val = param_item->driverinit_value;
>  
>  	return 0;
>  }

Re: [patch net-next v2 4/7] devlink: use xa_for_each_start() helper in devlink_nl_cmd_port_get_dump_one()

[Jacob Keller]

On 2/10/2023 2:01 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> As xarray has an iterator helper that allows to start from specified
> index, use this directly and avoid repeated iteration from 0.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> Acked-by: Jakub Kicinski <kuba@kernel.org>

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

Re: [patch net-next v2 5/7] devlink: convert param list to xarray

[Jacob Keller]

On 2/10/2023 2:01 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Loose the linked list for params and use xarray instead.
> 
> Note that this is required to be eventually possible to call
> devl_param_driverinit_value_get() without holding instance lock.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> Acked-by: Jakub Kicinski <kuba@kernel.org>

That makes loading by the parameter id faster too since we don't have to
iterate the full list. Nice.

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

Re: [patch net-next v2 6/7] devlink: allow to call devl_param_driverinit_value_get() without holding instance lock

[Jacob Keller]

On 2/10/2023 2:01 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> If the driver maintains following basic sane behavior, the
> devl_param_driverinit_value_get() function could be called without
> holding instance lock:
> 
> 1) Driver ensures a call to devl_param_driverinit_value_get() cannot
>    race with registering/unregistering the parameter with
>    the same parameter ID.
> 2) Driver ensures a call to devl_param_driverinit_value_get() cannot
>    race with devl_param_driverinit_value_set() call with
>    the same parameter ID.
> 3) Driver ensures a call to devl_param_driverinit_value_get() cannot
>    race with reload operation.
> 
> By the nature of params usage, these requirements should be
> trivially achievable. If the driver for some off reason
> is not able to comply, it has to take the devlink->lock while
> calling devl_param_driverinit_value_get().
> 
> Remove the lock assertion and add comment describing
> the locking requirements.
> 
> This fixes a splat in mlx5 driver introduced by the commit
> referenced in the "Fixes" tag.

Just to clarify, is it possible for mlx5 to take the instance lock
instead of these changes? I agree the improvements make sense here, but
it seems like an alternative fix would be to grab the lock around the
get function call in mlx5.

Either way, the assumptions here seem reasonable and the series makes sense.

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

Thanks,
Jake

> 
> Lore: https://lore.kernel.org/netdev/719de4f0-76ac-e8b9-38a9-167ae239efc7@amd.com/
> Reported-by: Kim Phillips <kim.phillips@amd.com>
> Fixes: 075935f0ae0f ("devlink: protect devlink param list by instance lock")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> Acked-by: Jakub Kicinski <kuba@kernel.org>
> ---
>  net/devlink/leftover.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
> index 6d3988f4e843..9f0256c2c323 100644
> --- a/net/devlink/leftover.c
> +++ b/net/devlink/leftover.c
> @@ -9628,14 +9628,23 @@ EXPORT_SYMBOL_GPL(devlink_params_unregister);
>   *
>   *	This function should be used by the driver to get driverinit
>   *	configuration for initialization after reload command.
> + *
> + *	Note that lockless call of this function relies on the
> + *	driver to maintain following basic sane behavior:
> + *	1) Driver ensures a call to this function cannot race with
> + *	   registering/unregistering the parameter with the same parameter ID.
> + *	2) Driver ensures a call to this function cannot race with
> + *	   devl_param_driverinit_value_set() call with the same parameter ID.
> + *	3) Driver ensures a call to this function cannot race with
> + *	   reload operation.
> + *	If the driver is not able to comply, it has to take the devlink->lock
> + *	while calling this.
>   */
>  int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
>  				    union devlink_param_value *val)
>  {
>  	struct devlink_param_item *param_item;
>  
> -	lockdep_assert_held(&devlink->lock);
> -
>  	if (WARN_ON(!devlink_reload_supported(devlink->ops)))
>  		return -EOPNOTSUPP;
>  

Re: [patch net-next v2 7/7] devlink: add forgotten devlink instance lock assertion to devl_param_driverinit_value_set()

[Jacob Keller]

On 2/10/2023 2:01 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> Driver calling devl_param_driverinit_value_set() has to hold devlink
> instance lock while doing that. Put an assertion there.
> 
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> Acked-by: Jakub Kicinski <kuba@kernel.org>

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

> ---
>  net/devlink/leftover.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
> index 9f0256c2c323..38cdbc2039dd 100644
> --- a/net/devlink/leftover.c
> +++ b/net/devlink/leftover.c
> @@ -9682,6 +9682,8 @@ void devl_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
>  {
>  	struct devlink_param_item *param_item;
>  
> +	devl_assert_locked(devlink);
> +
>  	param_item = devlink_param_find_by_id(&devlink->params, param_id);
>  	if (WARN_ON(!param_item))
>  		return;

Re: [patch net-next v2 6/7] devlink: allow to call devl_param_driverinit_value_get() without holding instance lock

[Kim Phillips]

On 2/10/23 4:01 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> If the driver maintains following basic sane behavior, the
> devl_param_driverinit_value_get() function could be called without
> holding instance lock:
> 
> 1) Driver ensures a call to devl_param_driverinit_value_get() cannot
>     race with registering/unregistering the parameter with
>     the same parameter ID.
> 2) Driver ensures a call to devl_param_driverinit_value_get() cannot
>     race with devl_param_driverinit_value_set() call with
>     the same parameter ID.
> 3) Driver ensures a call to devl_param_driverinit_value_get() cannot
>     race with reload operation.
> 
> By the nature of params usage, these requirements should be
> trivially achievable. If the driver for some off reason
> is not able to comply, it has to take the devlink->lock while
> calling devl_param_driverinit_value_get().
> 
> Remove the lock assertion and add comment describing
> the locking requirements.
> 
> This fixes a splat in mlx5 driver introduced by the commit
> referenced in the "Fixes" tag.
> 
> Lore: https://lore.kernel.org/netdev/719de4f0-76ac-e8b9-38a9-167ae239efc7@amd.com/
> Reported-by: Kim Phillips <kim.phillips@amd.com>
> Fixes: 075935f0ae0f ("devlink: protect devlink param list by instance lock")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
> Reviewed-by: Simon Horman <simon.horman@corigine.com>
> Acked-by: Jakub Kicinski <kuba@kernel.org>
> ---

Please add my:

Tested-by: Kim Phillips <kim.phillips@amd.com>

to this patch, if not the entire series.

Thanks!

Kim

p.s. Sorry about my testing snafu on v1 of this series - it wasn't
clear to me what fixes were where...

Re: [patch net-next v2 6/7] devlink: allow to call devl_param_driverinit_value_get() without holding instance lock

[Jiri Pirko]

Fri, Feb 10, 2023 at 08:41:27PM CET, jacob.e.keller@intel.com wrote:
>
>
>On 2/10/2023 2:01 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@nvidia.com>
>> 
>> If the driver maintains following basic sane behavior, the
>> devl_param_driverinit_value_get() function could be called without
>> holding instance lock:
>> 
>> 1) Driver ensures a call to devl_param_driverinit_value_get() cannot
>>    race with registering/unregistering the parameter with
>>    the same parameter ID.
>> 2) Driver ensures a call to devl_param_driverinit_value_get() cannot
>>    race with devl_param_driverinit_value_set() call with
>>    the same parameter ID.
>> 3) Driver ensures a call to devl_param_driverinit_value_get() cannot
>>    race with reload operation.
>> 
>> By the nature of params usage, these requirements should be
>> trivially achievable. If the driver for some off reason
>> is not able to comply, it has to take the devlink->lock while
>> calling devl_param_driverinit_value_get().
>> 
>> Remove the lock assertion and add comment describing
>> the locking requirements.
>> 
>> This fixes a splat in mlx5 driver introduced by the commit
>> referenced in the "Fixes" tag.
>
>Just to clarify, is it possible for mlx5 to take the instance lock
>instead of these changes? I agree the improvements make sense here, but
>it seems like an alternative fix would be to grab the lock around the
>get function call in mlx5.

You are correct, yes.

>
>Either way, the assumptions here seem reasonable and the series makes sense.
>
>Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
>
>Thanks,
>Jake
>
>> 
>> Lore: https://lore.kernel.org/netdev/719de4f0-76ac-e8b9-38a9-167ae239efc7@amd.com/
>> Reported-by: Kim Phillips <kim.phillips@amd.com>
>> Fixes: 075935f0ae0f ("devlink: protect devlink param list by instance lock")
>> Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>> Reviewed-by: Simon Horman <simon.horman@corigine.com>
>> Acked-by: Jakub Kicinski <kuba@kernel.org>
>> ---
>>  net/devlink/leftover.c | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>> 
>> diff --git a/net/devlink/leftover.c b/net/devlink/leftover.c
>> index 6d3988f4e843..9f0256c2c323 100644
>> --- a/net/devlink/leftover.c
>> +++ b/net/devlink/leftover.c
>> @@ -9628,14 +9628,23 @@ EXPORT_SYMBOL_GPL(devlink_params_unregister);
>>   *
>>   *	This function should be used by the driver to get driverinit
>>   *	configuration for initialization after reload command.
>> + *
>> + *	Note that lockless call of this function relies on the
>> + *	driver to maintain following basic sane behavior:
>> + *	1) Driver ensures a call to this function cannot race with
>> + *	   registering/unregistering the parameter with the same parameter ID.
>> + *	2) Driver ensures a call to this function cannot race with
>> + *	   devl_param_driverinit_value_set() call with the same parameter ID.
>> + *	3) Driver ensures a call to this function cannot race with
>> + *	   reload operation.
>> + *	If the driver is not able to comply, it has to take the devlink->lock
>> + *	while calling this.
>>   */
>>  int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
>>  				    union devlink_param_value *val)
>>  {
>>  	struct devlink_param_item *param_item;
>>  
>> -	lockdep_assert_held(&devlink->lock);
>> -
>>  	if (WARN_ON(!devlink_reload_supported(devlink->ops)))
>>  		return -EOPNOTSUPP;
>>  

Re: [patch net-next v2 0/7] devlink: params cleanups and devl_param_driverinit_value_get() fix

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Fri, 10 Feb 2023 11:01:24 +0100 you wrote:
> From: Jiri Pirko <jiri@nvidia.com>
> 
> The primary motivation of this patchset is the patch #6, which fixes an
> issue introduced by 075935f0ae0f ("devlink: protect devlink param list
> by instance lock") and reported by Kim Phillips <kim.phillips@amd.com>
> (https://lore.kernel.org/netdev/719de4f0-76ac-e8b9-38a9-167ae239efc7@amd.com/)
> and my colleagues doing mlx5 driver regression testing.
> 
> [...]

Here is the summary with links:
  - [net-next,v2,1/7] devlink: don't use strcpy() to copy param value
    https://git.kernel.org/netdev/net-next/c/fa2f921f3bf1
  - [net-next,v2,2/7] devlink: make sure driver does not read updated driverinit param before reload
    https://git.kernel.org/netdev/net-next/c/afd888c3e19c
  - [net-next,v2,3/7] devlink: fix the name of value arg of devl_param_driverinit_value_get()
    https://git.kernel.org/netdev/net-next/c/94ba1c316b9c
  - [net-next,v2,4/7] devlink: use xa_for_each_start() helper in devlink_nl_cmd_port_get_dump_one()
    https://git.kernel.org/netdev/net-next/c/fbcf938150ec
  - [net-next,v2,5/7] devlink: convert param list to xarray
    https://git.kernel.org/netdev/net-next/c/a72e17b45232
  - [net-next,v2,6/7] devlink: allow to call devl_param_driverinit_value_get() without holding instance lock
    https://git.kernel.org/netdev/net-next/c/280f7b2adca0
  - [net-next,v2,7/7] devlink: add forgotten devlink instance lock assertion to devl_param_driverinit_value_set()
    https://git.kernel.org/netdev/net-next/c/6b4bfa43ce29

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