From: Brian Swetland on
2010/8/4 Paul E. McKenney <paulmck(a)linux.vnet.ibm.com>:
> On Wed, Aug 04, 2010 at 03:08:33PM -0700, Arve Hjønnevåg wrote:
>> On Wed, Aug 4, 2010 at 1:56 PM, Matthew Garrett <mjg59(a)srcf.ucam.org> wrote:
>
> [ . . . ]
>
>> > having this conversation? :) It'd be good to have some feedback from
>> > Google as to whether this satisfies their functional requirements.
>>
>> That is "this"? The merged code? If so, no it does not satisfy our
>> requirements. The in kernel api, while offering similar functionality
>> to the wakelock interface, does not use any handles which makes it
>> impossible to get reasonable stats (You don't know which pm_stay_awake
>> request pm_relax is reverting). The proposed in user-space interface
>> of calling into every process that receives wakeup events before every
>> suspend call is also not compatible with existing apps.
>
> I should have asked this earlier...  What exactly are the apps'
> compatibility constraints?  Source-level APIs?  Byte-code class-library
> invocations?  C/C++ dynamic linking?  C/C++ static linking (in other
> words, syscall)?

For Java/Dalvik apps, the wakelock API is pertty high level -- it
talks to a service via RPC (Binder) that actually interacts with the
kernel. Changing the basic kernel<->userspace interface (within
reason) is not unthinkable. For example, Arve's suspend_blocker patch
provides a device interface rather than the proc interface the older
wakelock patches use. We'd have to make some userspace changes to
support that but they're pretty low level and minor.

In the current model, only a few processes need to specifically
interact with the kernel (the power management service in the
system_server, possibly the media_server and the radio interface
glue). A model where every process needs to have a bunch of
instrumentation is not very desirable from our point of view. We
definitely do need reasonable statistics in order to enable debugging
and to enable reporting to endusers (through the Battery Usage UI)
what's keeping the device awake.

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: Matt Helsley on
On Wed, Aug 04, 2010 at 07:02:55PM -0700, Brian Swetland wrote:
> On Wed, Aug 4, 2010 at 6:58 PM, Matt Helsley <matthltc(a)us.ibm.com> wrote:
> > On Wed, Aug 04, 2010 at 05:35:09PM +0100, Matthew Garrett wrote:
> >>
> >> The main problem I see with the cgroups solution is that it doesn't seem
> >> to do anything to handle avoiding loss of wakeup events.
> >
> > cgroups alone don't but there is a solution which doesn't require routing
> > all event data through a single server -- use SIGIO.
> >
> > Suppose we've got two cgroups of tasks -- those in the initial freezer
> > cgroup and those in a freezer cgroup meant for power-naive apps. Let's
> > call the second cgroup the naive cgroup.
> >
> > One task -- let's call it the "waker" -- is in the initial cgroup is normaly
> > asleep waiting for SIGIO. Note it's not an "app" -- it's been trusted/blessed
> > to be a non-power-naive task. It will be signaled via SIGIO by the
> > applications which want to be unfrozen when an event comes in.
> >
> > When the power-naive app in the naive cgroup opens a file descriptor it's
> > going through the Android interpretter to make the syscall. The interpretter
> > can do fcntl() on the fd to cause SIGIO to be delivered to the waker task.
> > When the waker gets SIGIO it unfreezes the naive cgroup and thus wakes the
> > power-naive app. When the power-naive app wakes it will
> > poll/return-from-poll/read/return-from-read and thus retrieve the event.
>
> The Android execution model includes native code in addition to the
> dalvik VM, and in the future could include other runtimes -- there are
> many standard libraries that directly make posix file IO calls, and
> apps can bundle native libraries which can also directly make
> syscalls. It's not practical to instrument every piece of userspace
> code that opens a fd somehow to make additional fcntl calls in various
> places.

Perhaps using an LD_PRELOAD will work.

