Xen devel - Feb 2012 - [PATCH] vcpu hotplug fix for v3.3-rc3 (v1).

When playing with xl/xm vcpu-set I found out that Linux 3.3 has a bunch of
problems.
Some of them were in generic code base (patches for that are queued up by Greg
KH),
and some were in the Xen base. Previously I had posted:

    xen/smp: Fix CPU online/offline bug triggering a BUG: scheduling while
atomic.
    xen/bootup: During bootup suppress XENBUS: Unable to read cpu state

and this is meant to be on top of those to make vCPU hotplugging work properly
with 3.3 kernel. Some of those should be back-ported to 3.0 but I am not sure
which ones (and if there are other ones as well) so won''t do that until
I get some time
to do a proper testing of them on v3.0.

If you want to full kitchensink, please see #testing branch. If you just want
to see those patches above (and others queued up for 3.3-rc3), take a look at
#stable/for-linus-fixes-3.3 in
git://git.kernel.org/pub/scm/linux/kernel/git/konrad/xen.git

Offlining CPUs works. Onlining does not work anymore and it looks
like we were missing a cpu_up() call in the hotplug bring up path.

When the vCPUs are initialized, if they are onlined or never had
been in use (so maxcpus > vcpus) the vCPUs are in xen_play_dead
having called VCPUOP_down and are not running. When a vCPU is
onlined, the bootup (or any other currently running CPU) is suppose
to call VCPUOP_up on the vCPU that is not running and make the
vCPU unhinge itself out of the xen_play_dead and continue in
cpu_idle. We did not make the hypercall on onlining, nor did we
recreate the timer, spinlocks, ipi interrupts so the vCPU never
was onlined. This patch fixes it by calling cpu_on() and we
also make the code more readable.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
 drivers/xen/cpu_hotplug.c |   15 ++++++++-------
 1 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/xen/cpu_hotplug.c b/drivers/xen/cpu_hotplug.c
index 4dcfced..df80af8 100644
--- a/drivers/xen/cpu_hotplug.c
+++ b/drivers/xen/cpu_hotplug.c
@@ -8,18 +8,20 @@
 
 static void enable_hotplug_cpu(int cpu)
 {
-	if (!cpu_present(cpu))
+	if (!cpu_present(cpu)) {
 		arch_register_cpu(cpu);
-
-	set_cpu_present(cpu, true);
+		set_cpu_present(cpu, true);
+		(void)cpu_up(cpu);
+	}
 }
 
 static void disable_hotplug_cpu(int cpu)
 {
-	if (cpu_present(cpu))
+	if (cpu_present(cpu)) {
+		(void)cpu_down(cpu);
+		set_cpu_present(cpu, false);
 		arch_unregister_cpu(cpu);
-
-	set_cpu_present(cpu, false);
+	}
 }
 
 static int vcpu_online(unsigned int cpu)
@@ -53,7 +55,6 @@ static void vcpu_hotplug(unsigned int cpu)
 		enable_hotplug_cpu(cpu);
 		break;
 	case 0:
-		(void)cpu_down(cpu);
 		disable_hotplug_cpu(cpu);
 		break;
 	default:
-- 
1.7.7.5

On Fri, 2012-02-10 at 14:43 +0000, Konrad Rzeszutek Wilk
wrote:
> Offlining CPUs works. Onlining does not work anymore and it looks
> like we were missing a cpu_up() call in the hotplug bring up path.
Wasn''t this a deliberate decision as part of the pvops upstreaming to
be
consistent with native cpu hotplug? After hotplugging a CPU
administrator action is needed to actually online it.

http://xen.1045712.n5.nabble.com/cpu-down-but-no-cpu-up-in-drivers-xen-cpu-hotplug-c-td2545244.html

The fix is to add a udev rule:
http://wiki.xen.org/wiki/Paravirt_Linux_CPU_Hotplug

Ian.

On Fri, Feb 10, 2012 at 03:03:46PM +0000, Ian Campbell
wrote:
> On Fri, 2012-02-10 at 14:43 +0000, Konrad Rzeszutek Wilk wrote:
> > Offlining CPUs works. Onlining does not work anymore and it looks
> > like we were missing a cpu_up() call in the hotplug bring up path.
> 
> Wasn''t this a deliberate decision as part of the pvops upstreaming
to be
> consistent with native cpu hotplug? After hotplugging a CPU
Ah, hadn''t seen that. Thanks for spotting that!
> administrator action is needed to actually online it.
> 
>
http://xen.1045712.n5.nabble.com/cpu-down-but-no-cpu-up-in-drivers-xen-cpu-hotplug-c-td2545244.html
> 
> The fix is to add a udev rule:
> http://wiki.xen.org/wiki/Paravirt_Linux_CPU_Hotplug
Ah, so I see that it is run (the udev does it) but it actually does not bring
the CPU up.

It looks as if the ''online'' attribute is actually not present
at all, even
after bringing the CPU "up-but-not-online". Oh joy! Should be easy to
reproduce
on baremetal.