On Fri, 17 Aug 2007, Nick Piggin wrote:
> Satyam Sharma wrote:
> >
> > On Fri, 17 Aug 2007, Nick Piggin wrote:
>
> > > Also, why would you want to make these insane accessors for atomic_t
> > > types? Just make sure everybody knows the basics of barriers, and they
> > > can apply that knowledge to atomic_t and all other lockless memory
> > > accesses as well.
> >
> >
> > Code that looks like:
> >
> > while (!atomic_read(&v)) {
> > ...
> > cpu_relax_no_barrier();
> > forget(v.counter);
> > ^^^^^^^^
> > }
> >
> > would be uglier. Also think about code such as:
>
> I think they would both be equally ugly,
You think both these are equivalent in terms of "looks":
|
while (!atomic_read(&v)) { | while (!atomic_read_xxx(&v)) {
... | ...
cpu_relax_no_barrier(); | cpu_relax_no_barrier();
order_atomic(&v); | }
} |
(where order_atomic() is an atomic_t
specific wrapper as you mentioned below)
?
Well, taste varies, but ...
> but the atomic_read_volatile
> variant would be more prone to subtle bugs because of the weird
> implementation.
What bugs?
> And it would be more ugly than introducing an order(x) statement for
> all memory operations, and adding an order_atomic() wrapper for it
> for atomic types.
Oh, that order() / forget() macro [forget() was named such by Chuck Ebbert
earlier in this thread where he first mentioned it, btw] could definitely
be generically introduced for any memory operations.
> > a = atomic_read();
> > if (!a)
> > do_something();
> >
> > forget();
> > a = atomic_read();
> > ... /* some code that depends on value of a, obviously */
> >
> > forget();
> > a = atomic_read();
> > ...
> >
> > So much explicit sprinkling of "forget()" looks ugly.
>
> Firstly, why is it ugly? It's nice because of those nice explicit
> statements there that give us a good heads up and would have some
> comments attached to them
atomic_read_xxx (where xxx = whatever naming sounds nice to you) would
obviously also give a heads up, and could also have some comments
attached to it.
> (also, lack of the word "volatile" is always a plus).
Ok, xxx != volatile.
> Secondly, what sort of code would do such a thing?
See the nodemgr_host_thread() that does something similar, though not
exactly same.
> > on the other hand, looks neater. The "_volatile()" suffix makes it also
> > no less explicit than an explicit barrier-like macro that this primitive
> > is something "special", for code clarity purposes.
>
> Just don't use the word volatile,
That sounds amazingly frivolous, but hey, why not. As I said, ok,
xxx != volatile.
> and have barriers both before and after the memory operation,
How could that lead to bugs? (if you can point to existing code,
but just some testcase / sample code would be fine as well).
> [...] I don't see
> the point though, when you could just have a single barrier(x)
> barrier function defined for all memory locations,
As I said, barrier() is too heavy-handed.
> rather than
> this odd thing that only works for atomics
Why would it work only for atomics? You could use that generic macro
for anything you well damn please.
> (and would have to
> be duplicated for atomic_set.
#define atomic_set_xxx for something similar. Big deal ... NOT.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
