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

["Rafael J. Wysocki" <rjw@sisk.pl>]

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.

I guess I can do that ...

Thanks,
Rafael