Neil Conway <neilc@samurai.com> writes:
> LWLocks certainly do protect shared data, and if the compiler rearranged
> loads and stores around LWLocks acquire/release, it would result in
> corruption. Tom was arguing it is unlikely the compiler will actually do
> this (because LWLockAcquire is an out-of-line function call that might
> invoke a system call, unlike SpinLockAcquire).
Actually it seems the sore spot is more likely to be SpinLockRelease,
which on many architectures expands to nothing more than an inlined
*(lockptr) = 0;
The lock pointer is declared as pointer to volatile, which should
discourage the compiler from removing the store altogether, but as
we've seen the compiler may be willing to rearrange the store with
respect to adjacent loads/stores that aren't marked volatile.
SpinLockAcquire contains an out-of-line function call, so although
the compiler could theoretically misoptimize things in the fast path,
it's probably much less risky than SpinLockRelease.
BTW, we may be perfectly safe on architectures like PPC, where
S_UNLOCK includes an __asm__ __volatile__ section for a hardware-level
optimization fence instruction. I wonder though if it'd be a good idea
to be marking those fence instructions with the "clobbers memory"
qualifier to ensure this?
regards, tom lane