Re: [PATCH] watchdog: sbsa_gwdt: stop the watchdog across the whole system-sleep transition
From: Guenter Roeck
Date: Wed Sep 16 2026 - 20:20:32 EST
On Sat, Sep 12, 2026 at 11:21:07AM -0700, David Cemin wrote:
> The driver stops a running watchdog in its own device suspend callback
> and restarts it in its resume callback. That leaves the watchdog armed,
> with nobody refreshing it, for the entire early part of suspend entry:
> userspace freeze, kernel thread freeze, and every device suspend
> callback that runs before this device's own. The same window exists at
> the tail end of resume.
>
> When the watchdog is already running when the driver binds (started by
> firmware, 10 s default timeout) and any device stalls its suspend
> callback past the timeout, the watchdog resets the system in the middle
> of suspend entry. On an arm64 laptop platform this fired on about 7% of
> suspend attempts in a randomized suspend stress run (9 resets in 124
> suspends, with the watchdog reset status set in the SoC's reset status
> register). Two elimination runs confirm the mechanism: the identical
> stress matrix with the watchdog stopped produced zero resets in 118
> suspends, and with the first version of this change (notifier plus the
> original device callbacks) applied, zero resets in 198 suspends across
> four runs, where the baseline rate predicts about 14. The version here
> keeps that mechanism, removes the device resume callback and adds the
> locking described below; it went through a further 120 suspends (60
> s2idle, 60 S3, randomized order, console recorded through every entry)
> with the watchdog armed from boot and zero resets.
>
> Stop the watchdog from a PM notifier at the *_PREPARE events, before
> tasks are frozen and device callbacks run, and restart it at the
> PM_POST_* events, after everything has resumed. The driver state (armed,
> stopped for sleep) lives under a lock shared with the watchdog ops, so a
> userspace stop or magic close after thaw cannot race the restart, and a
> start requested while the transition is in progress is deferred until
> PM_POST_* instead of arming hardware nobody can refresh; the transition
> is recorded at *_PREPARE whether or not the watchdog was armed at that
> point, so a start between *_PREPARE and task freezing is deferred as
> well. The notifier is
> registered before anything can arm the watchdog and its failure fails
> the probe. A suspend-only device callback remains as the final guard for
> a device whose probe overlapped the *_PREPARE event; it has no resume
> counterpart, so nothing re-arms the watchdog during device resume,
> before PM_POST_SUSPEND. The initial hardware state is adopted under the
> same lock, so a firmware-started watchdog discovered by a probe that
> lost the race with *_PREPARE is stopped at once and armed again at
> PM_POST_*; the stop path is idempotent so the device callback remains
> an effective fallback whatever the ordering.
>
> Fixes: 57d2caaabfc7 ("Watchdog: introduce ARM SBSA watchdog driver")
> Signed-off-by: David Cemin <dcemin@xxxxxxxxxx>
After thinking about it, I concluded that this is the wrong solution.
First of all, it is a solution in search of a problem: Simply increasing
the watchdog timeout to be safe would solve it. If anything, if the default
timeout of 10 seconds for this watchdog is not safe, increase it to a safe
level. However, a system requiring more than 10 seconds to suspend should
really set a sufficient watchdog timeout to prevent the described problem
from happening.
Second, the patch leaves the system unprotected during the suspend
operation, which is arguable one of the most critical operations in
the kernel. On top of that, it _is_ arguable a problem affecting all
watchdog drivers. If we'd want to do anything about it, it would have
to be in the watchdog core, and the solution should leave the watchdog
running but ping it from the kernel while the suspend operation is going
on. However, even that would require some timeout because we would not
want suspend operations to stall forever.
Thanks,
Guenter