2004-11-19 04:13:40

by Steven Rostedt

[permalink] [raw]
Subject: what is the need for task_rq in setscheduler?

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


2004-11-19 04:39:25

by Robert Love

[permalink] [raw]
Subject: [patch] no need to recalculate rq

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 <[email protected]>

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


2004-11-19 08:49:40

by Ingo Molnar

[permalink] [raw]
Subject: Re: [patch] no need to recalculate rq


* Robert Love <[email protected]> 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 <[email protected]>

Ingo