Messages in this thread | | | Date | Fri, 13 Feb 2009 09:53:02 -0800 (PST) | From | Linus Torvalds <> | Subject | Re: [ltt-dev] [RFC git tree] Userspace RCU (urcu) for Linux (repost) |
| |
On Fri, 13 Feb 2009, Mathieu Desnoyers wrote: > > The whole idea behind _LOAD_SHARED() is that it does not translate in > any different assembly output than a standard load. So no, it cannot be > possibly slower. It has no more side-effect than a simple comment in the > code, and that's its purpose : to identify those variables. So if we > find a code path doing > > _STORE_SHARED(x, v); > smp_mc(); > while (_LOAD_SHARED(z) != val) > cpu_relax(); > > We can verify very easily the code correctness : > > A write cache flush is required after _STORE_SHARED > A read cache flush is required before _LOAD_SHARED > Read cache flushes are required to happen eventually between > _LOAD_SHARED in the loop.
That makes no sense.
First off, you had the comment that LOAD_SHARED() would flush caches, so your argument that it's just a load, nothing else, is in violation with your own statements. And I told you why such a thing is INSANE.
As to the underscore-version, what can it do? Nothing. It's perfectly fine to have something like this:
while (_LOAD_SHARED(x) && _LOAD_SHARED(y)) { cpu_relax(); }
and the thing is, there is no reason to do read-cache flushes between those two _LOAD_SHARED. So warning about it would be incorrect, and all it can do is be purely ugly "documentation" about the fact that it's doing a shared load, because it's not really allowed to warn about the fact that shared loads should have a cache flush in between, because THEY SHOULD NOT.
But it is also _ugly_.
And more importantly - if you see it as a documentation thing, then it's broken in the first place - you're documenting the places that you already know about, and already know are important, rather than finding places that might be buggy. So what does it help us? Nothing.
You might as well just document the cpu_relax(). Which it implicitly does: it's a barrier in a tight loop.
In other words, I see no real point. Your [_][LOAD|STORE]_SHARED is ugly and doesn't add value, or adds value (the cache flush) in really really bad ways that aren't even very well-defined.
Linus
| |