[RFD] Automatic suspend

[RFD] Automatic suspend

[Rafael J. Wysocki]

Hi,

The recent descussion about the Android PM patches sent by Arve shows that
there is a need to introduce a mechanism allowing us to:
(1) automatically put the system as a whole into a sleep state (eg. suspend to
    RAM) when it is found to be "idle", where the meaning of "idle" has to be
    defined too,
(2) put given subset of devices into low power states whenever they are not
    used, without putting the entire system into a sleep state.
To allow these two things to happen, the Andriod patches introduced the
wakelocks with the associated infrastructure and the "early suspend" mechanism.
However, quite a number of reviewers did not like these patches, for various
reasons, so they cannot be regarded as generally acceptable.  For this reason,
I think, we should discuss the problem in detail from the beginning and try to
find a solution that everyone interested will be comfortable with.

For this purpose, IMO, we should at least reach an agreement on what the
kernel's and the userland's roles in (1) and (2) are going to be.  There also
are quite a few questions that need to be answered.  For instance, what
conditions are going to be used to decide whether or not the system is idle
enough so that we can put it into a sleep state?  How are we going to check
these conditions?  What is going to happen if one (or more) of them changes
while a system-wide power transition (eg. suspend) is in progress?  Are we
going to allow the user space to take part in this and if so, to what extent?
What mechanisms are going be used to put devices into low power states at run
time (ie. before starting any system-wide power transition) and how are they
going to be related to the suspend-resume infrastructure used during
system-wide power transitions?  The answers to these (and other related)
questions will likely affect all of the future Linux PM developlent, so IMO
this is a very important matter.

Opinions and comments welcome.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 00:10:15 +0100
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> Hi,
> 
> The recent descussion about the Android PM patches sent by Arve shows
> that there is a need to introduce a mechanism allowing us to:
> (1) automatically put the system as a whole into a sleep state (eg.
> suspend to RAM) when it is found to be "idle", where the meaning of
> "idle" has to be defined too,
> (2) put given subset of devices into low power states whenever they
> are not used, without putting the entire system into a sleep state.


For (2), for me the answer is very obvious:

The Device Driver needs to make the decision to put the device to sleep.
There are no ifs and buts about this.

It's the driver who 
a) knows if there's any activity, such as open users
and 
b) is in the right position to know how to put things to sleep.

Of course, the subsystem the driver belongs to can provide helpers
(such as generic activity timeout handlers etc).

For many cases, the drivers do this today already.
There are cases where doing this has side effects, mostly in terms of
latency. It is reasonable to have a general mechanism that provides a
central mechanism to track tolerable latencies; in fact PMQOS provides
this on a high level, and I can imagine that PMQOS needs to be extended
to provide a wider range of type of latencies.

Userland should never ever control the state of a device like this
directly. It should do so by a) closing the device and b) setting
latency / functional requirements.




-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Benjamin Herrenschmidt]

On Sun, 2009-02-15 at 16:44 -0800, Arjan van de Ven wrote:
> On Mon, 16 Feb 2009 00:10:15 +0100
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > Hi,
> > 
> > The recent descussion about the Android PM patches sent by Arve shows
> > that there is a need to introduce a mechanism allowing us to:
> > (1) automatically put the system as a whole into a sleep state (eg.
> > suspend to RAM) when it is found to be "idle", where the meaning of
> > "idle" has to be defined too,
> > (2) put given subset of devices into low power states whenever they
> > are not used, without putting the entire system into a sleep state.
> 
> 
> For (2), for me the answer is very obvious:

 .../...

I agree with pretty much everything Arjan wrote here.

> Userland should never ever control the state of a device like this
> directly. It should do so by a) closing the device and b) setting
> latency / functional requirements

With the possible exception of things like wifi/bt killswitch or network
or similar where the driver cannot shut the device down without losing
the ability to detect activity (ie, switch your PHY off is nice but you
lose the ability to monitor the link for example).

Cheers,
Ben.

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 13:12:40 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Sun, 2009-02-15 at 16:44 -0800, Arjan van de Ven wrote:
> > On Mon, 16 Feb 2009 00:10:15 +0100
> > "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> > 
> > > Hi,
> > > 
> > > The recent descussion about the Android PM patches sent by Arve
> > > shows that there is a need to introduce a mechanism allowing us
> > > to: (1) automatically put the system as a whole into a sleep
> > > state (eg. suspend to RAM) when it is found to be "idle", where
> > > the meaning of "idle" has to be defined too,
> > > (2) put given subset of devices into low power states whenever
> > > they are not used, without putting the entire system into a sleep
> > > state.
> > 
> > 
> > For (2), for me the answer is very obvious:
> 
>  .../...
> 
> I agree with pretty much everything Arjan wrote here.
> 
> > Userland should never ever control the state of a device like this
> > directly. It should do so by a) closing the device and b) setting
> > latency / functional requirements
> 
> With the possible exception of things like wifi/bt killswitch 

this is a functional switch; but yeah it is a little bit of an odd-one
out. rfkill is also not REALLY for power, it's more for FAA stuff.

>or
> network or similar where the driver cannot shut the device down
> without losing the ability to detect activity (ie, switch your PHY
> off is nice but you lose the ability to monitor the link for example).

what this needs is a separation between "interface down, all off" and
"interface down enough so that link detection works".
Some NICs can go all the way down and still do link detection, others
cannot. This is obviously something the driver will know, nothing else
in the system should know.

> 
> Cheers,
> Ben.
> 


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Benjamin Herrenschmidt]

On Sun, 2009-02-15 at 18:20 -0800, Arjan van de Ven wrote:
> what this needs is a separation between "interface down, all off" and
> "interface down enough so that link detection works".
> Some NICs can go all the way down and still do link detection, others
> cannot. This is obviously something the driver will know, nothing else
> in the system should know.

Sure, my point was just that the driver cannot make the decision to go
all-off (and thus not detect link) without some kind of user setting
allowing it to do so.

Cheers,
Ben.

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 14:23:41 +1100
Benjamin Herrenschmidt <benh@kernel.crashing.org> wrote:

> On Sun, 2009-02-15 at 18:20 -0800, Arjan van de Ven wrote:
> > what this needs is a separation between "interface down, all off"
> > and "interface down enough so that link detection works".
> > Some NICs can go all the way down and still do link detection,
> > others cannot. This is obviously something the driver will know,
> > nothing else in the system should know.
> 
> Sure, my point was just that the driver cannot make the decision to go
> all-off (and thus not detect link) without some kind of user setting
> allowing it to do so.

to a very large degree, "want linkdetection" is a form of keeping the
device open.. even if the current code doesn't quite work that way
(maybe it should)



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Roland Dreier]

 > > (2) put given subset of devices into low power states whenever they
 > > are not used, without putting the entire system into a sleep state.

 > For (2), for me the answer is very obvious:

 > The Device Driver needs to make the decision to put the device to sleep.
 > There are no ifs and buts about this.

For PC-like systems this is probably all that needs to be said.  However
for highly integrated SoC systems (as Android is obviously targeting)
there is another level of complexity due to the interdependency among
various devices... eg things like if I know the SD controller and the
wifi chip are both asleep then I can put my SDIO controller to sleep;
and if the SDIO controller and the NAND controller are both asleep then
I can stop clock X and save more power; etc etc.

This is what the PowerOp/DPM work was all about.  Unfortunately that
doesn't seem to have made much progress upstream.  But there's no doubt
in my mind that we need some framework beyond individual drivers that
manages the system's power as a whole.  And the current device tree is
probably not sufficient -- eg the bus hierarchy of a device may not
match up with the clock tree at all.

 - R.

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Monday 16 February 2009 01:44:56 schrieb Arjan van de Ven:
> On Mon, 16 Feb 2009 00:10:15 +0100
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > Hi,
> > 
> > The recent descussion about the Android PM patches sent by Arve shows
> > that there is a need to introduce a mechanism allowing us to:
> > (1) automatically put the system as a whole into a sleep state (eg.
> > suspend to RAM) when it is found to be "idle", where the meaning of
> > "idle" has to be defined too,
> > (2) put given subset of devices into low power states whenever they
> > are not used, without putting the entire system into a sleep state.
> 
> 
> For (2), for me the answer is very obvious:
> 
> The Device Driver needs to make the decision to put the device to sleep.
> There are no ifs and buts about this.
> 
> It's the driver who 
> a) knows if there's any activity, such as open users
> and 
> b) is in the right position to know how to put things to sleep.

I think you need  third option between decides to put to sleep
and refuses to put to sleep. It is probably necessary to let drivers
state that they would want to go along if the whole system goes
to sleep.

> Of course, the subsystem the driver belongs to can provide helpers
> (such as generic activity timeout handlers etc).
> 
> For many cases, the drivers do this today already.
> There are cases where doing this has side effects, mostly in terms of
> latency. It is reasonable to have a general mechanism that provides a

Exactly.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Monday 16 February 2009 03:20:12 schrieb Arjan van de Ven:
> >or
> > network or similar where the driver cannot shut the device down
> > without losing the ability to detect activity (ie, switch your PHY
> > off is nice but you lose the ability to monitor the link for example).
> 
> what this needs is a separation between "interface down, all off" and
> "interface down enough so that link detection works".
> Some NICs can go all the way down and still do link detection, others
> cannot. This is obviously something the driver will know, nothing else
> in the system should know.

Why shouldn't user space know the consequences of still requesting
a service and be able to decide on that basis which services to request?

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Sun, 15 Feb 2009 22:23:04 -0800
Roland Dreier <rdreier@cisco.com> wrote:

>  > > (2) put given subset of devices into low power states whenever
>  > > they are not used, without putting the entire system into a
>  > > sleep state.
> 
>  > For (2), for me the answer is very obvious:
> 
>  > The Device Driver needs to make the decision to put the device to
>  > sleep. There are no ifs and buts about this.
> 
> For PC-like systems this is probably all that needs to be said.
> However for highly integrated SoC systems (as Android is obviously
> targeting) there is another level of complexity due to the
> interdependency among various devices... eg things like if I know the
> SD controller and the wifi chip are both asleep then I can put my
> SDIO controller to sleep; and if the SDIO controller and the NAND
> controller are both asleep then I can stop clock X and save more
> power; etc etc.

This does not change the rules.
The individual drivers still make up the decision of what state their
own block should be in.

What you're talking about is coordination. Some systems do this in
silicon, and others, like you mention, need a small software layer to
do this coordination. Fine. But this doesn't change the fundamentals.

> 
> This is what the PowerOp/DPM work was all about.  Unfortunately that
> doesn't seem to have made much progress upstream. 

PowerOP/DPM were... more or less hampered because they operated in
global "modes". Everyone (including the OMAP guys) has pretty much gone
away from the idea that modes are global. Power management is not
global; if you treat it as global then you're in trouble, since the
decisions really are local (and in better hw, even done in the hw)



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 08:06:13 +0100
Oliver Neukum <oliver@neukum.org> wrote:

> Am Monday 16 February 2009 03:20:12 schrieb Arjan van de Ven:
> > >or
> > > network or similar where the driver cannot shut the device down
> > > without losing the ability to detect activity (ie, switch your PHY
> > > off is nice but you lose the ability to monitor the link for
> > > example).
> > 
> > what this needs is a separation between "interface down, all off"
> > and "interface down enough so that link detection works".
> > Some NICs can go all the way down and still do link detection,
> > others cannot. This is obviously something the driver will know,
> > nothing else in the system should know.
> 
> Why shouldn't user space know the consequences of still requesting
> a service and be able to decide on that basis which services to
> request?

That's not what I said. Userspace should not be the part that goes "oh
this is a realtek not an intel nic, so ..."
That is the drivers job.

Userspace should just be the part that says "I'm interested in link
notifications". The driver then just goes to the lowest power state for
its device that still gives those notifications.


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Monday 16 February 2009 16:40:48 schrieb Arjan van de Ven:
> > Why shouldn't user space know the consequences of still requesting
> > a service and be able to decide on that basis which services to
> > request?
> 
> That's not what I said. Userspace should not be the part that goes "oh
> this is a realtek not an intel nic, so ..."
> That is the drivers job.

True.

> Userspace should just be the part that says "I'm interested in link
> notifications". The driver then just goes to the lowest power state for
> its device that still gives those notifications.

Should there be a channel through which drivers can tell user space
what the consequences in terms of reaching a sleep state are if a service
is requested?

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 17:48:06 +0100
Oliver Neukum <oliver@neukum.org> wrote:

> > Userspace should just be the part that says "I'm interested in link
> > notifications". The driver then just goes to the lowest power state
> > for its device that still gives those notifications.
> 
> Should there be a channel through which drivers can tell user space
> what the consequences in terms of reaching a sleep state are if a
> service is requested?

Personally, I don't think so.
Think about it: what would userspace do?
Not ask for the functionality if it means no sleep state?
Maybe it should just never ask for it then as a required functionality!



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Kyle Moffett]

On Mon, Feb 16, 2009 at 12:31 PM, Arjan van de Ven <arjan@infradead.org> wrote:
> On Mon, 16 Feb 2009 17:48:06 +0100 Oliver Neukum <oliver@neukum.org> wrote:
>> Should there be a channel through which drivers can tell user space
>> what the consequences in terms of reaching a sleep state are if a
>> service is requested?
>
> Personally, I don't think so.
> Think about it: what would userspace do?
> Not ask for the functionality if it means no sleep state?
> Maybe it should just never ask for it then as a required functionality!

Userspace may not have a useful decision-making capability regarding
functionality versus power-saving, but the user it's talking to might!

I think it would be very useful to have a mechanism for userspace to
find out "Link detection on this NIC costs ~50mW" and "No link has
been detected for the last 15 minutes", so that it can compile all
that sort of information and prompt the user what devices they would
like to disable.  Perhaps an "airline mode" sort of thing?

Cheers,
Kyle Moffett

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 15:08:09 -0500
Kyle Moffett <kyle@moffetthome.net> wrote:

> I think it would be very useful to have a mechanism for userspace to
> find out "Link detection on this NIC costs ~50mW" and "No link has
> been detected for the last 15 minutes", so that it can compile all
> that sort of information and prompt the user what devices they would
> like to disable.  Perhaps an "airline mode" sort of thing

modes suck... they're very much the wrong concept for power management
(but that's a different story)

if the user (or something on its behalf) doesn't need link detection,
he can turn it off. Really. He doesn't need to know that maybe on
machine one this saves some power, and on machine two that power was
already mostly saved while on machine three it doesn't save anything at
all. Not to mention that the driver is extremely unlikely to know how
much actual power will be saved...
 



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Alan Stern]

On Mon, 16 Feb 2009, Kyle Moffett wrote:

> Userspace may not have a useful decision-making capability regarding
> functionality versus power-saving, but the user it's talking to might!
> 
> I think it would be very useful to have a mechanism for userspace to
> find out "Link detection on this NIC costs ~50mW" and "No link has
> been detected for the last 15 minutes", so that it can compile all
> that sort of information and prompt the user what devices they would
> like to disable.  Perhaps an "airline mode" sort of thing?

Even if drivers don't know how much power their devices consume (and
generally I wouldn't expect them to), it still makes sense to offer
userspace the discretionary power to force devices into a low-power
mode.  USB does this.

Alan Stern

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 15:39:15 -0500 (EST)
Alan Stern <stern@rowland.harvard.edu> wrote:

> On Mon, 16 Feb 2009, Kyle Moffett wrote:
> 
> > Userspace may not have a useful decision-making capability regarding
> > functionality versus power-saving, but the user it's talking to
> > might!
> > 
> > I think it would be very useful to have a mechanism for userspace to
> > find out "Link detection on this NIC costs ~50mW" and "No link has
> > been detected for the last 15 minutes", so that it can compile all
> > that sort of information and prompt the user what devices they would
> > like to disable.  Perhaps an "airline mode" sort of thing?
> 
> Even if drivers don't know how much power their devices consume (and
> generally I wouldn't expect them to), it still makes sense to offer
> userspace the discretionary power to force devices into a low-power
> mode.  USB does this.

"stop using it" isn't this discretionary power ?
(for NICs this is "ifconfig .. down")

-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

RE: [RFD] Automatic suspend

[Woodruff, Richard]

> From: Arjan van de Ven [mailto:arjan@infradead.org]
> Sent: Monday, February 16, 2009 2:45 PM

> On Mon, 16 Feb 2009 15:39:15 -0500 (EST)
> Alan Stern <stern@rowland.harvard.edu> wrote:
>
> > On Mon, 16 Feb 2009, Kyle Moffett wrote:
> >
> > > Userspace may not have a useful decision-making capability regarding
> > > functionality versus power-saving, but the user it's talking to
> > > might!
> > >
> > > I think it would be very useful to have a mechanism for userspace to
> > > find out "Link detection on this NIC costs ~50mW" and "No link has
> > > been detected for the last 15 minutes", so that it can compile all
> > > that sort of information and prompt the user what devices they would
> > > like to disable.  Perhaps an "airline mode" sort of thing?
> >
> > Even if drivers don't know how much power their devices consume (and
> > generally I wouldn't expect them to), it still makes sense to offer
> > userspace the discretionary power to force devices into a low-power
> > mode.  USB does this.
>
> "stop using it" isn't this discretionary power ?
> (for NICs this is "ifconfig .. down")

Allowing a trusted user space process some hook in kernel PM has turned out useful for us.

- It gives a way for frame works which load codecs into DSPs a way to specify needs before loading. The DSP has its own internal dynamic predictor but it is not always good enough. Feeding this info in from user space is easy as it knows everything about the DSP algorithm it is loading. There isn't a reason to re-export all of that back into the kernel and might hurt DSP performance if it ran on it natively.

- It provides a way to handle overdrive/turbo operating points out of band from the generically tuned cpufreq governors like ondemand.  The way we characterize overdrive is stricter then what Intel has talked about for x86.

In TI-reference code we effectively export sysfs knobs which influence system p-state and c-state choices (we borrowed Intel terms here as they are useful). This was done before pmqos was available. We would have considered extending pmqos but a couple things stopped this. One being it wasn't in kernel at the time and two being any change picks up a year of disagreements. As such we put some front end wrappers on the calls which use it to some degree.

There is a community-reference targeting kernel merging but I think it is only now reaching the point of the user space interface. People working there have expressed interest in a direct pmqos interface and removing our wrapping calls.

What we have done more follows what happens when you have a complex driver/subsystem which has pieces in both users and kernel space.  It works ok for this.

This is ok for making sure the complex driver works well. It doesn't address all the other user space things going on which also need some kind of management so they don't sink system power. This is where wakelocks start to do something more generically useful.

Right now it seems no one is willing to own the whole system. It is easier to just say user space sucks and someone else should fix it...or my driver is fine it is someone else's problem. However, by not giving some kind of useful constraints interface to help control bad (or uncontrollable) elements the end result are non-competitive Linux systems or ugly out of tree vendor hacks. This is where wakelocks actually stand out.  They actually try and do something useful generically.

It seems the perfect is often the enemy of the good when it comes to Linux PM discussions.

Regards,
Richard W.

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Monday 16 February 2009, Arjan van de Ven wrote:
> On Mon, 16 Feb 2009 00:10:15 +0100
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > Hi,
> > 
> > The recent descussion about the Android PM patches sent by Arve shows
> > that there is a need to introduce a mechanism allowing us to:
> > (1) automatically put the system as a whole into a sleep state (eg.
> > suspend to RAM) when it is found to be "idle", where the meaning of
> > "idle" has to be defined too,
> > (2) put given subset of devices into low power states whenever they
> > are not used, without putting the entire system into a sleep state.
> 
> 
> For (2), for me the answer is very obvious:
> 
> The Device Driver needs to make the decision to put the device to sleep.
> There are no ifs and buts about this.
> 
> It's the driver who 
> a) knows if there's any activity, such as open users
> and 
> b) is in the right position to know how to put things to sleep.
> 
> Of course, the subsystem the driver belongs to can provide helpers
> (such as generic activity timeout handlers etc).

While I generally agree, there is a difficulty that the driver itself need not
have control over all of the bits needed to power manage the device.  For
example, we're going to make the PCI bus type handle the changing of PCI
devices' power states, so the registers used for this purpose will be
controlled by the PCI subsystem rather than by individual drivers.

In such cases we may need a mechanism by which the drivers can notify the
higher layer that the device is inactive, so that the higher layer can put it
into a low power state.

> For many cases, the drivers do this today already.
> There are cases where doing this has side effects, mostly in terms of
> latency. It is reasonable to have a general mechanism that provides a
> central mechanism to track tolerable latencies; in fact PMQOS provides
> this on a high level, and I can imagine that PMQOS needs to be extended
> to provide a wider range of type of latencies.

That indeed would be useful.

> Userland should never ever control the state of a device like this
> directly. It should do so by a) closing the device and b) setting
> latency / functional requirements.

The user, however, may want to forcibly put a device into a low power state
without stopping all of the applications that depend on it (eg. have it open).
Do you think that we shouldn't allow users to do such things?

Rafael

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 15:32:06 -0600
"Woodruff, Richard" <r-woodruff2@ti.com> wrote:
> 
> - It provides a way to handle overdrive/turbo operating points out of
> band from the generically tuned cpufreq governors like ondemand.  The
> way we characterize overdrive is stricter then what Intel has talked
> about for x86.

if you have an improved-for-your-systems governor then I'm sure that is
very welcome in the kernel.

I agree that perfect is the enemy of good.

I think just about all of us agree that the final decision needs to be
in the driver (possibly followed by something that gets various device
requests and combines it into hw settings); we're just talking about
what inputs feed into that decision ;)

And for different types of devices that's guaranteed to be different...
and sometimes we'll be hampered by existing interfaces, so we might end
up with hacky stuff.



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Peter Zijlstra]

On Mon, 2009-02-16 at 22:48 +0100, Rafael J. Wysocki wrote:
> > Userland should never ever control the state of a device like this
> > directly. It should do so by a) closing the device and b) setting
> > latency / functional requirements.
> 
> The user, however, may want to forcibly put a device into a low power state
> without stopping all of the applications that depend on it (eg. have it open).
> Do you think that we shouldn't allow users to do such things?

Sounds like a recipe for disaster. 

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 22:48:32 +0100
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> > Userland should never ever control the state of a device like this
> > directly. It should do so by a) closing the device and b) setting
> > latency / functional requirements.
> 
> The user, however, may want to forcibly put a device into a low power
> state without stopping all of the applications that depend on it (eg.
> have it open). Do you think that we shouldn't allow users to do such
> things?

I would think that that is a bad mistake to do. It basically breaks the 
contract that the kernel has with userspace.... 

-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Monday 16 February 2009, Arjan van de Ven wrote:
> On Mon, 16 Feb 2009 22:48:32 +0100
> "Rafael J. Wysocki" <rjw@sisk.pl> wrote:
> 
> > > Userland should never ever control the state of a device like this
> > > directly. It should do so by a) closing the device and b) setting
> > > latency / functional requirements.
> > 
> > The user, however, may want to forcibly put a device into a low power
> > state without stopping all of the applications that depend on it (eg.
> > have it open). Do you think that we shouldn't allow users to do such
> > things?
> 
> I would think that that is a bad mistake to do. It basically breaks the 
> contract that the kernel has with userspace.... 

OK, so I think there are two things that user space may be allowed to do as
far as putting devices into low power states is concerned:
* disable/enable the automatic power management of the device (provided that
  the driver supports the automatic PM)
* check what power states devices are in.

Thanks,
Rafael

RE: [RFD] Automatic suspend

[Woodruff, Richard]

> From: Arjan van de Ven [mailto:arjan@infradead.org]
> Sent: Monday, February 16, 2009 3:52 PM
> On Mon, 16 Feb 2009 15:32:06 -0600
> "Woodruff, Richard" <r-woodruff2@ti.com> wrote:
> >
> > - It provides a way to handle overdrive/turbo operating points out of
> > band from the generically tuned cpufreq governors like ondemand.  The
> > way we characterize overdrive is stricter then what Intel has talked
> > about for x86.
>
> if you have an improved-for-your-systems governor then I'm sure that is
> very welcome in the kernel.

No, the generic governors were not substantially improved. Not every customer is using cpufreq for DVFS. As such we went underneath it. If we had more community presence at the start time we might have also tried that.

> I think just about all of us agree that the final decision needs to be
> in the driver (possibly followed by something that gets various device
> requests and combines it into hw settings); we're just talking about
> what inputs feed into that decision ;)
>
> And for different types of devices that's guaranteed to be different...
> and sometimes we'll be hampered by existing interfaces, so we might end
> up with hacky stuff.

Is there some kind of roadmap which can be plotted which goes from course to more granular control?

Phase 1 (all or nothing): like wakelocks (minus back light): system-auto-suspend-on, system-auto-suspend-off.  The drivers can still veto as they do today.

Phase 2: Subsystem generic tunable, on say latency + bandwidth. Start with CPU then move to classes like USB.

Phase 3: ??? if needed add direct device control, hints on an extended fadvise(), and explicit control left to open/close and direct ioctls?

I don't know that 1,2,3 above even make sense.  However, the notion of actually plotting out a course with goals does as it will take a long time and it is good to get some benefit along the way.

Thanks,
Richard W.

Re: [RFD] Automatic suspend

[Alan Stern]

On Mon, 16 Feb 2009, Rafael J. Wysocki wrote:

> OK, so I think there are two things that user space may be allowed to do as
> far as putting devices into low power states is concerned:
> * disable/enable the automatic power management of the device (provided that
>   the driver supports the automatic PM)

Set the automatic PM parameters (idle timeout, state to go to, etc.).  
And disabling automatic PM altogether (effectively the same as setting
the idle timeout to infinity).

> * check what power states devices are in.

What about situations where we want to distinguish between the power 
state of the device itself and the power state of the link?  For a disk 
drive we may want to power the link on and off quite a lot, as that 
has low latency, but spinning the disk up and down takes a long time 
and so should have a longer idle-time value.

Alan Stern

Re: [RFD] Automatic suspend

[Pavel Machek]

On Sun 2009-02-15 22:23:04, Roland Dreier wrote:
>  > > (2) put given subset of devices into low power states whenever they
>  > > are not used, without putting the entire system into a sleep state.
> 
>  > For (2), for me the answer is very obvious:
> 
>  > The Device Driver needs to make the decision to put the device to sleep.
>  > There are no ifs and buts about this.
> 
> For PC-like systems this is probably all that needs to be said.  However
> for highly integrated SoC systems (as Android is obviously targeting)
> there is another level of complexity due to the interdependency among
> various devices... eg things like if I know the SD controller and the
> wifi chip are both asleep then I can put my SDIO controller to sleep;
> and if the SDIO controller and the NAND controller are both asleep then
> I can stop clock X and save more power; etc etc.
> 
> This is what the PowerOp/DPM work was all about.  Unfortunately that
> doesn't seem to have made much progress upstream.  But there's no doubt
> in my mind that we need some framework beyond individual drivers that
> manages the system's power as a whole.  And the current device tree is
> probably not sufficient -- eg the bus hierarchy of a device may not
> match up with the clock tree at all.

Well, that's why clock framework exists. But none  of this is visible
to userland. 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> > Userland should never ever control the state of a device like this
> > directly. It should do so by a) closing the device and b) setting
> > latency / functional requirements.
> 
> The user, however, may want to forcibly put a device into a low power state
> without stopping all of the applications that depend on it (eg. have it open).
> Do you think that we shouldn't allow users to do such things?

I don't think we should do this... of you want device to be powered
down, just don't keep it open. revoke should be enough :-).

...one execption here is the disk driver; unmounting filesystems to
let the disk spin down is probably too heavy.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 23:51:16 +0100
Pavel Machek <pavel@suse.cz> wrote:

> Hi!
> 
> > > Userland should never ever control the state of a device like this
> > > directly. It should do so by a) closing the device and b) setting
> > > latency / functional requirements.
> > 
> > The user, however, may want to forcibly put a device into a low
> > power state without stopping all of the applications that depend on
> > it (eg. have it open). Do you think that we shouldn't allow users
> > to do such things?
> 
> I don't think we should do this... of you want device to be powered
> down, just don't keep it open. revoke should be enough :-).
> 
> ...one execption here is the disk driver; unmounting filesystems to
> let the disk spin down is probably too heavy.

but spin down happens on non-activity ;)
it's more a slumber than a power down



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 17:40:10 -0500 (EST)
Alan Stern <stern@rowland.harvard.edu> wrote:

> On Mon, 16 Feb 2009, Rafael J. Wysocki wrote:
> 
> > OK, so I think there are two things that user space may be allowed
> > to do as far as putting devices into low power states is concerned:
> > * disable/enable the automatic power management of the device
> > (provided that the driver supports the automatic PM)
> 
> Set the automatic PM parameters (idle timeout, state to go to,
> etc.). And disabling automatic PM altogether (effectively the same as
> setting the idle timeout to infinity).
> 

shouldn't idle timeout etc be internal to the driver?
Yes policy preferences / constraints makes sense to communicate,
actual settings do not. For one they keep changing fast all the time
anyway.
 


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> The recent descussion about the Android PM patches sent by Arve shows that
> there is a need to introduce a mechanism allowing us to:
> (1) automatically put the system as a whole into a sleep state (eg. suspend to
>     RAM) when it is found to be "idle", where the meaning of "idle" has to be
>     defined too,

Well; I'd prefer to keep the interface very minimal.

If no devices are being used, and next wakeup is far enough in the
future, just put system to sleep. Long enough == so far away that
suspend/wakeup is short compared to that... like 20 seconds on PC.

Ok, it probably needs to be opt-in: echo auto-mem > /sys/power/state
should do the trick.

This will need either fixing all the applications not to poll, or
maybe someone SIGSTOP-ing apps that are in background and poll too
often.

> (2) put given subset of devices into low power states whenever they are not
>     used, without putting the entire system into a sleep state.

Arjan said this nicely...

									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 16:36:43 -0600
"Woodruff, Richard" <r-woodruff2@ti.com> wrote:

> 
> > From: Arjan van de Ven [mailto:arjan@infradead.org]
> > Sent: Monday, February 16, 2009 3:52 PM
> > On Mon, 16 Feb 2009 15:32:06 -0600
> > "Woodruff, Richard" <r-woodruff2@ti.com> wrote:
> > >
> > > - It provides a way to handle overdrive/turbo operating points
> > > out of band from the generically tuned cpufreq governors like
> > > ondemand.  The way we characterize overdrive is stricter then
> > > what Intel has talked about for x86.
> >
> > if you have an improved-for-your-systems governor then I'm sure
> > that is very welcome in the kernel.
> 
> No, the generic governors were not substantially improved. Not every
> customer is using cpufreq for DVFS. As such we went underneath it. If
> we had more community presence at the start time we might have also
> tried that.
> 
> > I think just about all of us agree that the final decision needs to
> > be in the driver (possibly followed by something that gets various
> > device requests and combines it into hw settings); we're just
> > talking about what inputs feed into that decision ;)
> >
> > And for different types of devices that's guaranteed to be
> > different... and sometimes we'll be hampered by existing
> > interfaces, so we might end up with hacky stuff.
> 
> Is there some kind of roadmap which can be plotted which goes from
> course to more granular control?
> 
> Phase 1 (all or nothing): like wakelocks (minus back light):
> system-auto-suspend-on, system-auto-suspend-off.  The drivers can
> still veto as they do today.
> 
> Phase 2: Subsystem generic tunable, on say latency + bandwidth. Start
> with CPU then move to classes like USB.

I think your phases are the wrong way around.
Phase 2 is already in progress and many of the "good" drivers and
subsystems already do them.


Whole system suspend is a much more drastic thing; I don't see that
happen generically in 2009 to be honest (not to mention how fragile
linux suspend is). In addition, in hardware there does seem to be the
trend where idle and suspend are converging (just look at the OLPC)...
with chipsets powering down all unused pieces, and cpus in idle taking
basically no power, there no longer is much of a difference in reality
between suspend and good-idle...


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Monday 16 February 2009, Pavel Machek wrote:
> Hi!
> 
> > > Userland should never ever control the state of a device like this
> > > directly. It should do so by a) closing the device and b) setting
> > > latency / functional requirements.
> > 
> > The user, however, may want to forcibly put a device into a low power state
> > without stopping all of the applications that depend on it (eg. have it open).
> > Do you think that we shouldn't allow users to do such things?
> 
> I don't think we should do this... of you want device to be powered
> down, just don't keep it open. revoke should be enough :-).
> 
> ...one execption here is the disk driver; unmounting filesystems to
> let the disk spin down is probably too heavy.

Well, I don't think it's viable to make any exceptions here.  Either we do it
or we don't.  And the agreement appears to be that we don't.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Monday 16 February 2009, Alan Stern wrote:
> On Mon, 16 Feb 2009, Rafael J. Wysocki wrote:
> 
> > OK, so I think there are two things that user space may be allowed to do as
> > far as putting devices into low power states is concerned:
> > * disable/enable the automatic power management of the device (provided that
> >   the driver supports the automatic PM)
> 
> Set the automatic PM parameters (idle timeout, state to go to, etc.).  

Yes.  I'm not sure about the state part, though.

> And disabling automatic PM altogether (effectively the same as setting
> the idle timeout to infinity).

Yes.

> > * check what power states devices are in.
> 
> What about situations where we want to distinguish between the power 
> state of the device itself and the power state of the link?  For a disk 
> drive we may want to power the link on and off quite a lot, as that 
> has low latency, but spinning the disk up and down takes a long time 
> and so should have a longer idle-time value.

Well, I'm not sure at the moment.

Do you have any suggestions?

Rafael

Re: [RFD] Automatic suspend

[Pavel Machek]

On Mon 2009-02-16 14:23:41, Benjamin Herrenschmidt wrote:
> On Sun, 2009-02-15 at 18:20 -0800, Arjan van de Ven wrote:
> > what this needs is a separation between "interface down, all off" and
> > "interface down enough so that link detection works".
> > Some NICs can go all the way down and still do link detection, others
> > cannot. This is obviously something the driver will know, nothing else
> > in the system should know.
> 
> Sure, my point was just that the driver cannot make the decision to go
> all-off (and thus not detect link) without some kind of user setting
> allowing it to do so.

Maybe we need ifconfig eth0 reallyreallydown ?
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Monday 16 February 2009, Woodruff, Richard wrote:
> 
> > From: Arjan van de Ven [mailto:arjan@infradead.org]
> > Sent: Monday, February 16, 2009 3:52 PM
> > On Mon, 16 Feb 2009 15:32:06 -0600
> > "Woodruff, Richard" <r-woodruff2@ti.com> wrote:
> > >
> > > - It provides a way to handle overdrive/turbo operating points out of
> > > band from the generically tuned cpufreq governors like ondemand.  The
> > > way we characterize overdrive is stricter then what Intel has talked
> > > about for x86.
> >
> > if you have an improved-for-your-systems governor then I'm sure that is
> > very welcome in the kernel.
> 
> No, the generic governors were not substantially improved. Not every customer is using cpufreq for DVFS. As such we went underneath it. If we had more community presence at the start time we might have also tried that.
> 
> > I think just about all of us agree that the final decision needs to be
> > in the driver (possibly followed by something that gets various device
> > requests and combines it into hw settings); we're just talking about
> > what inputs feed into that decision ;)
> >
> > And for different types of devices that's guaranteed to be different...
> > and sometimes we'll be hampered by existing interfaces, so we might end
> > up with hacky stuff.
> 
> Is there some kind of roadmap which can be plotted which goes from course to more granular control?
> 
> Phase 1 (all or nothing): like wakelocks (minus back light):
> system-auto-suspend-on, system-auto-suspend-off.  The drivers can still veto
> as they do today.
> 
> Phase 2: Subsystem generic tunable, on say latency + bandwidth. Start with
> CPU then move to classes like USB.
> 
> Phase 3: ??? if needed add direct device control, hints on an extended
> fadvise(), and explicit control left to open/close and direct ioctls?
> 
> I don't know that 1,2,3 above even make sense.

