Re: [PATCH 4/8] watchdog: core: Stop pretimeout hrtimer on suspend

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] watchdog: missed software pretimeout on resume due to timer desynchronization
--

commit b401093697a53f53413364d61f3bff8840d84ffc
Author: Guenter Roeck <linux@roeck-us.net>

watchdog: core: Stop pretimeout hrtimer on suspend

This commit stops the pretimeout hrtimer during system suspend and ensures
it is correctly restarted on resume if the hardware watchdog is running. It
also prevents the timer from starting while the watchdog is suspended.

> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index d652d6e137c43..f1d14ac433fd9 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c

[ ... ]

> @@ -1350,6 +1351,8 @@ int watchdog_dev_resume(struct watchdog_device *wdd)
>  	clear_bit(_WDOG_SUSPENDED, &wd_data->status);
>  	if (watchdog_worker_should_ping(wd_data))
>  		ret = __watchdog_ping(wd_data->wdd);
> +	if (watchdog_hw_running(wdd) && !ret)
> +		watchdog_hrtimer_pretimeout_start(wdd);

[Severity: High]
Could this explicit restart push the software pretimeout out of sync with the
hardware counter?

If the watchdog is past the open deadline, watchdog_worker_should_ping()
returns false. The kernel intentionally stops pinging the hardware watchdog
so it will reset the system.

When suspended and resumed in this state, __watchdog_ping() is skipped, but
watchdog_hw_running() remains true. Since ret is 0, we restart the pretimeout
timer.

Does this assume the hardware counter was just reset to now, artificially
delaying the software pretimeout beyond the actual hardware reset?

>  	mutex_unlock(&wd_data->lock);
>  
>  	return ret;

[ ... ]

> diff --git a/drivers/watchdog/watchdog_hrtimer_pretimeout.c b/drivers/watchdog/watchdog_hrtimer_pretimeout.c
> index 49a05ea60c979..5c88e6467994b 100644
> --- a/drivers/watchdog/watchdog_hrtimer_pretimeout.c
> +++ b/drivers/watchdog/watchdog_hrtimer_pretimeout.c
> @@ -31,7 +31,8 @@ void watchdog_hrtimer_pretimeout_start(struct watchdog_device *wdd)
>  {
>  	if (!(wdd->info->options & WDIOF_PRETIMEOUT) &&
>  	    wdd->pretimeout &&
> -	    !watchdog_pretimeout_invalid(wdd, wdd->pretimeout))
> +	    !watchdog_pretimeout_invalid(wdd, wdd->pretimeout) &&
> +	    !test_bit(_WDOG_SUSPENDED, &wdd->wd_data->status))
>  		hrtimer_start(&wdd->wd_data->pretimeout_timer,
>  			      ktime_set(wdd->timeout - wdd->pretimeout, 0),
>  			      HRTIMER_MODE_REL);

[Severity: High]
Since hrtimer_start() here uses HRTIMER_MODE_REL, the timer is always
scheduled strictly relative to now.

If the hardware watchdog was last pinged before the suspend cycle, it will
reboot the system at last_hw_keepalive + timeout.

If the elapsed time (which includes the suspend duration) exceeds pretimeout,
will this relative timer be scheduled to fire after the hardware reset occurs,
causing the pretimeout event to be completely missed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929134635.2567137-1-linux@roeck-us.net?part=4