Re: [PATCH 1/8] PM: Opportunistic suspend support.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Rafael J. Wysocki]

On Tuesday 25 May 2010, Alan Stern wrote:
> On Tue, 25 May 2010, Rafael J. Wysocki wrote:
> 
> > So, basically, you'd prefer to move the entire complexity to user space.
> 
> No, just the complexity of the userspace suspend blockers.  That was 
> one of the parts of the interface that people objected to, after all.
> 
> > I'm not sure if that's a big win
> 
> It's not a _big_ win, but it is an improvement IMO.
> 
> >  and I'm not sure anyone is actually going to
> > implement it (and some drivers would still have to be modified to participate
> > in this framework).
> 
> Of course drivers have to be modified.  The kernel-layer suspend
> blockers aren't affected by this proposal, so they still have to be
> implemented.
> 
> >  So again, we have a hunch that the goal may be achieved
> > in a different way, but at this point we'd rather need a _working_ _solution_
> > (in the form of code one can build and actually use).
> 
> It's not very different from what has been submitted, and I think
> there's little doubt that it could be built and used fairly easily.  
> All we're talking about is removing the userspace suspend-blocker API
> and the opportunistic workqueue, replacing them with an "opportunistic"
> entry in /sys/power/state, and setting up a userspace power manager
> process.
> 
> > I don't think it's realistic to expect the Android people to go and redesign
> > their stuff along the lines you've described, because they have a working
> > implementation (in the kernel) that they are satisfied with.
> 
> The redesign would be pretty small.  The kernel changes relative to
> what they have submitted are minimal, mostly just removing a few of
> their additions.  Furthermore, we've been told that Android _already_
> funnels all its userspace suspend-blocker work through a single
> process.  All that would be needed would be to make that process
> initiate an opportunistic suspend whenever no userspace suspend
> blockers were active.
> 
> > Now, we can reject their patches, but that's not going to cause any progress
> > to happen, realistically.  Quite on the contrary, Android will continue to use
> > wakelocks and Android driver writers will continue to ignore the mainline
> > and the gap between the two kernel lines will only get wider and wider over
> > time.
> > 
> > And what really is the drawback if we merge the patches?  Quite frankly,
> > I don't see any.
> 
> You don't seem to appreciate how small a change Dmitry has proposed.  
> Almost all of the suspend-blocker work would remain as in the submitted
> patches.  The only difference is that the userspace API and
> opportunistic-suspend implementation would be simplified, by moving
> some of the work out of the kernel.

No, I don't really think it's going to be a small change.  The problem is that
for the Android people changing user space is very hard, so I don't think
this realy is an option, given that they would have to implement it themselves,
test it, validate it on multiple different hardware platforms etc. and _then_
resubmit the feature without any guarantee that it will be merged.

So, my opinion is that we only have a choice to either take the feature as is
now, or reject it altogether and live with the consequeces in each case.  And
quite frankly I don't feel like I'm in position to make that decision.

Thanks,
Rafael

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Arve Hjønnevåg]

On Tue, May 25, 2010 at 2:44 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Tuesday 25 May 2010, Alan Stern wrote:
>> On Tue, 25 May 2010, Rafael J. Wysocki wrote:
>>
>> > So, basically, you'd prefer to move the entire complexity to user space.
>>
>> No, just the complexity of the userspace suspend blockers.  That was
>> one of the parts of the interface that people objected to, after all.
>>
>> > I'm not sure if that's a big win
>>
>> It's not a _big_ win, but it is an improvement IMO.
>>
>> >  and I'm not sure anyone is actually going to
>> > implement it (and some drivers would still have to be modified to participate
>> > in this framework).
>>
>> Of course drivers have to be modified.  The kernel-layer suspend
>> blockers aren't affected by this proposal, so they still have to be
>> implemented.
>>
>> >  So again, we have a hunch that the goal may be achieved
>> > in a different way, but at this point we'd rather need a _working_ _solution_
>> > (in the form of code one can build and actually use).
>>
>> It's not very different from what has been submitted, and I think
>> there's little doubt that it could be built and used fairly easily.
>> All we're talking about is removing the userspace suspend-blocker API
>> and the opportunistic workqueue, replacing them with an "opportunistic"
>> entry in /sys/power/state, and setting up a userspace power manager
>> process.
>>
>> > I don't think it's realistic to expect the Android people to go and redesign
>> > their stuff along the lines you've described, because they have a working
>> > implementation (in the kernel) that they are satisfied with.
>>
>> The redesign would be pretty small.  The kernel changes relative to
>> what they have submitted are minimal, mostly just removing a few of
>> their additions.  Furthermore, we've been told that Android _already_
>> funnels all its userspace suspend-blocker work through a single
>> process.  All that would be needed would be to make that process
>> initiate an opportunistic suspend whenever no userspace suspend
>> blockers were active.
>>
>> > Now, we can reject their patches, but that's not going to cause any progress
>> > to happen, realistically.  Quite on the contrary, Android will continue to use
>> > wakelocks and Android driver writers will continue to ignore the mainline
>> > and the gap between the two kernel lines will only get wider and wider over
>> > time.
>> >
>> > And what really is the drawback if we merge the patches?  Quite frankly,
>> > I don't see any.
>>
>> You don't seem to appreciate how small a change Dmitry has proposed.
>> Almost all of the suspend-blocker work would remain as in the submitted
>> patches.  The only difference is that the userspace API and
>> opportunistic-suspend implementation would be simplified, by moving
>> some of the work out of the kernel.
>
> No, I don't really think it's going to be a small change.  The problem is that
> for the Android people changing user space is very hard, so I don't think
> this realy is an option, given that they would have to implement it themselves,
> test it, validate it on multiple different hardware platforms etc. and _then_
> resubmit the feature without any guarantee that it will be merged.
>

The biggest problem here is not that it is hard to change our
user-space, but that the proposed change is inferior to what we have
now. It forces us to poll until all drivers stop aborting suspend. On
one hand we have people telling us that all code that polls is broken
and must be fixed (instead of suspending to limit the damage), and on
the other hand we have people suggesting we implement opportunistic
suspend by polling from user-space until suspend succeeds.


> So, my opinion is that we only have a choice to either take the feature as is
> now, or reject it altogether and live with the consequeces in each case.  And
> quite frankly I don't feel like I'm in position to make that decision.
>



-- 
Arve Hjønnevåg

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Peter Zijlstra]

On Tue, 2010-05-25 at 23:44 +0200, Rafael J. Wysocki wrote:
> 
> No, I don't really think it's going to be a small change.  The problem is that
> for the Android people changing user space is very hard, so I don't think
> this realy is an option, given that they would have to implement it themselves,
> test it, validate it on multiple different hardware platforms etc. and _then_
> resubmit the feature without any guarantee that it will be merged. 

So your position is basically that we're going to merge this stuff
because google doesn't want to expend effort into investigating a
potential saner solution?

So why don't we simply merge all the crap people throw at us and be done
with it.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Peter Zijlstra]

On Tue, 2010-05-25 at 15:33 -0700, Arve Hjønnevåg wrote:
> The biggest problem here is not that it is hard to change our
> user-space, but that the proposed change is inferior to what we have
> now. It forces us to poll until all drivers stop aborting suspend. On
> one hand we have people telling us that all code that polls is broken
> and must be fixed (instead of suspending to limit the damage), and on
> the other hand we have people suggesting we implement opportunistic
> suspend by polling from user-space until suspend succeeds. 

No it does _not_. You're really not getting that Dmitry is proposing.


So your proposal is that when we wake userspace, it
opens /dev/suspend_blocker _before_ it consumes whatever event, consumes
the event, deals with the event, then closes the suspend_blocker. Then
the kernel, upon reaching a 0 suspend_blocker count, will try to suspend
again.


What Dmitry proposes is that, the app _before_ it consumes the event,
pokes at this suspend manager, it increases a blocker count, then
consumes the event (the kernel will _not_ auto-suspend), handles it and
then again pokes the suspend manager, this time decreasing the blocker
count.

The suspend manager will, upon reaching a 0 block count, suspend the
machine. If that fails, it means there's something to do, an app will
inc, work, dec its count, and it will try again once it reaches 0 again.

There is no polling what-so-ever in this model.

The only thing is that the kernel will not try to auto-suspend and there
is no user-space suspend blocker API.
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Peter Zijlstra]

On Wed, 2010-05-26 at 10:42 +0200, Peter Zijlstra wrote:
> What Dmitry proposes is that, the app _before_ it consumes the event,
> pokes at this suspend manager, it increases a blocker count, then
> consumes the event (the kernel will _not_ auto-suspend), handles it and
> then again pokes the suspend manager, this time decreasing the blocker
> count.
> 
> The suspend manager will, upon reaching a 0 block count, suspend the
> machine. If that fails, it means there's something to do, an app will
> inc, work, dec its count, and it will try again once it reaches 0 again.

Alternatively the suspend manager could simply cancel the opportunistic
suspend mode on !0, and re-instate it on 0.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Florian Mickler]

On Wed, 26 May 2010 10:42:22 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Tue, 2010-05-25 at 15:33 -0700, Arve Hjønnevåg wrote:
> > The biggest problem here is not that it is hard to change our
> > user-space, but that the proposed change is inferior to what we have
> > now. It forces us to poll until all drivers stop aborting suspend. On
> > one hand we have people telling us that all code that polls is broken
> > and must be fixed (instead of suspending to limit the damage), and on
> > the other hand we have people suggesting we implement opportunistic
> > suspend by polling from user-space until suspend succeeds. 
> 
> No it does _not_. You're really not getting that Dmitry is proposing.
> 
> 
> So your proposal is that when we wake userspace, it
> opens /dev/suspend_blocker _before_ it consumes whatever event, consumes
> the event, deals with the event, then closes the suspend_blocker. Then
> the kernel, upon reaching a 0 suspend_blocker count, will try to suspend
> again.
> 
> 
> What Dmitry proposes is that, the app _before_ it consumes the event,
> pokes at this suspend manager, it increases a blocker count, then
> consumes the event (the kernel will _not_ auto-suspend), handles it and
> then again pokes the suspend manager, this time decreasing the blocker
> count.
> 
> The suspend manager will, upon reaching a 0 block count, suspend the
> machine. If that fails, it means there's something to do, an app will
> inc, work, dec its count, and it will try again once it reaches 0 again.
> 
> There is no polling what-so-ever in this model.
> 
> The only thing is that the kernel will not try to auto-suspend and there
> is no user-space suspend blocker API.

There is polling, because the suspend manager in userspace doesn't have
the whole picture. i.e. it doesn't know if a suspend will be
successfull. 
So for aggressive suspending as a powersave-feature you need to poll
(i.e. retry upon failure). because you don't want to stay unsuspended. 

This discussion really is going in circles. You could all may well be
just posting references to postings by now, instead of actually writing
mails.

Cheers,
Flo

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Peter Zijlstra]

On Wed, 2010-05-26 at 11:23 +0200, Florian Mickler wrote:

> There is polling, because the suspend manager in userspace doesn't have
> the whole picture. i.e. it doesn't know if a suspend will be
> successfull. 
> So for aggressive suspending as a powersave-feature you need to poll
> (i.e. retry upon failure). because you don't want to stay unsuspended. 

Clearly if it fails, there something to be done, right? So whoever does
the thing will communicate with the suspend manager that it's going to
do and has finished doing its thing, at which point it will try again.

That's event driven, not polled.

Also, if you want you can keep the kernel-side auto-suspend side, and
have the suspend manager clear on !0 and re-establish the auto-suspend
state on 0.

But you really don't need this device thingy.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Arve Hjønnevåg]

On Wed, May 26, 2010 at 2:33 AM, Peter Zijlstra <peterz@infradead.org> wrote:
> On Wed, 2010-05-26 at 11:23 +0200, Florian Mickler wrote:
>
>> There is polling, because the suspend manager in userspace doesn't have
>> the whole picture. i.e. it doesn't know if a suspend will be
>> successfull.
>> So for aggressive suspending as a powersave-feature you need to poll
>> (i.e. retry upon failure). because you don't want to stay unsuspended.
>
> Clearly if it fails, there something to be done, right? So whoever does
> the thing will communicate with the suspend manager that it's going to
> do and has finished doing its thing, at which point it will try again.
>

If you are talking about user-space code here, then that does not
work. Not all kernel events that need to block suspend make it to
user-space.

> That's event driven, not polled.
>
> Also, if you want you can keep the kernel-side auto-suspend side, and
> have the suspend manager clear on !0 and re-establish the auto-suspend
> state on 0.
>
> But you really don't need this device thingy.
>

I'm not sure what you are proposing that we use instead. Both
user-space and kernel code needs to block suspend. If we don't have
suspend blockers in the kernel then user-space needs to poll when a
driver blocks suspend by returning an error from its suspend hook.

-- 
Arve Hjønnevåg

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Peter Zijlstra]

On Wed, 2010-05-26 at 02:54 -0700, Arve Hjønnevåg wrote:
> 
> I'm not sure what you are proposing that we use instead. Both
> user-space and kernel code needs to block suspend. If we don't have
> suspend blockers in the kernel then user-space needs to poll when a
> driver blocks suspend by returning an error from its suspend hook. 

In particular I'm suggesting you ditch the /dev/suspend_block thing.

With a single suspend manager process that manages the suspend state you
can achieve the same goal.

When the suspend manager has a !0 busy-task count, it ensures the kernel
won't auto-suspend, when it again reaches a 0 busy-task count, it
re-instates the auto-suspend feature.

That's pretty much what that device would do too.

Ideally we would not do the auto-suspend thing at all and have
runtime-PM improved. Not running apps when they expect to run is like
the world turned upside down.

'Evil' apps could always report themselves as blocker anyway.
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Arve Hjønnevåg]

2010/5/26 Peter Zijlstra <peterz@infradead.org>:
> On Wed, 2010-05-26 at 02:54 -0700, Arve Hjønnevåg wrote:
>>
>> I'm not sure what you are proposing that we use instead. Both
>> user-space and kernel code needs to block suspend. If we don't have
>> suspend blockers in the kernel then user-space needs to poll when a
>> driver blocks suspend by returning an error from its suspend hook.
>
> In particular I'm suggesting you ditch the /dev/suspend_block thing.
>
> With a single suspend manager process that manages the suspend state you
> can achieve the same goal.
>

Yes we don't need the /dev interface, but it is useful. Without it any
program that needs to block suspend has to make a blocking ipc call
into the suspend manager process. Android already does this for java
code, but system processes written in C block suspend directly with
the kernel since they cannot use the java APIs.

> When the suspend manager has a !0 busy-task count, it ensures the kernel
> won't auto-suspend, when it again reaches a 0 busy-task count, it
> re-instates the auto-suspend feature.
>
> That's pretty much what that device would do too.
>
> Ideally we would not do the auto-suspend thing at all and have
> runtime-PM improved. Not running apps when they expect to run is like
> the world turned upside down.
>
> 'Evil' apps could always report themselves as blocker anyway.
>

-- 
Arve Hjønnevåg

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Peter Zijlstra]

