Commit 76e079fe authored by Andrea Parri's avatar Andrea Parri Committed by Ingo Molnar
Browse files

sched/core: Use smp_mb() in wake_woken_function() 



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: default avatarAndrea Parri <andrea.parri@amarulasolutions.com>
Signed-off-by: default avatarPaul E. McKenney <paulmck@linux.vnet.ibm.com>
Acked-by: default avatarPeter 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: default avatarIngo Molnar <mingo@kernel.org>
parent 70b83069

kernel/sched/wait.c

+21 −26
Original line number Diff line number Diff line
@@ -392,35 +392,36 @@ static inline bool is_kthread_should_stop(void) 
 *     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);

CRA Git | Maintained and supported by SUSTech CRA and CCSE