lkml.org 
[lkml]   [2018]   [Sep]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.18 166/235] sched/core: Use smp_mb() in wake_woken_function()
    Date
    4.18-stable review patch.  If anyone has any objections, please let me know.

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

    From: Andrea Parri <andrea.parri@amarulasolutions.com>

    [ Upstream commit 76e079fefc8f62bd9b2cd2950814d1ee806e31a5 ]

    wake_woken_function() synchronizes with wait_woken() as follows:

    [wait_woken] [wake_woken_function]

    entry->flags &= ~wq_flag_woken; condition = true;
    smp_mb(); smp_wmb();
    if (condition) wq_entry->flags |= wq_flag_woken;
    break;

    This commit replaces the above smp_wmb() with an smp_mb() in order to
    guarantee that either wait_woken() sees the wait condition being true
    or the store to wq_entry->flags in woken_wake_function() follows the
    store in wait_woken() in the coherence order (so that the former can
    eventually be observed by wait_woken()).

    The commit also fixes a comment associated to set_current_state() in
    wait_woken(): the comment pairs the barrier in set_current_state() to
    the above smp_wmb(), while the actual pairing involves the barrier in
    set_current_state() and the barrier executed by the try_to_wake_up()
    in wake_woken_function().

    Signed-off-by: Andrea Parri <andrea.parri@amarulasolutions.com>
    Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
    Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Thomas Gleixner <tglx@linutronix.de>
    Cc: akiyks@gmail.com
    Cc: boqun.feng@gmail.com
    Cc: dhowells@redhat.com
    Cc: j.alglave@ucl.ac.uk
    Cc: linux-arch@vger.kernel.org
    Cc: luc.maranget@inria.fr
    Cc: npiggin@gmail.com
    Cc: parri.andrea@gmail.com
    Cc: stern@rowland.harvard.edu
    Cc: will.deacon@arm.com
    Link: http://lkml.kernel.org/r/20180716180605.16115-10-paulmck@linux.vnet.ibm.com
    Signed-off-by: Ingo Molnar <mingo@kernel.org>
    Signed-off-by: Sasha Levin <alexander.levin@microsoft.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
    ---
    kernel/sched/wait.c | 47 +++++++++++++++++++++--------------------------
    1 file changed, 21 insertions(+), 26 deletions(-)

    --- a/kernel/sched/wait.c
    +++ b/kernel/sched/wait.c
    @@ -392,35 +392,36 @@ static inline bool is_kthread_should_sto
    * if (condition)
    * break;
    *
    - * p->state = mode; condition = true;
    - * smp_mb(); // A smp_wmb(); // C
    - * if (!wq_entry->flags & WQ_FLAG_WOKEN) wq_entry->flags |= WQ_FLAG_WOKEN;
    - * schedule() try_to_wake_up();
    - * p->state = TASK_RUNNING; ~~~~~~~~~~~~~~~~~~
    - * wq_entry->flags &= ~WQ_FLAG_WOKEN; condition = true;
    - * smp_mb() // B smp_wmb(); // C
    - * wq_entry->flags |= WQ_FLAG_WOKEN;
    - * }
    - * remove_wait_queue(&wq_head, &wait);
    + * // in wait_woken() // in woken_wake_function()
    *
    + * p->state = mode; wq_entry->flags |= WQ_FLAG_WOKEN;
    + * smp_mb(); // A try_to_wake_up():
    + * if (!(wq_entry->flags & WQ_FLAG_WOKEN)) <full barrier>
    + * schedule() if (p->state & mode)
    + * p->state = TASK_RUNNING; p->state = TASK_RUNNING;
    + * wq_entry->flags &= ~WQ_FLAG_WOKEN; ~~~~~~~~~~~~~~~~~~
    + * smp_mb(); // B condition = true;
    + * } smp_mb(); // C
    + * remove_wait_queue(&wq_head, &wait); wq_entry->flags |= WQ_FLAG_WOKEN;
    */
    long wait_woken(struct wait_queue_entry *wq_entry, unsigned mode, long timeout)
    {
    - set_current_state(mode); /* A */
    /*
    - * The above implies an smp_mb(), which matches with the smp_wmb() from
    - * woken_wake_function() such that if we observe WQ_FLAG_WOKEN we must
    - * also observe all state before the wakeup.
    + * The below executes an smp_mb(), which matches with the full barrier
    + * executed by the try_to_wake_up() in woken_wake_function() such that
    + * either we see the store to wq_entry->flags in woken_wake_function()
    + * or woken_wake_function() sees our store to current->state.
    */
    + set_current_state(mode); /* A */
    if (!(wq_entry->flags & WQ_FLAG_WOKEN) && !is_kthread_should_stop())
    timeout = schedule_timeout(timeout);
    __set_current_state(TASK_RUNNING);

    /*
    - * The below implies an smp_mb(), it too pairs with the smp_wmb() from
    - * woken_wake_function() such that we must either observe the wait
    - * condition being true _OR_ WQ_FLAG_WOKEN such that we will not miss
    - * an event.
    + * The below executes an smp_mb(), which matches with the smp_mb() (C)
    + * in woken_wake_function() such that either we see the wait condition
    + * being true or the store to wq_entry->flags in woken_wake_function()
    + * follows ours in the coherence order.
    */
    smp_store_mb(wq_entry->flags, wq_entry->flags & ~WQ_FLAG_WOKEN); /* B */

    @@ -430,14 +431,8 @@ EXPORT_SYMBOL(wait_woken);

    int woken_wake_function(struct wait_queue_entry *wq_entry, unsigned mode, int sync, void *key)
    {
    - /*
    - * Although this function is called under waitqueue lock, LOCK
    - * doesn't imply write barrier and the users expects write
    - * barrier semantics on wakeup functions. The following
    - * smp_wmb() is equivalent to smp_wmb() in try_to_wake_up()
    - * and is paired with smp_store_mb() in wait_woken().
    - */
    - smp_wmb(); /* C */
    + /* Pairs with the smp_store_mb() in wait_woken(). */
    + smp_mb(); /* C */
    wq_entry->flags |= WQ_FLAG_WOKEN;

    return default_wake_function(wq_entry, mode, sync, key);

    \
     
     \ /
      Last update: 2018-09-24 14:49    [W:2.624 / U:0.172 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site