lkml.org 
[lkml]   [2018]   [Sep]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 103/165] stop_machine: Reflow cpu_stop_queue_two_works()
    Date
    4.14-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    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;
    }

    \
     
     \ /
      Last update: 2018-09-03 19:31    [W:3.704 / U:0.212 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site