On Sat, Jun 20, 2020 at 12:49:05PM -0500, minyard@xxxxxxx wrote:
> From: Corey Minyard <cminyard@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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)
