* [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-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
* 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
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).