x86: Do no thold cpu_add_remove_lock across stop_machine_run().

Instead we protect against concurrent online/offline requests for a
single CPU asynchronously, using a cpumask for bookkeeping.

This means that cpu_add_remove_lock no longer needs to be acquired via
spin_trylock.

Signed-off-by: Keir Fraser <keir.fraser@citrix.com>

diff --git a/xen/arch/x86/platform_hypercall.c b/xen/arch/x86/platform_hypercall.c
index 81639e91f1b45d8e41c9081d75bb7e98fc7b7532..67a5fbb73c36b870554436d99157c7cc4cc78309 100644 (file)
--- a/xen/arch/x86/platform_hypercall.c
+++ b/xen/arch/x86/platform_hypercall.c
@@ -409,12 +409,7 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xen_platform_op_t) u_xenpf_op)
 
         g_info = &op->u.pcpu_info;
 
-        /* spin_trylock() avoids deadlock with stop_machine_run(). */
-        if ( !spin_trylock(&cpu_add_remove_lock) )
-        {
-            ret = -EBUSY;
-            break;
-        }
+        spin_lock(&cpu_add_remove_lock);
 
         if ( (g_info->xen_cpuid >= NR_CPUS) ||
              (g_info->xen_cpuid < 0) ||

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 963bc24a525374c95771f33b73be216b188581cd..b6583fe4abe896f84a5de296c4fe784480802771 100644 (file)
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -1330,24 +1330,25 @@ static int take_cpu_down(void *unused)
        return __cpu_disable();
 }
 
+/*
+ * Protects against concurrent offline/online requests for a single CPU.
+ * We need this extra protection because cpu_down() cannot continuously hold
+ * the cpu_add_remove_lock, as it cannot be held across stop_machine_run().
+ */
+static cpumask_t cpu_offlining;
+
 int cpu_down(unsigned int cpu)
 {
        int err = 0;
 
-       /* spin_trylock() avoids deadlock with stop_machine_run(). */
-       if (!spin_trylock(&cpu_add_remove_lock))
-               return -EBUSY;
+       spin_lock(&cpu_add_remove_lock);
 
-       /* Can not offline BSP */
-       if (cpu == 0) {
-               err = -EINVAL;
-               goto out;
+       if ((cpu == 0) || !cpu_online(cpu) || cpu_isset(cpu, cpu_offlining)) {
+               spin_unlock(&cpu_add_remove_lock);
+               return -EINVAL;
        }
 
-       if (!cpu_online(cpu)) {
-               err = -EINVAL;
-               goto out;
-       }
+       cpu_set(cpu, cpu_offlining);
 
        err = cpupool_cpu_remove(cpu);
        if (err)
@@ -1357,14 +1358,16 @@ int cpu_down(unsigned int cpu)
 
        cpufreq_del_cpu(cpu);
 
+       spin_unlock(&cpu_add_remove_lock);
        err = stop_machine_run(take_cpu_down, NULL, cpu);
+       spin_lock(&cpu_add_remove_lock);
+
        if (err < 0) {
                cpupool_cpu_add(cpu);
                goto out;
        }
 
        __cpu_die(cpu);
-
        BUG_ON(cpu_online(cpu));
 
        migrate_tasklets_from_cpu(cpu);
@@ -1373,6 +1376,7 @@ int cpu_down(unsigned int cpu)
 out:
        if (!err)
                send_guest_global_virq(dom0, VIRQ_PCPU_STATE);
+       cpu_clear(cpu, cpu_offlining);
        spin_unlock(&cpu_add_remove_lock);
        return err;
 }
@@ -1381,13 +1385,10 @@ int cpu_up(unsigned int cpu)
 {
        int err = 0;
 
-       /* spin_trylock() avoids deadlock with stop_machine_run(). */
-       if (!spin_trylock(&cpu_add_remove_lock))
-           return -EBUSY;
+       spin_lock(&cpu_add_remove_lock);
 
-       if (cpu_online(cpu)) {
-               printk("Bring up a online cpu. Bogus!\n");
-               err = -EBUSY;
+       if (cpu_online(cpu) || cpu_isset(cpu, cpu_offlining)) {
+               err = -EINVAL;
                goto out;
        }
 
@@ -1485,9 +1486,7 @@ int cpu_add(uint32_t apic_id, uint32_t acpi_id, uint32_t pxm)
        if ( physid_isset(apic_id, phys_cpu_present_map) )
                return -EEXIST;
 
-       /* spin_trylock() avoids deadlock with stop_machine_run(). */
-       if (!spin_trylock(&cpu_add_remove_lock))
-               return -EBUSY;
+       spin_lock(&cpu_add_remove_lock);
 
        cpu = mp_register_lapic(apic_id, 1);