Cheers,
-Matt Helsley
--
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 06:02:28PM -0700, Arve Hj�nnev�g wrote:
> 2010/8/4 Rafael J. Wysocki <rjw(a)sisk.pl>:
> > On Thursday, August 05, 2010, Arve Hj�nnev�g wrote:
> >> On Wed, Aug 4, 2010 at 1:56 PM, Matthew Garrett <mjg59(a)srcf.ucam.org> wrote:
> >> > On Wed, Aug 04, 2010 at 10:51:07PM +0200, Rafael J. Wysocki wrote:
> >> >> On Wednesday, August 04, 2010, Matthew Garrett wrote:
> >> >> > No! And that's precisely the issue. Android's existing behaviour could
> >> >> > be entirely implemented in the form of binary that manually triggers
> >> >> > suspend when (a) the screen is off and (b) no userspace applications
> >> >> > have indicated that the system shouldn't sleep, except for the wakeup
> >> >> > event race. Imagine the following:
> >> >> >
> >> >> > 1) The policy timeout is about to expire. No applications are holding
> >> >> > wakelocks. The system will suspend providing nothing takes a wakelock.
> >> >> > 2) A network packet arrives indicating an incoming SIP call
> >> >> > 3) The VOIP application takes a wakelock and prevents the phone from
> >> >> > suspending while the call is in progress
> >> >> >
> >> >> > What stops the system going to sleep between (2) and (3)? cgroups don't,
> >> >> > because the voip app is an otherwise untrusted application that you've
> >> >> > just told the scheduler to ignore.
> >> >>
> >> >> I _think_ you can use the just-merged /sys/power/wakeup_count mechanism to
> >> >> avoid the race (if pm_wakeup_event() is called at 2)).
> >> >
> >> > Yes, I think that solves the problem. The only question then is whether
> >>
> >> How? By passing a timeout to pm_wakeup_event when the network driver
> >> gets the packet or by passing 0. If you pass a timeout it is the same
> >> as using a wakelock with a timeout and should work (assuming the
> >> timeout you picked is long enough). If you don't pass a timeout it
> >> does not work, since the packet may not be visible to user-space yet.
> >
> > Alternatively, pm_stay_awake() / pm_relax() can be used.
>
> Which makes the driver and/or network stack changes identical to using
> wakelocks, right?

Arve, you say that like it is a bad thing. ;-)

Seriously, the hope is that Rafael's implementation is useful to other
projects in addition to Android. And, all else being equal, the more
people who need a given facility, the more likely that facility is to
make it to mainline, right?

And yes, I see you call out some additional things that Android needs,
but hopefully this gap can be closed one way or another.

Thanx, Paul

