Re: [PATCH] Differentiate scenarios when watchdog is closed

[Guenter Roeck <linux@roeck-us.net>]

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