Skip to contentSkip to conversation

Restore Idle task's priority during CPU_DEAD notification

PATCH In review Srivatsa Vaddagiri 2 participants 3 messages May 26, 2004 2 messages not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): A28EFEDC5416054BA1026D892753E9AF059A50EC@orsmsx404.amr.corp.intel.com

A message that is not in the archive (sent off-list or not yet received): 1085537205.2639.61.camel@bach

Srivatsa Vaddagiri

author
#2 lore raw

PATCH 1/1 Restore Idle task's priority during CPU_DEAD notification

Linus,
Patch below (against 2.6.7-rc1) fixes a CPU Hotplug problem wherein
idle task's "->prio" value is not restored to MAX_PRIO during CPU_DEAD handling. Without this patch, once a CPU is offlined and then later onlined, it becomes "more or less" useless (does not run any task other than its idle task!) Please apply.

---

 linux-2.6.7-rc1-vatsa/kernel/sched.c |    1 +
 1 files changed, 1 insertion(+)
kernel/sched.c renamed from kernel/sched.c~restore_idle_prio +1 -0
diff -puN kernel/sched.c~restore_idle_prio kernel/sched.c
@@ -3569,6 +3569,7 @@ static int migration_call(struct notifie
 		rq = task_rq_lock(rq->idle, &flags);
 		deactivate_task(rq->idle, rq);
 		__setscheduler(rq->idle, SCHED_NORMAL, MAX_PRIO);
+		rq->idle->prio = MAX_PRIO;
 		task_rq_unlock(rq, &flags);
  		BUG_ON(rq->nr_running != 0);

_

Quoted 13 lines · Rusty Russell
Quoted 3 lines · Ashok

Thanks Vatsa...

It seems to work right now....

It seems obviously correct and harmless to me. As it also fixes a problem, I'd say this should go to Linus & Andrew ASAP.

Thanks!
Rusty.

Signature · 1 line
Anyone who quotes me in their signature is an idiot -- Rusty Russell
Signature · 5 lines
Thanks and Regards,
Srivatsa Vaddagiri,
Linux Technology Center,
IBM Software Labs,
Bangalore, INDIA - 560017
#3 lore raw

Review static int migration_call(struct notifie · line 3574

Quoted 7 lines · Srivatsa Vaddagiri
(quoted diff) +1 -0
@@ -3569,6 +3569,7 @@ static int migration_call(struct notifie
 		rq = task_rq_lock(rq->idle, &flags);
 		deactivate_task(rq->idle, rq);
 		__setscheduler(rq->idle, SCHED_NORMAL, MAX_PRIO);
+		rq->idle->prio = MAX_PRIO;
 		task_rq_unlock(rq, &flags);
  		BUG_ON(rq->nr_running != 0);

Looks good. A small nit: while your patch creates a perfectly correct idle thread too, i'd prefer the modified variant below. The __setscheduler() call is (technically) incorrect because in the SCHED_NORMAL case the prio should be zero. So it's a bit cleaner to set up the static priority to MAX_PRIO and then revert the policy to SCHED_NORMAL via __setscheduler(). Ok?

Ingo

From: Srivatsa Vaddagiri <[email protected]>

Signed-off-by Ingo Molnar <[email protected]>
kernel/sched.c renamed from kernel/sched.c.orig +2 -1
@@ -3566,7 +3566,8 @@ static int migration_call(struct notifie
 		/* Idle task back to normal (off runqueue, low prio) */
 		rq = task_rq_lock(rq->idle, &flags);
 		deactivate_task(rq->idle, rq);
-		__setscheduler(rq->idle, SCHED_NORMAL, MAX_PRIO);
+		rq->idle->static_prio = MAX_PRIO;
+		__setscheduler(rq->idle, SCHED_NORMAL, 0);
 		task_rq_unlock(rq, &flags);
  		BUG_ON(rq->nr_running != 0);
author
#4 lore raw
Quoted 6 lines · Ingo Molnar

Looks good. A small nit: while your patch creates a perfectly correct idle thread too, i'd prefer the modified variant below. The __setscheduler() call is (technically) incorrect because in the SCHED_NORMAL case the prio should be zero. So it's a bit cleaner to set up the static priority to MAX_PRIO and then revert the policy to SCHED_NORMAL via __setscheduler(). Ok?

Fine. Since we will have to setup the static priority everytime CPU_DEAD is invoked, would it make sense if we setup this static priority (once and for all) in init_idle instead?

Untested patch below:

---

 linux-2.6.7-rc1-vatsa/kernel/sched.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
kernel/sched.c renamed from kernel/sched.c~restore_idle_prio +2 -2
diff -puN kernel/sched.c~restore_idle_prio kernel/sched.c
@@ -3246,7 +3246,7 @@ void __devinit init_idle(task_t *idle, i
 	idle_rq->curr = idle_rq->idle = idle;
 	deactivate_task(idle, rq);
 	idle->array = NULL;
-	idle->prio = MAX_PRIO;
+	idle->prio = idle->static_prio = MAX_PRIO;
 	idle->state = TASK_RUNNING;
 	set_task_cpu(idle, cpu);
 	double_rq_unlock(idle_rq, rq);

@@ -3568,7 +3568,7 @@ static int migration_call(struct notifie
 		/* Idle task back to normal (off runqueue, low prio) */
 		rq = task_rq_lock(rq->idle, &flags);
 		deactivate_task(rq->idle, rq);
-		__setscheduler(rq->idle, SCHED_NORMAL, MAX_PRIO);
+		__setscheduler(rq->idle, SCHED_NORMAL, 0);
 		task_rq_unlock(rq, &flags);
  		BUG_ON(rq->nr_running != 0);

_

Signature · 5 lines
Thanks and Regards,
Srivatsa Vaddagiri,
Linux Technology Center,
IBM Software Labs,
Bangalore, INDIA - 560017