what is the need for task_rq in setscheduler?

what is the need for task_rq in setscheduler?

[Steven Rostedt]

I'm curious to the need for the task_rq in setscheduler in the following
code:

   3316         rq = task_rq_lock(p, &flags);
   3317         /* recheck policy now with rq lock held */
   3318         if (unlikely(oldpolicy != -1 && oldpolicy != p->policy)) {
   3319                 policy = oldpolicy = -1;
   3320                 task_rq_unlock(rq, &flags);
   3321                 goto recheck;
   3322         }
   3323         array = p->array;
   3324         if (array)
   3325                 deactivate_task(p, task_rq(p));
   3326         retval = 0;
   3327         oldprio = p->prio;
   3328         __setscheduler(p, policy, lp.sched_priority);
   3329         if (array) {
   3330                 __activate_task(p, task_rq(p));
   3331                 /*
   3332                  * Reschedule if we are currently running on this runqueue and
   3333                  * our priority decreased, or if we are not currently running on
   3334                  * this runqueue and our priority is higher than the current's
   3335                  */
   3336                 if (task_running(rq, p)) {
   3337                         if (p->prio > oldprio)
   3338                                 resched_task(rq->curr);
   3339                 } else if (TASK_PREEMPTS_CURR(p, rq))
   3340                         resched_task(rq->curr);
   3341         }
   3342         task_rq_unlock(rq, &flags);


On lines 3325 and 3330 with the calls to deactivate_task and
__activate_task respectively. The runqueue is locked at 3316. Can the
runqueue returned by task_rq change in this setup? If not, what is the
need for the call to task_rq. Can't rq just be used instead, or is this
just some extra paranoia? 

Thanks,

-- Steve

[patch] no need to recalculate rq

[Robert Love]

On Thu, 2004-11-18 at 23:13 -0500, Steven Rostedt wrote:

> On lines 3325 and 3330 with the calls to deactivate_task and
> __activate_task respectively. The runqueue is locked at 3316. Can the
> runqueue returned by task_rq change in this setup? If not, what is the
> need for the call to task_rq. Can't rq just be used instead, or is this
> just some extra paranoia? 

I don't see any reason that it is needed.  The runqueue is locked and
the task is not going anywhere.  There is no need to recalculate the
runqueue.

Patch below uses the cached runqueue, rq, saving a few instructions.

	Robert Love


no need to call task_rq in setscheduler; just use rq

Signed-Off-By: Robert Love <rml@novell.com>

 kernel/sched.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff -urN linux-2.6.10-rc2/kernel/sched.c linux/kernel/sched.c
--- linux-2.6.10-rc2/kernel/sched.c	2004-11-18 23:32:54.189696376 -0500
+++ linux/kernel/sched.c	2004-11-18 23:35:54.309314032 -0500
@@ -3144,12 +3144,12 @@
 	}
 	array = p->array;
 	if (array)
-		deactivate_task(p, task_rq(p));
+		deactivate_task(p, rq);
 	retval = 0;
 	oldprio = p->prio;
 	__setscheduler(p, policy, lp.sched_priority);
 	if (array) {
-		__activate_task(p, task_rq(p));
+		__activate_task(p, rq);
 		/*
 		 * Reschedule if we are currently running on this runqueue and
 		 * our priority decreased, or if we are not currently running on

Re: [patch] no need to recalculate rq

[Ingo Molnar]

* Robert Love <rml@novell.com> wrote:

> -		deactivate_task(p, task_rq(p));
> +		deactivate_task(p, rq);
>  	retval = 0;
>  	oldprio = p->prio;
>  	__setscheduler(p, policy, lp.sched_priority);
>  	if (array) {
> -		__activate_task(p, task_rq(p));
> +		__activate_task(p, rq);

Acked-by: Ingo Molnar <mingo@elte.hu>

	Ingo