[<prev] [next>] [day] [month] [year] [list]
Message-ID: <000000000000cfe69e05b1c9cf0e@google.com>
Date: Fri, 16 Oct 2020 13:33:36 +0000
From: <josephjang@...gle.com>
To: rafael@...nel.org, gregkh@...uxfoundation.org, rjw@...ysocki.net,
pavel@....cz, len.brown@...el.com, pmladek@...e.com,
sergey.senozhatsky@...il.com, rostedt@...dmis.org
Cc: linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org,
jonglin@...gle.com, woodylin@...gle.com, markcheng@...gle.com,
josephjang@...gle.com
Subject: [PATCH] power: suspend: Add suspend timeout handler
> >
> > On Fri, Oct 16, 2020 at 3:22 PM <josephjang@...gle.com> wrote:
> > >
> > > Thank you Rafael's promptly response.
> > >
> > > > On Fri, Oct 16, 2020 at 5:51 AM Joseph Jang <josephjang@...gle.com>
> wrote:
> > > > >
> > > > > From: josephjang <josephjang@...gle.com>
> > > > >
> > > > > Add suspend timeout handler to prevent device stuck during
> suspend/
> > > > > resume process. Suspend timeout handler will dump disk sleep task
> > > > > at first round timeout and trigger kernel panic at second round
> timeout.
> > > > > The default timer for each round is 30 seconds.
> > > > >
> > > > > Note: Can use following command to simulate suspend hang for
> testing.
> > > > > adb shell echo 1 > /sys/power/pm_hang
> > > > > adb shell echo mem > /sys/power/state
> > > > > Signed-off-by: josephjang <josephjang@...gle.com>
> > > > > ---
> > > > > include/linux/console.h | 1 +
> > > > > kernel/power/Kconfig | 9 +++
> > > > > kernel/power/main.c | 66 ++++++++++++++++
> > > > > kernel/power/suspend.c | 162
> ++++++++++++++++++++++++++++++++++++++++
> > > > > kernel/printk/printk.c | 5 ++
> > > > > 5 files changed, 243 insertions(+)
> > > > >
> > > > > diff --git a/include/linux/console.h b/include/linux/console.h
> > > > > index 0670d3491e0e..ac468c602c0b 100644
> > > > > --- a/include/linux/console.h
> > > > > +++ b/include/linux/console.h
> > > > > @@ -192,6 +192,7 @@ static inline void console_sysfs_notify(void)
> > > > > { }
> > > > > #endif
> > > > > extern bool console_suspend_enabled;
> > > > > +extern int is_console_suspended(void);
> > > > >
> > > > > /* Suspend and resume console messages over PM events */
> > > > > extern void suspend_console(void);
> > > > > diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig
> > > > > index a7320f07689d..52b7a181b6d8 100644
> > > > > --- a/kernel/power/Kconfig
> > > > > +++ b/kernel/power/Kconfig
> > > > > @@ -207,6 +207,15 @@ config PM_SLEEP_DEBUG
> > > > > def_bool y
> > > > > depends on PM_DEBUG && PM_SLEEP
> > > > >
> > > > > +config PM_SLEEP_MONITOR
> > > > > + bool "Linux kernel suspend/resume process monitor"
> > > > > + depends on PM_SLEEP
> > > > > + help
> > > > > + This option will enable suspend/resume monitor to prevent
> device
> > > > > + stuck during suspend/resume process. Suspend timeout
> handler
> > > > will
> > > > > + dump disk sleep task at first round timeout and trigger
> kernel
> > > > panic
> > > > > + at second round timeout. The default timer for each round
> is 30
> > > > seconds.
> > > > > +
> > >
> > > > The facility associated with the Kconfig entry right below is
> supposed
> > > > to do exactly the same thing.
> > >
> > > > What's the reason to add another one? What is missing?
> > >
> > >
> > >
> > > > > config DPM_WATCHDOG
> > > > > bool "Device suspend/resume watchdog"
> > > > > depends on PM_DEBUG && PSTORE && EXPERT
> > >
> > > Because we found some suspend hand issue that cannot be detected by
> > > "CONFIG_DPM_WATCHDOG" (which is focus on device PM).
> >
> > What's that issue?
> >
> > > Our suspend timeout monitor can cover PM core and Device PM hang
> issues.
> >
> > Then I'd suggest to extend the existing watchdog instead of adding a
> > whole new implementation.
Since kernel/power/suspend.c doesn't need "struct device *dev" like
following.
Can we create a new suspend_timer to cover PM core and Device PM hang
issues?
/**
* dpm_watchdog_set - Enable pm watchdog for given device.
* @wd: Watchdog. Must be allocated on the stack.
* @dev: Device to handle.
*/
static void dpm_watchdog_set(struct dpm_watchdog *wd, struct device *dev)
{
struct timer_list *timer = &wd->timer;
wd->dev = dev;
wd->tsk = current;
...
Thank you,
Joseph.
Powered by blists - more mailing lists