We can replace both the global and local part of the lglock by better
usage of cpu_stopper::lock.

By having stop_two_cpus() acquire two cpu_stopper::locks we gain full
order against the global stop_machine which takes each of these locks
in order.

Cc: Rik van Riel <r...@redhat.com>
Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org>
---
 kernel/stop_machine.c |   52 ++++++++++++++++++++++++++++----------------------
 lib/Kconfig           |    5 ++++
 2 files changed, 35 insertions(+), 22 deletions(-)

--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -20,7 +20,6 @@
 #include <linux/kallsyms.h>
 #include <linux/smpboot.h>
 #include <linux/atomic.h>
-#include <linux/lglock.h>
 
 /*
  * Structure to determine completion condition and record errors.  May
@@ -44,14 +43,6 @@ static DEFINE_PER_CPU(struct cpu_stopper
 static DEFINE_PER_CPU(struct task_struct *, cpu_stopper_task);
 static bool stop_machine_initialized = false;
 
-/*
- * Avoids a race between stop_two_cpus and global stop_cpus, where
- * the stoppers could get queued up in reverse order, leading to
- * system deadlock. Using an lglock means stop_two_cpus remains
- * relatively cheap.
- */
-DEFINE_STATIC_LGLOCK(stop_cpus_lock);
-
 static void cpu_stop_init_done(struct cpu_stop_done *done, unsigned int 
nr_todo)
 {
        memset(done, 0, sizeof(*done));
@@ -71,21 +62,26 @@ static void cpu_stop_signal_done(struct
 }
 
 /* queue @work to @stopper.  if offline, @work is completed immediately */
-static void cpu_stop_queue_work(unsigned int cpu, struct cpu_stop_work *work)
+static void __cpu_stop_queue_work(unsigned int cpu, struct cpu_stop_work *work)
 {
        struct cpu_stopper *stopper = &per_cpu(cpu_stopper, cpu);
        struct task_struct *p = per_cpu(cpu_stopper_task, cpu);
 
-       unsigned long flags;
-
-       spin_lock_irqsave(&stopper->lock, flags);
-
        if (stopper->enabled) {
                list_add_tail(&work->list, &stopper->works);
                wake_up_process(p);
-       } else
+       } else {
                cpu_stop_signal_done(work->done, false);
+       }
+}
 
+static void cpu_stop_queue_work(unsigned int cpu, struct cpu_stop_work *work)
+{
+       struct cpu_stopper *stopper = &per_cpu(cpu_stopper, cpu);
+       unsigned long flags;
+
+       spin_lock_irqsave(&stopper->lock, flags);
+       __cpu_stop_queue_work(cpu, work);
        spin_unlock_irqrestore(&stopper->lock, flags);
 }
 
@@ -224,9 +220,14 @@ static int multi_cpu_stop(void *data)
  */
 int stop_two_cpus(unsigned int cpu1, unsigned int cpu2, cpu_stop_fn_t fn, void 
*arg)
 {
-       struct cpu_stop_done done;
+       struct cpu_stopper *stopper1, *stopper2;
        struct cpu_stop_work work1, work2;
        struct multi_stop_data msdata;
+       struct cpu_stop_done done;
+       unsigned long flags;
+
+       if (cpu2 < cpu1)
+               swap(cpu1, cpu2);
 
        preempt_disable();
        msdata = (struct multi_stop_data){
@@ -258,10 +259,17 @@ int stop_two_cpus(unsigned int cpu1, uns
                return -ENOENT;
        }
 
-       lg_double_lock(&stop_cpus_lock, cpu1, cpu2);
-       cpu_stop_queue_work(cpu1, &work1);
-       cpu_stop_queue_work(cpu2, &work2);
-       lg_double_unlock(&stop_cpus_lock, cpu1, cpu2);
+       stopper1 = per_cpu_ptr(&cpu_stopper, cpu1);
+       stopper2 = per_cpu_ptr(&cpu_stopper, cpu2);
+
+       spin_lock_irqsave(&stopper1->lock, flags);
+       spin_lock(&stopper2->lock);
+
+       __cpu_stop_queue_work(cpu1, &work1);
+       __cpu_stop_queue_work(cpu2, &work2);
+
+       spin_unlock(&stopper2->lock);
+       spin_unlock_irqrestore(&stopper1->lock, flags);
 
        preempt_enable();
 
@@ -315,10 +323,10 @@ static void queue_stop_cpus_work(const s
         * preempted by a stopper which might wait for other stoppers
         * to enter @fn which can lead to deadlock.
         */
-       lg_global_lock(&stop_cpus_lock);
+       preempt_disable();
        for_each_cpu(cpu, cpumask)
                cpu_stop_queue_work(cpu, &per_cpu(stop_cpus_work, cpu));
-       lg_global_unlock(&stop_cpus_lock);
+       preempt_enable();
 }
 
 static int __stop_cpus(const struct cpumask *cpumask,
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -61,6 +61,11 @@ config PERCPU_RWSEM_HOTPLUG
        depends on HOTPLUG_CPU
        select PERCPU_RWSEM
 
+config PERCPU_RWSEM_SMP
+       def_bool y
+       depends on SMP
+       select PERCPU_RWSEM
+
 config ARCH_USE_CMPXCHG_LOCKREF
        bool
 


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to