On Wed, 2010-05-26 at 03:17 -0700, Arve Hjønnevåg wrote:
> > With a single suspend manager process that manages the suspend state you
> > can achieve the same goal.
> >
> 
> Yes we don't need the /dev interface, but it is useful. Without it any
> program that needs to block suspend has to make a blocking ipc call
> into the suspend manager process. Android already does this for java
> code, but system processes written in C block suspend directly with
> the kernel since they cannot use the java APIs. 

So provide a C interface to it as well?

Surely you can have the java thing have a unix socket or something a C
app can talk to. That shouldn't be hard at all.

Or make the suspend manager a C proglet and provide a JNI interface, or
whatever.
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Pekka Enberg]

On Wed, 2010-05-26 at 03:17 -0700, Arve Hjønnevåg wrote:
>> > With a single suspend manager process that manages the suspend state you
>> > can achieve the same goal.
>>
>> Yes we don't need the /dev interface, but it is useful. Without it any
>> program that needs to block suspend has to make a blocking ipc call
>> into the suspend manager process. Android already does this for java
>> code, but system processes written in C block suspend directly with
>> the kernel since they cannot use the java APIs.

2010/5/26 Peter Zijlstra <peterz@infradead.org>:
> So provide a C interface to it as well?
>
> Surely you can have the java thing have a unix socket or something a C
> app can talk to. That shouldn't be hard at all.
>
> Or make the suspend manager a C proglet and provide a JNI interface, or
> whatever.

Yup, I don't quite get Arve's argument either. C code can interact
with Java code (and vice versa) just fine in userspace.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Arve Hjønnevåg]

2010/5/26 Peter Zijlstra <peterz@infradead.org>:
> On Wed, 2010-05-26 at 03:17 -0700, Arve Hjønnevåg wrote:
>> > With a single suspend manager process that manages the suspend state you
>> > can achieve the same goal.
>> >
>>
>> Yes we don't need the /dev interface, but it is useful. Without it any
>> program that needs to block suspend has to make a blocking ipc call
>> into the suspend manager process. Android already does this for java
>> code, but system processes written in C block suspend directly with
>> the kernel since they cannot use the java APIs.
>
> So provide a C interface to it as well?
>

We could, but the result would be that any program that needs to block
suspend has to be android specific.

> Surely you can have the java thing have a unix socket or something a C
> app can talk to. That shouldn't be hard at all.
>
> Or make the suspend manager a C proglet and provide a JNI interface, or
> whatever.
>


-- 
Arve Hjønnevåg

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Pekka Enberg]

2010/5/26 Arve Hjønnevåg <arve@android.com>:
>> So provide a C interface to it as well?
>
> We could, but the result would be that any program that needs to block
> suspend has to be android specific.

Why? You could put the userspace daemon to tools/suspend-manager, for example?

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Vitaly Wool]

2010/5/26 Arve Hjønnevåg <arve@android.com>:
> 2010/5/26 Peter Zijlstra <peterz@infradead.org>:
>> On Wed, 2010-05-26 at 03:17 -0700, Arve Hjønnevåg wrote:
>>> > With a single suspend manager process that manages the suspend state you
>>> > can achieve the same goal.
>>> >
>>>
>>> Yes we don't need the /dev interface, but it is useful. Without it any
>>> program that needs to block suspend has to make a blocking ipc call
>>> into the suspend manager process. Android already does this for java
>>> code, but system processes written in C block suspend directly with
>>> the kernel since they cannot use the java APIs.
>>
>> So provide a C interface to it as well?
>>
>
> We could, but the result would be that any program that needs to block
> suspend has to be android specific.

Just a suspicion, but... The things you're saying don't make sense to
me other than if you're fighting with GPL in userspace here.

~Vitaly

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Matthew Garrett]

While this approach could be made to work, it's ugly in other ways. 
After wakeup, userspace has to pause for a while before it can trigger 
another sleep in order to give all the apps an opportunity to check for 
wakeup events and block suspend if they wish to. That's additional 
runtime that doesn't exist in the kernel-mediated case.

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

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Peter Zijlstra]

On Wed, 2010-05-26 at 13:49 +0100, Matthew Garrett wrote:

Lack of quoting makes it hard to see what your 'this' refers to. I'll
assume its the userspace suspend manager.

> While this approach could be made to work, it's ugly in other ways. 
> After wakeup, userspace has to pause for a while before it can trigger 
> another sleep in order to give all the apps an opportunity to check for 
> wakeup events and block suspend if they wish to. That's additional 
> runtime that doesn't exist in the kernel-mediated case.

I fail to see why. In both cases the woken userspace will contact a
central governing task, either the kernel or the userspace suspend
manager, and inform it there is work to be done, and please don't
suspend now.

Also, since we did get woken, there clearly is work to do and we should
only try suspending again once someone did inform us of completing it.

In both cases, once the event is fully handled, will there be
communication of this fact and suspend can be attempted.

I don't see a reason to 'wait' for anything -- except maybe speculate on
the avgerage wakeup rate and decide not to suspend quite yet.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Matthew Garrett]

On Wed, May 26, 2010 at 02:57:45PM +0200, Peter Zijlstra wrote:

> I fail to see why. In both cases the woken userspace will contact a
> central governing task, either the kernel or the userspace suspend
> manager, and inform it there is work to be done, and please don't
> suspend now.

Thinking about this, you're right - we don't have to wait, but that does 
result in another problem. Imagine we get two wakeup events 
approximately simultaneously. In the kernel-level universe the kernel 
knows when both have been handled. In the user-level universe, we may 
have one task schedule, bump the count, handle the event, drop the count 
and then we attempt a suspend again because the second event handler 
hasn't had an opportunity to run yet. We'll then attempt a suspend and 
immediately bounce back up. That's kind of wasteful, although it'd be 
somewhat mitigated by checking that right at the top of suspend entry 
and returning -EAGAIN or similar.

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

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[James Bottomley]

On Wed, 2010-05-26 at 13:29 +0300, Pekka Enberg wrote:
> Yup, I don't quite get Arve's argument either. C code can interact
> with Java code (and vice versa) just fine in userspace.

This is an incorrect statement.  It's possible for java to call C via
the JNI, even though there are quite a few gotchas that mean not just
*any* C code can do it (been there, tripped over some of them, although
they were all ultimately ironed out).  It's very difficult for C to call
directly into Java without being specially coded because it involves
creating and managing a JVM (so in general, arbitrary C code can't do
this).  The usual way we do C -> Java is process to process via some
intermediary like RPC or Corba or SOAP (or even JSON) ... which gets
very messy for a mobile device.

On Wed, 2010-05-26 at 12:21 +0200, Peter Zijlstra wrote:
> So provide a C interface to it as well?

The way Android is currently coded, all user space suspend blocks are
handled in Java code at what's called the Frameworks layer (Frameworks
is the Java API for apps) this is also how the suspend permissions are
managed and presented to the user.  The few C applications which block
suspend just manipulate the device directly.

> Surely you can have the java thing have a unix socket or something a C
> app can talk to. That shouldn't be hard at all.
> 
> Or make the suspend manager a C proglet and provide a JNI interface,
> or whatever.

It's a fairly large piece of code to try to rewrite in C, so I don't
think that's feasible on a reasonable timescale.  Android does have the
concept of special sockets that can be used to communicate from less to
more privileged processes (it has a very segmented runtime model), so
these might be usable ... they have a drawback that they're essentially
named pipes, so no multiplexing, but one per suspend influencing C
process shouldn't be a huge burden.

James

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Peter Zijlstra]

On Wed, 2010-05-26 at 11:18 -0500, James Bottomley wrote:
> > Or make the suspend manager a C proglet and provide a JNI interface,
> > or whatever.
> 
> It's a fairly large piece of code to try to rewrite in C, so I don't
> think that's feasible on a reasonable timescale.  Android does have the
> concept of special sockets that can be used to communicate from less to
> more privileged processes (it has a very segmented runtime model), so
> these might be usable ... they have a drawback that they're essentially
> named pipes, so no multiplexing, but one per suspend influencing C
> process shouldn't be a huge burden. 

It wouldn't need to convert the whole Frameworks layer into C, just
enough to manage the suspend state.

Anyway, I think there's been enough arguments against even the concept
of opportunistic/auto-suspend, and I for one will object with a NAK if
Rafael send this to Linus.

The whole idea of segregating userspace like that, and not letting
runnable thing run is very ill considered indeed.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Kevin Hilman]

Peter Zijlstra <peterz@infradead.org> writes:

> Anyway, I think there's been enough arguments against even the concept
> of opportunistic/auto-suspend, and I for one will object with a NAK if
> Rafael send this to Linus.

It's already been submitted to Linus for 2.6.35, but not on LKML:

  https://lists.linux-foundation.org/pipermail/linux-pm/2010-May/025811.html

Kevin

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[James Bottomley]

On Wed, 2010-05-26 at 18:28 +0200, Peter Zijlstra wrote:
> On Wed, 2010-05-26 at 11:18 -0500, James Bottomley wrote:
> > > Or make the suspend manager a C proglet and provide a JNI interface,
> > > or whatever.
> > 
> > It's a fairly large piece of code to try to rewrite in C, so I don't
> > think that's feasible on a reasonable timescale.  Android does have the
> > concept of special sockets that can be used to communicate from less to
> > more privileged processes (it has a very segmented runtime model), so
> > these might be usable ... they have a drawback that they're essentially
> > named pipes, so no multiplexing, but one per suspend influencing C
> > process shouldn't be a huge burden. 
> 
> It wouldn't need to convert the whole Frameworks layer into C, just
> enough to manage the suspend state.

That's actually what I was saying ... converting the whole frameworks
would be impossible.  I'm saying the suspend state manager is still a
large piece of work.

> Anyway, I think there's been enough arguments against even the concept
> of opportunistic/auto-suspend, and I for one will object with a NAK if
> Rafael send this to Linus.

Well, I suppose that's your prerogative.

> The whole idea of segregating userspace like that, and not letting
> runnable thing run is very ill considered indeed.

OK, so this is an approach difference.  I see it as controlling an
application resource problem, which is similar to quotas on filesystems.
The opinion divide seems to come down to those who think the application
stack has to be fixed before a working device can be released, and those
who want to make the devices work now and supply pressure to fix the
applications as well.

I fail to see how, in an environment where the whole value of the device
is the third party supplied applications that the former view is
reasonable.  I equally fail to see how the latter is achievable without
segregating userspace into trusted and untrusted and taking action to
curb the power consumption of untrusted applications.

Given that I'm in the latter category, I think suspend blockers is a
reasonable solution to an existing problem.  I like Alan's idea of
restricting the API into a single user space program so we contain the
API contamination ... but realistically that's mostly the current
suspend blockers anyway.

James

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Pavel Machek]

On Wed 2010-05-26 18:28:28, Peter Zijlstra wrote:
> On Wed, 2010-05-26 at 11:18 -0500, James Bottomley wrote:
> > > Or make the suspend manager a C proglet and provide a JNI interface,
> > > or whatever.
> > 
> > It's a fairly large piece of code to try to rewrite in C, so I don't
> > think that's feasible on a reasonable timescale.  Android does have the
> > concept of special sockets that can be used to communicate from less to
> > more privileged processes (it has a very segmented runtime model), so
> > these might be usable ... they have a drawback that they're essentially
> > named pipes, so no multiplexing, but one per suspend influencing C
> > process shouldn't be a huge burden. 
> 
> It wouldn't need to convert the whole Frameworks layer into C, just
> enough to manage the suspend state.
> 
> Anyway, I think there's been enough arguments against even the concept
> of opportunistic/auto-suspend, and I for one will object with a NAK if
> Rafael send this to Linus.

It was submitted already. I tried to followup with NAK, but can't
currently see it in the archive.
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Peter Zijlstra]

On Wed, 2010-05-26 at 11:54 -0500, James Bottomley wrote:
> Given that I'm in the latter category, I think suspend blockers is a
> reasonable solution to an existing problem.  I like Alan's idea of
> restricting the API into a single user space program so we contain the
> API contamination ... but realistically that's mostly the current
> suspend blockers anyway. 

There's a _large_ difference between resource limits and these wonky
suspend blockers.

The main and most important one being that suspend is a global property
and can/will hurt sensible tasks. It puts the whole task model upside
down.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Peter Zijlstra]

On Wed, 2010-05-26 at 18:59 +0200, Pavel Machek wrote:
> On Wed 2010-05-26 18:28:28, Peter Zijlstra wrote:
> > On Wed, 2010-05-26 at 11:18 -0500, James Bottomley wrote:
> > > > Or make the suspend manager a C proglet and provide a JNI interface,
> > > > or whatever.
> > > 
> > > It's a fairly large piece of code to try to rewrite in C, so I don't
> > > think that's feasible on a reasonable timescale.  Android does have the
> > > concept of special sockets that can be used to communicate from less to
> > > more privileged processes (it has a very segmented runtime model), so
> > > these might be usable ... they have a drawback that they're essentially
> > > named pipes, so no multiplexing, but one per suspend influencing C
> > > process shouldn't be a huge burden. 
> > 
> > It wouldn't need to convert the whole Frameworks layer into C, just
> > enough to manage the suspend state.
> > 
> > Anyway, I think there's been enough arguments against even the concept
> > of opportunistic/auto-suspend, and I for one will object with a NAK if
> > Rafael send this to Linus.
> 
> It was submitted already. I tried to followup with NAK, but can't
> currently see it in the archive.

It was apparently hidden on some funky list. Hiding pull requests is bad
enough, but hiding pull requests for contended features is just plain
wrong.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[James Bottomley]

On Wed, 2010-05-26 at 19:00 +0200, Peter Zijlstra wrote:
> On Wed, 2010-05-26 at 11:54 -0500, James Bottomley wrote:
> > Given that I'm in the latter category, I think suspend blockers is a
> > reasonable solution to an existing problem.  I like Alan's idea of
> > restricting the API into a single user space program so we contain the
> > API contamination ... but realistically that's mostly the current
> > suspend blockers anyway. 
> 
> There's a _large_ difference between resource limits and these wonky
> suspend blockers.

Well, you have policy and then you have implementation ... suspend
blockers just looks like an implementation to me.  It seems to be
reasonably well suited in that regard ... after all, we kill processes
that exhaust memory for instance or cut off write privileges to those
that go over quota.  Preventing power hungry processes from consuming
power by not allowing them to run until there's a wakeup event is fairly
gentle by those standards.

> The main and most important one being that suspend is a global property
> and can/will hurt sensible tasks. It puts the whole task model upside
> down.

OK, so I believe you have an android phone ... it already implements
this model ... specifically what are the problems on that platform this
causes?

James

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Peter Zijlstra]

On Wed, 2010-05-26 at 12:14 -0500, James Bottomley wrote:
> On Wed, 2010-05-26 at 19:00 +0200, Peter Zijlstra wrote:
> > On Wed, 2010-05-26 at 11:54 -0500, James Bottomley wrote:
> > > Given that I'm in the latter category, I think suspend blockers is a
> > > reasonable solution to an existing problem.  I like Alan's idea of
> > > restricting the API into a single user space program so we contain the
> > > API contamination ... but realistically that's mostly the current
> > > suspend blockers anyway. 
> > 
> > There's a _large_ difference between resource limits and these wonky
> > suspend blockers.
> 
> Well, you have policy and then you have implementation ... suspend
> blockers just looks like an implementation to me.  It seems to be
> reasonably well suited in that regard ... after all, we kill processes
> that exhaust memory for instance or cut off write privileges to those
> that go over quota.  Preventing power hungry processes from consuming
> power by not allowing them to run until there's a wakeup event is fairly
> gentle by those standards.

The difference is that the limit should be per task. In this model a
process that only runs a little still gets suspended.

> > The main and most important one being that suspend is a global property
> > and can/will hurt sensible tasks. It puts the whole task model upside
> > down.
> 
> OK, so I believe you have an android phone ... it already implements
> this model ... specifically what are the problems on that platform this
> causes?

I do not have one, nor have I ever written an application for it (nor
will I likely ever do that, since I detest Java), but I would expect an
application to run when its runnable.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[James Bottomley]

On Wed, 2010-05-26 at 19:01 +0200, Peter Zijlstra wrote:
> On Wed, 2010-05-26 at 18:59 +0200, Pavel Machek wrote:
> > On Wed 2010-05-26 18:28:28, Peter Zijlstra wrote:
> > > On Wed, 2010-05-26 at 11:18 -0500, James Bottomley wrote:
> > > > > Or make the suspend manager a C proglet and provide a JNI interface,
> > > > > or whatever.
> > > > 
> > > > It's a fairly large piece of code to try to rewrite in C, so I don't
> > > > think that's feasible on a reasonable timescale.  Android does have the
> > > > concept of special sockets that can be used to communicate from less to
> > > > more privileged processes (it has a very segmented runtime model), so
> > > > these might be usable ... they have a drawback that they're essentially
> > > > named pipes, so no multiplexing, but one per suspend influencing C
> > > > process shouldn't be a huge burden. 
> > > 
> > > It wouldn't need to convert the whole Frameworks layer into C, just
> > > enough to manage the suspend state.
> > > 
> > > Anyway, I think there's been enough arguments against even the concept
> > > of opportunistic/auto-suspend, and I for one will object with a NAK if
> > > Rafael send this to Linus.
> > 
> > It was submitted already. I tried to followup with NAK, but can't
> > currently see it in the archive.

You mean this one:

https://lists.linux-foundation.org/pipermail/linux-pm/2010-May/025689.html

?

> It was apparently hidden on some funky list.

Sending a PM pull request to the PM list doesn't really strike me as the
height of obfuscation.  Plus almost everyone who objected was on the cc
list.

>  Hiding pull requests is bad enough, but hiding pull requests for
> contended features is just plain wrong.

I don't think it's a conspiracy ... just standard operating procedure
for this subsystem.  I do think cc'ing lkml is good practise (having
been yelled at for not doing that in the past) but it's certainly not
universal practise.

James

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Pekka Enberg]

Hi James,

On Wed, 2010-05-26 at 13:29 +0300, Pekka Enberg wrote:
>> Yup, I don't quite get Arve's argument either. C code can interact
>> with Java code (and vice versa) just fine in userspace.

On Wed, May 26, 2010 at 7:18 PM, James Bottomley
<James.Bottomley@suse.de> wrote:
> This is an incorrect statement.  It's possible for java to call C via
> the JNI, even though there are quite a few gotchas that mean not just
> *any* C code can do it (been there, tripped over some of them, although
> they were all ultimately ironed out).  It's very difficult for C to call
> directly into Java without being specially coded because it involves
> creating and managing a JVM (so in general, arbitrary C code can't do
> this).  The usual way we do C -> Java is process to process via some
> intermediary like RPC or Corba or SOAP (or even JSON) ... which gets
> very messy for a mobile device.

Incorrect statement how exactly? A JVM can do mmap(), for example,
just fine through FileChannel.map() so there's no need for
heavy-weight RPC. Furthermore, the whole discussion is moot anyway as
Android runs Dalvik which can be hacked to support whatever
communication mechanism is the best choice here.

So can we drop the whole "we need to do it in kernel because Java is
hard" nonsense and concentrate on real issues?

                               Pekka

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Pavel Machek]

On Wed 2010-05-26 12:14:02, James Bottomley wrote:
> On Wed, 2010-05-26 at 19:00 +0200, Peter Zijlstra wrote:
> > On Wed, 2010-05-26 at 11:54 -0500, James Bottomley wrote:
> > > Given that I'm in the latter category, I think suspend blockers is a
> > > reasonable solution to an existing problem.  I like Alan's idea of
> > > restricting the API into a single user space program so we contain the
> > > API contamination ... but realistically that's mostly the current
> > > suspend blockers anyway. 
> > 
> > There's a _large_ difference between resource limits and these wonky
> > suspend blockers.
> 
> Well, you have policy and then you have implementation ... suspend
> blockers just looks like an implementation to me.  It seems to be
> reasonably well suited in that regard ... after all, we kill processes
> that exhaust memory for instance or cut off write privileges to those
> that go over quota.  Preventing power hungry processes from consuming
> power by not allowing them to run until there's a wakeup event is fairly
> gentle by those standards.

Well, generic mechanism would stop processes when they run over
something -- not arbitrarily link it to device wakeup.

> > The main and most important one being that suspend is a global property
> > and can/will hurt sensible tasks. It puts the whole task model upside
> > down.
> 
> OK, so I believe you have an android phone ... it already implements
> this model ... specifically what are the problems on that platform this
> causes?

Besides that it is not linux system at all?

Yes, with custom userspace it works extremely nicely.

Had anyone even tried running oportunistic suspend on normal desktop?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[James Bottomley]

On Wed, 2010-05-26 at 19:23 +0200, Peter Zijlstra wrote:
> On Wed, 2010-05-26 at 12:14 -0500, James Bottomley wrote:
> > On Wed, 2010-05-26 at 19:00 +0200, Peter Zijlstra wrote:
> > > On Wed, 2010-05-26 at 11:54 -0500, James Bottomley wrote:
> > > > Given that I'm in the latter category, I think suspend blockers is a
> > > > reasonable solution to an existing problem.  I like Alan's idea of
> > > > restricting the API into a single user space program so we contain the
> > > > API contamination ... but realistically that's mostly the current
> > > > suspend blockers anyway. 
> > > 
> > > There's a _large_ difference between resource limits and these wonky
> > > suspend blockers.
> > 
> > Well, you have policy and then you have implementation ... suspend
> > blockers just looks like an implementation to me.  It seems to be
> > reasonably well suited in that regard ... after all, we kill processes
> > that exhaust memory for instance or cut off write privileges to those
> > that go over quota.  Preventing power hungry processes from consuming
> > power by not allowing them to run until there's a wakeup event is fairly
> > gentle by those standards.
> 
> The difference is that the limit should be per task.

How?  You've got two different limits ... one the power the application
should be consuming when doing useful work for the user and the other is
the idle power.  A badly constructed app may only be bad on idle
power ... how is the scheduler going to detect this, exactly?  And what
do we do to applications we've detected are over consuming idle power?

>  In this model a
> process that only runs a little still gets suspended.

That's why I think it looks like a reasonable solution.  For this to
work, I agree you have to have all events the user is interested in wake
the system up ... but on most embedded platforms, they do.

> > > The main and most important one being that suspend is a global property
> > > and can/will hurt sensible tasks. It puts the whole task model upside
> > > down.
> > 
> > OK, so I believe you have an android phone ... it already implements
> > this model ... specifically what are the problems on that platform this
> > causes?
> 
> I do not have one, nor have I ever written an application for it (nor
> will I likely ever do that, since I detest Java), but I would expect an
> application to run when its runnable.

OK, so I've got one ... tell me what I should see and I'll try to
reproduce.

James

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[James Bottomley]

On Wed, 2010-05-26 at 20:25 +0300, Pekka Enberg wrote:
> Hi James,
> 
> On Wed, 2010-05-26 at 13:29 +0300, Pekka Enberg wrote:
> >> Yup, I don't quite get Arve's argument either. C code can interact
> >> with Java code (and vice versa) just fine in userspace.
> 
> On Wed, May 26, 2010 at 7:18 PM, James Bottomley
> <James.Bottomley@suse.de> wrote:
> > This is an incorrect statement.  It's possible for java to call C via
> > the JNI, even though there are quite a few gotchas that mean not just
> > *any* C code can do it (been there, tripped over some of them, although
> > they were all ultimately ironed out).  It's very difficult for C to call
> > directly into Java without being specially coded because it involves
> > creating and managing a JVM (so in general, arbitrary C code can't do
> > this).  The usual way we do C -> Java is process to process via some
> > intermediary like RPC or Corba or SOAP (or even JSON) ... which gets
> > very messy for a mobile device.
> 
> Incorrect statement how exactly? A JVM can do mmap(), for example,
> just fine through FileChannel.map() so there's no need for
> heavy-weight RPC.

Incorrect in that an arbitrary C application can't link to a java API.
mmap and some other messaging (like signals) is just another form of
IPC ... the list I gave wasn't exhaustive.

>  Furthermore, the whole discussion is moot anyway as
> Android runs Dalvik which can be hacked to support whatever
> communication mechanism is the best choice here.
> 
> So can we drop the whole "we need to do it in kernel because Java is
> hard" nonsense and concentrate on real issues?

I've lost you.  This argument seems to hinge on whether or not you
believe that suspend in any form is a solution to the rogue app
problem ... whether it's implemented in Java or C is an ancillary issue.

James

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Pavel Machek]

Hi!

> > > > The main and most important one being that suspend is a global property
> > > > and can/will hurt sensible tasks. It puts the whole task model upside
> > > > down.
> > > 
> > > OK, so I believe you have an android phone ... it already implements
> > > this model ... specifically what are the problems on that platform this
> > > causes?
> > 
> > I do not have one, nor have I ever written an application for it (nor
> > will I likely ever do that, since I detest Java), but I would expect an
> > application to run when its runnable.
> 
> OK, so I've got one ... tell me what I should see and I'll try to
> reproduce.

Umm... try to boot ordinary distro and see how it copes with
opportunistic suspend?

I do have android here, and of course it work well with custom
userland. Question is: can common distro be reasonably modified to
work with suspend blockers, in a way that's backward compatible?
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Thomas Gleixner]

On Wed, 26 May 2010, James Bottomley wrote:
> On Wed, 2010-05-26 at 19:01 +0200, Peter Zijlstra wrote:
> > On Wed, 2010-05-26 at 18:59 +0200, Pavel Machek wrote:
> > > On Wed 2010-05-26 18:28:28, Peter Zijlstra wrote:
> > > > On Wed, 2010-05-26 at 11:18 -0500, James Bottomley wrote:
> > > > > > Or make the suspend manager a C proglet and provide a JNI interface,
> > > > > > or whatever.
> > > > > 
> > > > > It's a fairly large piece of code to try to rewrite in C, so I don't
> > > > > think that's feasible on a reasonable timescale.  Android does have the
> > > > > concept of special sockets that can be used to communicate from less to
> > > > > more privileged processes (it has a very segmented runtime model), so
> > > > > these might be usable ... they have a drawback that they're essentially
> > > > > named pipes, so no multiplexing, but one per suspend influencing C
> > > > > process shouldn't be a huge burden. 
> > > > 
> > > > It wouldn't need to convert the whole Frameworks layer into C, just
> > > > enough to manage the suspend state.
> > > > 
> > > > Anyway, I think there's been enough arguments against even the concept
> > > > of opportunistic/auto-suspend, and I for one will object with a NAK if
> > > > Rafael send this to Linus.
> > > 
> > > It was submitted already. I tried to followup with NAK, but can't
> > > currently see it in the archive.
> 
> You mean this one:
> 
> https://lists.linux-foundation.org/pipermail/linux-pm/2010-May/025689.html
> 
> ?
> 
> > It was apparently hidden on some funky list.
> 
> Sending a PM pull request to the PM list doesn't really strike me as the
> height of obfuscation.  Plus almost everyone who objected was on the cc
> list.
> 
> >  Hiding pull requests is bad enough, but hiding pull requests for
> > contended features is just plain wrong.
> 
> I don't think it's a conspiracy ... just standard operating procedure
> for this subsystem.  I do think cc'ing lkml is good practise (having
> been yelled at for not doing that in the past) but it's certainly not
> universal practise.

At least it would be good style for a topic which is

   1) contended like this one

   2) pushing an intrusive feature last minute which has been merged
      into the pm tree barely two days ago.

Darn, _we_ have to deal with that forever as it sets a crappy user
space ABI in stone.

Thanks,

	tglx

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Pekka Enberg]

On Wed, May 26, 2010 at 8:40 PM, James Bottomley
<James.Bottomley@suse.de> wrote:
>> On Wed, 2010-05-26 at 13:29 +0300, Pekka Enberg wrote:
>> >> Yup, I don't quite get Arve's argument either. C code can interact
>> >> with Java code (and vice versa) just fine in userspace.
>>
>> On Wed, May 26, 2010 at 7:18 PM, James Bottomley
>> <James.Bottomley@suse.de> wrote:
>> > This is an incorrect statement.  It's possible for java to call C via
>> > the JNI, even though there are quite a few gotchas that mean not just
>> > *any* C code can do it (been there, tripped over some of them, although
>> > they were all ultimately ironed out).  It's very difficult for C to call
>> > directly into Java without being specially coded because it involves
>> > creating and managing a JVM (so in general, arbitrary C code can't do
>> > this).  The usual way we do C -> Java is process to process via some
>> > intermediary like RPC or Corba or SOAP (or even JSON) ... which gets
>> > very messy for a mobile device.
>>
>> Incorrect statement how exactly? A JVM can do mmap(), for example,
>> just fine through FileChannel.map() so there's no need for
>> heavy-weight RPC.
>
> Incorrect in that an arbitrary C application can't link to a java API.

Dunno what you mean by "arbitrary" C application but yes, I do agree
that using JNI is somewhat cumbersome. Not sure if that's relevant,
though, as it can all be hidden in the framework APIs.

> mmap and some other messaging (like signals) is just another form of
> IPC ... the list I gave wasn't exhaustive.

The protocols you mentioned are meant for RPC (as in remote) and
messaging, not for IPC which I'm assume is needed for the suspend
manager process Peter is talking about.

>>  Furthermore, the whole discussion is moot anyway as
>> Android runs Dalvik which can be hacked to support whatever
>> communication mechanism is the best choice here.
>>
>> So can we drop the whole "we need to do it in kernel because Java is
>> hard" nonsense and concentrate on real issues?
>
> I've lost you.  This argument seems to hinge on whether or not you
> believe that suspend in any form is a solution to the rogue app
> problem ... whether it's implemented in Java or C is an ancillary issue.

I'm responding to the original argument from Arve that C programs
can't use the existing suspend manager because it's written in Java.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[James Bottomley]

On Wed, 2010-05-26 at 19:42 +0200, Pavel Machek wrote:
> Hi!
> 
> > > > > The main and most important one being that suspend is a global property
> > > > > and can/will hurt sensible tasks. It puts the whole task model upside
> > > > > down.
> > > > 
> > > > OK, so I believe you have an android phone ... it already implements
> > > > this model ... specifically what are the problems on that platform this
> > > > causes?
> > > 
> > > I do not have one, nor have I ever written an application for it (nor
> > > will I likely ever do that, since I detest Java), but I would expect an
> > > application to run when its runnable.
> > 
> > OK, so I've got one ... tell me what I should see and I'll try to
> > reproduce.
> 
> Umm... try to boot ordinary distro and see how it copes with
> opportunistic suspend?

