[PATCH][2/4] sched: add discrete weighted cpu load function

[PATCH][2/4] sched: add discrete weighted cpu load function

[Con Kolivas]

When the type of weighting is known to be zero we can use a simpler
version of source_load with a weighted_cpuload() function. Export that
function to allow relative weighted cpu load to be used by other
subsystems if desired.

Signed-off-by: Con Kolivas <kernel@kolivas.org>

---
 include/linux/sched.h |    1 +
 kernel/sched.c        |   10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

Index: linux-2.6.16-rc6-mm1/include/linux/sched.h
===================================================================
--- linux-2.6.16-rc6-mm1.orig/include/linux/sched.h	2006-03-13 13:25:31.000000000 +1100
+++ linux-2.6.16-rc6-mm1/include/linux/sched.h	2006-03-13 18:29:34.000000000 +1100
@@ -102,6 +102,7 @@ extern int nr_processes(void);
 extern unsigned long nr_running(void);
 extern unsigned long nr_uninterruptible(void);
 extern unsigned long nr_iowait(void);
+extern unsigned long weighted_cpuload(const int cpu);
 
 #include <linux/time.h>
 #include <linux/param.h>
Index: linux-2.6.16-rc6-mm1/kernel/sched.c
===================================================================
--- linux-2.6.16-rc6-mm1.orig/kernel/sched.c	2006-03-13 13:25:39.000000000 +1100
+++ linux-2.6.16-rc6-mm1/kernel/sched.c	2006-03-13 17:05:26.000000000 +1100
@@ -914,6 +914,12 @@ inline int task_curr(const task_t *p)
 	return cpu_curr(task_cpu(p)) == p;
 }
 
+/* Used instead of source_load when we know the type == 0 */
+unsigned long weighted_cpuload(const int cpu)
+{
+	return (cpu_rq(cpu)->raw_weighted_load);
+}
+
 #ifdef CONFIG_SMP
 typedef struct {
 	struct list_head list;
@@ -1114,7 +1120,7 @@ find_idlest_cpu(struct sched_group *grou
 	cpus_and(tmp, group->cpumask, p->cpus_allowed);
 
 	for_each_cpu_mask(i, tmp) {
-		load = source_load(i, 0);
+		load = weighted_cpuload(i);
 
 		if (load < min_load || (load == min_load && i == this_cpu)) {
 			min_load = load;
@@ -2186,7 +2192,7 @@ static runqueue_t *find_busiest_queue(st
 	int i;
 
 	for_each_cpu_mask(i, group->cpumask) {
-		load = source_load(i, 0);
+		load = weighted_cpuload(i);
 
 		if (load > max_load) {
 			max_load = load;

Re: [PATCH][2/4] sched: add discrete weighted cpu load function

[Ingo Molnar]

* Con Kolivas <kernel@kolivas.org> wrote:

> +/* Used instead of source_load when we know the type == 0 */
> +unsigned long weighted_cpuload(const int cpu)
> +{
> +	return (cpu_rq(cpu)->raw_weighted_load);

no braces in return please. Looks good to me otherwise.

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo

Re: [PATCH][2/4] sched: add discrete weighted cpu load function

[Con Kolivas]

On Monday 13 March 2006 20:06, Ingo Molnar wrote:
> * Con Kolivas <kernel@kolivas.org> wrote:
> > +/* Used instead of source_load when we know the type == 0 */
> > +unsigned long weighted_cpuload(const int cpu)
> > +{
> > +	return (cpu_rq(cpu)->raw_weighted_load);
>
> no braces in return please. Looks good to me otherwise.
>
> Acked-by: Ingo Molnar <mingo@elte.hu>

Thanks. Respin with that one change below.

Cheers,
Con
---
When the type of weighting is known to be zero we can use a simpler
version of source_load with a weighted_cpuload() function. Export that
function to allow relative weighted cpu load to be used by other
subsystems if desired.

Signed-off-by: Con Kolivas <kernel@kolivas.org>

---
 include/linux/sched.h |    1 +
 kernel/sched.c        |   10 ++++++++--
 2 files changed, 9 insertions(+), 2 deletions(-)

Index: linux-2.6.16-rc6-mm1/include/linux/sched.h
===================================================================
--- linux-2.6.16-rc6-mm1.orig/include/linux/sched.h	2006-03-13 18:29:42.000000000 +1100
+++ linux-2.6.16-rc6-mm1/include/linux/sched.h	2006-03-13 20:11:25.000000000 +1100
@@ -102,6 +102,7 @@ extern int nr_processes(void);
 extern unsigned long nr_running(void);
 extern unsigned long nr_uninterruptible(void);
 extern unsigned long nr_iowait(void);
+extern unsigned long weighted_cpuload(const int cpu);
 
 #include <linux/time.h>
 #include <linux/param.h>
Index: linux-2.6.16-rc6-mm1/kernel/sched.c
===================================================================
--- linux-2.6.16-rc6-mm1.orig/kernel/sched.c	2006-03-13 18:29:42.000000000 +1100
+++ linux-2.6.16-rc6-mm1/kernel/sched.c	2006-03-13 20:12:15.000000000 +1100
@@ -914,6 +914,12 @@ inline int task_curr(const task_t *p)
 	return cpu_curr(task_cpu(p)) == p;
 }
 
+/* Used instead of source_load when we know the type == 0 */
+unsigned long weighted_cpuload(const int cpu)
+{
+	return cpu_rq(cpu)->raw_weighted_load;
+}
+
 #ifdef CONFIG_SMP
 typedef struct {
 	struct list_head list;
@@ -1114,7 +1120,7 @@ find_idlest_cpu(struct sched_group *grou
 	cpus_and(tmp, group->cpumask, p->cpus_allowed);
 
 	for_each_cpu_mask(i, tmp) {
-		load = source_load(i, 0);
+		load = weighted_cpuload(i);
 
 		if (load < min_load || (load == min_load && i == this_cpu)) {
 			min_load = load;
@@ -2186,7 +2192,7 @@ static runqueue_t *find_busiest_queue(st
 	int i;
 
 	for_each_cpu_mask(i, group->cpumask) {
-		load = source_load(i, 0);
+		load = weighted_cpuload(i);
 
 		if (load > max_load) {
 			max_load = load;

Re: [PATCH][2/4] sched: add discrete weighted cpu load function

[Peter Williams]

Con Kolivas wrote:
> When the type of weighting is known to be zero we can use a simpler
> version of source_load with a weighted_cpuload() function. Export that
> function to allow relative weighted cpu load to be used by other
> subsystems if desired.
> 
> Signed-off-by: Con Kolivas <kernel@kolivas.org>
> 
> ---
>  include/linux/sched.h |    1 +
>  kernel/sched.c        |   10 ++++++++--
>  2 files changed, 9 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6.16-rc6-mm1/include/linux/sched.h
> ===================================================================
> --- linux-2.6.16-rc6-mm1.orig/include/linux/sched.h	2006-03-13 13:25:31.000000000 +1100
> +++ linux-2.6.16-rc6-mm1/include/linux/sched.h	2006-03-13 18:29:34.000000000 +1100
> @@ -102,6 +102,7 @@ extern int nr_processes(void);
>  extern unsigned long nr_running(void);
>  extern unsigned long nr_uninterruptible(void);
>  extern unsigned long nr_iowait(void);
> +extern unsigned long weighted_cpuload(const int cpu);
>  
>  #include <linux/time.h>
>  #include <linux/param.h>
> Index: linux-2.6.16-rc6-mm1/kernel/sched.c
> ===================================================================
> --- linux-2.6.16-rc6-mm1.orig/kernel/sched.c	2006-03-13 13:25:39.000000000 +1100
> +++ linux-2.6.16-rc6-mm1/kernel/sched.c	2006-03-13 17:05:26.000000000 +1100
> @@ -914,6 +914,12 @@ inline int task_curr(const task_t *p)
>  	return cpu_curr(task_cpu(p)) == p;
>  }
>  
> +/* Used instead of source_load when we know the type == 0 */
> +unsigned long weighted_cpuload(const int cpu)
> +{
> +	return (cpu_rq(cpu)->raw_weighted_load);
> +}
> +

Wouldn't this be a candidate for inlining?

>  #ifdef CONFIG_SMP
>  typedef struct {
>  	struct list_head list;
> @@ -1114,7 +1120,7 @@ find_idlest_cpu(struct sched_group *grou
>  	cpus_and(tmp, group->cpumask, p->cpus_allowed);
>  
>  	for_each_cpu_mask(i, tmp) {
> -		load = source_load(i, 0);
> +		load = weighted_cpuload(i);
>  
>  		if (load < min_load || (load == min_load && i == this_cpu)) {
>  			min_load = load;
> @@ -2186,7 +2192,7 @@ static runqueue_t *find_busiest_queue(st
>  	int i;
>  
>  	for_each_cpu_mask(i, group->cpumask) {
> -		load = source_load(i, 0);
> +		load = weighted_cpuload(i);
>  
>  		if (load > max_load) {
>  			max_load = load;
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/


-- 
Peter Williams                                   pwil3058@bigpond.net.au

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce

Re: [PATCH][2/4] sched: add discrete weighted cpu load function

[Con Kolivas]

[-- Attachment #1: Type: text/plain, Size: 278 bytes --]

Peter Williams writes:

> Con Kolivas wrote:
>> +unsigned long weighted_cpuload(const int cpu)
>> +{
>> +	return (cpu_rq(cpu)->raw_weighted_load);
>> +}
>> +
> 
> Wouldn't this be a candidate for inlining?

That would make it unsuitable for exporting via sched.h.

Cheers,
Con


[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH][2/4] sched: add discrete weighted cpu load function

[Peter Williams]

Con Kolivas wrote:
> Peter Williams writes:
> 
>> Con Kolivas wrote:
>>
>>> +unsigned long weighted_cpuload(const int cpu)
>>> +{
>>> +    return (cpu_rq(cpu)->raw_weighted_load);
>>> +}
>>> +
>>
>>
>> Wouldn't this be a candidate for inlining?
> 
> 
> That would make it unsuitable for exporting via sched.h.

Right, no cpu_rq() out there.

Sorry, for the noise,
Peter
-- 
Peter Williams                                   pwil3058@bigpond.net.au

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce

Re: [PATCH][2/4] sched: add discrete weighted cpu load function

[Peter Williams]

Con Kolivas wrote:
> Peter Williams writes:
> 
>> Con Kolivas wrote:
>>
>>> +unsigned long weighted_cpuload(const int cpu)
>>> +{
>>> +    return (cpu_rq(cpu)->raw_weighted_load);
>>> +}
>>> +
>>
>>
>> Wouldn't this be a candidate for inlining?
> 
> 
> That would make it unsuitable for exporting via sched.h.

If above_background_load() were implemented inside sched.c instead of in 
sched.h there would be no need to export weighted_cpuload() would there? 
  This would allow weighted_cpuload() to be inline and the efficiency 
would be better as above_background_load() doesn't gain a lot by being 
inline as having weighted_cpulpad() non inline means that it's doing a 
function call several times in a loop i.e. it may save one function call 
by being inline but requires (up to) one function call for every CPU.

The other way around the cost would be just one function call.

Peter
-- 
Peter Williams                                   pwil3058@bigpond.net.au

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce

Re: [PATCH][2/4] sched: add discrete weighted cpu load function

[Con Kolivas]

On Wednesday 15 March 2006 09:09, Peter Williams wrote:
> Con Kolivas wrote:
> > Peter Williams writes:
> >> Con Kolivas wrote:
> >>> +unsigned long weighted_cpuload(const int cpu)
> >>> +{
> >>> +    return (cpu_rq(cpu)->raw_weighted_load);
> >>> +}
> >>> +
> >>
> >> Wouldn't this be a candidate for inlining?
> >
> > That would make it unsuitable for exporting via sched.h.
>
> If above_background_load() were implemented inside sched.c instead of in
> sched.h there would be no need to export weighted_cpuload() would there?
>   This would allow weighted_cpuload() to be inline and the efficiency
> would be better as above_background_load() doesn't gain a lot by being
> inline

I don't care about above_background_load() being inline; that's done because 
all functions in header files need to be static inline to not become a mess.

> as having weighted_cpulpad() non inline means that it's doing a 
> function call several times in a loop i.e. it may save one function call
> by being inline but requires (up to) one function call for every CPU.

I haven't checked but gcc may well inline weighted_cpuload anyway? We're 
moving away from inlining most things manually since the compiler is doing it 
well these days.

> The other way around the cost would be just one function call.

The way you're suggesting adds a function that is never used by anything but 
swap prefetch which would then need to be 'ifdef'ed out to not be needlessly 
built on every system. Adding ifdefs is frowned upon already, and to have an 
mm/ specific ifdef in sched.c would be rather ugly.

Cheers,
Con

Re: [PATCH][2/4] sched: add discrete weighted cpu load function

[Peter Williams]

Con Kolivas wrote:
> On Wednesday 15 March 2006 09:09, Peter Williams wrote:
> 
>>Con Kolivas wrote:
>>
>>>Peter Williams writes:
>>>
>>>>Con Kolivas wrote:
>>>>
>>>>>+unsigned long weighted_cpuload(const int cpu)
>>>>>+{
>>>>>+    return (cpu_rq(cpu)->raw_weighted_load);
>>>>>+}
>>>>>+
>>>>
>>>>Wouldn't this be a candidate for inlining?
>>>
>>>That would make it unsuitable for exporting via sched.h.
>>
>>If above_background_load() were implemented inside sched.c instead of in
>>sched.h there would be no need to export weighted_cpuload() would there?
>>  This would allow weighted_cpuload() to be inline and the efficiency
>>would be better as above_background_load() doesn't gain a lot by being
>>inline
> 
> 
> I don't care about above_background_load() being inline; that's done because 
> all functions in header files need to be static inline to not become a mess.
> 
> 
>>as having weighted_cpulpad() non inline means that it's doing a 
>>function call several times in a loop i.e. it may save one function call
>>by being inline but requires (up to) one function call for every CPU.
> 
> 
> I haven't checked but gcc may well inline weighted_cpuload anyway?

It may be doing so for internal uses inside sched.c but I'm pretty sure 
that it won't for external calls.

> We're 
> moving away from inlining most things manually since the compiler is doing it 
> well these days.
> 

OK.  Even without explicit inlining you need to give the compiler a 
chance to optimize function call overhead away.  It cant do that with 
the present arrangement.

> 
>>The other way around the cost would be just one function call.
> 
> 
> The way you're suggesting adds a function that is never used by anything but 
> swap prefetch which would then need to be 'ifdef'ed out to not be needlessly 
> built on every system. Adding ifdefs is frowned upon already, and to have an 
> mm/ specific ifdef in sched.c would be rather ugly.

Sometimes ugliness is the best option.

Peter
-- 
Peter Williams                                   pwil3058@bigpond.net.au

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce

Re: [PATCH][2/4] sched: add discrete weighted cpu load function

[Con Kolivas]

On Wednesday 15 March 2006 09:45, Peter Williams wrote:
> Con Kolivas wrote:
> > I haven't checked but gcc may well inline weighted_cpuload anyway?
>
> It may be doing so for internal uses inside sched.c but I'm pretty sure
> that it won't for external calls.

Hmm I investigated briefly and only C99 inlining (whatever that means) will 
allow me to locally inline and export as well. It would do so if I specified 
-finline-functions which is not our default at all in the kernel (we only 
recently disable -fnoinline-functions I believe). Anyway checking positively 
shows me this on gcc 4.1.0:

0xc0111283 <find_busiest_queue+83>:     call   0xc0110dc0 <weighted_cpuload>

So no, it doesn't get inlined.

> > The way you're suggesting adds a function that is never used by anything
> > but swap prefetch which would then need to be 'ifdef'ed out to not be
> > needlessly built on every system. Adding ifdefs is frowned upon already,
> > and to have an mm/ specific ifdef in sched.c would be rather ugly.
>
> Sometimes ugliness is the best option.

I spent quite some time trying to find the least cost way to do this without 
uglifying code. I don't feel strongly about just how to do it though. 
Comments from Andrew and Ingo would be most welcome on this matter.

Cheers,
Con