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

["Chen, Kenneth W" <kenneth.w.chen@intel.com>]

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