On Sat, Jul 28, 2012 at 6:33 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
> A concern I have is whether the XLogCtlWrite *Write pointer needs to
> be declared volatile, to prevent the compiler from pushing operations
> on them outside of the locks (and so memory barriers) that formally
> protect them. However I see that existing code with Insert also does
> not use volatile, so maybe my concern is baseless. Perhaps the
> compiler guarantees to not move operations on pointers over the
> boundaries of function calls? The pattern elsewhere in the code seems
> to be to use volatiles for things protected by spin-locks (implemented
> by macros) but not for things protected by LWLocks.
Yes, our code is only correct if we assume that the compiler performs
no global optimizations - i.e. no movement of code between functions.
IMHO, the way we have it now is kind of a mess. SpinLockAcquire and
SpinLockRelease are required to be CPU barriers, but they are not
required to be compiler barriers. If we changed that so that they
were required to act as barriers of both flavors, then (1) we wouldn't
need volatile in as many places, (2) we would be less prone to bugs
caused by the omission of not-obviously-necessary volatile markings,
and (3) we would remove one possible source of breakage that might be
induced by a globally optimizing compiler. As things stand today,
making a previously-global function static could result in working
code breaking, because the static function might be inlined where the
global function wasn't. Ouch.
Anyway, unless and until we make a definitional change of the sort
described above, any pointers used within a spinlock critical section
must be volatile; and pray that the compiler doesn't inline anything
you weren't expecting.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company