That's not really going to help, is it?  The issue I was curious are
what are the bad things that result from interfering with the regular
scheduling of processes ... because undeniably suspend (whether
opportunistic or ordinary) does produce this interference.

I could boot debian on an android and have it suspend ... that's still
not going to answer my question.

> I do have android here, and of course it work well with custom
> userland. Question is: can common distro be reasonably modified to
> work with suspend blockers, in a way that's backward compatible?

You mean how an app could run if it was compiled with suspend blockers
but the platform doesn't support it?  That's a simple runtime switch in
the library surely?

James

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[James Bottomley]

On Wed, 2010-05-26 at 19:51 +0200, Thomas Gleixner wrote:
> On Wed, 26 May 2010, James Bottomley wrote:
> > On Wed, 2010-05-26 at 19:01 +0200, Peter Zijlstra wrote:
> > > On Wed, 2010-05-26 at 18:59 +0200, Pavel Machek wrote:
> > > > On Wed 2010-05-26 18:28:28, Peter Zijlstra wrote:
> > > > > On Wed, 2010-05-26 at 11:18 -0500, James Bottomley wrote:
> > > > > > > Or make the suspend manager a C proglet and provide a JNI interface,
> > > > > > > or whatever.
> > > > > > 
> > > > > > It's a fairly large piece of code to try to rewrite in C, so I don't
> > > > > > think that's feasible on a reasonable timescale.  Android does have the
> > > > > > concept of special sockets that can be used to communicate from less to
> > > > > > more privileged processes (it has a very segmented runtime model), so
> > > > > > these might be usable ... they have a drawback that they're essentially
> > > > > > named pipes, so no multiplexing, but one per suspend influencing C
> > > > > > process shouldn't be a huge burden. 
> > > > > 
> > > > > It wouldn't need to convert the whole Frameworks layer into C, just
> > > > > enough to manage the suspend state.
> > > > > 
> > > > > Anyway, I think there's been enough arguments against even the concept
> > > > > of opportunistic/auto-suspend, and I for one will object with a NAK if
> > > > > Rafael send this to Linus.
> > > > 
> > > > It was submitted already. I tried to followup with NAK, but can't
> > > > currently see it in the archive.
> > 
> > You mean this one:
> > 
> > https://lists.linux-foundation.org/pipermail/linux-pm/2010-May/025689.html
> > 
> > ?
> > 
> > > It was apparently hidden on some funky list.
> > 
> > Sending a PM pull request to the PM list doesn't really strike me as the
> > height of obfuscation.  Plus almost everyone who objected was on the cc
> > list.
> > 
> > >  Hiding pull requests is bad enough, but hiding pull requests for
> > > contended features is just plain wrong.
> > 
> > I don't think it's a conspiracy ... just standard operating procedure
> > for this subsystem.  I do think cc'ing lkml is good practise (having
> > been yelled at for not doing that in the past) but it's certainly not
> > universal practise.
> 
> At least it would be good style for a topic which is
> 
>    1) contended like this one
> 
>    2) pushing an intrusive feature last minute which has been merged
>       into the pm tree barely two days ago.

Don't disagree ... I was just explaining how it could happen while not
being a conspiracy.

> Darn, _we_ have to deal with that forever as it sets a crappy user
> space ABI in stone.

I really don't see how it is ... the ABI comes with a switch that allows
it to be disabled, so only platforms wishing to use it have to support
it.  Even on those platforms that do support it, we can translate most
of it into pm QoS stuff and if one day someone solves the rogue app
problem, we can migrate over.

James

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Valdis.Kletnieks]

[-- Attachment #1.1: Type: text/plain, Size: 863 bytes --]

On Wed, 26 May 2010 13:23:00 CDT, James Bottomley said:
> On Wed, 2010-05-26 at 19:51 +0200, Thomas Gleixner wrote:
> > Darn, _we_ have to deal with that forever as it sets a crappy user
> > space ABI in stone.
> 
> I really don't see how it is ... the ABI comes with a switch that allows
> it to be disabled, so only platforms wishing to use it have to support
> it.  Even on those platforms that do support it, we can translate most
> of it into pm QoS stuff and if one day someone solves the rogue app
> problem, we can migrate over.

And yet, the OSS drivers are *still* in-tree, even though similar arguments
apply to an OSS->ALSA migration.  And there's a bunch of other stuff in
Documentation/feature-removal-schedule.txt in a similar situation.

Remember - programming is like sex. One poorly planned release and you're
stuck maintaining it for years. :)

[-- Attachment #1.2: Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Florian Mickler]

On Wed, 26 May 2010 19:28:24 +0200
Pavel Machek <pavel@ucw.cz> wrote:


> Besides that it is not linux system at all?

I believe the kernel to be a layer between userspace and hardware. What
business is it to the kernel if it runs whatever android
uses as init process or /bin/bash, sys-v-init or systemd?

There is this thing called choice. 

> 
> Yes, with custom userspace it works extremely nicely.
> 
> Had anyone even tried running oportunistic suspend on normal desktop?
> 									Pavel

I don't think this is a valid concern. Just because current linux/gnu
systems don't implement the userspace part of this interface doesn't
mean it is useless or broken. 

Cheers,
Flo

-- 
Just because I argue an issue, doesn't mean I agree with either side of
it.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[James Bottomley]

On Wed, 2010-05-26 at 14:50 -0400, Valdis.Kletnieks@vt.edu wrote:
> On Wed, 26 May 2010 13:23:00 CDT, James Bottomley said:
> > On Wed, 2010-05-26 at 19:51 +0200, Thomas Gleixner wrote:
> > > Darn, _we_ have to deal with that forever as it sets a crappy user
> > > space ABI in stone.
> > 
> > I really don't see how it is ... the ABI comes with a switch that allows
> > it to be disabled, so only platforms wishing to use it have to support
> > it.  Even on those platforms that do support it, we can translate most
> > of it into pm QoS stuff and if one day someone solves the rogue app
> > problem, we can migrate over.
> 
> And yet, the OSS drivers are *still* in-tree, even though similar arguments
> apply to an OSS->ALSA migration.  And there's a bunch of other stuff in
> Documentation/feature-removal-schedule.txt in a similar situation.

On the other hand APM, which is what all laptops used to use to suspend
is gone from the x86 kernel today ... replaced by ACPI or other platform
mechanisms, so it is at least possible.

James

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Rafael J. Wysocki]

On Wednesday 26 May 2010, Matthew Garrett wrote:
> On Wed, May 26, 2010 at 02:57:45PM +0200, Peter Zijlstra wrote:
> 
> > I fail to see why. In both cases the woken userspace will contact a
> > central governing task, either the kernel or the userspace suspend
> > manager, and inform it there is work to be done, and please don't
> > suspend now.
> 
> Thinking about this, you're right - we don't have to wait, but that does 
> result in another problem. Imagine we get two wakeup events 
> approximately simultaneously. In the kernel-level universe the kernel 
> knows when both have been handled. In the user-level universe, we may 
> have one task schedule, bump the count, handle the event, drop the count 
> and then we attempt a suspend again because the second event handler 
> hasn't had an opportunity to run yet. We'll then attempt a suspend and 
> immediately bounce back up. That's kind of wasteful, although it'd be 
> somewhat mitigated by checking that right at the top of suspend entry 
> and returning -EAGAIN or similar.

I still think it would cause a loop-alike behavior between the user space
power manager and the kernel PM core to happen, because the power manager
will always have to check the user space counter after a failing suspend
attempt.

Rafael

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Rafael J. Wysocki]

On Wednesday 26 May 2010, Florian Mickler wrote:
> On Wed, 26 May 2010 19:28:24 +0200
> Pavel Machek <pavel@ucw.cz> wrote:
> 
> 
> > Besides that it is not linux system at all?
> 
> I believe the kernel to be a layer between userspace and hardware. What
> business is it to the kernel if it runs whatever android
> uses as init process or /bin/bash, sys-v-init or systemd?
> 
> There is this thing called choice. 
> 
> > 
> > Yes, with custom userspace it works extremely nicely.
> > 
> > Had anyone even tried running oportunistic suspend on normal desktop?
> > 									Pavel
> 
> I don't think this is a valid concern. Just because current linux/gnu
> systems don't implement the userspace part of this interface doesn't
> mean it is useless or broken. 

Agreed.

BTW, this is a valid point.  We are used to think about Linux as the kernel
plus the GNU user space, but that's not how it has to be.

Thanks,
Rafael

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Rafael J. Wysocki]

On Wednesday 26 May 2010, Peter Zijlstra wrote:
> On Wed, 2010-05-26 at 18:59 +0200, Pavel Machek wrote:
> > On Wed 2010-05-26 18:28:28, Peter Zijlstra wrote:
> > > On Wed, 2010-05-26 at 11:18 -0500, James Bottomley wrote:
> > > > > Or make the suspend manager a C proglet and provide a JNI interface,
> > > > > or whatever.
> > > > 
> > > > It's a fairly large piece of code to try to rewrite in C, so I don't
> > > > think that's feasible on a reasonable timescale.  Android does have the
> > > > concept of special sockets that can be used to communicate from less to
> > > > more privileged processes (it has a very segmented runtime model), so
> > > > these might be usable ... they have a drawback that they're essentially
> > > > named pipes, so no multiplexing, but one per suspend influencing C
> > > > process shouldn't be a huge burden. 
> > > 
> > > It wouldn't need to convert the whole Frameworks layer into C, just
> > > enough to manage the suspend state.
> > > 
> > > Anyway, I think there's been enough arguments against even the concept
> > > of opportunistic/auto-suspend, and I for one will object with a NAK if
> > > Rafael send this to Linus.
> > 
> > It was submitted already. I tried to followup with NAK, but can't
> > currently see it in the archive.
> 
> It was apparently hidden on some funky list. Hiding pull requests is bad
> enough, but hiding pull requests for contended features is just plain
> wrong.

It was not intentionally hidden.  I think my mailer did that wrong because
the CC list was too long or something like this.  I surely intended to send
it to the LKML, so sorry for the inconvenience.

Rafael

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Rafael J. Wysocki]

On Wednesday 26 May 2010, Thomas Gleixner wrote:
> On Wed, 26 May 2010, James Bottomley wrote:
> > On Wed, 2010-05-26 at 19:01 +0200, Peter Zijlstra wrote:
> > > On Wed, 2010-05-26 at 18:59 +0200, Pavel Machek wrote:
> > > > On Wed 2010-05-26 18:28:28, Peter Zijlstra wrote:
> > > > > On Wed, 2010-05-26 at 11:18 -0500, James Bottomley wrote:
> > > > > > > Or make the suspend manager a C proglet and provide a JNI interface,
> > > > > > > or whatever.
> > > > > > 
> > > > > > It's a fairly large piece of code to try to rewrite in C, so I don't
> > > > > > think that's feasible on a reasonable timescale.  Android does have the
> > > > > > concept of special sockets that can be used to communicate from less to
> > > > > > more privileged processes (it has a very segmented runtime model), so
> > > > > > these might be usable ... they have a drawback that they're essentially
> > > > > > named pipes, so no multiplexing, but one per suspend influencing C
> > > > > > process shouldn't be a huge burden. 
> > > > > 
> > > > > It wouldn't need to convert the whole Frameworks layer into C, just
> > > > > enough to manage the suspend state.
> > > > > 
> > > > > Anyway, I think there's been enough arguments against even the concept
> > > > > of opportunistic/auto-suspend, and I for one will object with a NAK if
> > > > > Rafael send this to Linus.
> > > > 
> > > > It was submitted already. I tried to followup with NAK, but can't
> > > > currently see it in the archive.
> > 
> > You mean this one:
> > 
> > https://lists.linux-foundation.org/pipermail/linux-pm/2010-May/025689.html
> > 
> > ?
> > 
> > > It was apparently hidden on some funky list.
> > 
> > Sending a PM pull request to the PM list doesn't really strike me as the
> > height of obfuscation.  Plus almost everyone who objected was on the cc
> > list.
> > 
> > >  Hiding pull requests is bad enough, but hiding pull requests for
> > > contended features is just plain wrong.
> > 
> > I don't think it's a conspiracy ... just standard operating procedure
> > for this subsystem.  I do think cc'ing lkml is good practise (having
> > been yelled at for not doing that in the past) but it's certainly not
> > universal practise.
> 
> At least it would be good style for a topic which is

As I said in another message, I intended to send the pull request to the LKML,
but for some reason that didn't work.  Perhaps that was my fault, so sorry for
that, but I think the CC list contained people who had objections, so they
easily could respond (adding the CC to the LKML).

>    1) contended like this one
> 
>    2) pushing an intrusive feature last minute which has been merged
>       into the pm tree barely two days ago.

It had been discussed for a month before without any serious progress, though,
so I simply had to do something to make things go. :-)

> Darn, _we_ have to deal with that forever as it sets a crappy user
> space ABI in stone.

Well, I don't think it's _that_ bad, although it isn't nice either.

The whole reason why it's there is because the Google people want to collect
suspend blocker usage statistics, so that they can easily check how often
applications block suspending.  If we dropped the statistics part, the
interface might be substantially simplified.

Thanks,
Rafael



> 
> Thanks,
> 
> 	tglx
> 
> 

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Neil Brown]

On Wed, 26 May 2010 14:20:51 +0100
Matthew Garrett <mjg59@srcf.ucam.org> wrote:

> On Wed, May 26, 2010 at 02:57:45PM +0200, Peter Zijlstra wrote:
> 
> > I fail to see why. In both cases the woken userspace will contact a
> > central governing task, either the kernel or the userspace suspend
> > manager, and inform it there is work to be done, and please don't
> > suspend now.
> 
> Thinking about this, you're right - we don't have to wait, but that does 
> result in another problem. Imagine we get two wakeup events 
> approximately simultaneously. In the kernel-level universe the kernel 
> knows when both have been handled. In the user-level universe, we may 
> have one task schedule, bump the count, handle the event, drop the count 
> and then we attempt a suspend again because the second event handler 
> hasn't had an opportunity to run yet. We'll then attempt a suspend and 
> immediately bounce back up. That's kind of wasteful, although it'd be 
> somewhat mitigated by checking that right at the top of suspend entry 
> and returning -EAGAIN or similar.
> 

(I'm coming a little late to this party, so excuse me if I say something that
has already been covered however...)

The above triggers a sequence of thoughts which (When they settled down) look
a bit like this.

At the hardware level, there is a thing that we could call a "suspend
blocker".  It is an interrupt (presumably level-triggered) that causes the
processor to come out of suspend, or not to go into it.

Maybe it makes sense to export a similar thing from the kernel to user-space.
When any event happens that would wake the device (and drivers need to know
about these already), it would present something to user-space to say that
the event happened.

When user-space processes the event, it clears the event indicator.

When there are no more current event indicators, userspace is allowed to
request a suspend.  Obviously this could fail as an event could happen at any
moment, but the same is true when the kernel asks the device to suspend, an
interrupt might happen immediately to stop it.  But in either case an event
will be reported.  So when userspace requests a suspend and it fails, it
will see events reported and so will wait for them to be handled.

