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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
