Re: [RFD] Automatic suspend

Arve Hjønnevåg
2009-02-27T22:56:30Z

On Fri, Feb 27, 2009 at 12:55 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> On Friday 27 February 2009, Pavel Machek wrote:
>> On Fri 2009-02-27 15:22:39, Rafael J. Wysocki wrote:
>> > On Friday 27 February 2009, Pavel Machek wrote:
>> > > Hi!
>> > >
>> > > > > > Then, the decision making logic will be able to use /sys/power/sleep whenever
>> > > > > > it wishes to and the kernel will be able to refuse to suspend if it's not
>> > > > > > desirable at the moment.
>> > > > > >
>> > > > > > It seems to be flexible enough to me.
>> > > > >
>> > > > > This seems flexible enough to avoid race conditions, but it forces the
>> > > > > user space power manager to poll when the kernel refuse suspend.
>> > > >
>> > > > And if the kernel is supposed to start automatic suspend, it has to monitor
>> > > > all of the wakelocks.  IMO, it's better to allow the power manager to poll the
>> > > > kernel if it refuses to suspend.
>> > >
>> > > polling is evil -- it keeps CPU wake up => wastes power.
>> > >
>> > > Wakelocks done right are single atomic_t... and if you set it to 0,
>> > > you just unblock "sleeper" thread or something. Zero polling and very
>> > > simple...
>> >
>> > Except that you have to check all of the wakelocks periodically in a loop =>
>> > polling.  So?
>>
>> No. I want to have single atomic_t for all the wakelocks... at least
>> in non-debug version. Debug version will be slower. I believe you
>> originally suggested that.
>
> I did, but please don't call it "wakelocks".  It's confusing.

What you are talking about here is mostly an optimization of the
wakelock api. You have removed timeout support and made each wakelock
reference counted. If you ignore wakelocks with timeouts, the current
wakelock interface can be implemented with a global atomic_t to
prevent suspend, and a per wakelock atomic_t to prevent a single
client from changing the global reference count by more than one.

There are a couple of reasons that I have not done this. It removes
the ability to easily inspect the system when it is not suspending. I
do provide an option to turn off the wakelock stats, which makes
wake_lock/unlock significantly faster, but we never run with wakelock
stats off. Also, it pushes timeout handling to the drivers. I know may
of you don't like timeout support, but ignoring the problem is not a
solution. If each driver that needs timeouts uses its own timer, then
you will often wakeup from idle just to unlock a wakelock that will
not trigger suspend. This wakeup is a thousand times as costly on the
msm platform as a wakelock/unlock pair (with wakelock stats enabled).

I just checked my phone, and over a 24 hour awake time (370 hours
uptime) period, it acquired about 5 million wakelocks (mostly for
input events). If these were cache hits, and took as long as my
benchmark did, that accounts for 20 seconds of overhead (0.023% of
awake, 0.1% of not-idle (5.5h).

--
Arve Hjønnevåg