| Message ID | 20200620174907.20229-9-minyard@acm.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Convert the IPMI watchdog to use the watchdog | expand |
On Sat, Jun 20, 2020 at 12:49:05PM -0500, minyard@acm.org wrote: > From: Corey Minyard <cminyard@mvista.com> > > If reboot_timeout is set in the watchdog device struct, set the timeout > to that value on a reboot. This way more time can be given for a reboot > to complete. > I think this should be aligned with watchdog_set_open_deadline(), ie use a function to set the reboot timeout instead of adding it to struct watchdog_device. > Signed-off-by: Corey Minyard <cminyard@mvista.com> > --- > drivers/watchdog/watchdog_core.c | 2 ++ > include/linux/watchdog.h | 4 ++++ > 2 files changed, 6 insertions(+) > > diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c > index 03943a34e9fb..5792f9bca645 100644 > --- a/drivers/watchdog/watchdog_core.c > +++ b/drivers/watchdog/watchdog_core.c > @@ -165,6 +165,8 @@ static int watchdog_reboot_notifier(struct notifier_block *nb, > if (ret) > return NOTIFY_BAD; > } > + } else if (wdd->reboot_timeout) { > + watchdog_set_timeout(wdd, wdd->reboot_timeout); This has no practical impact; the code above checks for SYS_DOWN, and for whatever reason SYS_DOWN has the same value as SYS_RESTART. So the only value is for SYS_POWER_OFF, and that should arguably be included in the first check (meaning we should probably remove that check entirely, if anything). Also, the reboot notifier is only called if WDOG_STOP_ON_REBOOT is set, which contradicts the idea behind this change. Guenter > } > > return NOTIFY_DONE; > diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h > index 1eefae61215d..0fb57f29346c 100644 > --- a/include/linux/watchdog.h > +++ b/include/linux/watchdog.h > @@ -92,6 +92,9 @@ struct watchdog_ops { > * @status: Field that contains the devices internal status bits. > * @deferred: Entry in wtd_deferred_reg_list which is used to > * register early initialized watchdogs. > + * @reboot_timeout: > + * If non-zero, the timeout will be set to this value > + * on a reboot. This lets a reboot be given more time. > * > * The watchdog_device structure contains all information about a > * watchdog timer device. > @@ -125,6 +128,7 @@ struct watchdog_device { > #define WDOG_HW_RUNNING 3 /* True if HW watchdog running */ > #define WDOG_STOP_ON_UNREGISTER 4 /* Should be stopped on unregister */ > struct list_head deferred; > + unsigned int reboot_timeout; > }; > > #define WATCHDOG_NOWAYOUT IS_BUILTIN(CONFIG_WATCHDOG_NOWAYOUT)
diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c index 03943a34e9fb..5792f9bca645 100644 --- a/drivers/watchdog/watchdog_core.c +++ b/drivers/watchdog/watchdog_core.c @@ -165,6 +165,8 @@ static int watchdog_reboot_notifier(struct notifier_block *nb, if (ret) return NOTIFY_BAD; } + } else if (wdd->reboot_timeout) { + watchdog_set_timeout(wdd, wdd->reboot_timeout); } return NOTIFY_DONE; diff --git a/include/linux/watchdog.h b/include/linux/watchdog.h index 1eefae61215d..0fb57f29346c 100644 --- a/include/linux/watchdog.h +++ b/include/linux/watchdog.h @@ -92,6 +92,9 @@ struct watchdog_ops { * @status: Field that contains the devices internal status bits. * @deferred: Entry in wtd_deferred_reg_list which is used to * register early initialized watchdogs. + * @reboot_timeout: + * If non-zero, the timeout will be set to this value + * on a reboot. This lets a reboot be given more time. * * The watchdog_device structure contains all information about a * watchdog timer device. @@ -125,6 +128,7 @@ struct watchdog_device { #define WDOG_HW_RUNNING 3 /* True if HW watchdog running */ #define WDOG_STOP_ON_UNREGISTER 4 /* Should be stopped on unregister */ struct list_head deferred; + unsigned int reboot_timeout; }; #define WATCHDOG_NOWAYOUT IS_BUILTIN(CONFIG_WATCHDOG_NOWAYOUT)