Date:	Tue,  8 Sep 2015 09:14:01 +0800
From:	Boqun Feng <boqun.feng@...il.com>
To:	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org
Cc:	"Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
	Jonathan Corbet <corbet@....net>,
	Michal Hocko <mhocko@...nel.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	David Howells <dhowells@...hat.com>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Boqun Feng <boqun.feng@...il.com>
Subject: [PATCH] Documentation: Remove misleading examples of the barriers in wake_*()

Two examples for barriers in wake_up() and co. in memory-barriers.txt
are misleading, along with their explanations:

1.	The example which wanted to explain the write barrier in
	wake_up() and co. [spotted by Oleg Nesterov <oleg@...hat.com>]

2.	The example which wanted to explain that the write barriers in
	wake_up() and co. only exist iff a wakeup actually occurs.

For example #1, according to Oleg Nesterov:

>
>       The barrier occurs before the task state is cleared
>
> is not actually right. This is misleading. What is really important is that
> we have a barrier before we _read_ the task state. And again, again, the
> fact that we actually have the write barrier is just the implementation
> detail.
>

And the example #2 is actually an example which could explain that the
barriers in wait_event() and co. only exist iff a sleep actually occurs.

Further more, these barriers are only used for the correctness of
sleeping and waking up, i.e. they exist only to guarantee the ordering
of memory accesses to the task states and the global variables
indicating an event. Users can't rely on them for other things, so
memory-barriers.txt had better to call this out and remove the
misleading examples.

This patch removes the misleading examples along with their
explanations, calls it out that those implied barriers are only for
sleep and wakeup related variables and adds a new example to explain the
implied barrier in wake_up() and co.

Signed-off-by: Boqun Feng <boqun.feng@...il.com>
---
 Documentation/memory-barriers.txt | 42 +++++++++++++++++----------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
index eafa6a5..07de72f 100644
--- a/Documentation/memory-barriers.txt
+++ b/Documentation/memory-barriers.txt
@@ -1948,6 +1948,10 @@ these appear to happen in the right order, the primitives to begin the process
 of going to sleep, and the primitives to initiate a wake up imply certain
 barriers.
 
+[!] Note that these implied barriers are only for the correctness of sleep and
+wake-up. So don't rely on these barriers for things that are neither the task
+states nor the global variables indicating the events.
+
 Firstly, the sleeper normally follows something like this sequence of events:
 
 	for (;;) {
@@ -1997,32 +2001,22 @@ or:
 	event_indicated = 1;
 	wake_up_process(event_daemon);
 
-A write memory barrier is implied by wake_up() and co. if and only if they wake
-something up.  The barrier occurs before the task state is cleared, and so sits
-between the STORE to indicate the event and the STORE to set TASK_RUNNING:
-
-	CPU 1				CPU 2
-	===============================	===============================
-	set_current_state();		STORE event_indicated
-	  smp_store_mb();		wake_up();
-	    STORE current->state	  <write barrier>
-	    <general barrier>		  STORE current->state
-	LOAD event_indicated
+A memory barrier is implied by wake_up() and co. if and only if they wake
+something up. The memory barrier here is not necessary to be a general barrier,
+it only needs to guarantee a STORE preceding this barrier can never be
+reordered after a LOAD following this barrier(i.e. a STORE-LOAD barrier). This
+barrier guarantees that the event has been indicated before the waker read the
+wakee's task state:
 
-To repeat, this write memory barrier is present if and only if something
-is actually awakened.  To see this, consider the following sequence of
-events, where X and Y are both initially zero:
+	CPU 1
+	===============================
+	STORE event_indicated;
+	wake_up_process(wakee);
+	  <STORE-LOAD barrier>
+	  LOAD wakee->state;
 
-	CPU 1				CPU 2
-	===============================	===============================
-	X = 1;				STORE event_indicated
-	smp_mb();			wake_up();
-	Y = 1;				wait_event(wq, Y == 1);
-	wake_up();			  load from Y sees 1, no memory barrier
-					load from X might see 0
-
-In contrast, if a wakeup does occur, CPU 2's load from X would be guaranteed
-to see 1.
+This barrier pairs with the general barrier implied by set_current_state() on
+the sleeper side.
 
 The available waker functions include:
 
-- 
2.5.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists