From: Matthew Garrett on
On Thu, Aug 05, 2010 at 07:22:53AM -0700, david(a)lang.hm wrote:

> I'm not sure it's possible to completely eliminate the race, even with
> wakelocks there is some time between the time you last check if the
> wakelock is set and when the hardware finishes responding to your
> commands to go to sleep (Unless you can set a level-based interrupt that
> will wake you up as soon as you finish going to sleep)

It's typically the case that you can - we've even seen that on x86 with
ACPI GPEs.

--
Matthew Garrett | mjg59(a)srcf.ucam.org
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: david on
On Thu, 5 Aug 2010, Matthew Garrett wrote:

> On Wed, Aug 04, 2010 at 03:20:04PM -0700, david(a)lang.hm wrote:
>
>> similar to the current implementation, the arrival of a packet could be
>> counted as activity that keeps the system awake for a bit (your timeout)
>
> How? You're either polling in order to know whether there's network
> activity, or you're having every network packet wake up the policy
> daemon, or you've got something that looks like the kernel side of
> wakelocks.

infrequent polling (on the order of seconds or 10s of seconds) is not a
very significant impact on your power

>>> Cell networks typically have no background traffic, for obvious reasons.
>>
>> but don't most new smartphones also connect up to wifi networks? those
>> are FAR from quiet.
>
> Wifi radio typically dwarfs your other power consumption anyway. The
> typical situation is one where you're not attached to a wifi network.

on some devices (my kindle for example) the cell network dwarfs my other
power consumption today.

>> I'm not suggesting running all events through some central server (unless
>> you count the kernel as that server), I'm saying that the decision that
>> the system is idle and therefor can be stopped should be able to take
>> this infomation into account, and if there's a race here, it should be a
>> race that exists everywhere else, so there should be a general solution,
>> not something specific to one use-case. (and definantly not something
>> that requires all software to be modified and trusted to implement)
>
> You're right. It's a race that exists everywhere else, and it's inherent
> unless you have some handshaking between userspace and the kernel when
> entering suspend. Which is what wakelocks provide. Nobody has proposed a
> solution that works without modifying existing code.

I think the issue is confusion between the different purposes of the
wakelock.

as you say, I don't think anyone is opposed to the kernel being modified
to eliminate this race. The disagreement is far more about the interface
to userspace and userspace use of wakelocks.

the kernel implementation of this doesn't have to set a single flag, and
it doesn't have to notify userspace directly. The kernel already produces
many stats today and it may be appropriate for the kernel to produce some
more.

for example, if you want to abort the suspend because there is network
activity, you can check the packet count of your network interface, decide
to go to sleep, setup the network interface to raise a 'wake me up'
interrupt (because you have decided in a userspace policy that you want
this), and then check to see if the packet count has changed. If it has,
abort the suspend, if not continue the suspend and once you are suspended
if the 'wake me up' interrupt is set you will wake back up.

there are probably cleaner/better ways of doing this than the simple logic
that I'm listing, but why wouldn't the simple logic provide the correct
result?

David Lang
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Brian Swetland on
On Thu, Aug 5, 2010 at 7:22 AM, <david(a)lang.hm> wrote:
>
> Ok, it is now sounding to me like there are two different (but somewhat
> related) purposes that wakelocks are being used for
>
> 1. deciding if the system should go to sleep now or not (what most of the
> discussion has been about)
>
> 2. narrowing the race between going to sleep and wakeup events.
>
> I'm not sure it's possible to completely eliminate the race, even with
> wakelocks there is some time between the time you last check if the wakelock
> is set and when the hardware finishes responding to your commands to go to
> sleep (Unless you can set a level-based interrupt that will wake you up as
> soon as you finish going to sleep)

The transition into sleep is race-free in the wakelock model -- either
the wakeup event happens during the kernel suspend (and grabs a
wakelock), in which case suspend is aborted (and not attempted again
until there are once again no more wakelocks held), or the system
fully suspends to its lowest power mode until a wakeup event brings it
back out again. Entry to lowest power mode must abort if a wakeup
event/interrupt occurs while it's in process -- exactly how the
handoff happens here is hardware dependent but in practice this is
doable on just about any modern system.

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Brian Swetland on
On Thu, Aug 5, 2010 at 7:34 AM, <david(a)lang.hm> wrote:
>
> for example, if you want to abort the suspend because there is network
> activity, you can check the packet count of your network interface, decide
> to go to sleep, setup the network interface to raise a 'wake me up'
> interrupt (because you have decided in a userspace policy that you want
> this), and then check to see if the packet count has changed. If it has,
> abort the suspend, if not continue the suspend and once you are suspended if
> the 'wake me up' interrupt is set you will wake back up.
>
> there are probably cleaner/better ways of doing this than the simple logic
> that I'm listing, but why wouldn't the simple logic provide the correct
> result?