I imagine a sysfs directory with files that appear when events are pending.
We could have some separate mechanism for user-space processes to request
that the suspend-daemon not suspend.  Then it suspends whenever there are no
pending requests from user-space or from the kernel.

The advantage of this model of suspend-blockers is that it is a close
analogue for something that already exists in hardware so it isn't really
creating new concepts, just giving the Linux virtual-machine features that
have proved themselves in physical machines.

The cost is that any wake-up event needs to not only be handled, but also
explicitly acknowledged by clearing the relevant suspend-blocker (i.e.
removing the file from sysfs, or whatever interface was ultimately chosen).
I'm hoping that isn't a big cost.

NeilBrown

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Neil Brown]

On Wed, 26 May 2010 12:21:02 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Wed, 2010-05-26 at 03:17 -0700, Arve Hjønnevåg wrote:
> > > With a single suspend manager process that manages the suspend state you
> > > can achieve the same goal.
> > >
> > 
> > Yes we don't need the /dev interface, but it is useful. Without it any
> > program that needs to block suspend has to make a blocking ipc call
> > into the suspend manager process. Android already does this for java
> > code, but system processes written in C block suspend directly with
> > the kernel since they cannot use the java APIs. 
> 
> So provide a C interface to it as well?
> 
> Surely you can have the java thing have a unix socket or something a C
> app can talk to. That shouldn't be hard at all.
> 
> Or make the suspend manager a C proglet and provide a JNI interface, or
> whatever.

I fail to understand the modern fascination with complex IPC mechanisms.

If you have a userspace process that initiates suspends, and you want other
user-space processes to be able to block that suspend, then I would suggest
the use of a lock-file.  /var/run/suspend/blocked maybe.

To block suspend, you open the file and get a read lock.
To initiate a suspend you take a write-lock (blocking if necessary), then
ask the kernel to suspend.
To restrict access to particular users you use permissions - either group
based or ACLs (or both).
This is all easy to do from C or python or perl or presumably even java..
(I use this mechanism on my Freerunner and even have shell scripts that
happily prevent suspend).

NeilBrown
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Bernd Petrovitsch]

On Mit, 2010-05-26 at 21:15 +0200, Florian Mickler wrote:
> On Wed, 26 May 2010 19:28:24 +0200
> Pavel Machek <pavel@ucw.cz> wrote:
> 
> > Besides that it is not linux system at all?
> 
> I believe the kernel to be a layer between userspace and hardware. What
> business is it to the kernel if it runs whatever android
> uses as init process or /bin/bash, sys-v-init or systemd?

The kernel has no business and doesn't care. But precisely that is the
reason to clearly separate policy from means. And that separation is not
done in a good way by a loosely-defined API but by a (well-defined)
kernel-API which allows user-space so implement the policy (whatever it
might feel) by using the means (wiht appopriate parameters).
So the (kernel-)API should not use policy-defined (read: user-space)
semantics/values - plain simply because they will change (in no time).
But the kernel must use policy-independent semantics/values - if only
the suspend/wakeup-latency or whatever technically makes sense.
Perhaps some "will/want to be waken up every 0.1 second" makes sense for
periodic jobs.

> There is this thing called choice. 
>
> > Yes, with custom userspace it works extremely nicely.
> > 
> > Had anyone even tried running oportunistic suspend on normal desktop?
>
> I don't think this is a valid concern. Just because current linux/gnu

A lot of stuff is not (or no longer) implemented because it's - plain
simply - a bad idea in the first place.
Other than that: Linux/GNU-systems come from the server and desktop
world where power is not an important part. And laptop producers
obviously didn't care enough ...
And you do realize that in the non-embedded (and non-Apple) world the
user-space is not controlled by one entity in any way.
So forcing "good apps" vs "bad apps" doesn't workout - except you have
direct feedback like "powertop" which makes it easy for everyone
(especially the users) to rate the apps.
So the kernel actually must have means to do that. If user-space defines
the semantics of the API, the kernel can't do that because user-space is
always right.

> systems don't implement the userspace part of this interface doesn't
> mean it is useless or broken. 

Just because Android designed and/or implements it doesn't mean it's not
broken (in the medium to long run).
So what?

	Bernd
-- 
Bernd Petrovitsch                  Email : bernd@petrovitsch.priv.at
                     LUGA : http://www.luga.at

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Bernd Petrovitsch]

On Mit, 2010-05-26 at 13:23 -0500, James Bottomley wrote:
> On Wed, 2010-05-26 at 19:51 +0200, Thomas Gleixner wrote:
[...]
> > Darn, _we_ have to deal with that forever as it sets a crappy user
> > space ABI in stone.
> 
> I really don't see how it is ... the ABI comes with a switch that allows
> it to be disabled, so only platforms wishing to use it have to support
> it.  Even on those platforms that do support it, we can translate most

You completely missed the point: The crappy user interface - and
interferences with pother subsystems - must be maintained for ages - and
that is independent if one uses it or not. Even worse if it's not widely
used.

> of it into pm QoS stuff and if one day someone solves the rogue app
> problem, we can migrate over.

If it's so important for Android and no one else, Android can carry it
out of tree.

	Bernd
-- 
Bernd Petrovitsch                  Email : bernd@petrovitsch.priv.at
                     LUGA : http://www.luga.at

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Arve Hjønnevåg]

On Thu, May 27, 2010 at 1:17 AM, Bernd Petrovitsch
<bernd@petrovitsch.priv.at> wrote:
> On Mit, 2010-05-26 at 13:23 -0500, James Bottomley wrote:
>> On Wed, 2010-05-26 at 19:51 +0200, Thomas Gleixner wrote:
> [...]
>> > Darn, _we_ have to deal with that forever as it sets a crappy user
>> > space ABI in stone.
>>
>> I really don't see how it is ... the ABI comes with a switch that allows
>> it to be disabled, so only platforms wishing to use it have to support
>> it.  Even on those platforms that do support it, we can translate most
>
> You completely missed the point: The crappy user interface - and
> interferences with pother subsystems - must be maintained for ages - and
> that is independent if one uses it or not. Even worse if it's not widely
> used.

When (or if) the time comes that suspend is no longer useful, this api
becomes a NOP.

>
>> of it into pm QoS stuff and if one day someone solves the rogue app
>> problem, we can migrate over.
>
> If it's so important for Android and no one else, Android can carry it
> out of tree.
>

This is not only important for Android. If you use suspend on a
current Linux system you run the risk of loosing wakeup events. If you
have wakeup events that you cannot afford to lose your only option is
to never suspend. On some hardware (e.g. x86) the cost of not
suspending is always huge, on other hardware (many ARM SOCs) the cost
is only huge if your apps behave poorly.

-- 
Arve Hjønnevåg

Just fix the bug - Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Neil Brown]

On Thu, 27 May 2010 02:07:21 -0700
Arve Hjønnevåg <arve@android.com> wrote:

> >
> >> of it into pm QoS stuff and if one day someone solves the rogue app
> >> problem, we can migrate over.
> >
> > If it's so important for Android and no one else, Android can carry it
> > out of tree.
> >
> 
> This is not only important for Android. If you use suspend on a
> current Linux system you run the risk of loosing wakeup events. If you
> have wakeup events that you cannot afford to lose your only option is
> to never suspend. On some hardware (e.g. x86) the cost of not
> suspending is always huge, on other hardware (many ARM SOCs) the cost
> is only huge if your apps behave poorly.
> 

So here is my suggestion.
Rather than trying to push a feature that is clearly meeting lots of
resistance, the Android devs should state the problem as a bug that needs
fixing.  As you have.
Upstream is a lot more receptive of fixing bugs than adding features.

In this case the bug is that you cannot suspend without the risk of losing
wakeup events.  This is a real bug that for your use case is a serious
bug.  I've toyed with several ways of fixing this but the one that seems most
promising is to note that in the kernel the suspend process is two-stage with
a 'prepare' followed by a 'suspend'. Userspace cannot make that distinction
and so ends up with a race.

Maybe userspace should be able to say "prepare to suspend" with the meaning
that after a successful return, any event which would cause a wakeup sets a
flag so that the final suspend returns immediately (without actually going to
the lower power state).

Then your opportunistic suspend could be entirely in userspace where you
wouldn't have to fight with the kernel crowd :-)
The suspend-daemon would:
  Wait for all user-space suspend blocks to be dropped.
  Tell the kernel to "prepare to suspend".
  Tell all userspace programs which have registered for the message that they
    should prepare to suspend.  They have the opportunity at this point to
    take out a new suspend block if they notice an event that has
    just arrived
  Wait for all those programs to acknowledge
  If there are no new suspend blocks, tell the kernel to suspend
  else tell the kernel to abort the suspend.

This (I think) allows race-free opportunistic suspend in user-space where
you can do all the accounting you need.

I don't fully understand your requirements for accounting of devices drivers
rejecting or blocking a suspend, so I cannot say precisely how that would fit
in.  Maybe you just need to know - whenever the 'suspend request' completes -
what the wakeup events were.  It shouldn't be too hard to export that to
user-space via sysfs.

I won't propose an exact enhancement to the user-space interface for
requesting a suspend, but I suspect it should expose each of
  suspend_prepare
  suspend_devices_and_enter
  suspend_finish
(or close analogues there-of) to user-space.  It is tempting to map those to
"open-for-write", "write", "close", but I'm not sure that suspend_prepare
would be appropriate if the app was about to write "disk" - it is a pity that
both suspend and hibernate use the same sysfs file.

So just fix the bug, and everyone will be happy :-)

Thanks,
NeilBrown
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: Just fix the bug - Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Arve Hjønnevåg]

2010/5/27 Neil Brown <neilb@suse.de>:
> On Thu, 27 May 2010 02:07:21 -0700
> Arve Hjønnevåg <arve@android.com> wrote:
>
>> >
>> >> of it into pm QoS stuff and if one day someone solves the rogue app
>> >> problem, we can migrate over.
>> >
>> > If it's so important for Android and no one else, Android can carry it
>> > out of tree.
>> >
>>
>> This is not only important for Android. If you use suspend on a
>> current Linux system you run the risk of loosing wakeup events. If you
>> have wakeup events that you cannot afford to lose your only option is
>> to never suspend. On some hardware (e.g. x86) the cost of not
>> suspending is always huge, on other hardware (many ARM SOCs) the cost
>> is only huge if your apps behave poorly.
>>
>
> So here is my suggestion.
> Rather than trying to push a feature that is clearly meeting lots of
> resistance, the Android devs should state the problem as a bug that needs
> fixing.  As you have.
> Upstream is a lot more receptive of fixing bugs than adding features.
>
> In this case the bug is that you cannot suspend without the risk of losing
> wakeup events.  This is a real bug that for your use case is a serious
> bug.  I've toyed with several ways of fixing this but the one that seems most
> promising is to note that in the kernel the suspend process is two-stage with
> a 'prepare' followed by a 'suspend'. Userspace cannot make that distinction
> and so ends up with a race.
>
> Maybe userspace should be able to say "prepare to suspend" with the meaning
> that after a successful return, any event which would cause a wakeup sets a
> flag so that the final suspend returns immediately (without actually going to
> the lower power state).
>
> Then your opportunistic suspend could be entirely in userspace where you
> wouldn't have to fight with the kernel crowd :-)
> The suspend-daemon would:
>  Wait for all user-space suspend blocks to be dropped.
>  Tell the kernel to "prepare to suspend".
>  Tell all userspace programs which have registered for the message that they
>    should prepare to suspend.  They have the opportunity at this point to
>    take out a new suspend block if they notice an event that has
>    just arrived
>  Wait for all those programs to acknowledge
>  If there are no new suspend blocks, tell the kernel to suspend
>  else tell the kernel to abort the suspend.
>
> This (I think) allows race-free opportunistic suspend in user-space where
> you can do all the accounting you need.
>

Perhaps, but it forces all user space programs that get events from
the kernel to also receive messages from the suspend-daemon, check for
other events again, then respond to the suspend-daemon. The current
suspend blocker interface is easier to use. I don't see how your
suggestion avoids races for events that pass trough several kernel
layers though. If a wakeup event happened before the "prepare to
suspend" call but has not yet been passed to user-space, the
user-space program that needs this event will not know that it needs
to block suspend when it gets the prepare-to-suspend message.

> I don't fully understand your requirements for accounting of devices drivers
> rejecting or blocking a suspend, so I cannot say precisely how that would fit
> in.  Maybe you just need to know - whenever the 'suspend request' completes -
> what the wakeup events were.  It shouldn't be too hard to export that to
> user-space via sysfs.
>
> I won't propose an exact enhancement to the user-space interface for
> requesting a suspend, but I suspect it should expose each of
>  suspend_prepare
>  suspend_devices_and_enter
>  suspend_finish
> (or close analogues there-of) to user-space.  It is tempting to map those to
> "open-for-write", "write", "close", but I'm not sure that suspend_prepare
> would be appropriate if the app was about to write "disk" - it is a pity that
> both suspend and hibernate use the same sysfs file.
>
> So just fix the bug, and everyone will be happy :-)
>

I already have, but everyone do not appear to be happy.

-- 
Arve Hjønnevåg

Re: Just fix the bug - Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Neil Brown]

On Thu, 27 May 2010 21:57:02 -0700
Arve Hjønnevåg <arve@android.com> wrote:

> 2010/5/27 Neil Brown <neilb@suse.de>:
> > On Thu, 27 May 2010 02:07:21 -0700
> > Arve Hjønnevåg <arve@android.com> wrote:
> >
> >> >
> >> >> of it into pm QoS stuff and if one day someone solves the rogue app
> >> >> problem, we can migrate over.
> >> >
> >> > If it's so important for Android and no one else, Android can carry it
> >> > out of tree.
> >> >
> >>
> >> This is not only important for Android. If you use suspend on a
> >> current Linux system you run the risk of loosing wakeup events. If you
> >> have wakeup events that you cannot afford to lose your only option is
> >> to never suspend. On some hardware (e.g. x86) the cost of not
> >> suspending is always huge, on other hardware (many ARM SOCs) the cost
> >> is only huge if your apps behave poorly.
> >>
> >
> > So here is my suggestion.
> > Rather than trying to push a feature that is clearly meeting lots of
> > resistance, the Android devs should state the problem as a bug that needs
> > fixing.  As you have.
> > Upstream is a lot more receptive of fixing bugs than adding features.
> >
> > In this case the bug is that you cannot suspend without the risk of losing
> > wakeup events.  This is a real bug that for your use case is a serious
> > bug.  I've toyed with several ways of fixing this but the one that seems most
> > promising is to note that in the kernel the suspend process is two-stage with
> > a 'prepare' followed by a 'suspend'. Userspace cannot make that distinction
> > and so ends up with a race.
> >
> > Maybe userspace should be able to say "prepare to suspend" with the meaning
> > that after a successful return, any event which would cause a wakeup sets a
> > flag so that the final suspend returns immediately (without actually going to
> > the lower power state).
> >
> > Then your opportunistic suspend could be entirely in userspace where you
> > wouldn't have to fight with the kernel crowd :-)
> > The suspend-daemon would:
> >  Wait for all user-space suspend blocks to be dropped.
> >  Tell the kernel to "prepare to suspend".
> >  Tell all userspace programs which have registered for the message that they
> >    should prepare to suspend.  They have the opportunity at this point to
> >    take out a new suspend block if they notice an event that has
> >    just arrived
> >  Wait for all those programs to acknowledge
> >  If there are no new suspend blocks, tell the kernel to suspend
> >  else tell the kernel to abort the suspend.
> >
> > This (I think) allows race-free opportunistic suspend in user-space where
> > you can do all the accounting you need.
> >
> 
> Perhaps, but it forces all user space programs that get events from
> the kernel to also receive messages from the suspend-daemon, check for
> other events again, then respond to the suspend-daemon. The current
> suspend blocker interface is easier to use.

