Re: [PATCH] watchdog: softdog: make pretimeout support a compile option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jan 06, 2017 at 09:41:44AM +0100, Wolfram Sang wrote:
> It occured to me that the panic pretimeout governor will stall the
> softdog, because it is purely software which simply halts on panic.
> Testing governors with the softdog on the other hand is really useful.
> So, make this feature a compile time option which needs to be enabled
> explicitly. This also removes the overhead if pretimeout support is not
> used because it will now be compiled away (saving ~10% on ARM32).
> 
> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
> ---
>  drivers/watchdog/Kconfig   |  8 ++++++++
>  drivers/watchdog/softdog.c | 27 +++++++++++++++++++--------
>  2 files changed, 27 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index acb00b53a5207b..70726ce3d166e8 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -71,6 +71,14 @@ config SOFT_WATCHDOG
>  	  To compile this driver as a module, choose M here: the
>  	  module will be called softdog.
>  
> +config SOFT_WATCHDOG_PRETIMEOUT
> +	bool "Software watchdog pretimeout governor support"
> +	depends on SOFT_WATCHDOG && WATCHDOG_PRETIMEOUT_GOV
> +	help
> +	  Enable this if you want to use pretimeout governors with the software
> +	  watchdog. Be aware that governors might affect the watchdog because it
> +	  is purely software, e.g. the panic governor will stall it!
> +
>  config DA9052_WATCHDOG
>  	tristate "Dialog DA9052 Watchdog"
>  	depends on PMIC_DA9052
> diff --git a/drivers/watchdog/softdog.c b/drivers/watchdog/softdog.c
> index c7bdc986dca1c2..bc8b7da61d8aa7 100644
> --- a/drivers/watchdog/softdog.c
> +++ b/drivers/watchdog/softdog.c
> @@ -74,6 +74,7 @@ static struct timer_list softdog_ticktock =
>  
>  static struct watchdog_device softdog_dev;
>  
> +#if IS_ENABLED(CONFIG_SOFT_WATCHDOG_PRETIMEOUT)
>  static void softdog_pretimeout(unsigned long data)

I would prefer __maybe_unused here ..

>  {
>  	watchdog_notify_pretimeout(&softdog_dev);
> @@ -82,16 +83,23 @@ static void softdog_pretimeout(unsigned long data)
>  static struct timer_list softdog_preticktock =
>  		TIMER_INITIALIZER(softdog_pretimeout, 0, 0);
>  
> +static struct timer_list *softdog_preticktock_ptr = &softdog_preticktock;
> +#else
> +static void *softdog_preticktock_ptr = NULL;
> +#endif /* CONFIG_SOFT_WATCHDOG_PRETIMEOUT */
> +
>  static int softdog_ping(struct watchdog_device *w)
>  {
>  	if (!mod_timer(&softdog_ticktock, jiffies + (w->timeout * HZ)))
>  		__module_get(THIS_MODULE);
>  
> -	if (w->pretimeout)
> -		mod_timer(&softdog_preticktock, jiffies +
> -			  (w->timeout - w->pretimeout) * HZ);
> -	else
> -		del_timer(&softdog_preticktock);
> +	if (softdog_preticktock_ptr) {

and "if (IS_ENABLED(CONFIG_SOFT_WATCHDOG_PRETIMEOUT))" here.

> +		if (w->pretimeout)
> +			mod_timer(softdog_preticktock_ptr, jiffies +
> +				  (w->timeout - w->pretimeout) * HZ);
> +		else
> +			del_timer(softdog_preticktock_ptr);
> +	}
>  
>  	return 0;
>  }
> @@ -101,15 +109,15 @@ static int softdog_stop(struct watchdog_device *w)
>  	if (del_timer(&softdog_ticktock))
>  		module_put(THIS_MODULE);
>  
> -	del_timer(&softdog_preticktock);
> +	if (softdog_preticktock_ptr)

Is this conditional needed (assuming we get rid of softdog_preticktock_ptr) ?
Ok though if you want to use it to drop the code if not needed.

> +		del_timer(softdog_preticktock_ptr);
>  
>  	return 0;
>  }
>  
>  static struct watchdog_info softdog_info = {
>  	.identity = "Software Watchdog",
> -	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE |
> -		   WDIOF_PRETIMEOUT,
> +	.options = WDIOF_SETTIMEOUT | WDIOF_KEEPALIVEPING | WDIOF_MAGICCLOSE,
>  };
>  
>  static const struct watchdog_ops softdog_ops = {
> @@ -134,6 +142,9 @@ static int __init softdog_init(void)
>  	watchdog_set_nowayout(&softdog_dev, nowayout);
>  	watchdog_stop_on_reboot(&softdog_dev);
>  
> +	if (softdog_preticktock_ptr)
> +		softdog_info.options |= WDIOF_PRETIMEOUT;
> +
>  	ret = watchdog_register_device(&softdog_dev);
>  	if (ret)
>  		return ret;
> -- 
> 2.11.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux