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 (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to