I'd like to talk about this. 

Phase 1: I agree that system-auto-suspend-on, system-auto-suspend-off would be
useful, but I don't like the wakelocks interface.  Do you think there is an
alternative way/mechanism of doing this?

Phase 2: I'm not sure.  We may want to introduce such things for various
subsystems in parallel (eg. USB already has such things).

Phase 3: Probably explicit control left to open/close.

> However, the notion of actually plotting out a course with goals does as it
> will take a long time and it is good to get some benefit along the way.

Agreed.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Matthew Garrett]

On Mon, Feb 16, 2009 at 11:58:31PM +0100, Pavel Machek wrote:

> If no devices are being used, and next wakeup is far enough in the
> future, just put system to sleep. Long enough == so far away that
> suspend/wakeup is short compared to that... like 20 seconds on PC.

This is intrinsically difficult with PCs, since we have such a poorly 
defined set of wakeup events. We can't wakeup on generic network 
traffic, just WoL. Many machines won't wake up on keyboard events. 
Meanwhile, on embedded it's becoming a less interesting problem because 
idle and suspended are often now equivalent states. Concentrating on 
runtime PM of as much hardware as possible is arguably more interesting 
for the majority of use cases.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFD] Automatic suspend

[Pavel Machek]

On Tue 2009-02-17 00:00:41, Rafael J. Wysocki wrote:
> On Monday 16 February 2009, Pavel Machek wrote:
> > Hi!
> > 
> > > > Userland should never ever control the state of a device like this
> > > > directly. It should do so by a) closing the device and b) setting
> > > > latency / functional requirements.
> > > 
> > > The user, however, may want to forcibly put a device into a low power state
> > > without stopping all of the applications that depend on it (eg. have it open).
> > > Do you think that we shouldn't allow users to do such things?
> > 
> > I don't think we should do this... of you want device to be powered
> > down, just don't keep it open. revoke should be enough :-).
> > 
> > ...one execption here is the disk driver; unmounting filesystems to
> > let the disk spin down is probably too heavy.
> 
> Well, I don't think it's viable to make any exceptions here.  Either we do it
> or we don't.  And the agreement appears to be that we don't.

We already have interface to force disk down (for disk protectioon
stuff), so this is already done for other reasons.

But in general, close(joystick) is nicer interface then 'echo
turn-off-joystick-even-when-open > /sys/somewhere'.
					

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Monday 16 February 2009, Arjan van de Ven wrote:
> On Mon, 16 Feb 2009 16:36:43 -0600
> "Woodruff, Richard" <r-woodruff2@ti.com> wrote:
> 
> > 
> > > From: Arjan van de Ven [mailto:arjan@infradead.org]
> > > Sent: Monday, February 16, 2009 3:52 PM
> > > On Mon, 16 Feb 2009 15:32:06 -0600
> > > "Woodruff, Richard" <r-woodruff2@ti.com> wrote:
> > > >
> > > > - It provides a way to handle overdrive/turbo operating points
> > > > out of band from the generically tuned cpufreq governors like
> > > > ondemand.  The way we characterize overdrive is stricter then
> > > > what Intel has talked about for x86.
> > >
> > > if you have an improved-for-your-systems governor then I'm sure
> > > that is very welcome in the kernel.
> > 
> > No, the generic governors were not substantially improved. Not every
> > customer is using cpufreq for DVFS. As such we went underneath it. If
> > we had more community presence at the start time we might have also
> > tried that.
> > 
> > > I think just about all of us agree that the final decision needs to
> > > be in the driver (possibly followed by something that gets various
> > > device requests and combines it into hw settings); we're just
> > > talking about what inputs feed into that decision ;)
> > >
> > > And for different types of devices that's guaranteed to be
> > > different... and sometimes we'll be hampered by existing
> > > interfaces, so we might end up with hacky stuff.
> > 
> > Is there some kind of roadmap which can be plotted which goes from
> > course to more granular control?
> > 
> > Phase 1 (all or nothing): like wakelocks (minus back light):
> > system-auto-suspend-on, system-auto-suspend-off.  The drivers can
> > still veto as they do today.
> > 
> > Phase 2: Subsystem generic tunable, on say latency + bandwidth. Start
> > with CPU then move to classes like USB.
> 
> I think your phases are the wrong way around.
> Phase 2 is already in progress and many of the "good" drivers and
> subsystems already do them.
> 
> 
> Whole system suspend is a much more drastic thing; I don't see that
> happen generically in 2009 to be honest (not to mention how fragile
> linux suspend is).

This applies to x86/ACPI mostly, IMO.  Embedded platforms appear to be much
more controllable and tend to work better w/ system-wide sleep states.

> In addition, in hardware there does seem to be the trend where idle and
> suspend are converging (just look at the OLPC)...   with chipsets powering
> down all unused pieces, and cpus in idle taking basically no power, there
> no longer is much of a difference in reality between suspend and good-idle...

This, again, seems to be a bit x86-centric. :-)  The Android people are telling
us that on the hardware they deal with it does make sense to put the entire
system to sleep even for relatively short periods of time, since the latencies
involved are not too bad.

While I agree that on a PC it would probably make more sense to go towards
fine-grained run-time power management, maybe even without using system-wide
suspend at all (I don't see this happen any time soon, though), I'm not sure
about the other platforms.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Monday 16 February 2009 18:31:08 schrieb Arjan van de Ven:
> On Mon, 16 Feb 2009 17:48:06 +0100
> Oliver Neukum <oliver@neukum.org> wrote:
> 
> > > Userspace should just be the part that says "I'm interested in link
> > > notifications". The driver then just goes to the lowest power state
> > > for its device that still gives those notifications.
> > 
> > Should there be a channel through which drivers can tell user space
> > what the consequences in terms of reaching a sleep state are if a
> > service is requested?
> 
> Personally, I don't think so.
> Think about it: what would userspace do?
> Not ask for the functionality if it means no sleep state?
> Maybe it should just never ask for it then as a required functionality!

1. You may have a human being using the information to make the tradeoff.
2. User space may be able to select among several devices the one cheapest
to use

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Pavel Machek]

On Mon 2009-02-16 23:13:24, Matthew Garrett wrote:
> On Mon, Feb 16, 2009 at 11:58:31PM +0100, Pavel Machek wrote:
> 
> > If no devices are being used, and next wakeup is far enough in the
> > future, just put system to sleep. Long enough == so far away that
> > suspend/wakeup is short compared to that... like 20 seconds on PC.
> 
> This is intrinsically difficult with PCs, since we have such a poorly 
> defined set of wakeup events. We can't wakeup on generic network 
> traffic, just WoL. 

At least e1000 in thinkpad can wakeup on generic network traffic. If
particular network card can't, you just can't sleep when network is up.

> Many machines won't wake up on keyboard events. 

Too bad. You'll have to close the lid for autosuspend to work on such
hw. (Or maybe you can do some hack telling the kernel to suspend
anyway if just keyboard prevents it.)

> Meanwhile, on embedded it's becoming a less interesting problem because 
> idle and suspended are often now equivalent states. 

Android people tell us otherwise.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Matthew Garrett]

On Tue, Feb 17, 2009 at 12:19:38AM +0100, Rafael J. Wysocki wrote:

> This, again, seems to be a bit x86-centric. :-)  The Android people are telling
> us that on the hardware they deal with it does make sense to put the entire
> system to sleep even for relatively short periods of time, since the latencies
> involved are not too bad.

Arve said that the power state was equivalent in idle and suspend, but 
that they preferred suspend because it stopped any periodic timers. I'd 
be more interested in making sure that unnecessary timers aren't running 
than focusing on automatically entering system-wide suspend - Nokia have 
been managing this since 2005 with good results.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Monday 16 February 2009, Pavel Machek wrote:
> Hi!
> 
> > The recent descussion about the Android PM patches sent by Arve shows that
> > there is a need to introduce a mechanism allowing us to:
> > (1) automatically put the system as a whole into a sleep state (eg. suspend to
> >     RAM) when it is found to be "idle", where the meaning of "idle" has to be
> >     defined too,
> 
> Well; I'd prefer to keep the interface very minimal.
> 
> If no devices are being used,

We'll need some kind of infrastructure to detect such situations.

> and next wakeup is far enough in the future,

What exactly do you mean by "next wakeup"?

> just put system to sleep.  Long enough == so far away that suspend/wakeup
> is short compared to that... like 20 seconds on PC.

Well 10s will be fine on the majority of my test boxes, but that's a detail.

> Ok, it probably needs to be opt-in: echo auto-mem > /sys/power/state
> should do the trick.

Agreed.

> This will need either fixing all the applications not to poll, or
> maybe someone SIGSTOP-ing apps that are in background and poll too
> often.

Polling in general is not PM-friendly, so we'll need to deal with it somehow
at one point anyway.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Matthew Garrett]

On Tue, Feb 17, 2009 at 12:22:59AM +0100, Pavel Machek wrote:
> On Mon 2009-02-16 23:13:24, Matthew Garrett wrote:
> > Meanwhile, on embedded it's becoming a less interesting problem because 
> > idle and suspended are often now equivalent states. 
> 
> Android people tell us otherwise.

https://lists.linux-foundation.org/pipermail/linux-pm/2009-January/019493.html 
says that they're the same on their current platform.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Monday 16 February 2009, Arjan van de Ven wrote:
> On Mon, 16 Feb 2009 17:40:10 -0500 (EST)
> Alan Stern <stern@rowland.harvard.edu> wrote:
> 
> > On Mon, 16 Feb 2009, Rafael J. Wysocki wrote:
> > 
> > > OK, so I think there are two things that user space may be allowed
> > > to do as far as putting devices into low power states is concerned:
> > > * disable/enable the automatic power management of the device
> > > (provided that the driver supports the automatic PM)
> > 
> > Set the automatic PM parameters (idle timeout, state to go to,
> > etc.). And disabling automatic PM altogether (effectively the same as
> > setting the idle timeout to infinity).
> > 
> 
> shouldn't idle timeout etc be internal to the driver?
> Yes policy preferences / constraints makes sense to communicate,
> actual settings do not. For one they keep changing fast all the time
> anyway.

Usually different devices are handled by the same driver.  I don't really
think the same value will be suitable for all of them, so the users should
at least be able to override the driver default, IMO.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Monday 16 February 2009 23:51:16 schrieb Pavel Machek:
> > The user, however, may want to forcibly put a device into a low power state
> > without stopping all of the applications that depend on it (eg. have it open).
> > Do you think that we shouldn't allow users to do such things?
> 
> I don't think we should do this... of you want device to be powered
> down, just don't keep it open. revoke should be enough :-).

Keeping a device open also guarantees identity. Secondly you may
not have opened a device yourself and indeed be unable to do so.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Tue, 17 Feb 2009 00:19:38 +0100
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> > In addition, in hardware there does seem to be the trend where idle
> > and suspend are converging (just look at the OLPC)...   with
> > chipsets powering down all unused pieces, and cpus in idle taking
> > basically no power, there no longer is much of a difference in
> > reality between suspend and good-idle...
> 
> This, again, seems to be a bit x86-centric. :-)  

actually absolutely not. To the contrary. It is, as you said, the
embedded systems that have, and increasingly get, finer grained per
device controls. (x86 embedded and otherwise). The more fine grained
control you have, the less a system wide state will give you in
additional power savings. This is pretty much the industry trend for
all power sensitive systems.

Android might be using last years hardware. I'm not saying we shouldn't
support that, but we need to be careful to not design the system only
looking backwards.



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Alan Stern]

On Mon, 16 Feb 2009, Arjan van de Ven wrote:

> > Set the automatic PM parameters (idle timeout, state to go to,
> > etc.). And disabling automatic PM altogether (effectively the same as
> > setting the idle timeout to infinity).
> > 
> 
> shouldn't idle timeout etc be internal to the driver?
> Yes policy preferences / constraints makes sense to communicate,
> actual settings do not. For one they keep changing fast all the time
> anyway.

Sorry, I don't agree.  A single driver may control many different kinds
of device -- it may not even be aware of the distinction!  Consider the
SCSI disk driver: It has to handle both traditional rotating media and
solid-state disks.  Clearly they should have different runtime PM
parameters.  But the driver isn't in a good position to know what those
parameters should be.

Alan Stern

Re: [RFD] Automatic suspend

[Alan Stern]

On Tue, 17 Feb 2009, Rafael J. Wysocki wrote:

> On Monday 16 February 2009, Alan Stern wrote:
> > On Mon, 16 Feb 2009, Rafael J. Wysocki wrote:
> > 
> > > OK, so I think there are two things that user space may be allowed to do as
> > > far as putting devices into low power states is concerned:
> > > * disable/enable the automatic power management of the device (provided that
> > >   the driver supports the automatic PM)
> > 
> > Set the automatic PM parameters (idle timeout, state to go to, etc.).  
> 
> Yes.  I'm not sure about the state part, though.

Maybe, maybe not.  IMO it's too early to tell whether anyone will need 
this ability, so we shouldn't rule it out.

> > What about situations where we want to distinguish between the power 
> > state of the device itself and the power state of the link?  For a disk 
> > drive we may want to power the link on and off quite a lot, as that 
> > has low latency, but spinning the disk up and down takes a long time 
> > and so should have a longer idle-time value.
> 
> Well, I'm not sure at the moment.
> 
> Do you have any suggestions?

Not very well fleshed-out ones.  I've got a vague idea for allowing a
disk to have a 3-level power arrangement: full power, link disabled but
drive still spinning, and device suspended.  Arranging for automatic
transitions among those states will be a little clumsy but it can be
done.  As an example of the clumsiness, this scheme requires that the
drive has _two_ idle-timeout values, one for the link and one for the
drive itself.

Another possibility is to set up independent runtime PM for the
transport and the device.  This means allowing the possibility that the
transport is suspended while its child (the device) is not.  This is a
little simpler (there's only one idle-timeout per device, since the
link is treated as an independent device), but it violates the
principle of never suspending a parent while there is an active child.

Alan Stern

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 16 Feb 2009 21:43:52 -0500 (EST)
Alan Stern <stern@rowland.harvard.edu> wrote:

> On Mon, 16 Feb 2009, Arjan van de Ven wrote:
> 
> > > Set the automatic PM parameters (idle timeout, state to go to,
> > > etc.). And disabling automatic PM altogether (effectively the
> > > same as setting the idle timeout to infinity).
> > > 
> > 
> > shouldn't idle timeout etc be internal to the driver?
> > Yes policy preferences / constraints makes sense to communicate,
> > actual settings do not. For one they keep changing fast all the time
> > anyway.
> 
> Sorry, I don't agree.  A single driver may control many different
> kinds of device -- it may not even be aware of the distinction!
> Consider the SCSI disk driver: It has to handle both traditional
> rotating media and solid-state disks.  Clearly they should have
> different runtime PM parameters.  But the driver isn't in a good
> position to know what those parameters should be.

... and userspace is ?
Sorry I don't buy that. Kernel is supposed to abstract the hardware...
That's its fundamental task. Now if the driver doesn't know it can get
help from the subsystem, that's perfectly fine.


(and quiet often the kernel gets updated more often as the userspace)


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Alan Stern]

On Tue, 17 Feb 2009, Rafael J. Wysocki wrote:

> Phase 1: I agree that system-auto-suspend-on, system-auto-suspend-off would be
> useful, but I don't like the wakelocks interface.  Do you think there is an
> alternative way/mechanism of doing this?

I rather like the suggestions Matthew Garrett has been making.  They 
show how to improve the wakelock interface without losing any function.

Really, the idea behind wakelocks comes down to the question of how to
determine when the system is sufficiently idle to go into auto-suspend.  
There may be occasions when no task is runnable but userspace knows
that the system should not go to sleep because some work will be done
in the near future.  (Arve's example of a non-empty input buffer is
such a case.)  How should userspace let the kernel know whether it's
okay to suspend at these times?  That is the problem userspace
wakelocks are meant to solve.

Kernel wakelocks are a separate matter.  They are more like a form of 
optimization, preventing the kernel from starting an auto-suspend when 
some driver knows beforehand that it will return -EBUSY.

> Phase 3: Probably explicit control left to open/close.

While that's generally a good idea, it's important to recognize that 
some devices should be runtime-suspended even while they are open.  
Basically, any device that is "always open" falls in this category.  
Some examples are the screen, the keyboard, the mouse, and disk drives.  
And of course, some of these things already have runtime power 
management.

Alan Stern

Re: [RFD] Automatic suspend

[Alan Stern]

On Mon, 16 Feb 2009, Arjan van de Ven wrote:

> > > shouldn't idle timeout etc be internal to the driver?
> > > Yes policy preferences / constraints makes sense to communicate,
> > > actual settings do not. For one they keep changing fast all the time
> > > anyway.
> > 
> > Sorry, I don't agree.  A single driver may control many different
> > kinds of device -- it may not even be aware of the distinction!
> > Consider the SCSI disk driver: It has to handle both traditional
> > rotating media and solid-state disks.  Clearly they should have
> > different runtime PM parameters.  But the driver isn't in a good
> > position to know what those parameters should be.
> 
> ... and userspace is ?

Certainly it is.  Programs like hal can maintain databases of device 
IDs and appropriate timeout values, and allow the user to change the 
values as needed.  Such databases don't belong in the kernel.

Furthermore, the user may have a better idea of the kind of workload he
wants to impose and his latency requirements than the kernel does.

> Sorry I don't buy that. Kernel is supposed to abstract the hardware...
> That's its fundamental task. Now if the driver doesn't know it can get
> help from the subsystem, that's perfectly fine.

There are various levels of abstraction.  If the kernel really 
abstracted _everything_ then we wouldn't need ioctl.  :-)

> (and quiet often the kernel gets updated more often as the userspace)

(That's hardly a valid argument for putting something into the kernel
when it belongs in userspace.)

Alan Stern

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Tuesday 17 February 2009 00:23:30 schrieb Matthew Garrett:
> On Tue, Feb 17, 2009 at 12:19:38AM +0100, Rafael J. Wysocki wrote:
> 
> > This, again, seems to be a bit x86-centric. :-)  The Android people are telling
> > us that on the hardware they deal with it does make sense to put the entire
> > system to sleep even for relatively short periods of time, since the latencies
> > involved are not too bad.
> 
> Arve said that the power state was equivalent in idle and suspend, but 
> that they preferred suspend because it stopped any periodic timers. I'd 
> be more interested in making sure that unnecessary timers aren't running 
> than focusing on automatically entering system-wide suspend - Nokia have 
> been managing this since 2005 with good results.

I agree. For a general purpose system it would be very hard to call
some timers unnecessary and others necessary.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Mark Brown]

On Mon, Feb 16, 2009 at 11:23:30PM +0000, Matthew Garrett wrote:

> Arve said that the power state was equivalent in idle and suspend, but 
> that they preferred suspend because it stopped any periodic timers. I'd 
> be more interested in making sure that unnecessary timers aren't running 
> than focusing on automatically entering system-wide suspend - Nokia have 
> been managing this since 2005 with good results.

Definitely.  Even in the embedded case there is hardware which takes a
relatively long time to fully power down and resume without winning an
enormous amount on the power front (analogue stuff is a common culprit
here, it can take a noticable amount of time to bring it up cleanly).

Re: [RFD] Automatic suspend

[Brian Swetland]

[Matthew Garrett <mjg59@srcf.ucam.org>]
> On Tue, Feb 17, 2009 at 12:22:59AM +0100, Pavel Machek wrote:
> > On Mon 2009-02-16 23:13:24, Matthew Garrett wrote:
> > > Meanwhile, on embedded it's becoming a less interesting problem because 
> > > idle and suspended are often now equivalent states. 
> > 
> > Android people tell us otherwise.
> 
> https://lists.linux-foundation.org/pipermail/linux-pm/2009-January/019493.html 
> says that they're the same on their current platform.

Well on the Qualcomm MSM72xx SoCs, yeah they're pretty similar -- we can
reasonably enter power collapse (full shutdown of the ARM11 and
peripherals) from idle.  But this is not (to my knowledge) true of *all*
SoCs out there.  Also, resuming from this state can take 5-30ms
(depending on how distracted the baseband processor is by what's going
on on the network).

Brian

Re: [RFD] Automatic suspend

[Matthew Garrett]

On Tue, Feb 17, 2009 at 06:14:34AM -0800, Brian Swetland wrote:
> [Matthew Garrett <mjg59@srcf.ucam.org>]
> > https://lists.linux-foundation.org/pipermail/linux-pm/2009-January/019493.html 
> > says that they're the same on their current platform.
> 
> Well on the Qualcomm MSM72xx SoCs, yeah they're pretty similar -- we can
> reasonably enter power collapse (full shutdown of the ARM11 and
> peripherals) from idle.  But this is not (to my knowledge) true of *all*
> SoCs out there.  Also, resuming from this state can take 5-30ms
> (depending on how distracted the baseband processor is by what's going
> on on the network).

Mm. Resume from deep idle on x86 is typically in the usec range, but 
this is the kind of constraint that pm_qos exposes nicely.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFD] Automatic suspend

[Brian Swetland]

[Matthew Garrett <mjg59@srcf.ucam.org>]
> On Tue, Feb 17, 2009 at 12:19:38AM +0100, Rafael J. Wysocki wrote:
> 
> > This, again, seems to be a bit x86-centric. :-)  The Android people are telling
> > us that on the hardware they deal with it does make sense to put the entire
> > system to sleep even for relatively short periods of time, since the latencies
> > involved are not too bad.
> 
> Arve said that the power state was equivalent in idle and suspend, but 
> that they preferred suspend because it stopped any periodic timers. I'd 
> be more interested in making sure that unnecessary timers aren't running 
> than focusing on automatically entering system-wide suspend - Nokia have 
> been managing this since 2005 with good results.

We'd totally agree that doing something about periodic timers would be a
big win.  There's also the situation that the longest ARM linux can sit
in idle right now is ~2s at a time (Arve can expand on the exact issue
relating to a 32bit signed nanosecond value somewhere iirc), which we'd
want to sort out as well.

Of course that still doesn't address userspace.  Aggressively going to
suspend lets us compensate for userspace programs that do somewhat silly
things (I agree that it would be best if they didn't but they do and
getting *everyone* to write their userspace code to avoid spinning or
avoid waking up on short-duration timers to poll is a losing battle).

Brian

Re: [RFD] Automatic suspend

[Matthew Garrett]

On Tue, Feb 17, 2009 at 06:20:01AM -0800, Brian Swetland wrote:

> Of course that still doesn't address userspace.  Aggressively going to
> suspend lets us compensate for userspace programs that do somewhat silly
> things (I agree that it would be best if they didn't but they do and
> getting *everyone* to write their userspace code to avoid spinning or
> avoid waking up on short-duration timers to poll is a losing battle).

Like Pavel pointed out, you could (in principle) handle this by sending 
a SIGSTOP to everything. The obvious problem with doing so is that this 
wouldn't result in the processes letting go of the devices, so you 
wouldn't neccessarily actually get to enter the runtime idle state as a 
result. Hmm. I'll think about this.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Tue, 17 Feb 2009 06:20:01 -0800
Brian Swetland <swetland@google.com> wrote:

> Of course that still doesn't address userspace.  Aggressively going to
> suspend lets us compensate for userspace programs that do somewhat
> silly things (I agree that it would be best if they didn't but they
> do and getting *everyone* to write their userspace code to avoid
> spinning or avoid waking up on short-duration timers to poll is a
> losing battle).

actually with powertop... on the open source side things are actually
won. It took all of 6 months...
I don't see that as a valid excuse. In fact, if this kind of solution
makes real userspace scheduled timers to be missed then I consider it a
serious functionality misfeature.


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Matthew Garrett]

On Tue, Feb 17, 2009 at 06:46:30AM -0800, Arjan van de Ven wrote:

> actually with powertop... on the open source side things are actually
> won. It took all of 6 months...
> I don't see that as a valid excuse. In fact, if this kind of solution
> makes real userspace scheduled timers to be missed then I consider it a
> serious functionality misfeature.

Remember that Android has an open marketplace designed to appeal to Java 
programmers - users are going to end up downloading code from there and 
then blaming the platform if their battery life heads towards zero. I 
think "We can't trust our userland not to be dumb" is a valid concern.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Tue, 17 Feb 2009 14:51:41 +0000
Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Tue, Feb 17, 2009 at 06:46:30AM -0800, Arjan van de Ven wrote:
> 
> > actually with powertop... on the open source side things are
> > actually won. It took all of 6 months...
> > I don't see that as a valid excuse. In fact, if this kind of
> > solution makes real userspace scheduled timers to be missed then I
> > consider it a serious functionality misfeature.
> 
> Remember that Android has an open marketplace designed to appeal to
> Java programmers - users are going to end up downloading code from
> there and then blaming the platform if their battery life heads
> towards zero. I think "We can't trust our userland not to be dumb" is
> a valid concern.

so use range timers / timer slack for those apps that you do not trust.
That is not a big deal, and solves the issue of timer wakeups...


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Tuesday 17 February 2009 15:24:53 schrieb Matthew Garrett:
> On Tue, Feb 17, 2009 at 06:20:01AM -0800, Brian Swetland wrote:
> 
> > Of course that still doesn't address userspace.  Aggressively going to
> > suspend lets us compensate for userspace programs that do somewhat silly
> > things (I agree that it would be best if they didn't but they do and
> > getting *everyone* to write their userspace code to avoid spinning or
> > avoid waking up on short-duration timers to poll is a losing battle).
> 
> Like Pavel pointed out, you could (in principle) handle this by sending 
> a SIGSTOP to everything. The obvious problem with doing so is that this 
> wouldn't result in the processes letting go of the devices, so you 
> wouldn't neccessarily actually get to enter the runtime idle state as a 
> result. Hmm. I'll think about this.

They don't let go of the devices if you suspend either. If you want that
you need active cooperation from user space. In fact if you suspend you
give drivers the guarantee of not getting any output without prior
notification, making their lives easier.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Igor Stoppa]

Hi,
On Tue, 2009-02-17 at 15:51 +0100, ext Matthew Garrett wrote:
> On Tue, Feb 17, 2009 at 06:46:30AM -0800, Arjan van de Ven wrote:

> Remember that Android has an open marketplace designed to appeal to Java
> programmers - users are going to end up downloading code from there and
> then blaming the platform if their battery life heads towards zero. I
> think "We can't trust our userland not to be dumb" is a valid concern.

It took few years of preaching but eventually our (admittedly small)
userbase did start to see the truth and not blame the platform but
rather the applications they had installed on top of it.

Powertop is a good tool.

Maybe slamming a nice GUI on top of that would turn all the userbase
into a horde of bug reporters for the offending apps.

Or maybe it's just wishful thinking ....  :) 

-- 

Cheers, Igor

---

Igor Stoppa
Maemo Software - Nokia Devices R&D - Helsinki

Re: [RFD] Automatic suspend

[Brian Swetland]

[Arjan van de Ven <arjan@infradead.org>]
> On Tue, 17 Feb 2009 06:20:01 -0800
> Brian Swetland <swetland@google.com> wrote:
> 
> > Of course that still doesn't address userspace.  Aggressively going to
> > suspend lets us compensate for userspace programs that do somewhat
> > silly things (I agree that it would be best if they didn't but they
> > do and getting *everyone* to write their userspace code to avoid
> > spinning or avoid waking up on short-duration timers to poll is a
> > losing battle).
> 
> actually with powertop... on the open source side things are actually
> won. It took all of 6 months...
> I don't see that as a valid excuse. In fact, if this kind of solution
> makes real userspace scheduled timers to be missed then I consider it a
> serious functionality misfeature.

While you can't expect the kernel to solve all the problems of
userspace, here's the broad situation one could end up in
(note this specific sequence is generic and not based on any one
specific product experience):

- carrier deploys a device 
- carrier agrees to allow installation of arbitrary third party apps
  without some horrible certification program requiring app authors
  to jump through hoops, wait ages for approval, etc
- users rejoice and install all kinds of apps
- some apps are poorly written and impact battery life
- users complain to carrier about battery life

You will end up with some crappy apps that do really dumb things.
However, even if they're badly written users may still install and use
these apps because hey, they do something the user likes.

>From the Android standpoint, we're trying to balance protecting the
system from poorly designed apps and somehow letting the user know "hey
app X is chewing up a lot of power" (work in progress on this).

While I'd love for every app developer to actively tune their apps for a
good mobile experience, I am skeptical that this is going to happen.

Brian

RE: [RFD] Automatic suspend

[Woodruff, Richard]

> From: Arjan van de Ven [mailto:arjan@infradead.org]
> Sent: Tuesday, February 17, 2009 8:56 AM

> On Tue, 17 Feb 2009 14:51:41 +0000
> Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>
> > On Tue, Feb 17, 2009 at 06:46:30AM -0800, Arjan van de Ven wrote:
> >
> > > actually with powertop... on the open source side things are
> > > actually won. It took all of 6 months...
> > > I don't see that as a valid excuse. In fact, if this kind of
> > > solution makes real userspace scheduled timers to be missed then I
> > > consider it a serious functionality misfeature.

Maybe for the optimization time scales x86 is targeting.  User space across OS still throws off our MHz a good distance from ideal estimates.

I still hear complaints about frequent wakes even when using a bare minimum user space which can get 95+% lowest C-state residency with average 700mS.  If you frequency scale in lowest C-State YMMV if your external power chip is slow.  Waiting on voltage ramp times is lost power.

One of our Application people (application is low level here) went though and tweaked kernel in a few spots with minimum user space and could get (this is with out a meaningful UI on NFS root mounted system):

1st set of hacks got:
Cn Avg residency
C0 (cpu running) ( 0.0%)
C1 0.0ms ( 0.0%)
C2 0.0ms ( 0.0%)
C3 217.2ms ( 1.8%)
C4 0.0ms ( 0.0%)
C5 0.0ms ( 0.0%)
C6 354.9ms ( 0.3%)
C7 1953.4ms (97.8%)

Second set got:
# powertop -d -t 300
PowerTOP 1.10 (C) 2007 Intel Corporation
Collecting data for 300 seconds
Cn Avg residency
C0 (cpu running) ( 0.0%)
C1 0.0ms ( 0.0%)
C2 0.2ms ( 0.0%)
C3 3.8ms ( 0.0%)
C4 0.0ms ( 0.0%)
C5 0.0ms ( 0.0%)
C6 0.0ms ( 0.0%)
C7 73165.9ms (100.0%)

I'll see about posting his change write up.  Not all probably can be done but several could.

> > Remember that Android has an open marketplace designed to appeal to
> > Java programmers - users are going to end up downloading code from
> > there and then blaming the platform if their battery life heads
> > towards zero. I think "We can't trust our userland not to be dumb" is
> > a valid concern.
>
> so use range timers / timer slack for those apps that you do not trust.
> That is not a big deal, and solves the issue of timer wakeups...

I not so sure it is that straight forward in practice.  End systems integrate a lot of 3rd party software who view performance 1st and have no thought of power.

It might be android has a better chance to control more of those api's being exported but its coverage isn't complete and others gluing pieces together won't.

Regards,
Richard W.

Re: [RFD] Automatic suspend

[mark gross]

On Mon, Feb 16, 2009 at 11:13:24PM +0000, Matthew Garrett wrote:
> On Mon, Feb 16, 2009 at 11:58:31PM +0100, Pavel Machek wrote:
> 
> > If no devices are being used, and next wakeup is far enough in the
> > future, just put system to sleep. Long enough == so far away that
> > suspend/wakeup is short compared to that... like 20 seconds on PC.
> 
> This is intrinsically difficult with PCs, since we have such a poorly 
> defined set of wakeup events. We can't wakeup on generic network 
> traffic, just WoL. Many machines won't wake up on keyboard events. 
> Meanwhile, on embedded it's becoming a less interesting problem because 
> idle and suspended are often now equivalent states. Concentrating on 
> runtime PM of as much hardware as possible is arguably more interesting 
> for the majority of use cases.

Putting the wake on key event issue aside, is it possible to have wake
up's on the ms time scale?  I ask because I thought the XO did exactly
this (but left the screen live).  Why does it take 20 sec to get into
or out of S3 on my laptop?

--mgross

> 
> -- 
> Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFD] Automatic suspend

[Matthew Garrett]

On Tue, Feb 17, 2009 at 01:57:17PM -0800, mark gross wrote:

> Putting the wake on key event issue aside, is it possible to have wake
> up's on the ms time scale?  I ask because I thought the XO did exactly
> this (but left the screen live).  Why does it take 20 sec to get into
> or out of S3 on my laptop?

Graphics reinit, dumping graphics contents back into RAM, us resuming 
devices in series, that kind of thing. On some hardware you'll spend a 
noticable amount of time in the BIOS before any of the Linux resume code 
gets touched. I thought the XO had got sub second, but I wasn't sure 
that they were in the low ms range.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFD] Automatic suspend

[Jesse Barnes]

On Tuesday, February 17, 2009 2:04 pm Matthew Garrett wrote:
> On Tue, Feb 17, 2009 at 01:57:17PM -0800, mark gross wrote:
> > Putting the wake on key event issue aside, is it possible to have wake
> > up's on the ms time scale?  I ask because I thought the XO did exactly
> > this (but left the screen live).  Why does it take 20 sec to get into
> > or out of S3 on my laptop?
>
> Graphics reinit, dumping graphics contents back into RAM, us resuming
> devices in series, that kind of thing. On some hardware you'll spend a
> noticable amount of time in the BIOS before any of the Linux resume code
> gets touched. I thought the XO had got sub second, but I wasn't sure
> that they were in the low ms range.

IME a good chunk of it is BIOS time.  On my x200s resume is very fast (on the 
order of a second or two though I haven't measured), while on my Eee and T61 
machines it's much slower, even though they're all using Intel gfx.

Of course we should really be shooting for sub-second times or about the time 
it takes you to open your lid (or even much faster in the case of demand 
suspend/resume for servers/desktops).

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Tuesday 17 February 2009, Alan Stern wrote:
> On Tue, 17 Feb 2009, Rafael J. Wysocki wrote:
> 
> > Phase 1: I agree that system-auto-suspend-on, system-auto-suspend-off would be
> > useful, but I don't like the wakelocks interface.  Do you think there is an
> > alternative way/mechanism of doing this?
> 
> I rather like the suggestions Matthew Garrett has been making.  They 
> show how to improve the wakelock interface without losing any function.
> 
> Really, the idea behind wakelocks comes down to the question of how to
> determine when the system is sufficiently idle to go into auto-suspend.  
> There may be occasions when no task is runnable but userspace knows
> that the system should not go to sleep because some work will be done
> in the near future.  (Arve's example of a non-empty input buffer is
> such a case.)  How should userspace let the kernel know whether it's
> okay to suspend at these times?  That is the problem userspace
> wakelocks are meant to solve.

Still, do we really need multiple user space wakelocks (I'd prefer to call them
sleeplocks)?  It seems that one such lock and a user space manager controlling
it should be sufficient.

> Kernel wakelocks are a separate matter.  They are more like a form of 
> optimization, preventing the kernel from starting an auto-suspend when 
> some driver knows beforehand that it will return -EBUSY.

I think kernel-side autosuspend (or rather autosleep) should only happen
after certain subset of devices have been suspended using a per-device
run-time autosuspend mechanism.

> > Phase 3: Probably explicit control left to open/close.
> 
> While that's generally a good idea, it's important to recognize that 
> some devices should be runtime-suspended even while they are open.

>From the kernel side, yes (and that should be transparent to the user space
having them open).  By the user space, no.
 
> Basically, any device that is "always open" falls in this category.  
> Some examples are the screen, the keyboard, the mouse, and disk drives.  
> And of course, some of these things already have runtime power 
> management.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Tuesday 17 February 2009, Alan Stern wrote:
> On Tue, 17 Feb 2009, Rafael J. Wysocki wrote:
> 
> > On Monday 16 February 2009, Alan Stern wrote:
> > > On Mon, 16 Feb 2009, Rafael J. Wysocki wrote:
> > > 
> > > > OK, so I think there are two things that user space may be allowed to do as
> > > > far as putting devices into low power states is concerned:
> > > > * disable/enable the automatic power management of the device (provided that
> > > >   the driver supports the automatic PM)
> > > 
> > > Set the automatic PM parameters (idle timeout, state to go to, etc.).  
> > 
> > Yes.  I'm not sure about the state part, though.
> 
> Maybe, maybe not.  IMO it's too early to tell whether anyone will need 
> this ability, so we shouldn't rule it out.
> 
> > > What about situations where we want to distinguish between the power 
> > > state of the device itself and the power state of the link?  For a disk 
> > > drive we may want to power the link on and off quite a lot, as that 
> > > has low latency, but spinning the disk up and down takes a long time 
> > > and so should have a longer idle-time value.
> > 
> > Well, I'm not sure at the moment.
> > 
> > Do you have any suggestions?
> 
> Not very well fleshed-out ones.  I've got a vague idea for allowing a
> disk to have a 3-level power arrangement: full power, link disabled but
> drive still spinning, and device suspended.

I was thinking about that too.

> Arranging for automatic transitions among those states will be a little
> clumsy but it can be done.  As an example of the clumsiness, this scheme
> requires that the drive has _two_ idle-timeout values, one for the link and
> one for the drive itself.

Well, this generally is the case for PCI devices supporting more than two
power states.

> Another possibility is to set up independent runtime PM for the
> transport and the device.  This means allowing the possibility that the
> transport is suspended while its child (the device) is not.  This is a
> little simpler (there's only one idle-timeout per device, since the
> link is treated as an independent device), but it violates the
> principle of never suspending a parent while there is an active child.

Well, I think the first approach would be better.

Thanks,
Rafael

RE: [RFD] Automatic suspend

[Woodruff, Richard]

> From: Jesse Barnes [mailto:jbarnes@virtuousgeek.org]
> Sent: Tuesday, February 17, 2009 4:19 PM

> On Tuesday, February 17, 2009 2:04 pm Matthew Garrett wrote:
> > On Tue, Feb 17, 2009 at 01:57:17PM -0800, mark gross wrote:
> > > Putting the wake on key event issue aside, is it possible to have wake
> > > up's on the ms time scale?  I ask because I thought the XO did exactly
> > > this (but left the screen live).  Why does it take 20 sec to get into
> > > or out of S3 on my laptop?
> >
> > Graphics reinit, dumping graphics contents back into RAM, us resuming
> > devices in series, that kind of thing. On some hardware you'll spend a
> > noticable amount of time in the BIOS before any of the Linux resume code
> > gets touched. I thought the XO had got sub second, but I wasn't sure
> > that they were in the low ms range.
>
> IME a good chunk of it is BIOS time.  On my x200s resume is very fast (on the
> order of a second or two though I haven't measured), while on my Eee and T61
> machines it's much slower, even though they're all using Intel gfx.
>
> Of course we should really be shooting for sub-second times or about the time
> it takes you to open your lid (or even much faster in the case of demand
> suspend/resume for servers/desktops).

This is good example of embedded ARM time scales not matching up with x86 ones.  For OMAP3 our resumption from OFF mode (full SOC context lost) software budget was in the 10s of milliSeconds.  There is no BIOS and we must generate most of the code.

As such very frequent fast OFF mode transitions are useful and possible.  As I said in another part of the thread, the slow part is Voltage ramp time. Also clock setup time of external oscillator can be 5-10mS by itself.

This tight timing was the requirement of some modems which required a response ASAP when data was coming.  Not every modem is so tight but some are.

Regards,
Richard W.

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Tue, 17 Feb 2009 09:32:46 -0600
"Woodruff, Richard" <r-woodruff2@ti.com> wrote:

> > so use range timers / timer slack for those apps that you do not
> > trust. That is not a big deal, and solves the issue of timer
> > wakeups...
> 
> I not so sure it is that straight forward in practice.  End systems
> integrate a lot of 3rd party software who view performance 1st and
> have no thought of power.

you know that with the range timers/slack, you can control the
"rounding" of the timer of the application, right?
You can *directly* throttle the number of wakeups an application causes
that way to a value you set.




-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

RE: [RFD] Automatic suspend

[Woodruff, Richard]

> On Tue, 17 Feb 2009 09:32:46 -0600
> "Woodruff, Richard" <r-woodruff2@ti.com> wrote:
>
> > > so use range timers / timer slack for those apps that you do not
> > > trust. That is not a big deal, and solves the issue of timer
> > > wakeups...
> >
> > I not so sure it is that straight forward in practice.  End systems
> > integrate a lot of 3rd party software who view performance 1st and
> > have no thought of power.
>
> you know that with the range timers/slack, you can control the
> "rounding" of the timer of the application, right?

I've not explored user space for this.

Can on a per-application basis some controlling application cause timers of a target process to be rounded or is it global? Or do you need to link the new application to use special glib variants (as described in OLS papers a few years ago)?

> You can *directly* throttle the number of wakeups an application causes
> that way to a value you set.

Are you talking about your work as seen in lwn.net summary?

http://lwn.net/Articles/296578/

Your change here does look like something which could be used to control timers.  Don't you still need some dynamic way to set the fuzz/slack if its globally applied?  It seems like you might want some timers precise and others fuzzy.

Would the holding of a wakelock or some activity counter be a good trigger for switching rounding time?  If wakelocks held "minor adjustment" else "major adjustment"

Thanks for the good pointer assuming I understood it in quick scan.

Thanks,
Richard W.

Re: [RFD] Automatic suspend

[mark gross]

On Mon, Feb 16, 2009 at 12:10:15AM +0100, Rafael J. Wysocki wrote:
> Hi,
> 
> The recent descussion about the Android PM patches sent by Arve shows that
> there is a need to introduce a mechanism allowing us to:
> (1) automatically put the system as a whole into a sleep state (eg. suspend to
>     RAM) when it is found to be "idle", where the meaning of "idle" has to be
>     defined too,

Can a fast STR state be created for the generic kernel across
architectures?  Something that can go into and out of SRT in < 20ms? (I
picked 20ms as that gives 50Hz screen refresh) I guess the XO does
something like this between keystrokes. 

Note: I don't know (but I should) if such a PM design popping into and
out of STR at worst every 20ms is worth doing on IA but lets assume its
not so bad for the sake of argument.

One could assume the system would be idle except for selects on HID
events when operating in this mode.

> (2) put given subset of devices into low power states whenever they are not
>     used, without putting the entire system into a sleep state.

This is a driver deal to drop the hardware they control into lower power
states.  Give information for PM-QOS the drivers could do the right
things here.  (or should be able too) 


Also how do we attempt to coordinate this with an automatic STR state?

How to coexist with user requested STR?

Do we need to more carefully define what STR really means?  (I'm
thinking we do)


> To allow these two things to happen, the Andriod patches introduced the
> wakelocks with the associated infrastructure and the "early suspend" mechanism.
> However, quite a number of reviewers did not like these patches, for various
> reasons, so they cannot be regarded as generally acceptable.  For this reason,
> I think, we should discuss the problem in detail from the beginning and try to
> find a solution that everyone interested will be comfortable with.
> 
> For this purpose, IMO, we should at least reach an agreement on what the
> kernel's and the userland's roles in (1) and (2) are going to be.  There also
> are quite a few questions that need to be answered.  For instance, what
> conditions are going to be used to decide whether or not the system is idle
> enough so that we can put it into a sleep state?  How are we going to check
> these conditions?  What is going to happen if one (or more) of them changes
> while a system-wide power transition (eg. suspend) is in progress?  Are we
> going to allow the user space to take part in this and if so, to what extent?
> What mechanisms are going be used to put devices into low power states at run
> time (ie. before starting any system-wide power transition) and how are they
> going to be related to the suspend-resume infrastructure used during
> system-wide power transitions?  The answers to these (and other related)
> questions will likely affect all of the future Linux PM developlent, so IMO
> this is a very important matter.
> 
> Opinions and comments welcome.
>

thank you for starting this FRD.
 
--mgross
> Thanks,
> Rafael

Re: [RFD] Automatic suspend

[mark gross]

On Tue, Feb 17, 2009 at 02:19:13PM -0800, Jesse Barnes wrote:
> On Tuesday, February 17, 2009 2:04 pm Matthew Garrett wrote:
> > On Tue, Feb 17, 2009 at 01:57:17PM -0800, mark gross wrote:
> > > Putting the wake on key event issue aside, is it possible to have wake
> > > up's on the ms time scale?  I ask because I thought the XO did exactly
> > > this (but left the screen live).  Why does it take 20 sec to get into
> > > or out of S3 on my laptop?
> >
> > Graphics reinit, dumping graphics contents back into RAM, us resuming
> > devices in series, that kind of thing. On some hardware you'll spend a
> > noticable amount of time in the BIOS before any of the Linux resume code
> > gets touched. I thought the XO had got sub second, but I wasn't sure
> > that they were in the low ms range.
> 
> IME a good chunk of it is BIOS time.  On my x200s resume is very fast (on the 
> order of a second or two though I haven't measured), while on my Eee and T61 
> machines it's much slower, even though they're all using Intel gfx.
> 
> Of course we should really be shooting for sub-second times or about the time 
> it takes you to open your lid (or even much faster in the case of demand 
> suspend/resume for servers/desktops).

yeah I was thinking vertical blank refresh times.

We would need a state where the graphics stayed on somehow too.   

--mgross

Re: [RFD] Automatic suspend

[mark gross]

On Mon, Feb 16, 2009 at 11:23:30PM +0000, Matthew Garrett wrote:
> On Tue, Feb 17, 2009 at 12:19:38AM +0100, Rafael J. Wysocki wrote:
> 
> > This, again, seems to be a bit x86-centric. :-)  The Android people are telling
> > us that on the hardware they deal with it does make sense to put the entire
> > system to sleep even for relatively short periods of time, since the latencies
> > involved are not too bad.
> 
> Arve said that the power state was equivalent in idle and suspend, but 
> that they preferred suspend because it stopped any periodic timers. I'd 
> be more interested in making sure that unnecessary timers aren't running 
> than focusing on automatically entering system-wide suspend - Nokia have 
> been managing this since 2005 with good results.
>

I think they where talking timers as hardware devices pulling electrons,
not SW timers triggering wake ups.  I guess the timer hardware could be
running, and pulling power, even when not programmed to trigger any
event or IRQ.

--mgross

Re: [RFD] Automatic suspend

[mark gross]

On Tue, Feb 17, 2009 at 04:04:21PM -0800, Arjan van de Ven wrote:
> On Tue, 17 Feb 2009 09:32:46 -0600
> "Woodruff, Richard" <r-woodruff2@ti.com> wrote:
> 
> > > so use range timers / timer slack for those apps that you do not
> > > trust. That is not a big deal, and solves the issue of timer
> > > wakeups...
> > 
> > I not so sure it is that straight forward in practice.  End systems
> > integrate a lot of 3rd party software who view performance 1st and
> > have no thought of power.
> 
> you know that with the range timers/slack, you can control the
> "rounding" of the timer of the application, right?
> You can *directly* throttle the number of wakeups an application causes
> that way to a value you set.
> 

As the application code is mostly managed it should be a slam dunk to
make range timers work withing the android framework or Dalvik layers.

--mgross

Re: [RFD] Automatic suspend

[mark gross]

On Tue, Feb 17, 2009 at 07:28:11AM -0800, Brian Swetland wrote:
> [Arjan van de Ven <arjan@infradead.org>]
> > On Tue, 17 Feb 2009 06:20:01 -0800
> > Brian Swetland <swetland@google.com> wrote:
> > 
> > > Of course that still doesn't address userspace.  Aggressively going to
> > > suspend lets us compensate for userspace programs that do somewhat
> > > silly things (I agree that it would be best if they didn't but they
> > > do and getting *everyone* to write their userspace code to avoid
> > > spinning or avoid waking up on short-duration timers to poll is a
> > > losing battle).
> > 
> > actually with powertop... on the open source side things are actually
> > won. It took all of 6 months...
> > I don't see that as a valid excuse. In fact, if this kind of solution
> > makes real userspace scheduled timers to be missed then I consider it a
> > serious functionality misfeature.
> 
> While you can't expect the kernel to solve all the problems of
> userspace, here's the broad situation one could end up in
> (note this specific sequence is generic and not based on any one
> specific product experience):
> 
> - carrier deploys a device 
> - carrier agrees to allow installation of arbitrary third party apps
>   without some horrible certification program requiring app authors
>   to jump through hoops, wait ages for approval, etc
> - users rejoice and install all kinds of apps
> - some apps are poorly written and impact battery life
> - users complain to carrier about battery life

Carrier pushes ISV for fix app, and updates that don't suck are
deployed.

Where is the problem?

--mgross

> 
> You will end up with some crappy apps that do really dumb things.
> However, even if they're badly written users may still install and use
> these apps because hey, they do something the user likes.
> 
> >From the Android standpoint, we're trying to balance protecting the
> system from poorly designed apps and somehow letting the user know "hey
> app X is chewing up a lot of power" (work in progress on this).
> 
> While I'd love for every app developer to actively tune their apps for a
> good mobile experience, I am skeptical that this is going to happen.
> 
> Brian

Re: [RFD] Automatic suspend

[Benjamin Herrenschmidt]

> You will end up with some crappy apps that do really dumb things.
> However, even if they're badly written users may still install and use
> these apps because hey, they do something the user likes.
> 
> >From the Android standpoint, we're trying to balance protecting the
> system from poorly designed apps and somehow letting the user know "hey
> app X is chewing up a lot of power" (work in progress on this).
> 
> While I'd love for every app developer to actively tune their apps for a
> good mobile experience, I am skeptical that this is going to happen.

One idea I've been toying around was to have powertune as some kind of
background thingy, and have it pop an icon in the notification area when
some app seems to behave badly (of course, defining "badly" is hard but
heh...). IE. actively inform the user "this application is sucking the
life out of your battery".

Cheers,
Ben.

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Tue, Feb 17, 2009 at 3:21 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday 17 February 2009, Alan Stern wrote:
>> On Tue, 17 Feb 2009, Rafael J. Wysocki wrote:
>>
>> > Phase 1: I agree that system-auto-suspend-on, system-auto-suspend-off would be
>> > useful, but I don't like the wakelocks interface.  Do you think there is an
>> > alternative way/mechanism of doing this?
>>
>> I rather like the suggestions Matthew Garrett has been making.  They
>> show how to improve the wakelock interface without losing any function.
>>
>> Really, the idea behind wakelocks comes down to the question of how to
>> determine when the system is sufficiently idle to go into auto-suspend.
>> There may be occasions when no task is runnable but userspace knows
>> that the system should not go to sleep because some work will be done
>> in the near future.  (Arve's example of a non-empty input buffer is
>> such a case.)  How should userspace let the kernel know whether it's
>> okay to suspend at these times?  That is the problem userspace
>> wakelocks are meant to solve.
>
> Still, do we really need multiple user space wakelocks (I'd prefer to call them
> sleeplocks)?  It seems that one such lock and a user space manager controlling
> it should be sufficient.

Yes, we could have a user space manager that all userspace wakelocks
go through, but it would have to start before any other processes that
need wakelocks and it would need a blocking ipc mechanism. The
wakelock api that is provided to android applications does all this,
but it is only available to java code. Supporting multiple userspace
wakelocks in the kernel is simpler than adding another userspace
wakelock layer.

>> Kernel wakelocks are a separate matter.  They are more like a form of
>> optimization, preventing the kernel from starting an auto-suspend when
>> some driver knows beforehand that it will return -EBUSY.
>
> I think kernel-side autosuspend (or rather autosleep) should only happen
> after certain subset of devices have been suspended using a per-device
> run-time autosuspend mechanism.

When the last wakelock is released the task that we woke up to perform
has finished. Why wait to re-enter suspend.

>
>> > Phase 3: Probably explicit control left to open/close.
>>
>> While that's generally a good idea, it's important to recognize that
>> some devices should be runtime-suspended even while they are open.
>
> From the kernel side, yes (and that should be transparent to the user space
> having them open).  By the user space, no.

Allowing user space to suspend input devices while they are still open
is useful. The user-space code that reads from the input devices does
not need to know if the device is suspended or not, and the kernel
cannot auto suspend input devices based on inactivity.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Tue, Feb 17, 2009 at 4:04 PM, Arjan van de Ven <arjan@infradead.org> wrote:
> On Tue, 17 Feb 2009 09:32:46 -0600
> "Woodruff, Richard" <r-woodruff2@ti.com> wrote:
>
>> > so use range timers / timer slack for those apps that you do not
>> > trust. That is not a big deal, and solves the issue of timer
>> > wakeups...
>>
>> I not so sure it is that straight forward in practice.  End systems
>> integrate a lot of 3rd party software who view performance 1st and
>> have no thought of power.
>
> you know that with the range timers/slack, you can control the
> "rounding" of the timer of the application, right?
> You can *directly* throttle the number of wakeups an application causes
> that way to a value you set.

I thought the point of range timers was to align multiple timers so
they wakeup at the same time, not to throttle individual timers.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Tue, 17 Feb 2009 18:18:59 -0600
"Woodruff, Richard" <r-woodruff2@ti.com> wrote:

> > On Tue, 17 Feb 2009 09:32:46 -0600
> > "Woodruff, Richard" <r-woodruff2@ti.com> wrote:
> >
> > > > so use range timers / timer slack for those apps that you do not
> > > > trust. That is not a big deal, and solves the issue of timer
> > > > wakeups...
> > >
> > > I not so sure it is that straight forward in practice.  End
> > > systems integrate a lot of 3rd party software who view
> > > performance 1st and have no thought of power.
> >
> > you know that with the range timers/slack, you can control the
> > "rounding" of the timer of the application, right?
> 
> I've not explored user space for this.
> 
> Can on a per-application basis some controlling application cause
> timers of a target process to be rounded or is it global?

it is actually per thread.. so rather fine grained
 
>Or do you
> need to link the new application to use special glib variants (as
> described in OLS papers a few years ago)?

no you can do it for 100% existing binary

> 
> Your change here does look like something which could be used to
> control timers.  Don't you still need some dynamic way to set the
> fuzz/slack if its globally applied?  It seems like you might want
> some timers precise and others fuzzy.

right now it's prctl() based. We have been looking for a good use case
for making it per syscall.. but haven't found a convincing one yet.

-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Tue, 17 Feb 2009 21:11:31 -0800
Arve Hjønnevåg <arve@android.com> wrote:

> On Tue, Feb 17, 2009 at 4:04 PM, Arjan van de Ven
> <arjan@infradead.org> wrote:
> > On Tue, 17 Feb 2009 09:32:46 -0600
> > "Woodruff, Richard" <r-woodruff2@ti.com> wrote:
> >
> >> > so use range timers / timer slack for those apps that you do not
> >> > trust. That is not a big deal, and solves the issue of timer
> >> > wakeups...
> >>
> >> I not so sure it is that straight forward in practice.  End systems
> >> integrate a lot of 3rd party software who view performance 1st and
> >> have no thought of power.
> >
> > you know that with the range timers/slack, you can control the
> > "rounding" of the timer of the application, right?
> > You can *directly* throttle the number of wakeups an application
> > causes that way to a value you set.
> 
> I thought the point of range timers was to align multiple timers so
> they wakeup at the same time, not to throttle individual timers.

it works both ways.



-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Wednesday 18 February 2009 00:26:53 schrieb Rafael J. Wysocki:
> > Another possibility is to set up independent runtime PM for the
> > transport and the device.  This means allowing the possibility that the
> > transport is suspended while its child (the device) is not.  This is a
> > little simpler (there's only one idle-timeout per device, since the
> > link is treated as an independent device), but it violates the
> > principle of never suspending a parent while there is an active child.
> 
> Well, I think the first approach would be better.

I am afraid it wouldn't be. How do you deal with shared transports?

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Wed, 18 Feb 2009 09:53:48 +0100
Oliver Neukum <oliver@neukum.org> wrote:

> Am Wednesday 18 February 2009 00:26:53 schrieb Rafael J. Wysocki:
> > > Another possibility is to set up independent runtime PM for the
> > > transport and the device.  This means allowing the possibility
> > > that the transport is suspended while its child (the device) is
> > > not.  This is a little simpler (there's only one idle-timeout per
> > > device, since the link is treated as an independent device), but
> > > it violates the principle of never suspending a parent while
> > > there is an active child.
> > 
> > Well, I think the first approach would be better.
> 
> I am afraid it wouldn't be. How do you deal with shared transports?
> 

realistically, something like this you need to design like this
Step 1) Assume the hardware is smart and can do this for you on the fly,
        but it might need guidance.
        (For many busses there are platforms that do this)
Step 2) For hardware that is not smart, emulate the smartness in the
        driver, with help of the subsystem. These two together have
        the right knowledge to make such decisions.


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Wednesday 18 February 2009 15:51:58 schrieb Arjan van de Ven:
> On Wed, 18 Feb 2009 09:53:48 +0100
> Oliver Neukum <oliver@neukum.org> wrote:
> 
> > Am Wednesday 18 February 2009 00:26:53 schrieb Rafael J. Wysocki:
> > > > Another possibility is to set up independent runtime PM for the
> > > > transport and the device.  This means allowing the possibility
> > > > that the transport is suspended while its child (the device) is
> > > > not.  This is a little simpler (there's only one idle-timeout per
> > > > device, since the link is treated as an independent device), but
> > > > it violates the principle of never suspending a parent while
> > > > there is an active child.
> > > 
> > > Well, I think the first approach would be better.
> > 
> > I am afraid it wouldn't be. How do you deal with shared transports?
> > 
> 
> realistically, something like this you need to design like this
> Step 1) Assume the hardware is smart and can do this for you on the fly,
>         but it might need guidance.
>         (For many busses there are platforms that do this)
> Step 2) For hardware that is not smart, emulate the smartness in the
>         driver, with help of the subsystem. These two together have
>         the right knowledge to make such decisions.

But the transport and the driver may be in different subsystems, e.g.
usb storage.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Alan Stern]

On Wed, 18 Feb 2009, Arjan van de Ven wrote:

> On Wed, 18 Feb 2009 09:53:48 +0100
> Oliver Neukum <oliver@neukum.org> wrote:
> 
> > Am Wednesday 18 February 2009 00:26:53 schrieb Rafael J. Wysocki:
> > > > Another possibility is to set up independent runtime PM for the
> > > > transport and the device.  This means allowing the possibility
> > > > that the transport is suspended while its child (the device) is
> > > > not.  This is a little simpler (there's only one idle-timeout per
> > > > device, since the link is treated as an independent device), but
> > > > it violates the principle of never suspending a parent while
> > > > there is an active child.
> > > 
> > > Well, I think the first approach would be better.
> > 
> > I am afraid it wouldn't be. How do you deal with shared transports?
> > 
> 
> realistically, something like this you need to design like this
> Step 1) Assume the hardware is smart and can do this for you on the fly,
>         but it might need guidance.
>         (For many busses there are platforms that do this)
> Step 2) For hardware that is not smart, emulate the smartness in the
>         driver, with help of the subsystem. These two together have
>         the right knowledge to make such decisions.

Arjan, you're missing the point.  This part of the discussion is 
already focussed on your Step 2.  The question we are concerned with 
is: _How_ shall the driver and subsystem go about emulating the 
smartness?

Oliver's comment is a good one.  The principle of not suspending a
parent with active children has never been set in stone; with care we
should be able to circumvent it.

Alan Stern

Re: [RFD] Automatic suspend

[Alan Stern]

On Wed, 18 Feb 2009, Oliver Neukum wrote:

> But the transport and the driver may be in different subsystems, e.g.
> usb storage.

We would have to add a part of usb-storage into the SCSI subsystem, by 
creating a USB transport class.  Perhaps this class could be shared 
with Firewire/IEEE1394.

Alan Stern

Re: [RFD] Automatic suspend

[Matthew Garrett]

On Tue, Feb 17, 2009 at 06:56:22AM -0800, Arjan van de Ven wrote:

> so use range timers / timer slack for those apps that you do not trust.
> That is not a big deal, and solves the issue of timer wakeups...

So have userspace say something like "The screen is turned off now, 
change the timer precision on all untrusted userland to sub 10 seconds"? 
That would work.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

RE: [RFD] Automatic suspend

[Woodruff, Richard]

> > so use range timers / timer slack for those apps that you do not trust.
> > That is not a big deal, and solves the issue of timer wakeups...
>
> So have userspace say something like "The screen is turned off now,
> change the timer precision on all untrusted userland to sub 10 seconds"?
> That would work.

I also made a similar comment in thread to Arjan yesterday.

He did respond saying there was a pctrl to hit this.  If it's a big list it might be messy.  Unless it can be global.  Probably would be good to look at code rather than speculate.

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Wednesday 18 February 2009 16:10:48 schrieb Alan Stern:
> On Wed, 18 Feb 2009, Oliver Neukum wrote:
> 
> > But the transport and the driver may be in different subsystems, e.g.
> > usb storage.
> 
> We would have to add a part of usb-storage into the SCSI subsystem, by 
> creating a USB transport class.  Perhaps this class could be shared 
> with Firewire/IEEE1394.

That would be a specific solution. But the problem is generic. Take
for example HID. Several drivers may share one transport, but the transport
may be USB 2.0/3.0 or bluetooth.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Alan Stern]

On Wed, 18 Feb 2009, Oliver Neukum wrote:

> Am Wednesday 18 February 2009 16:10:48 schrieb Alan Stern:
> > On Wed, 18 Feb 2009, Oliver Neukum wrote:
> > 
> > > But the transport and the driver may be in different subsystems, e.g.
> > > usb storage.
> > 
> > We would have to add a part of usb-storage into the SCSI subsystem, by 
> > creating a USB transport class.  Perhaps this class could be shared 
> > with Firewire/IEEE1394.
> 
> That would be a specific solution. But the problem is generic. Take
> for example HID. Several drivers may share one transport, but the transport
> may be USB 2.0/3.0 or bluetooth.

In essence what you're saying is that we won't be able to do
link-suspend for HID until something like HID transport classes (or
something simpler, devoted specifically to runtime PM) is implemented.  
And likewise for other subsystems.  I can't argue with this conclusion.

Alan Stern

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Wednesday 18 February 2009 17:39:21 schrieb Alan Stern:
> In essence what you're saying is that we won't be able to do
> link-suspend for HID until something like HID transport classes (or
> something simpler, devoted specifically to runtime PM) is implemented.  
> And likewise for other subsystems.  I can't argue with this conclusion.

I'd say, if it has sensible operations, it is a device and should be in the
tree as such.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Chris Ball]

Hi Mark,

   > Putting the wake on key event issue aside, is it possible to have
   > wake up's on the ms time scale?  I ask because I thought the XO did
   > exactly this (but left the screen live).

We're currently at 890ms for suspend+resume with USB on, and 180ms for
suspend+resume when booted "nousb".  Anything done upstream to make the
first number closer to the second one will be very valuable to us.

   > Why does it take 20 sec to get into or out of S3 on my laptop?

We found that the SMM version of S3 in our original development BIOS
also took that long.  We switched to using Open Firmware, turned off
ACPI, worked out exactly which registers to use to enter S3, and
scheduled device reinitialization such that hardware-required delays
(such as clock stabilization for our GPU) start in the right order to
keep the critical path short.

The main difference was the switch away from the vendor BIOS, though --
we were seeing something like a five-second delay between Linux entering
S3 and the hardware noticing and putting the system into suspend, for
example.  There's not much market pressure to write efficient EC code
for suspend/resume.  (Or, we discovered, to write EC code that's able
to handle suspend/resume more than a hundred times per boot without
crashing.)

- Chris.
-- 
Chris Ball   <cjb@laptop.org>

Re: [RFD] Automatic suspend

[Jesse Barnes]

On Tuesday, February 17, 2009 6:40 pm Benjamin Herrenschmidt wrote:
> > You will end up with some crappy apps that do really dumb things.
> > However, even if they're badly written users may still install and use
> > these apps because hey, they do something the user likes.
> >
> > >From the Android standpoint, we're trying to balance protecting the
> >
> > system from poorly designed apps and somehow letting the user know "hey
> > app X is chewing up a lot of power" (work in progress on this).
> >
> > While I'd love for every app developer to actively tune their apps for a
> > good mobile experience, I am skeptical that this is going to happen.
>
> One idea I've been toying around was to have powertune as some kind of
> background thingy, and have it pop an icon in the notification area when
> some app seems to behave badly (of course, defining "badly" is hard but
> heh...). IE. actively inform the user "this application is sucking the
> life out of your battery".

I think "powernag" would be a better name, and it would probably work pretty 
well.  Users would be notified that certain apps should probably be shut down 
while they're on battery, and nag the authors to fix it.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [RFD] Automatic suspend

[Matthew Garrett]

On Wed, Feb 18, 2009 at 09:48:04AM -0800, Jesse Barnes wrote:

> I think "powernag" would be a better name, and it would probably work pretty 
> well.  Users would be notified that certain apps should probably be shut down 
> while they're on battery, and nag the authors to fix it.

How do you usefully tell the difference between an application that's 
waking up a lot because it's dumb and an application that's waking up a 
lot because it's doing something useful? One downside of powertop is 
that it's resulted in a lot of "How do I reduce the number of wakeups 
caused by i8042 whenever I use the mouse"-type questions. We don't want 
innocent app writers to be hit by this kind of thing.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFD] Automatic suspend

[Jesse Barnes]

On Wednesday, February 18, 2009 9:52 am Matthew Garrett wrote:
> On Wed, Feb 18, 2009 at 09:48:04AM -0800, Jesse Barnes wrote:
> > I think "powernag" would be a better name, and it would probably work
> > pretty well.  Users would be notified that certain apps should probably
> > be shut down while they're on battery, and nag the authors to fix it.
>
> How do you usefully tell the difference between an application that's
> waking up a lot because it's dumb and an application that's waking up a
> lot because it's doing something useful? One downside of powertop is
> that it's resulted in a lot of "How do I reduce the number of wakeups
> caused by i8042 whenever I use the mouse"-type questions. We don't want
> innocent app writers to be hit by this kind of thing.

Well Ben did mention that it would be hard to figure out a good set of 
heuristics, but a userspace app could be pretty flexible, allowing the user 
to whitelist apps they know are fine, average the data over a period of time, 
etc.  Challenging, sure, but it should be possible to come up with something 
that's useful in many cases.

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Wednesday 18 February 2009, mark gross wrote:
> On Mon, Feb 16, 2009 at 12:10:15AM +0100, Rafael J. Wysocki wrote:
> > Hi,
> > 
> > The recent descussion about the Android PM patches sent by Arve shows that
> > there is a need to introduce a mechanism allowing us to:
> > (1) automatically put the system as a whole into a sleep state (eg. suspend to
> >     RAM) when it is found to be "idle", where the meaning of "idle" has to be
> >     defined too,
> 
> Can a fast STR state be created for the generic kernel across
> architectures?  Something that can go into and out of SRT in < 20ms?

I don't think it's generally doable on PCs.

> (I picked 20ms as that gives 50Hz screen refresh) I guess the XO does
> something like this between keystrokes. 
> 
> Note: I don't know (but I should) if such a PM design popping into and
> out of STR at worst every 20ms is worth doing on IA but lets assume its
> not so bad for the sake of argument.
> 
> One could assume the system would be idle except for selects on HID
> events when operating in this mode.
> 
> > (2) put given subset of devices into low power states whenever they are not
> >     used, without putting the entire system into a sleep state.
> 
> This is a driver deal to drop the hardware they control into lower power
> states.  Give information for PM-QOS the drivers could do the right
> things here.  (or should be able too) 

That may also depend on the bus the driver is on (like PCI for one example).

> Also how do we attempt to coordinate this with an automatic STR state?

That's one thing to think about.  In principle we can avoid suspending
devices that already are in low power states before a system-wide transition
is started (caveat: wake-up devices on ACPI systems).
 
> How to coexist with user requested STR?
> 
> Do we need to more carefully define what STR really means?  (I'm
> thinking we do)

Well, yes and no.  In fact ACPI defines STR as one of the system sleep states
in which the CPU does not execute instructions, but the ACPI definition need
not be applicable to non-ACPI platforms.

> > To allow these two things to happen, the Andriod patches introduced the
> > wakelocks with the associated infrastructure and the "early suspend" mechanism.
> > However, quite a number of reviewers did not like these patches, for various
> > reasons, so they cannot be regarded as generally acceptable.  For this reason,
> > I think, we should discuss the problem in detail from the beginning and try to
> > find a solution that everyone interested will be comfortable with.
> > 
> > For this purpose, IMO, we should at least reach an agreement on what the
> > kernel's and the userland's roles in (1) and (2) are going to be.  There also
> > are quite a few questions that need to be answered.  For instance, what
> > conditions are going to be used to decide whether or not the system is idle
> > enough so that we can put it into a sleep state?  How are we going to check
> > these conditions?  What is going to happen if one (or more) of them changes
> > while a system-wide power transition (eg. suspend) is in progress?  Are we
> > going to allow the user space to take part in this and if so, to what extent?
> > What mechanisms are going be used to put devices into low power states at run
> > time (ie. before starting any system-wide power transition) and how are they
> > going to be related to the suspend-resume infrastructure used during
> > system-wide power transitions?  The answers to these (and other related)
> > questions will likely affect all of the future Linux PM developlent, so IMO
> > this is a very important matter.
> > 
> > Opinions and comments welcome.
> >
> 
> thank you for starting this FRD.

