On 2014-09-09 13:52:40 -0400, Robert Haas wrote: > I had forgotten that it needed an update. Thanks for the reminder. Here's > v2.
I've attached a incremental patch fixing minor gripes. Other than that I think you can go ahead with this once the buildfarm accepts the sparc fixes (man, those machines are slow. spoonbill apparently takes ~5h for one run). I've done a read through s_lock.h and the only remaining potential issue that I see is that I have no idea if unixware's tas() is actually a safe compiler barrier (it is a memory barrier). And I really, really can't make myself care. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/storage/lmgr/s_lock.c b/src/backend/storage/lmgr/s_lock.c index 38dc34d..e8d3502 100644 --- a/src/backend/storage/lmgr/s_lock.c +++ b/src/backend/storage/lmgr/s_lock.c @@ -159,6 +159,7 @@ void s_unlock(slock_t *lock) { #ifdef TAS_ACTIVE_WORD + /* HP's PA-RISC */ *TAS_ACTIVE_WORD(lock) = -1; #else *lock = 0; diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h index 8e0c4c3..5f62a2c 100644 --- a/src/include/storage/s_lock.h +++ b/src/include/storage/s_lock.h @@ -57,8 +57,8 @@ * * It is the responsibility of these macros to make sure that the compiler * does not re-order accesses to shared memory to precede the actual lock - * acquisition, or follow the lock release. Prior to PostgreSQL 9.5, this - * was the caller's responsibility, which meant that callers had to use + * acquisition, or following the lock release. Prior to PostgreSQL 9.5, + * this was the caller's responsibility, which meant that callers had to use * volatile-qualified pointers to refer to both the spinlock itself and the * shared data being accessed within the spinlocked critical section. This * was notationally awkward, easy to forget (and thus error-prone), and @@ -403,7 +403,7 @@ tas(volatile slock_t *lock) * No stbar or membar available, luckily no actually produced hardware * requires a barrier. */ -#define S_UNLOCK(lock) (*((volatile slock_t *) (lock)) = 0) +/* use the default S_UNLOCK definition for gcc */ #elif __sparcv8 /* stbar is available (and required for both PSO, RMO), membar isn't */ #define S_UNLOCK(lock) \ @@ -921,14 +921,14 @@ extern int tas_sema(volatile slock_t *lock); * store) after a following store; platforms where this is possible must * define their own S_UNLOCK. But CPU reordering is not the only concern: * if we simply defined S_UNLOCK() as an inline macro, the compiler might - * reorder instructions from the critical section to occur after the lock - * release. Since the compiler probably can't know what the external + * reorder instructions from inside the critical section to occur after the + * lock release. Since the compiler probably can't know what the external * function s_unlock is doing, putting the same logic there should be adequate. * A sufficiently-smart globally optimizing compiler could break that * assumption, though, and the cost of a function call for every spinlock * release may hurt performance significantly, so we use this implementation * only for platforms where we don't know of a suitable intrinsic. For the - * most part, those are relatively obscure platform/compiler platforms to + * most part, those are relatively obscure platform/compiler platforms to * which the PostgreSQL project does not have access. */ #define USE_DEFAULT_S_UNLOCK
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers