From: Arve Hjønnevåg on
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.

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

--
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: david on
On Wed, 4 Aug 2010, Matthew Garrett wrote:

> On Wed, Aug 04, 2010 at 12:29:36PM -0700, david(a)lang.hm wrote:
>
>> why would you trust it to take a wakelock, but not trust it the rest of
>> the time?
>
> Because I trust that when the application author says "I explicitly need
> the machine to stay awake" that they mean it, whereas I don't trust the
> application author to write an application that avoids consuming
> background CPU. The distinction is pretty important.

on the other hand, making an application avoid consuming inappropriate
background resources helps everywhere. an explicit "don't let the machine
sleep" only works if you are trusted by a system that implements this
flag.

yes, it is painful to make the change, but the end result is better (and
there are more tools out there to figure things out)

>> in my proposal I'm saying that if you would trust the application to take
>> a wakelock, you instead trust it to be sane in the rest of it's power
>> activity (avoiding polling, etc) and so you consider it for sleep
>> decisions.
>
> When we say "trust", we're not using the same meaning as we do with
> security. Yes, it's possible that an application that can block suspend
> will do so at inopportune times. But given that blocking suspend is an
> explicit act it's much more likely that the developer will only use it
> in reasonable ways, while it's still entirely plausible that the
> application will generate unnecessary wakeups. Pretending otherwise is
> unrealistic. I recently had to fix the fact that the kernel IPMI layer
> would generate a constant 1000 wakeups a second even if it had an
> interrupt-driven controller or was entirely idle.

I'm not sure I buy the distinction, but I'm not a maintainer so it's
others you have to convince.

it's better to have one type of problem with one set of tools that
document why the machine isn't sleeping, than to have orthoginal ways of
influencing power management.

but if you have an application in the mid-level trust situation, go ahead
and have it talk to a 'keepalive' daemon that is in the 'trusted' set and
let the rest of the app run untrusted. As I noted elsewhere, the keepalive
daemon would need very little in the way of resources and can implement
much more complex policies than anyone is going to be willing to put in
the kernel.

>>> The second is that the incoming network packet causes
>>> the kernel to take a wakelock that will be released once userspace has
>>> processed the network packet. This ensures that at least one wakelock is
>>> held for the entire relevant period of time.
>>
>> how do you determine that userspace has processed the network packet so
>> that the kernel can release the wakelock (or is this one of the cases
>> where there is a timer related to the wakelock)
>
> The current implementation uses a timer, but Rafael's implementation
> should allow userspace to explicitly acknowledge it.

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)

>> two things here,
>>
>> on the dirty networks that I see as common, refusing to sleep if network
>> packets are arriving will mean that you never go to sleep.
>
> 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.

>> secondly, nothing stops the code doing the idle/suspend decision from
>> considering network activity. I would be surprised if there weren't
>> already options to support this today.
>
> If you proxy every potential wakeup event through some central server
> then this becomes much easier, but it's also a performance hit. The
> alternative is that you poll for network activity, but that's a power
> hit.

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)

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: david on
On Wed, 4 Aug 2010, Matthew Garrett 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
> 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
> having this conversation? :) It'd be good to have some feedback from
> Google as to whether this satisfies their functional requirements.

the proposal that I nade was not to use cgroups to freeze some processes
and not others, but to use cgroups to decide to ignore some processes when
deciding if the system is idle, stop everything or nothing. cgroups are
just a way of easily grouping processes (and their children) into
different groups.

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: david on
On Wed, 4 Aug 2010, Florian Mickler wrote:

> On Wed, 4 Aug 2010 23:15:13 +0200
> Pavel Machek <pavel(a)ucw.cz> wrote:
>
>> Hi!
>>
>>>>>> If this doesn't work for the Android folks for whatever reason, another
>>>>>> approach would be to do the freeze in user code, which could track
>>>>>> whether any user-level resources (pthread mutexes, SysV semas, whatever)
>>>>>> where held, and do the freeze on a thread-by-thread basis within each
>>>>>> "victim" application as the threads reach safe points.
>>>>>
>>>>> 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.
>>>>
>>>> In different message, Arve said they are actually using low-power idle
>>>> to emulate suspend on Android.
>>>
>>> Hello, Pavel,
>>>
>>> Could you please point me at this message?
>>
>> AFAICT, this tells us that idle and suspend is the same hardware state
>> on current Android hardware:
>> Pavel
>>
>> Message-ID: <AANLkTinjH0C0bSK=Y2wKASnbJFsR2BN303xBXkaHbmRC(a)mail.gmail.com>
>>
>> Arve said:
>>
>> If you just program the alarm you will wake up see that the monotonic
>> clock has not advanced and set the alarm another n seconds into the
>> future. Or are proposing that suspend should be changed to keep the
>> monotonic clock running? If you are, why? We can enter the same
>> hardware states from idle, and modifying suspend to wake up more often
>> would increase the average power consumption in suspend, not improve
>> it for idle. In other words, if suspend wakes up as often as idle, why
>> use suspend?
>>
>>
>
> They always told us from the beginning, that on the msm platform they
> reach the same powerlevel from suspend and idle. They still get gains
> from using opportunistic suspend.

Yes, you will always get gains if you shutdown while there is still work
to do.

the question I am raising is.

If, instead of doing opportunistic suspend (with wakelocks to keep things
awake, requiring explicit application code changes to implement), how
close would you be able to get if instead you just were able to tell the
system to ignore some processes when considering if the system can sleep
or not?

With badly written 'trusted' apps you will have poor power useage, but
with badly written apps grabbing wakelocks inappropriately you will have
poor power useage.

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: david on
On Wed, 4 Aug 2010, Arve Hj?nnev?g wrote:

> On Wed, Aug 4, 2010 at 3:31 PM, <david(a)lang.hm> wrote:
>> On Wed, 4 Aug 2010, Matthew Garrett 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
>>> 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
>>> having this conversation? :) It'd be good to have some feedback from
>>> Google as to whether this satisfies their functional requirements.
>>
>> the proposal that I nade was not to use cgroups to freeze some processes and
>> not others, but to use cgroups to decide to ignore some processes when
>> deciding if the system is idle, stop everything or nothing. cgroups are just
>> a way of easily grouping processes (and their children) into different
>> groups.
>>
>
> That does not avoid the dependency problem. A process may be waiting
> on a resource that a process you ignore owns. I you ignore the process
> that owns the resource and enter idle when it is ready to run (or
> waiting on a timer), you are still effectively blocking the other
> process.

and if you don't have a wakelock the same thing will happen. If you expect
the process to take a while you can set a timeout to wake up every 30
seconds or so and wait again, this would be enough to prevent you from
going to sleep (or am I misunderstanding how long before you go into
suspend without a wakelock set, see my other e-mail for the full question)

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/