Maybe so.  There are quite possibly better ways to fix the bug.  There are a
variety of different tradeoffs possible and I suspect we could have fun
arguing about those.  My main point though is that if we focus on a bug that
needs to be fixed, we should be able to keep the conversation more focussed.
Currently it seems to be branching all over the place which doesn't seem very
helpful.

>                                              I don't see how your
> suggestion avoids races for events that pass trough several kernel
> layers though. If a wakeup event happened before the "prepare to
> suspend" call but has not yet been passed to user-space, the
> user-space program that needs this event will not know that it needs
> to block suspend when it gets the prepare-to-suspend message.

Each layer must understand that the event is a wake-up event, and the
"prepare to suspend call" to each layer should drive all pending events
through to the next layer.  If the layers (drivers) are prepared in the
"right" order, this will force the event all the way to user-space.  If they
are called in exactly the "Wrong" order, this will require multiple
prepare/suspend/instant-resume cycles to get the event through, but I suspect
that this would very rarely result in truly pathological behaviour.

I'm guessing your current code (sorry, I haven't looked at all) already tracks
events up through multiple layers so they can interact with a pending
opporunistic-suspend request.  I suspect that aspect of the code isn't
particularly controversial.
Keep that, and use it precisely to implement a race-free "suspend" request
from user-space.

> 
> > I don't fully understand your requirements for accounting of devices drivers
> > rejecting or blocking a suspend, so I cannot say precisely how that would fit
> > in.  Maybe you just need to know - whenever the 'suspend request' completes -
> > what the wakeup events were.  It shouldn't be too hard to export that to
> > user-space via sysfs.
> >
> > I won't propose an exact enhancement to the user-space interface for
> > requesting a suspend, but I suspect it should expose each of
> >  suspend_prepare
> >  suspend_devices_and_enter
> >  suspend_finish
> > (or close analogues there-of) to user-space.  It is tempting to map those to
> > "open-for-write", "write", "close", but I'm not sure that suspend_prepare
> > would be appropriate if the app was about to write "disk" - it is a pity that
> > both suspend and hibernate use the same sysfs file.
> >
> > So just fix the bug, and everyone will be happy :-)
> >
> 
> I already have, but everyone do not appear to be happy.
> 

I don't think you have.  You have proposed a significant new feature: a
suspend-as-soon-as-you-can request which *user*space*can*block*.  People
don't like that because it seems like a poor second cousin to something that
would be really useful (user-space setting more general latency
requirements).

I am suggesting that you stick with the feature we have, which is that
user-space can request a suspend and the kernel/hardware can cause a
subsequent (possibly immediate) resume in response to a wake-up event.
Argue that the current definition of "wake-up event" is too weak and does not
allow the feature to be used safely.  Present an implementation (I suspect you
have most of it already) where a wake-up event is tracked all the way from
the hardware to user-space and is still a 'wake-up event' until userspace
actually consumes it.

Then user-space simply has to:
  poll for event to be ready
  request suspend-block in user-space
  consume event
  handle event
  release suspend-block
  loop

No change to API.  No new concepts.  Simply a bug and a direct fix.

You probably want the "suspend" request to block until there are no pending
events (and then immediately fail if there were) so that there is no risk of
the suspend-daemon spinning asking of a suspend which appears to resume
immediately (until some other process sees an event and blocks suspend.

NeilBrown
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: Just fix the bug - Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Arve Hjønnevåg]

2010/5/27 Neil Brown <neilb@suse.de>:
> On Thu, 27 May 2010 21:57:02 -0700
> Arve Hjønnevåg <arve@android.com> wrote:
>
>> 2010/5/27 Neil Brown <neilb@suse.de>:
>> > On Thu, 27 May 2010 02:07:21 -0700
>> > Arve Hjønnevåg <arve@android.com> wrote:
>> >
>> >> >
>> >> >> of it into pm QoS stuff and if one day someone solves the rogue app
>> >> >> problem, we can migrate over.
>> >> >
>> >> > If it's so important for Android and no one else, Android can carry it
>> >> > out of tree.
>> >> >
>> >>
>> >> This is not only important for Android. If you use suspend on a
>> >> current Linux system you run the risk of loosing wakeup events. If you
>> >> have wakeup events that you cannot afford to lose your only option is
>> >> to never suspend. On some hardware (e.g. x86) the cost of not
>> >> suspending is always huge, on other hardware (many ARM SOCs) the cost
>> >> is only huge if your apps behave poorly.
>> >>
>> >
>> > So here is my suggestion.
>> > Rather than trying to push a feature that is clearly meeting lots of
>> > resistance, the Android devs should state the problem as a bug that needs
>> > fixing.  As you have.
>> > Upstream is a lot more receptive of fixing bugs than adding features.
>> >
>> > In this case the bug is that you cannot suspend without the risk of losing
>> > wakeup events.  This is a real bug that for your use case is a serious
>> > bug.  I've toyed with several ways of fixing this but the one that seems most
>> > promising is to note that in the kernel the suspend process is two-stage with
>> > a 'prepare' followed by a 'suspend'. Userspace cannot make that distinction
>> > and so ends up with a race.
>> >
>> > Maybe userspace should be able to say "prepare to suspend" with the meaning
>> > that after a successful return, any event which would cause a wakeup sets a
>> > flag so that the final suspend returns immediately (without actually going to
>> > the lower power state).
>> >
>> > Then your opportunistic suspend could be entirely in userspace where you
>> > wouldn't have to fight with the kernel crowd :-)
>> > The suspend-daemon would:
>> >  Wait for all user-space suspend blocks to be dropped.
>> >  Tell the kernel to "prepare to suspend".
>> >  Tell all userspace programs which have registered for the message that they
>> >    should prepare to suspend.  They have the opportunity at this point to
>> >    take out a new suspend block if they notice an event that has
>> >    just arrived
>> >  Wait for all those programs to acknowledge
>> >  If there are no new suspend blocks, tell the kernel to suspend
>> >  else tell the kernel to abort the suspend.
>> >
>> > This (I think) allows race-free opportunistic suspend in user-space where
>> > you can do all the accounting you need.
>> >
>>
>> Perhaps, but it forces all user space programs that get events from
>> the kernel to also receive messages from the suspend-daemon, check for
>> other events again, then respond to the suspend-daemon. The current
>> suspend blocker interface is easier to use.
>
> Maybe so.  There are quite possibly better ways to fix the bug.  There are a
> variety of different tradeoffs possible and I suspect we could have fun
> arguing about those.  My main point though is that if we focus on a bug that
> needs to be fixed, we should be able to keep the conversation more focussed.
> Currently it seems to be branching all over the place which doesn't seem very
> helpful.
>
>>                                              I don't see how your
>> suggestion avoids races for events that pass trough several kernel
>> layers though. If a wakeup event happened before the "prepare to
>> suspend" call but has not yet been passed to user-space, the
>> user-space program that needs this event will not know that it needs
>> to block suspend when it gets the prepare-to-suspend message.
>
> Each layer must understand that the event is a wake-up event, and the
> "prepare to suspend call" to each layer should drive all pending events
> through to the next layer.  If the layers (drivers) are prepared in the
> "right" order, this will force the event all the way to user-space.  If they
> are called in exactly the "Wrong" order, this will require multiple
> prepare/suspend/instant-resume cycles to get the event through, but I suspect
> that this would very rarely result in truly pathological behaviour.
>
> I'm guessing your current code (sorry, I haven't looked at all) already tracks
> events up through multiple layers so they can interact with a pending
> opporunistic-suspend request.  I suspect that aspect of the code isn't
> particularly controversial.
> Keep that, and use it precisely to implement a race-free "suspend" request
> from user-space.
>
>>
>> > I don't fully understand your requirements for accounting of devices drivers
>> > rejecting or blocking a suspend, so I cannot say precisely how that would fit
>> > in.  Maybe you just need to know - whenever the 'suspend request' completes -
>> > what the wakeup events were.  It shouldn't be too hard to export that to
>> > user-space via sysfs.
>> >
>> > I won't propose an exact enhancement to the user-space interface for
>> > requesting a suspend, but I suspect it should expose each of
>> >  suspend_prepare
>> >  suspend_devices_and_enter
>> >  suspend_finish
>> > (or close analogues there-of) to user-space.  It is tempting to map those to
>> > "open-for-write", "write", "close", but I'm not sure that suspend_prepare
>> > would be appropriate if the app was about to write "disk" - it is a pity that
>> > both suspend and hibernate use the same sysfs file.
>> >
>> > So just fix the bug, and everyone will be happy :-)
>> >
>>
>> I already have, but everyone do not appear to be happy.
>>
>
> I don't think you have.  You have proposed a significant new feature: a
> suspend-as-soon-as-you-can request which *user*space*can*block*.  People
> don't like that because it seems like a poor second cousin to something that
> would be really useful (user-space setting more general latency
> requirements).
>

The ability to block the suspend request is necessary to avoid races
with wakeup events.

> I am suggesting that you stick with the feature we have, which is that
> user-space can request a suspend and the kernel/hardware can cause a
> subsequent (possibly immediate) resume in response to a wake-up event.

Since not all wakeup events propagate to user-space it is very useful
to allow the kernel to reenter suspend when there are no active
suspend blockers remaining.

> Argue that the current definition of "wake-up event" is too weak and does not
> allow the feature to be used safely.  Present an implementation (I suspect you
> have most of it already) where a wake-up event is tracked all the way from
> the hardware to user-space and is still a 'wake-up event' until userspace
> actually consumes it.
>
> Then user-space simply has to:
>  poll for event to be ready
>  request suspend-block in user-space
>  consume event
>  handle event
>  release suspend-block
>  loop

This you copy this from the example in the documentation file added by
this patch? This is exactly what we do for input events, but for this
to work the kernel also has to block suspend while an event is ready
but not consumed.

>
> No change to API.  No new concepts.  Simply a bug and a direct fix.
>
I don't know how you got to that conclusion.

> You probably want the "suspend" request to block until there are no pending
> events (and then immediately fail if there were) so that there is no risk of
> the suspend-daemon spinning asking of a suspend which appears to resume
> immediately (until some other process sees an event and blocks suspend.
>
> NeilBrown
>

-- 
Arve Hjønnevåg

Re: Just fix the bug - Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Pavel Machek]

Hi!

> > I don't fully understand your requirements for accounting of devices drivers
> > rejecting or blocking a suspend, so I cannot say precisely how that would fit
> > in.  Maybe you just need to know - whenever the 'suspend request' completes -
> > what the wakeup events were.  It shouldn't be too hard to export that to
> > user-space via sysfs.
> >
> > I won't propose an exact enhancement to the user-space interface for
> > requesting a suspend, but I suspect it should expose each of
> >  suspend_prepare
> >  suspend_devices_and_enter
> >  suspend_finish
> > (or close analogues there-of) to user-space.  It is tempting to map those to
> > "open-for-write", "write", "close", but I'm not sure that suspend_prepare
> > would be appropriate if the app was about to write "disk" - it is a pity that
> > both suspend and hibernate use the same sysfs file.
> >
> > So just fix the bug, and everyone will be happy :-)
> >
> 
> I already have, but everyone do not appear to be happy.

Half of the code you submitted was infrastructure for debugging
userspace. Sorry, that does not count as a bugfix.

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

Re: Just fix the bug - Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Florian Mickler]

On Fri, 28 May 2010 15:35:16 +0200
Pavel Machek <pavel@ucw.cz> wrote:

> Hi!
> 
> > > I don't fully understand your requirements for accounting of devices drivers
> > > rejecting or blocking a suspend, so I cannot say precisely how that would fit
> > > in.  Maybe you just need to know - whenever the 'suspend request' completes -
> > > what the wakeup events were.  It shouldn't be too hard to export that to
> > > user-space via sysfs.
> > >
> > > I won't propose an exact enhancement to the user-space interface for
> > > requesting a suspend, but I suspect it should expose each of
> > >  suspend_prepare
> > >  suspend_devices_and_enter
> > >  suspend_finish
> > > (or close analogues there-of) to user-space.  It is tempting to map those to
> > > "open-for-write", "write", "close", but I'm not sure that suspend_prepare
> > > would be appropriate if the app was about to write "disk" - it is a pity that
> > > both suspend and hibernate use the same sysfs file.
> > >
> > > So just fix the bug, and everyone will be happy :-)
> > >
> > 
> > I already have, but everyone do not appear to be happy.
> 
> Half of the code you submitted was infrastructure for debugging
> userspace. Sorry, that does not count as a bugfix.
> 
That is not the issue here.

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[mark gross]

On Thu, May 27, 2010 at 05:23:54PM +1000, Neil Brown wrote:
> On Wed, 26 May 2010 14:20:51 +0100
> Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> 
> > On Wed, May 26, 2010 at 02:57:45PM +0200, Peter Zijlstra wrote:
> > 
> > > I fail to see why. In both cases the woken userspace will contact a
> > > central governing task, either the kernel or the userspace suspend
> > > manager, and inform it there is work to be done, and please don't
> > > suspend now.
> > 
> > Thinking about this, you're right - we don't have to wait, but that does 
> > result in another problem. Imagine we get two wakeup events 
> > approximately simultaneously. In the kernel-level universe the kernel 
> > knows when both have been handled. In the user-level universe, we may 
> > have one task schedule, bump the count, handle the event, drop the count 
> > and then we attempt a suspend again because the second event handler 
> > hasn't had an opportunity to run yet. We'll then attempt a suspend and 
> > immediately bounce back up. That's kind of wasteful, although it'd be 
> > somewhat mitigated by checking that right at the top of suspend entry 
> > and returning -EAGAIN or similar.
> > 
> 
> (I'm coming a little late to this party, so excuse me if I say something that
> has already been covered however...)
> 
> The above triggers a sequence of thoughts which (When they settled down) look
> a bit like this.
> 
> At the hardware level, there is a thing that we could call a "suspend
> blocker".  It is an interrupt (presumably level-triggered) that causes the
> processor to come out of suspend, or not to go into it.
> 
> Maybe it makes sense to export a similar thing from the kernel to user-space.
> When any event happens that would wake the device (and drivers need to know
> about these already), it would present something to user-space to say that
> the event happened.
> 
> When user-space processes the event, it clears the event indicator.

we did I proposed making the suspend enabling a oneshot type of thing
and all sorts of weak arguments came spewing forth.  I honestly couldn't
tell if I was reading valid input or fanboy BS.

--mgross


