Re: [ltt-dev] [RFC git tree] Userspace RCU (urcu) for Linux (repost)

[Linus Torvalds <torvalds@linux-foundation.org>]

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