* [PATCH] sched: Fix boolean expression in move_tasks()
@ 2006-04-24 23:47 Peter Williams
2006-04-25 2:13 ` Andrew Morton
0 siblings, 1 reply; 4+ messages in thread
From: Peter Williams @ 2006-04-24 23:47 UTC (permalink / raw)
To: Andrew Morton
Cc: Con Kolivas, Ingo Molnar, Siddha, Suresh B, Mike Galbraith,
Nick Piggin, Linux Kernel Mailing List, Chen, Kenneth W
[-- Attachment #1: Type: text/plain, Size: 647 bytes --]
Problem:
In the patch
sched-avoid-unnecessarily-moving-highest-priority-task-move_tasks.patch
I got a the sense of a boolean expression wrong when assigning a value
to skip_for_load. The expression should have been negated before being
assigned.
Solution:
Negate the expression and apply de Marcos rule to simplify it. This
patch is on top of
sched-avoid-unnecessarily-moving-highest-priority-task-move_tasks.patch
Signed-off-by: Peter Williams <pwil3058@bigpond.com.au>
--
Peter Williams pwil3058@bigpond.net.au
"Learning, n. The kind of ignorance distinguishing the studious."
-- Ambrose Bierce
[-- Attachment #2: smpnice-fix-boolean-expression --]
[-- Type: text/plain, Size: 653 bytes --]
Index: MM-2.6.17-rc1-mm3/kernel/sched.c
===================================================================
--- MM-2.6.17-rc1-mm3.orig/kernel/sched.c 2006-04-21 12:26:54.000000000 +1000
+++ MM-2.6.17-rc1-mm3/kernel/sched.c 2006-04-25 09:09:54.000000000 +1000
@@ -2108,7 +2108,7 @@ skip_queue:
*/
skip_for_load = tmp->load_weight > rem_load_move;
if (skip_for_load && idx < this_best_prio)
- skip_for_load = busiest_best_prio_seen || idx != busiest_best_prio;
+ skip_for_load = !busiest_best_prio_seen && idx == busiest_best_prio;
if (skip_for_load ||
!can_migrate_task(tmp, busiest, this_cpu, sd, idle, &pinned)) {
if (curr != head)
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] sched: Fix boolean expression in move_tasks()
2006-04-24 23:47 [PATCH] sched: Fix boolean expression in move_tasks() Peter Williams
@ 2006-04-25 2:13 ` Andrew Morton
2006-04-25 2:37 ` Peter Williams
0 siblings, 1 reply; 4+ messages in thread
From: Andrew Morton @ 2006-04-25 2:13 UTC (permalink / raw)
To: Peter Williams
Cc: kernel, mingo, suresh.b.siddha, efault, nickpiggin, linux-kernel,
kenneth.w.chen
Peter Williams <pwil3058@bigpond.net.au> wrote:
>
> Negate the expression and apply de Marcos rule to simplify it. This
> patch is on top of
> sched-avoid-unnecessarily-moving-highest-priority-task-move_tasks.patch
>
> Signed-off-by: Peter Williams <pwil3058@bigpond.com.au>
>
> --
> Peter Williams pwil3058@bigpond.net.au
>
> "Learning, n. The kind of ignorance distinguishing the studious."
> -- Ambrose Bierce
>
>
> [smpnice-fix-boolean-expression text/plain (824 bytes)]
> Index: MM-2.6.17-rc1-mm3/kernel/sched.c
> ===================================================================
> --- MM-2.6.17-rc1-mm3.orig/kernel/sched.c 2006-04-21 12:26:54.000000000 +1000
> +++ MM-2.6.17-rc1-mm3/kernel/sched.c 2006-04-25 09:09:54.000000000 +1000
> @@ -2108,7 +2108,7 @@ skip_queue:
> */
> skip_for_load = tmp->load_weight > rem_load_move;
> if (skip_for_load && idx < this_best_prio)
> - skip_for_load = busiest_best_prio_seen || idx != busiest_best_prio;
> + skip_for_load = !busiest_best_prio_seen && idx == busiest_best_prio;
But Suresh's
sched-avoid-unnecessarily-moving-highest-priority-task-move_tasks-fix.patch
changed all this code:
/*
* To help distribute high priority tasks accross CPUs we don't
* skip a task if it will be the highest priority task (i.e. smallest
* prio value) on its new queue regardless of its load weight
*/
skip_for_load = tmp->load_weight > rem_load_move;
if (skip_for_load && idx < this_best_prio && idx == busiest_best_prio)
skip_for_load = !busiest_best_prio_seen &&
head->next == head->prev;
if (skip_for_load ||
!can_migrate_task(tmp, busiest, this_cpu, sd, idle, &pinned)) {
if (curr != head)
goto skip_queue;
idx++;
goto skip_bitmap;
}
What to do?
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] sched: Fix boolean expression in move_tasks()
2006-04-25 2:13 ` Andrew Morton
@ 2006-04-25 2:37 ` Peter Williams
2006-04-25 2:47 ` Andrew Morton
0 siblings, 1 reply; 4+ messages in thread
From: Peter Williams @ 2006-04-25 2:37 UTC (permalink / raw)
To: Andrew Morton
Cc: kernel, mingo, suresh.b.siddha, efault, nickpiggin, linux-kernel,
kenneth.w.chen
Andrew Morton wrote:
> Peter Williams <pwil3058@bigpond.net.au> wrote:
>> Negate the expression and apply de Marcos rule to simplify it. This
>> patch is on top of
>> sched-avoid-unnecessarily-moving-highest-priority-task-move_tasks.patch
>>
>> Signed-off-by: Peter Williams <pwil3058@bigpond.com.au>
>>
>> --
>> Peter Williams pwil3058@bigpond.net.au
>>
>> "Learning, n. The kind of ignorance distinguishing the studious."
>> -- Ambrose Bierce
>>
>>
>> [smpnice-fix-boolean-expression text/plain (824 bytes)]
>> Index: MM-2.6.17-rc1-mm3/kernel/sched.c
>> ===================================================================
>> --- MM-2.6.17-rc1-mm3.orig/kernel/sched.c 2006-04-21 12:26:54.000000000 +1000
>> +++ MM-2.6.17-rc1-mm3/kernel/sched.c 2006-04-25 09:09:54.000000000 +1000
>> @@ -2108,7 +2108,7 @@ skip_queue:
>> */
>> skip_for_load = tmp->load_weight > rem_load_move;
>> if (skip_for_load && idx < this_best_prio)
>> - skip_for_load = busiest_best_prio_seen || idx != busiest_best_prio;
>> + skip_for_load = !busiest_best_prio_seen && idx == busiest_best_prio;
>
> But Suresh's
> sched-avoid-unnecessarily-moving-highest-priority-task-move_tasks-fix.patch
> changed all this code:
>
> /*
> * To help distribute high priority tasks accross CPUs we don't
> * skip a task if it will be the highest priority task (i.e. smallest
> * prio value) on its new queue regardless of its load weight
> */
> skip_for_load = tmp->load_weight > rem_load_move;
> if (skip_for_load && idx < this_best_prio && idx == busiest_best_prio)
> skip_for_load = !busiest_best_prio_seen &&
> head->next == head->prev;
> if (skip_for_load ||
> !can_migrate_task(tmp, busiest, this_cpu, sd, idle, &pinned)) {
> if (curr != head)
> goto skip_queue;
> idx++;
> goto skip_bitmap;
> }
>
>
> What to do?
Suresh's patch was wrong and this was intended as an alternative.
Unfortunately, it is also in adequate and the setting of
busiest_best_prio_seen needs to be moved to just after skip_for_load is
set. I have another patch that does that (and adds to the comments).
Should I send that separately or roll the two patches together?
Peter
--
Peter Williams pwil3058@bigpond.net.au
"Learning, n. The kind of ignorance distinguishing the studious."
-- Ambrose Bierce
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] sched: Fix boolean expression in move_tasks()
2006-04-25 2:37 ` Peter Williams
@ 2006-04-25 2:47 ` Andrew Morton
0 siblings, 0 replies; 4+ messages in thread
From: Andrew Morton @ 2006-04-25 2:47 UTC (permalink / raw)
To: Peter Williams
Cc: kernel, mingo, suresh.b.siddha, efault, nickpiggin, linux-kernel,
kenneth.w.chen
Peter Williams <pwil3058@bigpond.net.au> wrote:
>
> > What to do?
>
> Suresh's patch was wrong and this was intended as an alternative.
OIC.
> Unfortunately, it is also in adequate and the setting of
> busiest_best_prio_seen needs to be moved to just after skip_for_load is
> set. I have another patch that does that (and adds to the comments).
> Should I send that separately or roll the two patches together?
I don't mind, really - rolled together, I guess.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2006-04-25 2:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-04-24 23:47 [PATCH] sched: Fix boolean expression in move_tasks() Peter Williams
2006-04-25 2:13 ` Andrew Morton
2006-04-25 2:37 ` Peter Williams
2006-04-25 2:47 ` Andrew Morton
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.