diff options
author | Tom Lane <tgl@sss.pgh.pa.us> | 2016-04-16 19:53:38 -0400 |
---|---|---|
committer | Tom Lane <tgl@sss.pgh.pa.us> | 2016-04-16 19:53:38 -0400 |
commit | 4039c736eb0955cb1daf88e211f105dbbb78f7ea (patch) | |
tree | f7b97a487f996c824fbce1f68d2fd368453d9bda | |
parent | 5fdda1ceab35311367ed0dbb283cd8aea896e49b (diff) | |
download | postgresql-4039c736eb0955cb1daf88e211f105dbbb78f7ea.tar.gz postgresql-4039c736eb0955cb1daf88e211f105dbbb78f7ea.zip |
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.
-rw-r--r-- | src/backend/storage/lmgr/spin.c | 30 |
1 files 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 */ |