Commit 238052e0 authored by David S. Miller's avatar David S. Miller

Merge branch 'devlink-params-cleanup'

Jiri Pirko says:

====================
devlink: params cleanups and devl_param_driverinit_value_get() fix

The primary motivation of this patchset is the patch #6, which fixes an
issue introduced by 075935f0 ("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
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 170677fe 6b4bfa43
...@@ -489,6 +489,10 @@ struct devlink_param_item { ...@@ -489,6 +489,10 @@ struct devlink_param_item {
const struct devlink_param *param; const struct devlink_param *param;
union devlink_param_value driverinit_value; union devlink_param_value driverinit_value;
bool driverinit_value_valid; 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 { enum devlink_param_generic_id {
...@@ -1780,7 +1784,7 @@ void devlink_params_unregister(struct devlink *devlink, ...@@ -1780,7 +1784,7 @@ void devlink_params_unregister(struct devlink *devlink,
const struct devlink_param *params, const struct devlink_param *params,
size_t params_count); size_t params_count);
int devl_param_driverinit_value_get(struct devlink *devlink, u32 param_id, 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, void devl_param_driverinit_value_set(struct devlink *devlink, u32 param_id,
union devlink_param_value init_val); union devlink_param_value init_val);
void devl_param_value_changed(struct devlink *devlink, u32 param_id); void devl_param_value_changed(struct devlink *devlink, u32 param_id);
......
...@@ -212,6 +212,7 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops, ...@@ -212,6 +212,7 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
devlink->dev = dev; devlink->dev = dev;
devlink->ops = ops; devlink->ops = ops;
xa_init_flags(&devlink->ports, XA_FLAGS_ALLOC); 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); xa_init_flags(&devlink->snapshot_ids, XA_FLAGS_ALLOC);
write_pnet(&devlink->_net, net); write_pnet(&devlink->_net, net);
INIT_LIST_HEAD(&devlink->rate_list); INIT_LIST_HEAD(&devlink->rate_list);
...@@ -219,7 +220,6 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops, ...@@ -219,7 +220,6 @@ struct devlink *devlink_alloc_ns(const struct devlink_ops *ops,
INIT_LIST_HEAD(&devlink->sb_list); INIT_LIST_HEAD(&devlink->sb_list);
INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list); INIT_LIST_HEAD_RCU(&devlink->dpipe_table_list);
INIT_LIST_HEAD(&devlink->resource_list); INIT_LIST_HEAD(&devlink->resource_list);
INIT_LIST_HEAD(&devlink->param_list);
INIT_LIST_HEAD(&devlink->region_list); INIT_LIST_HEAD(&devlink->region_list);
INIT_LIST_HEAD(&devlink->reporter_list); INIT_LIST_HEAD(&devlink->reporter_list);
INIT_LIST_HEAD(&devlink->trap_list); INIT_LIST_HEAD(&devlink->trap_list);
...@@ -255,7 +255,6 @@ void devlink_free(struct devlink *devlink) ...@@ -255,7 +255,6 @@ void devlink_free(struct devlink *devlink)
WARN_ON(!list_empty(&devlink->trap_list)); WARN_ON(!list_empty(&devlink->trap_list));
WARN_ON(!list_empty(&devlink->reporter_list)); WARN_ON(!list_empty(&devlink->reporter_list));
WARN_ON(!list_empty(&devlink->region_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->resource_list));
WARN_ON(!list_empty(&devlink->dpipe_table_list)); WARN_ON(!list_empty(&devlink->dpipe_table_list));
WARN_ON(!list_empty(&devlink->sb_list)); WARN_ON(!list_empty(&devlink->sb_list));
...@@ -264,6 +263,7 @@ void devlink_free(struct devlink *devlink) ...@@ -264,6 +263,7 @@ void devlink_free(struct devlink *devlink)
WARN_ON(!xa_empty(&devlink->ports)); WARN_ON(!xa_empty(&devlink->ports));
xa_destroy(&devlink->snapshot_ids); xa_destroy(&devlink->snapshot_ids);
xa_destroy(&devlink->params);
xa_destroy(&devlink->ports); xa_destroy(&devlink->ports);
WARN_ON_ONCE(unregister_netdevice_notifier(&devlink->netdevice_nb)); WARN_ON_ONCE(unregister_netdevice_notifier(&devlink->netdevice_nb));
......
...@@ -369,6 +369,9 @@ int devlink_reload(struct devlink *devlink, struct net *dest_net, ...@@ -369,6 +369,9 @@ int devlink_reload(struct devlink *devlink, struct net *dest_net,
if (dest_net && !net_eq(dest_net, curr_net)) if (dest_net && !net_eq(dest_net, curr_net))
devlink_reload_netns_change(devlink, curr_net, dest_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); err = devlink->ops->reload_up(devlink, action, limit, actions_performed, extack);
devlink_reload_failed_set(devlink, !!err); devlink_reload_failed_set(devlink, !!err);
if (err) if (err)
......
...@@ -29,7 +29,7 @@ struct devlink { ...@@ -29,7 +29,7 @@ struct devlink {
struct list_head sb_list; struct list_head sb_list;
struct list_head dpipe_table_list; struct list_head dpipe_table_list;
struct list_head resource_list; struct list_head resource_list;
struct list_head param_list; struct xarray params;
struct list_head region_list; struct list_head region_list;
struct list_head reporter_list; struct list_head reporter_list;
struct devlink_dpipe_headers *dpipe_headers; struct devlink_dpipe_headers *dpipe_headers;
...@@ -189,6 +189,9 @@ static inline bool devlink_reload_supported(const struct devlink_ops *ops) ...@@ -189,6 +189,9 @@ static inline bool devlink_reload_supported(const struct devlink_ops *ops)
return ops->reload_down && ops->reload_up; return ops->reload_down && ops->reload_up;
} }
/* Params */
void devlink_params_driverinit_load_new(struct devlink *devlink);
/* Resources */ /* Resources */
struct devlink_resource; struct devlink_resource;
int devlink_resources_validate(struct devlink *devlink, int devlink_resources_validate(struct devlink *devlink,
......
This diff is collapsed.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment