Re: [Experimental CPU Hotplug PATCH] - Move migrate_all_tasks to CPU_DEAD handling

[Rusty Russell <rusty@rustcorp.com.au>]

On Wed, 2004-04-07 at 15:01, Srivatsa Vaddagiri wrote:
> I restore the mask though (under covers of lock_cpu_hotplug) before
> returning from cpu_down. Task should never see this violated affinity.

But other tasks can do a getaffinity() on it and see the wrong affinity.
Probably not a big issue.

> Rusty,
> 	What do you think abt the whole patch? It has withstood 
> my stress-test harness :-)

I agree with Ingo: it's clever, well done.  Minor nitpicks:

+void migrate_all_tasks(int cpu)
 {
        struct task_struct *tsk, *t;
        int dest_cpu, src_cpu;
        unsigned int node;
 
-       /* We're nailed to this CPU. */
-       src_cpu = smp_processor_id();
+       src_cpu = cpu;

Just make the parameter name "src_cpu"?


+               /* Take idle task off runqueue and restore it's
+                * policy/priority
+                */
+               rq = task_rq_lock(rq->idle, &flags);
+
+               /* Call init_idle instead ?? init_idle doesn't restore
the
+                * policy though for us ..
+                */
+               deactivate_task(rq->idle, rq);
+               __setscheduler(rq->idle, SCHED_NORMAL, MAX_PRIO);
+
+               task_rq_unlock(rq, &flags);

One-line comments and compact code good:

		/* 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);
		task_rq_unlock(rq, &flags);



+               /* Force scheduler to switch to idle task when we yield.
+                * We expect idle task to _immediately_ notice that it's
cpu
+                * is offline and die quickly.
+                *
+                * This allows us to defer calling mirate_all_tasks
until
+                * CPU_DEAD notification time.
+                */
+               sched_idle_next();

This comment's very big.  They don't need to know all the things we
don't do.  I'd prefer:

		/* Force idle task to run as soon as we yield: it should
		   immediately notice cpu is offline and die quickly. */

I'm happy for this to go in..
Rusty
-- 
Anyone who quotes me in their signature is an idiot -- Rusty Russell