On Wed, 2005-12-10 at 22:46 -0400, Tom Lane wrote:
> No --- we didn't have any per-buffer spinlocks before 8.1.
Right. To summarize the problem as I understand it: we need to force $CC
not to move loads and stores around spinlock acquire/release. This can't
be done by just marking the spinlock variables themselves "volatile", as
the volatile qualifier only affects the rearrangement of loads and
stores WRT other volatile variables.
Before 8.1, all the places that use spinlocks directly also write to
shared memory through a volatile pointer (code that just uses LWLocks
*should* be okay, as Tom says). That ensures loads and stores aren't
rearranged outside spinlock acquire/release -- the problem with the 8.1
bufmgr changes is that this was not done. Tom's fix was to add the
necessary volatile qualifiers.
Another way to fix the problem would be to have S_LOCK() and S_UNLOCK()
force $CC to not rearrange loads and stores by themselves, without
relying upon volatile pointers. If this is possible, I think it would be
better: forgetting a volatile qualifier *once* means we are vulnerable
to corruption of shared memory, depending on the vagaries of the
compiler. Does anyone know how this is done?
>From talking with Andrew @ Supernews on IRC, he suggested the asm
"volatile" keyword as a possible solution ("volatile" is used for some
platform's S_LOCK/S_UNLOCK, but not the default S_UNLOCK(), which is
used by x86 and x86_64). [1] suggests that this keyword prevents
rearrangement of code around the inline ASM, but the GCC docs ([2])
don't actually state this: "The volatile keyword indicates that the
instruction has important side-effects ... Note that even a volatile asm
instruction can be moved relative to other code, including across jump
instructions." Per [3], it seems [1]'s understanding of the semantics of
"asm volatile" are incorrect -- so perhaps "asm volatile" isn't the
right tool, after all.
-Neil
[1]
http://www.ibiblio.org/gferg/ldp/GCC-Inline-Assembly-HOWTO.html#ss5.4
[2] http://gcc.gnu.org/onlinedocs/gcc-4.0.2/gcc/Extended-Asm.html
[3] http://gcc.gnu.org/bugzilla/show_bug.cgi?id=17884#c7