Jiang, Yunhong
2010-Mar-22 06:23 UTC
[Xen-devel] [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@intel.com> 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 ) { _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel