[Xen-changelog] [xen-unstable] x86: Do no thold cpu_add_remove_lock across stop_machine_run().

[Xen patchbot-unstable <patchbot-unstable@xxxxxxxxxxxxxxxxxxx>]

# HG changeset patch
# User Keir Fraser <keir.fraser@xxxxxxxxxx>
# Date 1273833555 -3600
# Node ID 578375084a9ee1063f4bc5a8e64e59677804cdb9
# Parent  9ff084bae83a1506f731ff6a926fefcccec5b1b1
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@xxxxxxxxxx>
---
 xen/arch/x86/platform_hypercall.c |    7 -----
 xen/arch/x86/smpboot.c            |   49 ++++++++++++++++++--------------------
 2 files changed, 25 insertions(+), 31 deletions(-)

diff -r 9ff084bae83a -r 578375084a9e xen/arch/x86/platform_hypercall.c
--- a/xen/arch/x86/platform_hypercall.c Fri May 14 10:13:30 2010 +0100
+++ b/xen/arch/x86/platform_hypercall.c Fri May 14 11:39:15 2010 +0100
@@ -409,12 +409,7 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
 
         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 -r 9ff084bae83a -r 578375084a9e xen/arch/x86/smpboot.c
--- a/xen/arch/x86/smpboot.c    Fri May 14 10:13:30 2010 +0100
+++ b/xen/arch/x86/smpboot.c    Fri May 14 11:39:15 2010 +0100
@@ -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;
-
-       /* Can not offline BSP */
-       if (cpu == 0) {
-               err = -EINVAL;
-               goto out;
-       }
-
-       if (!cpu_online(cpu)) {
-               err = -EINVAL;
-               goto out;
-       }
+       spin_lock(&cpu_add_remove_lock);
+
+       if ((cpu == 0) || !cpu_online(cpu) || cpu_isset(cpu, cpu_offlining)) {
+               spin_unlock(&cpu_add_remove_lock);
+               return -EINVAL;
+       }
+
+       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 @@ out:
 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;
-
-       if (cpu_online(cpu)) {
-               printk("Bring up a online cpu. Bogus!\n");
-               err = -EBUSY;
+       spin_lock(&cpu_add_remove_lock);
+
+       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 a
        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);
 

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog