[PATCH] sched: Avoid unnecessarily moving highest priority task move_tasks()

[PATCH] sched: Avoid unnecessarily moving highest priority task move_tasks()

[Peter Williams]

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

Problem:

To help distribute high priority tasks evenly across the available CPUs 
move_tasks() does not, under some circumstances, skip tasks whose load 
weight is bigger than the designated amount.  Because the highest 
priority task on the busiest queue may be on the expired array it may be 
moved as a result of this mechanism.  Apart from not being the most 
desirable way to redistribute the high priority tasks (we'd rather move 
the second highest priority task), there is a risk that this could set 
up a loop with this task bouncing backwards and forwards between the two 
queues.  (This latter possibility can be demonstrated by running a 
nice==-20 CPU bound task on an otherwise quiet 2 CPU system.)

Solution:

Modify the mechanism so that it does not override skip for the highest 
priority task on the CPU.  Of course, if there are more than one tasks 
at the highest priority then it will allow the override for one of them 
as this is a desirable redistribution of high priority tasks.

Signed-off-by: Peter Williams <pwil3058@bigpond.com.au>

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

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

[-- Attachment #2: smpnice-avoid-moving-highest --]
[-- Type: text/plain, Size: 2363 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:19:30.000000000 +1000
+++ MM-2.6.17-rc1-mm3/kernel/sched.c	2006-04-21 12:26:54.000000000 +1000
@@ -2029,6 +2029,7 @@ int can_migrate_task(task_t *p, runqueue
 	return 1;
 }
 
+#define rq_best_prio(rq) min((rq)->curr->prio, (rq)->best_expired_prio)
 /*
  * move_tasks tries to move up to max_nr_move tasks and max_load_move weighted
  * load from busiest to this_rq, as part of a balancing operation within
@@ -2043,7 +2044,9 @@ static int move_tasks(runqueue_t *this_r
 {
 	prio_array_t *array, *dst_array;
 	struct list_head *head, *curr;
-	int idx, pulled = 0, pinned = 0, this_min_prio;
+	int idx, pulled = 0, pinned = 0, this_best_prio, busiest_best_prio;
+	int busiest_best_prio_seen;
+	int skip_for_load; /* skip the task based on weighted load issues */
 	long rem_load_move;
 	task_t *tmp;
 
@@ -2052,7 +2055,13 @@ static int move_tasks(runqueue_t *this_r
 
 	rem_load_move = max_load_move;
 	pinned = 1;
-	this_min_prio = this_rq->curr->prio;
+	this_best_prio = rq_best_prio(this_rq);
+	busiest_best_prio = rq_best_prio(busiest);
+	/*
+	 * Enable handling of the case where there is more than one task
+	 * with the best priority.
+	 */
+	busiest_best_prio_seen = busiest_best_prio == busiest->curr->prio;
 
 	/*
 	 * We first consider expired tasks. Those will likely not be
@@ -2097,7 +2106,10 @@ skip_queue:
 	 * 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
 	 */
-	if ((idx >= this_min_prio && tmp->load_weight > rem_load_move) ||
+	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 ||
 	    !can_migrate_task(tmp, busiest, this_cpu, sd, idle, &pinned)) {
 		if (curr != head)
 			goto skip_queue;
@@ -2119,8 +2131,10 @@ skip_queue:
 	 * and the prescribed amount of weighted load.
 	 */
 	if (pulled < max_nr_move && rem_load_move > 0) {
-		if (idx < this_min_prio)
-			this_min_prio = idx;
+		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++;

Re: [PATCH] sched: Avoid unnecessarily moving highest priority task move_tasks()

[Siddha, Suresh B]

On Fri, Apr 21, 2006 at 02:22:57PM +1000, Peter Williams wrote:
> @@ -2052,7 +2055,13 @@ static int move_tasks(runqueue_t *this_r
>  
>  	rem_load_move = max_load_move;
>  	pinned = 1;
> -	this_min_prio = this_rq->curr->prio;
> +	this_best_prio = rq_best_prio(this_rq);
> +	busiest_best_prio = rq_best_prio(busiest);
> +	/*
> +	 * Enable handling of the case where there is more than one task
> +	 * with the best priority.
> +	 */
> +	busiest_best_prio_seen = busiest_best_prio == busiest->curr->prio;

>From this hunk, it seems like we don't want to override the skip of highest 
priority task as long as there is one such task in active list(even though
there may be few such tasks on expired list). Right? And why?

If we fix the above, we don't need busiest_best_prio_seen. Once we move one 
highest priority task, we are changing this_best_prio anyhow right?

This patch doesn't address the issue where we can skip the highest priority 
task movement if there is only one such task on the busy runqueue
(and is on the expired list..)

I can send a fix if I understand your intention for the above hunk.

thanks,
suresh

Re: [PATCH] sched: Avoid unnecessarily moving highest priority task move_tasks()

[Peter Williams]

Siddha, Suresh B wrote:
> On Fri, Apr 21, 2006 at 02:22:57PM +1000, Peter Williams wrote:
>> @@ -2052,7 +2055,13 @@ static int move_tasks(runqueue_t *this_r
>>  
>>  	rem_load_move = max_load_move;
>>  	pinned = 1;
>> -	this_min_prio = this_rq->curr->prio;
>> +	this_best_prio = rq_best_prio(this_rq);
>> +	busiest_best_prio = rq_best_prio(busiest);
>> +	/*
>> +	 * Enable handling of the case where there is more than one task
>> +	 * with the best priority.
>> +	 */
>> +	busiest_best_prio_seen = busiest_best_prio == busiest->curr->prio;
> 
>>From this hunk, it seems like we don't want to override the skip of highest 
> priority task as long as there is one such task in active list(even though
> there may be few such tasks on expired list). Right?

No, but nearly.  We don't want to override the skip of a task with the 
highest priority until we've skipped one such task OR we're sure one 
such task will be skipped (e.g. because it's the currently running task).

> And why?

If there are more than one task with the highest priority then it is 
desirable to move one of them by overriding the skip mechanism as it can 
be considered the second highest priority task.  This initialization 
just checks to see if the currently running task is one of the highest 
priority tasks.  If it is then it's OK to move the first task we find 
that also has the same priority otherwise we wait until we've skipped 
one before we move one.

> 
> If we fix the above, we don't need busiest_best_prio_seen. Once we move one 
> highest priority task, we are changing this_best_prio anyhow right?

Yes, but this is recording the fact that we've skipped one and that it's 
now OK to move one.

> 
> This patch doesn't address the issue where we can skip the highest priority 
> task movement if there is only one such task on the busy runqueue
> (and is on the expired list..)

I think that it does.

> 
> I can send a fix if I understand your intention for the above hunk.

If you think that there's still a problem after the above explanation 
then sending a patch might help me understand why you think it's wrong.

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

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

Re: [PATCH] sched: Avoid unnecessarily moving highest priority task move_tasks()

[Siddha, Suresh B]

On Sat, Apr 22, 2006 at 11:31:29AM +1000, Peter Williams wrote:
> If there are more than one task with the highest priority then it is 
> desirable to move one of them by overriding the skip mechanism as it can 
> be considered the second highest priority task.  

I think your patch is not doing what you intend to do.

> This initialization 
> just checks to see if the currently running task is one of the highest 
> priority tasks.  If it is then it's OK to move the first task we find 
> that also has the same priority otherwise we wait until we've skipped 
> one before we move one.

If this currently running task is of the highest priority, we set
busiest_best_prio_seen to '1' and looking at the code, because of this
we never consider any other busiest task which we come across on the expired
list.. This is coming from this piece of code.

	skip_for_load = busiest_best_prio_seen || idx != busiest_best_prio;

skip_for_load is always set to '1'(because of busiest_best_prio_seen)
and we will never be able to move any busiest task to the new queue.

> > This patch doesn't address the issue where we can skip the highest priority 
> > task movement if there is only one such task on the busy runqueue
> > (and is on the expired list..)
> 
> I think that it does.

No. It doesn't. In this case busiest_best_prio_seen will be set to '0', when
we traverse the only highest priority task on this queue(which happens to
be on expired list), we set skip_for_load to '0' And we will try
pulling the only highest priority task on this queue to the new queue..

Appended patch(ontop of 
sched-avoid-unnecessarily-moving-highest-priority-task-move_tasks.patch)
fixes these issues.

Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>

--- linux-2.6.17-rc1/kernel/sched.c~	2006-04-21 13:12:01.749853640 -0700
+++ linux-2.6.17-rc1/kernel/sched.c	2006-04-24 10:07:39.624162896 -0700
@@ -2045,7 +2045,7 @@ static int move_tasks(runqueue_t *this_r
 	prio_array_t *array, *dst_array;
 	struct list_head *head, *curr;
 	int idx, pulled = 0, pinned = 0, this_best_prio, busiest_best_prio;
-	int busiest_best_prio_seen;
+	int busiest_best_prio_seen = 0;
 	int skip_for_load; /* skip the task based on weighted load issues */
 	long rem_load_move;
 	task_t *tmp;
@@ -2057,11 +2057,6 @@ static int move_tasks(runqueue_t *this_r
 	pinned = 1;
 	this_best_prio = rq_best_prio(this_rq);
 	busiest_best_prio = rq_best_prio(busiest);
-	/*
-	 * Enable handling of the case where there is more than one task
-	 * with the best priority.
-	 */
-	busiest_best_prio_seen = busiest_best_prio == busiest->curr->prio;
 
 	/*
 	 * We first consider expired tasks. Those will likely not be
@@ -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. 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++;

Re: [PATCH] sched: Avoid unnecessarily moving highest priority task move_tasks()

[Peter Williams]

Siddha, Suresh B wrote:
> On Sat, Apr 22, 2006 at 11:31:29AM +1000, Peter Williams wrote:
>> If there are more than one task with the highest priority then it is 
>> desirable to move one of them by overriding the skip mechanism as it can 
>> be considered the second highest priority task.  
> 
> I think your patch is not doing what you intend to do.
> 
>> This initialization 
>> just checks to see if the currently running task is one of the highest 
>> priority tasks.  If it is then it's OK to move the first task we find 
>> that also has the same priority otherwise we wait until we've skipped 
>> one before we move one.
> 
> If this currently running task is of the highest priority, we set
> busiest_best_prio_seen to '1' and looking at the code, because of this
> we never consider any other busiest task which we come across on the expired
> list.. This is coming from this piece of code.
> 
> 	skip_for_load = busiest_best_prio_seen || idx != busiest_best_prio;
> 
> skip_for_load is always set to '1'(because of busiest_best_prio_seen)
> and we will never be able to move any busiest task to the new queue.

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.

> 
>>> This patch doesn't address the issue where we can skip the highest priority 
>>> task movement if there is only one such task on the busy runqueue
>>> (and is on the expired list..)
>> I think that it does.
> 
> No. It doesn't. In this case busiest_best_prio_seen will be set to '0', when
> we traverse the only highest priority task on this queue(which happens to
> be on expired list), we set skip_for_load to '0' And we will try
> pulling the only highest priority task on this queue to the new queue..
> 
> Appended patch(ontop of 
> sched-avoid-unnecessarily-moving-highest-priority-task-move_tasks.patch)
> fixes these issues.
> 
> Signed-off-by: Suresh Siddha <suresh.b.siddha@intel.com>
> 
> --- linux-2.6.17-rc1/kernel/sched.c~	2006-04-21 13:12:01.749853640 -0700
> +++ linux-2.6.17-rc1/kernel/sched.c	2006-04-24 10:07:39.624162896 -0700
> @@ -2045,7 +2045,7 @@ static int move_tasks(runqueue_t *this_r
>  	prio_array_t *array, *dst_array;
>  	struct list_head *head, *curr;
>  	int idx, pulled = 0, pinned = 0, this_best_prio, busiest_best_prio;
> -	int busiest_best_prio_seen;
> +	int busiest_best_prio_seen = 0;
>  	int skip_for_load; /* skip the task based on weighted load issues */
>  	long rem_load_move;
>  	task_t *tmp;
> @@ -2057,11 +2057,6 @@ static int move_tasks(runqueue_t *this_r
>  	pinned = 1;
>  	this_best_prio = rq_best_prio(this_rq);
>  	busiest_best_prio = rq_best_prio(busiest);
> -	/*
> -	 * Enable handling of the case where there is more than one task
> -	 * with the best priority.
> -	 */
> -	busiest_best_prio_seen = busiest_best_prio == busiest->curr->prio;
>  
>  	/*
>  	 * We first consider expired tasks. Those will likely not be
> @@ -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.

> 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

Re: [PATCH] sched: Avoid unnecessarily moving highest priority task move_tasks()

[Siddha, Suresh B]

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

Re: [PATCH] sched: Avoid unnecessarily moving highest priority task move_tasks()

[Peter Williams]

Siddha, Suresh B wrote:
> 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)

Yes, I just realized that.  It needs to be done after skip_for_load has 
been finalized.

> 
> And we also need to initialize busiest_best_prio_seen inside this check.
> (like in my patch)
> 	if (busiest->expired->nr_active) {

Why?  It's already initialized.  If the current running task has 
prio==busiest_best_prio then we know that can_migrate_task() will 
prevent it from being moved so it's safe to move any other tasks we meet 
with that priority.

> 
> 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..

No we don't.  Once we've skipped one it's OK to move any others that we 
find.  We'll never move more than one as a result of overriding the skip 
anyhow .

> 
>>> @@ -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.

But you initialized it to zero.

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

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

Re: [PATCH] sched: Avoid unnecessarily moving highest priority task move_tasks()

[Siddha, Suresh B]

On Tue, Apr 25, 2006 at 12:30:00PM +1000, Peter Williams wrote:
> Siddha, Suresh B wrote:
> > And we also need to initialize busiest_best_prio_seen inside this check.
> > (like in my patch)
> > 	if (busiest->expired->nr_active) {
> 
> Why?  It's already initialized.  If the current running task has 
> prio==busiest_best_prio then we know that can_migrate_task() will 
> prevent it from being moved so it's safe to move any other tasks we meet 
> with that priority.
> > 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..
> 
> No we don't.  Once we've skipped one it's OK to move any others that we 
> find.  We'll never move more than one as a result of overriding the skip 
> anyhow .

Ok.

> 
> > 
> >>> @@ -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.
> 
> But you initialized it to zero.

That comment refers to the assignment code below it..

Anyhow, now that we are going with fixes to your patch, this is a moot point.

thanks,
suresh