Message ID | 1579446268-26540-1-git-send-email-moshe@mellanox.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [net-next] devlink: Add health recover notifications on devlink flows | expand |
From: Moshe Shemesh <moshe@mellanox.com> Date: Sun, 19 Jan 2020 17:04:28 +0200 > Devlink health recover notifications were added only on driver direct > updates of health_state through devlink_health_reporter_state_update(). > Add notifications on updates of health_state by devlink flows of report > and recover. > > Fixes: 97ff3bd37fac ("devlink: add devink notification when reporter update health state") > Signed-off-by: Moshe Shemesh <moshe@mellanox.com> > Acked-by: Jiri Pirko <jiri@mellanox.com> I really dislike forward declarations and almost all of the time they are unnecessary. Could you please just rearrange the code as needed and resubmit? Thank you.
On 1/20/2020 11:50 AM, David Miller wrote: > From: Moshe Shemesh <moshe@mellanox.com> > Date: Sun, 19 Jan 2020 17:04:28 +0200 > >> Devlink health recover notifications were added only on driver direct >> updates of health_state through devlink_health_reporter_state_update(). >> Add notifications on updates of health_state by devlink flows of report >> and recover. >> >> Fixes: 97ff3bd37fac ("devlink: add devink notification when reporter update health state") >> Signed-off-by: Moshe Shemesh <moshe@mellanox.com> >> Acked-by: Jiri Pirko <jiri@mellanox.com> > I really dislike forward declarations and almost all of the time they are > unnecessary. > > Could you please just rearrange the code as needed and resubmit? Sure, I will resubmit. > Thank you.
On Mon, 2020-01-20 at 10:50 +0100, David Miller wrote: > From: Moshe Shemesh <moshe@mellanox.com> > Date: Sun, 19 Jan 2020 17:04:28 +0200 > > > Devlink health recover notifications were added only on driver > direct > > updates of health_state through > devlink_health_reporter_state_update(). > > Add notifications on updates of health_state by devlink flows of > report > > and recover. > > > > Fixes: 97ff3bd37fac ("devlink: add devink notification when > reporter update health state") > > Signed-off-by: Moshe Shemesh <moshe@mellanox.com> > > Acked-by: Jiri Pirko <jiri@mellanox.com> > > I really dislike forward declarations and almost all of the time they > are > unnecessary. > Hi Dave, a question just for educational purposes, i agree regarding function forward declarations, bottom-up organizing should always be followed. But how about type forward declaration ? for hiding struct details in specifc c file.. is it a bad practice ? I use this trick a lot in mlx5, for detail hiding and module separation .. > Could you please just rearrange the code as needed and resubmit? > > Thank you.
diff --git a/net/core/devlink.c b/net/core/devlink.c index b41b2e3..99f2057 100644 --- a/net/core/devlink.c +++ b/net/core/devlink.c @@ -4851,6 +4851,9 @@ struct devlink_health_reporter * } EXPORT_SYMBOL_GPL(devlink_health_reporter_recovery_done); +static void devlink_recover_notify(struct devlink_health_reporter *reporter, + enum devlink_command cmd); + static int devlink_health_reporter_recover(struct devlink_health_reporter *reporter, void *priv_ctx, struct netlink_ext_ack *extack) @@ -4869,6 +4872,7 @@ struct devlink_health_reporter * devlink_health_reporter_recovery_done(reporter); reporter->health_state = DEVLINK_HEALTH_REPORTER_STATE_HEALTHY; + devlink_recover_notify(reporter, DEVLINK_CMD_HEALTH_REPORTER_RECOVER); return 0; } @@ -4935,6 +4939,7 @@ int devlink_health_report(struct devlink_health_reporter *reporter, reporter->error_count++; prev_health_state = reporter->health_state; reporter->health_state = DEVLINK_HEALTH_REPORTER_STATE_ERROR; + devlink_recover_notify(reporter, DEVLINK_CMD_HEALTH_REPORTER_RECOVER); /* abort if the previous error wasn't recovered */ if (reporter->auto_recover &&