[PATCH] sched: Improve readability in update_cpu_load() code

[PATCH] sched: Improve readability in update_cpu_load() code

[Gautham R Shenoy]

Author: Gautham R Shenoy <ego@in.ibm.com>
Date:   Thu May 15 17:55:49 2008 +0530

    sched: Improve readability in update_cpu_load() code
    
    Currently the cpu_load[i] is calculated as:
    	this_rq->cpu_load[i] = (old_load*(scale-1) + new_load) >> i;
    
    However, since scale = 2^i, this can be simplified as:
    	this_rq->cpu_load[i] = old_load + ((new_load - old_load) >> i);
    
    Makes it easier to read.
    Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>

diff --git a/kernel/sched.c b/kernel/sched.c
index 2d7d8f1..e1a6985 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -2921,7 +2921,7 @@ static void update_cpu_load(struct rq *this_rq)
 		 */
 		if (new_load > old_load)
 			new_load += scale-1;
-		this_rq->cpu_load[i] = (old_load*(scale-1) + new_load) >> i;
+		this_rq->cpu_load[i] = old_load + ((new_load - old_load) >> i);
 	}
 }
 
-- 
Thanks and Regards
gautham

Re: [PATCH] sched: Improve readability in update_cpu_load() code

[Srivatsa Vaddagiri]

On Thu, May 15, 2008 at 06:34:59PM +0530, Gautham R Shenoy wrote:
> Author: Gautham R Shenoy <ego@in.ibm.com>
> Date:   Thu May 15 17:55:49 2008 +0530
> 
>     sched: Improve readability in update_cpu_load() code
>     
>     Currently the cpu_load[i] is calculated as:
>     	this_rq->cpu_load[i] = (old_load*(scale-1) + new_load) >> i;
>     
>     However, since scale = 2^i, this can be simplified as:
>     	this_rq->cpu_load[i] = old_load + ((new_load - old_load) >> i);
>     
>     Makes it easier to read.
>     Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
> 
> diff --git a/kernel/sched.c b/kernel/sched.c
> index 2d7d8f1..e1a6985 100644
> --- a/kernel/sched.c
> +++ b/kernel/sched.c
> @@ -2921,7 +2921,7 @@ static void update_cpu_load(struct rq *this_rq)
>  		 */
>  		if (new_load > old_load)
>  			new_load += scale-1;
> -		this_rq->cpu_load[i] = (old_load*(scale-1) + new_load) >> i;
> +		this_rq->cpu_load[i] = old_load + ((new_load - old_load) >> i);

This wont work when new_load < old_load ..

For ex: I tried this prog:

#include <stdio.h>

main()
{
	unsigned long old_load = 100, new_load = 90, this_load, this_load1;
	int i = 1, scale = 2 << i;

	this_load = (old_load*(scale-1) + new_load) >> i;
	this_load1 = old_load + ((new_load - old_load) >> i);

	printf ("this_load = %u, this_load1 = %u \n", this_load, this_load1);
}

This is what I get:

$ ./a.out
this_load = 195, this_load1 = 2147483743
$

	

-- 
Regards,
vatsa

Re: [PATCH] sched: Improve readability in update_cpu_load() code

[Srivatsa Vaddagiri]

On Thu, May 15, 2008 at 08:22:15PM +0530, Srivatsa Vaddagiri wrote:
> This wont work when new_load < old_load ..
> 
> For ex: I tried this prog:
> 
> #include <stdio.h>
> 
> main()
> {
> 	unsigned long old_load = 100, new_load = 90, this_load, this_load1;
> 	int i = 1, scale = 2 << i;

s/scale = 2<< i/scale = 2 << (i-1)

> 
> 	this_load = (old_load*(scale-1) + new_load) >> i;
> 	this_load1 = old_load + ((new_load - old_load) >> i);
> 
> 	printf ("this_load = %u, this_load1 = %u \n", this_load, this_load1);
> }
> 
> This is what I get:
> 
> $ ./a.out
> this_load = 195, this_load1 = 2147483743
> $

[with above modification:]

this_load = 95, this_load1 = 2147483743


-- 
Regards,
vatsa

Re: [PATCH] sched: Improve readability in update_cpu_load() code

[Dmitry Adamushko]

