On Fri, 17 Aug 2007, Nick Piggin wrote:

Satyam Sharma wrote:
[...]

quoted

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)

?

I think the LHS is better if your atomic_read_xxx primitive is using the
crazy one-sided barrier,

  ^^^^^

I'd say it's purposefully one-sided.

because the LHS code you immediately know what
barriers are happening, and with the RHS you have to look at the
atomic_read_xxx
definition.

No. As I said, the _xxx (whatever the heck you want to name it as) should
give the same heads-up that your "order_atomic" thing is supposed to give.

If your atomic_read_xxx implementation was more intuitive, then both are
pretty well equal. More lines != ugly code.

quoted

[...]
What bugs?

You can't think for yourself? Your atomic_read_volatile contains a compiler
barrier to the atomic variable before the load. 2 such reads from different
locations look like this:

asm volatile("" : "+m" (v1));
atomic_read(&v1);
asm volatile("" : "+m" (v2));
atomic_read(&v2);

Which implies that the load of v1 can be reordered to occur after the load
of v2.

And how would that be a bug? (sorry, I really can't think for myself)

quoted

quoted

Secondly, what sort of code would do such a thing?

See the nodemgr_host_thread() that does something similar, though not
exactly same.

I'm sorry, all this waffling about made up code which might do this and
that is just a waste of time.

First, you could try looking at the code.

And by the way, as I've already said (why do *require* people to have to
repeat things to you?) this isn't even about only existing code.