From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sat, 16 Apr 2016 23:53:38 +0000 (-0400)
Subject: Adjust spin.c's spinlock emulation so that 0 is not a valid spinlock value.
X-Git-Tag: REL9_6_BETA1~139
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=4039c736eb0955cb1daf88e211f105dbbb78f7ea;p=postgresql

Adjust spin.c's spinlock emulation so that 0 is not a valid spinlock value.

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.
---

diff --git a/src/backend/storage/lmgr/spin.c b/src/backend/storage/lmgr/spin.c
index 4a8f5bfe7b..5039141430 100644
--- a/src/backend/storage/lmgr/spin.c
+++ b/src/backend/storage/lmgr/spin.c
@@ -22,10 +22,6 @@
  */
 #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 */