lkml.org 
[lkml]   [2009]   [Feb]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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


\
 
 \ /
  Last update: 2009-02-13 18:57    [W:0.102 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site