[PATCH] Differentiate scenarios when watchdog is closed

[PATCH] Differentiate scenarios when watchdog is closed

[Charles Haithcock]

Presenty, when a watchdog device is closed, we print "watchdog did not
stop" in a few different scenarios;

1. When nowayout is set
2. When the watchdog is able to close, has received the magic character
   to stop, but fails to close in device-specific code paths
3. When userspace delierately closes it without stopping it

For 1, we explicitly print we can not close because of nowayout. Nothing
differentiates the other two however.

This change adds a print to indicate the watchdog was closed while still
running.

Signed-off-by: Charles Haithcock <chaithco@redhat.com>
---

 drivers/watchdog/watchdog_dev.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
index d7895009a2..a571dea353 100644
--- a/drivers/watchdog/watchdog_dev.c
+++ b/drivers/watchdog/watchdog_dev.c
@@ -955,14 +955,17 @@ static int watchdog_release(struct inode *inode, struct file *file)
 	if (!watchdog_active(wdd))
 		err = 0;
 	else if (test_and_clear_bit(_WDOG_ALLOW_RELEASE, &wd_data->status) ||
-		 !(wdd->info->options & WDIOF_MAGICCLOSE))
+		 !(wdd->info->options & WDIOF_MAGICCLOSE)) {
 		err = watchdog_stop(wdd);
 
-	/* If the watchdog was not stopped, send a keepalive ping */
-	if (err < 0) {
-		pr_crit("watchdog%d: watchdog did not stop!\n", wdd->id);
-		watchdog_ping(wdd);
+		/* If the watchdog was not stopped, send a keepalive ping */
+		if (err < 0) {
+			pr_crit("watchdog%d: watchdog did not stop!\n", wdd->id);
+			watchdog_ping(wdd);
+		}
 	}
+	else
+		pr_info("watchdog%d: closing while running!\n", wdd->id);
 
 	watchdog_update_worker(wdd);
 
-- 
2.55.0

Re: [PATCH] Differentiate scenarios when watchdog is closed

[Guenter Roeck]

On 8/24/26 13:50, Charles Haithcock wrote:
> Presenty, when a watchdog device is closed, we print "watchdog did not

Presently

Also, the subject should start with the subsystem name ("watchdog:")

> stop" in a few different scenarios;
> 
> 1. When nowayout is set
> 2. When the watchdog is able to close, has received the magic character
>     to stop, but fails to close in device-specific code paths
> 3. When userspace delierately closes it without stopping it

deliberately

> 
> For 1, we explicitly print we can not close because of nowayout. Nothing
> differentiates the other two however.
> 
> This change adds a print to indicate the watchdog was closed while still
> running.
> 
> Signed-off-by: Charles Haithcock <chaithco@redhat.com>
> ---
> 
>   drivers/watchdog/watchdog_dev.c | 13 ++++++++-----
>   1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/watchdog/watchdog_dev.c b/drivers/watchdog/watchdog_dev.c
> index d7895009a2..a571dea353 100644
> --- a/drivers/watchdog/watchdog_dev.c
> +++ b/drivers/watchdog/watchdog_dev.c
> @@ -955,14 +955,17 @@ static int watchdog_release(struct inode *inode, struct file *file)
>   	if (!watchdog_active(wdd))
>   		err = 0;
>   	else if (test_and_clear_bit(_WDOG_ALLOW_RELEASE, &wd_data->status) ||
> -		 !(wdd->info->options & WDIOF_MAGICCLOSE))
> +		 !(wdd->info->options & WDIOF_MAGICCLOSE)) {
>   		err = watchdog_stop(wdd);
>   
> -	/* If the watchdog was not stopped, send a keepalive ping */
> -	if (err < 0) {
> -		pr_crit("watchdog%d: watchdog did not stop!\n", wdd->id);
> -		watchdog_ping(wdd);
> +		/* If the watchdog was not stopped, send a keepalive ping */
> +		if (err < 0) {
> +			pr_crit("watchdog%d: watchdog did not stop!\n", wdd->id);
> +			watchdog_ping(wdd);
> +		}
>   	}
> +	else
> +		pr_info("watchdog%d: closing while running!\n", wdd->id);

As Sashiko points out, this changes behavior if the watchdog is active
and was not stopped. Also, I personally find "closing while running"
not very informative. Also, while technically userspace may close the
watchdog deliberately while it is running, that is not what happens
on a regular basis. I find the previous unconditional "watchdog did
not stop" message more informative and relevant.

If you want to make a change, I would suggest to add an error message
into watchdog_stop() to report an error if the stop callback returns
an error. That would distinguish 2/3 without making functional changes.

Thanks,
Guenter

Re: [PATCH] Differentiate scenarios when watchdog is closed

[chaithco]

On Thu, Aug 27 2026 at 09:01:47 AM -07:00:00, Guenter Roeck 
<linux@roeck-us.net> wrote:
> Presently
> [...]
> Also, the subject should start with the subsystem name ("watchdog:")
> [...]
> deliberately

Thank you for catching these! Please accept my apologies. I can fix 
those up in the next submission.

> [...] Also, while technically userspace may close the
> watchdog deliberately while it is running, that is not what happens
> on a regular basis.

This is actually what initiated a bug report at 
https://bugzilla.redhat.com/show_bug.cgi?id=1991285 it turns out 
systemd explicitly does this to help ensure a system shutting down 
actually eventually goes down even if the shutdown process hits some 
snags. It does this on every shutdown. Given the prevalence of systemd, 
this is a regular occurrence. The end result is that, when using iTCO, 
it shows an error on every shutdown when systemd is in use as init.

> If you want to make a change, I would suggest to add an error message
> into watchdog_stop() to report an error if the stop callback returns
> an error. That would distinguish 2/3 without making functional 
> changes.

Thank you! So something like this?

        if (wdd->ops->stop) {
                clear_bit(WDOG_HW_RUNNING, &wdd->status);
                err = wdd->ops->stop(wdd);
+               if (err < 0)
+                       pr_info("watchdog%d: closed while still 
enabled!\n");
                trace_watchdog_stop(wdd, err);
        } else {
                set_bit(WDOG_HW_RUNNING, &wdd->status);


While responding to this, an additional thought occurred to me; given 
the primary reason a user would see this is because systemd is shutting 
down a system, it may be more worth while to have systemd log something 
about closing the watchdog without disarming it to at least explain a 
pr_crit kernel log line. Otherwise, it just looks like "something bad 
happened" with watchdog. I am additionally unsure of what would be best 
to go in watchdog_stop that helps differentiate intentional closing of 
the watchdog without disabling vs malicious/accidental closing. The 
intent would lie within the entity closing the watchdog; "closed while 
still enabled!" still seems like "something bad happened" with info on 
if it was intentional or not.

Thank you!
- Charles

Re: [PATCH] Differentiate scenarios when watchdog is closed

[Guenter Roeck]

On 8/27/26 10:09, chaithco@redhat.com wrote:
> 
> 
> On Thu, Aug 27 2026 at 09:01:47 AM -07:00:00, Guenter Roeck <linux@roeck-us.net> wrote:
>> Presently
>> [...]
>> Also, the subject should start with the subsystem name ("watchdog:")
>> [...]
>> deliberately
> 
> Thank you for catching these! Please accept my apologies. I can fix those up in the next submission.
> 
>> [...] Also, while technically userspace may close the
>> watchdog deliberately while it is running, that is not what happens
>> on a regular basis.
> 
> This is actually what initiated a bug report at https://bugzilla.redhat.com/show_bug.cgi?id=1991285 it turns out systemd explicitly does this to help ensure a system shutting down actually eventually goes down even if the shutdown process hits some snags. It does this on every shutdown. Given the prevalence of systemd, this is a regular occurrence. The end result is that, when using iTCO, it shows an error on every shutdown when systemd is in use as init.
> 
>> If you want to make a change, I would suggest to add an error message
>> into watchdog_stop() to report an error if the stop callback returns
>> an error. That would distinguish 2/3 without making functional changes.
> 
> Thank you! So something like this?
> 
>         if (wdd->ops->stop) {
>                 clear_bit(WDOG_HW_RUNNING, &wdd->status);
>                 err = wdd->ops->stop(wdd);
> +               if (err < 0)
> +                       pr_info("watchdog%d: closed while still enabled!\n");

More like

			pr_err(""watchdog%d: Failed to stop watchdog: %pe\n", wdd->id, ERR_PTR(err));

since this would be a real error.

The "watchdog%d: watchdog did not stop!" message will then follow
(unconditionally).

>                 trace_watchdog_stop(wdd, err);
>         } else {
>                 set_bit(WDOG_HW_RUNNING, &wdd->status);
> 
> 
> While responding to this, an additional thought occurred to me; given the primary reason a user would see this is because systemd is shutting down a system, it may be more worth while to have systemd log something about closing the watchdog without disarming it to at least explain a pr_crit kernel log line. Otherwise, it just looks like "something bad happened" with watchdog. I am additionally unsure of what would be best to go in watchdog_stop that helps differentiate intentional closing of the watchdog without disabling vs malicious/accidental closing. The intent would lie within the entity closing the watchdog; "closed while still enabled!" still seems like "something bad happened" with info on if it was intentional or not.
> 

Problem is that we don't know if "something bad happened". The same message
will be seen if the watchdog daemon was killed or crashed. We can not just
assume that closing the watchdog device was intentional.

Guenter