RE: Regression seen for patch "sched:dont decrease idle sleep avg"

Chen, Kenneth W
2006-05-18T23:34:43Z

Con Kolivas wrote on Wednesday, May 17, 2006 10:52 PM
> The relationship between INTERACTIVE_SLEEP and the ceiling is not perfect
> and not explicit enough. The sleep boost is not supposed to be any larger
> than without this code and the comment is not clear enough about what exactly
> it does, just the reason it does it.
>
> There is a ceiling to the priority beyond which tasks that only ever sleep
> for very long periods cannot surpass.
>
> Opportunity to micro-optimise and re-use the ceiling variable.
>
> --- linux-2.6.17-rc4-mm1.orig/kernel/sched.c 2006-05-17 15:57:49.000000000 +1000
> +++ linux-2.6.17-rc4-mm1/kernel/sched.c 2006-05-18 15:48:47.000000000 +1000
> @@ -925,12 +924,12 @@ static int recalc_task_prio(task_t *p, u
> * are likely to be waiting on I/O
> */
> if (p->sleep_type == SLEEP_NONINTERACTIVE && p->mm) {
> - if (p->sleep_avg >= INTERACTIVE_SLEEP(p))
> + if (p->sleep_avg >= ceiling)
> sleep_time = 0;
> else if (p->sleep_avg + sleep_time >=
> - INTERACTIVE_SLEEP(p)) {
> - p->sleep_avg = INTERACTIVE_SLEEP(p);
> - sleep_time = 0;
> + ceiling) {
> + p->sleep_avg = ceiling;
> + sleep_time = 0;

Watch for white space damage, last two lines has one extra tab on the indentation.

By the way, there is all kinds of non-linear behavior with priority boost
adjustment:

if (p->sleep_type == SLEEP_NONINTERACTIVE && p->mm) {
if (p->sleep_avg >= ceiling)
sleep_time = 0;
else if (p->sleep_avg + sleep_time >= ceiling) {
p->sleep_avg = ceiling;
sleep_time = 0;
}
}

For large p->sleep_avg, kernel don't clamp it to ceiling, yet clamp small
incremental sleep. This all seems very fragile.

- Ken