stop_machine: Reflow cpu_stop_queue_two_works()

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>

diff --git a/kernel/stop_machine.c b/kernel/stop_machine.c
index e190d1ef3a23b005279fb3e950c0db355895eeec..34b6652e867796ab89919d77d70e40a24d5f8498 100644 (file)
--- a/kernel/stop_machine.c
+++ b/kernel/stop_machine.c
@@ -236,13 +236,24 @@ static int cpu_stop_queue_two_works(int cpu1, struct cpu_stop_work *work1,
        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;
 }