Re: CPU hotplug: system hang on CPU hot remove during `pfmon --system-wide'

[Linus Torvalds <torvalds@linux-foundation.org>]

On Mon, 28 May 2007, Srivatsa Vaddagiri wrote:
>
> 	So is it settled now on what approach we are going to follow (freezer 
> vs lock based) for cpu hotplug? I thought that Linus was not favouring freezer 
> based approach sometime back ..

As far as I'm concerned, we should
 - use "preempt_disable()" to protect against CPU's coming and going 
 - use "stop_machine()" or similar that already honors preemption, and 
   which I trust a whole lot  more than freezer.
 - .. especially since this is already how we are supposed to be protected 
   against CPU's going away, and we've already started doing that (for an 
   example of this, see things like e18f3ffb9c from Andrew)

It really does seem fairly straightforward to make "__cpu_up()" be called 
through stop_machine too. Looking at _cpu_down:

        mutex_lock(&cpu_bitmask_lock);
        p = __stop_machine_run(take_cpu_down, NULL, cpu);
        mutex_unlock(&cpu_bitmask_lock);

and then looking at _cpu_up:

        mutex_lock(&cpu_bitmask_lock);
        ret = __cpu_up(cpu);
        mutex_unlock(&cpu_bitmask_lock);

I just go "Aww, wouldn't it be nice to just make that "__cpu_up()" call be 
done through __stop_machine_run() too?"

Hmm?

Then, you could get the "cpu_bitmask_lock" if you need to sleep, but if 
you don't want to do that (and quite often you don't), just doing a 
"preempt_disable()" or taking a spinlock will *also* guarantee that no new 
CPU's suddenly show up, so it's safe to look at the CPU online bitmasks.

Do we really need anything else?

As mentioned, it's actually fairly easy to add verification calls to make 
sure that certain accesses are done with preemption disabled, so..

			Linus
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/