Message-ID: <87fx1fio2l.fsf@deeprootsystems.com>
Date:	Tue, 25 May 2010 16:00:50 -0700
From:	Kevin Hilman <khilman@...prootsystems.com>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
Cc:	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Alan Stern <stern@...land.harvard.edu>,
	Arve Hjønnevåg <arve@...roid.com>,
	"Linux-pm mailing list" <linux-pm@...ts.linux-foundation.org>,
	Kernel development list <linux-kernel@...r.kernel.org>,
	Len Brown <len.brown@...el.com>, Pavel Machek <pavel@....cz>,
	Randy Dunlap <rdunlap@...otime.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andi Kleen <ak@...ux.intel.com>,
	Cornelia Huck <cornelia.huck@...ibm.com>,
	Tejun Heo <tj@...nel.org>,
	Jesse Barnes <jbarnes@...tuousgeek.org>,
	Nigel Cunningham <nigel@...onice.net>,
	Ming Lei <tom.leiming@...il.com>,
	Wu Fengguang <fengguang.wu@...el.com>,
	Maxim Levitsky <maximlevitsky@...il.com>,
	linux-doc@...r.kernel.org
Subject: Re: [PATCH 1/8] PM: Opportunistic suspend support.

"Rafael J. Wysocki" <rjw@...k.pl> writes:

> On Tuesday 25 May 2010, Dmitry Torokhov wrote:
>> On Tue, May 25, 2010 at 09:47:22PM +0200, Rafael J. Wysocki wrote:
>> > On Tuesday 25 May 2010, Dmitry Torokhov wrote:
>> > > On Tuesday 25 May 2010 11:08:03 am Alan Stern wrote:
>> > > > On Tue, 25 May 2010, Dmitry Torokhov wrote:
>> > > > > > > I don't see a big difference between 2 and 3. You can use suspend
>> > > > > > > blockers to handle either.
>> > > > > > 
>> > > > > > You can, but they aren't necessary.  If 2 were the only reason for
>> > > > > > suspend blockers, I would say they shouldn't be merged.
>> > > > > > 
>> > > > > > Whereas 3, on the other hand, can _not_ be handled by any existing
>> > > > > > mechanism.  3 is perhaps the most important reason for using suspend
>> > > > > > blockers.
>> > > > > 
>> > > > > I do not see why 3 has to be implemented using suspend blockers either.
>> > > > > If you are concerned that event gets stuck somewhere in the stack make
>> > > > > sure that devices in the stack do not suspend while their queue is not
>> > > > > empty. This way if you try opportunistic suspend it will keep failing
>> > > > > until you drained all important queues.
>> > > > 
>> > > > Here's the scenario:
>> > > > 
>> > > > The system is awake, and the user presses a key. The keyboard driver
>> > > > processes the keystroke and puts it in an input queue.  A user process
>> > > > reads it from the event queue, thereby emptying the queue.
>> > > > 
>> > > > At that moment, the system decides to go into opportunistic suspend.
>> > > > Since the input queue is empty, there's nothing to stop it.  As the
>> > > > first step, userspace is frozen -- before the process has a chance to
>> > > > do anything with the keystroke it just read.  As a result, the system
>> > > > stays asleep until something else wakes it up, even though the
>> > > > keystroke was important and should have prevented it from sleeping.
>> > > > 
>> > > > Suspend blockers protect against this scenario.  Here's how:
>> > > > 
>> > > > The user process doesn't read the input queue directly; instead it
>> > > > does a select or poll.  When it sees there is data in the queue, it
>> > > > first acquires a suspend blocker and then reads the data.
>> > > > 
>> > > > Now the system _can't_ go into opportunistic suspend, because a suspend
>> > > > blocker is active.  The user process can do whatever it wants with the
>> > > > keystroke.  When it is finished, it releases the suspend blocker and
>> > > > loops back to the select/poll call.
>> > > > 
>> > > 
>> > > What you describe can be done in userspace though, via a "suspend manager" 
>> > > process. Tasks reading input events will post "busy" events to stop the 
>> > > manager process from sending system into suspend. But this can be confined to 
>> > > Android userspace, leaving the kernel as is (well, kernel needs to be modified 
>> > > to not go into suspend with full queues, but that is using existing kernel 
>> > > APIs).
>> > 
>> > For that to work, you'd have to make the user space suspend manager prevent
>> > key-reading processes from emptying the queue before it orders the kernel to
>> > put the system to sleep.  Otherwise it still is possible that the queue will be
>> > emptied right at the moment it writes to /sys/power/state and the scenario
>> > described by Alan is going to happen.
>> >
>> 
>> You do exactly the same as what Alan done, but in userspace - poll, post
>> "busy" event to suspend manager, read, process, retract "busy".
>> Basically you still have the suspend blocker, but it is confined to your
>> userspace.
>
> OK, now the question is why this is actually better.

A couple things come to mind...

1. Fixes problems for *all* kernel users, not just Android.

The kernel changes (refuse to suspend) would be done in a way that
would fix problems in the traditional suspend path as well as the
opportunistic suspend path, thus benefiting everyone.

2. Keep policy out of the kernel

A userspace suspend manager could implement _policy_ decisions in a
platform specific way, rather than having policy hard-coded into the
kernel.

Keeping the policy/governor in userspace would also allow various
governor techniques to be experimented with (polling/timeout
intervals, etc.) without having to patch the kernel.

Kevin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists