From 4039c736eb0955cb1daf88e211f105dbbb78f7ea Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 16 Apr 2016 19:53:38 -0400 Subject: [PATCH] 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. --- src/backend/storage/lmgr/spin.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/src/backend/storage/lmgr/spin.c b/src/backend/storage/lmgr/spin.c index 4a8f5bfe7bb..50391414305 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 */