No big deal. :-)

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Wednesday 18 February 2009, Arve Hjønnevåg wrote:
> On Tue, Feb 17, 2009 at 3:21 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Tuesday 17 February 2009, Alan Stern wrote:
> >> On Tue, 17 Feb 2009, Rafael J. Wysocki wrote:
> >>
> >> > Phase 1: I agree that system-auto-suspend-on, system-auto-suspend-off would be
> >> > useful, but I don't like the wakelocks interface.  Do you think there is an
> >> > alternative way/mechanism of doing this?
> >>
> >> I rather like the suggestions Matthew Garrett has been making.  They
> >> show how to improve the wakelock interface without losing any function.
> >>
> >> Really, the idea behind wakelocks comes down to the question of how to
> >> determine when the system is sufficiently idle to go into auto-suspend.
> >> There may be occasions when no task is runnable but userspace knows
> >> that the system should not go to sleep because some work will be done
> >> in the near future.  (Arve's example of a non-empty input buffer is
> >> such a case.)  How should userspace let the kernel know whether it's
> >> okay to suspend at these times?  That is the problem userspace
> >> wakelocks are meant to solve.
> >
> > Still, do we really need multiple user space wakelocks (I'd prefer to call them
> > sleeplocks)?  It seems that one such lock and a user space manager controlling
> > it should be sufficient.
> 
> Yes, we could have a user space manager that all userspace wakelocks
> go through, but it would have to start before any other processes that
> need wakelocks and it would need a blocking ipc mechanism. The
> wakelock api that is provided to android applications does all this,
> but it is only available to java code. Supporting multiple userspace
> wakelocks in the kernel is simpler than adding another userspace
> wakelock layer.
> 
> >> Kernel wakelocks are a separate matter.  They are more like a form of
> >> optimization, preventing the kernel from starting an auto-suspend when
> >> some driver knows beforehand that it will return -EBUSY.
> >
> > I think kernel-side autosuspend (or rather autosleep) should only happen
> > after certain subset of devices have been suspended using a per-device
> > run-time autosuspend mechanism.
> 
> When the last wakelock is released the task that we woke up to perform
> has finished. Why wait to re-enter suspend.

I don't really understand this comment.  Could you please explain a bit?

> >> > Phase 3: Probably explicit control left to open/close.
> >>
> >> While that's generally a good idea, it's important to recognize that
> >> some devices should be runtime-suspended even while they are open.
> >
> > From the kernel side, yes (and that should be transparent to the user space
> > having them open).  By the user space, no.
> 
> Allowing user space to suspend input devices while they are still open
> is useful. The user-space code that reads from the input devices does
> not need to know if the device is suspended or not, and the kernel
> cannot auto suspend input devices based on inactivity.

Hmm.  Why can't it?

Rafael

Re: [RFD] Automatic suspend

[Benjamin Herrenschmidt]

On Wed, 2009-02-18 at 17:52 +0000, Matthew Garrett wrote:
> On Wed, Feb 18, 2009 at 09:48:04AM -0800, Jesse Barnes wrote:
> 
> > I think "powernag" would be a better name, and it would probably work pretty 
> > well.  Users would be notified that certain apps should probably be shut down 
> > while they're on battery, and nag the authors to fix it.
> 
> How do you usefully tell the difference between an application that's 
> waking up a lot because it's dumb and an application that's waking up a 
> lot because it's doing something useful? One downside of powertop is 
> that it's resulted in a lot of "How do I reduce the number of wakeups 
> caused by i8042 whenever I use the mouse"-type questions. We don't want 
> innocent app writers to be hit by this kind of thing.

Which is why nagging might not be the best thing... One thing I quite
liked was something i had on MacOS 9 which was a watt-meter. I could
basically see at any time what my consumption was. If the needle wasn't
all the way to the left while I was expecting it to, I would dig for
the culprit.

Maybe we can try to do something similar, and when clicking on it,
you get an ordered list of apps from the biggest consumer down).

The main problem of course is for that "meter" to work, one need to have
an idea of what min and max are on a given machine... Absolute watt
numbers or things like that are a lot less telling to Joe average.

Cheers,
Ben.

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Wed, Feb 18, 2009 at 1:17 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Wednesday 18 February 2009, Arve Hjønnevåg wrote:
>> On Tue, Feb 17, 2009 at 3:21 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > On Tuesday 17 February 2009, Alan Stern wrote:

>> >> Kernel wakelocks are a separate matter.  They are more like a form of
>> >> optimization, preventing the kernel from starting an auto-suspend when
>> >> some driver knows beforehand that it will return -EBUSY.
>> >
>> > I think kernel-side autosuspend (or rather autosleep) should only happen
>> > after certain subset of devices have been suspended using a per-device
>> > run-time autosuspend mechanism.
>>
>> When the last wakelock is released the task that we woke up to perform
>> has finished. Why wait to re-enter suspend.
>
> I don't really understand this comment.  Could you please explain a bit?

If some devices are autosuspended after a local inactivity timeout, I
don't want to wait for those devices to autosuspend if I know the code
that needed to run is done. This could cause delays in the normal
case, and it could prevent suspend if a background process (not using
wakelocks) is accessing a disk more frequently than its idle timeout.

>
>> >> > Phase 3: Probably explicit control left to open/close.
>> >>
>> >> While that's generally a good idea, it's important to recognize that
>> >> some devices should be runtime-suspended even while they are open.
>> >
>> > From the kernel side, yes (and that should be transparent to the user space
>> > having them open).  By the user space, no.
>>
>> Allowing user space to suspend input devices while they are still open
>> is useful. The user-space code that reads from the input devices does
>> not need to know if the device is suspended or not, and the kernel
>> cannot auto suspend input devices based on inactivity.
>
> Hmm.  Why can't it?

Because they stop working. It is OK for us to turn off the touchscreen
when the screen is off, but when the screen is on the user will touch
items on the screen and expect them to respond. (We could also turn
off the touchscreen when the keyguard is on, but we don't currently do
this.)

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Wednesday 18 February 2009 23:35:26 schrieb Arve Hjønnevåg:
> >> Allowing user space to suspend input devices while they are still open
> >> is useful. The user-space code that reads from the input devices does
> >> not need to know if the device is suspended or not, and the kernel
> >> cannot auto suspend input devices based on inactivity.
> >
> > Hmm.  Why can't it?
> 
> Because they stop working. It is OK for us to turn off the touchscreen
> when the screen is off, but when the screen is on the user will touch
> items on the screen and expect them to respond. (We could also turn
> off the touchscreen when the keyguard is on, but we don't currently do
> this.)

This depends on the capabilities of the input device in question. Some
USB keyboards can be autosuspended under these conditions because
they can do remote wakeup.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Wednesday 18 February 2009, Arve Hjønnevåg wrote:
> On Wed, Feb 18, 2009 at 1:17 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Wednesday 18 February 2009, Arve Hjønnevåg wrote:
> >> On Tue, Feb 17, 2009 at 3:21 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> > On Tuesday 17 February 2009, Alan Stern wrote:
> 
> >> >> Kernel wakelocks are a separate matter.  They are more like a form of
> >> >> optimization, preventing the kernel from starting an auto-suspend when
> >> >> some driver knows beforehand that it will return -EBUSY.
> >> >
> >> > I think kernel-side autosuspend (or rather autosleep) should only happen
> >> > after certain subset of devices have been suspended using a per-device
> >> > run-time autosuspend mechanism.
> >>
> >> When the last wakelock is released the task that we woke up to perform
> >> has finished. Why wait to re-enter suspend.
> >
> > I don't really understand this comment.  Could you please explain a bit?
> 
> If some devices are autosuspended after a local inactivity timeout, I
> don't want to wait for those devices to autosuspend if I know the code
> that needed to run is done. This could cause delays in the normal
> case,

Isn't it a matter of adjusting the inactivity timeouts in a suitable way?

> and it could prevent suspend if a background process (not using
> wakelocks) is accessing a disk more frequently than its idle timeout.

Well, actually, shouldn't it prevent suspend from happening?  Arguably, it just
means that the disk is continuously being accessed with respect to the inactive
timeout granularity.

Rafael

Re: [RFD] Automatic suspend

[Alan Stern]

On Thu, 19 Feb 2009, Rafael J. Wysocki wrote:

> > If some devices are autosuspended after a local inactivity timeout, I
> > don't want to wait for those devices to autosuspend if I know the code
> > that needed to run is done. This could cause delays in the normal
> > case,
> 
> Isn't it a matter of adjusting the inactivity timeouts in a suitable way?

It's not that simple.  A single device driver has a very local view, 
not suitable for deciding whether the entire system should go to sleep.

So for example, a disk driver might think it's appropriate to spin down 
the disk after 10 seconds of inactivity.  But an overall system monitor 
might realize that nothing is going on right now and want to put the 
system to sleep immediately, without waiting the 10 seconds for the 
disk to autosuspend.

> > and it could prevent suspend if a background process (not using
> > wakelocks) is accessing a disk more frequently than its idle timeout.
> 
> Well, actually, shouldn't it prevent suspend from happening?  Arguably, it just
> means that the disk is continuously being accessed with respect to the inactive
> timeout granularity.

That's true, but it shows the problem of making the autosleep decision
based on disk activity.  An auto-sleep should not have to wait for
every device (or some suitable subset) to become idle for some minimum
time; it should be able to kick in at short notice.

Alan Stern

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Thursday 19 February 2009, Alan Stern wrote:
> On Thu, 19 Feb 2009, Rafael J. Wysocki wrote:
> 
> > > If some devices are autosuspended after a local inactivity timeout, I
> > > don't want to wait for those devices to autosuspend if I know the code
> > > that needed to run is done. This could cause delays in the normal
> > > case,
> > 
> > Isn't it a matter of adjusting the inactivity timeouts in a suitable way?
> 
> It's not that simple.  A single device driver has a very local view, 
> not suitable for deciding whether the entire system should go to sleep.
> 
> So for example, a disk driver might think it's appropriate to spin down 
> the disk after 10 seconds of inactivity.  But an overall system monitor 
> might realize that nothing is going on right now and want to put the 
> system to sleep immediately, without waiting the 10 seconds for the 
> disk to autosuspend.

Now, the question is what criteria would the overall system monitor use to make
such a decision.

> > > and it could prevent suspend if a background process (not using
> > > wakelocks) is accessing a disk more frequently than its idle timeout.
> > 
> > Well, actually, shouldn't it prevent suspend from happening?  Arguably, it just
> > means that the disk is continuously being accessed with respect to the inactive
> > timeout granularity.
> 
> That's true, but it shows the problem of making the autosleep decision
> based on disk activity.  An auto-sleep should not have to wait for
> every device (or some suitable subset) to become idle for some minimum
> time; it should be able to kick in at short notice.

Again, the decision to trigger automatic suspend has to be based on some
well defined criteria and the (in)activity of devices seems to be one of them.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Alan Stern]

On Thu, 19 Feb 2009, Rafael J. Wysocki wrote:

> On Thursday 19 February 2009, Alan Stern wrote:
> > On Thu, 19 Feb 2009, Rafael J. Wysocki wrote:
> > 
> > > > If some devices are autosuspended after a local inactivity timeout, I
> > > > don't want to wait for those devices to autosuspend if I know the code
> > > > that needed to run is done. This could cause delays in the normal
> > > > case,
> > > 
> > > Isn't it a matter of adjusting the inactivity timeouts in a suitable way?
> > 
> > It's not that simple.  A single device driver has a very local view, 
> > not suitable for deciding whether the entire system should go to sleep.
> > 
> > So for example, a disk driver might think it's appropriate to spin down 
> > the disk after 10 seconds of inactivity.  But an overall system monitor 
> > might realize that nothing is going on right now and want to put the 
> > system to sleep immediately, without waiting the 10 seconds for the 
> > disk to autosuspend.
> 
> Now, the question is what criteria would the overall system monitor use to make
> such a decision.
> 
> > > > and it could prevent suspend if a background process (not using
> > > > wakelocks) is accessing a disk more frequently than its idle timeout.
> > > 
> > > Well, actually, shouldn't it prevent suspend from happening?  Arguably, it just
> > > means that the disk is continuously being accessed with respect to the inactive
> > > timeout granularity.
> > 
> > That's true, but it shows the problem of making the autosleep decision
> > based on disk activity.  An auto-sleep should not have to wait for
> > every device (or some suitable subset) to become idle for some minimum
> > time; it should be able to kick in at short notice.
> 
> Again, the decision to trigger automatic suspend has to be based on some
> well defined criteria and the (in)activity of devices seems to be one of them.

I don't know what criteria the system monitor would use.  It might have 
to be platform-specific.  The Android people seem to have a pretty good 
idea of what criteria will work for them.

Inactivity of devices isn't always a good criterion.  There might be a
background task which wakes up every few seconds to do something as
long as the system is awake, thereby keeping some device always active.  
The activity from this background task shouldn't prevent an auto-sleep.

Alan Stern

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Thursday 19 February 2009, Alan Stern wrote:
> On Thu, 19 Feb 2009, Rafael J. Wysocki wrote:
> 
> > On Thursday 19 February 2009, Alan Stern wrote:
> > > On Thu, 19 Feb 2009, Rafael J. Wysocki wrote:
> > > 
> > > > > If some devices are autosuspended after a local inactivity timeout, I
> > > > > don't want to wait for those devices to autosuspend if I know the code
> > > > > that needed to run is done. This could cause delays in the normal
> > > > > case,
> > > > 
> > > > Isn't it a matter of adjusting the inactivity timeouts in a suitable way?
> > > 
> > > It's not that simple.  A single device driver has a very local view, 
> > > not suitable for deciding whether the entire system should go to sleep.
> > > 
> > > So for example, a disk driver might think it's appropriate to spin down 
> > > the disk after 10 seconds of inactivity.  But an overall system monitor 
> > > might realize that nothing is going on right now and want to put the 
> > > system to sleep immediately, without waiting the 10 seconds for the 
> > > disk to autosuspend.
> > 
> > Now, the question is what criteria would the overall system monitor use to make
> > such a decision.
> > 
> > > > > and it could prevent suspend if a background process (not using
> > > > > wakelocks) is accessing a disk more frequently than its idle timeout.
> > > > 
> > > > Well, actually, shouldn't it prevent suspend from happening?  Arguably, it just
> > > > means that the disk is continuously being accessed with respect to the inactive
> > > > timeout granularity.
> > > 
> > > That's true, but it shows the problem of making the autosleep decision
> > > based on disk activity.  An auto-sleep should not have to wait for
> > > every device (or some suitable subset) to become idle for some minimum
> > > time; it should be able to kick in at short notice.
> > 
> > Again, the decision to trigger automatic suspend has to be based on some
> > well defined criteria and the (in)activity of devices seems to be one of them.
> 
> I don't know what criteria the system monitor would use.

I don't know either and this is the whole point.  They need to be specified
somehow and I'm not sure if "we suspend if no one is holding a wakelock" is the
right approach.

> It might have to be platform-specific.  The Android people seem to have a
> pretty good idea of what criteria will work for them.

I'd really like to know in what situations Androind is supposed to suspend
automatically.

> Inactivity of devices isn't always a good criterion.  There might be a
> background task which wakes up every few seconds to do something as
> long as the system is awake, thereby keeping some device always active.  
> The activity from this background task shouldn't prevent an auto-sleep.

In fact there are two problems here.  First, there may be a task preventing
some devices from becoming inactive (like syslog).  Second, there may be
a task waiting for something important to happen, such that automatic suspend
cannot be triggered while it's waiting.  In both cases, IMO, the kernel is not
in a point to decide whether to suspend or not, because the user space knows
better.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Brian Swetland]

["Rafael J. Wysocki" <rjw@sisk.pl>]
> 
> > It might have to be platform-specific.  The Android people seem to have a
> > pretty good idea of what criteria will work for them.
> 
> I'd really like to know in what situations Androind is supposed to suspend
> automatically.

Currently, whenever the display is off (after the user-selectable
timeout), we aggressively enter suspend -- wakelocks are used to prevent
suspend when a driver or userspace entity needs the CPU to actually be
awake.

Brian

Re: [RFD] Automatic suspend

[Alan Stern]

On Thu, 19 Feb 2009, Rafael J. Wysocki wrote:

> > > Again, the decision to trigger automatic suspend has to be based on some
> > > well defined criteria and the (in)activity of devices seems to be one of them.
> > 
> > I don't know what criteria the system monitor would use.
> 
> I don't know either and this is the whole point.  They need to be specified
> somehow and I'm not sure if "we suspend if no one is holding a wakelock" is the
> right approach.

That isn't really a criterion; it's just a mechanism.  All it does is
push the problem back one level.  Now the question becomes: When is it
appropriate/necessary to hold a wakelock?

> > It might have to be platform-specific.  The Android people seem to have a
> > pretty good idea of what criteria will work for them.
> 
> I'd really like to know in what situations Androind is supposed to suspend
> automatically.

It might be better to ask in what situations Android is _not_ supposed
to sleep automatically.  In other words, in what situations is a
wakelock acquired?  Since the whole system is only a phone, this
question should have a reasonably well-defined answer.

> > Inactivity of devices isn't always a good criterion.  There might be a
> > background task which wakes up every few seconds to do something as
> > long as the system is awake, thereby keeping some device always active.  
> > The activity from this background task shouldn't prevent an auto-sleep.
> 
> In fact there are two problems here.  First, there may be a task preventing
> some devices from becoming inactive (like syslog).

Which means that device inactivity isn't always a good indicator for
auto-sleep.  (But then there can be different levels of activity: A
disk should always block auto-sleep while it is carrying out I/O, but
it might not block auto-sleep just because it is spinning.)

>  Second, there may be
> a task waiting for something important to happen, such that automatic suspend
> cannot be triggered while it's waiting.  In both cases, IMO, the kernel is not
> in a point to decide whether to suspend or not, because the user space knows
> better.

That's the whole point behind userspace wakelocks!  They provide a
mechanism for userspace to tell the kernel when (as far as userspace is
concerned) it is or is not okay to auto-sleep.

Alan Stern

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Thursday 19 February 2009, Alan Stern wrote:
> On Thu, 19 Feb 2009, Rafael J. Wysocki wrote:
> 
> > > > Again, the decision to trigger automatic suspend has to be based on some
> > > > well defined criteria and the (in)activity of devices seems to be one of them.
> > > 
> > > I don't know what criteria the system monitor would use.
> > 
> > I don't know either and this is the whole point.  They need to be specified
> > somehow and I'm not sure if "we suspend if no one is holding a wakelock" is the
> > right approach.
> 
> That isn't really a criterion; it's just a mechanism.  All it does is
> push the problem back one level.  Now the question becomes: When is it
> appropriate/necessary to hold a wakelock?
> 
> > > It might have to be platform-specific.  The Android people seem to have a
> > > pretty good idea of what criteria will work for them.
> > 
> > I'd really like to know in what situations Androind is supposed to suspend
> > automatically.
> 
> It might be better to ask in what situations Android is _not_ supposed
> to sleep automatically.  In other words, in what situations is a
> wakelock acquired?  Since the whole system is only a phone, this
> question should have a reasonably well-defined answer.
> 
> > > Inactivity of devices isn't always a good criterion.  There might be a
> > > background task which wakes up every few seconds to do something as
> > > long as the system is awake, thereby keeping some device always active.  
> > > The activity from this background task shouldn't prevent an auto-sleep.
> > 
> > In fact there are two problems here.  First, there may be a task preventing
> > some devices from becoming inactive (like syslog).
> 
> Which means that device inactivity isn't always a good indicator for
> auto-sleep.  (But then there can be different levels of activity: A
> disk should always block auto-sleep while it is carrying out I/O, but
> it might not block auto-sleep just because it is spinning.)
> 
> >  Second, there may be
> > a task waiting for something important to happen, such that automatic suspend
> > cannot be triggered while it's waiting.  In both cases, IMO, the kernel is not
> > in a point to decide whether to suspend or not, because the user space knows
> > better.
> 
> That's the whole point behind userspace wakelocks!  They provide a
> mechanism for userspace to tell the kernel when (as far as userspace is
> concerned) it is or is not okay to auto-sleep.

Still, one can go further and observe that the user space can in fact start
automatic suspend by itself whenever it knows it's appropriate ...

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Donnerstag 19 Februar 2009 23:21:46 schrieb Rafael J. Wysocki:
> > That's the whole point behind userspace wakelocks!  They provide a
> > mechanism for userspace to tell the kernel when (as far as userspace 
is
> > concerned) it is or is not okay to auto-sleep.
>
> Still, one can go further and observe that the user space can in fact start
> automatic suspend by itself whenever it knows it's appropriate ...

User space initiating this is a race condition.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>> > It might have to be platform-specific.  The Android people seem to have a
>> > pretty good idea of what criteria will work for them.
>>
>> I'd really like to know in what situations Androind is supposed to suspend
>> automatically.
>
> It might be better to ask in what situations Android is _not_ supposed
> to sleep automatically.  In other words, in what situations is a
> wakelock acquired?  Since the whole system is only a phone, this
> question should have a reasonably well-defined answer.

On an android phone, any code that needs to run when the screen is off
must hold a wakelock (directly or indirectly). In general if an
application or the system is processing an event that may cause a user
notification (new email, incoming phone call, alarm, etc.) it needs to
prevent suspend. But, we also use wakelocks to upload stats or
download system updates in the background, and for media player or
(gps) data logging applications.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Mon, Feb 16, 2009 at 3:23 PM, Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> On Tue, Feb 17, 2009 at 12:19:38AM +0100, Rafael J. Wysocki wrote:
>
>> This, again, seems to be a bit x86-centric. :-)  The Android people are telling
>> us that on the hardware they deal with it does make sense to put the entire
>> system to sleep even for relatively short periods of time, since the latencies
>> involved are not too bad.
>
> Arve said that the power state was equivalent in idle and suspend, but
> that they preferred suspend because it stopped any periodic timers. I'd
> be more interested in making sure that unnecessary timers aren't running
> than focusing on automatically entering system-wide suspend - Nokia have
> been managing this since 2005 with good results.

I'm sure you can build a product with acceptable battery life without
suspend. The average power draw on the G1 while idle with everything
off is less than the power draw was in suspend on our previous
hardware. However, on the same hardware you get 40 days standby using
suspend versus 20 days using idle. If you turn on the radio and use
the device every day, this starts to become irrelevant since you are
lucky to get more than a few days standby time. However, if you
install an application that wakes up 60 times a second (to check if it
need to draw something on the screen) the difference between idle and
suspend becomes 10 hours versus 40 days, or 10 hours versus 3 days
with my use.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Thursday 19 February 2009, Oliver Neukum wrote:
> Am Donnerstag 19 Februar 2009 23:21:46 schrieb Rafael J. Wysocki:
> > > That's the whole point behind userspace wakelocks!  They provide a
> > > mechanism for userspace to tell the kernel when (as far as userspace 
> is
> > > concerned) it is or is not okay to auto-sleep.
> >
> > Still, one can go further and observe that the user space can in fact start
> > automatic suspend by itself whenever it knows it's appropriate ...
> 
> User space initiating this is a race condition.

Do you mean a race with the other user space processes or with the kernel?

Rafael

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Friday 20 February 2009, Arve Hjønnevåg wrote:
> On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> >> > It might have to be platform-specific.  The Android people seem to have a
> >> > pretty good idea of what criteria will work for them.
> >>
> >> I'd really like to know in what situations Androind is supposed to suspend
> >> automatically.
> >
> > It might be better to ask in what situations Android is _not_ supposed
> > to sleep automatically.  In other words, in what situations is a
> > wakelock acquired?  Since the whole system is only a phone, this
> > question should have a reasonably well-defined answer.
> 
> On an android phone, any code that needs to run when the screen is off
> must hold a wakelock (directly or indirectly). In general if an
> application or the system is processing an event that may cause a user
> notification (new email, incoming phone call, alarm, etc.) it needs to
> prevent suspend. But, we also use wakelocks to upload stats or
> download system updates in the background, and for media player or
> (gps) data logging applications.

All of this doesn't seem to require wakelocks acuired from kernel space.
What do you need those wakelocks for?

Rafael

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Fri, Feb 20, 2009 at 2:49 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Friday 20 February 2009, Arve Hjønnevåg wrote:
>> On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>> >> > It might have to be platform-specific.  The Android people seem to have a
>> >> > pretty good idea of what criteria will work for them.
>> >>
>> >> I'd really like to know in what situations Androind is supposed to suspend
>> >> automatically.
>> >
>> > It might be better to ask in what situations Android is _not_ supposed
>> > to sleep automatically.  In other words, in what situations is a
>> > wakelock acquired?  Since the whole system is only a phone, this
>> > question should have a reasonably well-defined answer.
>>
>> On an android phone, any code that needs to run when the screen is off
>> must hold a wakelock (directly or indirectly). In general if an
>> application or the system is processing an event that may cause a user
>> notification (new email, incoming phone call, alarm, etc.) it needs to
>> prevent suspend. But, we also use wakelocks to upload stats or
>> download system updates in the background, and for media player or
>> (gps) data logging applications.
>
> All of this doesn't seem to require wakelocks acuired from kernel space.
> What do you need those wakelocks for?

Most events do not originate in user-space. Alarms start in our alarm
driver which locks a wakelock when its timer interrupt occurs. This
wakelock stays locked until the user-space alarm manager calls the
driver to wait for the next alarm. I've described how input events are
handled before. Without kernel wakelocks, if the user space power
manager had already turned off the screen and decided to suspend right
before a wakeup key is pressed, then that key could sit in the
in-kernel input queue until another key is pressed. Even if the
user-space thread read the key event before being frozen, it cannot
abort the suspend operation that was already started.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Thu, 19 Feb 2009 21:35:39 -0800
Arve Hjønnevåg <arve@android.com> wrote:

> However, if you
> install an application that wakes up 60 times a second (to check if it
> need to draw something on the screen) the difference between idle and
> suspend becomes 10 hours versus 40 days, or 10 hours versus 3 days
> with my use.

then that is the point where you use the timerslack to throttle such
non-system applications.... that's very obvious and there today..


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Friday 20 February 2009, Arve Hjønnevåg wrote:
> On Fri, Feb 20, 2009 at 2:49 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
> >> On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> >> >> > It might have to be platform-specific.  The Android people seem to have a
> >> >> > pretty good idea of what criteria will work for them.
> >> >>
> >> >> I'd really like to know in what situations Androind is supposed to suspend
> >> >> automatically.
> >> >
> >> > It might be better to ask in what situations Android is _not_ supposed
> >> > to sleep automatically.  In other words, in what situations is a
> >> > wakelock acquired?  Since the whole system is only a phone, this
> >> > question should have a reasonably well-defined answer.
> >>
> >> On an android phone, any code that needs to run when the screen is off
> >> must hold a wakelock (directly or indirectly). In general if an
> >> application or the system is processing an event that may cause a user
> >> notification (new email, incoming phone call, alarm, etc.) it needs to
> >> prevent suspend. But, we also use wakelocks to upload stats or
> >> download system updates in the background, and for media player or
> >> (gps) data logging applications.
> >
> > All of this doesn't seem to require wakelocks acuired from kernel space.
> > What do you need those wakelocks for?
> 
> Most events do not originate in user-space. Alarms start in our alarm
> driver which locks a wakelock when its timer interrupt occurs. This
> wakelock stays locked until the user-space alarm manager calls the
> driver to wait for the next alarm. I've described how input events are
> handled before. Without kernel wakelocks, if the user space power
> manager had already turned off the screen and decided to suspend right
> before a wakeup key is pressed, then that key could sit in the
> in-kernel input queue until another key is pressed. Even if the
> user-space thread read the key event before being frozen, it cannot
> abort the suspend operation that was already started.

OK, so what about the following approach:

* Keep the decision making logic (power manager etc.) in user space.  Reasons:
  - It may be arbitrarily complicated
  - It may include such things as s2ram quirks or hal quirks needed for some
    graphics adapters

* Have a per-process (per-task or per-thread group, but the former would be
  easier IMO) "I_do_not_want_automatic_suspend_to_occur" flag.

* Add a new callback, say ->acknowledge(), to the set of each driver's PM
  operations, that will be called to check if the driver has anything against
  automatic suspend (true - suspend can happen right now, false - suspend can't
  happen).

* Introduce /sys/power/sleep that will work like /sys/power/state, but:
  - First, it will call ->acknowledge() for each driver (via bus types) to
    check if any of them wants to postpone the suspend (this will prevent tasks
    from being frozen unnecessarily if it is known in advance that the suspend
    should not happen at the moment).
  - Next, it will check the "I_do_not_want_automatic_suspend_to_occur" flag
    of each process and the suspend will be aborted if it is true for any of
    them (quite frankly, I think that should be integrated with the freezer,
    in particular the tasks that have TIF_FREEZE set shouldn't be able to set
    this flag and it should be checked in the freezer loop for every task with
    TIF_FREEZE unset).
  - Next, it will proceed with suspending just like /sys/power/state does (the
    drivers that missed the opportunity to abort the suspend by returning
    'false' from ->acknowledge() can still abort the suspend by failing their
    ->suspend() routines).

Then, the decision making logic will be able to use /sys/power/sleep whenever
it wishes to and the kernel will be able to refuse to suspend if it's not
desirable at the moment.

It seems to be flexible enough to me.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Kyle Moffett]

On Fri, Feb 20, 2009 at 10:56 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> * Introduce /sys/power/sleep that will work like /sys/power/state, but:
>  - First, it will call ->acknowledge() for each driver (via bus types) to
>    check if any of them wants to postpone the suspend (this will prevent tasks
>    from being frozen unnecessarily if it is known in advance that the suspend
>    should not happen at the moment).
>  - Next, it will check the "I_do_not_want_automatic_suspend_to_occur" flag
>    of each process and the suspend will be aborted if it is true for any of
>    them (quite frankly, I think that should be integrated with the freezer,
>    in particular the tasks that have TIF_FREEZE set shouldn't be able to set
>    this flag and it should be checked in the freezer loop for every task with
>    TIF_FREEZE unset).
>  - Next, it will proceed with suspending just like /sys/power/state does (the
>    drivers that missed the opportunity to abort the suspend by returning
>    'false' from ->acknowledge() can still abort the suspend by failing their
>    ->suspend() routines).
>
> Then, the decision making logic will be able to use /sys/power/sleep whenever
> it wishes to and the kernel will be able to refuse to suspend if it's not
> desirable at the moment.

Then, for a userspace process handling input events:

1)  Unset "no-suspend flag"
2)  Poll event device
3)  When the poll returns true, set the "no-suspend flag"
4)  Read the event device and process the event(s)
5)  Go back to step 1

The event driver would responsible for atomically preventing suspend
from the moment a button is pressed until it is actually delivered to
a userspace process.  Since at that time the userspace process will be
preventing suspend, there will be no window in which we could go to
sleep with pending events.

Cheers,
Kyle Moffett

Re: [RFD] Automatic suspend

[Pavel Machek]

On Tue 2009-02-17 00:20:12, Oliver Neukum wrote:
> Am Monday 16 February 2009 18:31:08 schrieb Arjan van de Ven:
> > On Mon, 16 Feb 2009 17:48:06 +0100
> > Oliver Neukum <oliver@neukum.org> wrote:
> > 
> > > > Userspace should just be the part that says "I'm interested in link
> > > > notifications". The driver then just goes to the lowest power state
> > > > for its device that still gives those notifications.
> > > 
> > > Should there be a channel through which drivers can tell user space
> > > what the consequences in terms of reaching a sleep state are if a
> > > service is requested?
> > 
> > Personally, I don't think so.
> > Think about it: what would userspace do?
> > Not ask for the functionality if it means no sleep state?
> > Maybe it should just never ask for it then as a required functionality!
> 
> 1. You may have a human being using the information to make the tradeoff.
> 2. User space may be able to select among several devices the one cheapest
> to use

Well, that sounds complex/fragile... and it is really exporting
hardware impormation driver does not even neccessarily know.

If this is for user, perhaps it should be in documentation and/or
hardware user manual?

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

 On Mon 2009-02-16 23:23:30, Matthew Garrett wrote:
> On Tue, Feb 17, 2009 at 12:19:38AM +0100, Rafael J. Wysocki wrote:
> 
> > This, again, seems to be a bit x86-centric. :-)  The Android people are telling
> > us that on the hardware they deal with it does make sense to put the entire
> > system to sleep even for relatively short periods of time, since the latencies
> > involved are not too bad.
> 
> Arve said that the power state was equivalent in idle and suspend, but 

...for Arve's hardware. Not all embedded systems are like that, and
OLPC / PCs are definitely not like that.

								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Matthew Garrett]

On Fri, Feb 20, 2009 at 07:22:15PM +0100, Pavel Machek wrote:
>  On Mon 2009-02-16 23:23:30, Matthew Garrett wrote:
> > On Tue, Feb 17, 2009 at 12:19:38AM +0100, Rafael J. Wysocki wrote:
> > 
> > > This, again, seems to be a bit x86-centric. :-)  The Android people are telling
> > > us that on the hardware they deal with it does make sense to put the entire
> > > system to sleep even for relatively short periods of time, since the latencies
> > > involved are not too bad.
> > 
> > Arve said that the power state was equivalent in idle and suspend, but 
> 
> ...for Arve's hardware. Not all embedded systems are like that, and
> OLPC / PCs are definitely not like that.

And PCs have enough latency over suspend/resume that we can't do it 
automatically in a non-intrusive manner anyway, so runtime pm is more 
interesting there as well.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Friday 20 February 2009, Matthew Garrett wrote:
> On Fri, Feb 20, 2009 at 07:22:15PM +0100, Pavel Machek wrote:
> >  On Mon 2009-02-16 23:23:30, Matthew Garrett wrote:
> > > On Tue, Feb 17, 2009 at 12:19:38AM +0100, Rafael J. Wysocki wrote:
> > > 
> > > > This, again, seems to be a bit x86-centric. :-)  The Android people are telling
> > > > us that on the hardware they deal with it does make sense to put the entire
> > > > system to sleep even for relatively short periods of time, since the latencies
> > > > involved are not too bad.
> > > 
> > > Arve said that the power state was equivalent in idle and suspend, but 
> > 
> > ...for Arve's hardware. Not all embedded systems are like that, and
> > OLPC / PCs are definitely not like that.
> 
> And PCs have enough latency over suspend/resume that we can't do it 
> automatically in a non-intrusive manner anyway, so runtime pm is more 
> interesting there as well.

Agreed.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Friday 20 February 2009, Kyle Moffett wrote:
> On Fri, Feb 20, 2009 at 10:56 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > * Introduce /sys/power/sleep that will work like /sys/power/state, but:
> >  - First, it will call ->acknowledge() for each driver (via bus types) to
> >    check if any of them wants to postpone the suspend (this will prevent tasks
> >    from being frozen unnecessarily if it is known in advance that the suspend
> >    should not happen at the moment).
> >  - Next, it will check the "I_do_not_want_automatic_suspend_to_occur" flag
> >    of each process and the suspend will be aborted if it is true for any of
> >    them (quite frankly, I think that should be integrated with the freezer,
> >    in particular the tasks that have TIF_FREEZE set shouldn't be able to set
> >    this flag and it should be checked in the freezer loop for every task with
> >    TIF_FREEZE unset).
> >  - Next, it will proceed with suspending just like /sys/power/state does (the
> >    drivers that missed the opportunity to abort the suspend by returning
> >    'false' from ->acknowledge() can still abort the suspend by failing their
> >    ->suspend() routines).
> >
> > Then, the decision making logic will be able to use /sys/power/sleep whenever
> > it wishes to and the kernel will be able to refuse to suspend if it's not
> > desirable at the moment.
> 
> Then, for a userspace process handling input events:
> 
> 1)  Unset "no-suspend flag"
> 2)  Poll event device
> 3)  When the poll returns true, set the "no-suspend flag"
> 4)  Read the event device and process the event(s)
> 5)  Go back to step 1
> 
> The event driver would responsible for atomically preventing suspend
> from the moment a button is pressed until it is actually delivered to
> a userspace process.  Since at that time the userspace process will be
> preventing suspend, there will be no window in which we could go to
> sleep with pending events.

