diff mbox series

[01/10] watchdog: Ignore stop_on_reboot if no stop function

Message ID 20200620174907.20229-2-minyard@acm.org (mailing list archive)
State Changes Requested
Headers show
Series Convert the IPMI watchdog to use the watchdog | expand

Commit Message

Corey Minyard June 20, 2020, 5:48 p.m. UTC 
From: Corey Minyard <cminyard@mvista.com>

The reboot notifier unconditionally calls the stop function on the
watchdog, which would result in a crash if the watchdog didn't have a
stop function.  So check at register time to see if there is a stop
function, and don't do stop_on_reboot if it is NULL.

Signed-off-by: Corey Minyard <cminyard@mvista.com>
---
 drivers/watchdog/watchdog_core.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Comments

Guenter Roeck July 19, 2020, 2:11 p.m. UTC | #1 
On Sat, Jun 20, 2020 at 12:48:58PM -0500, minyard@acm.org wrote:
> From: Corey Minyard <cminyard@mvista.com>
> 
> The reboot notifier unconditionally calls the stop function on the
> watchdog, which would result in a crash if the watchdog didn't have a
> stop function.  So check at register time to see if there is a stop
> function, and don't do stop_on_reboot if it is NULL.
> 
> Signed-off-by: Corey Minyard <cminyard@mvista.com>
> ---
>  drivers/watchdog/watchdog_core.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
> index 423844757812..03943a34e9fb 100644
> --- a/drivers/watchdog/watchdog_core.c
> +++ b/drivers/watchdog/watchdog_core.c
> @@ -260,10 +260,16 @@ static int __watchdog_register_device(struct watchdog_device *wdd)
>  
>  	/* Module parameter to force watchdog policy on reboot. */
>  	if (stop_on_reboot != -1) {
> -		if (stop_on_reboot)
> -			set_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
> -		else
> +		if (stop_on_reboot) {
> +			if (!wdd->ops->stop) {
> +				pr_err("watchdog%d: stop_on_reboot set, but no stop function.  Ignoring stop_on_reboot.\n", wdd->id);

This should be pr_notice(). It is not an error (otherwise I would expect
registration to abort). Also:

WARNING: line length of 133 exceeds 100 columns
#108: FILE: drivers/watchdog/watchdog_core.c:265:
+				pr_err("watchdog%d: stop_on_reboot set, but no stop function.  Ignoring stop_on_reboot.\n", wdd->id);


This patch that is independent from the rest of the series and should be
applied/handled independently.

Thanks,
Guenter

> +				clear_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
> +			} else {
> +				set_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
> +			}
> +		} else {
>  			clear_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
> +		}
>  	}
>  
>  	if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status)) {
diff mbox series

Patch

diff --git a/drivers/watchdog/watchdog_core.c b/drivers/watchdog/watchdog_core.c
index 423844757812..03943a34e9fb 100644
--- a/drivers/watchdog/watchdog_core.c
+++ b/drivers/watchdog/watchdog_core.c
@@ -260,10 +260,16 @@  static int __watchdog_register_device(struct watchdog_device *wdd)
 
 	/* Module parameter to force watchdog policy on reboot. */
 	if (stop_on_reboot != -1) {
-		if (stop_on_reboot)
-			set_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
-		else
+		if (stop_on_reboot) {
+			if (!wdd->ops->stop) {
+				pr_err("watchdog%d: stop_on_reboot set, but no stop function.  Ignoring stop_on_reboot.\n", wdd->id);
+				clear_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
+			} else {
+				set_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
+			}
+		} else {
 			clear_bit(WDOG_STOP_ON_REBOOT, &wdd->status);
+		}
 	}
 
 	if (test_bit(WDOG_STOP_ON_REBOOT, &wdd->status)) {