Re: [RFC][PATCH 0/7] PM: Implement autosleep and "wake locks", take2

Srivatsa S. Bhat
2012-02-25T19:20:55Z

On 02/25/2012 04:51 AM, Rafael J. Wysocki wrote:

> On Friday, February 24, 2012, Srivatsa S. Bhat wrote:
>> On 02/24/2012 03:02 AM, Rafael J. Wysocki wrote:
>>
>>> On Thursday, February 23, 2012, Rafael J. Wysocki wrote:
>>>> On Thursday, February 23, 2012, Srivatsa S. Bhat wrote:
>>>>> On 02/23/2012 03:40 AM, Rafael J. Wysocki wrote:
> [...]
>>>>>
>>>>> By the way, I am just curious.. how difficult will this make it for userspace
>>>>> to disable autosleep? I mean, would a trylock mean that the user has to keep
>>>>> fighting until he finally gets a chance to disable autosleep?
>>>>
>>>> That's a good point, so I think it may be a good idea to do
>>>> mutex_lock_interruptible() in pm_autosleep_set_state() instead.
>>>
>>> Now that I think of it, perhaps it's a good idea to just make
>>> pm_autosleep_lock() do mutex_lock_interruptible() _and_ make
>>> pm_autosleep_set_state() use pm_autosleep_lock().
>>>
>>> What do you think?
>>>
>>
>>
>> Well, I don't think mutex_lock_interruptible() would help us much..
>> Consider what would happen, if we use it:
>>
>> * pm-suspend got initiated as part of autosleep. Acquired autosleep lock.
>> * Userspace is about to get frozen.
>> * Now, the user tries to write "off" to autosleep. And hence, he is waiting
>> for autosleep lock, interruptibly.
>> * The freezer sent a fake signal to all userspace processes and hence
>> this process also got interrupted.. it is no longer waiting on autosleep
>> lock - it got the signal and returned, and got frozen.
>> (And when the userspace gets thawed later, this process won't have the
>> autosleep lock - which is a different (but yet another) problem).
>>
>> So ultimately the only thing we achieved is to ensure that freezing of
>> userspace goes smoothly. But the user process could not succeed in
>> disabling autosleep. Of course we can work around that by having the
>> mutex_lock_interruptible() in a loop and so on, but that gets very
>> ugly pretty soon.
>>
>> So, I would suggest the following solution:
>>
>> We want to achieve 2 things here:
>> a. A user process trying to write to /sys/power/state or
>> /sys/power/autosleep should not cause freezing failures.
>> b. When a user process writes "off" to autosleep, the suspend/hibernate
>> attempt that is on-going, if any, must be immediately aborted, to give
>> the user the feeling that his preference has been noticed and respected.
>>
>> And to achieve this, we note that a user process can write "off" to autosleep
>> only until the userspace gets frozen. No chance after that.
>>
>> So, let's do this:
>> 1. Drop the autosleep lock before entering pm-suspend/hibernate.
>> 2. This means, a user process can get hold of this lock and successfully
>> disable autosleep a moment after we initiated suspend, but before userspace
>> got frozen fully.
>> 3. So, to respect the user's wish, we add a check immediately after the
>> freezing of userspace is complete - we check if the user disabled autosleep
>> and bail out, if he did. Otherwise, we continue and suspend the machine.
>>
>> IOW, this is like hitting 2 birds with one stone ;-)
>> We don't hold autosleep lock throughout suspend/hibernate, but still react
>> instantly when the user disables autosleep. And of course, freezing of tasks
>> won't fail, ever! :-)
>
> Well, you essentially are postulating to restore the "interface" wakeup source
> that was present in the previous version of this patch and that I dropped in
> order to simplify the code.
>

Oh is it? I guess I haven't followed this thread very closely...

> I guess I can do that ...
>

Oh by the way, this scheme doesn't solve all problems. It might be effective
in reacting "instantly" to a request by the user to *switch off* autosleep.
But say, when the user wants to switch to suspend instead of hibernate as the
autosleep preference, for example, I don't think it would be as quick in
responding... (I mean, it might do the old operation one more time before
switching to the new one..)

But I guess at this point it might be wiser to say "sigh.. we can do only so
much..." instead of complicating the code too much in an attempt to meet
everybody's expectations :-)

Regards,
Srivatsa S. Bhat