That's correct.

How would you like to prevent it from happening?

Rafael

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Friday 20 February 2009, Rafael J. Wysocki wrote:
> On Friday 20 February 2009, Kyle Moffett wrote:
> > On Fri, Feb 20, 2009 at 10:56 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > > * Introduce /sys/power/sleep that will work like /sys/power/state, but:
> > >  - First, it will call ->acknowledge() for each driver (via bus types) to
> > >    check if any of them wants to postpone the suspend (this will prevent tasks
> > >    from being frozen unnecessarily if it is known in advance that the suspend
> > >    should not happen at the moment).
> > >  - Next, it will check the "I_do_not_want_automatic_suspend_to_occur" flag
> > >    of each process and the suspend will be aborted if it is true for any of
> > >    them (quite frankly, I think that should be integrated with the freezer,
> > >    in particular the tasks that have TIF_FREEZE set shouldn't be able to set
> > >    this flag and it should be checked in the freezer loop for every task with
> > >    TIF_FREEZE unset).
> > >  - Next, it will proceed with suspending just like /sys/power/state does (the
> > >    drivers that missed the opportunity to abort the suspend by returning
> > >    'false' from ->acknowledge() can still abort the suspend by failing their
> > >    ->suspend() routines).
> > >
> > > Then, the decision making logic will be able to use /sys/power/sleep whenever
> > > it wishes to and the kernel will be able to refuse to suspend if it's not
> > > desirable at the moment.
> > 
> > Then, for a userspace process handling input events:
> > 
> > 1)  Unset "no-suspend flag"
> > 2)  Poll event device
> > 3)  When the poll returns true, set the "no-suspend flag"
> > 4)  Read the event device and process the event(s)
> > 5)  Go back to step 1
> > 
> > The event driver would responsible for atomically preventing suspend
> > from the moment a button is pressed until it is actually delivered to
> > a userspace process.  Since at that time the userspace process will be
> > preventing suspend, there will be no window in which we could go to
> > sleep with pending events.
> 
> That's correct.
> 
> How would you like to prevent it from happening?

To clarify, we're talking about automatic suspend only.  The "old" suspend
interface is going to stay so that one can force suspend (or hibernation) to
happen.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Freitag 20 Februar 2009 11:46:55 schrieb Rafael J. Wysocki:
> On Thursday 19 February 2009, Oliver Neukum wrote:
> > Am Donnerstag 19 Februar 2009 23:21:46 schrieb Rafael J. Wysocki:
> > > > That's the whole point behind userspace wakelocks!  They provide a
> > > > mechanism for userspace to tell the kernel when (as far as userspace
> >
> > is
> >
> > > > concerned) it is or is not okay to auto-sleep.
> > >
> > > Still, one can go further and observe that the user space can in fact
> > > start automatic suspend by itself whenever it knows it's appropriate
> > > ...
> >
> > User space initiating this is a race condition.
>
> Do you mean a race with the other user space processes or with the kernel?

With the set of runnable processes.There's always a window between
evaluating the current set of runnable tasks and telling the kernel to sleep.
IMO the most elegant solution would be a task attribute that would signal
the kernel that a task should not count as keeping the system busy even
if it is runnable and trigger the sleep in kernel space.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Fri, Feb 20, 2009 at 12:49 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Friday 20 February 2009, Matthew Garrett wrote:
>> On Fri, Feb 20, 2009 at 07:22:15PM +0100, Pavel Machek wrote:
>> >  On Mon 2009-02-16 23:23:30, Matthew Garrett wrote:
>> > > On Tue, Feb 17, 2009 at 12:19:38AM +0100, Rafael J. Wysocki wrote:
>> > >
>> > > > This, again, seems to be a bit x86-centric. :-)  The Android people are telling
>> > > > us that on the hardware they deal with it does make sense to put the entire
>> > > > system to sleep even for relatively short periods of time, since the latencies
>> > > > involved are not too bad.
>> > >
>> > > Arve said that the power state was equivalent in idle and suspend, but
>> >
>> > ...for Arve's hardware. Not all embedded systems are like that, and
>> > OLPC / PCs are definitely not like that.
>>
>> And PCs have enough latency over suspend/resume that we can't do it
>> automatically in a non-intrusive manner anyway, so runtime pm is more
>> interesting there as well.
>
> Agreed.

That high latency is coupled to a large difference in power use. If
the latency of the lowest power state is too high to use from idle, it
makes it more important, not less important, to enter suspend when
possible.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Friday 20 February 2009, Oliver Neukum wrote:
> Am Freitag 20 Februar 2009 11:46:55 schrieb Rafael J. Wysocki:
> > On Thursday 19 February 2009, Oliver Neukum wrote:
> > > Am Donnerstag 19 Februar 2009 23:21:46 schrieb Rafael J. Wysocki:
> > > > > That's the whole point behind userspace wakelocks!  They provide a
> > > > > mechanism for userspace to tell the kernel when (as far as userspace
> > >
> > > is
> > >
> > > > > concerned) it is or is not okay to auto-sleep.
> > > >
> > > > Still, one can go further and observe that the user space can in fact
> > > > start automatic suspend by itself whenever it knows it's appropriate
> > > > ...
> > >
> > > User space initiating this is a race condition.
> >
> > Do you mean a race with the other user space processes or with the kernel?
> 
> With the set of runnable processes.There's always a window between
> evaluating the current set of runnable tasks and telling the kernel to sleep.
> IMO the most elegant solution would be a task attribute that would signal
> the kernel that a task should not count as keeping the system busy even
> if it is runnable and trigger the sleep in kernel space.

Please see my last reply to Arve (http://lkml.org/lkml/2009/2/20/182) and tell
me what you think.

Best,
Rafael

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Fri, Feb 20, 2009 at 7:56 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Friday 20 February 2009, Arve Hjønnevåg wrote:
>> On Fri, Feb 20, 2009 at 2:49 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
>> >> On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>> >> >> > It might have to be platform-specific.  The Android people seem to have a
>> >> >> > pretty good idea of what criteria will work for them.
>> >> >>
>> >> >> I'd really like to know in what situations Androind is supposed to suspend
>> >> >> automatically.
>> >> >
>> >> > It might be better to ask in what situations Android is _not_ supposed
>> >> > to sleep automatically.  In other words, in what situations is a
>> >> > wakelock acquired?  Since the whole system is only a phone, this
>> >> > question should have a reasonably well-defined answer.
>> >>
>> >> On an android phone, any code that needs to run when the screen is off
>> >> must hold a wakelock (directly or indirectly). In general if an
>> >> application or the system is processing an event that may cause a user
>> >> notification (new email, incoming phone call, alarm, etc.) it needs to
>> >> prevent suspend. But, we also use wakelocks to upload stats or
>> >> download system updates in the background, and for media player or
>> >> (gps) data logging applications.
>> >
>> > All of this doesn't seem to require wakelocks acuired from kernel space.
>> > What do you need those wakelocks for?
>>
>> Most events do not originate in user-space. Alarms start in our alarm
>> driver which locks a wakelock when its timer interrupt occurs. This
>> wakelock stays locked until the user-space alarm manager calls the
>> driver to wait for the next alarm. I've described how input events are
>> handled before. Without kernel wakelocks, if the user space power
>> manager had already turned off the screen and decided to suspend right
>> before a wakeup key is pressed, then that key could sit in the
>> in-kernel input queue until another key is pressed. Even if the
>> user-space thread read the key event before being frozen, it cannot
>> abort the suspend operation that was already started.
>
> OK, so what about the following approach:
>
> * Keep the decision making logic (power manager etc.) in user space.  Reasons:
>  - It may be arbitrarily complicated
>  - It may include such things as s2ram quirks or hal quirks needed for some
>    graphics adapters

I don't using wakelocks in any different in this respect. When user
space decides that it is ok for the kernel to suspend it should have
performed all theses steps in both cases.

>
> * Have a per-process (per-task or per-thread group, but the former would be
>  easier IMO) "I_do_not_want_automatic_suspend_to_occur" flag.

A per-process (thread group) flag allows wakelocks to be implemented
in a user space library. A per-thread flag does not. However, I don't
see much benefit in tying this to a process instead of using a file
descriptor.

>
> * Add a new callback, say ->acknowledge(), to the set of each driver's PM
>  operations, that will be called to check if the driver has anything against
>  automatic suspend (true - suspend can happen right now, false - suspend can't
>  happen).
>
> * Introduce /sys/power/sleep that will work like /sys/power/state, but:
>  - First, it will call ->acknowledge() for each driver (via bus types) to
>    check if any of them wants to postpone the suspend (this will prevent tasks
>    from being frozen unnecessarily if it is known in advance that the suspend
>    should not happen at the moment).
>  - Next, it will check the "I_do_not_want_automatic_suspend_to_occur" flag
>    of each process and the suspend will be aborted if it is true for any of
>    them (quite frankly, I think that should be integrated with the freezer,
>    in particular the tasks that have TIF_FREEZE set shouldn't be able to set
>    this flag and it should be checked in the freezer loop for every task with
>    TIF_FREEZE unset).
>  - Next, it will proceed with suspending just like /sys/power/state does (the
>    drivers that missed the opportunity to abort the suspend by returning
>    'false' from ->acknowledge() can still abort the suspend by failing their
>    ->suspend() routines).
>
> Then, the decision making logic will be able to use /sys/power/sleep whenever
> it wishes to and the kernel will be able to refuse to suspend if it's not
> desirable at the moment.
>
> It seems to be flexible enough to me.

This seems flexible enough to avoid race conditions, but it forces the
user space power manager to poll when the kernel refuse suspend. Also,
like my original wakelock patches, it breaks sleep requests through
/sys/power/state when there are input events queued.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Fri, Feb 20, 2009 at 2:05 PM, Oliver Neukum <oliver@neukum.org> wrote:
> Am Freitag 20 Februar 2009 11:46:55 schrieb Rafael J. Wysocki:
>> On Thursday 19 February 2009, Oliver Neukum wrote:
>> > Am Donnerstag 19 Februar 2009 23:21:46 schrieb Rafael J. Wysocki:
>> > > > That's the whole point behind userspace wakelocks!  They provide a
>> > > > mechanism for userspace to tell the kernel when (as far as userspace
>> >
>> > is
>> >
>> > > > concerned) it is or is not okay to auto-sleep.
>> > >
>> > > Still, one can go further and observe that the user space can in fact
>> > > start automatic suspend by itself whenever it knows it's appropriate
>> > > ...
>> >
>> > User space initiating this is a race condition.
>>
>> Do you mean a race with the other user space processes or with the kernel?
>
> With the set of runnable processes.There's always a window between
> evaluating the current set of runnable tasks and telling the kernel to sleep.
> IMO the most elegant solution would be a task attribute that would signal
> the kernel that a task should not count as keeping the system busy even
> if it is runnable and trigger the sleep in kernel space.

It is not always safe to enter suspend when no tasks are runnable. For
instance, a key event could be on a user space queue, but the code
that reads from that queue has been paged out.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Freitag 20 Februar 2009 16:56:59 schrieb Rafael J. Wysocki:

> OK, so what about the following approach:
>
> * Keep the decision making logic (power manager etc.) in user space. 
> Reasons: - It may be arbitrarily complicated

Therefore something functionally equivalent to wakelocks
is needed.

>   - It may include such things as s2ram quirks or hal quirks needed for
> some graphics adapters

If you can't do that I'd say the situation is hopeless.
You need full support for all devices including graphics
cards.

> * Have a per-process (per-task or per-thread group, but the former would be
>   easier IMO) "I_do_not_want_automatic_suspend_to_occur" flag.

I think you got the default logic inverted.

> * Add a new callback, say ->acknowledge(), to the set of each driver's PM
>   operations, that will be called to check if the driver has anything
> against automatic suspend (true - suspend can happen right now, false -
> suspend can't happen).
>
> * Introduce /sys/power/sleep that will work like /sys/power/state, but:
>   - First, it will call ->acknowledge() for each driver (via bus types) to
>     check if any of them wants to postpone the suspend (this will prevent
> tasks from being frozen unnecessarily if it is known in advance that the
> suspend should not happen at the moment).

Ideally most devices should be already suspended, so I am not
sure a new callback is really needed. In any cases it is an optimization
only.

>   - Next, it will check the "I_do_not_want_automatic_suspend_to_occur" flag
>     of each process and the suspend will be aborted if it is true for any
> of them (quite frankly, I think that should be integrated with the freezer,
> in particular the tasks that have TIF_FREEZE set shouldn't be able to set
> this flag and it should be checked in the freezer loop for every task with
> TIF_FREEZE unset).

Why in this order?

>   - Next, it will proceed with suspending just like /sys/power/state does
> (the drivers that missed the opportunity to abort the suspend by returning
> 'false' from ->acknowledge() can still abort the suspend by failing their
> ->suspend() routines).
>
> Then, the decision making logic will be able to use /sys/power/sleep
> whenever it wishes to and the kernel will be able to refuse to suspend if
> it's not desirable at the moment.

I think everything that uses a "trigger" logic from user space is not
a good idea. This will necessary introduce a race between the decision
and the execution during which circumstances can change.

So it seems to me that an allow/disallow system from user space
would be better.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Samstag 21 Februar 2009 00:11:28 schrieb Arve Hjønnevåg:
> On Fri, Feb 20, 2009 at 2:05 PM, Oliver Neukum <oliver@neukum.org> wrote:
> > Am Freitag 20 Februar 2009 11:46:55 schrieb Rafael J. Wysocki:
> >> On Thursday 19 February 2009, Oliver Neukum wrote:

> > With the set of runnable processes.There's always a window between
> > evaluating the current set of runnable tasks and telling the kernel to
> > sleep. IMO the most elegant solution would be a task attribute that would
> > signal the kernel that a task should not count as keeping the system busy
> > even if it is runnable and trigger the sleep in kernel space.
>
> It is not always safe to enter suspend when no tasks are runnable. For
> instance, a key event could be on a user space queue, but the code
> that reads from that queue has been paged out.

In that case you'd have a task waiting for IO. The driver should refuse
to suspend or wake up the system as IO is completed. It may be
inefficient to suspend for such a presumably short time, but it is not
a correctness issue.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Saturday 21 February 2009, Oliver Neukum wrote:
> Am Freitag 20 Februar 2009 16:56:59 schrieb Rafael J. Wysocki:
> 
> > OK, so what about the following approach:
> >
> > * Keep the decision making logic (power manager etc.) in user space. 
> > Reasons: - It may be arbitrarily complicated
> 
> Therefore something functionally equivalent to wakelocks
> is needed.
> 
> >   - It may include such things as s2ram quirks or hal quirks needed for
> > some graphics adapters
> 
> If you can't do that I'd say the situation is hopeless.
> You need full support for all devices including graphics
> cards.

We can't do the quirks from the kernel and that's one of the reasons why I
think user space should start automatic suspend (it can do the quirks if
necessary).

> > * Have a per-process (per-task or per-thread group, but the former would be
> >   easier IMO) "I_do_not_want_automatic_suspend_to_occur" flag.
> 
> I think you got the default logic inverted.

Well, it may be "suspend_possible_right_now" just as well. :-)
 
> > * Add a new callback, say ->acknowledge(), to the set of each driver's PM
> >   operations, that will be called to check if the driver has anything
> > against automatic suspend (true - suspend can happen right now, false -
> > suspend can't happen).
> >
> > * Introduce /sys/power/sleep that will work like /sys/power/state, but:
> >   - First, it will call ->acknowledge() for each driver (via bus types) to
> >     check if any of them wants to postpone the suspend (this will prevent
> > tasks from being frozen unnecessarily if it is known in advance that the
> > suspend should not happen at the moment).
> 
> Ideally most devices should be already suspended, so I am not
> sure a new callback is really needed. In any cases it is an optimization
> only.

Yes, it is, but an important one (the total time of freezing and thawing tasks
can be substantial).

> >   - Next, it will check the "I_do_not_want_automatic_suspend_to_occur" flag
> >     of each process and the suspend will be aborted if it is true for any
> > of them (quite frankly, I think that should be integrated with the freezer,
> > in particular the tasks that have TIF_FREEZE set shouldn't be able to set
> > this flag and it should be checked in the freezer loop for every task with
> > TIF_FREEZE unset).
> 
> Why in this order?

Because the freezing of tasks takes time and it would be a bad idea to do it
unnecessarily.

> >   - Next, it will proceed with suspending just like /sys/power/state does
> > (the drivers that missed the opportunity to abort the suspend by returning
> > 'false' from ->acknowledge() can still abort the suspend by failing their
> > ->suspend() routines).
> >
> > Then, the decision making logic will be able to use /sys/power/sleep
> > whenever it wishes to and the kernel will be able to refuse to suspend if
> > it's not desirable at the moment.
> 
> I think everything that uses a "trigger" logic from user space is not
> a good idea. This will necessary introduce a race between the decision
> and the execution during which circumstances can change.

Well, in this particulare case if the circumstances change in the meantime,
the kernel will just refuse to suspend.  Also, even if the kernel starts
automatic suspend, it _still_ is possible that the conditions change in the
meantime (for example, one of the tasks may enter a state in which it wouldn't
like to the suspend to happen just after the operation is started and before
it's frozen).

> So it seems to me that an allow/disallow system from user space
> would be better.

I don't really see the benefit, but I can easily see drawbacks (the handling
of graphics that requires user space quirks, for instance).

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Matthew Garrett]

On Sat, Feb 21, 2009 at 12:40:48AM +0100, Rafael J. Wysocki wrote:

> We can't do the quirks from the kernel and that's one of the reasons why I
> think user space should start automatic suspend (it can do the quirks if
> necessary).

Any sort of automatic suspend is simply impossible in a world where your 
drivers don't work properly. There's code for making intel and radeon 
handle suspend/resume themselves now, and nvidia before g80 should also 
be easy. More recent nvidia is going to be some work.

But yeah. Let's not have "Work around broken drivers" as a design goal. 
The argument against timeouts in wakelocks works just as well here.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Saturday 21 February 2009, Arve Hjønnevåg wrote:
> On Fri, Feb 20, 2009 at 7:56 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
> >> On Fri, Feb 20, 2009 at 2:49 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
> >> >> On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> >> >> >> > It might have to be platform-specific.  The Android people seem to have a
> >> >> >> > pretty good idea of what criteria will work for them.
> >> >> >>
> >> >> >> I'd really like to know in what situations Androind is supposed to suspend
> >> >> >> automatically.
> >> >> >
> >> >> > It might be better to ask in what situations Android is _not_ supposed
> >> >> > to sleep automatically.  In other words, in what situations is a
> >> >> > wakelock acquired?  Since the whole system is only a phone, this
> >> >> > question should have a reasonably well-defined answer.
> >> >>
> >> >> On an android phone, any code that needs to run when the screen is off
> >> >> must hold a wakelock (directly or indirectly). In general if an
> >> >> application or the system is processing an event that may cause a user
> >> >> notification (new email, incoming phone call, alarm, etc.) it needs to
> >> >> prevent suspend. But, we also use wakelocks to upload stats or
> >> >> download system updates in the background, and for media player or
> >> >> (gps) data logging applications.
> >> >
> >> > All of this doesn't seem to require wakelocks acuired from kernel space.
> >> > What do you need those wakelocks for?
> >>
> >> Most events do not originate in user-space. Alarms start in our alarm
> >> driver which locks a wakelock when its timer interrupt occurs. This
> >> wakelock stays locked until the user-space alarm manager calls the
> >> driver to wait for the next alarm. I've described how input events are
> >> handled before. Without kernel wakelocks, if the user space power
> >> manager had already turned off the screen and decided to suspend right
> >> before a wakeup key is pressed, then that key could sit in the
> >> in-kernel input queue until another key is pressed. Even if the
> >> user-space thread read the key event before being frozen, it cannot
> >> abort the suspend operation that was already started.
> >
> > OK, so what about the following approach:
> >
> > * Keep the decision making logic (power manager etc.) in user space.  Reasons:
> >  - It may be arbitrarily complicated
> >  - It may include such things as s2ram quirks or hal quirks needed for some
> >    graphics adapters
> 
> I don't using wakelocks in any different in this respect. When user
> space decides that it is ok for the kernel to suspend it should have
> performed all theses steps in both cases.

If automatic suspend is to be started by the kernel, you have to make sure
that at least one wakelock is held until these steps are completed.  That may
be somewhat tricky in general.

> > * Have a per-process (per-task or per-thread group, but the former would be
> >  easier IMO) "I_do_not_want_automatic_suspend_to_occur" flag.
> 
> A per-process (thread group) flag allows wakelocks to be implemented
> in a user space library. A per-thread flag does not. However, I don't
> see much benefit in tying this to a process instead of using a file
> descriptor.

Using a flag would allow us to remove the window between checking the
wakelocks and freezing tasks (in principle a task may take a wakelock just
before it's frozen).

> >
> > * Add a new callback, say ->acknowledge(), to the set of each driver's PM
> >  operations, that will be called to check if the driver has anything against
> >  automatic suspend (true - suspend can happen right now, false - suspend can't
> >  happen).
> >
> > * Introduce /sys/power/sleep that will work like /sys/power/state, but:
> >  - First, it will call ->acknowledge() for each driver (via bus types) to
> >    check if any of them wants to postpone the suspend (this will prevent tasks
> >    from being frozen unnecessarily if it is known in advance that the suspend
> >    should not happen at the moment).
> >  - Next, it will check the "I_do_not_want_automatic_suspend_to_occur" flag
> >    of each process and the suspend will be aborted if it is true for any of
> >    them (quite frankly, I think that should be integrated with the freezer,
> >    in particular the tasks that have TIF_FREEZE set shouldn't be able to set
> >    this flag and it should be checked in the freezer loop for every task with
> >    TIF_FREEZE unset).
> >  - Next, it will proceed with suspending just like /sys/power/state does (the
> >    drivers that missed the opportunity to abort the suspend by returning
> >    'false' from ->acknowledge() can still abort the suspend by failing their
> >    ->suspend() routines).
> >
> > Then, the decision making logic will be able to use /sys/power/sleep whenever
> > it wishes to and the kernel will be able to refuse to suspend if it's not
> > desirable at the moment.
> >
> > It seems to be flexible enough to me.
> 
> This seems flexible enough to avoid race conditions, but it forces the
> user space power manager to poll when the kernel refuse suspend.

And if the kernel is supposed to start automatic suspend, it has to monitor
all of the wakelocks.  IMO, it's better to allow the power manager to poll the
kernel if it refuses to suspend.

> Also, like my original wakelock patches, it breaks sleep requests through
> /sys/power/state when there are input events queued.

The idea is to have both /sys/power/state and /sys/power/sleep at the same
time, where /sys/power/state will work just like it does right now.  Sure,
there must be mutual exclusion between the two, but that's a matter of
implementation IMO.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Fri, Feb 20, 2009 at 3:57 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Saturday 21 February 2009, Arve Hjønnevåg wrote:
>> On Fri, Feb 20, 2009 at 7:56 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
>> >> On Fri, Feb 20, 2009 at 2:49 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
>> >> >> On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>> >> >> >> > It might have to be platform-specific.  The Android people seem to have a
>> >> >> >> > pretty good idea of what criteria will work for them.
>> >> >> >>
>> >> >> >> I'd really like to know in what situations Androind is supposed to suspend
>> >> >> >> automatically.
>> >> >> >
>> >> >> > It might be better to ask in what situations Android is _not_ supposed
>> >> >> > to sleep automatically.  In other words, in what situations is a
>> >> >> > wakelock acquired?  Since the whole system is only a phone, this
>> >> >> > question should have a reasonably well-defined answer.
>> >> >>
>> >> >> On an android phone, any code that needs to run when the screen is off
>> >> >> must hold a wakelock (directly or indirectly). In general if an
>> >> >> application or the system is processing an event that may cause a user
>> >> >> notification (new email, incoming phone call, alarm, etc.) it needs to
>> >> >> prevent suspend. But, we also use wakelocks to upload stats or
>> >> >> download system updates in the background, and for media player or
>> >> >> (gps) data logging applications.
>> >> >
>> >> > All of this doesn't seem to require wakelocks acuired from kernel space.
>> >> > What do you need those wakelocks for?
>> >>
>> >> Most events do not originate in user-space. Alarms start in our alarm
>> >> driver which locks a wakelock when its timer interrupt occurs. This
>> >> wakelock stays locked until the user-space alarm manager calls the
>> >> driver to wait for the next alarm. I've described how input events are
>> >> handled before. Without kernel wakelocks, if the user space power
>> >> manager had already turned off the screen and decided to suspend right
>> >> before a wakeup key is pressed, then that key could sit in the
>> >> in-kernel input queue until another key is pressed. Even if the
>> >> user-space thread read the key event before being frozen, it cannot
>> >> abort the suspend operation that was already started.
>> >
>> > OK, so what about the following approach:
>> >
>> > * Keep the decision making logic (power manager etc.) in user space.  Reasons:
>> >  - It may be arbitrarily complicated
>> >  - It may include such things as s2ram quirks or hal quirks needed for some
>> >    graphics adapters
>>
>> I don't using wakelocks in any different in this respect. When user
>> space decides that it is ok for the kernel to suspend it should have
>> performed all theses steps in both cases.
>
> If automatic suspend is to be started by the kernel, you have to make sure
> that at least one wakelock is held until these steps are completed.  That may
> be somewhat tricky in general.

That is not tricky. If your code holds a wakelock, then at least one
wakelock is held.

>
>> > * Have a per-process (per-task or per-thread group, but the former would be
>> >  easier IMO) "I_do_not_want_automatic_suspend_to_occur" flag.
>>
>> A per-process (thread group) flag allows wakelocks to be implemented
>> in a user space library. A per-thread flag does not. However, I don't
>> see much benefit in tying this to a process instead of using a file
>> descriptor.
>
> Using a flag would allow us to remove the window between checking the
> wakelocks and freezing tasks (in principle a task may take a wakelock just
> before it's frozen).

You can check if there are wakelocks held from the same spot as you
check your flag.

>
>> >
>> > * Add a new callback, say ->acknowledge(), to the set of each driver's PM
>> >  operations, that will be called to check if the driver has anything against
>> >  automatic suspend (true - suspend can happen right now, false - suspend can't
>> >  happen).
>> >
>> > * Introduce /sys/power/sleep that will work like /sys/power/state, but:
>> >  - First, it will call ->acknowledge() for each driver (via bus types) to
>> >    check if any of them wants to postpone the suspend (this will prevent tasks
>> >    from being frozen unnecessarily if it is known in advance that the suspend
>> >    should not happen at the moment).
>> >  - Next, it will check the "I_do_not_want_automatic_suspend_to_occur" flag
>> >    of each process and the suspend will be aborted if it is true for any of
>> >    them (quite frankly, I think that should be integrated with the freezer,
>> >    in particular the tasks that have TIF_FREEZE set shouldn't be able to set
>> >    this flag and it should be checked in the freezer loop for every task with
>> >    TIF_FREEZE unset).
>> >  - Next, it will proceed with suspending just like /sys/power/state does (the
>> >    drivers that missed the opportunity to abort the suspend by returning
>> >    'false' from ->acknowledge() can still abort the suspend by failing their
>> >    ->suspend() routines).
>> >
>> > Then, the decision making logic will be able to use /sys/power/sleep whenever
>> > it wishes to and the kernel will be able to refuse to suspend if it's not
>> > desirable at the moment.
>> >
>> > It seems to be flexible enough to me.
>>
>> This seems flexible enough to avoid race conditions, but it forces the
>> user space power manager to poll when the kernel refuse suspend.
>
> And if the kernel is supposed to start automatic suspend, it has to monitor
> all of the wakelocks.  IMO, it's better to allow the power manager to poll the
> kernel if it refuses to suspend.

What is better about polling in userspace?

>
>> Also, like my original wakelock patches, it breaks sleep requests through
>> /sys/power/state when there are input events queued.
>
> The idea is to have both /sys/power/state and /sys/power/sleep at the same
> time, where /sys/power/state will work just like it does right now.  Sure,
> there must be mutual exclusion between the two, but that's a matter of
> implementation IMO.

If you want to only prevent suspend though one interface, you have to
also pass information to the driver about its suspend hook is being
called so it can conditionally return -EBUSY. The wakelock interface
requires less code in each driver.


-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Fri, Feb 20, 2009 at 3:20 PM, Oliver Neukum <oliver@neukum.org> wrote:
> Am Samstag 21 Februar 2009 00:11:28 schrieb Arve Hjønnevåg:
>> On Fri, Feb 20, 2009 at 2:05 PM, Oliver Neukum <oliver@neukum.org> wrote:
>> > Am Freitag 20 Februar 2009 11:46:55 schrieb Rafael J. Wysocki:
>> >> On Thursday 19 February 2009, Oliver Neukum wrote:
>
>> > With the set of runnable processes.There's always a window between
>> > evaluating the current set of runnable tasks and telling the kernel to
>> > sleep. IMO the most elegant solution would be a task attribute that would
>> > signal the kernel that a task should not count as keeping the system busy
>> > even if it is runnable and trigger the sleep in kernel space.
>>
>> It is not always safe to enter suspend when no tasks are runnable. For
>> instance, a key event could be on a user space queue, but the code
>> that reads from that queue has been paged out.
>
> In that case you'd have a task waiting for IO. The driver should refuse
> to suspend or wake up the system as IO is completed. It may be
> inefficient to suspend for such a presumably short time, but it is not
> a correctness issue.

In that case, yes. My point was that no-runnable-tasks is not a
sufficient indicator of whether suspend should fail or not. We have no
control of what a user space thread is waiting for though. It could be
waiting on a mutex that is locked by a thread that is marked not to
prevent suspend.


-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Saturday 21 February 2009, Arve Hjønnevåg wrote:
> On Fri, Feb 20, 2009 at 3:57 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Saturday 21 February 2009, Arve Hjønnevåg wrote:
> >> On Fri, Feb 20, 2009 at 7:56 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
> >> >> On Fri, Feb 20, 2009 at 2:49 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
> >> >> >> On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> >> >> >> >> > It might have to be platform-specific.  The Android people seem to have a
> >> >> >> >> > pretty good idea of what criteria will work for them.
> >> >> >> >>
> >> >> >> >> I'd really like to know in what situations Androind is supposed to suspend
> >> >> >> >> automatically.
> >> >> >> >
> >> >> >> > It might be better to ask in what situations Android is _not_ supposed
> >> >> >> > to sleep automatically.  In other words, in what situations is a
> >> >> >> > wakelock acquired?  Since the whole system is only a phone, this
> >> >> >> > question should have a reasonably well-defined answer.
> >> >> >>
> >> >> >> On an android phone, any code that needs to run when the screen is off
> >> >> >> must hold a wakelock (directly or indirectly). In general if an
> >> >> >> application or the system is processing an event that may cause a user
> >> >> >> notification (new email, incoming phone call, alarm, etc.) it needs to
> >> >> >> prevent suspend. But, we also use wakelocks to upload stats or
> >> >> >> download system updates in the background, and for media player or
> >> >> >> (gps) data logging applications.
> >> >> >
> >> >> > All of this doesn't seem to require wakelocks acuired from kernel space.
> >> >> > What do you need those wakelocks for?
> >> >>
> >> >> Most events do not originate in user-space. Alarms start in our alarm
> >> >> driver which locks a wakelock when its timer interrupt occurs. This
> >> >> wakelock stays locked until the user-space alarm manager calls the
> >> >> driver to wait for the next alarm. I've described how input events are
> >> >> handled before. Without kernel wakelocks, if the user space power
> >> >> manager had already turned off the screen and decided to suspend right
> >> >> before a wakeup key is pressed, then that key could sit in the
> >> >> in-kernel input queue until another key is pressed. Even if the
> >> >> user-space thread read the key event before being frozen, it cannot
> >> >> abort the suspend operation that was already started.
> >> >
> >> > OK, so what about the following approach:
> >> >
> >> > * Keep the decision making logic (power manager etc.) in user space.  Reasons:
> >> >  - It may be arbitrarily complicated
> >> >  - It may include such things as s2ram quirks or hal quirks needed for some
> >> >    graphics adapters
> >>
> >> I don't using wakelocks in any different in this respect. When user
> >> space decides that it is ok for the kernel to suspend it should have
> >> performed all theses steps in both cases.
> >
> > If automatic suspend is to be started by the kernel, you have to make sure
> > that at least one wakelock is held until these steps are completed.  That may
> > be somewhat tricky in general.
> 
> That is not tricky. If your code holds a wakelock, then at least one
> wakelock is held.

I meant, if one task releases its wakelock and another one takes a wakelock,
you have to make sure there's no window between those events.  Of course,
you can have a single task holding a wakelock all the time and releasing it
when it makes sure everything is ready, but then it would act as a user space
power manager anyway.

> >> > * Have a per-process (per-task or per-thread group, but the former would be
> >> >  easier IMO) "I_do_not_want_automatic_suspend_to_occur" flag.
> >>
> >> A per-process (thread group) flag allows wakelocks to be implemented
> >> in a user space library. A per-thread flag does not. However, I don't
> >> see much benefit in tying this to a process instead of using a file
> >> descriptor.
> >
> > Using a flag would allow us to remove the window between checking the
> > wakelocks and freezing tasks (in principle a task may take a wakelock just
> > before it's frozen).
> 
> You can check if there are wakelocks held from the same spot as you
> check your flag.

That would be _much_ more complicated and much less efficient (browsing a list
vs checking a flag).  And it would require additional locking if it's to be
done in a non-racy way.

> >> > * Add a new callback, say ->acknowledge(), to the set of each driver's PM
> >> >  operations, that will be called to check if the driver has anything against
> >> >  automatic suspend (true - suspend can happen right now, false - suspend can't
> >> >  happen).
> >> >
> >> > * Introduce /sys/power/sleep that will work like /sys/power/state, but:
> >> >  - First, it will call ->acknowledge() for each driver (via bus types) to
> >> >    check if any of them wants to postpone the suspend (this will prevent tasks
> >> >    from being frozen unnecessarily if it is known in advance that the suspend
> >> >    should not happen at the moment).
> >> >  - Next, it will check the "I_do_not_want_automatic_suspend_to_occur" flag
> >> >    of each process and the suspend will be aborted if it is true for any of
> >> >    them (quite frankly, I think that should be integrated with the freezer,
> >> >    in particular the tasks that have TIF_FREEZE set shouldn't be able to set
> >> >    this flag and it should be checked in the freezer loop for every task with
> >> >    TIF_FREEZE unset).
> >> >  - Next, it will proceed with suspending just like /sys/power/state does (the
> >> >    drivers that missed the opportunity to abort the suspend by returning
> >> >    'false' from ->acknowledge() can still abort the suspend by failing their
> >> >    ->suspend() routines).
> >> >
> >> > Then, the decision making logic will be able to use /sys/power/sleep whenever
> >> > it wishes to and the kernel will be able to refuse to suspend if it's not
> >> > desirable at the moment.
> >> >
> >> > It seems to be flexible enough to me.
> >>
> >> This seems flexible enough to avoid race conditions, but it forces the
> >> user space power manager to poll when the kernel refuse suspend.
> >
> > And if the kernel is supposed to start automatic suspend, it has to monitor
> > all of the wakelocks.  IMO, it's better to allow the power manager to poll the
> > kernel if it refuses to suspend.
> 
> What is better about polling in userspace?

One kernel thread less, for example?

> >> Also, like my original wakelock patches, it breaks sleep requests through
> >> /sys/power/state when there are input events queued.
> >
> > The idea is to have both /sys/power/state and /sys/power/sleep at the same
> > time, where /sys/power/state will work just like it does right now.  Sure,
> > there must be mutual exclusion between the two, but that's a matter of
> > implementation IMO.
> 
> If you want to only prevent suspend though one interface, you have to
> also pass information to the driver about its suspend hook is being
> called so it can conditionally return -EBUSY. The wakelock interface
> requires less code in each driver.

Well, I don't think so.  Moreover, it requires you to spread wakelocks all
over the place if you don't use the timeouted ones which, let's face it, is
hardly acceptable.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Saturday 21 February 2009, Matthew Garrett wrote:
> On Sat, Feb 21, 2009 at 12:40:48AM +0100, Rafael J. Wysocki wrote:
> 
> > We can't do the quirks from the kernel and that's one of the reasons why I
> > think user space should start automatic suspend (it can do the quirks if
> > necessary).
> 
> Any sort of automatic suspend is simply impossible in a world where your 
> drivers don't work properly. There's code for making intel and radeon 
> handle suspend/resume themselves now, and nvidia before g80 should also 
> be easy. More recent nvidia is going to be some work.
> 
> But yeah. Let's not have "Work around broken drivers" as a design goal. 

OK, I'm withdrawing it, then. :-)

> The argument against timeouts in wakelocks works just as well here.

So, what do you thing about the approach desribed earlier in this thread
(http://lkml.org/lkml/2009/2/20/182)?

Rafael

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Sat, Feb 21, 2009 at 1:47 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Saturday 21 February 2009, Arve Hjønnevåg wrote:
>> On Fri, Feb 20, 2009 at 3:57 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > On Saturday 21 February 2009, Arve Hjønnevåg wrote:
>> >> On Fri, Feb 20, 2009 at 7:56 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
>> >> >> On Fri, Feb 20, 2009 at 2:49 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
>> >> >> >> On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>> >> >> >> >> > It might have to be platform-specific.  The Android people seem to have a
>> >> >> >> >> > pretty good idea of what criteria will work for them.
>> >> >> >> >>
>> >> >> >> >> I'd really like to know in what situations Androind is supposed to suspend
>> >> >> >> >> automatically.
>> >> >> >> >
>> >> >> >> > It might be better to ask in what situations Android is _not_ supposed
>> >> >> >> > to sleep automatically.  In other words, in what situations is a
>> >> >> >> > wakelock acquired?  Since the whole system is only a phone, this
>> >> >> >> > question should have a reasonably well-defined answer.
>> >> >> >>
>> >> >> >> On an android phone, any code that needs to run when the screen is off
>> >> >> >> must hold a wakelock (directly or indirectly). In general if an
>> >> >> >> application or the system is processing an event that may cause a user
>> >> >> >> notification (new email, incoming phone call, alarm, etc.) it needs to
>> >> >> >> prevent suspend. But, we also use wakelocks to upload stats or
>> >> >> >> download system updates in the background, and for media player or
>> >> >> >> (gps) data logging applications.
>> >> >> >
>> >> >> > All of this doesn't seem to require wakelocks acuired from kernel space.
>> >> >> > What do you need those wakelocks for?
>> >> >>
>> >> >> Most events do not originate in user-space. Alarms start in our alarm
>> >> >> driver which locks a wakelock when its timer interrupt occurs. This
>> >> >> wakelock stays locked until the user-space alarm manager calls the
>> >> >> driver to wait for the next alarm. I've described how input events are
>> >> >> handled before. Without kernel wakelocks, if the user space power
>> >> >> manager had already turned off the screen and decided to suspend right
>> >> >> before a wakeup key is pressed, then that key could sit in the
>> >> >> in-kernel input queue until another key is pressed. Even if the
>> >> >> user-space thread read the key event before being frozen, it cannot
>> >> >> abort the suspend operation that was already started.
>> >> >
>> >> > OK, so what about the following approach:
>> >> >
>> >> > * Keep the decision making logic (power manager etc.) in user space.  Reasons:
>> >> >  - It may be arbitrarily complicated
>> >> >  - It may include such things as s2ram quirks or hal quirks needed for some
>> >> >    graphics adapters
>> >>
>> >> I don't using wakelocks in any different in this respect. When user
>> >> space decides that it is ok for the kernel to suspend it should have
>> >> performed all theses steps in both cases.
>> >
>> > If automatic suspend is to be started by the kernel, you have to make sure
>> > that at least one wakelock is held until these steps are completed.  That may
>> > be somewhat tricky in general.
>>
>> That is not tricky. If your code holds a wakelock, then at least one
>> wakelock is held.
>
> I meant, if one task releases its wakelock and another one takes a wakelock,
> you have to make sure there's no window between those events.  Of course,
> you can have a single task holding a wakelock all the time and releasing it
> when it makes sure everything is ready, but then it would act as a user space
> power manager anyway.

If you are turning on and off hardware in user space that cannot
survive a kernel suspend while it is on, then you hold a wakelock when
this hardware is on. It should not matter how many times the kernel
enter and exit suspend while your hardware is off.

>
>> >> > * Have a per-process (per-task or per-thread group, but the former would be
>> >> >  easier IMO) "I_do_not_want_automatic_suspend_to_occur" flag.
>> >>
>> >> A per-process (thread group) flag allows wakelocks to be implemented
>> >> in a user space library. A per-thread flag does not. However, I don't
>> >> see much benefit in tying this to a process instead of using a file
>> >> descriptor.
>> >
>> > Using a flag would allow us to remove the window between checking the
>> > wakelocks and freezing tasks (in principle a task may take a wakelock just
>> > before it's frozen).
>>
>> You can check if there are wakelocks held from the same spot as you
>> check your flag.
>
> That would be _much_ more complicated and much less efficient (browsing a list
> vs checking a flag).  And it would require additional locking if it's to be
> done in a non-racy way.

Browsing an empty list is not that inefficient. If you are concerned
about the overhead of the extra spinlock, you could check if the list
is empty before locking the spinlock.

>
>> >> > * Add a new callback, say ->acknowledge(), to the set of each driver's PM
>> >> >  operations, that will be called to check if the driver has anything against
>> >> >  automatic suspend (true - suspend can happen right now, false - suspend can't
>> >> >  happen).
>> >> >
>> >> > * Introduce /sys/power/sleep that will work like /sys/power/state, but:
>> >> >  - First, it will call ->acknowledge() for each driver (via bus types) to
>> >> >    check if any of them wants to postpone the suspend (this will prevent tasks
>> >> >    from being frozen unnecessarily if it is known in advance that the suspend
>> >> >    should not happen at the moment).
>> >> >  - Next, it will check the "I_do_not_want_automatic_suspend_to_occur" flag
>> >> >    of each process and the suspend will be aborted if it is true for any of
>> >> >    them (quite frankly, I think that should be integrated with the freezer,
>> >> >    in particular the tasks that have TIF_FREEZE set shouldn't be able to set
>> >> >    this flag and it should be checked in the freezer loop for every task with
>> >> >    TIF_FREEZE unset).
>> >> >  - Next, it will proceed with suspending just like /sys/power/state does (the
>> >> >    drivers that missed the opportunity to abort the suspend by returning
>> >> >    'false' from ->acknowledge() can still abort the suspend by failing their
>> >> >    ->suspend() routines).
>> >> >
>> >> > Then, the decision making logic will be able to use /sys/power/sleep whenever
>> >> > it wishes to and the kernel will be able to refuse to suspend if it's not
>> >> > desirable at the moment.
>> >> >
>> >> > It seems to be flexible enough to me.
>> >>
>> >> This seems flexible enough to avoid race conditions, but it forces the
>> >> user space power manager to poll when the kernel refuse suspend.
>> >
>> > And if the kernel is supposed to start automatic suspend, it has to monitor
>> > all of the wakelocks.  IMO, it's better to allow the power manager to poll the
>> > kernel if it refuses to suspend.
>>
>> What is better about polling in userspace?
>
> One kernel thread less, for example?
>
>> >> Also, like my original wakelock patches, it breaks sleep requests through
>> >> /sys/power/state when there are input events queued.
>> >
>> > The idea is to have both /sys/power/state and /sys/power/sleep at the same
>> > time, where /sys/power/state will work just like it does right now.  Sure,
>> > there must be mutual exclusion between the two, but that's a matter of
>> > implementation IMO.
>>
>> If you want to only prevent suspend though one interface, you have to
>> also pass information to the driver about its suspend hook is being
>> called so it can conditionally return -EBUSY. The wakelock interface
>> requires less code in each driver.
>
> Well, I don't think so.  Moreover, it requires you to spread wakelocks all
> over the place if you don't use the timeouted ones which, let's face it, is
> hardly acceptable.

Your method does not reduce the number of places that has to be
modified. Any component where we add a wakelock, you have to add a
suspend handler to abort suspend when we would have held a wakelock.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Alan Stern]

On Sat, 21 Feb 2009, Rafael J. Wysocki wrote:

> > I think everything that uses a "trigger" logic from user space is not
> > a good idea. This will necessary introduce a race between the decision
> > and the execution during which circumstances can change.
> 
> Well, in this particulare case if the circumstances change in the meantime,
> the kernel will just refuse to suspend.  Also, even if the kernel starts
> automatic suspend, it _still_ is possible that the conditions change in the
> meantime (for example, one of the tasks may enter a state in which it wouldn't
> like to the suspend to happen just after the operation is started and before
> it's frozen).
> 
> > So it seems to me that an allow/disallow system from user space
> > would be better.
> 
> I don't really see the benefit, but I can easily see drawbacks (the handling
> of graphics that requires user space quirks, for instance).

This discussion is circling around an important point: How should 
auto-sleep be initiated?

If userspace holds any wakelocks then the system mustn't auto-sleep.  
So auto-sleep can be initiated when the last userspace wakelock is
released.  That requires calling into the kernel anyway, so it isn't a
problem.

But what about kernel wakelocks?  Again, the simplest approach is to 
initiate an auto-sleep when the last one is released.  But now this 
depends on how the implementation works.

In Rafael's scheme there isn't really such a thing as a kernel 
wakelock.  Instead there are driver methods, so the only way to find 
out whether auto-sleep is allowed is to poll every driver.  This is not 
good for systems that want to auto-sleep as soon as possible.

A variant on the scheme would use a new field in the dev_pm_info 
structure.  I don't know if this is better or worse the a new method; 
it seems likely that the new method would have to work by checking the 
value of some field anyway.  In any case, it shares the drawback that 
polling is required.

If kernel wakelocks were implemented more like refcounts, then
releasing the last one could immediately initiate an auto-sleep.  The 
problem with refcounts is that you can't tell (for accounting or 
debugging purposes) who owns the outstanding references.  However we 
ought to be able to come up with something intermediate between a 
full-blown wakelock and a simple refcount that would satisfy everybody.  

For example, we could use _both_ a new field in dev_pm_info and a 
refcount.  Or even a per-cpu refcount, to avoid cache-line contention 
since drivers are likely to acquire and release these things quite 
often.

What about the overhead of having a permanent kernel thread that does
nothing but handle auto-sleeps?  This might well be an acceptable
tradeoff for many people.  Besides, you need something like it if a
driver wants to release the last wakelock while in interrupt context.  
Unless you fall back on polling -- and then you need a thread to do the
polling.

Alan Stern

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Saturday 21 February 2009, Arve Hjønnevåg wrote:
> On Sat, Feb 21, 2009 at 1:47 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Saturday 21 February 2009, Arve Hjønnevåg wrote:
> >> On Fri, Feb 20, 2009 at 3:57 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> > On Saturday 21 February 2009, Arve Hjønnevåg wrote:
> >> >> On Fri, Feb 20, 2009 at 7:56 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
> >> >> >> On Fri, Feb 20, 2009 at 2:49 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> >> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
> >> >> >> >> On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
[--snip--]
> >> > The idea is to have both /sys/power/state and /sys/power/sleep at the same
> >> > time, where /sys/power/state will work just like it does right now.  Sure,
> >> > there must be mutual exclusion between the two, but that's a matter of
> >> > implementation IMO.
> >>
> >> If you want to only prevent suspend though one interface, you have to
> >> also pass information to the driver about its suspend hook is being
> >> called so it can conditionally return -EBUSY. The wakelock interface
> >> requires less code in each driver.
> >
> > Well, I don't think so.  Moreover, it requires you to spread wakelocks all
> > over the place if you don't use the timeouted ones which, let's face it, is
> > hardly acceptable.
> 
> Your method does not reduce the number of places that has to be
> modified. Any component where we add a wakelock, you have to add a
> suspend handler to abort suspend when we would have held a wakelock.

Well, maybe not, but it doesn't introduce entirely new API for device drivers.
Instead, it extends the existing interfaces which I think is more appropriate.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Saturday 21 February 2009, Alan Stern wrote:
> On Sat, 21 Feb 2009, Rafael J. Wysocki wrote:
> 
> > > I think everything that uses a "trigger" logic from user space is not
> > > a good idea. This will necessary introduce a race between the decision
> > > and the execution during which circumstances can change.
> > 
> > Well, in this particulare case if the circumstances change in the meantime,
> > the kernel will just refuse to suspend.  Also, even if the kernel starts
> > automatic suspend, it _still_ is possible that the conditions change in the
> > meantime (for example, one of the tasks may enter a state in which it wouldn't
> > like to the suspend to happen just after the operation is started and before
> > it's frozen).
> > 
> > > So it seems to me that an allow/disallow system from user space
> > > would be better.
> > 
> > I don't really see the benefit, but I can easily see drawbacks (the handling
> > of graphics that requires user space quirks, for instance).
> 
> This discussion is circling around an important point: How should 
> auto-sleep be initiated?
> 
> If userspace holds any wakelocks then the system mustn't auto-sleep.  
> So auto-sleep can be initiated when the last userspace wakelock is
> released.  That requires calling into the kernel anyway, so it isn't a
> problem.
> 
> But what about kernel wakelocks?  Again, the simplest approach is to 
> initiate an auto-sleep when the last one is released.  But now this 
> depends on how the implementation works.
> 
> In Rafael's scheme there isn't really such a thing as a kernel 
> wakelock.  Instead there are driver methods, so the only way to find 
> out whether auto-sleep is allowed is to poll every driver.  This is not 
> good for systems that want to auto-sleep as soon as possible.

You have a valid point here, although I don't think it would be a problem in
practice (I don't expect the systems using automatic suspend to have that
many drivers).

> A variant on the scheme would use a new field in the dev_pm_info 
> structure.  I don't know if this is better or worse the a new method; 
> it seems likely that the new method would have to work by checking the 
> value of some field anyway.  In any case, it shares the drawback that 
> polling is required.
> 
> If kernel wakelocks were implemented more like refcounts, then
> releasing the last one could immediately initiate an auto-sleep.  The 
> problem with refcounts is that you can't tell (for accounting or 
> debugging purposes) who owns the outstanding references.  However we 
> ought to be able to come up with something intermediate between a 
> full-blown wakelock and a simple refcount that would satisfy everybody.  
> 
> For example, we could use _both_ a new field in dev_pm_info and a 
> refcount.  Or even a per-cpu refcount, to avoid cache-line contention 
> since drivers are likely to acquire and release these things quite 
> often.

Yes, I like this idea.

Automatic suspend will not occur if the refcount is greater than zero and then
the debug/stats code can use the field in dev_pm_info to report who had
increased it.

> What about the overhead of having a permanent kernel thread that does
> nothing but handle auto-sleeps?  This might well be an acceptable
> tradeoff for many people.  Besides, you need something like it if a
> driver wants to release the last wakelock while in interrupt context.  
> Unless you fall back on polling -- and then you need a thread to do the
> polling.

But that may be a user space process.

I generally think that deciding whether to start automatic suspend should
belong to the user space, because it may involve some policies and user
preferences etc.

If you agree with that, there are two possible approaches.  First, there may be
a kernel thread checking periodically if automatic suspend is possible and
initiating it if that's the case.  For this purpose, the user space has to be
able to tell the kernel thread whether it wants automatic suspend to happen
(this is where user space wakelocks are handy).  Second, there may be a user
space process calling the kernel whenever it finds that automatic suspend is
desirable.  I personally prefer the latter, since the user space process can
listen to some events and react to them as soon as they occur (also, it
adheres to the rule of thumb that if something can be implemented in user
space, it better should be left in there ;-)).

In both cases we need to be able to abort an already started automatic suspend
if the conditions change after the decision has been made and I think that
using a per-process flag for that would be efficient, since we have to freeze
the user space anyway, so we need to check some per-task flags for each task.
Still, it may be optimized a bit by using a refcount (that may be the same
refcount as for drivers IMO) that will be increased every time a process sets
its "automatic_suspend_undesirable" flag.  Then, we may export the refcount
via sysfs so that the user space power manager can monitor it and take its
value into account when deciding whether to start automatic suspend.

To summarize, we can:
* Use a refcount such that automatic suspend will only be possible if it's
  equal to zero (but that need not be the only criterion).
* Use a per-device flag in dev_pm_info that will be set whenever the device
  driver increases the refcount and unset whenever the driver decreases the
  refcount.
* Use a per-process flag that will be set whenever the process increases the
  refcount and unset whenever the process decreases the refcount.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> > > The recent descussion about the Android PM patches sent by Arve shows that
> > > there is a need to introduce a mechanism allowing us to:
> > > (1) automatically put the system as a whole into a sleep state (eg. suspend to
> > >     RAM) when it is found to be "idle", where the meaning of "idle" has to be
> > >     defined too,
> > 
> > Well; I'd prefer to keep the interface very minimal.
> > 
> > If no devices are being used,
> 
> We'll need some kind of infrastructure to detect such situations.

Yes, I like the latest 'atomic_t + bit in struct device' ideas...
 
> > and next wakeup is far enough in the future,
> 
> What exactly do you mean by "next wakeup"?

Next timer in kernel and next sleep/select timeout in userspace.

								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> > Again, the decision to trigger automatic suspend has to be based on some
> > well defined criteria and the (in)activity of devices seems to be one of them.
> 
> I don't know what criteria the system monitor would use.  It might have 
> to be platform-specific.  The Android people seem to have a pretty good 
> idea of what criteria will work for them.
> 
> Inactivity of devices isn't always a good criterion.  There might be a
> background task which wakes up every few seconds to do something as
> long as the system is awake, thereby keeping some device always active.  
> The activity from this background task shouldn't prevent an auto-sleep.

Then we'd need a interface to mark such background task... and what is
reasonable use for such task, anyway? Either its job is neccessary, or
it should not waste power.

IOW maybe it is easier to eliminate such stuff?
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Sonntag 22 Februar 2009 15:03:11 schrieb Pavel Machek:
> Hi!
>
> > > Again, the decision to trigger automatic suspend has to be based on
> > > some well defined criteria and the (in)activity of devices seems to be
> > > one of them.
> >
> > I don't know what criteria the system monitor would use.  It might have
> > to be platform-specific.  The Android people seem to have a pretty good
> > idea of what criteria will work for them.
> >
> > Inactivity of devices isn't always a good criterion.  There might be a
> > background task which wakes up every few seconds to do something as
> > long as the system is awake, thereby keeping some device always active.
> > The activity from this background task shouldn't prevent an auto-sleep.
>
> Then we'd need a interface to mark such background task... and what is
> reasonable use for such task, anyway? Either its job is neccessary, or
> it should not waste power.

Why? We have decided that some tasks are more important than
other tasks, to the point of starving some tasks. Posix real time
already does that.

We can just as well have a class of tasks less important than power
saving. They'd just run when power saving is not active for some other
reason. Just like other such schemes we end up with the problem
of priority inversion with locking.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Sat, Feb 21, 2009 at 12:20 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Saturday 21 February 2009, Arve Hjønnevåg wrote:
>> On Sat, Feb 21, 2009 at 1:47 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > On Saturday 21 February 2009, Arve Hjønnevåg wrote:
>> >> On Fri, Feb 20, 2009 at 3:57 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> > On Saturday 21 February 2009, Arve Hjønnevåg wrote:
>> >> >> On Fri, Feb 20, 2009 at 7:56 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
>> >> >> >> On Fri, Feb 20, 2009 at 2:49 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> >> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
>> >> >> >> >> On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> [--snip--]
>> >> > The idea is to have both /sys/power/state and /sys/power/sleep at the same
>> >> > time, where /sys/power/state will work just like it does right now.  Sure,
>> >> > there must be mutual exclusion between the two, but that's a matter of
>> >> > implementation IMO.
>> >>
>> >> If you want to only prevent suspend though one interface, you have to
>> >> also pass information to the driver about its suspend hook is being
>> >> called so it can conditionally return -EBUSY. The wakelock interface
>> >> requires less code in each driver.
>> >
>> > Well, I don't think so.  Moreover, it requires you to spread wakelocks all
>> > over the place if you don't use the timeouted ones which, let's face it, is
>> > hardly acceptable.
>>
>> Your method does not reduce the number of places that has to be
>> modified. Any component where we add a wakelock, you have to add a
>> suspend handler to abort suspend when we would have held a wakelock.
>
> Well, maybe not, but it doesn't introduce entirely new API for device drivers.
> Instead, it extends the existing interfaces which I think is more appropriate.

The existing interfaces require polling. I don't think extending these
interfaces to make the polling faster is a better solution than adding
an interface to avoid polling.

Also, with your solution, how would you modify evdev.c to prevent
suspend while the event queue is not empty. This code does not have
any suspend hooks and the queue is not tied to any thread.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Tuesday 24 February 2009, Arve Hjønnevåg wrote:
> On Sat, Feb 21, 2009 at 12:20 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Saturday 21 February 2009, Arve Hjønnevåg wrote:
> >> On Sat, Feb 21, 2009 at 1:47 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> > On Saturday 21 February 2009, Arve Hjønnevåg wrote:
> >> >> On Fri, Feb 20, 2009 at 3:57 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> >> > On Saturday 21 February 2009, Arve Hjønnevåg wrote:
> >> >> >> On Fri, Feb 20, 2009 at 7:56 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> >> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
> >> >> >> >> On Fri, Feb 20, 2009 at 2:49 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> >> >> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
> >> >> >> >> >> On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> > [--snip--]
> >> >> > The idea is to have both /sys/power/state and /sys/power/sleep at the same
> >> >> > time, where /sys/power/state will work just like it does right now.  Sure,
> >> >> > there must be mutual exclusion between the two, but that's a matter of
> >> >> > implementation IMO.
> >> >>
> >> >> If you want to only prevent suspend though one interface, you have to
> >> >> also pass information to the driver about its suspend hook is being
> >> >> called so it can conditionally return -EBUSY. The wakelock interface
> >> >> requires less code in each driver.
> >> >
> >> > Well, I don't think so.  Moreover, it requires you to spread wakelocks all
> >> > over the place if you don't use the timeouted ones which, let's face it, is
> >> > hardly acceptable.
> >>
> >> Your method does not reduce the number of places that has to be
> >> modified. Any component where we add a wakelock, you have to add a
> >> suspend handler to abort suspend when we would have held a wakelock.
> >
> > Well, maybe not, but it doesn't introduce entirely new API for device drivers.
> > Instead, it extends the existing interfaces which I think is more appropriate.
> 
> The existing interfaces require polling. I don't think extending these
> interfaces to make the polling faster is a better solution than adding
> an interface to avoid polling.
> 
> Also, with your solution, how would you modify evdev.c to prevent
> suspend while the event queue is not empty. This code does not have
> any suspend hooks and the queue is not tied to any thread.

Well, why do you need to modify evdev.c in the first place?

Rafael

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Mon, Feb 23, 2009 at 3:53 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday 24 February 2009, Arve Hjønnevåg wrote:
>> On Sat, Feb 21, 2009 at 12:20 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > On Saturday 21 February 2009, Arve Hjønnevåg wrote:
>> >> On Sat, Feb 21, 2009 at 1:47 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> > On Saturday 21 February 2009, Arve Hjønnevåg wrote:
>> >> >> On Fri, Feb 20, 2009 at 3:57 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> >> > On Saturday 21 February 2009, Arve Hjønnevåg wrote:
>> >> >> >> On Fri, Feb 20, 2009 at 7:56 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> >> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
>> >> >> >> >> On Fri, Feb 20, 2009 at 2:49 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> >> >> >> > On Friday 20 February 2009, Arve Hjønnevåg wrote:
>> >> >> >> >> >> On Thu, Feb 19, 2009 at 2:08 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
>> > [--snip--]
>> >> >> > The idea is to have both /sys/power/state and /sys/power/sleep at the same
>> >> >> > time, where /sys/power/state will work just like it does right now.  Sure,
>> >> >> > there must be mutual exclusion between the two, but that's a matter of
>> >> >> > implementation IMO.
>> >> >>
>> >> >> If you want to only prevent suspend though one interface, you have to
>> >> >> also pass information to the driver about its suspend hook is being
>> >> >> called so it can conditionally return -EBUSY. The wakelock interface
>> >> >> requires less code in each driver.
>> >> >
>> >> > Well, I don't think so.  Moreover, it requires you to spread wakelocks all
>> >> > over the place if you don't use the timeouted ones which, let's face it, is
>> >> > hardly acceptable.
>> >>
>> >> Your method does not reduce the number of places that has to be
>> >> modified. Any component where we add a wakelock, you have to add a
>> >> suspend handler to abort suspend when we would have held a wakelock.
>> >
>> > Well, maybe not, but it doesn't introduce entirely new API for device drivers.
>> > Instead, it extends the existing interfaces which I think is more appropriate.
>>
>> The existing interfaces require polling. I don't think extending these
>> interfaces to make the polling faster is a better solution than adding
>> an interface to avoid polling.
>>
>> Also, with your solution, how would you modify evdev.c to prevent
>> suspend while the event queue is not empty. This code does not have
>> any suspend hooks and the queue is not tied to any thread.
>
> Well, why do you need to modify evdev.c in the first place?

It is the only code that knows that there are unprocessed input events.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> > Arve said that the power state was equivalent in idle and suspend, but 
> > that they preferred suspend because it stopped any periodic timers. I'd 
> > be more interested in making sure that unnecessary timers aren't running 
> > than focusing on automatically entering system-wide suspend - Nokia have 
> > been managing this since 2005 with good results.
> 
> We'd totally agree that doing something about periodic timers would be a
> big win.  There's also the situation that the longest ARM linux can sit
> in idle right now is ~2s at a time (Arve can expand on the exact issue
> relating to a 32bit signed nanosecond value somewhere iirc), which we'd
> want to sort out as well.
> 
> Of course that still doesn't address userspace.  Aggressively going to
> suspend lets us compensate for userspace programs that do somewhat silly
> things (I agree that it would be best if they didn't but they do and
> getting *everyone* to write their userspace code to avoid spinning or
> avoid waking up on short-duration timers to poll is a losing battle).

Well, really? AFAICS, we can either

1) Android solution: let the polling code live, and modify everyone
that _really_ wants to run to use wakelocks.

2) Solution proposed here: fix the polling code.

