Re: [PATCH 01/13] PM: Add wake lock api.

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

On Wed, Feb 11, 2009 at 2:23 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
>> >> >> It allows wakelocks with timeouts
>> >> >
>> >> > OK
>> >> >
>> >> > What for?
>> >>
>> >> So we do not have to change everything at once, and so we can allow
>> >> code that we do not really trust a chance to run.
>> >
>> > Well, are you sure this is the best way of achieving this goal?
>>
>> No, but I have not seen any better suggestions, that solve the problem.
>
> Wakelocks with timeout are not a solution IMO.

It is an improvement over the current situation where you have to use
timeouts for everything.

>> >> > However, it might be better to use a refcount along with some mechanism
>> >> > allowing user space processes to increase it only once before decreasing.
>> >> > That would require a per-task flag, but I think you can reuse one of the
>> >> > freezer flags for that (the freezer is not going to run as long as a wakelock
>> >> > is held, right?).
>> >>
>> >> How would this be better?
>> >
>> > Simpler code, less overhead.  But I'm not insisting, just showing you another
>> > possible approach.
>>
>> I agree that a global reference count is less overhead, but we need
>> timeout support the make the system work for now.
>
> I don't really think we need it in the mainline kernel.

We do if we want anyone to be able to ship an android device capable
of suspending with the mainline kernel.

>
>> I don't see what a per-task flag would be helpful with. Not all wakelocks are
>> associated with tasks.
>
> What are they associated with, then?

Data (usually queues). If by per task you mean per process and not per
thread, you could use use your flag to clean up when a process dies
though.

>> We report the name and how long each wakelock is active in
>> /proc/wakelocks. Without a name, those stats are not very useful. You
>> can also enable debug options to print a message the kernel log every
>> time a wakelock is locked, unlocked or used to trigger a wakeup.
>
> Well, you put quite a lot of effort into making this nicely debuggable and so
> on, but I think you should have submitted the minimal core functionality first
> to see if people were comfortable with it.

The code I submitted is usable and tested. Without wakelocks we cannot
use suspend, and without wakelock timeouts we cannot pass events to
components that do not use wakelocks.

-- 
Arve Hjønnevåg