Re: [PATCH 0/8] Suspend block api (version 7)

["Arve Hjønnevåg" <arve@android.com>]

2010/5/18 Rafael J. Wysocki <rjw@sisk.pl>:
> On Tuesday 18 May 2010, Arve Hjønnevåg wrote:
>> On Mon, May 17, 2010 at 2:44 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> > On Monday 17 May 2010, Brian Swetland wrote:
>> >> On Mon, May 17, 2010 at 1:40 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> > On Monday 17 May 2010, Arve Hjønnevåg wrote:
>> >> >>
>> >> >> It should get out of that loop as soon as someone blocks suspend. If
>> >> >> someone is constantly aborting suspend without using a suspend blocker
>> >> >> it will be very inefficient, but it should still work.
>> >> >
>> >> > Well, the scenario I have in mind is the following.  Someone wants to check
>> >> > the feature and simply writes "opportunistic" to /sys/power/policy and "mem" to
>> >> > /sys/power/state without any drivers or apps that use suspend blockers.
>> >> >
>> >> > How in that case is the system supposed to break out of the suspend-resume loop
>> >> > resulting from this?  I don't see right now, because the main blocker is
>> >> > inactive, there are no other blockers that can be activated and it is next to
>> >> > impossible to write to /sys/power/state again.
>> >>
>> >> I guess we could set a flag when a suspend blocker is registered and
>> >> refuse to enter opportunistic mode if no blockers have ever been
>> >> registered.
>> >>
>> >> It does seem like extra effort to go through to handle a "don't do
>> >> that" type scenario (entering into opportunistic suspend without
>> >> anything that will prevent it).
>> >
>> > I agree, but I think it's necessary.  We shouldn't add interfaces that hurt
>> > users if not used with care.
>> >
>>
>> I'm not sure this can be "fixed".
>
> Yes, it can, but perhaps a workaround would be sufficient (see below).
>
>> The user asked that the system to suspend whenever possible, which is what it
>> is doing. I don't think disabling opportunistic suspend if no suspend
>> blockers have been registered will work. As soon as we register a suspend
>> blocker we are back in the same situation.
>
> Not really, because the new suspend blocker is not added by the _framework_ _itself_.
>

I'm not sure what you mean by this. If we add a workaround that is
disabled when the first suspend blocker is registered, then we cannot
add any suspend blockers without disabling the workaround (for
instance the power supply framework patch later in this patch set).

> Now, to make it more "user-friendly", we can simply use
> queue_delayed_work() with a reasonable delay instead of queue_work() to queue
> the suspend work (the delay may be configurable via sysfs).
>

I can add a delay (and the timeout support code does add a delay as an
optimization) to the unknown wakeup case, but this does not fix the
problem of a user turning on opportunistic suspend with a user space
framework that does not use suspend blockers. If the kernel uses
suspend blockers to make sure the wakeup event makes it to user space,
but user space does not block suspend, then the system will suspend
before the event is processed.

-- 
Arve Hjønnevåg