> 
> When there are no more current event indicators, userspace is allowed to
> request a suspend.  Obviously this could fail as an event could happen at any
> moment, but the same is true when the kernel asks the device to suspend, an
> interrupt might happen immediately to stop it.  But in either case an event
> will be reported.  So when userspace requests a suspend and it fails, it
> will see events reported and so will wait for them to be handled.
> 
> I imagine a sysfs directory with files that appear when events are pending.
> We could have some separate mechanism for user-space processes to request
> that the suspend-daemon not suspend.  Then it suspends whenever there are no
> pending requests from user-space or from the kernel.
> 
> The advantage of this model of suspend-blockers is that it is a close
> analogue for something that already exists in hardware so it isn't really
> creating new concepts, just giving the Linux virtual-machine features that
> have proved themselves in physical machines.
> 
> The cost is that any wake-up event needs to not only be handled, but also
> explicitly acknowledged by clearing the relevant suspend-blocker (i.e.
> removing the file from sysfs, or whatever interface was ultimately chosen).
> I'm hoping that isn't a big cost.
> 
> NeilBrown
> _______________________________________________
> linux-pm mailing list
> linux-pm@lists.linux-foundation.org
> https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Arve Hjønnevåg]

On Fri, May 28, 2010 at 7:52 PM, mark gross <640e9920@gmail.com> wrote:
> On Thu, May 27, 2010 at 05:23:54PM +1000, Neil Brown wrote:
>> On Wed, 26 May 2010 14:20:51 +0100
>> Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>>
>> > On Wed, May 26, 2010 at 02:57:45PM +0200, Peter Zijlstra wrote:
>> >
>> > > I fail to see why. In both cases the woken userspace will contact a
>> > > central governing task, either the kernel or the userspace suspend
>> > > manager, and inform it there is work to be done, and please don't
>> > > suspend now.
>> >
>> > Thinking about this, you're right - we don't have to wait, but that does
>> > result in another problem. Imagine we get two wakeup events
>> > approximately simultaneously. In the kernel-level universe the kernel
>> > knows when both have been handled. In the user-level universe, we may
>> > have one task schedule, bump the count, handle the event, drop the count
>> > and then we attempt a suspend again because the second event handler
>> > hasn't had an opportunity to run yet. We'll then attempt a suspend and
>> > immediately bounce back up. That's kind of wasteful, although it'd be
>> > somewhat mitigated by checking that right at the top of suspend entry
>> > and returning -EAGAIN or similar.
>> >
>>
>> (I'm coming a little late to this party, so excuse me if I say something that
>> has already been covered however...)
>>
>> The above triggers a sequence of thoughts which (When they settled down) look
>> a bit like this.
>>
>> At the hardware level, there is a thing that we could call a "suspend
>> blocker".  It is an interrupt (presumably level-triggered) that causes the
>> processor to come out of suspend, or not to go into it.
>>
>> Maybe it makes sense to export a similar thing from the kernel to user-space.
>> When any event happens that would wake the device (and drivers need to know
>> about these already), it would present something to user-space to say that
>> the event happened.
>>
>> When user-space processes the event, it clears the event indicator.
>
> we did I proposed making the suspend enabling a oneshot type of thing
> and all sorts of weak arguments came spewing forth.  I honestly couldn't
> tell if I was reading valid input or fanboy BS.
>

Can you be more specific? If you are talking about only letting
drivers abort suspend, not block it, then the main argument against
that is that you are forcing user-space to poll until the driver stops
aborting suspend (which according to people arguing against us using
suspend would make the power-manager a "bad" process). Or are you
talking about blocking the request from user-space until all other
suspend-blockers have been released and then doing a single suspend
cycle before returning. This would not be as bad, but it would force
the user-space power manager to be multi-threaded since it now would
have way to cancel the request. Either way, what problem are you
trying to solve by making it a one-shot request?

-- 
Arve Hjønnevåg

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Neil Brown]

On Fri, 28 May 2010 21:04:53 -0700
Arve Hjønnevåg <arve@android.com> wrote:

> On Fri, May 28, 2010 at 7:52 PM, mark gross <640e9920@gmail.com> wrote:
> > On Thu, May 27, 2010 at 05:23:54PM +1000, Neil Brown wrote:
> >> On Wed, 26 May 2010 14:20:51 +0100
> >> Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> >>
> >> > On Wed, May 26, 2010 at 02:57:45PM +0200, Peter Zijlstra wrote:
> >> >
> >> > > I fail to see why. In both cases the woken userspace will contact a
> >> > > central governing task, either the kernel or the userspace suspend
> >> > > manager, and inform it there is work to be done, and please don't
> >> > > suspend now.
> >> >
> >> > Thinking about this, you're right - we don't have to wait, but that does
> >> > result in another problem. Imagine we get two wakeup events
> >> > approximately simultaneously. In the kernel-level universe the kernel
> >> > knows when both have been handled. In the user-level universe, we may
> >> > have one task schedule, bump the count, handle the event, drop the count
> >> > and then we attempt a suspend again because the second event handler
> >> > hasn't had an opportunity to run yet. We'll then attempt a suspend and
> >> > immediately bounce back up. That's kind of wasteful, although it'd be
> >> > somewhat mitigated by checking that right at the top of suspend entry
> >> > and returning -EAGAIN or similar.
> >> >
> >>
> >> (I'm coming a little late to this party, so excuse me if I say something that
> >> has already been covered however...)
> >>
> >> The above triggers a sequence of thoughts which (When they settled down) look
> >> a bit like this.
> >>
> >> At the hardware level, there is a thing that we could call a "suspend
> >> blocker".  It is an interrupt (presumably level-triggered) that causes the
> >> processor to come out of suspend, or not to go into it.
> >>
> >> Maybe it makes sense to export a similar thing from the kernel to user-space.
> >> When any event happens that would wake the device (and drivers need to know
> >> about these already), it would present something to user-space to say that
> >> the event happened.
> >>
> >> When user-space processes the event, it clears the event indicator.
> >
> > we did I proposed making the suspend enabling a oneshot type of thing
> > and all sorts of weak arguments came spewing forth.  I honestly couldn't
> > tell if I was reading valid input or fanboy BS.
> >
> 
> Can you be more specific? If you are talking about only letting
> drivers abort suspend, not block it, then the main argument against
> that is that you are forcing user-space to poll until the driver stops
> aborting suspend (which according to people arguing against us using
> suspend would make the power-manager a "bad" process). Or are you
> talking about blocking the request from user-space until all other
> suspend-blockers have been released and then doing a single suspend
> cycle before returning. This would not be as bad, but it would force
> the user-space power manager to be multi-threaded since it now would
> have way to cancel the request. Either way, what problem are you
> trying to solve by making it a one-shot request?
> 

I don't know exactly what Mark has in mind, but I would advocate 1-shot
simply because what we currently have (echo mem > /sys/power/state) is
1-shot and I don't believe you need to do more than fix the bugs in that.

Your question of whether to abort or block suspend in central I think - the
answer to that question will make or break a possible solution. 

Simply aborting the suspend cannot work as you rightly say - the suspend
daemon would then spin until other user-space processes get into action.
Simply blocking while there are any unhandled 'wakeup events' - then aborting
if there were any - is how I think it should work.  However as it
doesn't work that way now I don't think it is safe to make it work that way
unconditionally.  If we did we could find that existing configurations always
block suspend indefinitely with would clearly be a regression.

I think we still need some sort of "suspend_prepare".  This would have two
particular effects.
1/ it sets the start time for interpreting the word "were" above.  i.e. the
  suspend would abort of there were any unhandled wakeup events since the
  "suspend_prepare" was issued.
2/ It would allow unhandled wakeup events to abort the suspend.  If no
  suspend_prepare had been issued, then only "new" wakeup events would
  be allowed to abort the suspend (i.e. the old racy version of suspend).

So the suspend daemon does:

   wait for there to be no user-space suspend blocks
   issue suspend_prepare
   check there are still no suspend blocks
   if there are, loop (possibly issue suspend_abort if needed)
   issue suspend request
   loop

processes that handle wakeup events would

   poll for event to be available
   request suspend-block
   consume event
   release suspend-block
   loop

(where consuming the event would quite possibly cause some other
suspend-block to become active - e.g. it might request that the display
be unlocked which would block suspends for a time).

NeilBrown
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Rafael J. Wysocki]

On Sunday 30 May 2010, Neil Brown wrote:
> On Fri, 28 May 2010 21:04:53 -0700
> Arve Hjønnevåg <arve@android.com> wrote:
> 
> > On Fri, May 28, 2010 at 7:52 PM, mark gross <640e9920@gmail.com> wrote:
> > > On Thu, May 27, 2010 at 05:23:54PM +1000, Neil Brown wrote:
> > >> On Wed, 26 May 2010 14:20:51 +0100
> > >> Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > >>
> > >> > On Wed, May 26, 2010 at 02:57:45PM +0200, Peter Zijlstra wrote:
> > >> >
> > >> > > I fail to see why. In both cases the woken userspace will contact a
> > >> > > central governing task, either the kernel or the userspace suspend
> > >> > > manager, and inform it there is work to be done, and please don't
> > >> > > suspend now.
> > >> >
> > >> > Thinking about this, you're right - we don't have to wait, but that does
> > >> > result in another problem. Imagine we get two wakeup events
> > >> > approximately simultaneously. In the kernel-level universe the kernel
> > >> > knows when both have been handled. In the user-level universe, we may
> > >> > have one task schedule, bump the count, handle the event, drop the count
> > >> > and then we attempt a suspend again because the second event handler
> > >> > hasn't had an opportunity to run yet. We'll then attempt a suspend and
> > >> > immediately bounce back up. That's kind of wasteful, although it'd be
> > >> > somewhat mitigated by checking that right at the top of suspend entry
> > >> > and returning -EAGAIN or similar.
> > >> >
> > >>
> > >> (I'm coming a little late to this party, so excuse me if I say something that
> > >> has already been covered however...)
> > >>
> > >> The above triggers a sequence of thoughts which (When they settled down) look
> > >> a bit like this.
> > >>
> > >> At the hardware level, there is a thing that we could call a "suspend
> > >> blocker".  It is an interrupt (presumably level-triggered) that causes the
> > >> processor to come out of suspend, or not to go into it.
> > >>
> > >> Maybe it makes sense to export a similar thing from the kernel to user-space.
> > >> When any event happens that would wake the device (and drivers need to know
> > >> about these already), it would present something to user-space to say that
> > >> the event happened.
> > >>
> > >> When user-space processes the event, it clears the event indicator.
> > >
> > > we did I proposed making the suspend enabling a oneshot type of thing
> > > and all sorts of weak arguments came spewing forth.  I honestly couldn't
> > > tell if I was reading valid input or fanboy BS.
> > >
> > 
> > Can you be more specific? If you are talking about only letting
> > drivers abort suspend, not block it, then the main argument against
> > that is that you are forcing user-space to poll until the driver stops
> > aborting suspend (which according to people arguing against us using
> > suspend would make the power-manager a "bad" process). Or are you
> > talking about blocking the request from user-space until all other
> > suspend-blockers have been released and then doing a single suspend
> > cycle before returning. This would not be as bad, but it would force
> > the user-space power manager to be multi-threaded since it now would
> > have way to cancel the request. Either way, what problem are you
> > trying to solve by making it a one-shot request?
> > 
> 
> I don't know exactly what Mark has in mind, but I would advocate 1-shot
> simply because what we currently have (echo mem > /sys/power/state) is
> 1-shot and I don't believe you need to do more than fix the bugs in that.
> 
> Your question of whether to abort or block suspend in central I think - the
> answer to that question will make or break a possible solution. 
> 
> Simply aborting the suspend cannot work as you rightly say - the suspend
> daemon would then spin until other user-space processes get into action.
> Simply blocking while there are any unhandled 'wakeup events' - then aborting
> if there were any - is how I think it should work.  However as it
> doesn't work that way now I don't think it is safe to make it work that way
> unconditionally.  If we did we could find that existing configurations always
> block suspend indefinitely with would clearly be a regression.
> 
> I think we still need some sort of "suspend_prepare".  This would have two
> particular effects.
> 1/ it sets the start time for interpreting the word "were" above.  i.e. the
>   suspend would abort of there were any unhandled wakeup events since the
>   "suspend_prepare" was issued.
> 2/ It would allow unhandled wakeup events to abort the suspend.  If no
>   suspend_prepare had been issued, then only "new" wakeup events would
>   be allowed to abort the suspend (i.e. the old racy version of suspend).
> 
> So the suspend daemon does:
> 
>    wait for there to be no user-space suspend blocks
>    issue suspend_prepare
>    check there are still no suspend blocks
>    if there are, loop (possibly issue suspend_abort if needed)
>    issue suspend request
>    loop
> 
> processes that handle wakeup events would
> 
>    poll for event to be available
>    request suspend-block
>    consume event
>    release suspend-block
>    loop
> 
> (where consuming the event would quite possibly cause some other
> suspend-block to become active - e.g. it might request that the display
> be unlocked which would block suspends for a time).

Well, please have a look at the Alan Stern's proposal here:
http://lkml.org/lkml/2010/5/29/77

Rafael
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[mark gross]

On Sun, May 30, 2010 at 06:08:46PM +1000, Neil Brown wrote:
> On Fri, 28 May 2010 21:04:53 -0700
> Arve Hjønnevåg <arve@android.com> wrote:
> 
> > On Fri, May 28, 2010 at 7:52 PM, mark gross <640e9920@gmail.com> wrote:
> > > On Thu, May 27, 2010 at 05:23:54PM +1000, Neil Brown wrote:
> > >> On Wed, 26 May 2010 14:20:51 +0100
> > >> Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > >>
> > >> > On Wed, May 26, 2010 at 02:57:45PM +0200, Peter Zijlstra wrote:
> > >> >
> > >> > > I fail to see why. In both cases the woken userspace will contact a
> > >> > > central governing task, either the kernel or the userspace suspend
> > >> > > manager, and inform it there is work to be done, and please don't
> > >> > > suspend now.
> > >> >
> > >> > Thinking about this, you're right - we don't have to wait, but that does
> > >> > result in another problem. Imagine we get two wakeup events
> > >> > approximately simultaneously. In the kernel-level universe the kernel
> > >> > knows when both have been handled. In the user-level universe, we may
> > >> > have one task schedule, bump the count, handle the event, drop the count
> > >> > and then we attempt a suspend again because the second event handler
> > >> > hasn't had an opportunity to run yet. We'll then attempt a suspend and
> > >> > immediately bounce back up. That's kind of wasteful, although it'd be
> > >> > somewhat mitigated by checking that right at the top of suspend entry
> > >> > and returning -EAGAIN or similar.
> > >> >
> > >>
> > >> (I'm coming a little late to this party, so excuse me if I say something that
> > >> has already been covered however...)
> > >>
> > >> The above triggers a sequence of thoughts which (When they settled down) look
> > >> a bit like this.
> > >>
> > >> At the hardware level, there is a thing that we could call a "suspend
> > >> blocker".  It is an interrupt (presumably level-triggered) that causes the
> > >> processor to come out of suspend, or not to go into it.
> > >>
> > >> Maybe it makes sense to export a similar thing from the kernel to user-space.
> > >> When any event happens that would wake the device (and drivers need to know
> > >> about these already), it would present something to user-space to say that
> > >> the event happened.
> > >>
> > >> When user-space processes the event, it clears the event indicator.
> > >
> > > we did I proposed making the suspend enabling a oneshot type of thing
> > > and all sorts of weak arguments came spewing forth.  I honestly couldn't
> > > tell if I was reading valid input or fanboy BS.
> > >
> > 
> > Can you be more specific? If you are talking about only letting
> > drivers abort suspend, not block it, then the main argument against
> > that is that you are forcing user-space to poll until the driver stops
> > aborting suspend (which according to people arguing against us using
> > suspend would make the power-manager a "bad" process). Or are you
> > talking about blocking the request from user-space until all other
> > suspend-blockers have been released and then doing a single suspend
> > cycle before returning. This would not be as bad, but it would force
> > the user-space power manager to be multi-threaded since it now would
> > have way to cancel the request. Either way, what problem are you
> > trying to solve by making it a one-shot request?
> > 

Sorry about missing Avr's email, I've been fighting with getting my
email forwarding working right.

The problems I want to solve with the one-shot styled interface are:

1) of having to sprinkle suspend blocking sections from isr up to
usermode and get them right.

2) provide a platform / architecture independent framework supporting
other low power modes.

I've just posted a patch that expresses what I was trying to express.
Its hard to take design over email without tossing code back and forth.
It's my turn to toss some code.



> 
> Simply aborting the suspend cannot work as you rightly say - the suspend
> daemon would then spin until other user-space processes get into action.

Not if you have sensible event messaging to the user-space processes.
I've posted a patch that attempts to add some messaging.

> Simply blocking while there are any unhandled 'wakeup events' - then aborting
> if there were any - is how I think it should work.  However as it
> doesn't work that way now I don't think it is safe to make it work that way
> unconditionally.  If we did we could find that existing configurations always
> block suspend indefinitely with would clearly be a regression.
> 
> I think we still need some sort of "suspend_prepare".  This would have two
> particular effects.
> 1/ it sets the start time for interpreting the word "were" above.  i.e. the
>   suspend would abort of there were any unhandled wakeup events since the
>   "suspend_prepare" was issued.
> 2/ It would allow unhandled wakeup events to abort the suspend.  If no
>   suspend_prepare had been issued, then only "new" wakeup events would
>   be allowed to abort the suspend (i.e. the old racy version of suspend).
> 
> So the suspend daemon does:
> 
>    wait for there to be no user-space suspend blocks
>    issue suspend_prepare
>    check there are still no suspend blocks
>    if there are, loop (possibly issue suspend_abort if needed)
>    issue suspend request
>    loop
> 
> processes that handle wakeup events would
> 
>    poll for event to be available
>    request suspend-block
>    consume event
>    release suspend-block
>    loop

In my mind I'm thinking of something like:
	attempt low power entry (suspend)
	if blocked, get -EBUSY back
		select on block_file until block is released
		retry suspend
	else // entered low power and exited
		read wake_event
		do something with event; typically wait a bit and loop.

--mgross

> 
> (where consuming the event would quite possibly cause some other
> suspend-block to become active - e.g. it might request that the display
> be unlocked which would block suspends for a time).
> 
> NeilBrown

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Neil Brown]

On Sun, 30 May 2010 21:52:14 +0200
"Rafael J. Wysocki" <rjw@sisk.pl> wrote:

> On Sunday 30 May 2010, Neil Brown wrote:
> > On Fri, 28 May 2010 21:04:53 -0700
> > Arve Hjønnevåg <arve@android.com> wrote:
> > 
> > > On Fri, May 28, 2010 at 7:52 PM, mark gross <640e9920@gmail.com> wrote:
> > > > On Thu, May 27, 2010 at 05:23:54PM +1000, Neil Brown wrote:
> > > >> On Wed, 26 May 2010 14:20:51 +0100
> > > >> Matthew Garrett <mjg59@srcf.ucam.org> wrote:
> > > >>
> > > >> > On Wed, May 26, 2010 at 02:57:45PM +0200, Peter Zijlstra wrote:
> > > >> >
> > > >> > > I fail to see why. In both cases the woken userspace will contact a
> > > >> > > central governing task, either the kernel or the userspace suspend
> > > >> > > manager, and inform it there is work to be done, and please don't
> > > >> > > suspend now.
> > > >> >
> > > >> > Thinking about this, you're right - we don't have to wait, but that does
> > > >> > result in another problem. Imagine we get two wakeup events
> > > >> > approximately simultaneously. In the kernel-level universe the kernel
> > > >> > knows when both have been handled. In the user-level universe, we may
> > > >> > have one task schedule, bump the count, handle the event, drop the count
> > > >> > and then we attempt a suspend again because the second event handler
> > > >> > hasn't had an opportunity to run yet. We'll then attempt a suspend and
> > > >> > immediately bounce back up. That's kind of wasteful, although it'd be
> > > >> > somewhat mitigated by checking that right at the top of suspend entry
> > > >> > and returning -EAGAIN or similar.
> > > >> >
> > > >>
> > > >> (I'm coming a little late to this party, so excuse me if I say something that
> > > >> has already been covered however...)
> > > >>
> > > >> The above triggers a sequence of thoughts which (When they settled down) look
> > > >> a bit like this.
> > > >>
> > > >> At the hardware level, there is a thing that we could call a "suspend
> > > >> blocker".  It is an interrupt (presumably level-triggered) that causes the
> > > >> processor to come out of suspend, or not to go into it.
> > > >>
> > > >> Maybe it makes sense to export a similar thing from the kernel to user-space.
> > > >> When any event happens that would wake the device (and drivers need to know
> > > >> about these already), it would present something to user-space to say that
> > > >> the event happened.
> > > >>
> > > >> When user-space processes the event, it clears the event indicator.
> > > >
> > > > we did I proposed making the suspend enabling a oneshot type of thing
> > > > and all sorts of weak arguments came spewing forth.  I honestly couldn't
> > > > tell if I was reading valid input or fanboy BS.
> > > >
> > > 
> > > Can you be more specific? If you are talking about only letting
> > > drivers abort suspend, not block it, then the main argument against
> > > that is that you are forcing user-space to poll until the driver stops
> > > aborting suspend (which according to people arguing against us using
> > > suspend would make the power-manager a "bad" process). Or are you
> > > talking about blocking the request from user-space until all other
> > > suspend-blockers have been released and then doing a single suspend
> > > cycle before returning. This would not be as bad, but it would force
> > > the user-space power manager to be multi-threaded since it now would
> > > have way to cancel the request. Either way, what problem are you
> > > trying to solve by making it a one-shot request?
> > > 
> > 
> > I don't know exactly what Mark has in mind, but I would advocate 1-shot
> > simply because what we currently have (echo mem > /sys/power/state) is
> > 1-shot and I don't believe you need to do more than fix the bugs in that.
> > 
> > Your question of whether to abort or block suspend in central I think - the
> > answer to that question will make or break a possible solution. 
> > 
> > Simply aborting the suspend cannot work as you rightly say - the suspend
> > daemon would then spin until other user-space processes get into action.
> > Simply blocking while there are any unhandled 'wakeup events' - then aborting
> > if there were any - is how I think it should work.  However as it
> > doesn't work that way now I don't think it is safe to make it work that way
> > unconditionally.  If we did we could find that existing configurations always
> > block suspend indefinitely with would clearly be a regression.
> > 
> > I think we still need some sort of "suspend_prepare".  This would have two
> > particular effects.
> > 1/ it sets the start time for interpreting the word "were" above.  i.e. the
> >   suspend would abort of there were any unhandled wakeup events since the
> >   "suspend_prepare" was issued.
> > 2/ It would allow unhandled wakeup events to abort the suspend.  If no
> >   suspend_prepare had been issued, then only "new" wakeup events would
> >   be allowed to abort the suspend (i.e. the old racy version of suspend).
> > 
> > So the suspend daemon does:
> > 
> >    wait for there to be no user-space suspend blocks
> >    issue suspend_prepare
> >    check there are still no suspend blocks
> >    if there are, loop (possibly issue suspend_abort if needed)
> >    issue suspend request
> >    loop
> > 
> > processes that handle wakeup events would
> > 
> >    poll for event to be available
> >    request suspend-block
> >    consume event
> >    release suspend-block
> >    loop
> > 
> > (where consuming the event would quite possibly cause some other
> > suspend-block to become active - e.g. it might request that the display
> > be unlocked which would block suspends for a time).
> 
> Well, please have a look at the Alan Stern's proposal here:
> http://lkml.org/lkml/2010/5/29/77
>

Thanks for the reference.
Some of the details are different, but the idea seems almost exactly the same
as mine.
The apparent dependence on signals makes me feel a little uncomfortable
(interfaces that depend on using signals seem to be easy to use wrongly), but
I don't think that is a serious flaw.

Maybe the biggest difference is philosophical.  Alan's proposal appears to be
adding a new feature.  Mine presents as trying to fix an existing feature so
that it can be used reliably.
It is easier to argue against a feature than against a bug-fix (??).

Thanks,
NeilBrown
_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/linux-pm

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Neil Brown]

On Mon, 31 May 2010 09:03:29 +1000
Neil Brown <neilb@suse.de> wrote:

> > Well, please have a look at the Alan Stern's proposal here:
> > http://lkml.org/lkml/2010/5/29/77
> >
> 
> Thanks for the reference.
> Some of the details are different, but the idea seems almost exactly the same
> as mine.
> The apparent dependence on signals makes me feel a little uncomfortable
> (interfaces that depend on using signals seem to be easy to use wrongly), but
> I don't think that is a serious flaw.
> 
> Maybe the biggest difference is philosophical.  Alan's proposal appears to be
> adding a new feature.  Mine presents as trying to fix an existing feature so
> that it can be used reliably.
> It is easier to argue against a feature than against a bug-fix (??).

Addendum...

The use of signals by Alan here got me thinking.. and combining it which what 
Alan Cox and Thomas Gleixner (and others?) have been saying about detecting
idleness led me down an interesting path that I thought I would share.

An absolutely minimal change to allow a user-space initiated auto-suspend to
be race-free would be achieved by placing an interruptible wait between
       pm_notifier_call_chain(PM_SUSPEND_PREPARE);
and 
       suspend_freeze_processes();

which waits for all kernel threads to have passed through an 'idle' state.

Then any wake event that happened after the PM_SUSPEND_PREPARE would
presumably either abort the suspend later, or would cause an immediate
wake-up.
Any event that happened before the PM_SUSPEND_PREPARE would have been fully
processed by any kernel thread and so would be queued for user-space (or have
been discarded).

If we then require any process that handles wakeup events to use the
equivalent of fcntl(F_SETOWN) to direct a SIGIO (or other signal) at the
process which requests the suspend, then wakeup events won't be lost.
They will either send a signal to the suspend-daemon in time to abort the
suspend, or will be late enough to either abort the suspend in-kernel, or
trigger an immediate wakeup.

Yes: this would require quite a large change of the android code, and the
F_SETOWN usage is fairly ugly, but balanced against it actually working and
it being minimal, that might be OK.

The hardest part would be waiting for all kernel-threads to pass through idle
(or all threads to pass through idle or user-space).  We'd need help to make
that work, but at least it would be a fairly localised change.

Maybe it is something to keep in mind when other possibilities have proven
intractable.

NeilBrown

Re: [PATCH 1/8] PM: Opportunistic suspend support.

[Arve Hjønnevåg]

On Sun, May 30, 2010 at 1:32 PM, mark gross <640e9920@gmail.com> wrote:
> On Sun, May 30, 2010 at 06:08:46PM +1000, Neil Brown wrote:
>> On Fri, 28 May 2010 21:04:53 -0700
>> Arve Hjønnevåg <arve@android.com> wrote:
>>
>> > On Fri, May 28, 2010 at 7:52 PM, mark gross <640e9920@gmail.com> wrote:
>> > > On Thu, May 27, 2010 at 05:23:54PM +1000, Neil Brown wrote:
>> > >> On Wed, 26 May 2010 14:20:51 +0100
>> > >> Matthew Garrett <mjg59@srcf.ucam.org> wrote:
>> > >>
>> > >> > On Wed, May 26, 2010 at 02:57:45PM +0200, Peter Zijlstra wrote:
>> > >> >
>> > >> > > I fail to see why. In both cases the woken userspace will contact a
>> > >> > > central governing task, either the kernel or the userspace suspend
>> > >> > > manager, and inform it there is work to be done, and please don't
>> > >> > > suspend now.
>> > >> >
>> > >> > Thinking about this, you're right - we don't have to wait, but that does
>> > >> > result in another problem. Imagine we get two wakeup events
>> > >> > approximately simultaneously. In the kernel-level universe the kernel
>> > >> > knows when both have been handled. In the user-level universe, we may
>> > >> > have one task schedule, bump the count, handle the event, drop the count
>> > >> > and then we attempt a suspend again because the second event handler
>> > >> > hasn't had an opportunity to run yet. We'll then attempt a suspend and
>> > >> > immediately bounce back up. That's kind of wasteful, although it'd be
>> > >> > somewhat mitigated by checking that right at the top of suspend entry
>> > >> > and returning -EAGAIN or similar.
>> > >> >
>> > >>
>> > >> (I'm coming a little late to this party, so excuse me if I say something that
>> > >> has already been covered however...)
>> > >>
>> > >> The above triggers a sequence of thoughts which (When they settled down) look
>> > >> a bit like this.
>> > >>
>> > >> At the hardware level, there is a thing that we could call a "suspend
>> > >> blocker".  It is an interrupt (presumably level-triggered) that causes the
>> > >> processor to come out of suspend, or not to go into it.
>> > >>
>> > >> Maybe it makes sense to export a similar thing from the kernel to user-space.
>> > >> When any event happens that would wake the device (and drivers need to know
>> > >> about these already), it would present something to user-space to say that
>> > >> the event happened.
>> > >>
>> > >> When user-space processes the event, it clears the event indicator.
>> > >
>> > > we did I proposed making the suspend enabling a oneshot type of thing
>> > > and all sorts of weak arguments came spewing forth.  I honestly couldn't
>> > > tell if I was reading valid input or fanboy BS.
>> > >
>> >
>> > Can you be more specific? If you are talking about only letting
>> > drivers abort suspend, not block it, then the main argument against
>> > that is that you are forcing user-space to poll until the driver stops
>> > aborting suspend (which according to people arguing against us using
>> > suspend would make the power-manager a "bad" process). Or are you
>> > talking about blocking the request from user-space until all other
>> > suspend-blockers have been released and then doing a single suspend
>> > cycle before returning. This would not be as bad, but it would force
>> > the user-space power manager to be multi-threaded since it now would
>> > have way to cancel the request. Either way, what problem are you
>> > trying to solve by making it a one-shot request?
>> >
>
> Sorry about missing Avr's email, I've been fighting with getting my
> email forwarding working right.
>
> The problems I want to solve with the one-shot styled interface are:
>
> 1) of having to sprinkle suspend blocking sections from isr up to
> usermode and get them right.

Making the interface one-shot does not change where you need to block suspend.

>
> 2) provide a platform / architecture independent framework supporting
> other low power modes.
>

I assume you did not mean that a one-shot interface has any impact on
this. What other low power modes are you trying to support. Suspend
blocker block suspend regardless of the mode selected (standby/mem).
Are there cases where you need to block one but not the other?



-- 
Arve Hjønnevåg