Re: [PATCH 1/8] PM: Add suspend block api.

Paul Walmsley
2010-05-18T02:17:58Z

[-- Attachment #1: Type: TEXT/PLAIN, Size: 3446 bytes --]

Hello Arve,

On Fri, 14 May 2010, Arve Hjønnevåg wrote:

> On Thu, May 13, 2010 at 11:27 PM, Paul Walmsley <paul@pwsan.com> wrote:
> >
> > On Thu, 13 May 2010, Arve Hjønnevåg wrote:
> >
> >> Adds /sys/power/policy that selects the behaviour of /sys/power/state.
> >> After setting the policy to opportunistic, writes to /sys/power/state
> >> become non-blocking requests that specify which suspend state to enter
> >> when no suspend blockers are active. A special state, "on", stops the
> >> process by activating the "main" suspend blocker.
> >>
> >> Signed-off-by: Arve Hjønnevåg <arve@android.com>
> >
> > Looking at the way that suspend-blocks are used in the current Android
> > 'msm' kernel tree[1], they seem likely to cause layering violations,
> > fragile kernel code with userspace dependencies, and code that consumes
> > power unnecessarily.
> >
> > For example, in drivers/mmc/core/core.c:721, in mmc_rescan() [2], we find
> > the following code:
> >
> >        /* give userspace some time to react */
> >        wake_lock_timeout(&mmc_delayed_work_wake_lock, HZ / 2);
> >
> > This is a layering violation.  The MMC subsystem should have nothing to do
> > with "giving userspace time to react."  That is the responsibility of the
> > Linux scheduler.
> >
> > This code is also intrinsically fragile and use-case dependent.  What if
> > userspace occasionally needs more than (HZ / 2) to react?  If the
> > distributor is lucky enough to catch this before the product ships, then
> > the distributor can patch in a new magic number.  But if the device makes
> > it to the consumer, the result is an unstable device that unpredictably
> > suspends.
> >
> > The above code will also waste power.  Even if userspace takes less than
> > (HZ / 2) to react, the system will still be prevented from entering a
> > system-wide low power state for the duration of the remaining time.  This
> > is in contrast to an approach that uses the idle loop to enter a
> > system-wide low power state.  The moment that the system has no further
> > work to do, it can start saving power.
> >
>
> Yes, preventing suspend for 1/2 second wastes some power, but are you
> seriously suggesting that it wastes more power then never suspending?
> Opportunitic suspend does not prevent entering low power modes from
> idle.

Sorry, my E-mail was unclear. Here is my intended point:

The current Android opportunistic suspend governor does not enter full
system suspend until all suspend-blocks are released. If the governor
were modified to respect the scheduler, then it could enter suspend the
moment that the system had no further work to do.

So during part of the (HZ / 2) time interval in the above code, the system
is running some kernel or userspace code. But after that work is done, an
idle-loop-based opportunistic suspend governor could enter idle during
the remaining portion of that (HZ / 2) time interval, while the current
governor must keep the system out of suspend.

Of course, to do this, a different method for freezing processes would be
needed; one possible approach is described here[1]; others have proposed
similar approaches.

regards,

- Paul

1. Paragraphs B4A and B4B of Paul Walmsley E-mail to the linux-pm mailing
list, dated Mon May 17 18:39:44 PDT 2010:
https://lists.linux-foundation.org/pipermail/linux-pm/2010-May/025663.html

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]