2008/5/15 Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>:
> On Thu, May 15, 2008 at 06:34:59PM +0530, Gautham R Shenoy wrote:
>> Author: Gautham R Shenoy <ego@in.ibm.com>
>> Date:   Thu May 15 17:55:49 2008 +0530
>>
>>     sched: Improve readability in update_cpu_load() code
>>
>>     Currently the cpu_load[i] is calculated as:
>>       this_rq->cpu_load[i] = (old_load*(scale-1) + new_load) >> i;
>>
>>     However, since scale = 2^i, this can be simplified as:
>>       this_rq->cpu_load[i] = old_load + ((new_load - old_load) >> i);
>>
>>     Makes it easier to read.
>>     Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
>>
>> diff --git a/kernel/sched.c b/kernel/sched.c
>> index 2d7d8f1..e1a6985 100644
>> --- a/kernel/sched.c
>> +++ b/kernel/sched.c
>> @@ -2921,7 +2921,7 @@ static void update_cpu_load(struct rq *this_rq)
>>                */
>>               if (new_load > old_load)
>>                       new_load += scale-1;
>> -             this_rq->cpu_load[i] = (old_load*(scale-1) + new_load) >> i;
>> +             this_rq->cpu_load[i] = old_load + ((new_load - old_load) >> i);
>
> This wont work when new_load < old_load ..
>
> For ex: I tried this prog:
>
> #include <stdio.h>
>
> main()
> {
>        unsigned long old_load = 100, new_load = 90, this_load, this_load1;
>        int i = 1, scale = 2 << i;
>
>        this_load = (old_load*(scale-1) + new_load) >> i;
>        this_load1 = old_load + ((new_load - old_load) >> i);

it should be

this_load2 = 2 * old_load + (new_load >> i) - (old_load >> i);

scale == 2 << i == 1 << (i + 1), so scale >> i = 2.

and it may result in (probably) little difefrences due to different rounding.
I think, for me the currently existing version is more easy-to-read.


> --
> Regards,
> vatsa

-- 
Best regards,
Dmitry Adamushko

Re: [PATCH] sched: Improve readability in update_cpu_load() code

[Dmitry Adamushko]

2008/5/15 Dmitry Adamushko <dmitry.adamushko@gmail.com>:
> 2008/5/15 Srivatsa Vaddagiri <vatsa@linux.vnet.ibm.com>:
>> On Thu, May 15, 2008 at 06:34:59PM +0530, Gautham R Shenoy wrote:
>>> Author: Gautham R Shenoy <ego@in.ibm.com>
>>> Date:   Thu May 15 17:55:49 2008 +0530
>>>
>>>     sched: Improve readability in update_cpu_load() code
>>>
>>>     Currently the cpu_load[i] is calculated as:
>>>       this_rq->cpu_load[i] = (old_load*(scale-1) + new_load) >> i;
>>>
>>>     However, since scale = 2^i, this can be simplified as:
>>>       this_rq->cpu_load[i] = old_load + ((new_load - old_load) >> i);
>>>
>>>     Makes it easier to read.
>>>     Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
>>>
>>> diff --git a/kernel/sched.c b/kernel/sched.c
>>> index 2d7d8f1..e1a6985 100644
>>> --- a/kernel/sched.c
>>> +++ b/kernel/sched.c
>>> @@ -2921,7 +2921,7 @@ static void update_cpu_load(struct rq *this_rq)
>>>                */
>>>               if (new_load > old_load)
>>>                       new_load += scale-1;
>>> -             this_rq->cpu_load[i] = (old_load*(scale-1) + new_load) >> i;
>>> +             this_rq->cpu_load[i] = old_load + ((new_load - old_load) >> i);
>>
>> This wont work when new_load < old_load ..
>>
>> For ex: I tried this prog:
>>
>> #include <stdio.h>
>>
>> main()
>> {
>>        unsigned long old_load = 100, new_load = 90, this_load, this_load1;
>>        int i = 1, scale = 2 << i;
>>
>>        this_load = (old_load*(scale-1) + new_load) >> i;
>>        this_load1 = old_load + ((new_load - old_load) >> i);
>
> it should be
>
> this_load2 = 2 * old_load + (new_load >> i) - (old_load >> i);
>
> scale == 2 << i == 1 << (i + 1), so scale >> i = 2.

argh.. sorry, it's (scale = 2 << i) in your test-program which is
wrong. 2^i == (1 << i), so the (2 *) part is redundant, of course.

this_load2 = old_load + (new_load >> i) - (old_load >> i);


-- 
Best regards,
Dmitry Adamushko

Re: [PATCH] sched: Improve readability in update_cpu_load() code

[Gautham R Shenoy]

On Thu, May 15, 2008 at 08:22:15PM +0530, Srivatsa Vaddagiri wrote:
> On Thu, May 15, 2008 at 06:34:59PM +0530, Gautham R Shenoy wrote:
> > Author: Gautham R Shenoy <ego@in.ibm.com>
> > Date:   Thu May 15 17:55:49 2008 +0530
> > 
> >     sched: Improve readability in update_cpu_load() code
> >     
> >     Currently the cpu_load[i] is calculated as:
> >     	this_rq->cpu_load[i] = (old_load*(scale-1) + new_load) >> i;
> >     
> >     However, since scale = 2^i, this can be simplified as:
> >     	this_rq->cpu_load[i] = old_load + ((new_load - old_load) >> i);
> >     
> >     Makes it easier to read.
> >     Signed-off-by: Gautham R Shenoy <ego@in.ibm.com>
> > 
> > diff --git a/kernel/sched.c b/kernel/sched.c
> > index 2d7d8f1..e1a6985 100644
> > --- a/kernel/sched.c
> > +++ b/kernel/sched.c
> > @@ -2921,7 +2921,7 @@ static void update_cpu_load(struct rq *this_rq)
> >  		 */
> >  		if (new_load > old_load)
> >  			new_load += scale-1;
> > -		this_rq->cpu_load[i] = (old_load*(scale-1) + new_load) >> i;
> > +		this_rq->cpu_load[i] = old_load + ((new_load - old_load) >> i);
> 
> This wont work when new_load < old_load ..
>

Sorry for the noise. I must have read the previous if() condition
incorrectly.

> For ex: I tried this prog:
> 
> #include <stdio.h>
> 
> main()
> {
> 	unsigned long old_load = 100, new_load = 90, this_load, this_load1;
> 	int i = 1, scale = 2 << i;
> 
> 	this_load = (old_load*(scale-1) + new_load) >> i;
> 	this_load1 = old_load + ((new_load - old_load) >> i);
> 
> 	printf ("this_load = %u, this_load1 = %u \n", this_load, this_load1);
> }
> 
> This is what I get:
> 
> $ ./a.out
> this_load = 195, this_load1 = 2147483743
> $
> 
> 	
> 
> -- 
> Regards,
> vatsa

-- 
Thanks and Regards
gautham