IMNSHO 1) means modifying many applications, too... See android
marked, 50%+ apps request "keep system woke up" capability. Plus it
means modifying _good_ code, and leaving _bad_ code unfixed. So I'd
prefer 2). I believe ammount of work is pretty similar.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> > > Then, the decision making logic will be able to use /sys/power/sleep whenever
> > > it wishes to and the kernel will be able to refuse to suspend if it's not
> > > desirable at the moment.
> > >
> > > It seems to be flexible enough to me.
> > 
> > This seems flexible enough to avoid race conditions, but it forces the
> > user space power manager to poll when the kernel refuse suspend.
> 
> And if the kernel is supposed to start automatic suspend, it has to monitor
> all of the wakelocks.  IMO, it's better to allow the power manager to poll the
> kernel if it refuses to suspend.

polling is evil -- it keeps CPU wake up => wastes power.

Wakelocks done right are single atomic_t... and if you set it to 0,
you just unblock "sleeper" thread or something. Zero polling and very
simple...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> > > And if the kernel is supposed to start automatic suspend, it has to monitor
> > > all of the wakelocks.  IMO, it's better to allow the power manager to poll the
> > > kernel if it refuses to suspend.
> > 
> > What is better about polling in userspace?
> 
> One kernel thread less, for example?

One kernel thread is about as costly as one userland thread. And I
believe we should not design new code that _needs_ polling.

I think it is better to have kernel thread than userland polling us
all the time.

Now... on PC it will be "interesting". Your solution has great
advantage of being able to work with s2ram quirks... But s2ram quirks
are not an issue on embedded hardware or OLPC, and hopefully kernel
mode setting will fix it on PCs, too...
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

> To summarize, we can:
> * Use a refcount such that automatic suspend will only be possible if it's
>   equal to zero (but that need not be the only criterion).
> * Use a per-device flag in dev_pm_info that will be set whenever the device
>   driver increases the refcount and unset whenever the driver decreases the
>   refcount.
> * Use a per-process flag that will be set whenever the process increases the
>   refcount and unset whenever the process decreases the refcount.

Yes, that sounds sane, and that's how reasonable wakelock
implementation should look like.


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

On Mon 2009-02-23 15:04:22, Oliver Neukum wrote:
> Am Sonntag 22 Februar 2009 15:03:11 schrieb Pavel Machek:
> > Hi!
> >
> > > > Again, the decision to trigger automatic suspend has to be based on
> > > > some well defined criteria and the (in)activity of devices seems to be
> > > > one of them.
> > >
> > > I don't know what criteria the system monitor would use.  It might have
> > > to be platform-specific.  The Android people seem to have a pretty good
> > > idea of what criteria will work for them.
> > >
> > > Inactivity of devices isn't always a good criterion.  There might be a
> > > background task which wakes up every few seconds to do something as
> > > long as the system is awake, thereby keeping some device always active.
> > > The activity from this background task shouldn't prevent an auto-sleep.
> >
> > Then we'd need a interface to mark such background task... and what is
> > reasonable use for such task, anyway? Either its job is neccessary, or
> > it should not waste power.
> 
> Why? We have decided that some tasks are more important than
> other tasks, to the point of starving some tasks. Posix real time
> already does that.
> 
> We can just as well have a class of tasks less important than power
> saving. They'd just run when power saving is not active for some other
> reason. Just like other such schemes we end up with the problem
> of priority inversion with locking.

Ok, I guess this could be interesting in some cases... maybe. What are
real examples of such tasks?

Note that this will be fairly hard to implement due to all the
priority inversions; so it better be pretty important.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Friday 27 February 2009, Pavel Machek wrote:
> Hi!
> 
> > > > Then, the decision making logic will be able to use /sys/power/sleep whenever
> > > > it wishes to and the kernel will be able to refuse to suspend if it's not
> > > > desirable at the moment.
> > > >
> > > > It seems to be flexible enough to me.
> > > 
> > > This seems flexible enough to avoid race conditions, but it forces the
> > > user space power manager to poll when the kernel refuse suspend.
> > 
> > And if the kernel is supposed to start automatic suspend, it has to monitor
> > all of the wakelocks.  IMO, it's better to allow the power manager to poll the
> > kernel if it refuses to suspend.
> 
> polling is evil -- it keeps CPU wake up => wastes power.
> 
> Wakelocks done right are single atomic_t... and if you set it to 0,
> you just unblock "sleeper" thread or something. Zero polling and very
> simple...

Except that you have to check all of the wakelocks periodically in a loop =>
polling.  So?

Rafael

Re: [RFD] Automatic suspend

[Matthew Garrett]

On Fri, Feb 27, 2009 at 03:22:39PM +0100, Rafael J. Wysocki wrote:
> On Friday 27 February 2009, Pavel Machek wrote:
> > Wakelocks done right are single atomic_t... and if you set it to 0,
> > you just unblock "sleeper" thread or something. Zero polling and very
> > simple...
> 
> Except that you have to check all of the wakelocks periodically in a loop =>
> polling.  So?

Why do you need to check them? If you're taking this approach you just 
have something like:

suspend_unblock() {
	if (atomc_dec_and_test(&suspend_lock))
		suspend();
}

and then check that the lock count is still 0 after device_suspend(). 
There's no need to poll.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

Re: [RFD] Automatic suspend

[Alan Stern]

On Fri, 27 Feb 2009, Pavel Machek wrote:

> 
> > To summarize, we can:
> > * Use a refcount such that automatic suspend will only be possible if it's
> >   equal to zero (but that need not be the only criterion).
> > * Use a per-device flag in dev_pm_info that will be set whenever the device
> >   driver increases the refcount and unset whenever the driver decreases the
> >   refcount.
> > * Use a per-process flag that will be set whenever the process increases the
> >   refcount and unset whenever the process decreases the refcount.
> 
> Yes, that sounds sane, and that's how reasonable wakelock
> implementation should look like.

One small point: If you add a per-device flag and a per-process flag as 
described above, then drivers and processes must not acquire nested 
references.

Obviously this is fixable, but it's worth mentioning...

Alan Stern

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Freitag 27 Februar 2009 11:18:18 schrieb Pavel Machek:
> > We can just as well have a class of tasks less important than power
> > saving. They'd just run when power saving is not active for some other
> > reason. Just like other such schemes we end up with the problem
> > of priority inversion with locking.
>
> Ok, I guess this could be interesting in some cases... maybe. What are
> real examples of such tasks?

The classical example is Seti@HOME. Any monitoring tool like umtsmon
or kwifimanager.

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Chris Friesen]

Pavel Machek wrote:
> On Mon 2009-02-23 15:04:22, Oliver Neukum wrote:

>> We can just as well have a class of tasks less important than power
>> saving. They'd just run when power saving is not active for some other
>> reason. Just like other such schemes we end up with the problem
>> of priority inversion with locking.
> 
> Ok, I guess this could be interesting in some cases... maybe. What are
> real examples of such tasks?

Some people might put casual system monitoring tools into this 
category--top, xload, gkrellm, xclock, etc.

It'd be nice if the animated banner ad in my web browser could be 
treated this way. :)

Chris

Re: [RFD] Automatic suspend

[Pavel Machek]

On Fri 2009-02-27 15:22:39, Rafael J. Wysocki wrote:
> On Friday 27 February 2009, Pavel Machek wrote:
> > Hi!
> > 
> > > > > Then, the decision making logic will be able to use /sys/power/sleep whenever
> > > > > it wishes to and the kernel will be able to refuse to suspend if it's not
> > > > > desirable at the moment.
> > > > >
> > > > > It seems to be flexible enough to me.
> > > > 
> > > > This seems flexible enough to avoid race conditions, but it forces the
> > > > user space power manager to poll when the kernel refuse suspend.
> > > 
> > > And if the kernel is supposed to start automatic suspend, it has to monitor
> > > all of the wakelocks.  IMO, it's better to allow the power manager to poll the
> > > kernel if it refuses to suspend.
> > 
> > polling is evil -- it keeps CPU wake up => wastes power.
> > 
> > Wakelocks done right are single atomic_t... and if you set it to 0,
> > you just unblock "sleeper" thread or something. Zero polling and very
> > simple...
> 
> Except that you have to check all of the wakelocks periodically in a loop =>
> polling.  So?

