Re: [v3,11/41] mips: reuse asm-generic/barrier.h
From: Paul E. McKenney <hidden>
Date: 2016-01-28 00:46:00
Also in:
linux-arm-kernel, linux-mips, linux-s390, linux-sh, linux-um, linuxppc-dev, lkml, sparclinux
On Wed, Jan 27, 2016 at 10:04:47AM +0800, Boqun Feng wrote:
quoted hunk ↗ jump to hunk
On Tue, Jan 26, 2016 at 03:29:21PM -0800, Paul E. McKenney wrote:quoted
On Tue, Jan 26, 2016 at 02:33:40PM -0800, Linus Torvalds wrote:quoted
On Tue, Jan 26, 2016 at 2:15 PM, Linus Torvalds [off-list ref] wrote:quoted
You might as well just write it as struct foo x = READ_ONCE(*ptr); x->bar = 5; because that "smp_read_barrier_depends()" does NOTHING wrt the second write.Just to clarify: on alpha it adds a memory barrier, but that memory barrier is useless.No trailing data-dependent read, so agreed, no smp_read_barrier_depends() needed. That said, I believe that we should encourage rcu_dereference*() or lockless_dereference() instead of READ_ONCE() for documentation reasons, though.quoted
On non-alpha, it is a no-op, and obviously does nothing simply because it generates no code. So if anybody believes that the "smp_read_barrier_depends()" does something, they are *wrong*.The other problem with smp_read_barrier_depends() is that it is often a pain figuring out which prior load it is supposed to apply to. Hence my preference for rcu_dereference*() and lockless_dereference().Because semantically speaking, rcu_derefence*() and lockless_dereference() are CONSUME(i.e. data/address dependent read->read and read->write pairs are ordered), whereas smp_read_barrier_depends() only guarantees read->read pairs with data dependency are ordered, right? If so, maybe we need to call it out in memory-barriers.txt, for example:diff --git a/Documentation/memory-barriers.txt b/Documentation/memory-barriers.txt index 904ee42..6b262c2 100644 --- a/Documentation/memory-barriers.txt +++ b/Documentation/memory-barriers.txt@@ -1703,8 +1703,8 @@ There are some more advanced barrier functions: (*) lockless_dereference(); - This can be thought of as a pointer-fetch wrapper around the - smp_read_barrier_depends() data-dependency barrier. + This is a load, and any load or store that has a data dependency on the + value returned by this load won't be reordered before this load.
This is a good start, but more is needed to warn people off of smp_read_barrier_depends(). But yes, better explanation would be good. Thanx, Paul
This is also similar to rcu_dereference(), but in cases where object lifetime is handled by some mechanism other than RCU, for Regards, Boqunquoted
quoted
And if anybody sends out an email with that smp_read_barrier_depends() in an example, they are actively just confusing other people, which is even worse than just being wrong. Which is why I jumped in. So stop perpetuating the myth that smp_read_barrier_depends() does something here. It does not. It's a bug, and it has become this "mind virus" for some people that seem to believe that it does something.It looks like I should add words to memory-barriers.txt de-emphasizing smp_read_barrier_depends(). I will take a look at that.quoted
I had to remove this crap once from the kernel already, see commit 105ff3cbf225 ("atomic: remove all traces of READ_ONCE_CTRL() and atomic*_read_ctrl()"). I don't want to ever see that broken construct again. And I want to make sure that everybody is educated about how broken it was. I'm extremely unhappy that it came up again.Well, if it makes you feel better, that was control dependencies and this was data dependencies. So it was not -exactly- the same. ;-) (Sorry, couldn't resist...)quoted
If it turns out that some architecture does actually need a barrier between a read and a dependent write, then that will mean that (a) we'll have to make up a _new_ barrier, because "smp_read_barrier_depends()" is not that barrier. We'll presumably then have to make that new barrier part of "rcu_derefence()" and friends.Agreed. We can worry about whether or not we replace the current smp_read_barrier_depends() with that new barrier when and if such hardware appears.quoted
(b) we will have found an architecture with even worse memory ordering semantics than alpha, and we'll have to stop castigating alpha for being the worst memory ordering ever.;-) ;-) ;-)quoted
but I sincerely hope that we'll never find that kind of broken architecture.Apparently at least some hardware vendors are reading memory-barriers.txt, so perhaps the odds of that kind of breakage have reduced. Thanx, Paul