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 Signed-off-by: Peter Zijlstra (Intel) --- 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 #include #include -#include /* * 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/