No. I want to have single atomic_t for all the wakelocks... at least
in non-debug version. Debug version will be slower. I believe you
originally suggested that.
								Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

On Fri 2009-02-27 11:09:45, Chris Friesen wrote:
> Pavel Machek wrote:
>> On Mon 2009-02-23 15:04:22, Oliver Neukum wrote:
>
>>> We can just as well have a class of tasks less important than power
>>> saving. They'd just run when power saving is not active for some other
>>> reason. Just like other such schemes we end up with the problem
>>> of priority inversion with locking.
>>
>> Ok, I guess this could be interesting in some cases... maybe. What are
>> real examples of such tasks?
>
> Some people might put casual system monitoring tools into this  
> category--top, xload, gkrellm, xclock, etc.

I guess system monitoring should stop itself when screen is
blanked... no need to compute list of processes if user can't see the
output.

But I see that may be slightly hard for command-line tool like
top. .... but it still seems like right solution. Why waste power/cycles
displaying xclock at tty9 when user is looking at tty10?

> It'd be nice if the animated banner ad in my web browser could be  
> treated this way. :)

Well, my web browser already has an option 'animate once'. No need to
waste power with that at all :-).
							Pavel 
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Friday 27 February 2009, Matthew Garrett wrote:
> On Fri, Feb 27, 2009 at 03:22:39PM +0100, Rafael J. Wysocki wrote:
> > On Friday 27 February 2009, Pavel Machek wrote:
> > > Wakelocks done right are single atomic_t... and if you set it to 0,
> > > you just unblock "sleeper" thread or something. Zero polling and very
> > > simple...
> > 
> > Except that you have to check all of the wakelocks periodically in a loop =>
> > polling.  So?
> 
> Why do you need to check them? If you're taking this approach you just 
> have something like:
> 
> suspend_unblock() {
> 	if (atomc_dec_and_test(&suspend_lock))
> 		suspend();
> }
> 
> and then check that the lock count is still 0 after device_suspend(). 
> There's no need to poll.

I was talking about wakelocks as originally proposed.

Using a refcount along with per-driver and per-process flags (probably two
refcounts would be more efficient in fact) is the latest idea and I agree it
wouldn't require polling.  However, if you refer to that, please don't use the
name "wakelock". :-)

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Friday 27 February 2009, Pavel Machek wrote:
> On Fri 2009-02-27 15:22:39, Rafael J. Wysocki wrote:
> > On Friday 27 February 2009, Pavel Machek wrote:
> > > Hi!
> > > 
> > > > > > Then, the decision making logic will be able to use /sys/power/sleep whenever
> > > > > > it wishes to and the kernel will be able to refuse to suspend if it's not
> > > > > > desirable at the moment.
> > > > > >
> > > > > > It seems to be flexible enough to me.
> > > > > 
> > > > > This seems flexible enough to avoid race conditions, but it forces the
> > > > > user space power manager to poll when the kernel refuse suspend.
> > > > 
> > > > And if the kernel is supposed to start automatic suspend, it has to monitor
> > > > all of the wakelocks.  IMO, it's better to allow the power manager to poll the
> > > > kernel if it refuses to suspend.
> > > 
> > > polling is evil -- it keeps CPU wake up => wastes power.
> > > 
> > > Wakelocks done right are single atomic_t... and if you set it to 0,
> > > you just unblock "sleeper" thread or something. Zero polling and very
> > > simple...
> > 
> > Except that you have to check all of the wakelocks periodically in a loop =>
> > polling.  So?
> 
> No. I want to have single atomic_t for all the wakelocks... at least
> in non-debug version. Debug version will be slower. I believe you
> originally suggested that.

I did, but please don't call it "wakelocks".  It's confusing.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Friday 27 February 2009, Alan Stern wrote:
> On Fri, 27 Feb 2009, Pavel Machek wrote:
> 
> > 
> > > To summarize, we can:
> > > * Use a refcount such that automatic suspend will only be possible if it's
> > >   equal to zero (but that need not be the only criterion).
> > > * Use a per-device flag in dev_pm_info that will be set whenever the device
> > >   driver increases the refcount and unset whenever the driver decreases the
> > >   refcount.
> > > * Use a per-process flag that will be set whenever the process increases the
> > >   refcount and unset whenever the process decreases the refcount.
> > 
> > Yes, that sounds sane, and that's how reasonable wakelock
> > implementation should look like.
> 
> One small point: If you add a per-device flag and a per-process flag as 
> described above, then drivers and processes must not acquire nested 
> references.
> 
> Obviously this is fixable, but it's worth mentioning...

Yes, it's important to remeber IMO.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Fri, Feb 27, 2009 at 12:54 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Friday 27 February 2009, Matthew Garrett wrote:
>> On Fri, Feb 27, 2009 at 03:22:39PM +0100, Rafael J. Wysocki wrote:
>> > On Friday 27 February 2009, Pavel Machek wrote:
>> > > Wakelocks done right are single atomic_t... and if you set it to 0,
>> > > you just unblock "sleeper" thread or something. Zero polling and very
>> > > simple...
>> >
>> > Except that you have to check all of the wakelocks periodically in a loop =>
>> > polling.  So?
>>
>> Why do you need to check them? If you're taking this approach you just
>> have something like:
>>
>> suspend_unblock() {
>>       if (atomc_dec_and_test(&suspend_lock))
>>               suspend();
>> }
>>
>> and then check that the lock count is still 0 after device_suspend().
>> There's no need to poll.
>
> I was talking about wakelocks as originally proposed.

Can you be more specific? My wakelock implementation triggers suspend
when the active list becomes empty. No polling required.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Friday 27 February 2009, Arve Hjønnevåg wrote:
> On Fri, Feb 27, 2009 at 12:54 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Friday 27 February 2009, Matthew Garrett wrote:
> >> On Fri, Feb 27, 2009 at 03:22:39PM +0100, Rafael J. Wysocki wrote:
> >> > On Friday 27 February 2009, Pavel Machek wrote:
> >> > > Wakelocks done right are single atomic_t... and if you set it to 0,
> >> > > you just unblock "sleeper" thread or something. Zero polling and very
> >> > > simple...
> >> >
> >> > Except that you have to check all of the wakelocks periodically in a loop =>
> >> > polling.  So?
> >>
> >> Why do you need to check them? If you're taking this approach you just
> >> have something like:
> >>
> >> suspend_unblock() {
> >>       if (atomc_dec_and_test(&suspend_lock))
> >>               suspend();
> >> }
> >>
> >> and then check that the lock count is still 0 after device_suspend().
> >> There's no need to poll.
> >
> > I was talking about wakelocks as originally proposed.
> 
> Can you be more specific? My wakelock implementation triggers suspend
> when the active list becomes empty. No polling required.

Sorry, I overlooked and/or didn't remember that.  My bad.

Rafael

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Fri, Feb 27, 2009 at 12:55 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Friday 27 February 2009, Pavel Machek wrote:
>> On Fri 2009-02-27 15:22:39, Rafael J. Wysocki wrote:
>> > On Friday 27 February 2009, Pavel Machek wrote:
>> > > Hi!
>> > >
>> > > > > > Then, the decision making logic will be able to use /sys/power/sleep whenever
>> > > > > > it wishes to and the kernel will be able to refuse to suspend if it's not
>> > > > > > desirable at the moment.
>> > > > > >
>> > > > > > It seems to be flexible enough to me.
>> > > > >
>> > > > > This seems flexible enough to avoid race conditions, but it forces the
>> > > > > user space power manager to poll when the kernel refuse suspend.
>> > > >
>> > > > And if the kernel is supposed to start automatic suspend, it has to monitor
>> > > > all of the wakelocks.  IMO, it's better to allow the power manager to poll the
>> > > > kernel if it refuses to suspend.
>> > >
>> > > polling is evil -- it keeps CPU wake up => wastes power.
>> > >
>> > > Wakelocks done right are single atomic_t... and if you set it to 0,
>> > > you just unblock "sleeper" thread or something. Zero polling and very
>> > > simple...
>> >
>> > Except that you have to check all of the wakelocks periodically in a loop =>
>> > polling.  So?
>>
>> No. I want to have single atomic_t for all the wakelocks... at least
>> in non-debug version. Debug version will be slower. I believe you
>> originally suggested that.
>
> I did, but please don't call it "wakelocks".  It's confusing.

What you are talking about here is mostly an optimization of the
wakelock api. You have removed timeout support and made each wakelock
reference counted. If you ignore wakelocks with timeouts, the current
wakelock interface can be implemented with a global atomic_t to
prevent suspend, and a per wakelock atomic_t to prevent a single
client from changing the global reference count by more than one.

There are a couple of reasons that I have not done this. It removes
the ability to easily inspect the system when it is not suspending. I
do provide an option to turn off the wakelock stats, which makes
wake_lock/unlock significantly faster, but we never run with wakelock
stats off. Also, it pushes timeout handling to the drivers. I know may
of you don't like timeout support, but ignoring the problem is not a
solution. If each driver that needs timeouts uses its own timer, then
you will often wakeup from idle just to unlock a wakelock that will
not trigger suspend. This wakeup is a thousand times as costly on the
msm platform as a wakelock/unlock pair (with wakelock stats enabled).

I just checked my phone, and over a 24 hour awake time (370 hours
uptime) period, it acquired about 5 million wakelocks (mostly for
input events). If these were cache hits, and took as long as my
benchmark did, that accounts for 20 seconds of overhead (0.023% of
awake, 0.1% of not-idle (5.5h).

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> >> > > Wakelocks done right are single atomic_t... and if you set it to 0,
> >> > > you just unblock "sleeper" thread or something. Zero polling and very
> >> > > simple...
> >> >
> >> > Except that you have to check all of the wakelocks periodically in a loop =>
> >> > polling.  So?
> >>
> >> No. I want to have single atomic_t for all the wakelocks... at least
> >> in non-debug version. Debug version will be slower. I believe you
> >> originally suggested that.
> >
> > I did, but please don't call it "wakelocks".  It's confusing.
> 
> What you are talking about here is mostly an optimization of the
> wakelock api. You have removed timeout support and made each wakelock
> reference counted. If you ignore wakelocks with timeouts, the current
> wakelock interface can be implemented with a global atomic_t to
> prevent suspend, and a per wakelock atomic_t to prevent a single
> client from changing the global reference count by more than one.

Actually I'd go to global atomic_t to prevent suspend, and make sure
single client only changes it once by design.

> There are a couple of reasons that I have not done this. It removes
> the ability to easily inspect the system when it is not suspending. I
> do provide an option to turn off the wakelock stats, which makes
> wake_lock/unlock significantly faster, but we never run with wakelock
> stats off.

It seems wrong to design for the "debugging" case. ... even if you run
with debugging enabled.

> Also, it pushes timeout handling to the drivers. I know
> may
> of you don't like timeout support, but ignoring the problem is not a
> solution. If each driver that needs timeouts uses its own timer, then
> you will often wakeup from idle just to unlock a wakelock that will
> not trigger suspend. This wakeup is a thousand times as costly on the
> msm platform as a wakelock/unlock pair (with wakelock stats
> enabled).

Well, you are free to have a library (or something) those broken
drivers can use. But you have to understand that those drivers are
broken... and please don't make it part of core API.

It should also make the merging easier; merge non-contraversial parts
first (single atomic_t), then add debugging infrastructure, then maybe
add timeout handling if those drivers can really not be fixed in a
nicer way...
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Fri, Feb 27, 2009 at 3:44 PM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> >> > > Wakelocks done right are single atomic_t... and if you set it to 0,
>> >> > > you just unblock "sleeper" thread or something. Zero polling and very
>> >> > > simple...
>> >> >
>> >> > Except that you have to check all of the wakelocks periodically in a loop =>
>> >> > polling.  So?
>> >>
>> >> No. I want to have single atomic_t for all the wakelocks... at least
>> >> in non-debug version. Debug version will be slower. I believe you
>> >> originally suggested that.
>> >
>> > I did, but please don't call it "wakelocks".  It's confusing.
>>
>> What you are talking about here is mostly an optimization of the
>> wakelock api. You have removed timeout support and made each wakelock
>> reference counted. If you ignore wakelocks with timeouts, the current
>> wakelock interface can be implemented with a global atomic_t to
>> prevent suspend, and a per wakelock atomic_t to prevent a single
>> client from changing the global reference count by more than one.
>
> Actually I'd go to global atomic_t to prevent suspend, and make sure
> single client only changes it once by design.

If you go with only a global atomic_t then you automatically allow
clients to change the count by more than one. Our userland api calls
this a reference counted wakelock, and reference counted wakelocks
have been a frequent source of bugs. We have code that calls wake_lock
every time an item is added to a queue and wake_unlock when it gets a
notification that item was removed. If you miss a notification the
lock is never released. With non reference counted wakelocks, can
unlock every time the queue is empty, and lock either when adding the
first item or every time you add an item (useful when combined with
timeouts).

I could extend the wakelock api to allow reference counted wakelocks,
but I have not come across any drivers where it would be beneficial
(to the driver, not the wakelock implementation), and causes conflicts
with timeouts.

>> There are a couple of reasons that I have not done this. It removes
>> the ability to easily inspect the system when it is not suspending. I
>> do provide an option to turn off the wakelock stats, which makes
>> wake_lock/unlock significantly faster, but we never run with wakelock
>> stats off.
>
> It seems wrong to design for the "debugging" case. ... even if you run
> with debugging enabled.

I'm not sure I would call wakelock stats purely a debugging feature.
Either way, designing the api to pass in an object, allows stats and
debugging features to be implemented. Designing the api to be a global
reference count without an object prevents both.

I realize that some of the proposed alternatives allow the same stats
and debugging information to be added to other kernel objects (tasks
and devices), but I prefer a separate wakelock object. We have drivers
that use more then one wakelock per device, and many wakelocks do not
have any association with a task either.

>
>> Also, it pushes timeout handling to the drivers. I know
>> may
>> of you don't like timeout support, but ignoring the problem is not a
>> solution. If each driver that needs timeouts uses its own timer, then
>> you will often wakeup from idle just to unlock a wakelock that will
>> not trigger suspend. This wakeup is a thousand times as costly on the
>> msm platform as a wakelock/unlock pair (with wakelock stats
>> enabled).
>
> Well, you are free to have a library (or something) those broken
> drivers can use. But you have to understand that those drivers are
> broken... and please don't make it part of core API.

I don't agree. Not all the timeout uses are broken. The unknown-wakeup
wakelock that is used if suspend fails preserved compatibility with
existing drivers without introducing any race conditions. I also use a
wakelock with a one second timeout if the alarm driver is suspended
less than a second before the alarm is supposed to expire. I could
probably rewrite this driver to not use a timeout, but the current
implementation is not broken.

> It should also make the merging easier; merge non-contraversial parts
> first (single atomic_t), then add debugging infrastructure, then maybe
> add timeout handling if those drivers can really not be fixed in a
> nicer way...

Why is a single atomic_t less controversial? It is just as invasive as
the wakelock interface and it provides no help in interacting with
unmodified subsystems.

I'm more interested in getting code merged that can easily be used in
a fully functional system, than code that only works for test systems,
but if the implementation details of the wakelock interface is the
only thing that prevents it from being merged, I can change it. I do
find it odd that this is what is objectionable though, as it is
already more efficient than the pm_qos interface that people want us
to use instead of idle wakelocks.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Friday 27 February 2009, Arve Hjønnevåg wrote:
> On Fri, Feb 27, 2009 at 12:55 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Friday 27 February 2009, Pavel Machek wrote:
> >> On Fri 2009-02-27 15:22:39, Rafael J. Wysocki wrote:
> >> > On Friday 27 February 2009, Pavel Machek wrote:
> >> > > Hi!
> >> > >
> >> > > > > > Then, the decision making logic will be able to use /sys/power/sleep whenever
> >> > > > > > it wishes to and the kernel will be able to refuse to suspend if it's not
> >> > > > > > desirable at the moment.
> >> > > > > >
> >> > > > > > It seems to be flexible enough to me.
> >> > > > >
> >> > > > > This seems flexible enough to avoid race conditions, but it forces the
> >> > > > > user space power manager to poll when the kernel refuse suspend.
> >> > > >
> >> > > > And if the kernel is supposed to start automatic suspend, it has to monitor
> >> > > > all of the wakelocks.  IMO, it's better to allow the power manager to poll the
> >> > > > kernel if it refuses to suspend.
> >> > >
> >> > > polling is evil -- it keeps CPU wake up => wastes power.
> >> > >
> >> > > Wakelocks done right are single atomic_t... and if you set it to 0,
> >> > > you just unblock "sleeper" thread or something. Zero polling and very
> >> > > simple...
> >> >
> >> > Except that you have to check all of the wakelocks periodically in a loop =>
> >> > polling.  So?
> >>
> >> No. I want to have single atomic_t for all the wakelocks... at least
> >> in non-debug version. Debug version will be slower. I believe you
> >> originally suggested that.
> >
> > I did, but please don't call it "wakelocks".  It's confusing.
> 
> What you are talking about here is mostly an optimization of the
> wakelock api. You have removed timeout support and made each wakelock
> reference counted.

I also removed the arbitrary number of wakelocks (I really _hate_ the name,
can we please stop using it from now on?).

> If you ignore wakelocks with timeouts, the current
> wakelock interface can be implemented with a global atomic_t to
> prevent suspend, and a per wakelock atomic_t to prevent a single
> client from changing the global reference count by more than one.
> 
> There are a couple of reasons that I have not done this. It removes
> the ability to easily inspect the system when it is not suspending.

Care to elaborate?

> I do provide an option to turn off the wakelock stats, which makes
> wake_lock/unlock significantly faster, but we never run with wakelock
> stats off. Also, it pushes timeout handling to the drivers. I know may
> of you don't like timeout support, but ignoring the problem is not a
> solution. If each driver that needs timeouts uses its own timer, then
> you will often wakeup from idle just to unlock a wakelock that will
> not trigger suspend. This wakeup is a thousand times as costly on the
> msm platform as a wakelock/unlock pair (with wakelock stats enabled).

Well, at least a couple of people told you that the timeouts are hardly
acceptable and they told you why.  Please stop repeating the same arguments you
have given already for a couple of times.  They're not convincing.

Instead of trying to convince everyone to accept your solution that you're
not willing to change, please try to listen and think how to do things
differently so that everyone is comfortable with them.  I'm sure you're more
than capable of doing that.

I do realize that you consider your current solution as the best thing since
the sliced bread, but please accept the fact that the other people think
differently.

> I just checked my phone, and over a 24 hour awake time (370 hours
> uptime) period, it acquired about 5 million wakelocks (mostly for
> input events). If these were cache hits, and took as long as my
> benchmark did, that accounts for 20 seconds of overhead (0.023% of
> awake, 0.1% of not-idle (5.5h).

Which proves what exactly?

Rafael

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Sat, Feb 28, 2009 at 2:18 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Friday 27 February 2009, Arve Hjønnevåg wrote:
>> On Fri, Feb 27, 2009 at 12:55 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > On Friday 27 February 2009, Pavel Machek wrote:
>> >> On Fri 2009-02-27 15:22:39, Rafael J. Wysocki wrote:
>> >> > On Friday 27 February 2009, Pavel Machek wrote:
>> >> > > Hi!
>> >> > >
>> >> > > > > > Then, the decision making logic will be able to use /sys/power/sleep whenever
>> >> > > > > > it wishes to and the kernel will be able to refuse to suspend if it's not
>> >> > > > > > desirable at the moment.
>> >> > > > > >
>> >> > > > > > It seems to be flexible enough to me.
>> >> > > > >
>> >> > > > > This seems flexible enough to avoid race conditions, but it forces the
>> >> > > > > user space power manager to poll when the kernel refuse suspend.
>> >> > > >
>> >> > > > And if the kernel is supposed to start automatic suspend, it has to monitor
>> >> > > > all of the wakelocks.  IMO, it's better to allow the power manager to poll the
>> >> > > > kernel if it refuses to suspend.
>> >> > >
>> >> > > polling is evil -- it keeps CPU wake up => wastes power.
>> >> > >
>> >> > > Wakelocks done right are single atomic_t... and if you set it to 0,
>> >> > > you just unblock "sleeper" thread or something. Zero polling and very
>> >> > > simple...
>> >> >
>> >> > Except that you have to check all of the wakelocks periodically in a loop =>
>> >> > polling.  So?
>> >>
>> >> No. I want to have single atomic_t for all the wakelocks... at least
>> >> in non-debug version. Debug version will be slower. I believe you
>> >> originally suggested that.
>> >
>> > I did, but please don't call it "wakelocks".  It's confusing.
>>
>> What you are talking about here is mostly an optimization of the
>> wakelock api. You have removed timeout support and made each wakelock
>> reference counted.
>
> I also removed the arbitrary number of wakelocks (I really _hate_ the name,
> can we please stop using it from now on?).

What do you mean by this? You removed the struct wake_lock?

>
>> If you ignore wakelocks with timeouts, the current
>> wakelock interface can be implemented with a global atomic_t to
>> prevent suspend, and a per wakelock atomic_t to prevent a single
>> client from changing the global reference count by more than one.
>>
>> There are a couple of reasons that I have not done this. It removes
>> the ability to easily inspect the system when it is not suspending.
>
> Care to elaborate?

If you have a single atomic_t and it is not 0, you do not know who
incremented it.

>> I do provide an option to turn off the wakelock stats, which makes
>> wake_lock/unlock significantly faster, but we never run with wakelock
>> stats off. Also, it pushes timeout handling to the drivers. I know may
>> of you don't like timeout support, but ignoring the problem is not a
>> solution. If each driver that needs timeouts uses its own timer, then
>> you will often wakeup from idle just to unlock a wakelock that will
>> not trigger suspend. This wakeup is a thousand times as costly on the
>> msm platform as a wakelock/unlock pair (with wakelock stats enabled).
>
> Well, at least a couple of people told you that the timeouts are hardly
> acceptable and they told you why.  Please stop repeating the same arguments you
> have given already for a couple of times.  They're not convincing.

And you keep ignoring them.

> Instead of trying to convince everyone to accept your solution that you're
> not willing to change, please try to listen and think how to do things
> differently so that everyone is comfortable with them.  I'm sure you're more
> than capable of doing that.

Can you summarize what the problems with my current api are? I get the
impression that you think the overhead of using a list is too high,
and that timeout support should be removed because you think all
drivers that use it are broken.

> I do realize that you consider your current solution as the best thing since
> the sliced bread, but please accept the fact that the other people think
> differently.

I certainly do not think my current solution is the best, it is very
invasive. I do however think your proposed solution is worse. The only
proposed alternative that we could actually ship a product on today is
to not use suspend at all.

>
>> I just checked my phone, and over a 24 hour awake time (370 hours
>> uptime) period, it acquired about 5 million wakelocks (mostly for
>> input events). If these were cache hits, and took as long as my
>> benchmark did, that accounts for 20 seconds of overhead (0.023% of
>> awake, 0.1% of not-idle (5.5h).
>
> Which proves what exactly?

You seem to be mainly focused on the overhead of the lock/unlock path,
so I thought some numbers would be useful.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Saturday 28 February 2009, Arve Hjønnevåg wrote:
> On Sat, Feb 28, 2009 at 2:18 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Friday 27 February 2009, Arve Hjønnevåg wrote:
> >> On Fri, Feb 27, 2009 at 12:55 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> > On Friday 27 February 2009, Pavel Machek wrote:
> >> >> On Fri 2009-02-27 15:22:39, Rafael J. Wysocki wrote:
> >> >> > On Friday 27 February 2009, Pavel Machek wrote:
> >> >> > > Hi!
> >> >> > >
> >> >> > > > > > Then, the decision making logic will be able to use /sys/power/sleep whenever
> >> >> > > > > > it wishes to and the kernel will be able to refuse to suspend if it's not
> >> >> > > > > > desirable at the moment.
> >> >> > > > > >
> >> >> > > > > > It seems to be flexible enough to me.
> >> >> > > > >
> >> >> > > > > This seems flexible enough to avoid race conditions, but it forces the
> >> >> > > > > user space power manager to poll when the kernel refuse suspend.
> >> >> > > >
> >> >> > > > And if the kernel is supposed to start automatic suspend, it has to monitor
> >> >> > > > all of the wakelocks.  IMO, it's better to allow the power manager to poll the
> >> >> > > > kernel if it refuses to suspend.
> >> >> > >
> >> >> > > polling is evil -- it keeps CPU wake up => wastes power.
> >> >> > >
> >> >> > > Wakelocks done right are single atomic_t... and if you set it to 0,
> >> >> > > you just unblock "sleeper" thread or something. Zero polling and very
> >> >> > > simple...
> >> >> >
> >> >> > Except that you have to check all of the wakelocks periodically in a loop =>
> >> >> > polling.  So?
> >> >>
> >> >> No. I want to have single atomic_t for all the wakelocks... at least
> >> >> in non-debug version. Debug version will be slower. I believe you
> >> >> originally suggested that.
> >> >
> >> > I did, but please don't call it "wakelocks".  It's confusing.
> >>
> >> What you are talking about here is mostly an optimization of the
> >> wakelock api. You have removed timeout support and made each wakelock
> >> reference counted.
> >
> > I also removed the arbitrary number of wakelocks (I really _hate_ the name,
> > can we please stop using it from now on?).
> 
> What do you mean by this? You removed the struct wake_lock?

Basically, yes.

> >> If you ignore wakelocks with timeouts, the current
> >> wakelock interface can be implemented with a global atomic_t to
> >> prevent suspend, and a per wakelock atomic_t to prevent a single
> >> client from changing the global reference count by more than one.
> >>
> >> There are a couple of reasons that I have not done this. It removes
> >> the ability to easily inspect the system when it is not suspending.
> >
> > Care to elaborate?
> 
> If you have a single atomic_t and it is not 0, you do not know who
> incremented it.

That's why it was proposed to use per-process and per-driver flags in addition
to the refcount.

> >> I do provide an option to turn off the wakelock stats, which makes
> >> wake_lock/unlock significantly faster, but we never run with wakelock
> >> stats off. Also, it pushes timeout handling to the drivers. I know may
> >> of you don't like timeout support, but ignoring the problem is not a
> >> solution. If each driver that needs timeouts uses its own timer, then
> >> you will often wakeup from idle just to unlock a wakelock that will
> >> not trigger suspend. This wakeup is a thousand times as costly on the
> >> msm platform as a wakelock/unlock pair (with wakelock stats enabled).
> >
> > Well, at least a couple of people told you that the timeouts are hardly
> > acceptable and they told you why.  Please stop repeating the same arguments you
> > have given already for a couple of times.  They're not convincing.
> 
> And you keep ignoring them.

Not ignoring, but considering them as insufficient.  And since they've already
been considered as insufficient, there's no point repeating them over and over
again.  That doesn't make them any better.

> > Instead of trying to convince everyone to accept your solution that you're
> > not willing to change, please try to listen and think how to do things
> > differently so that everyone is comfortable with them.  I'm sure you're more
> > than capable of doing that.
> 
> Can you summarize what the problems with my current api are? I get the
> impression that you think the overhead of using a list is too high,
> and that timeout support should be removed because you think all
> drivers that use it are broken.

In no particular order:
1. One user space process can create an unlimited number of wakelocks.  This
   shouldn't be possible.  Moreover, it is not even necessary for any process
   to have more than one wakelock held at any time.
2. Timeouts are wrong, because they don't really _solve_ any problem.  They are
   useful for working around the fact that you can't or you don't want to
   modify every piece of code that in principle should take a wakelock and
   that's it.  However,  entire concept of having one code path acting on
   behalf of another one on a hunch that it might be doing something making
   suspend undesirable is conceptually broken IMO.
3. The overhead of using a list is unnecessary and _therefore_ too high (not
   just too high).
4. There seems to be a race between user space wakelocks and the freezer
   (perhaps I overlooked something, in which case please disregard this item).
5. The name "wakelocks" is confusing, because they aren't locks and they
   affect suspend, not wake.
6. Last time I saw the patches they were barely commented and the changelogs
   didn't describe the code well (if at all).
7. There's no clear distinction between debug/stats code and the basic
   functionality.

I'm sure the other people could add something to the list.

> > I do realize that you consider your current solution as the best thing since
> > the sliced bread, but please accept the fact that the other people think
> > differently.
> 
> I certainly do not think my current solution is the best, it is very
> invasive. I do however think your proposed solution is worse. The only
> proposed alternative that we could actually ship a product on today is
> to not use suspend at all.

Well, I'm sure your code is useful for the Android platform, but the question
is whether we want this code in the mainline kernel.  For now, the answer is
"no, we don't".  Moreover, since you're the one who wants the code to be
merged, it's your burden to make it acceptable for us.  However you're going
to do it is up to you, but certainly trying to force your current code on us
is not going to work.

BTW, I think you handled the thing wrong.  If I were you, I wouldn't even try
to push the code as you did.  I would instead make it as simple as reasonably
possible so that the basic idea was clear and understandable to everyone.
Then, if there were any doubts with respect to the basic idea, I'd try to
clarify them and I'd consider modifying the code to address objections.
I'd only try to add more features after the basic idea had been accepted.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Sat, Feb 28, 2009 at 2:53 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Saturday 28 February 2009, Arve Hjønnevåg wrote:
>> Can you summarize what the problems with my current api are? I get the
>> impression that you think the overhead of using a list is too high,
>> and that timeout support should be removed because you think all
>> drivers that use it are broken.
>
> In no particular order:
> 1. One user space process can create an unlimited number of wakelocks.  This
>   shouldn't be possible.  Moreover, it is not even necessary for any process
>   to have more than one wakelock held at any time.

This has been addressed. A user space process cannot create more
wakelocks than it has filedescriptors.

> 2. Timeouts are wrong, because they don't really _solve_ any problem.  They are
>   useful for working around the fact that you can't or you don't want to
>   modify every piece of code that in principle should take a wakelock and
>   that's it.

Yes, timeouts are sometimes wrong, but they are not always wrong. I
gave two examples where the use of timeouts was not incorrect.

>  However,  entire concept of having one code path acting on
>   behalf of another one on a hunch that it might be doing something making
>   suspend undesirable is conceptually broken IMO.

OK. Do you have an alternative?

I my opinion this is how the entire system works if you do autosuspend
without a mechanism like wakelocks.

> 3. The overhead of using a list is unnecessary and _therefore_ too high (not
>   just too high).

It is only unnecessary if you do not want accounting or timeouts. The
overhead of a timer going off when it is not needed (if you push
timeouts to the drivers) is way higher then the overhead putting
wakelocks on a list.

> 4. There seems to be a race between user space wakelocks and the freezer
>   (perhaps I overlooked something, in which case please disregard this item).

You are missing something. Wakelocks overlap.

> 5. The name "wakelocks" is confusing, because they aren't locks and they
>   affect suspend, not wake.

I can change the name. Currently suspend_blocker seems to be
acceptable many people, but some don't like this name either.

> 6. Last time I saw the patches they were barely commented and the changelogs
>   didn't describe the code well (if at all).

I have not added many inline comments, but I did add the kerneldoc
comments you requested, and a lot of documentation since the first
patches.

> 7. There's no clear distinction between debug/stats code and the basic
>   functionality.

I don't think this is still true.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Sat, Feb 28, 2009 at 2:53 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> What you are talking about here is mostly an optimization of the
>> >> wakelock api. You have removed timeout support and made each wakelock
>> >> reference counted.
>> >
>> > I also removed the arbitrary number of wakelocks (I really _hate_ the name,
>> > can we please stop using it from now on?).
>>
>> What do you mean by this? You removed the struct wake_lock?
>
> Basically, yes.

Why is this better? If you move the stats into devices and tasks, this
may take up more space than adding an object to the structures or
tasks that you are protecting.


>> >> I do provide an option to turn off the wakelock stats, which makes
>> >> wake_lock/unlock significantly faster, but we never run with wakelock
>> >> stats off. Also, it pushes timeout handling to the drivers. I know may
>> >> of you don't like timeout support, but ignoring the problem is not a
>> >> solution. If each driver that needs timeouts uses its own timer, then
>> >> you will often wakeup from idle just to unlock a wakelock that will
>> >> not trigger suspend. This wakeup is a thousand times as costly on the
>> >> msm platform as a wakelock/unlock pair (with wakelock stats enabled).
>> >
>> > Well, at least a couple of people told you that the timeouts are hardly
>> > acceptable and they told you why.  Please stop repeating the same arguments you
>> > have given already for a couple of times.  They're not convincing.
>>
>> And you keep ignoring them.
>
> Not ignoring, but considering them as insufficient.  And since they've already
> been considered as insufficient, there's no point repeating them over and over
> again.  That doesn't make them any better.

The problem is that what you consider insufficient is what allows us
to ship a product.

>> > I do realize that you consider your current solution as the best thing since
>> > the sliced bread, but please accept the fact that the other people think
>> > differently.
>>
>> I certainly do not think my current solution is the best, it is very
>> invasive. I do however think your proposed solution is worse. The only
>> proposed alternative that we could actually ship a product on today is
>> to not use suspend at all.
>
> Well, I'm sure your code is useful for the Android platform, but the question
> is whether we want this code in the mainline kernel.  For now, the answer is
> "no, we don't".  Moreover, since you're the one who wants the code to be
> merged, it's your burden to make it acceptable for us.  However you're going
> to do it is up to you, but certainly trying to force your current code on us
> is not going to work.

I don't think I am the only one who want this code in the mainline
kernel. Many people want to use the android platform, and support in
the mainline kernel would be beneficial to some of them. I made many
requested changes to my code that provides no benefit to us, but I
have not made any changes that breaks our own use.

> BTW, I think you handled the thing wrong.  If I were you, I wouldn't even try
> to push the code as you did.  I would instead make it as simple as reasonably
> possible so that the basic idea was clear and understandable to everyone.
> Then, if there were any doubts with respect to the basic idea, I'd try to
> clarify them and I'd consider modifying the code to address objections.
> I'd only try to add more features after the basic idea had been accepted.

The basic concept was developed long before android was a public project.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Benjamin Herrenschmidt]

On Sat, 2009-02-28 at 16:06 -0800, Arve Hjønnevåg wrote:

I'm not taking a position on the merit of the wakelocks per se nor
whether Rafael is right or wrong here, I haven't looked at the problem
closely enough. I just want to react to this:

> The basic concept was developed long before android was a public
> project.

This isn't going to bring you any good will. We don't care what was done
before it was a public project. That has strictly no relevance to how it
should be submitted upstream.

How long the code has been simmering internally to company X or Y or
even in a public tree doesn't matter. Some times, yes, we do take
something as a whole, when it makes no sense to do otherwise (a driver,
a filesystem, ...). 

But something like what you propose, it seems, could easily be broken
down into a basic concept, on which features are added one after the
other, and in this case, it's the right way to go, simply because it's
easier to argue for the basic concept alone if you don't have to handle
comments froms people who don't agree with aspect A B or C of the other
features involved.

And if the basic concept doesn't get accepted in the first place, then
the whole point is moot...

Cheers,
Ben.

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Sunday 01 March 2009, Benjamin Herrenschmidt wrote:
> On Sat, 2009-02-28 at 16:06 -0800, Arve Hjønnevåg wrote:
> 
> I'm not taking a position on the merit of the wakelocks per se nor
> whether Rafael is right or wrong here, I haven't looked at the problem
> closely enough. I just want to react to this:
> 
> > The basic concept was developed long before android was a public
> > project.
> 
> This isn't going to bring you any good will. We don't care what was done
> before it was a public project. That has strictly no relevance to how it
> should be submitted upstream.
> 
> How long the code has been simmering internally to company X or Y or
> even in a public tree doesn't matter. Some times, yes, we do take
> something as a whole, when it makes no sense to do otherwise (a driver,
> a filesystem, ...). 
> 
> But something like what you propose, it seems, could easily be broken
> down into a basic concept, on which features are added one after the
> other, and in this case, it's the right way to go, simply because it's
> easier to argue for the basic concept alone if you don't have to handle
> comments froms people who don't agree with aspect A B or C of the other
> features involved.
> 
> And if the basic concept doesn't get accepted in the first place, then
> the whole point is moot...

Exactly.  This is what I've been trying to say for some time now.

Thanks Ben!

Rafael

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Sunday 01 March 2009, Arve Hjønnevåg wrote:
> On Sat, Feb 28, 2009 at 2:53 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> >> What you are talking about here is mostly an optimization of the
> >> >> wakelock api. You have removed timeout support and made each wakelock
> >> >> reference counted.
> >> >
> >> > I also removed the arbitrary number of wakelocks (I really _hate_ the name,
> >> > can we please stop using it from now on?).
> >>
> >> What do you mean by this? You removed the struct wake_lock?
> >
> > Basically, yes.
> 
> Why is this better? If you move the stats into devices and tasks, this
> may take up more space than adding an object to the structures or
> tasks that you are protecting.

OK, this is a valid point as long as we're going to use the stats in the
original form (which I'm not sure we want).

Still, as far as the number of "locks" one process can take is limited, it's
fine.

> >> >> I do provide an option to turn off the wakelock stats, which makes
> >> >> wake_lock/unlock significantly faster, but we never run with wakelock
> >> >> stats off. Also, it pushes timeout handling to the drivers. I know may
> >> >> of you don't like timeout support, but ignoring the problem is not a
> >> >> solution. If each driver that needs timeouts uses its own timer, then
> >> >> you will often wakeup from idle just to unlock a wakelock that will
> >> >> not trigger suspend. This wakeup is a thousand times as costly on the
> >> >> msm platform as a wakelock/unlock pair (with wakelock stats enabled).
> >> >
> >> > Well, at least a couple of people told you that the timeouts are hardly
> >> > acceptable and they told you why.  Please stop repeating the same arguments you
> >> > have given already for a couple of times.  They're not convincing.
> >>
> >> And you keep ignoring them.
> >
> > Not ignoring, but considering them as insufficient.  And since they've already
> > been considered as insufficient, there's no point repeating them over and over
> > again.  That doesn't make them any better.
> 
> The problem is that what you consider insufficient is what allows us
> to ship a product.

This doesn't matter a whit, because the mainline kernel is not just your
product.

By the same rule you could say that every working vendor driver is worth
merging into the mainline kernel, which clearly is not the case.

> >> > I do realize that you consider your current solution as the best thing since
> >> > the sliced bread, but please accept the fact that the other people think
> >> > differently.
> >>
> >> I certainly do not think my current solution is the best, it is very
> >> invasive. I do however think your proposed solution is worse. The only
> >> proposed alternative that we could actually ship a product on today is
> >> to not use suspend at all.
> >
> > Well, I'm sure your code is useful for the Android platform, but the question
> > is whether we want this code in the mainline kernel.  For now, the answer is
> > "no, we don't".  Moreover, since you're the one who wants the code to be
> > merged, it's your burden to make it acceptable for us.  However you're going
> > to do it is up to you, but certainly trying to force your current code on us
> > is not going to work.
> 
> I don't think I am the only one who want this code in the mainline
> kernel. Many people want to use the android platform, and support in
> the mainline kernel would be beneficial to some of them. I made many
> requested changes to my code that provides no benefit to us, but I
> have not made any changes that breaks our own use.

OK, please resubmit the patches, then.

> > BTW, I think you handled the thing wrong.  If I were you, I wouldn't even try
> > to push the code as you did.  I would instead make it as simple as reasonably
> > possible so that the basic idea was clear and understandable to everyone.
> > Then, if there were any doubts with respect to the basic idea, I'd try to
> > clarify them and I'd consider modifying the code to address objections.
> > I'd only try to add more features after the basic idea had been accepted.
> 
> The basic concept was developed long before android was a public project.

Please refer to the Ben's message for a good answer to this.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Pavel Machek]

HI!

> > > Of course that still doesn't address userspace.  Aggressively going to
> > > suspend lets us compensate for userspace programs that do somewhat
> > > silly things (I agree that it would be best if they didn't but they
> > > do and getting *everyone* to write their userspace code to avoid
> > > spinning or avoid waking up on short-duration timers to poll is a
> > > losing battle).
> > 
> > actually with powertop... on the open source side things are actually
> > won. It took all of 6 months...
> > I don't see that as a valid excuse. In fact, if this kind of solution
> > makes real userspace scheduled timers to be missed then I consider it a
> > serious functionality misfeature.
> 
> While you can't expect the kernel to solve all the problems of
> userspace, here's the broad situation one could end up in
> (note this specific sequence is generic and not based on any one
> specific product experience):
> 
> - carrier deploys a device 
> - carrier agrees to allow installation of arbitrary third party apps
>   without some horrible certification program requiring app authors
>   to jump through hoops, wait ages for approval, etc
> - users rejoice and install all kinds of apps
> - some apps are poorly written and impact battery life
> - users complain to carrier about battery life
> 
> You will end up with some crappy apps that do really dumb things.
> However, even if they're badly written users may still install and use
> these apps because hey, they do something the user likes.

Yes, crappy apps exist. Fortunately for you, they are written in Java,
so you can do all the magic in the JVM, no need to hack it into
kernel.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> >> > Phase 3: Probably explicit control left to open/close.
> >>
> >> While that's generally a good idea, it's important to recognize that
> >> some devices should be runtime-suspended even while they are open.
> >
> > From the kernel side, yes (and that should be transparent to the user space
> > having them open).  By the user space, no.
> 
> Allowing user space to suspend input devices while they are still open
> is useful. The user-space code that reads from the input devices does
> not need to know if the device is suspended or not, and the kernel
> cannot auto suspend input devices based on inactivity.

Actually, I'd like you to fix your userspace and close input devices
when it does not need them. Given the way you control the platform it
should not be that hard. I do not see why we'd want to invent new
interface for "uhuh, I have opened the keyboard but I am not really
interested in keys being pressed".
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> >> > I do realize that you consider your current solution as the best thing since
> >> > the sliced bread, but please accept the fact that the other people think
> >> > differently.
> >>
> >> I certainly do not think my current solution is the best, it is very
> >> invasive. I do however think your proposed solution is worse. The only
> >> proposed alternative that we could actually ship a product on today is
> >> to not use suspend at all.
> >
> > Well, I'm sure your code is useful for the Android platform, but the question
> > is whether we want this code in the mainline kernel.  For now, the answer is
> > "no, we don't".  Moreover, since you're the one who wants the code to be
> > merged, it's your burden to make it acceptable for us.  However you're going
> > to do it is up to you, but certainly trying to force your current code on us
> > is not going to work.
> 
> I don't think I am the only one who want this code in the mainline
> kernel. Many people want to use the android platform, and support in
> the mainline kernel would be beneficial to some of them. I made many
> requested changes to my code that provides no benefit to us, but I
> have not made any changes that breaks our own use.

No, you are not the only one. OTOH... that can't be used as an
argument to get it into kernel "as is".

Yes, I'd like to see it in but yes, I understand and agree with (most)
Rafaels's concerns.

> > BTW, I think you handled the thing wrong.  If I were you, I wouldn't even try
> > to push the code as you did.  I would instead make it as simple as reasonably
> > possible so that the basic idea was clear and understandable to everyone.
> > Then, if there were any doubts with respect to the basic idea, I'd try to
> > clarify them and I'd consider modifying the code to address objections.
> > I'd only try to add more features after the basic idea had been accepted.
> 
> The basic concept was developed long before android was a public project.

That's not our fault... 
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Sunday 01 March 2009, Arve Hjønnevåg wrote:
> On Sat, Feb 28, 2009 at 2:53 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Saturday 28 February 2009, Arve Hjønnevåg wrote:
> >> Can you summarize what the problems with my current api are? I get the
> >> impression that you think the overhead of using a list is too high,
> >> and that timeout support should be removed because you think all
> >> drivers that use it are broken.
> >
> > In no particular order:
> > 1. One user space process can create an unlimited number of wakelocks.  This
> >   shouldn't be possible.  Moreover, it is not even necessary for any process
> >   to have more than one wakelock held at any time.
> 
> This has been addressed. A user space process cannot create more
> wakelocks than it has filedescriptors.
> 
> > 2. Timeouts are wrong, because they don't really _solve_ any problem.  They are
> >   useful for working around the fact that you can't or you don't want to
> >   modify every piece of code that in principle should take a wakelock and
> >   that's it.
> 
> Yes, timeouts are sometimes wrong, but they are not always wrong. I
> gave two examples where the use of timeouts was not incorrect.

There still is a problem that the same operation can take time X on one
platform and time Y on another, so how are you going to determine the timeouts
that will be suitable for all platforms?

> >  However,  entire concept of having one code path acting on
> >   behalf of another one on a hunch that it might be doing something making
> >   suspend undesirable is conceptually broken IMO.
> 
> OK. Do you have an alternative?

Well, IMO every code path doing something that makes automatic suspend
undesirable should use a suspend blocker of some sort.  I'm afraid any other
approach will be unreliable and racy.

> I my opinion this is how the entire system works if you do autosuspend
> without a mechanism like wakelocks.

It surely hasn't been designed with automatic suspend in mind.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Oliver Neukum]

Am Sonntag 01 März 2009 23:56:47 schrieb Pavel Machek:
> > Allowing user space to suspend input devices while they are still open
> > is useful. The user-space code that reads from the input devices does
> > not need to know if the device is suspended or not, and the kernel
> > cannot auto suspend input devices based on inactivity.
>
> Actually, I'd like you to fix your userspace and close input devices
> when it does not need them. Given the way you control the platform it
> should not be that hard. I do not see why we'd want to invent new
> interface for "uhuh, I have opened the keyboard but I am not really
> interested in keys being pressed".

Generally you can't do this. A task has an open fd.

- you cannot assume it can open the device again (fd may be inherited)
- keeping the device open makes sure you are talking to the same device
- you may want to avoid repeating expensive initialisations
- some input devices also do output

	Regards
		Oliver

Re: [RFD] Automatic suspend

[Pavel Machek]

On Mon 2009-03-02 09:24:36, Oliver Neukum wrote:
> Am Sonntag 01 März 2009 23:56:47 schrieb Pavel Machek:
> > > Allowing user space to suspend input devices while they are still open
> > > is useful. The user-space code that reads from the input devices does
> > > not need to know if the device is suspended or not, and the kernel
> > > cannot auto suspend input devices based on inactivity.
> >
> > Actually, I'd like you to fix your userspace and close input devices
> > when it does not need them. Given the way you control the platform it
> > should not be that hard. I do not see why we'd want to invent new
> > interface for "uhuh, I have opened the keyboard but I am not really
> > interested in keys being pressed".
> 
> Generally you can't do this. A task has an open fd.
> 
> - you cannot assume it can open the device again (fd may be
> inherited)

Well, those tasks that matter - X servers and similar - usually can.

> - keeping the device open makes sure you are talking to the same device

Well, you have to handle hotplug anyway, so... and the name will not
change unless you unplug/replug.

> - you may want to avoid repeating expensive initialisations

That kind of initializations should perhaps be done at insmod, not
open time?

> - some input devices also do output

I guess you want to separate those to two different devices, then.

Anyway...

* I'd prefer "close to powersave". I can see that can be tricky to
use. So the alternative is

* ioctl() "I'm not interested in exact keys, powersave" is something
that makes sense. It should really be discussed with input people.

I'd hate to see

* magic /sys/.../file where you echo 1 to powersave. It is too
disconnected from the input fd, and while it may make it easier to
retrofit powermanagement without modifying Xservers etc... it will
ultimately result in pretty ugly hacks.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Arjan van de Ven]

On Mon, 2 Mar 2009 09:24:36 +0100
Oliver Neukum <oliver@neukum.org> wrote:

> - you may want to avoid repeating expensive initialisations

btw this "expensive intialization" for a correct driver... is the same
thing  you need to do to get the device out of power saving mode..


-- 
Arjan van de Ven 	Intel Open Source Technology Centre
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Sun, Mar 1, 2009 at 3:17 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Sunday 01 March 2009, Arve Hjønnevåg wrote:
>> On Sat, Feb 28, 2009 at 2:53 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > On Saturday 28 February 2009, Arve Hjønnevåg wrote:
>> >> Can you summarize what the problems with my current api are? I get the
>> >> impression that you think the overhead of using a list is too high,
>> >> and that timeout support should be removed because you think all
>> >> drivers that use it are broken.
>> >
>> > In no particular order:
>> > 1. One user space process can create an unlimited number of wakelocks.  This
>> >   shouldn't be possible.  Moreover, it is not even necessary for any process
>> >   to have more than one wakelock held at any time.
>>
>> This has been addressed. A user space process cannot create more
>> wakelocks than it has filedescriptors.
>>
>> > 2. Timeouts are wrong, because they don't really _solve_ any problem.  They are
>> >   useful for working around the fact that you can't or you don't want to
>> >   modify every piece of code that in principle should take a wakelock and
>> >   that's it.
>>
>> Yes, timeouts are sometimes wrong, but they are not always wrong. I
>> gave two examples where the use of timeouts was not incorrect.
>
> There still is a problem that the same operation can take time X on one
> platform and time Y on another, so how are you going to determine the timeouts
> that will be suitable for all platforms?

This only applies to the timeouts that fall into the wrong category.
The timeout used when a driver returns -EBUSY is arbitrary, but any
value is technically correct. The one second timeout in the alarm
driver is not platform specific. It is one second because the
resolution of the rtc api is only one second.

For the timeouts that do fall into the wrong category (use a timeout
when passing data to a unmodified subsystem), the drivers are mostly
(if not all) platform specific.

>
>> >  However,  entire concept of having one code path acting on
>> >   behalf of another one on a hunch that it might be doing something making
>> >   suspend undesirable is conceptually broken IMO.
>>
>> OK. Do you have an alternative?
>
> Well, IMO every code path doing something that makes automatic suspend
> undesirable should use a suspend blocker of some sort.  I'm afraid any other
> approach will be unreliable and racy.

I agree with this, but I cannot change every code path at once. I also
don't know if every code path can be easily fixed. Using a timeout in
this case is a compromise. It is not as good as protecting every code
path, but it is much better than doing nothing. The race condition you
have when preventing suspend with a timeout is the same as every code
using a timeout. If the system is busy it can fail. The race condition
that you have with no protection happens with any load. If the system
decides to go to sleep at the same time as a wakeup event occur, the
system will sleep.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Sun, Mar 1, 2009 at 1:20 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> >> I do provide an option to turn off the wakelock stats, which makes
>> >> >> wake_lock/unlock significantly faster, but we never run with wakelock
>> >> >> stats off. Also, it pushes timeout handling to the drivers. I know may
>> >> >> of you don't like timeout support, but ignoring the problem is not a
>> >> >> solution. If each driver that needs timeouts uses its own timer, then
>> >> >> you will often wakeup from idle just to unlock a wakelock that will
>> >> >> not trigger suspend. This wakeup is a thousand times as costly on the
>> >> >> msm platform as a wakelock/unlock pair (with wakelock stats enabled).
>> >> >
>> >> > Well, at least a couple of people told you that the timeouts are hardly
>> >> > acceptable and they told you why.  Please stop repeating the same arguments you
>> >> > have given already for a couple of times.  They're not convincing.
>> >>
>> >> And you keep ignoring them.
>> >
>> > Not ignoring, but considering them as insufficient.  And since they've already
>> > been considered as insufficient, there's no point repeating them over and over
>> > again.  That doesn't make them any better.
>>
>> The problem is that what you consider insufficient is what allows us
>> to ship a product.
>
> This doesn't matter a whit, because the mainline kernel is not just your
> product.

Unless you are saying that changes in the mainline kernel does not
need be usable in practice, then it does matter. If we remove the
feature that allows us to interact with existing code, it will take
much longer before it is usable by anyone.

> By the same rule you could say that every working vendor driver is worth
> merging into the mainline kernel, which clearly is not the case.

Some people seem to think they are.

>
>> >> > I do realize that you consider your current solution as the best thing since
>> >> > the sliced bread, but please accept the fact that the other people think
>> >> > differently.
>> >>
>> >> I certainly do not think my current solution is the best, it is very
>> >> invasive. I do however think your proposed solution is worse. The only
>> >> proposed alternative that we could actually ship a product on today is
>> >> to not use suspend at all.
>> >
>> > Well, I'm sure your code is useful for the Android platform, but the question
>> > is whether we want this code in the mainline kernel.  For now, the answer is
>> > "no, we don't".  Moreover, since you're the one who wants the code to be
>> > merged, it's your burden to make it acceptable for us.  However you're going
>> > to do it is up to you, but certainly trying to force your current code on us
>> > is not going to work.
>>
>> I don't think I am the only one who want this code in the mainline
>> kernel. Many people want to use the android platform, and support in
>> the mainline kernel would be beneficial to some of them. I made many
>> requested changes to my code that provides no benefit to us, but I
>> have not made any changes that breaks our own use.
>
> OK, please resubmit the patches, then.

I submitted them three weeks ago. I'll submit a new set after I rename
the api (presumably to suspend_block(er)) but I would like more
agreement on the timeout issue first.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> >> > Not ignoring, but considering them as insufficient.  And since they've already
> >> > been considered as insufficient, there's no point repeating them over and over
> >> > again.  That doesn't make them any better.
> >>
> >> The problem is that what you consider insufficient is what allows us
> >> to ship a product.
> >
> > This doesn't matter a whit, because the mainline kernel is not just your
> > product.
> 
> Unless you are saying that changes in the mainline kernel does not
> need be usable in practice, then it does matter. If we remove the
> feature that allows us to interact with existing code, it will take
> much longer before it is usable by anyone.

Well, taking longer before "being usable" is good tradeoff if it means
"we get cleaner/actually correct system in mainline sooner".

> >> I don't think I am the only one who want this code in the mainline
> >> kernel. Many people want to use the android platform, and support in
> >> the mainline kernel would be beneficial to some of them. I made many
> >> requested changes to my code that provides no benefit to us, but I
> >> have not made any changes that breaks our own use.
> >
> > OK, please resubmit the patches, then.
> 
> I submitted them three weeks ago. I'll submit a new set after I rename
> the api (presumably to suspend_block(er)) but I would like more
> agreement on the timeout issue first.

I do believe that everyone (including you :-) agrees that timeouts are
ugly hack. So just reorder the series so they come at the end.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> >> If you ignore wakelocks with timeouts, the current
> >> wakelock interface can be implemented with a global atomic_t to
> >> prevent suspend, and a per wakelock atomic_t to prevent a single
> >> client from changing the global reference count by more than one.
> >>
> >> There are a couple of reasons that I have not done this. It removes
> >> the ability to easily inspect the system when it is not suspending.
> >
> > Care to elaborate?
> 
> If you have a single atomic_t and it is not 0, you do not know who
> incremented it.

Which is okay for already debugged system. For debugging, yes some
support can be nice.

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Rafael J. Wysocki]

On Tuesday 03 March 2009, Arve Hjønnevåg wrote:
> On Sun, Mar 1, 2009 at 3:17 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > On Sunday 01 March 2009, Arve Hjønnevåg wrote:
> >> On Sat, Feb 28, 2009 at 2:53 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> >> > On Saturday 28 February 2009, Arve Hjønnevåg wrote:
> >> >> Can you summarize what the problems with my current api are? I get the
> >> >> impression that you think the overhead of using a list is too high,
> >> >> and that timeout support should be removed because you think all
> >> >> drivers that use it are broken.
> >> >
> >> > In no particular order:
> >> > 1. One user space process can create an unlimited number of wakelocks.  This
> >> >   shouldn't be possible.  Moreover, it is not even necessary for any process
> >> >   to have more than one wakelock held at any time.
> >>
> >> This has been addressed. A user space process cannot create more
> >> wakelocks than it has filedescriptors.
> >>
> >> > 2. Timeouts are wrong, because they don't really _solve_ any problem.  They are
> >> >   useful for working around the fact that you can't or you don't want to
> >> >   modify every piece of code that in principle should take a wakelock and
> >> >   that's it.
> >>
> >> Yes, timeouts are sometimes wrong, but they are not always wrong. I
> >> gave two examples where the use of timeouts was not incorrect.
> >
> > There still is a problem that the same operation can take time X on one
> > platform and time Y on another, so how are you going to determine the timeouts
> > that will be suitable for all platforms?
> 
> This only applies to the timeouts that fall into the wrong category.
> The timeout used when a driver returns -EBUSY is arbitrary, but any
> value is technically correct. The one second timeout in the alarm
> driver is not platform specific. It is one second because the
> resolution of the rtc api is only one second.
> 
> For the timeouts that do fall into the wrong category (use a timeout
> when passing data to a unmodified subsystem), the drivers are mostly
> (if not all) platform specific.

What drivers are they?

> >> >  However,  entire concept of having one code path acting on
> >> >   behalf of another one on a hunch that it might be doing something making
> >> >   suspend undesirable is conceptually broken IMO.
> >>
> >> OK. Do you have an alternative?
> >
> > Well, IMO every code path doing something that makes automatic suspend
> > undesirable should use a suspend blocker of some sort.  I'm afraid any other
> > approach will be unreliable and racy.
> 
> I agree with this,

Good.

> but I cannot change every code path at once.

That need not happen at once (eg. in one patch or something).  Once we've
introduced the basics, the changes can be made gradually wherever necessary,
step by step.

> I also don't know if every code path can be easily fixed. Using a timeout in
> this case is a compromise. It is not as good as protecting every code
> path, but it is much better than doing nothing. The race condition you
> have when preventing suspend with a timeout is the same as every code
> using a timeout. If the system is busy it can fail. The race condition
> that you have with no protection happens with any load. If the system
> decides to go to sleep at the same time as a wakeup event occur, the
> system will sleep.

Well, if you have strictly limited time (eg. you want to ship a product at
specific date), you have to go for compromises like this, but we're not in a
hurry (or are we for some unspecified reason?).  There's no deadline etc., so
we can afford to do it right from the start (which BTW is likely to save us
time in future).

So, I'd suggest to just separate the timeouted suspend blockers from the
basic code and introduce the latter first.

IOW, let's first try to merge things that everybody is comfortable with and
postpone the merging of the rest.  I don't think we're going to lose
anything by doing it this way.

Thanks,
Rafael

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Tue, Mar 3, 2009 at 2:39 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday 03 March 2009, Arve Hjønnevåg wrote:
>> On Sun, Mar 1, 2009 at 3:17 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > On Sunday 01 March 2009, Arve Hjønnevåg wrote:
>> >> On Sat, Feb 28, 2009 at 2:53 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> > On Saturday 28 February 2009, Arve Hjønnevåg wrote:
>> >> >> Can you summarize what the problems with my current api are? I get the
>> >> >> impression that you think the overhead of using a list is too high,
>> >> >> and that timeout support should be removed because you think all
>> >> >> drivers that use it are broken.
>> >> >
>> >> > In no particular order:
>> >> > 1. One user space process can create an unlimited number of wakelocks.  This
>> >> >   shouldn't be possible.  Moreover, it is not even necessary for any process
>> >> >   to have more than one wakelock held at any time.
>> >>
>> >> This has been addressed. A user space process cannot create more
>> >> wakelocks than it has filedescriptors.
>> >>
>> >> > 2. Timeouts are wrong, because they don't really _solve_ any problem.  They are
>> >> >   useful for working around the fact that you can't or you don't want to
>> >> >   modify every piece of code that in principle should take a wakelock and
>> >> >   that's it.
>> >>
>> >> Yes, timeouts are sometimes wrong, but they are not always wrong. I
>> >> gave two examples where the use of timeouts was not incorrect.
>> >
>> > There still is a problem that the same operation can take time X on one
>> > platform and time Y on another, so how are you going to determine the timeouts
>> > that will be suitable for all platforms?
>>
>> This only applies to the timeouts that fall into the wrong category.
>> The timeout used when a driver returns -EBUSY is arbitrary, but any
>> value is technically correct. The one second timeout in the alarm
>> driver is not platform specific. It is one second because the
>> resolution of the rtc api is only one second.
>>
>> For the timeouts that do fall into the wrong category (use a timeout
>> when passing data to a unmodified subsystem), the drivers are mostly
>> (if not all) platform specific.
>
> What drivers are they?

Serial driver used for bluetooth, wifi driver and battery driver for
usb. The msm smd code also need a wakelock with a timeout before
passing data to the tty and network layers, but I did not find this.

>> >> >  However,  entire concept of having one code path acting on
>> >> >   behalf of another one on a hunch that it might be doing something making
>> >> >   suspend undesirable is conceptually broken IMO.
>> >>
>> >> OK. Do you have an alternative?
>> >
>> > Well, IMO every code path doing something that makes automatic suspend
>> > undesirable should use a suspend blocker of some sort.  I'm afraid any other
>> > approach will be unreliable and racy.
>>
>> I agree with this,
>
> Good.
>
>> but I cannot change every code path at once.
>
> That need not happen at once (eg. in one patch or something).  Once we've
> introduced the basics, the changes can be made gradually wherever necessary,
> step by step.

If you are OK with merging an unfinished system then this may work.

>> I also don't know if every code path can be easily fixed. Using a timeout in
>> this case is a compromise. It is not as good as protecting every code
>> path, but it is much better than doing nothing. The race condition you
>> have when preventing suspend with a timeout is the same as every code
>> using a timeout. If the system is busy it can fail. The race condition
>> that you have with no protection happens with any load. If the system
>> decides to go to sleep at the same time as a wakeup event occur, the
>> system will sleep.
>
> Well, if you have strictly limited time (eg. you want to ship a product at
> specific date), you have to go for compromises like this, but we're not in a
> hurry (or are we for some unspecified reason?).  There's no deadline etc., so
> we can afford to do it right from the start (which BTW is likely to save us
> time in future).
>
> So, I'd suggest to just separate the timeouted suspend blockers from the
> basic code and introduce the latter first.

How do you want to handle drivers that return -EBUSY from suspend. The
basic code uses a wakelock with a timeout to handle this now. Without
this we can either try suspend again immediately, or activate a
suspend blocker and use a timer to release it.

> IOW, let's first try to merge things that everybody is comfortable with and
> postpone the merging of the rest.  I don't think we're going to lose
> anything by doing it this way.

I think we do loose some flexibility by leaving out timeout support,
but I'll try to separate it from the first patch.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Tue, Mar 3, 2009 at 5:57 AM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> >> If you ignore wakelocks with timeouts, the current
>> >> wakelock interface can be implemented with a global atomic_t to
>> >> prevent suspend, and a per wakelock atomic_t to prevent a single
>> >> client from changing the global reference count by more than one.
>> >>
>> >> There are a couple of reasons that I have not done this. It removes
>> >> the ability to easily inspect the system when it is not suspending.
>> >
>> > Care to elaborate?
>>
>> If you have a single atomic_t and it is not 0, you do not know who
>> incremented it.
>
> Which is okay for already debugged system. For debugging, yes some
> support can be nice.

Yes, but installing an app can turn your debugged system into an
undebugged system. I think is fine to have a kernel option to disable
all debugging support, I just don't think it is the top priority. I
have two options for making the no-stats no-timeout configuration
faster. Option one, always use a reference count, and implement the
other configurations on top of this. This makes the shipping
configuration slower. Option two, use a completely separate
implementation when stats or timeouts are enabled. This makes the fast
version virtually untested. Neither of these are very appealing at the
moment.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Arve Hjønnevåg]

On Tue, Mar 3, 2009 at 5:51 AM, Pavel Machek <pavel@ucw.cz> wrote:
> Hi!
>
>> >> > Not ignoring, but considering them as insufficient.  And since they've already
>> >> > been considered as insufficient, there's no point repeating them over and over
>> >> > again.  That doesn't make them any better.
>> >>
>> >> The problem is that what you consider insufficient is what allows us
>> >> to ship a product.
>> >
>> > This doesn't matter a whit, because the mainline kernel is not just your
>> > product.
>>
>> Unless you are saying that changes in the mainline kernel does not
>> need be usable in practice, then it does matter. If we remove the
>> feature that allows us to interact with existing code, it will take
>> much longer before it is usable by anyone.
>
> Well, taking longer before "being usable" is good tradeoff if it means
> "we get cleaner/actually correct system in mainline sooner".

I think this could go either way. If the system is usable, it may get
more use from developers that know how to improve a specific subsystem
to not use timeouts. Or, it may be considered good enough, and nobody
bothers coming up with a correct solution. I think the latter is what
you are worried will happen.

>
>> >> I don't think I am the only one who want this code in the mainline
>> >> kernel. Many people want to use the android platform, and support in
>> >> the mainline kernel would be beneficial to some of them. I made many
>> >> requested changes to my code that provides no benefit to us, but I
>> >> have not made any changes that breaks our own use.
>> >
>> > OK, please resubmit the patches, then.
>>
>> I submitted them three weeks ago. I'll submit a new set after I rename
>> the api (presumably to suspend_block(er)) but I would like more
>> agreement on the timeout issue first.
>
> I do believe that everyone (including you :-) agrees that timeouts are
> ugly hack. So just reorder the series so they come at the end.

No, I think many uses of timeouts are a ugly hack, not all, but OK I
will try to move timeout support to a separate patch.

-- 
Arve Hjønnevåg

Re: [RFD] Automatic suspend

[Pavel Machek]

On Tue 2009-03-03 16:06:02, Arve Hj?nnev?g wrote:
> On Tue, Mar 3, 2009 at 5:51 AM, Pavel Machek <pavel@ucw.cz> wrote:
> > Hi!
> >
> >> >> > Not ignoring, but considering them as insufficient.  And since they've already
> >> >> > been considered as insufficient, there's no point repeating them over and over
> >> >> > again.  That doesn't make them any better.
> >> >>
> >> >> The problem is that what you consider insufficient is what allows us
> >> >> to ship a product.
> >> >
> >> > This doesn't matter a whit, because the mainline kernel is not just your
> >> > product.
> >>
> >> Unless you are saying that changes in the mainline kernel does not
> >> need be usable in practice, then it does matter. If we remove the
> >> feature that allows us to interact with existing code, it will take
> >> much longer before it is usable by anyone.
> >
> > Well, taking longer before "being usable" is good tradeoff if it means
> > "we get cleaner/actually correct system in mainline sooner".
> 
> I think this could go either way. If the system is usable, it may get
> more use from developers that know how to improve a specific subsystem
> to not use timeouts. Or, it may be considered good enough, and nobody
> bothers coming up with a correct solution. I think the latter is what
> you are worried will happen.

Yep.

> >> I submitted them three weeks ago. I'll submit a new set after I rename
> >> the api (presumably to suspend_block(er)) but I would like more
> >> agreement on the timeout issue first.
> >
> > I do believe that everyone (including you :-) agrees that timeouts are
> > ugly hack. So just reorder the series so they come at the end.
> 
> No, I think many uses of timeouts are a ugly hack, not all, but OK I
> will try to move timeout support to a separate patch.

Thanks.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

Hi!

> > That need not happen at once (eg. in one patch or something).  Once we've
> > introduced the basics, the changes can be made gradually wherever necessary,
> > step by step.
> 
> If you are OK with merging an unfinished system then this may work.

Good.

> > So, I'd suggest to just separate the timeouted suspend blockers from the
> > basic code and introduce the latter first.
> 
> How do you want to handle drivers that return -EBUSY from suspend. The
> basic code uses a wakelock with a timeout to handle this now. Without
> this we can either try suspend again immediately, or activate a
> suspend blocker and use a timer to release it.

Just printk() and complain. That should be good enough solution for
now... (Those drivers are arguably buggy already. If user told machine
to go to sleep, it should not randomly refuse to do that.)

									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

On Wed 2009-02-18 18:31:15, Alan Stern wrote:
> On Thu, 19 Feb 2009, Rafael J. Wysocki wrote:
> 
> > > If some devices are autosuspended after a local inactivity timeout, I
> > > don't want to wait for those devices to autosuspend if I know the code
> > > that needed to run is done. This could cause delays in the normal
> > > case,
> > 
> > Isn't it a matter of adjusting the inactivity timeouts in a suitable way?
> 
> It's not that simple.  A single device driver has a very local view, 
> not suitable for deciding whether the entire system should go to sleep.
> 
> So for example, a disk driver might think it's appropriate to spin down 
> the disk after 10 seconds of inactivity.  But an overall system monitor 
> might realize that nothing is going on right now and want to put the 
> system to sleep immediately, without waiting the 10 seconds for the 
> disk to autosuspend.

Hmm, I'm not sure. If system is so busy that it keeps the disk up, it
may not be good idea to sleep whole system.

Example: "idle fileserver". Current command is done, so we seem to be
idle; but when we sleep, new command comes. While system really is
idle, it is bad idea to suspend.

If the overall system monitor can decide disk will not be used, it can
just sleep the disk.

> > > and it could prevent suspend if a background process (not using
> > > wakelocks) is accessing a disk more frequently than its idle timeout.
> > 
> > Well, actually, shouldn't it prevent suspend from happening?  Arguably, it just
> > means that the disk is continuously being accessed with respect to the inactive
> > timeout granularity.
> 
> That's true, but it shows the problem of making the autosleep decision
> based on disk activity.  An auto-sleep should not have to wait for
> every device (or some suitable subset) to become idle for some minimum
> time; it should be able to kick in at short notice.

I believe we do want to wake for timeouts, as not all sources of
wakeup are local.
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [RFD] Automatic suspend

[Pavel Machek]

On Fri 2009-02-27 16:22:19, Oliver Neukum wrote:
> Am Freitag 27 Februar 2009 11:18:18 schrieb Pavel Machek:
> > > We can just as well have a class of tasks less important than power
> > > saving. They'd just run when power saving is not active for some other
> > > reason. Just like other such schemes we end up with the problem
> > > of priority inversion with locking.
> >
> > Ok, I guess this could be interesting in some cases... maybe. What are
> > real examples of such tasks?
> 
> The classical example is Seti@HOME.

So it is okay for you to waste 30W on seti on running machine on
battery power but you can't waste those remaining 10W...? I guess I'd
either expect seti to stop at battery power...

suspended machine 0.3W
awake machine 10W
machine running seti 40W

...? (thinkpad x60).

Ok... for something less computing intensive, it might make
sense... but I guess those examples are less widespread than you
expect.

> Any monitoring tool like umtsmon
> or kwifimanager.

I'd expect desktop environment to stop wasting cycles/electrons on
that when user can't see the screen.

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html