On Tue, Apr 25, 2006 at 09:04:41AM +1000, Peter Williams wrote:
> Looks like there's a NOT missing doesn't it. So there is an error but I
> don't think that your patch is the right way to fix it. We just need to
> negate the above assignment. E.g.
>
> skip_for_load = !(busiest_best_prio_seen || idx != busiest_best_prio);
>
> or
>
> skip_for_load = !busiest_best_prio_seen && idx == busiest_best_prio;
>
> whichever is more efficient.
That just will not be enough. busiest_best_prio needs to be set to '1'
whenever we skip the first best priority task(not whenever we move it)
And we also need to initialize busiest_best_prio_seen inside this check.
(like in my patch)
if (busiest->expired->nr_active) {
And we need to reset busiest_best_prio_seen to '0' whenever we finished
the checking of expired list (and move onto active list) and there are
no best prio tasks on expired list..
> > @@ -2072,6 +2067,13 @@ static int move_tasks(runqueue_t *this_r
> > if (busiest->expired->nr_active) {
> > array = busiest->expired;
> > dst_array = this_rq->expired;
> > + /*
> > + * We already have one or more busiest best prio tasks on
> > + * active list.
>
> This is a pretty bold assertion. How do we know that this is true.
That comment refers to when 'busiest_best_prio_seen' is initialized to '1'.
Comment needs to be fixed.
thanks,
suresh
>
> > So if we encounter any busiest best prio task
> > + * on expired list, consider it for the move, if it becomes
> > + * the best prio on new queue.
> > + */
> > + busiest_best_prio_seen = busiest_best_prio == busiest->curr->prio;
> > } else {
> > array = busiest->active;
> > dst_array = this_rq->active;
> > @@ -2089,6 +2091,7 @@ skip_bitmap:
> > if (array == busiest->expired && busiest->active->nr_active) {
> > array = busiest->active;
> > dst_array = this_rq->active;
> > + busiest_best_prio_seen = 0;
> > goto new_array;
> > }
> > goto out;
> > @@ -2107,8 +2110,9 @@ skip_queue:
> > * 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)
> > - skip_for_load = busiest_best_prio_seen || idx != busiest_best_prio;
> > + 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)
> > @@ -2133,8 +2137,6 @@ skip_queue:
> > if (pulled < max_nr_move && rem_load_move > 0) {
> > if (idx < this_best_prio)
> > this_best_prio = idx;
> > - if (idx == busiest_best_prio)
> > - busiest_best_prio_seen = 1;
> > if (curr != head)
> > goto skip_queue;
> > idx++;
>
>
> Peter
> --
> Peter Williams [email protected]
>
> "Learning, n. The kind of ignorance distinguishing the studious."
> -- Ambrose Bierce
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