> >> > it's preferable to use cgroups or suspend fully, which is pretty much up
> >> > to the implementation. In other words, is there a reason we're still
> >>
> >> I have seen no proposed way to use cgroups that will work. If you
> >> leave some processes running while other processes are frozen you run
> >> into problems when a frozen process holds a resource that a running
> >> process needs.
> >>
> >>
> >> > having this conversation? :) It'd be good to have some feedback from
> >> > Google as to whether this satisfies their functional requirements.
> >> >
> >>
> >> That is "this"? The merged code? If so, no it does not satisfy our
> >> requirements. The in kernel api, while offering similar functionality
> >> to the wakelock interface, does not use any handles which makes it
> >> impossible to get reasonable stats (You don't know which pm_stay_awake
> >> request pm_relax is reverting).
> >
> > Why is that a problem (out of curiosity)?
> >
>
> Not having stats or not knowing what pm_relax is undoing? We need
> stats to be able to debug the system. If the system does not suspend
> at all or is awake for too long, the wakelock stats tells us which
> component is at fault. Since pm_stay_awake and pm_relax does not
> operate on a handle, you cannot determine how long it prevented
> suspend for.
>
> >> The proposed in user-space interface
> >> of calling into every process that receives wakeup events before every
> >> suspend call
> >
> > Well, �you don't really need to do that.
> >
>
> Only if the driver blocks suspend until user-space has read the event.
> This means that for android to work we need to block suspend when
> input events are not processed, but a system using your scheme needs a
> pm_wakeup_event call when the input event is queued. How to you switch
> between them? Do we add separate ioctls in the input device to enable
> each scheme? If someone has a single threaded user space power manager
> that also reads input event it will deadlock if you block suspend
> until it reads the input events since you block when reading the wake
> count.
>
> >> is also not compatible with existing apps.
> >
> > Thanks,
> > Rafael
> > --
> > 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/
> >
>
>
>
> --
> Arve Hj�nnev�g
--
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 07:46:56PM -0700, Brian Swetland wrote:
> 2010/8/4 Paul E. McKenney <paulmck(a)linux.vnet.ibm.com>:
> > On Wed, Aug 04, 2010 at 03:08:33PM -0700, Arve Hj�nnev�g wrote:
> >> On Wed, Aug 4, 2010 at 1:56 PM, Matthew Garrett <mjg59(a)srcf.ucam.org> wrote:
> >
> > [ . . . ]
> >
> >> > having this conversation? :) It'd be good to have some feedback from
> >> > Google as to whether this satisfies their functional requirements.
> >>
> >> That is "this"? The merged code? If so, no it does not satisfy our
> >> requirements. The in kernel api, while offering similar functionality
> >> to the wakelock interface, does not use any handles which makes it
> >> impossible to get reasonable stats (You don't know which pm_stay_awake
> >> request pm_relax is reverting). The proposed in user-space interface
> >> of calling into every process that receives wakeup events before every
> >> suspend call is also not compatible with existing apps.
> >
> > I should have asked this earlier... �What exactly are the apps'
> > compatibility constraints? �Source-level APIs? �Byte-code class-library
> > invocations? �C/C++ dynamic linking? �C/C++ static linking (in other
> > words, syscall)?
>
> For Java/Dalvik apps, the wakelock API is pertty high level -- it
> talks to a service via RPC (Binder) that actually interacts with the
> kernel. Changing the basic kernel<->userspace interface (within
> reason) is not unthinkable. For example, Arve's suspend_blocker patch
> provides a device interface rather than the proc interface the older
> wakelock patches use. We'd have to make some userspace changes to
> support that but they're pretty low level and minor.
>
> In the current model, only a few processes need to specifically
> interact with the kernel (the power management service in the
> system_server, possibly the media_server and the radio interface
> glue). A model where every process needs to have a bunch of
> instrumentation is not very desirable from our point of view. We
> definitely do need reasonable statistics in order to enable debugging
> and to enable reporting to endusers (through the Battery Usage UI)
> what's keeping the device awake.

Thank you for the info!

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/
From: david on
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:
>>
>>> On Wed, Aug 04, 2010 at 04:49:22PM -0700, david(a)lang.hm wrote:
>>>> On Wed, 4 Aug 2010, Paul E. McKenney wrote:
>>>>
>>>>> On Wed, Aug 04, 2010 at 04:23:43PM -0700, david(a)lang.hm wrote:
>>>>>> On Wed, 4 Aug 2010, Arve Hj?nnev?g wrote:
>>>>>>
>>>>>>>
>>>>>>> We suspend as soon as no wakelocks are held. There is no delay.
>>>>>>
>>>>>> So, if I have a bookreader app that is not allowed to get the
>>>>>> wakelock, and nothing else is running, the system will suspend
>>>>>> immediatly after I click a button to go to the next page? it will
>>>>>> not stay awake to give me a chance to read the page at all?
>>>>>>
>>>>>> how can any application run without wakelock privilages?
>>>>>
>>>>> Isn't a wakelock held as long as the display is lit, so that the
>>>>> system would continue running as long as the page was visible?
>>>>
>>>> what holds this wakelock, and what sort of timeout does it have?
>>>> (and why could that same timeout be used in other ways instead)
>>>
>>> I defer to the Android guys on what exactly holds the display's
>>> wakelock. The timeout is the display-blank timeout.
>>>
>>>> how many apps really need to keep running after the screen blanks?
>>>> there are a few (audio output apps, including music player and
>>>> Navigation directions), but I don't have see a problem with them
>>>> being marked as the 'trusted' apps to pay attention instead.
>>>
>>> Downloading is another.
>>
>> this is definantly an interesting case, do you want an active
>> network connection to keep the machine awake? if so do you want it
>> for all network connections, or only for some...
>
> The Android approach is that everything is permitted to run when the
> device is not suspended. So that would be all network connections.

I wasn't suggesting freezing some network connections while letting others
run, I was thinking in terms of 'if the system patcher daemon is
downloading, don't go to sleep, but if it's dancing cows (or mail client
to an IMAP server) go ahead and sleep, even if there is some data passing
over the connection.

>>> 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.

>>>> 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.

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.

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.

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/