]> granicus.if.org Git - postgresql/commitdiff
Adjust spin.c's spinlock emulation so that 0 is not a valid spinlock value.
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 16 Apr 2016 23:53:38 +0000 (19:53 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 16 Apr 2016 23:53:38 +0000 (19:53 -0400)
We've had repeated troubles over the years with failures to initialize
spinlocks correctly; see 6b93fcd14 for a recent example.  Most of the time,
on most platforms, such oversights can escape notice because all-zeroes is
the expected initial content of an slock_t variable.  The only platform
we have where the initialized state of an slock_t isn't zeroes is HPPA,
and that's practically gone in the wild.  To make it easier to catch such
errors without needing one of those, adjust the --disable-spinlocks code
so that zero is not a valid value for an slock_t for it.

In passing, remove a bunch of unnecessary #include's from spin.c;
commit daa7527afc227443 removed all the intermodule coupling that
made them necessary.

src/backend/storage/lmgr/spin.c

index 4a8f5bfe7bb0391fd08092fb22a98efbf6b73cd6..50391414305f9d99fb017ebf891f9e5d8cba7414 100644 (file)
  */
 #include "postgres.h"
 
-#include "access/xlog.h"
-#include "miscadmin.h"
-#include "replication/walsender.h"
-#include "storage/lwlock.h"
 #include "storage/pg_sema.h"
 #include "storage/spin.h"
 
@@ -85,7 +81,17 @@ SpinlockSemaInit(PGSemaphore spinsemas)
 }
 
 /*
- * s_lock.h hardware-spinlock emulation
+ * s_lock.h hardware-spinlock emulation using semaphores
+ *
+ * We map all spinlocks onto a set of NUM_SPINLOCK_SEMAPHORES semaphores.
+ * It's okay to map multiple spinlocks onto one semaphore because no process
+ * should ever hold more than one at a time.  We just need enough semaphores
+ * so that we aren't adding too much extra contention from that.
+ *
+ * slock_t is just an int for this implementation; it holds the spinlock
+ * number from 1..NUM_SPINLOCK_SEMAPHORES.  We intentionally ensure that 0
+ * is not a valid value, so that testing with this code can help find
+ * failures to initialize spinlocks.
  */
 
 void
@@ -93,13 +99,17 @@ s_init_lock_sema(volatile slock_t *lock, bool nested)
 {
        static int      counter = 0;
 
-       *lock = (++counter) % NUM_SPINLOCK_SEMAPHORES;
+       *lock = ((++counter) % NUM_SPINLOCK_SEMAPHORES) + 1;
 }
 
 void
 s_unlock_sema(volatile slock_t *lock)
 {
-       PGSemaphoreUnlock(&SpinlockSemaArray[*lock]);
+       int                     lockndx = *lock;
+
+       if (lockndx <= 0 || lockndx > NUM_SPINLOCK_SEMAPHORES)
+               elog(ERROR, "invalid spinlock number: %d", lockndx);
+       PGSemaphoreUnlock(&SpinlockSemaArray[lockndx - 1]);
 }
 
 bool
@@ -113,8 +123,12 @@ s_lock_free_sema(volatile slock_t *lock)
 int
 tas_sema(volatile slock_t *lock)
 {
+       int                     lockndx = *lock;
+
+       if (lockndx <= 0 || lockndx > NUM_SPINLOCK_SEMAPHORES)
+               elog(ERROR, "invalid spinlock number: %d", lockndx);
        /* Note that TAS macros return 0 if *success* */
-       return !PGSemaphoreTryLock(&SpinlockSemaArray[*lock]);
+       return !PGSemaphoreTryLock(&SpinlockSemaArray[lockndx - 1]);
 }
 
 #endif   /* !HAVE_SPINLOCKS */