Re: [PATCH v2 2/3] PM: dpm_watchdog: Allow disabling DPM watchdog by default

[Randy Dunlap <rdunlap@infradead.org>]

Hi--

On 6/4/26 2:07 AM, Tzung-Bi Shih wrote:
> Introduce the CONFIG_DPM_WATCHDOG_DEFAULT_ENABLED Kconfig option to
> allow the device suspend/resume watchdog (DPM watchdog) to be disabled
> by default at compile time.
> 
> Additionally, introduce the "dpm_watchdog_enabled" module parameter to
> allow the watchdog to be enabled or disabled at boot time (via
> "power.dpm_watchdog_enabled") and at runtime (via sysfs).
> 
> This provides flexibility for systems that want the watchdog code
> compiled in but inactive by default, allowing it to be enabled only when
> needed.
> 
> Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
> ---
> v2:
> - Use module parameter and bool for dpm_watchdog_enabled.
> - Use IS_ENABLED().
> 
> v1: https://lore.kernel.org/all/20260528103215.505795-1-tzungbi@kernel.org
> 
>  Documentation/admin-guide/kernel-parameters.txt |  8 ++++++++
>  drivers/base/power/main.c                       | 11 +++++++++++
>  kernel/power/Kconfig                            |  9 +++++++++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 00375193bd26..0a0d5340b0c7 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -5399,6 +5399,14 @@ Kernel parameters
>  			function to NULL. On Idle the CPU just reduces
>  			execution priority.
>  
> +	power.dpm_watchdog_enabled=
> +			[KNL] Enable or disable the device suspend/resume

I think that [KNL] isn't very useful here (nor in many of its uses in
kernel-parameters.txt).
What is required to use this option are:
  CONFIG_PM_SLEEP, CONFIG_DPM_WATCHDOG
You should convey that information somehow.

Also, in kernel-parameters.txt, "pm_async=" is only valid when [PM]
is enabled, but "PM" is not defined/described anywhere.
That should be added near the beginning of kernel-parameters.txt (in
alphabetical order).


> +			watchdog (DPM watchdog).
> +			Format: {"0" | "1"}
> +			0: disable
> +			1: enable
> +			Default value is set by CONFIG_DPM_WATCHDOG_DEFAULT_ENABLED.
> +
>  	ppc_strict_facility_enable
>  			[PPC,ENABLE] This option catches any kernel floating point,
>  			Altivec, VSX and SPE outside of regions specifically
> diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> index cd864f3a2799..7822c29b7c8d 100644
> --- a/drivers/base/power/main.c
> +++ b/drivers/base/power/main.c
> @@ -534,6 +534,11 @@ module_param(dpm_watchdog_all_cpu_backtrace, bool, 0644);
>  MODULE_PARM_DESC(dpm_watchdog_all_cpu_backtrace,
>  		 "Backtrace all CPUs on DPM watchdog timeout");
>  
> +static bool __read_mostly dpm_watchdog_enabled =
> +				IS_ENABLED(CONFIG_DPM_WATCHDOG_DEFAULT_ENABLED);
> +module_param(dpm_watchdog_enabled, bool, 0644);
> +MODULE_PARM_DESC(dpm_watchdog_enabled, "Enable DPM watchdog");
> +
>  /**
>   * dpm_watchdog_handler - Driver suspend / resume watchdog handler.
>   * @t: The timer that PM watchdog depends on.
> @@ -577,6 +582,9 @@ static void dpm_watchdog_set(struct dpm_watchdog *wd, struct device *dev)
>  {
>  	struct timer_list *timer = &wd->timer;
>  
> +	if (!dpm_watchdog_enabled)
> +		return;
> +
>  	wd->dev = dev;
>  	wd->tsk = current;
>  	wd->fatal = CONFIG_DPM_WATCHDOG_TIMEOUT == CONFIG_DPM_WATCHDOG_WARNING_TIMEOUT;
> @@ -595,6 +603,9 @@ static void dpm_watchdog_clear(struct dpm_watchdog *wd)
>  {
>  	struct timer_list *timer = &wd->timer;
>  
> +	if (!dpm_watchdog_enabled)
> +		return;
> +
>  	timer_delete_sync(timer);
>  	timer_destroy_on_stack(timer);
>  }
> diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> index 530c897311d4..12a4a66d48d4 100644
> --- a/kernel/power/Kconfig
> +++ b/kernel/power/Kconfig
> @@ -268,6 +268,15 @@ config DPM_WATCHDOG
>  	  captured in pstore device for inspection in subsequent
>  	  boot session.
>  
> +config DPM_WATCHDOG_DEFAULT_ENABLED
> +	bool "Enable DPM watchdog by default"
> +	depends on DPM_WATCHDOG
> +	default y
> +	help
> +	  If you say Y here, the DPM watchdog will be enabled by default.
> +	  If you say N, it will be compiled in but disabled, requiring a
> +	  boot parameter to activate.
> +
>  config DPM_WATCHDOG_TIMEOUT
>  	int "Watchdog timeout to panic in seconds"
>  	range 1 120

-- 
~Randy