Messages in this thread Patch in this message | | | Date | Mon, 24 Apr 2006 16:48:00 -0700 | From | "Siddha, Suresh B" <> | Subject | Re: [PATCH] sched: Avoid unnecessarily moving highest priority task move_tasks() |
| |
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 pwil3058@bigpond.net.au > > "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 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/
| |