public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Will Deacon <will.deacon@arm.com>
To: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: linux-kernel@vger.kernel.org, ldr709@gmail.com,
	dhowells@redhat.com, peterz@infradead.org, corbet@lwn.net,
	stern@rowland.harvard.edu, parri.andrea@gmail.com,
	j.alglave@ucl.ac.uk, luc.maranget@inria.fr
Subject: Re: [PATCH] doc: Update memory-barriers.txt for read-to-write dependencies
Date: Tue, 4 Jul 2017 17:36:36 +0100	[thread overview]
Message-ID: <20170704163635.GK22175@arm.com> (raw)
In-Reply-To: <20170703174106.GE2393@linux.vnet.ibm.com>

Hello, Paul,

On Mon, Jul 03, 2017 at 10:41:06AM -0700, Paul E. McKenney wrote:
> On Mon, Jul 03, 2017 at 02:07:03PM +0100, Will Deacon wrote:
> > Might be worth mentioning that you have to careful with the compiler here,
> > and pointing to the section on "Control dependencies" so that people don't
> > just take these three points as guarantees in isolation.
> > 
> > >  
> > >  	CPU 1		      CPU 2
> > >  	===============	      ===============
> > > @@ -603,19 +606,19 @@ A data-dependency barrier must also order against dependent writes:
> > >  	<write barrier>
> > >  	WRITE_ONCE(P, &B);
> > >  			      Q = READ_ONCE(P);
> > > -			      <data dependency barrier>
> > >  			      *Q = 5;
> > 
> > Do we want that write to Q to be a WRITE_ONCE? Again, the control
> > dependencies section does call this out.
> 
> Both good points!  Like this?

More or less, yes! Just one wibble below:

> commit 00269a0e23dbc50f1c4f101b23c8d74992eace05
> Author: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Date:   Fri Jun 30 16:18:28 2017 -0700
> 
>     doc: Update memory-barriers.txt for read-to-write dependencies
>     
>     The memory-barriers.txt document contains an obsolete passage stating that
>     smp_read_barrier_depends() is required to force ordering for read-to-write
>     dependencies.  We now know that this is not required, even for DEC Alpha.
>     This commit therefore updates this passage to state that read-to-write
>     dependencies are respected even without smp_read_barrier_depends().
>     
>     Reported-by: Lance Roy <ldr709@gmail.com>
>     Signed-off-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
>     Cc: David Howells <dhowells@redhat.com>
>     Cc: Will Deacon <will.deacon@arm.com>
>     Cc: Peter Zijlstra <peterz@infradead.org>
>     Cc: Jonathan Corbet <corbet@lwn.net>
>     Cc: Alan Stern <stern@rowland.harvard.edu>
>     Cc: Andrea Parri <parri.andrea@gmail.com>
>     Cc: Jade Alglave <j.alglave@ucl.ac.uk>
>     Cc: Luc Maranget <luc.maranget@inria.fr>
>     [ paulmck: Reference control-dependencies sections and use WRITE_ONCE()
>       per Will Deacon.  Correctly place split-cache paragraph while there. ]
> 
> diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt
> index 9d5e0f853f08..7be80911e502 100644
> --- a/Documentation/memory-barriers.txt
> +++ b/Documentation/memory-barriers.txt
> @@ -594,7 +594,23 @@ between the address load and the data load:
>  This enforces the occurrence of one of the two implications, and prevents the
>  third possibility from arising.
>  
> -A data-dependency barrier must also order against dependent writes:
> +
> +[!] Note that this extremely counterintuitive situation arises most easily on
> +machines with split caches, so that, for example, one cache bank processes
> +even-numbered cache lines and the other bank processes odd-numbered cache
> +lines.  The pointer P might be stored in an odd-numbered cache line, and the
> +variable B might be stored in an even-numbered cache line.  Then, if the
> +even-numbered bank of the reading CPU's cache is extremely busy while the
> +odd-numbered bank is idle, one can see the new value of the pointer P (&B),
> +but the old value of the variable B (2).
> +
> +
> +A data-dependency barrier is not required to order dependent writes
> +because the CPUs that the Linux kernel supports don't do writes until
> +they are certain (1) that the write will actually happen, (2) of the
> +location of the write, and (3) of the value to be written.  But please
> +carefully read the "CONTROL DEPENDENCIES" section:  The compiler can
> +and does break control dependencies in a great many situations.

This makes it sound like only control dependencies are susceptible to
being broken by compiler optimisations, so I'd drop the "control" and
just say "The compiler can and does break dependencies in a great many
situations".

With that:

Acked-by: Will Deacon <will.deacon@arm.com>

Cheers,

Will

  reply	other threads:[~2017-07-04 16:36 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-06-30 23:28 [PATCH] doc: Update memory-barriers.txt for read-to-write dependencies Paul E. McKenney
2017-07-03 13:07 ` Will Deacon
2017-07-03 17:41   ` Paul E. McKenney
2017-07-04 16:36     ` Will Deacon [this message]
2017-07-04 21:46       ` Paul E. McKenney
2017-07-05  9:46         ` Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170704163635.GK22175@arm.com \
    --to=will.deacon@arm.com \
    --cc=corbet@lwn.net \
    --cc=dhowells@redhat.com \
    --cc=j.alglave@ucl.ac.uk \
    --cc=ldr709@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luc.maranget@inria.fr \
    --cc=parri.andrea@gmail.com \
    --cc=paulmck@linux.vnet.ibm.com \
    --cc=peterz@infradead.org \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox