[PATCH] sched: smpnice try to wakeup modification

[PATCH] sched: smpnice try to wakeup modification

[Tim Chen]

Peter,

If there is no load on this_cpu, (i.e. tl_per_task is 0), we will fail the  
"tl + target_load(cpu, idx) <= tl_per_task" check.  I think the original intention was
to put task on this_cpu if it has no load and when there's already one task on 
cpu. This helps spread tasks out for low load condition.

Thanks.

Tim

Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>

--- linux-2.6.16-mm2-a/kernel/sched.c	2006-03-28 16:00:37.091779904 -0800
+++ linux-2.6.16-mm2-b/kernel/sched.c	2006-03-28 16:09:08.237074008 -0800
@@ -1393,7 +1393,7 @@ static int try_to_wake_up(task_t *p, uns
 
 		if (this_sd->flags & SD_WAKE_AFFINE) {
 			unsigned long tl = this_load;
-			unsigned long tl_per_task = cpu_avg_load_per_task(this_cpu);
+			unsigned long sl_per_task = cpu_avg_load_per_task(cpu);
 
 			/*
 			 * If sync wakeup then subtract the (maximum possible)
@@ -1404,7 +1404,7 @@ static int try_to_wake_up(task_t *p, uns
 				tl -= current->load_weight;
 
 			if ((tl <= load &&
-				tl + target_load(cpu, idx) <= tl_per_task) ||
+				tl + target_load(cpu, idx) <= sl_per_task) ||
 				100*(tl + p->load_weight) <= imbalance*load) {
 				/*
 				 * This domain has SD_WAKE_AFFINE and

Re: [PATCH] sched: smpnice try to wakeup modification

[Peter Williams]

Tim Chen wrote:
> Peter,
> 
> If there is no load on this_cpu, (i.e. tl_per_task is 0), we will fail 
> the  "tl + target_load(cpu, idx) <= tl_per_task" check.

This isn't the case.  If this_cpu is idle tl_per_task will be set to 
SCHED_LOAD_SCALE (see implementation of cpu_avg_load_per_task()) and 
that expression should succeed unless the value returned by 
target_load(cpu, idx) is bigger than SCHED_LOAD_SCALE.  This is exactly 
the same as would have happened with the original code.

(BTW cpu_avg_load_per_task()'s original implementation would have had 
the effect you describe but it was modified when it was realized that it 
would break the code in a lot of places (not just here).  The thinking 
now is that if there isn't enough data available to calculate the 
average load per task for a run queue then the correct value to use is 
the theoretical average i.e. SCHED_LOAD_SCALE.)

>  I think the 
> original intention was
> to put task on this_cpu if it has no load and when there's already one 
> task on cpu. This helps spread tasks out for low load condition.
> 
> Thanks.
> 
> Tim
> 
> Signed-off-by: Tim Chen <tim.c.chen@linux.intel.com>
> 
> --- linux-2.6.16-mm2-a/kernel/sched.c    2006-03-28 16:00:37.091779904 
> -0800
> +++ linux-2.6.16-mm2-b/kernel/sched.c    2006-03-28 16:09:08.237074008 
> -0800
> @@ -1393,7 +1393,7 @@ static int try_to_wake_up(task_t *p, uns
> 
>         if (this_sd->flags & SD_WAKE_AFFINE) {
>             unsigned long tl = this_load;
> -            unsigned long tl_per_task = cpu_avg_load_per_task(this_cpu);
> +            unsigned long sl_per_task = cpu_avg_load_per_task(cpu);
> 
>             /*
>              * If sync wakeup then subtract the (maximum possible)
> @@ -1404,7 +1404,7 @@ static int try_to_wake_up(task_t *p, uns
>                 tl -= current->load_weight;
> 
>             if ((tl <= load &&
> -                tl + target_load(cpu, idx) <= tl_per_task) ||
> +                tl + target_load(cpu, idx) <= sl_per_task) ||
>                 100*(tl + p->load_weight) <= imbalance*load) {
>                 /*
>                  * This domain has SD_WAKE_AFFINE and

Nevertheless, in some cases, it was difficult to decide which run 
queue's average load per task should be used to replace SCHED_LOAD_SCALE 
and a careful review of the code by those with expertise in this area 
would be appreciated as I may have made mistakes.

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

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

Re: [PATCH] sched: smpnice try to wakeup modification

[Tim Chen]

Peter Williams wrote:
> Tim Chen wrote:
>> Peter,
>>
>> If there is no load on this_cpu, (i.e. tl_per_task is 0), we will 
>> fail the  "tl + target_load(cpu, idx) <= tl_per_task" check.
>
> This isn't the case.  If this_cpu is idle tl_per_task will be set to 
> SCHED_LOAD_SCALE (see implementation of cpu_avg_load_per_task()) and 
> that expression should succeed unless the value returned by 
> target_load(cpu, idx) is bigger than SCHED_LOAD_SCALE.  This is 
> exactly the same as would have happened with the original code.
>
> (BTW cpu_avg_load_per_task()'s original implementation would have had 
> the effect you describe but it was modified when it was realized that 
> it would break the code in a lot of places (not just here).  The 
> thinking now is that if there isn't enough data available to calculate 
> the average load per task for a run queue then the correct value to 
> use is the theoretical average i.e. SCHED_LOAD_SCALE.)
>
Ah, I was looking at the original smpnice patch.  Thanks for clarifying.

Tim