Re: [PATCH 03/10] freezer: add new freezable helpers using freezer_do_not_count()

[Colin Cross]

On Thu, May 2, 2013 at 5:48 AM, Pavel Machek <pavel@xxxxxx> wrote:
> On Mon 2013-04-29 14:45:39, Colin Cross wrote:
>> Freezing tasks will wake up almost every userspace task from
>> where it is blocking and force it to run until it hits a
>> call to try_to_sleep(), generally on the exit path from the syscall
>> it is blocking in.  On resume each task will run again, usually
>> restarting the syscall and running until it hits the same
>> blocking call as it was originally blocked in.
>
> Ok, so you are optimizing suspend at the cost of runtime operations,
> right?
The runtime operations are negligible, it is setting a bit before
blocking, and then clearing the bit and checking a single global
counter as the fast-path after blocking.  Both will be lost in the
noise of the context switch that is happening.

> Would it make sense to do suspends entirely without freezer in your
> configurations? With the right drivers, it should work ok.

No, we need userspace tasks to freeze.  That is one of the main
reasons we go to suspend, to prevent recurring timers from firing.

> Actually, would it make sense to simply enter deep idle and do runtime
> powersaving in the drivers... n900 does that rather successfully and
> it is certainly cleaner design.

It depends on the SoC if that is even possible, and if we did do it we
would still use cgroup freezers to prevent apps from running and
eating the battery so this patch would still be applicable.

>> +/* Like schedule_timeout(), but should not block the freezer. */
>> +#define freezable_schedule_timeout(timeout)                          \
>> +({                                                                   \
>> +     long __retval;                                                  \
>> +     freezer_do_not_count();                                         \
>> +     __retval = schedule_timeout(timeout);                           \
>> +     freezer_count();                                                \
>> +     __retval;                                                       \
>> +})
>
> You plan to use this a lot during normal operation, right? Is it going
> to have some measureable impact?

It has a very measurable impact on suspend/resume operations (reducing
freeze time by a factor of 10, reducing context switches from 1000 to
25).  I haven't measured the impact on total energy used for a
suspend/resume cycle, but based on previous measurements I expect it
to be beneficial.

> Also, why not static inline?

I copied the style of the existing helpers.  I can change them all to
static inlines if you prefer.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/