If your network interrupt happens before the network driver's
suspend() hook is called this works -- the check in the suspend hook
observes the change and returns an abort status.

If your network interrupt happens after the suspend() hook is called
this does not work -- the event comes after your opportunity to abort
suspend has happened, your interrupt handler processed it, set the
flag, but the system proceeds to suspend anyway, missing the event.

The wakelock/suspendblock mechanism avoids races like the above.

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Paul E. McKenney on
On Wed, Aug 04, 2010 at 10:18:40PM -0700, david(a)lang.hm wrote:
> On Wed, 4 Aug 2010, Paul E. McKenney wrote:
> >On Wed, Aug 04, 2010 at 05:25:53PM -0700, david(a)lang.hm wrote:
> >>On Wed, 4 Aug 2010, Paul E. McKenney wrote:

[ . . . ]

> >>>The music player is an interesting example. It would be idle most
> >>>of the time, given that audio output doesn't consume very much CPU.
> >>>So you would not want to suspend the system just because there were
> >>>no runnable processes. In contrast, allowing the music player to
> >>>hold a wake lock lets the system know that it would not be appropriate
> >>>to suspend.
> >>>
> >>>Or am I misunderstanding what you are proposing?
> >>
> >>the system would need to be idle for 'long enough' (configurable)
> >>before deciding to suspend, so as long as 'long enough' is longer
> >>than the music player is idle this would not be a problem.
> >
> >From a user standpoint, having the music player tell the system when
> >it is OK to suspend (e.g., when the user has paused playback) seems
> >a lot nicer than having configurable timeouts that need tweaking.
>
> every system that I have seen has a configurable "sleep if it's idle
> for this long" knob. On the iphone (work issue, I didn't want it)
> that I am currently using it can be configured from 1 min to 5 min.
>
> this is the sort of timeout I am talking about.
>
> with something in the multi-minute range for the 'do a full suspend'
> doing a wakeup every few 10s of seconds is perfectly safe.

Ah, I was assuming -much- shorter "do full suspend" timeouts.

My (possibly incorrect) assumption is based on the complaint that led
to my implementing RCU_FAST_NO_HZ. A (non-Android) embedded person was
quite annoyed (to put it mildly) at the earlier version of RCU because
it prevented the system from entering the power-saving dyntick-idle mode,
not for minutes, or even for seconds, but for a handful of -milliseconds-.
This was my first hint that "energy efficiency" means something completely
different in embedded systems than it does in the servers that I am
used to.

But I must defer to the Android guys on this -- who knows, perhaps
multi-minute delays to enter full-suspend mode are OK for them.

> >>>>if the backlight being on holds the wakelock, it would seem that
> >>>>almost every other use of the wakelock could (and probably should)
> >>>>be replaced by something that tickles the display to stay on longer.
> >>>
> >>>The problem with this approach is that the display consumes quite a
> >>>bit of power, so you don't want to leave it on unnecessarily. So if
> >>>the system is doing something (for example, playing music) that does
> >>>not require the display, you really want the display to be off.
> >>
> >>what percentage (and types) of apps are really useful with the
> >>display off. I think that there are relativly few apps that you
> >>really want to keep running if the display is off.
> >
> >The length of time those apps are running is the governing factor
> >for battery life, and not the number of such apps, right?
>
> correct, but the number of such apps indicates the scope of the problem.

The number of such apps certainly indicates the amount of effort required
to modify them, if required. Is that what you are getting at?

> >From another e-mail tonight it sounds like almost everything
> >already talks
>
> to a userspace daemon, so if "(the power management service in the
> system_server, possibly the media_server and the radio interface
> glue)" (plus possibly some kernel activity) are the only things
> looked at when considering if it's safe to sleep or not, all of
> these can (or already do) do 'something' every few seconds, making
> this problem sound significantly smaller than it sounded like
> before.
>
> Android could even keep it's user-space API between the system power
> daemon and the rest of userspace the same if they want to.
>
> over time, additional apps could be considered 'trusted' (or flagged
> that way by the user) and not have to interact with the power daemon
> to keep things alive.

Hmmm... Isn't it the "trusted" (AKA PM-driving) apps that interact with
the power daemon via suspend blockers, rather than the other way around?

> as for intramentation, the key tool to use to see why a system isn't
> going to sleep would be powertop, just like on other linux systems.

Powertop is indeed an extremely valuable tool, but I am not certain
that it really provides the information that the Android guys need.
If I understand Arve's and Brian's posts, here is the scenario that they
are trying to detect:

o Some PM-driving application has a bug in which it fails to
release a wakelock, thus blocking suspend indefinitely.

o This PM-driving application, otherwise being a good citizen,
blocks.

o There are numerous power-oblivious apps running, consuming
significant CPU.

What the Android developers need to know is that the trusted application
is wrongly holding a wakelock. Won't powertop instead tell them about
all the power-oblivious apps?

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