From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753515AbbIJCQi (ORCPT ); Wed, 9 Sep 2015 22:16:38 -0400 Received: from mail-oi0-f50.google.com ([209.85.218.50]:33079 "EHLO mail-oi0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752202AbbIJCQd (ORCPT ); Wed, 9 Sep 2015 22:16:33 -0400 Date: Thu, 10 Sep 2015 10:16:12 +0800 From: Boqun Feng To: "Paul E. McKenney" Cc: linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, Jonathan Corbet , Michal Hocko , Oleg Nesterov , Peter Zijlstra , David Howells , Linus Torvalds Subject: Re: [PATCH] Documentation: Remove misleading examples of the barriers in wake_*() Message-ID: <20150910021612.GC18494@fixme-laptop.cn.ibm.com> References: <1441674841-11498-1-git-send-email-boqun.feng@gmail.com> <20150909192822.GM4029@linux.vnet.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="JgQwtEuHJzHdouWu" Content-Disposition: inline In-Reply-To: <20150909192822.GM4029@linux.vnet.ibm.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --JgQwtEuHJzHdouWu Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Wed, Sep 09, 2015 at 12:28:22PM -0700, Paul E. McKenney wrote: > On Tue, Sep 08, 2015 at 09:14:01AM +0800, Boqun Feng wrote: > > Two examples for barriers in wake_up() and co. in memory-barriers.txt > > are misleading, along with their explanations: > >=20 > > 1. The example which wanted to explain the write barrier in > > wake_up() and co. [spotted by Oleg Nesterov ] > >=20 > > 2. The example which wanted to explain that the write barriers in > > wake_up() and co. only exist iff a wakeup actually occurs. > >=20 > > For example #1, according to Oleg Nesterov: > >=20 > > > > > > The barrier occurs before the task state is cleared > > > > > > is not actually right. This is misleading. What is really important i= s 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 implementati= on > > > detail. > > > > >=20 > > 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. > >=20 > > 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. > >=20 > > 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. > >=20 > > Signed-off-by: Boqun Feng >=20 > At this point, I would favor replacing that entire section with a short > paragraph describing what guarantees are provided, perhaps with an example > showing what added barriers/locks/whatever are required. My feeling is > that we should avoid saying too much about the internals of wait_event() > and wake_up(). Good idea! However, I think a little more words help understand. So I keep the original first paragraph and also add a paragraph for NOTE which, I think, may be a little redundant although ;-) How about the following new whole section? SLEEP AND WAKE-UP FUNCTIONS --------------------------- Sleeping and waking on an event flagged in global data can be viewed as an interaction between two pieces of data: the task state of the task waiting = for the event and the global data used to indicate the event. To make sure that these appear to happen in the right order, the primitives to begin the proc= ess of going to sleep, and the primitives to initiate a wake up imply certain barriers. The memory ordering requirement here can be expressed by two STORE-LOAD barriers(barriers which can guarantee a STORE perceding it can never be reordered after a LOAD following it). One STORE-LOAD barrier is needed on t= he sleeper/wakee side, before reading a variable used to indicate the event and after setting the state of the current task. Another STORE-LOAD barrier is needed on the waker side, before reading the state of the wakee task and af= ter setting a variable used to indicate the event. These two barriers can pair = with each other to avoid race conditions between sleepers/wakees and wakers: sleepr/wakee on CPU 1 waker on CPU 2 =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D { wakee->state =3D TASK_RUNNING, event_indicated =3D 0 } STORE current->state=3DTASK_INTERRUPTIBLE c =3D LOAD event_indicated=09 STORE event_indicated=3D1 s =3D LOAD wakee->state=09 assert(!(c=3D=3D0 && s =3D=3D TASK_RUNNING)); A STORE-LOAD barrier is implied after setting task state by wait-related fu= nctions: prepare_to_wait(); prepare_to_wait_exclusive(); prepare_to_wait_event(); A STORE-LOAD barrier is implied before reading task state by wake-related f= unctions: complete(); wake_up(); wake_up_all(); wake_up_bit(); wake_up_interruptible(); wake_up_interruptible_all(); wake_up_interruptible_nr(); wake_up_interruptible_poll(); wake_up_interruptible_sync(); wake_up_interruptible_sync_poll(); wake_up_locked(); wake_up_locked_poll(); wake_up_nr(); wake_up_poll(); wake_up_process(); Make sure an appropriate wake-related function is called after setting a gl= obal data used to indicate a event. [!] 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 ta= sk states nor the global variables indicating the events. git log --stat for this is: 1 file changed, 29 insertions(+), 108 deletions(-) , which I think it's better, thanks to your advice ;-) I will rewrite the commit message and send a new patch if this looks to you. Regards, Boqun >=20 > Or am I missing something? >=20 > Thanx, Paul --JgQwtEuHJzHdouWu Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQEcBAABCAAGBQJV8OfmAAoJEEl56MO1B/q459kH/jF2rUzQsbQ98HJnyjBQYtWE ywMcD5j6ks+lO3mwuU6w6i0osfK6ujbqSXlq+7WH3mljDb4hhue3X4B3XnE6N6Jl sVUNL8gGoyDuZIh4fp7+8Z61cg+nkNZsXBu8oo/1TOKZCniDsZerlxHkU3LP6Ynl eBteazgyc9g/eZ2sT2Uv+vCNzoIUOvLB33YTMLC1WRqMArogrQPgLjKRijLEnCX6 Cz28mG4x+M0Wse8xXLY4Rxk3Pu8/o6g1vX/FxH33/YXNRu6M8jQ7Uf4zDBHkvSiW npL/9EgR3ELQmmBu9mTbi6md0SmYv5rkzRFyxf1aD/ZTPPBOLIJxMw9D4gAg888= =BqNK -----END PGP SIGNATURE----- --JgQwtEuHJzHdouWu--