On Sat, May 28, 2022 at 01:15:30PM +0900, Akira Yokosawa wrote: > The term "data dependency barrier", which has been in > memory-barriers.txt ever since it was first authored by David Howells, > has become confusing due to the fact that in LKMM's explanations.txt > and elsewhere, "data dependency" is used mostly for load-to-store data > dependency. > > To prevent further confusions, do the following changes: > > - substitute "address-dependency barrier" for "data dependency barrier"; > - add note on the removal of kernel APIs for explicit address- > dependency barriers in kernel release v5.9; > - add note on the section title rename; > - use READ_ONCE_OLD() for READ_ONCE() of pre-4.15 (no address- > dependency implication) in code snippets; > - fix number of CPU memory barrier APIs; > - and a few more context adjustments. > > Note: Line break cleanups are deferred to a follow-up patch. > > Reported-by: "Michael S. Tsirkin" <mst@xxxxxxxxxx> > Signed-off-by: Akira Yokosawa <akiyks@xxxxxxxxx> > Cc: "Paul E. McKenney" <paulmck@xxxxxxxxxxxxx> > Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> > Cc: Will Deacon <will@xxxxxxxxxx> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx> > Cc: Boqun Feng <boqun.feng@xxxxxxxxx> > Cc: Andrea Parri <parri.andrea@xxxxxxxxx> > Cc: Nicholas Piggin <npiggin@xxxxxxxxx> > Cc: David Howells <dhowells@xxxxxxxxxx> > Cc: Daniel Lustig <dlustig@xxxxxxxxxx> > Cc: Joel Fernandes <joel@xxxxxxxxxxxxxxxxx> > Cc: Jonathan Corbet <corbet@xxxxxxx> > --- > This is a response to Michael's report back in last November [1]. > > [1]: "data dependency naming inconsistency": > https://lore.kernel.org/r/20211011064233-mutt-send-email-mst@xxxxxxxxxx/ > > In the thread, I suggested removing all the explanations of "data dependency > barriers", which Paul thought was reasonable. > > However, such removals would require rewriting the notoriously > hard-to-grasp document, which I'm not quite up to. > I have become more inclined to just substitute "address-dependency > barrier" for "data dependency barrier" considering the fact that > READ_ONCE() has an implicit memory barrier for Alpha. > > This RFC patch is the result of such an attempt. > > Note: I made a mistake in the thread above. Kernel APIs for explicit data > dependency barriers were removed in v5.9. > I confused the removal with the addition of the barrier to Alpha's > READ_ONCE() in v4.15. > > Any feedback is welcome! > > Thanks, Akira > -- > Documentation/memory-barriers.txt | 119 +++++++++++++++++------------- > 1 file changed, 67 insertions(+), 52 deletions(-) > > diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt > index b12df9137e1c..306afa1f9347 100644 > --- a/Documentation/memory-barriers.txt > +++ b/Documentation/memory-barriers.txt > @@ -52,7 +52,7 @@ CONTENTS > > - Varieties of memory barrier. > - What may not be assumed about memory barriers? > - - Data dependency barriers (historical). > + - Address-dependency barriers (historical). > - Control dependencies. > - SMP barrier pairing. > - Examples of memory barrier sequences. > @@ -187,7 +187,7 @@ As a further example, consider this sequence of events: > B = 4; Q = P; > P = &B; D = *Q; > > -There is an obvious data dependency here, as the value loaded into D depends on > +There is an obvious address dependency here, as the value loaded into D depends on > the address retrieved from P by CPU 2. At the end of the sequence, any of the > following results are possible: > > @@ -391,49 +391,53 @@ Memory barriers come in four basic varieties: > memory system as time progresses. All stores _before_ a write barrier > will occur _before_ all the stores after the write barrier. > > - [!] Note that write barriers should normally be paired with read or data > + [!] Note that write barriers should normally be paired with read- or address- > dependency barriers; see the "SMP barrier pairing" subsection. > > > - (2) Data dependency barriers. > + (2) Address-dependency barriers (historical). > > - A data dependency barrier is a weaker form of read barrier. In the case > + An address-dependency barrier is a weaker form of read barrier. In the case > where two loads are performed such that the second depends on the result > of the first (eg: the first load retrieves the address to which the second > - load will be directed), a data dependency barrier would be required to > + load will be directed), an address-dependency barrier would be required to > make sure that the target of the second load is updated after the address > obtained by the first load is accessed. > > - A data dependency barrier is a partial ordering on interdependent loads > + An address-dependency barrier is a partial ordering on interdependent loads > only; it is not required to have any effect on stores, independent loads > or overlapping loads. I suppose this isn't really a comment on your patch, as I much prefer the updated terminology, but the way this section is now worded really makes it sounds like address dependencies only order load -> load, whereas they equally order load -> store. Saying that "An address-dependency barrier... is not required to have any effect on stores" is really confusing to me: the barrier should only ever be used in conjunction with an address-dependency _anyway_ so whether or not it's the barrier or the dependency giving the order is an implementation detail. Perhaps the barrier should be called a "Read-read-address-dependency barrier", an "Address-dependency read barrier" or even a "Consume barrier" (:p) instead? Dunno, Alan is normally much better at naming these things than I am. Alternatively, maybe we should be removing the historical stuff from the document altogether if it's no longer needed. We don't have any occurrences of read_barrier_depends() anymore, so why confuse people with it? Will
