Messages in this thread | | | Date | Mon, 31 Aug 2020 12:48:27 +0200 | From | Jiri Pirko <> | Subject | Re: [PATCH net-next RFC v3 02/14] devlink: Add reload actions counters |
| |
Sun, Aug 30, 2020 at 05:27:22PM CEST, moshe@mellanox.com wrote: >Add reload actions counters to hold the history per reload action type. >For example, the number of times fw_activate has been done on this >device since the driver module was added or if the firmware activation >was done with or without reset. >The function devlink_reload_actions_cnts_update() is exported to enable >also drivers update on reload actions done, for example in case firmware >activation with reset finished successfully but was initiated by remote >host. > >Signed-off-by: Moshe Shemesh <moshe@mellanox.com> >--- >v2 -> v3: >- New patch >--- > include/net/devlink.h | 2 ++ > net/core/devlink.c | 24 +++++++++++++++++++++--- > 2 files changed, 23 insertions(+), 3 deletions(-) > >diff --git a/include/net/devlink.h b/include/net/devlink.h >index b8f0152a1fff..0547f0707d92 100644 >--- a/include/net/devlink.h >+++ b/include/net/devlink.h >@@ -38,6 +38,7 @@ struct devlink { > struct list_head trap_policer_list; > const struct devlink_ops *ops; > struct xarray snapshot_ids; >+ u32 reload_actions_cnts[DEVLINK_RELOAD_ACTION_MAX]; > struct device *dev; > possible_net_t _net; > struct mutex lock; /* Serializes access to devlink instance specific objects such as >@@ -1372,6 +1373,7 @@ void > devlink_health_reporter_recovery_done(struct devlink_health_reporter *reporter); > > bool devlink_is_reload_failed(const struct devlink *devlink); >+void devlink_reload_actions_cnts_update(struct devlink *devlink, unsigned long actions_done); > > void devlink_flash_update_begin_notify(struct devlink *devlink); > void devlink_flash_update_end_notify(struct devlink *devlink); >diff --git a/net/core/devlink.c b/net/core/devlink.c >index 8d4137ad40db..20a29c34ff71 100644 >--- a/net/core/devlink.c >+++ b/net/core/devlink.c >@@ -2969,10 +2969,23 @@ bool devlink_is_reload_failed(const struct devlink *devlink) > } > EXPORT_SYMBOL_GPL(devlink_is_reload_failed); > >+void devlink_reload_actions_cnts_update(struct devlink *devlink, unsigned long actions_done) >+{ >+ int action; >+ >+ for (action = 0; action < DEVLINK_RELOAD_ACTION_MAX; action++) { >+ if (!test_bit(action, &actions_done)) >+ continue; >+ devlink->reload_actions_cnts[action]++; >+ } >+} >+EXPORT_SYMBOL_GPL(devlink_reload_actions_cnts_update);
I don't follow why this is an exported symbol if you only use it from this .c. Looks like a leftover...
>+ > static int devlink_reload(struct devlink *devlink, struct net *dest_net, > enum devlink_reload_action action, struct netlink_ext_ack *extack, >- unsigned long *actions_done) >+ unsigned long *actions_done_out) > { >+ unsigned long actions_done; > int err; > > if (!devlink->reload_enabled) >@@ -2985,9 +2998,14 @@ static int devlink_reload(struct devlink *devlink, struct net *dest_net, > if (dest_net && !net_eq(dest_net, devlink_net(devlink))) > devlink_reload_netns_change(devlink, dest_net); > >- err = devlink->ops->reload_up(devlink, action, extack, actions_done); >+ err = devlink->ops->reload_up(devlink, action, extack, &actions_done); > devlink_reload_failed_set(devlink, !!err); >- return err; >+ if (err) >+ return err; >+ devlink_reload_actions_cnts_update(devlink, actions_done); >+ if (actions_done_out) >+ *actions_done_out = actions_done;
Why don't you just use the original actions_done directly without having extra local variable?
>+ return 0; > } > > static int >-- >2.17.1 >
| |