From 572c8fae484955bcbcecc97d63536ce5c9f5ee10 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Thu, 7 May 2026 15:32:50 -0500 Subject: [PATCH v5 3/3] Better express platform requirements in s_lock.h. --- src/backend/storage/lmgr/s_lock.c | 2 ++ src/include/storage/s_lock.h | 59 +++++++++++++++++-------------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/src/backend/storage/lmgr/s_lock.c b/src/backend/storage/lmgr/s_lock.c index 6df568eccb3..34c6de66773 100644 --- a/src/backend/storage/lmgr/s_lock.c +++ b/src/backend/storage/lmgr/s_lock.c @@ -91,6 +91,7 @@ s_lock_stuck(const char *file, int line, const char *func) #endif } +#ifdef USE_DEFAULT_S_LOCK /* * s_lock(lock) - platform-independent portion of waiting for a spinlock. */ @@ -110,6 +111,7 @@ s_lock(volatile slock_t *lock, const char *file, int line, const char *func) return delayStatus.delays; } +#endif #ifdef USE_DEFAULT_S_UNLOCK void diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h index fb872edd2f0..65f00c06518 100644 --- a/src/include/storage/s_lock.h +++ b/src/include/storage/s_lock.h @@ -44,23 +44,16 @@ * atomic test-and-set only when it appears free. * * TAS() and TAS_SPIN() are NOT part of the API, and should never be called - * directly. - * - * CAUTION: on some platforms TAS() and/or TAS_SPIN() may sometimes report - * failure to acquire a lock even when the lock is not locked. For example, - * on Alpha TAS() will "fail" if interrupted. Therefore a retry loop must - * always be used, even if you are certain the lock is free. + * directly. If a platform-specific TAS() is defined, the platform must + * _not_ define its own S_LOCK(). Conversely, if a platform-specific + * S_LOCK() is defined, the platform must _not_ define its own TAS(). + * Currently, all supported platforms define TAS() and use the default + * S_LOCK() implementation, so that is probably a good place to start if + * adding a new one. * * It is the responsibility of these macros to make sure that the compiler * does not re-order accesses to shared memory to precede the actual lock - * acquisition, or follow the lock release. Prior to PostgreSQL 9.5, this - * was the caller's responsibility, which meant that callers had to use - * volatile-qualified pointers to refer to both the spinlock itself and the - * shared data being accessed within the spinlocked critical section. This - * was notationally awkward, easy to forget (and thus error-prone), and - * prevented some useful compiler optimizations. For these reasons, we - * now require that the macros themselves prevent compiler re-ordering, - * so that the caller doesn't need to take special precautions. + * acquisition, or follow the lock release. * * On platforms with weak memory ordering, the TAS(), TAS_SPIN(), and * S_UNLOCK() macros must further include hardware-level memory fence @@ -72,7 +65,7 @@ * * On most supported platforms, TAS() uses a tas() function written * in assembly language to execute a hardware atomic-test-and-set - * instruction. Equivalent OS-supplied mutex routines could be used too. + * instruction. Equivalent compiler intrinsics are another popular option. * * * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group @@ -642,20 +635,27 @@ spin_delay(void) #endif /* !defined(TAS) */ -/* Blow up if we didn't have any way to do spinlocks */ -#ifndef TAS -#error PostgreSQL does not have spinlock support on this platform. Please report this to pgsql-bugs@lists.postgresql.org. -#endif - - /* * Default Definitions - override these above as needed. */ -#if !defined(S_LOCK) +/* + * Make sure S_LOCK is defined, either explicitly for the platform or via a TAS + * macro for the platform. Exactly one of either S_LOCK or TAS should be + * defined for a supported platform at this point in the file. + */ +#if defined(S_LOCK) +#if defined(TAS) +#error Both TAS and S_LOCK defined on this platform. Please report this to pgsql-bugs@lists.postgresql.org. +#endif +#elif defined(TAS) +#define USE_DEFAULT_S_LOCK +extern int s_lock(volatile slock_t *lock, const char *file, int line, const char *func); #define S_LOCK(lock) \ (TAS(lock) ? s_lock((lock), __FILE__, __LINE__, __func__) : 0) -#endif /* S_LOCK */ +#else +#error Neither TAS nor S_LOCK defined on this platform. Please report this to pgsql-bugs@lists.postgresql.org. +#endif #if !defined(S_UNLOCK) /* @@ -687,15 +687,22 @@ extern void s_unlock(volatile slock_t *lock); #define SPIN_DELAY() ((void) 0) #endif /* SPIN_DELAY */ -#if !defined(TAS_SPIN) +/* + * We can only define TAS_SPIN if TAS was defined. Otherwise, the platform + * defined its own S_LOCK without TAS. Since TAS_SPIN is only used by the + * default S_LOCK's helper function, there's no need to define TAS_SPIN at all + * in that case, unless you plan to use it in a platform-specific S_LOCK helper + * function. (Note that we currently do not have any platforms that don't + * define TAS.) + */ +#if !defined(TAS_SPIN) && defined(TAS) #define TAS_SPIN(lock) TAS(lock) -#endif /* TAS_SPIN */ +#endif /* ! TAS_SPIN && TAS */ /* * Platform-independent out-of-line support routines */ -extern int s_lock(volatile slock_t *lock, const char *file, int line, const char *func); /* Support for dynamic adjustment of spins_per_delay */ #define DEFAULT_SPINS_PER_DELAY 100 -- 2.50.1 (Apple Git-155)