Commit 4039c736 authored by Tom Lane's avatar Tom Lane

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 6b93fcd1 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 daa7527a removed all the intermodule coupling that
made them necessary.
parent 5fdda1ce
...@@ -22,10 +22,6 @@ ...@@ -22,10 +22,6 @@
*/ */
#include "postgres.h" #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/pg_sema.h"
#include "storage/spin.h" #include "storage/spin.h"
...@@ -85,7 +81,17 @@ SpinlockSemaInit(PGSemaphore spinsemas) ...@@ -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 void
...@@ -93,13 +99,17 @@ s_init_lock_sema(volatile slock_t *lock, bool nested) ...@@ -93,13 +99,17 @@ s_init_lock_sema(volatile slock_t *lock, bool nested)
{ {
static int counter = 0; static int counter = 0;
*lock = (++counter) % NUM_SPINLOCK_SEMAPHORES; *lock = ((++counter) % NUM_SPINLOCK_SEMAPHORES) + 1;
} }
void void
s_unlock_sema(volatile slock_t *lock) 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 bool
...@@ -113,8 +123,12 @@ s_lock_free_sema(volatile slock_t *lock) ...@@ -113,8 +123,12 @@ s_lock_free_sema(volatile slock_t *lock)
int int
tas_sema(volatile slock_t *lock) 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* */ /* Note that TAS macros return 0 if *success* */
return !PGSemaphoreTryLock(&SpinlockSemaArray[*lock]); return !PGSemaphoreTryLock(&SpinlockSemaArray[lockndx - 1]);
} }
#endif /* !HAVE_SPINLOCKS */ #endif /* !HAVE_SPINLOCKS */
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment