| From b80a2bfce85e1051056d98d04ecb2d0b55cbbc1c Mon Sep 17 00:00:00 2001 |
| From: Peter Zijlstra <peterz@infradead.org> |
| Date: Mon, 30 Jul 2018 13:21:40 +0200 |
| Subject: stop_machine: Reflow cpu_stop_queue_two_works() |
| |
| From: Peter Zijlstra <peterz@infradead.org> |
| |
| commit b80a2bfce85e1051056d98d04ecb2d0b55cbbc1c upstream. |
| |
| The code flow in cpu_stop_queue_two_works() is a little arcane; fix this by |
| lifting the preempt_disable() to the top to create more natural nesting wrt |
| the spinlocks and make the wake_up_q() and preempt_enable() unconditional |
| at the end. |
| |
| Furthermore, enable preemption in the -EDEADLK case, such that we spin-wait |
| with preemption enabled. |
| |
| Suggested-by: Thomas Gleixner <tglx@linutronix.de> |
| Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> |
| Signed-off-by: Thomas Gleixner <tglx@linutronix.de> |
| Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de> |
| Cc: isaacm@codeaurora.org |
| Cc: matt@codeblueprint.co.uk |
| Cc: psodagud@codeaurora.org |
| Cc: gregkh@linuxfoundation.org |
| Cc: pkondeti@codeaurora.org |
| Cc: stable@vger.kernel.org |
| Link: https://lkml.kernel.org/r/20180730112140.GH2494@hirez.programming.kicks-ass.net |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| kernel/stop_machine.c | 41 +++++++++++++++++++++++------------------ |
| 1 file changed, 23 insertions(+), 18 deletions(-) |
| |
| --- a/kernel/stop_machine.c |
| +++ b/kernel/stop_machine.c |
| @@ -236,13 +236,24 @@ static int cpu_stop_queue_two_works(int |
| struct cpu_stopper *stopper2 = per_cpu_ptr(&cpu_stopper, cpu2); |
| DEFINE_WAKE_Q(wakeq); |
| int err; |
| + |
| retry: |
| + /* |
| + * The waking up of stopper threads has to happen in the same |
| + * scheduling context as the queueing. Otherwise, there is a |
| + * possibility of one of the above stoppers being woken up by another |
| + * CPU, and preempting us. This will cause us to not wake up the other |
| + * stopper forever. |
| + */ |
| + preempt_disable(); |
| raw_spin_lock_irq(&stopper1->lock); |
| raw_spin_lock_nested(&stopper2->lock, SINGLE_DEPTH_NESTING); |
| |
| - err = -ENOENT; |
| - if (!stopper1->enabled || !stopper2->enabled) |
| + if (!stopper1->enabled || !stopper2->enabled) { |
| + err = -ENOENT; |
| goto unlock; |
| + } |
| + |
| /* |
| * Ensure that if we race with __stop_cpus() the stoppers won't get |
| * queued up in reverse order leading to system deadlock. |
| @@ -253,36 +264,30 @@ retry: |
| * It can be falsely true but it is safe to spin until it is cleared, |
| * queue_stop_cpus_work() does everything under preempt_disable(). |
| */ |
| - err = -EDEADLK; |
| - if (unlikely(stop_cpus_in_progress)) |
| - goto unlock; |
| + if (unlikely(stop_cpus_in_progress)) { |
| + err = -EDEADLK; |
| + goto unlock; |
| + } |
| |
| err = 0; |
| __cpu_stop_queue_work(stopper1, work1, &wakeq); |
| __cpu_stop_queue_work(stopper2, work2, &wakeq); |
| - /* |
| - * The waking up of stopper threads has to happen |
| - * in the same scheduling context as the queueing. |
| - * Otherwise, there is a possibility of one of the |
| - * above stoppers being woken up by another CPU, |
| - * and preempting us. This will cause us to n ot |
| - * wake up the other stopper forever. |
| - */ |
| - preempt_disable(); |
| + |
| unlock: |
| raw_spin_unlock(&stopper2->lock); |
| raw_spin_unlock_irq(&stopper1->lock); |
| |
| if (unlikely(err == -EDEADLK)) { |
| + preempt_enable(); |
| + |
| while (stop_cpus_in_progress) |
| cpu_relax(); |
| + |
| goto retry; |
| } |
| |
| - if (!err) { |
| - wake_up_q(&wakeq); |
| - preempt_enable(); |
| - } |
| + wake_up_q(&wakeq); |
| + preempt_enable(); |
| |
| return err; |
| } |