Re: [PATCH] x86 bitops.h commentary on instruction reordering

[Andi Kleen <ak@muc.de>]

Vladislav Bolkhovitin <vst@vlnb.net> writes:

> Marcelo Tosatti wrote:
>> Vladislav, There is no cache coherency issues on x86, it handles the
>> cache coherency
>> on hardware.
>
> Well, Marcelo, sorry if I'm getting too annoying, but we had a race
> with cache coherency during SCST (SCSI target mid-level)
> development. We discovered that on P4 Xeon after atomic_set() there is
> very small window, when atomic_read() on another CPUs returns the old
> value. We had to rewrite the code without using atomic_set(). Isn't it
> cache coherency issue?

Add an rmb() after the atomic_set. atomic_set doesn't have one by itself
(it is non locked on Linux/x86)

> And, BTW, returning to the original topic, would it be better to make
> set_bit() and friends guarantee not to be reordered on all
> architectures, instead of just add the comment. Otherwise, what is the

That makes them a *lot* slower on some systems. And most of the
set_bits in the kernel don't need strong ordering.

> difference with versions with `__` prefix (__set_bit(), for example)?
> Just adding the comments will lead to creating different functions
> with gurantees by everyone who need it in all over the kernel. Is it
> the right thing? In some places in SCST we heavy rely on non-ordering
> guarantees.

Better add lots of memory barriers then.

-Andi