diff mbox series

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

Message ID 20200620174907.20229-2-minyard@acm.org
State New
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)) {