[Xen-devel] [PATCH 2/2] smp: convert cpu_hotplug_begin into a blocking lock acquisition

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

Don't allow cpu_hotplug_begin to fail by converting the trylock into a
blocking lock acquisition. Write users of the cpu_add_remove_lock are
limited to CPU plug/unplug operations, and cannot deadlock between
themselves or other users taking the lock in read mode as
cpu_add_remove_lock is always locked with interrupts enabled. There
are also no other locks taken during the plug/unplug operations.

The exclusive lock usage in register_cpu_notifier is also converted
into a blocking lock acquisition, as it was previously not allowed to
fail anyway.

This is meaningful when running Xen in shim mode, since VCPU_{up/down}
hypercalls use cpu hotplug/unplug operations in the background, and
hence failing to take the lock results in VPCU_{up/down} failing with
-EBUSY, which most users are not prepared to handle.

Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
I've tested this and seems to work fine AFAICT either when running on
native or when used in the shim. I'm not sure if I'm missing something
that would prevent the write lock acquisition from being made
blocking.
---
 xen/arch/x86/smpboot.c |  3 +--
 xen/common/cpu.c       | 13 +++++--------
 xen/include/xen/cpu.h  |  2 +-
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 67ee490f94..cc0d62f9e2 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -1286,8 +1286,7 @@ int cpu_add(uint32_t apic_id, uint32_t acpi_id, uint32_t 
pxm)
          (pxm >= 256) )
         return -EINVAL;
 
-    if ( !cpu_hotplug_begin() )
-        return -EBUSY;
+    cpu_hotplug_begin();
 
     /* Detect if the cpu has been added before */
     if ( x86_acpiid_to_apicid[acpi_id] != BAD_APICID )
diff --git a/xen/common/cpu.c b/xen/common/cpu.c
index 0d7a10878c..31953f32e4 100644
--- a/xen/common/cpu.c
+++ b/xen/common/cpu.c
@@ -51,9 +51,9 @@ void put_cpu_maps(void)
     read_unlock(&cpu_add_remove_lock);
 }
 
-bool cpu_hotplug_begin(void)
+void cpu_hotplug_begin(void)
 {
-    return write_trylock(&cpu_add_remove_lock);
+    write_lock(&cpu_add_remove_lock);
 }
 
 void cpu_hotplug_done(void)
@@ -65,8 +65,7 @@ static NOTIFIER_HEAD(cpu_chain);
 
 void __init register_cpu_notifier(struct notifier_block *nb)
 {
-    if ( !write_trylock(&cpu_add_remove_lock) )
-        BUG(); /* Should never fail as we are called only during boot. */
+    write_lock(&cpu_add_remove_lock);
     notifier_chain_register(&cpu_chain, nb);
     write_unlock(&cpu_add_remove_lock);
 }
@@ -100,8 +99,7 @@ int cpu_down(unsigned int cpu)
     int err;
     struct notifier_block *nb = NULL;
 
-    if ( !cpu_hotplug_begin() )
-        return -EBUSY;
+    cpu_hotplug_begin();
 
     err = -EINVAL;
     if ( (cpu >= nr_cpu_ids) || (cpu == 0) )
@@ -142,8 +140,7 @@ int cpu_up(unsigned int cpu)
     int err;
     struct notifier_block *nb = NULL;
 
-    if ( !cpu_hotplug_begin() )
-        return -EBUSY;
+    cpu_hotplug_begin();
 
     err = -EINVAL;
     if ( (cpu >= nr_cpu_ids) || !cpu_present(cpu) )
diff --git a/xen/include/xen/cpu.h b/xen/include/xen/cpu.h
index e49172f64c..e8eeb217a0 100644
--- a/xen/include/xen/cpu.h
+++ b/xen/include/xen/cpu.h
@@ -10,7 +10,7 @@ bool get_cpu_maps(void);
 void put_cpu_maps(void);
 
 /* Safely perform CPU hotplug and update cpu_online_map, etc. */
-bool cpu_hotplug_begin(void);
+void cpu_hotplug_begin(void);
 void cpu_hotplug_done(void);
 
 /* Receive notification of CPU hotplug events. */
-- 
2.25.0


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel