Re: [PATCH v3 3/3] PM: dpm_watchdog: Add sysctl interface for DPM watchdog timeouts

[Tzung-Bi Shih]

On Mon, Jun 08, 2026 at 04:22:35PM +0200, Rafael J. Wysocki wrote:
> On Mon, Jun 8, 2026 at 4:16 AM Tzung-Bi Shih <tzungbi@xxxxxxxxxx> wrote:
> >
> > Introduce sysctl knobs to allow configuring DPM watchdog timeouts at
> > runtime.
> >
> > Currently, these timeouts are fixed at compile time via
> > CONFIG_DPM_WATCHDOG_TIMEOUT and CONFIG_DPM_WATCHDOG_WARNING_TIMEOUT.
> > This limits flexibility if the timeouts need to be adjusted for
> > different testing scenarios or hardware behaviors without rebuilding
> > the kernel.
> >
> > Add the following sysctl files under /proc/sys/kernel/:
> > - dpm_watchdog_timeout_secs: The total timeout before panic. The
> >   maximum value is capped at CONFIG_DPM_WATCHDOG_TIMEOUT to prevent
> >   unreasonably large timeouts.
> > - dpm_watchdog_warning_timeout_secs: The warning timeout. The maximum
> >   value is capped at the current dpm_watchdog_timeout_secs.
> > Both sysctls have a minimum value of 1.
> >
> > Signed-off-by: Tzung-Bi Shih <tzungbi@xxxxxxxxxx>
> > ---
> > v3:
> > - No changes.
> >
> > v2: https://lore.kernel.org/all/20260604090756.2884671-4-tzungbi@xxxxxxxxxx
> > - New to the series.
> >
> > v1: Doesn't exist.
> >
> >  drivers/base/power/main.c | 61 ++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 57 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
> > index 7822c29b7c8d..c1a4b30fafb2 100644
> > --- a/drivers/base/power/main.c
> > +++ b/drivers/base/power/main.c
> > @@ -28,6 +28,7 @@
> >  #include <linux/interrupt.h>
> >  #include <linux/sched.h>
> >  #include <linux/sched/debug.h>
> > +#include <linux/sysctl.h>
> >  #include <linux/async.h>
> >  #include <linux/suspend.h>
> >  #include <trace/events/power.h>
> > @@ -539,6 +540,58 @@ static bool __read_mostly dpm_watchdog_enabled =
> >  module_param(dpm_watchdog_enabled, bool, 0644);
> >  MODULE_PARM_DESC(dpm_watchdog_enabled, "Enable DPM watchdog");
> >
> > +static unsigned int __read_mostly dpm_watchdog_timeout = CONFIG_DPM_WATCHDOG_TIMEOUT;
> > +static unsigned int __read_mostly dpm_watchdog_warning_timeout =
> > +                                               CONFIG_DPM_WATCHDOG_WARNING_TIMEOUT;
> > +static const unsigned int dpm_watchdog_timeout_max = CONFIG_DPM_WATCHDOG_TIMEOUT;
> > +
> > +static int proc_dodpm_watchdog_timeout_secs(const struct ctl_table *table,
> > +                                           int write, void *buffer,
> > +                                           size_t *lenp, loff_t *ppos)
> > +{
> > +       struct ctl_table ctl = *table;
> > +       unsigned int val = dpm_watchdog_timeout;
> > +       int ret;
> > +
> > +       ctl.data = &val;
> > +       ret = proc_douintvec_minmax(&ctl, write, buffer, lenp, ppos);
> > +       if (ret || !write)
> > +               return ret;
> > +
> > +       if (val < dpm_watchdog_warning_timeout)
> > +               dpm_watchdog_warning_timeout = val;
> > +       dpm_watchdog_timeout = val;
> > +
> > +       return 0;
> > +}
> > +
> > +static const struct ctl_table dpm_watchdog_sysctls[] = {
> > +       {
> > +               .procname       = "dpm_watchdog_timeout_secs",
> > +               .maxlen         = sizeof(unsigned int),
> > +               .mode           = 0644,
> > +               .proc_handler   = proc_dodpm_watchdog_timeout_secs,
> > +               .extra1         = SYSCTL_ONE,
> > +               .extra2         = (void *)&dpm_watchdog_timeout_max,
> > +       },
> > +       {
> > +               .procname       = "dpm_watchdog_warning_timeout_secs",
> > +               .data           = &dpm_watchdog_warning_timeout,
> > +               .maxlen         = sizeof(unsigned int),
> > +               .mode           = 0644,
> > +               .proc_handler   = proc_douintvec_minmax,
> > +               .extra1         = SYSCTL_ONE,
> > +               .extra2         = (void *)&dpm_watchdog_timeout,
> > +       },
> > +};
> > +
> > +static int __init dpm_watchdog_sysctl_init(void)
> > +{
> > +       register_sysctl_init("kernel", dpm_watchdog_sysctls);
> > +       return 0;
> > +}
> > +subsys_initcall(dpm_watchdog_sysctl_init);
> > +
> >  /**
> >   * dpm_watchdog_handler - Driver suspend / resume watchdog handler.
> >   * @t: The timer that PM watchdog depends on.
> > @@ -564,9 +617,9 @@ static void dpm_watchdog_handler(struct timer_list *t)
> >                         dev_driver_string(wd->dev), dev_name(wd->dev));
> >         }
> >
> > -       time_left = CONFIG_DPM_WATCHDOG_TIMEOUT - CONFIG_DPM_WATCHDOG_WARNING_TIMEOUT;
> > +       time_left = dpm_watchdog_timeout - dpm_watchdog_warning_timeout;
> >         dev_warn(wd->dev, "**** DPM device timeout after %u seconds; %u seconds until panic ****\n",
> > -                CONFIG_DPM_WATCHDOG_WARNING_TIMEOUT, time_left);
> > +                dpm_watchdog_warning_timeout, time_left);
> >         show_stack(wd->tsk, NULL, KERN_WARNING);
> >
> >         wd->fatal = true;
> > @@ -587,11 +640,11 @@ static void dpm_watchdog_set(struct dpm_watchdog *wd, struct device *dev)
> >
> >         wd->dev = dev;
> >         wd->tsk = current;
> > -       wd->fatal = CONFIG_DPM_WATCHDOG_TIMEOUT == CONFIG_DPM_WATCHDOG_WARNING_TIMEOUT;
> > +       wd->fatal = dpm_watchdog_timeout == dpm_watchdog_warning_timeout;
> >
> >         timer_setup_on_stack(timer, dpm_watchdog_handler, 0);
> >         /* use same timeout value for both suspend and resume */
> > -       timer->expires = jiffies + HZ * CONFIG_DPM_WATCHDOG_WARNING_TIMEOUT;
> > +       timer->expires = jiffies + HZ * dpm_watchdog_warning_timeout;
> >         add_timer(timer);
> >  }
> >
> > --
> 
> I think that this can be applied without the other two patches in the
> series, so please let me know if you want me to apply it separately.

Ack.  The patch does have adjacent hunks with the preceding patch, which
might cause minor contextual conflicts if applied independently.

Would you want me to reorder the series in the next version to make this
the first patch?  In case the rest two patches may still take some time
to review.