Re: [PATCH 3/4] 2.6.2-rc2-mm2 CPU Hotplug: The Core

[Ingo Molnar <mingo@elte.hu>]

* Srivatsa Vaddagiri <vatsa@in.ibm.com> wrote:

> One possibility is the migration request will be in CPU 0's queue and
> not in CPU1's? Hence there won't be anything (in CPU1's migration
> request queue) to process when CPU1 is brought down. However by the
> time migration thread on CPU 0 gets around to processing the request,
> CPU1 is already down and hence the check.

IMO the semantics of this area are not defined clearly enough, hence
these problems. __migrate_task(cpu) is a wrapper around
set_cpus_allowed(). If a CPU goes away atomically then the only source
of some non-online CPU is from user-space or unsafe references of
smp_processor_id() [which are also preempt-unsafe so need to be fixed
anyway]. set_cpus_allowed() deals with invalid masks just fine: it does
nothing and returns an error. The following sequence shouldnt be
possible to trigger if CPU-down is an atomic event:

        /* Affinity changed (again). */
        if (!cpu_isset(dest_cpu, p->cpus_allowed))
                goto out;
        /* CPU went down. */
        if (cpu_is_offline(dest_cpu))
                goto out;

because p->cpus_allowed is fixed up atomically. If this task is in
CPU1's migration queue then the CPU-down mechanism takes care of it. If
this task is in CPU0's migration queue then the task will be migrated
off CPU1 by the CPU-down mechanism and CPU0's migration wont have alot
of work left.

kernel-space threads that rely on running on a specific CPU need the
callback mechanism on CPU-down, so there's no problem with them.

user-space tasks that rely on running on a specific CPU need a callback
too. (probably in the form of a signal, which, if unhandled, terminates
the task.) Eg. if a webserver has a mode to run one thread per CPU, then
the server needs to adapt to the new situation when a CPU goes away. We
cannot just unilaterally migrate a task and violate its affinity.

another thing: if the migrate-irqs op is done atomically too (together
with the migrate-tasks op) then the special-cases in idle_balance() and
rebalance_tick() could go away too.

	Ingo