Hi Paul, On Mon, Jan 18, 2016 at 07:46:29AM -0800, Paul E. McKenney wrote:> On Mon, Jan 18, 2016 at 04:19:29PM +0800, Herbert Xu wrote: > > Paul E. McKenney <paulmck at linux.vnet.ibm.com> wrote: > > > > > > You could use SYNC_ACQUIRE() to implement read_barrier_depends() and > > > smp_read_barrier_depends(), but SYNC_RMB probably does not suffice. > > > The reason for this is that smp_read_barrier_depends() must order the > > > pointer load against any subsequent read or write through a dereference > > > of that pointer. For example: > > > > > > p = READ_ONCE(gp); > > > smp_rmb(); > > > r1 = p->a; /* ordered by smp_rmb(). */ > > > p->b = 42; /* NOT ordered by smp_rmb(), BUG!!! */ > > > r2 = x; /* ordered by smp_rmb(), but doesn't need to be. */ > > > > > > In contrast: > > > > > > p = READ_ONCE(gp); > > > smp_read_barrier_depends(); > > > r1 = p->a; /* ordered by smp_read_barrier_depends(). */ > > > p->b = 42; /* ordered by smp_read_barrier_depends(). */ > > > r2 = x; /* not ordered by smp_read_barrier_depends(), which is OK. */ > > > > > > Again, if your hardware maintains local ordering for address > > > and data dependencies, you can have read_barrier_depends() and > > > smp_read_barrier_depends() be no-ops like they are for most > > > architectures. > > > > > > Does that help? > > > > This is crazy! smp_rmb started out being strictly stronger than > > smp_read_barrier_depends, when did this stop being the case? > > Hello, Herbert! > > It is true that most Linux kernel code relies only on the read-read > properties of dependencies, but the read-write properties are useful. > Admittedly relatively rarely, but useful. > > The better comparison for smp_read_barrier_depends(), especially in > its rcu_dereference*() form, is smp_load_acquire(). >Confused.. I recall that last time you and Linus came into a conclusion that even on Alpha, a barrier for read->write with data dependency is unnecessary: http://article.gmane.org/gmane.linux.kernel/2077661 And in an earlier mail of that thread, Linus made his point that smp_read_barrier_depends() should only be used to order read->read. So right now, are we going to extend the semantics of smp_read_barrier_depends()? Can we just make smp_read_barrier_depends() still only work for read->read, and assume all the architectures won't reorder read->write with data dependency, so that the code above having a smp_rmb() also works? Regards, Boqun -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 473 bytes Desc: not available URL: <http://lists.linuxfoundation.org/pipermail/virtualization/attachments/20160127/a33a9dab/attachment.sig>
On Wed, Jan 27, 2016 at 12:52:07AM +0800, Boqun Feng wrote:> I recall that last time you and Linus came into a conclusion that even > on Alpha, a barrier for read->write with data dependency is unnecessary: > > http://article.gmane.org/gmane.linux.kernel/2077661 > > And in an earlier mail of that thread, Linus made his point that > smp_read_barrier_depends() should only be used to order read->read. > > So right now, are we going to extend the semantics of > smp_read_barrier_depends()? Can we just make smp_read_barrier_depends() > still only work for read->read, and assume all the architectures won't > reorder read->write with data dependency, so that the code above having > a smp_rmb() also works?That discussions was about control dependencies. So writes that _depend_ on a prior read having an explicit value. So something like: struct foo *x = READ_ONCE(*ptr); smp_read_barrier_depends() if (x->val == 5) x->bar = 5; In that case, the load of x->val must be complete and its value determined _before_ the store to x->bar can happen. This is distinct from: struct foo *x = READ_ONCE(*ptr); smp_read_barrier_depends(); x->bar = 5; And its the second case where smp_read_barrier_depends() read->write order matters.
On Tue, Jan 26, 2016 at 9:22 AM, Peter Zijlstra <peterz at infradead.org> wrote:> > This is distinct from:That may be distinct, but:> struct foo *x = READ_ONCE(*ptr); > smp_read_barrier_depends(); > x->bar = 5;This case is complete BS. Stop perpetuating it. I already removed a number of bogus cases of it, and I removed the incorrect documentation that had this crap. It's called "smp_READ_barrier_depends()" for a reason. Alpha is the only one that needs it, and alpha needs it only for dependent READS. It's not called smp_read_write_barrier_depends(). It's not called "smp_mb_depends()". It's a weaker form of "smp_rmb()", nothing else. So alpha does have an implied dependency chain from a read to a subsequent dependent write, and does not need any extra barriers. Alpha does *not* have a dependency chain from a read to a subsequent read, which is why we need that horrible crappy smp_read_barrier_depends(). But it's the only reason. This is the alpha reference manual wrt read-to-write dependency: 5.6.1.7 Definition of Dependence Constraint The depends relation (DP) is defined as follows. Given u and v issued by processor Pi, where u is a read or an instruction fetch and v is a write, u precedes v in DP order (written u DP v, that is, v depends on u) in either of the following situations: ? u determines the execution of v, the location accessed by v, or the value written by v. ? u determines the execution or address or value of another memory access z that precedes v or might precede v (that is, would precede v in some execution path depending on the value read by u) by processor issue constraint (see Section 5.6.1.3). Note that the dependence barrier honors not only control flow, but address and data values too. This is a different syntax than we use, but 'u' is the READ_ONCE, and 'v' is the write. Any data, address or conditional dependency between the two implies an ordering. So no, "smp_read_barrier_depends()" is *ONLY* about two reads, where the second read is data-dependent on the first. Nothing else. So if you _ever_ see a "smp_read_barrier_depends()" that isn't about a barrier between two reads, then that is a bug. The above code is crap. It's exactly as much crap as a = READ_ONCE(x); smp_rmb(); WRITE_ONCE(b, y); because a "rmb()" simply doesn't have anything to do with read-vs-subsequent-write ordering. Linus
On Wed, Jan 27, 2016 at 12:52:07AM +0800, Boqun Feng wrote:> Hi Paul, > > On Mon, Jan 18, 2016 at 07:46:29AM -0800, Paul E. McKenney wrote: > > On Mon, Jan 18, 2016 at 04:19:29PM +0800, Herbert Xu wrote: > > > Paul E. McKenney <paulmck at linux.vnet.ibm.com> wrote: > > > > > > > > You could use SYNC_ACQUIRE() to implement read_barrier_depends() and > > > > smp_read_barrier_depends(), but SYNC_RMB probably does not suffice. > > > > The reason for this is that smp_read_barrier_depends() must order the > > > > pointer load against any subsequent read or write through a dereference > > > > of that pointer. For example: > > > > > > > > p = READ_ONCE(gp); > > > > smp_rmb(); > > > > r1 = p->a; /* ordered by smp_rmb(). */ > > > > p->b = 42; /* NOT ordered by smp_rmb(), BUG!!! */ > > > > r2 = x; /* ordered by smp_rmb(), but doesn't need to be. */ > > > > > > > > In contrast: > > > > > > > > p = READ_ONCE(gp); > > > > smp_read_barrier_depends(); > > > > r1 = p->a; /* ordered by smp_read_barrier_depends(). */ > > > > p->b = 42; /* ordered by smp_read_barrier_depends(). */ > > > > r2 = x; /* not ordered by smp_read_barrier_depends(), which is OK. */ > > > > > > > > Again, if your hardware maintains local ordering for address > > > > and data dependencies, you can have read_barrier_depends() and > > > > smp_read_barrier_depends() be no-ops like they are for most > > > > architectures. > > > > > > > > Does that help? > > > > > > This is crazy! smp_rmb started out being strictly stronger than > > > smp_read_barrier_depends, when did this stop being the case? > > > > Hello, Herbert! > > > > It is true that most Linux kernel code relies only on the read-read > > properties of dependencies, but the read-write properties are useful. > > Admittedly relatively rarely, but useful. > > > > The better comparison for smp_read_barrier_depends(), especially in > > its rcu_dereference*() form, is smp_load_acquire(). > > Confused.. > > I recall that last time you and Linus came into a conclusion that even > on Alpha, a barrier for read->write with data dependency is unnecessary: > > http://article.gmane.org/gmane.linux.kernel/2077661 > > And in an earlier mail of that thread, Linus made his point that > smp_read_barrier_depends() should only be used to order read->read.Those examples involved read-to-write with conditionals, as in: if (READ_ONCE(a)) WRITE_ONCE(b, 1); Without the "if", no ordering is guaranteed on weakly ordered CPUs. (The volatile accesses keep ordering within the compiler for once...> So right now, are we going to extend the semantics of > smp_read_barrier_depends()? Can we just make smp_read_barrier_depends() > still only work for read->read, and assume all the architectures won't > reorder read->write with data dependency, so that the code above having > a smp_rmb() also works?The semantics of smp_read_barrier_depends() has been both read-to-write and read-to-read for some time now, this patch just catches the documentation up with reality. Thanx, Paul