From e9f4795bcb0b85ef7d95bc16bb91b2b0ebeef131 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Thu, 1 Aug 2024 09:55:18 +1200 Subject: [PATCH v2 2/2] Add some assertions to spinlocks. In assertion builds, use a magic values to check that the spinlock was initialized, and also check that a lock was held when releasing. --- src/backend/storage/lmgr/s_lock.c | 4 +-- src/include/storage/spin.h | 46 ++++++++++++++++++++++++++----- 2 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/backend/storage/lmgr/s_lock.c b/src/backend/storage/lmgr/s_lock.c index 18a98b6e638..5785082a720 100644 --- a/src/backend/storage/lmgr/s_lock.c +++ b/src/backend/storage/lmgr/s_lock.c @@ -116,11 +116,11 @@ s_lock(volatile slock_t *lock, const char *file, int line, const char *func) * On these architectures, it is known to be more efficient to test * the lock with a relaxed load first, while spinning. */ - probably_free = pg_atomic_unlocked_test_flag(lock); + probably_free = pg_atomic_unlocked_test_flag(&lock->flag); #endif /* Try to get the lock. */ - if (probably_free && pg_atomic_test_set_flag(lock)) + if (probably_free && pg_atomic_test_set_flag(&lock->flag)) break; perform_spin_delay(&delayStatus); diff --git a/src/include/storage/spin.h b/src/include/storage/spin.h index 24edac4822d..e9bf1023954 100644 --- a/src/include/storage/spin.h +++ b/src/include/storage/spin.h @@ -54,12 +54,19 @@ #include "port/atomics.h" -typedef pg_atomic_flag slock_t; - - /* Support for dynamic adjustment of spins_per_delay */ #define DEFAULT_SPINS_PER_DELAY 100 +#define SLOCKT_T_MAGIC 0xc7f31f05 + +typedef struct +{ + pg_atomic_flag flag; +#ifdef USE_ASSERT_CHECKING + int magic; +#endif +} slock_t; + /* * Support for spin delay which is useful in various places where * spinlock-like procedures take place. @@ -97,9 +104,34 @@ extern int s_lock(volatile slock_t *lock, const char *file, int line, const char *func); -#define SpinLockInit(lock) pg_atomic_init_flag(lock) -#define SpinLockAcquire(lock) (pg_atomic_test_set_flag(lock) ? 0 : s_lock((lock), __FILE__, __LINE__, __func__)) -#define SpinLockRelease(lock) pg_atomic_clear_flag(lock) -#define SpinLockFree(lock) pg_atomic_unlocked_test_flag(lock) +static inline void +SpinLockInit(volatile slock_t *lock) +{ +#ifdef USE_ASSERT_CHECKING + /* Used to detect use of uninitialized spinlocks. */ + lock->magic = SLOCKT_T_MAGIC; +#endif + + pg_atomic_init_flag(&lock->flag); +} + +#define SpinLockAcquire(lock) \ + (AssertMacro((lock)->magic == SLOCKT_T_MAGIC), \ + pg_atomic_test_set_flag(&(lock)->flag) ? 0 : \ + s_lock((lock), __FILE__, __LINE__, __func__)) + +static inline void +SpinLockRelease(volatile slock_t *lock) +{ + Assert(lock->magic == SLOCKT_T_MAGIC); + + /* + * Use a relaxed load to see that it's currently held. That's OK because + * we expect the calling thread to be the one that set it. + */ + Assert(!pg_atomic_unlocked_test_flag(&(lock)->flag)); + + pg_atomic_clear_flag(&(lock)->flag); +} #endif /* SPIN_H */ -- 2.45.2