diff options
| author | Peter Zijlstra <peterz@infradead.org> | 2018-04-20 05:50:05 -0400 |
|---|---|---|
| committer | Ingo Molnar <mingo@kernel.org> | 2018-05-03 01:38:03 -0400 |
| commit | 0b26351b910fb8fe6a056f8a1bbccabe50c0e19f (patch) | |
| tree | 1e3f9c791212016ca5caf15e8200f1581ecaac32 /kernel/stop_machine.c | |
| parent | f4ef6a438cee86ca0c6d1b889aa53bec9c1f9de6 (diff) | |
stop_machine, sched: Fix migrate_swap() vs. active_balance() deadlock
Matt reported the following deadlock:
CPU0 CPU1
schedule(.prev=migrate/0) <fault>
pick_next_task() ...
idle_balance() migrate_swap()
active_balance() stop_two_cpus()
spin_lock(stopper0->lock)
spin_lock(stopper1->lock)
ttwu(migrate/0)
smp_cond_load_acquire() -- waits for schedule()
stop_one_cpu(1)
spin_lock(stopper1->lock) -- waits for stopper lock
Fix this deadlock by taking the wakeups out from under stopper->lock.
This allows the active_balance() to queue the stop work and finish the
context switch, which in turn allows the wakeup from migrate_swap() to
observe the context and complete the wakeup.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reported-by: Matt Fleming <matt@codeblueprint.co.uk>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Acked-by: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Michal Hocko <mhocko@suse.com>
Cc: Mike Galbraith <umgwanakikbuti@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20180420095005.GH4064@hirez.programming.kicks-ass.net
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Diffstat (limited to 'kernel/stop_machine.c')
| -rw-r--r-- | kernel/stop_machine.c | 19 |
1 files changed, 14 insertions, 5 deletions
diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c index b7591261652d..64c0291b579c 100644 --- a/kernel/stop_machine.c +++ b/kernel/stop_machine.c | |||
| @@ -21,6 +21,7 @@ | |||
| 21 | #include <linux/smpboot.h> | 21 | #include <linux/smpboot.h> |
| 22 | #include <linux/atomic.h> | 22 | #include <linux/atomic.h> |
| 23 | #include <linux/nmi.h> | 23 | #include <linux/nmi.h> |
| 24 | #include <linux/sched/wake_q.h> | ||
| 24 | 25 | ||
| 25 | /* | 26 | /* |
| 26 | * Structure to determine completion condition and record errors. May | 27 | * Structure to determine completion condition and record errors. May |
| @@ -65,27 +66,31 @@ static void cpu_stop_signal_done(struct cpu_stop_done *done) | |||
| 65 | } | 66 | } |
| 66 | 67 | ||
| 67 | static void __cpu_stop_queue_work(struct cpu_stopper *stopper, | 68 | static void __cpu_stop_queue_work(struct cpu_stopper *stopper, |
| 68 | struct cpu_stop_work *work) | 69 | struct cpu_stop_work *work, |
| 70 | struct wake_q_head *wakeq) | ||
| 69 | { | 71 | { |
| 70 | list_add_tail(&work->list, &stopper->works); | 72 | list_add_tail(&work->list, &stopper->works); |
| 71 | wake_up_process(stopper->thread); | 73 | wake_q_add(wakeq, stopper->thread); |
| 72 | } | 74 | } |
| 73 | 75 | ||
| 74 | /* queue @work to @stopper. if offline, @work is completed immediately */ | 76 | /* queue @work to @stopper. if offline, @work is completed immediately */ |
| 75 | static bool cpu_stop_queue_work(unsigned int cpu, struct cpu_stop_work *work) | 77 | static bool cpu_stop_queue_work(unsigned int cpu, struct cpu_stop_work *work) |
| 76 | { | 78 | { |
| 77 | struct cpu_stopper *stopper = &per_cpu(cpu_stopper, cpu); | 79 | struct cpu_stopper *stopper = &per_cpu(cpu_stopper, cpu); |
| 80 | DEFINE_WAKE_Q(wakeq); | ||
| 78 | unsigned long flags; | 81 | unsigned long flags; |
| 79 | bool enabled; | 82 | bool enabled; |
| 80 | 83 | ||
| 81 | spin_lock_irqsave(&stopper->lock, flags); | 84 | spin_lock_irqsave(&stopper->lock, flags); |
| 82 | enabled = stopper->enabled; | 85 | enabled = stopper->enabled; |
| 83 | if (enabled) | 86 | if (enabled) |
| 84 | __cpu_stop_queue_work(stopper, work); | 87 | __cpu_stop_queue_work(stopper, work, &wakeq); |
| 85 | else if (work->done) | 88 | else if (work->done) |
| 86 | cpu_stop_signal_done(work->done); | 89 | cpu_stop_signal_done(work->done); |
| 87 | spin_unlock_irqrestore(&stopper->lock, flags); | 90 | spin_unlock_irqrestore(&stopper->lock, flags); |
| 88 | 91 | ||
| 92 | wake_up_q(&wakeq); | ||
| 93 | |||
| 89 | return enabled; | 94 | return enabled; |
| 90 | } | 95 | } |
| 91 | 96 | ||
| @@ -229,6 +234,7 @@ static int cpu_stop_queue_two_works(int cpu1, struct cpu_stop_work *work1, | |||
| 229 | { | 234 | { |
| 230 | struct cpu_stopper *stopper1 = per_cpu_ptr(&cpu_stopper, cpu1); | 235 | struct cpu_stopper *stopper1 = per_cpu_ptr(&cpu_stopper, cpu1); |
| 231 | struct cpu_stopper *stopper2 = per_cpu_ptr(&cpu_stopper, cpu2); | 236 | struct cpu_stopper *stopper2 = per_cpu_ptr(&cpu_stopper, cpu2); |
| 237 | DEFINE_WAKE_Q(wakeq); | ||
| 232 | int err; | 238 | int err; |
| 233 | retry: | 239 | retry: |
| 234 | spin_lock_irq(&stopper1->lock); | 240 | spin_lock_irq(&stopper1->lock); |
| @@ -252,8 +258,8 @@ retry: | |||
| 252 | goto unlock; | 258 | goto unlock; |
| 253 | 259 | ||
| 254 | err = 0; | 260 | err = 0; |
| 255 | __cpu_stop_queue_work(stopper1, work1); | 261 | __cpu_stop_queue_work(stopper1, work1, &wakeq); |
| 256 | __cpu_stop_queue_work(stopper2, work2); | 262 | __cpu_stop_queue_work(stopper2, work2, &wakeq); |
| 257 | unlock: | 263 | unlock: |
| 258 | spin_unlock(&stopper2->lock); | 264 | spin_unlock(&stopper2->lock); |
| 259 | spin_unlock_irq(&stopper1->lock); | 265 | spin_unlock_irq(&stopper1->lock); |
| @@ -263,6 +269,9 @@ unlock: | |||
| 263 | cpu_relax(); | 269 | cpu_relax(); |
| 264 | goto retry; | 270 | goto retry; |
| 265 | } | 271 | } |
| 272 | |||
| 273 | wake_up_q(&wakeq); | ||
| 274 | |||
| 266 | return err; | 275 | return err; |
| 267 | } | 276 | } |
| 268 | /** | 277 | /** |
