Robert Haas <robertmhaas@gmail.com> writes:
> OK, done. I think while we're tidying up here we ought to do
> something about this comment:
> * ANOTHER CAUTION: be sure that TAS(), TAS_SPIN(), and
> S_UNLOCK() represent
> * sequence points, ie, loads and stores of other values must not be moved
> * across a lock or unlock. In most cases it suffices to make
> the operation
> * be done through a "volatile" pointer.
> IIUC, this is basically total nonsense.
It could maybe be rewritten for more clarity, but it's far from being
nonsense. The responsibility for having an actual hardware memory fence
instruction lies with the author of the TAS macro. But the
responsibility for keeping the compiler from reordering stuff around the
TAS call is that of the *user* of the TAS macro (or spinlock), and in
most cases the way to do that is to make sure that both the spinlock and
the shared data structure are referenced through volatile pointers.
This isn't academic; we've seen bugs from failure to do that. (BTW,
the reason for not being equivalently tense about LWLock-protected
structures is that the compiler generally won't try to move operations
around an out-of-line function call. It's the fact that spinlocks are
inline-able that creates the risk here.)
> In the department of loose ends, there are a bunch of other things
> that maybe need cleanup here: (1) the gcc/intel compiler cases on
> ia64, (2) PA-RISC, (3) ARM, (4) PowerPC... and we should also perhaps
> reconsider the 32-bit x86 case.
The results I got yesterday seem sufficient basis to change the
gcc/intel cases for IA64, so I will go do that if you didn't already.
I am also currently running tests on x86_64 and PPC using Red Hat test
machines --- expect results later today. Red Hat doesn't seem to own
any many-CPU machines that are 32-bit-only, and I rather wonder if there
are any. It might be that it only makes sense to optimize the x86 paths
for a few CPUs, in which case this test methodology may not be very
helpful.
regards, tom lane