On Tue, Jul 31, 2012 at 4:06 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > Robert Haas <robertmh...@gmail.com> writes: >> 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, > > Since they are macros, how do you propose to do that exactly?
Why does it matter that they are macros? > I agree that volatile-izing everything in the vicinity is a sucky > solution, but the last time we looked at this there did not seem to > be a better one. Well, Linux has a barrier() primitive which is defined as a compiler-barrier, so I don't see why we shouldn't be able to manage the same thing. In fact, we've already got it, though it's presently unused; see storage/barrier.h. Looking over s_lock.h, it looks like TAS is typically defined using __asm__ __volatile__, and the __asm__ is marked as clobbering memory. As the fine comments say "this prevents gcc from thinking it can cache the values of shared-memory fields across the asm code", which is another way of saying that it's a compiler barrier. However, there's no similar guard in S_UNLOCK, which is simply declared as a volatile store, and therefore compiler ordering is guaranteed only with respect to other volatile pointer references. If we added something of the form __asm__ __volatile__("" : : : "memory") in there, it should serve as a full compiler barrier. That might have to go in a static inline function as we do with TAS, but I think it should work. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers