* [PATCH] PM: avoid 'autosleep' in shutdown progress @ 2013-07-11 8:03 shuox.liu 2013-07-12 6:14 ` Yanmin Zhang 0 siblings, 1 reply; 6+ messages in thread From: shuox.liu @ 2013-07-11 8:03 UTC (permalink / raw) To: linux-kernel; +Cc: len.brown, pavel, rjw, yanmin_zhang, linux-pm From: Liu ShuoX <shuox.liu@intel.com> In shutdown progress, system is possible to do power transition (such as suspend-to-ram) in parallel. It is unreasonable. So, fixes it by adding a system_state checking and queue try_to_suspend again when system status is not running. Signed-off-by: Liu ShuoX <shuox.liu@intel.com> --- kernel/power/autosleep.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/kernel/power/autosleep.c b/kernel/power/autosleep.c index c6422ff..9012ecf 100644 --- a/kernel/power/autosleep.c +++ b/kernel/power/autosleep.c @@ -32,7 +32,8 @@ static void try_to_suspend(struct work_struct *work) mutex_lock(&autosleep_lock); - if (!pm_save_wakeup_count(initial_count)) { + if (!pm_save_wakeup_count(initial_count) || + system_state != SYSTEM_RUNNING) { mutex_unlock(&autosleep_lock); goto out; } -- 1.7.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: avoid 'autosleep' in shutdown progress 2013-07-11 8:03 [PATCH] PM: avoid 'autosleep' in shutdown progress shuox.liu @ 2013-07-12 6:14 ` Yanmin Zhang 2013-07-12 14:37 ` Alan Stern 2013-07-15 0:16 ` Rafael J. Wysocki 0 siblings, 2 replies; 6+ messages in thread From: Yanmin Zhang @ 2013-07-12 6:14 UTC (permalink / raw) To: shuox.liu; +Cc: linux-kernel, len.brown, pavel, rjw, linux-pm On Thu, 2013-07-11 at 16:03 +0800, shuox.liu@intel.com wrote: > From: Liu ShuoX <shuox.liu@intel.com> > > In shutdown progress, system is possible to do power transition > (such as suspend-to-ram) in parallel. It is unreasonable. So, > fixes it by adding a system_state checking and queue try_to_suspend > again when system status is not running. > > Signed-off-by: Liu ShuoX <shuox.liu@intel.com> > --- > kernel/power/autosleep.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > Without this patch, we hit an hang issue on Android. Scenario: Kernel starts shutdown and calls all device driver's shutdown callback. When a driver's shutdown is called, the last wakelock is released and suspend-to-ram starts. However, as some driver's shut down callbacks already shut down devices and disable runtime pm, the suspend-to-ram calls driver's suspend callback without noticing that device is already off and causes crash. We know the drivers should be fixed, but can we also change generic codes a little to make it stronger? > diff --git a/kernel/power/autosleep.c b/kernel/power/autosleep.c > index c6422ff..9012ecf 100644 > --- a/kernel/power/autosleep.c > +++ b/kernel/power/autosleep.c > @@ -32,7 +32,8 @@ static void try_to_suspend(struct work_struct *work) > > mutex_lock(&autosleep_lock); > > - if (!pm_save_wakeup_count(initial_count)) { > + if (!pm_save_wakeup_count(initial_count) || > + system_state != SYSTEM_RUNNING) { > mutex_unlock(&autosleep_lock); > goto out; > } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: avoid 'autosleep' in shutdown progress 2013-07-12 6:14 ` Yanmin Zhang @ 2013-07-12 14:37 ` Alan Stern 2013-07-13 11:56 ` Pavel Machek 2013-07-15 0:16 ` Rafael J. Wysocki 1 sibling, 1 reply; 6+ messages in thread From: Alan Stern @ 2013-07-12 14:37 UTC (permalink / raw) To: Yanmin Zhang; +Cc: shuox.liu, linux-kernel, len.brown, pavel, rjw, linux-pm On Fri, 12 Jul 2013, Yanmin Zhang wrote: > On Thu, 2013-07-11 at 16:03 +0800, shuox.liu@intel.com wrote: > > From: Liu ShuoX <shuox.liu@intel.com> > > > > In shutdown progress, system is possible to do power transition > > (such as suspend-to-ram) in parallel. It is unreasonable. So, > > fixes it by adding a system_state checking and queue try_to_suspend > > again when system status is not running. > > > > Signed-off-by: Liu ShuoX <shuox.liu@intel.com> > > --- > > kernel/power/autosleep.c | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > Without this patch, we hit an hang issue on Android. > > Scenario: > Kernel starts shutdown and calls all device driver's shutdown callback. > When a driver's shutdown is called, the last wakelock is released and > suspend-to-ram starts. However, as some driver's shut down callbacks > already shut down devices and disable runtime pm, the suspend-to-ram > calls driver's suspend callback without noticing that device is already > off and causes crash. > We know the drivers should be fixed, but can we also change generic > codes a little to make it stronger? Indeed, this does seem like the sort of thing we want to have. Allowing an "automatic" or "opportunistic" sleep in the middle of a shutdown makes no sense at all. In fact, it might be a good idea to disable system sleep completely at this time -- not even a write to /sys/power/state should be allowed to interrupt a shutdown. Alan Stern ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: avoid 'autosleep' in shutdown progress 2013-07-12 14:37 ` Alan Stern @ 2013-07-13 11:56 ` Pavel Machek 2013-07-15 0:51 ` Yanmin Zhang 0 siblings, 1 reply; 6+ messages in thread From: Pavel Machek @ 2013-07-13 11:56 UTC (permalink / raw) To: Alan Stern Cc: Yanmin Zhang, shuox.liu, linux-kernel, len.brown, rjw, linux-pm On Fri 2013-07-12 10:37:33, Alan Stern wrote: > On Fri, 12 Jul 2013, Yanmin Zhang wrote: > > > On Thu, 2013-07-11 at 16:03 +0800, shuox.liu@intel.com wrote: > > > From: Liu ShuoX <shuox.liu@intel.com> > > > > > > In shutdown progress, system is possible to do power transition > > > (such as suspend-to-ram) in parallel. It is unreasonable. So, > > > fixes it by adding a system_state checking and queue try_to_suspend > > > again when system status is not running. > > > > > > Signed-off-by: Liu ShuoX <shuox.liu@intel.com> > > > --- > > > kernel/power/autosleep.c | 3 ++- > > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > > Without this patch, we hit an hang issue on Android. > > > > Scenario: > > Kernel starts shutdown and calls all device driver's shutdown callback. > > When a driver's shutdown is called, the last wakelock is released and > > suspend-to-ram starts. However, as some driver's shut down callbacks > > already shut down devices and disable runtime pm, the suspend-to-ram > > calls driver's suspend callback without noticing that device is already > > off and causes crash. > > We know the drivers should be fixed, but can we also change generic > > codes a little to make it stronger? > > Indeed, this does seem like the sort of thing we want to have. > Allowing an "automatic" or "opportunistic" sleep in the middle of a > shutdown makes no sense at all. In fact, it might be a good idea to > disable system sleep completely at this time -- not even a write to > /sys/power/state should be allowed to interrupt a shutdown. I'm not completely sure, but as long as userland is running, we should have system_state == RUNNING, no? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: avoid 'autosleep' in shutdown progress 2013-07-13 11:56 ` Pavel Machek @ 2013-07-15 0:51 ` Yanmin Zhang 0 siblings, 0 replies; 6+ messages in thread From: Yanmin Zhang @ 2013-07-15 0:51 UTC (permalink / raw) To: Pavel Machek Cc: Alan Stern, shuox.liu, linux-kernel, len.brown, rjw, linux-pm On Sat, 2013-07-13 at 13:56 +0200, Pavel Machek wrote: > On Fri 2013-07-12 10:37:33, Alan Stern wrote: > > On Fri, 12 Jul 2013, Yanmin Zhang wrote: > > > > > On Thu, 2013-07-11 at 16:03 +0800, shuox.liu@intel.com wrote: > > > > From: Liu ShuoX <shuox.liu@intel.com> > > > > > > > > In shutdown progress, system is possible to do power transition > > > > (such as suspend-to-ram) in parallel. It is unreasonable. So, > > > > fixes it by adding a system_state checking and queue try_to_suspend > > > > again when system status is not running. > > > > > > > > Signed-off-by: Liu ShuoX <shuox.liu@intel.com> > > > > --- > > > > kernel/power/autosleep.c | 3 ++- > > > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > > > > Without this patch, we hit an hang issue on Android. > > > > > > Scenario: > > > Kernel starts shutdown and calls all device driver's shutdown callback. > > > When a driver's shutdown is called, the last wakelock is released and > > > suspend-to-ram starts. However, as some driver's shut down callbacks > > > already shut down devices and disable runtime pm, the suspend-to-ram > > > calls driver's suspend callback without noticing that device is already > > > off and causes crash. > > > We know the drivers should be fixed, but can we also change generic > > > codes a little to make it stronger? > > > > Indeed, this does seem like the sort of thing we want to have. > > Allowing an "automatic" or "opportunistic" sleep in the middle of a > > shutdown makes no sense at all. In fact, it might be a good idea to > > disable system sleep completely at this time -- not even a write to > > /sys/power/state should be allowed to interrupt a shutdown. > > I'm not completely sure, but as long as userland is running, we should > have system_state == RUNNING, no? No. The reboot/shutdown is started by application processes, then kernel changes system_state quickly. See kernel_restart_prepare, kernel_shutdown_prepare. But indeed, it's a good question. We hit many other issues around rebooting. When system is rebooting, user space processes are still running. There is no freeze step. Some processes might jump in to access devices. To avoid it, we have to add some tricks in some device drivers. It's a little crazy. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] PM: avoid 'autosleep' in shutdown progress 2013-07-12 6:14 ` Yanmin Zhang 2013-07-12 14:37 ` Alan Stern @ 2013-07-15 0:16 ` Rafael J. Wysocki 1 sibling, 0 replies; 6+ messages in thread From: Rafael J. Wysocki @ 2013-07-15 0:16 UTC (permalink / raw) To: yanmin_zhang; +Cc: shuox.liu, linux-kernel, len.brown, pavel, linux-pm On Friday, July 12, 2013 02:14:11 PM Yanmin Zhang wrote: > On Thu, 2013-07-11 at 16:03 +0800, shuox.liu@intel.com wrote: > > From: Liu ShuoX <shuox.liu@intel.com> > > > > In shutdown progress, system is possible to do power transition > > (such as suspend-to-ram) in parallel. It is unreasonable. So, > > fixes it by adding a system_state checking and queue try_to_suspend > > again when system status is not running. > > > > Signed-off-by: Liu ShuoX <shuox.liu@intel.com> > > --- > > kernel/power/autosleep.c | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > Without this patch, we hit an hang issue on Android. > > Scenario: > Kernel starts shutdown and calls all device driver's shutdown callback. > When a driver's shutdown is called, the last wakelock is released and > suspend-to-ram starts. However, as some driver's shut down callbacks > already shut down devices and disable runtime pm, the suspend-to-ram > calls driver's suspend callback without noticing that device is already > off and causes crash. OK, queued up as a fix for 3.11, with a modified changelog (I used your scenario above in it). Thanks, Rafael > We know the drivers should be fixed, but can we also change generic > codes a little to make it stronger? > > > diff --git a/kernel/power/autosleep.c b/kernel/power/autosleep.c > > index c6422ff..9012ecf 100644 > > --- a/kernel/power/autosleep.c > > +++ b/kernel/power/autosleep.c > > @@ -32,7 +32,8 @@ static void try_to_suspend(struct work_struct *work) > > > > mutex_lock(&autosleep_lock); > > > > - if (!pm_save_wakeup_count(initial_count)) { > > + if (!pm_save_wakeup_count(initial_count) || > > + system_state != SYSTEM_RUNNING) { > > mutex_unlock(&autosleep_lock); > > goto out; > > } > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pm" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- I speak only for myself. Rafael J. Wysocki, Intel Open Source Technology Center. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-07-15 0:51 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-07-11 8:03 [PATCH] PM: avoid 'autosleep' in shutdown progress shuox.liu 2013-07-12 6:14 ` Yanmin Zhang 2013-07-12 14:37 ` Alan Stern 2013-07-13 11:56 ` Pavel Machek 2013-07-15 0:51 ` Yanmin Zhang 2013-07-15 0:16 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).