WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] Not spin on the locks that be potentially held by st

To: Keir Fraser <keir.fraser@xxxxxxxxxxxxx>
Subject: [Xen-devel] [PATCH] Not spin on the locks that be potentially held by stop_machine_run
From: "Jiang, Yunhong" <yunhong.jiang@xxxxxxxxx>
Date: Mon, 22 Mar 2010 14:23:26 +0800
Accept-language: en-US
Acceptlanguage: en-US
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Sun, 21 Mar 2010 23:24:32 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
Thread-index: AcrJiC6fI4JuKqj4RqiE8qIzAnnhkQ==
Thread-topic: [PATCH] Not spin on the locks that be potentially held by stop_machine_run
Not spin on the locks that be potentially held by stop_machine_run

Currently stop_machine_run() will try to bring all CPUs to softirq context, 
with some locks held, like xenpf_lock or cpu_add_remove_lock etc. However, if 
another CPU is trying to get these locks, it may cause deadlock.

This patch replace all such spin_lock with spin_trylock. For xenpf_lock and 
sysctl_lock, we try to use hypercall_continuation, so that we will not cause 
trouble to user space tools. For cpu_hot_remove_lock, we simply return EBUSY if 
failure, since it will only impact small number of user space tools.

In the end, we should try to make the stop_machine_run as spinlock free.

Signed-off-by: Jiang, Yunhong <yunhong.jiang@xxxxxxxxx>

diff -r 2a31c8c8c7bd xen/arch/x86/platform_hypercall.c
--- a/xen/arch/x86/platform_hypercall.c Mon Mar 22 13:58:19 2010 +0800
+++ b/xen/arch/x86/platform_hypercall.c Mon Mar 22 13:58:21 2010 +0800
@@ -73,7 +73,10 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
     if ( op->interface_version != XENPF_INTERFACE_VERSION )
         return -EACCES;
 
-    spin_lock(&xenpf_lock);
+    while ( !spin_trylock(&xenpf_lock) )
+        if ( hypercall_preempt_check() )
+            return hypercall_create_continuation(
+                __HYPERVISOR_platform_op, "h", u_xenpf_op);
 
     switch ( op->cmd )
     {
@@ -398,7 +401,11 @@ ret_t do_platform_op(XEN_GUEST_HANDLE(xe
 
         g_info = &op->u.pcpu_info;
 
-        spin_lock(&cpu_add_remove_lock);
+        if (!spin_trylock(&cpu_add_remove_lock))
+        {
+            ret = -EBUSY;
+            break;
+        }
 
         if ( (g_info->xen_cpuid >= NR_CPUS) ||
              (g_info->xen_cpuid < 0) ||
diff -r 2a31c8c8c7bd xen/arch/x86/smpboot.c
--- a/xen/arch/x86/smpboot.c    Mon Mar 22 13:58:19 2010 +0800
+++ b/xen/arch/x86/smpboot.c    Mon Mar 22 14:06:20 2010 +0800
@@ -1340,7 +1340,11 @@ int cpu_down(unsigned int cpu)
 {
        int err = 0;
 
-       spin_lock(&cpu_add_remove_lock);
+       if ( !spin_trylock(&cpu_add_remove_lock) ) {
+               err = -EBUSY;
+               goto out;
+       }
+
        if (num_online_cpus() == 1) {
                err = -EBUSY;
                goto out;
@@ -1382,7 +1386,9 @@ int cpu_up(unsigned int cpu)
 {
        int err = 0;
 
-       spin_lock(&cpu_add_remove_lock);
+       if ( !spin_trylock(&cpu_add_remove_lock) )
+           return -EBUSY;
+
        if (cpu_online(cpu)) {
                printk("Bring up a online cpu. Bogus!\n");
                err = -EBUSY;
@@ -1417,6 +1423,8 @@ void disable_nonboot_cpus(void)
                if (cpu == 0)
                        continue;
                error = cpu_down(cpu);
+               /* No need to check EBUSY here */
+               ASSERT(error != -EBUSY);
                if (!error) {
                        cpu_set(cpu, frozen_cpus);
                        printk("CPU%d is down\n", cpu);
@@ -1437,6 +1445,8 @@ void enable_nonboot_cpus(void)
        mtrr_aps_sync_begin();
        for_each_cpu_mask(cpu, frozen_cpus) {
                error = cpu_up(cpu);
+               /* No conflict will happen here */
+               ASSERT(error != -EBUSY);
                if (!error) {
                        printk("CPU%d is up\n", cpu);
                        continue;
@@ -1479,7 +1489,8 @@ int cpu_add(uint32_t apic_id, uint32_t a
        if ( physid_isset(apic_id, phys_cpu_present_map) )
                return -EEXIST;
 
-       spin_lock(&cpu_add_remove_lock);
+       if ( !spin_trylock(&cpu_add_remove_lock))
+               return -EBUSY;
 
        cpu = mp_register_lapic(apic_id, 1);
 
diff -r 2a31c8c8c7bd xen/common/sysctl.c
--- a/xen/common/sysctl.c       Mon Mar 22 13:58:19 2010 +0800
+++ b/xen/common/sysctl.c       Mon Mar 22 13:58:21 2010 +0800
@@ -48,7 +48,10 @@ long do_sysctl(XEN_GUEST_HANDLE(xen_sysc
     if ( op->interface_version != XEN_SYSCTL_INTERFACE_VERSION )
         return -EACCES;
 
-    spin_lock(&sysctl_lock);
+    while ( !spin_trylock(&sysctl_lock) )
+        if (hypercall_preempt_check() )
+            return hypercall_create_continuation(
+                __HYPERVISOR_sysctl, "h", u_sysctl);
 
     switch ( op->cmd )
     {


Attachment: stop_machine_lock.patch
Description: stop_machine_lock.patch

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>
  • [Xen-devel] [PATCH] Not spin on the locks that be potentially held by stop_machine_run, Jiang, Yunhong <=