public inbox for [email protected]
help / color / mirror / Atom feedFrom: Nathan Bossart <[email protected]>
To: Heikki Linnakangas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: [email protected] <[email protected]>
Subject: convert SpinLock* macros to static inline functions
Date: Wed, 18 Feb 2026 11:28:01 -0600
Message-ID: <aZX2oUcKf7IzHnnK@nathan> (raw)
Starting a new thread from [0].
On Wed, Feb 18, 2026 at 02:17:34AM +0200, Heikki Linnakangas wrote:
> On 18/02/2026 01:11, Andres Freund wrote:
>> I think the spinlock functions should also assert this.
>
> +1
While working on adding assertions that we are not in a signal handler to
the spinlock functions, I figured I'd take the opportunity to convert the
spin.h macros to static inline functions. This was previously discussed
[1], but AFAICT there was debate over whether to remove S_LOCK and friends
altogether (which doesn't seem to have happened). But IIUC nobody was
horribly opposed to $subject, which I think makes sense to do as
prerequisite work for the aforementioned assertions.
However, as soon as I did this, I got a bunch of build failures because
various parts of the code still use volatile qualifiers quite liberally.
It looks like most of these (e.g., see code from commits 2487d872e0,
966fb05b58, 4bc15a8bfb, and 4db3744f1f) predate making spinlocks compiler
barriers (commit 0709b7ee72) or were cargo-culted from code that predated
it. So, IIUC, it's probably safe to remove these volatile qualifiers now.
We could alternatively add volatile qualifiers to the new static inline
function parameters, but that seems like it might just encourage continued
unnecessary use.
I also noticed that SpinLockFree() is unused (and apparently never was).
There seems to have been agreement back in 2020 to remove it [2], but it
still exists. I added a prerequisite patch for this, too.
Thoughts?
[0] https://postgr.es/m/2f25aa74-990d-4412-a032-c876dbcff667%40iki.fi
[1] https://postgr.es/m/flat/20200617183354.pm3biu3zbmo2pktq%40alap3.anarazel.de#160ac60fddc7934777a0d20...
[2] https://postgr.es/m/flat/20200608225338.m5zho424w6lpwb2d%40alap3.anarazel.de
--
nathan
From 9917d563397ffac4b41af2e9d58dbf99efbf0924 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 18 Feb 2026 10:31:24 -0600
Subject: [PATCH v1 1/3] Remove unnecessary volatile qualifiers.
---
src/backend/replication/syncrep.c | 19 ++++++++-----------
src/backend/storage/ipc/procsignal.c | 4 ++--
src/backend/storage/lmgr/lock.c | 2 +-
src/test/modules/test_shm_mq/setup.c | 4 ++--
src/test/modules/test_shm_mq/worker.c | 2 +-
5 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index 7ea6001e9ad..045fd35786a 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -473,7 +473,6 @@ SyncRepInitConfig(void)
void
SyncRepReleaseWaiters(void)
{
- volatile WalSndCtlData *walsndctl = WalSndCtl;
XLogRecPtr writePtr;
XLogRecPtr flushPtr;
XLogRecPtr applyPtr;
@@ -548,19 +547,19 @@ SyncRepReleaseWaiters(void)
* Set the lsn first so that when we wake backends they will release up to
* this location.
*/
- if (walsndctl->lsn[SYNC_REP_WAIT_WRITE] < writePtr)
+ if (WalSndCtl->lsn[SYNC_REP_WAIT_WRITE] < writePtr)
{
- walsndctl->lsn[SYNC_REP_WAIT_WRITE] = writePtr;
+ WalSndCtl->lsn[SYNC_REP_WAIT_WRITE] = writePtr;
numwrite = SyncRepWakeQueue(false, SYNC_REP_WAIT_WRITE);
}
- if (walsndctl->lsn[SYNC_REP_WAIT_FLUSH] < flushPtr)
+ if (WalSndCtl->lsn[SYNC_REP_WAIT_FLUSH] < flushPtr)
{
- walsndctl->lsn[SYNC_REP_WAIT_FLUSH] = flushPtr;
+ WalSndCtl->lsn[SYNC_REP_WAIT_FLUSH] = flushPtr;
numflush = SyncRepWakeQueue(false, SYNC_REP_WAIT_FLUSH);
}
- if (walsndctl->lsn[SYNC_REP_WAIT_APPLY] < applyPtr)
+ if (WalSndCtl->lsn[SYNC_REP_WAIT_APPLY] < applyPtr)
{
- walsndctl->lsn[SYNC_REP_WAIT_APPLY] = applyPtr;
+ WalSndCtl->lsn[SYNC_REP_WAIT_APPLY] = applyPtr;
numapply = SyncRepWakeQueue(false, SYNC_REP_WAIT_APPLY);
}
@@ -767,8 +766,7 @@ SyncRepGetCandidateStandbys(SyncRepStandbyData **standbys)
n = 0;
for (i = 0; i < max_wal_senders; i++)
{
- volatile WalSnd *walsnd; /* Use volatile pointer to prevent code
- * rearrangement */
+ WalSnd *walsnd;
SyncRepStandbyData *stby;
WalSndState state; /* not included in SyncRepStandbyData */
@@ -905,7 +903,6 @@ SyncRepGetStandbyPriority(void)
static int
SyncRepWakeQueue(bool all, int mode)
{
- volatile WalSndCtlData *walsndctl = WalSndCtl;
int numprocs = 0;
dlist_mutable_iter iter;
@@ -920,7 +917,7 @@ SyncRepWakeQueue(bool all, int mode)
/*
* Assume the queue is ordered by LSN
*/
- if (!all && walsndctl->lsn[mode] < proc->waitLSN)
+ if (!all && WalSndCtl->lsn[mode] < proc->waitLSN)
return numprocs;
/*
diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index 5d33559926a..97791f979ef 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -283,7 +283,7 @@ CleanupProcSignalState(int status, Datum arg)
int
SendProcSignal(pid_t pid, ProcSignalReason reason, ProcNumber procNumber)
{
- volatile ProcSignalSlot *slot;
+ ProcSignalSlot *slot;
if (procNumber != INVALID_PROC_NUMBER)
{
@@ -394,7 +394,7 @@ EmitProcSignalBarrier(ProcSignalBarrierType type)
*/
for (int i = NumProcSignalSlots - 1; i >= 0; i--)
{
- volatile ProcSignalSlot *slot = &ProcSignal->psh_slot[i];
+ ProcSignalSlot *slot = &ProcSignal->psh_slot[i];
pid_t pid = pg_atomic_read_u32(&slot->pss_pid);
if (pid != 0)
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index e1168ad3837..718de1b5e98 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -309,7 +309,7 @@ typedef struct
uint32 count[FAST_PATH_STRONG_LOCK_HASH_PARTITIONS];
} FastPathStrongRelationLockData;
-static volatile FastPathStrongRelationLockData *FastPathStrongRelationLocks;
+static FastPathStrongRelationLockData *FastPathStrongRelationLocks;
/*
diff --git a/src/test/modules/test_shm_mq/setup.c b/src/test/modules/test_shm_mq/setup.c
index 579e5933d28..a1ec13d66b9 100644
--- a/src/test/modules/test_shm_mq/setup.c
+++ b/src/test/modules/test_shm_mq/setup.c
@@ -36,7 +36,7 @@ static worker_state *setup_background_workers(int nworkers,
dsm_segment *seg);
static void cleanup_background_workers(dsm_segment *seg, Datum arg);
static void wait_for_workers_to_become_ready(worker_state *wstate,
- volatile test_shm_mq_header *hdr);
+ test_shm_mq_header *hdr);
static bool check_worker_status(worker_state *wstate);
/* value cached, fetched from shared memory */
@@ -256,7 +256,7 @@ cleanup_background_workers(dsm_segment *seg, Datum arg)
static void
wait_for_workers_to_become_ready(worker_state *wstate,
- volatile test_shm_mq_header *hdr)
+ test_shm_mq_header *hdr)
{
bool result = false;
diff --git a/src/test/modules/test_shm_mq/worker.c b/src/test/modules/test_shm_mq/worker.c
index 368e4f3f234..14972f7e7cd 100644
--- a/src/test/modules/test_shm_mq/worker.c
+++ b/src/test/modules/test_shm_mq/worker.c
@@ -50,7 +50,7 @@ test_shm_mq_main(Datum main_arg)
shm_toc *toc;
shm_mq_handle *inqh;
shm_mq_handle *outqh;
- volatile test_shm_mq_header *hdr;
+ test_shm_mq_header *hdr;
int myworkernumber;
PGPROC *registrant;
--
2.50.1 (Apple Git-155)
From 5033d7108d4203a34fb7eb84f793494ded735803 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 18 Feb 2026 11:03:01 -0600
Subject: [PATCH v1 2/3] Remove SpinLockFree() and S_LOCK_FREE().
---
src/backend/storage/lmgr/s_lock.c | 24 ------------------------
src/include/storage/s_lock.h | 8 --------
src/include/storage/spin.h | 6 ------
3 files changed, 38 deletions(-)
diff --git a/src/backend/storage/lmgr/s_lock.c b/src/backend/storage/lmgr/s_lock.c
index 5b79556bc9c..6df568eccb3 100644
--- a/src/backend/storage/lmgr/s_lock.c
+++ b/src/backend/storage/lmgr/s_lock.c
@@ -262,12 +262,6 @@ main()
return 1;
}
- if (!S_LOCK_FREE(&test_lock.lock))
- {
- printf("S_LOCK_TEST: failed, lock not initialized\n");
- return 1;
- }
-
S_LOCK(&test_lock.lock);
if (test_lock.pad1 != 0x44 || test_lock.pad2 != 0x44)
@@ -276,12 +270,6 @@ main()
return 1;
}
- if (S_LOCK_FREE(&test_lock.lock))
- {
- printf("S_LOCK_TEST: failed, lock not locked\n");
- return 1;
- }
-
S_UNLOCK(&test_lock.lock);
if (test_lock.pad1 != 0x44 || test_lock.pad2 != 0x44)
@@ -290,12 +278,6 @@ main()
return 1;
}
- if (!S_LOCK_FREE(&test_lock.lock))
- {
- printf("S_LOCK_TEST: failed, lock not unlocked\n");
- return 1;
- }
-
S_LOCK(&test_lock.lock);
if (test_lock.pad1 != 0x44 || test_lock.pad2 != 0x44)
@@ -304,12 +286,6 @@ main()
return 1;
}
- if (S_LOCK_FREE(&test_lock.lock))
- {
- printf("S_LOCK_TEST: failed, lock not re-locked\n");
- return 1;
- }
-
printf("S_LOCK_TEST: this will print %d stars and then\n", NUM_DELAYS);
printf(" exit with a 'stuck spinlock' message\n");
printf(" if S_LOCK() and TAS() are working.\n");
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index 3d9070e79d4..89d28ac40c9 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -21,10 +21,6 @@
* void S_UNLOCK(slock_t *lock)
* Unlock a previously acquired lock.
*
- * bool S_LOCK_FREE(slock_t *lock)
- * Tests if the lock is free. Returns true if free, false if locked.
- * This does *not* change the state of the lock.
- *
* void SPIN_DELAY(void)
* Delay operation to occur inside spinlock wait loop.
*
@@ -671,10 +667,6 @@ spin_delay(void)
(TAS(lock) ? s_lock((lock), __FILE__, __LINE__, __func__) : 0)
#endif /* S_LOCK */
-#if !defined(S_LOCK_FREE)
-#define S_LOCK_FREE(lock) (*(lock) == 0)
-#endif /* S_LOCK_FREE */
-
#if !defined(S_UNLOCK)
/*
* Our default implementation of S_UNLOCK is essentially *(lock) = 0. This
diff --git a/src/include/storage/spin.h b/src/include/storage/spin.h
index 0f160d0e395..78c95ae538b 100644
--- a/src/include/storage/spin.h
+++ b/src/include/storage/spin.h
@@ -18,10 +18,6 @@
* void SpinLockRelease(volatile slock_t *lock)
* Unlock a previously acquired lock.
*
- * bool SpinLockFree(slock_t *lock)
- * Tests if the lock is free. Returns true if free, false if locked.
- * This does *not* change the state of the lock.
- *
* Callers must beware that the macro argument may be evaluated multiple
* times!
*
@@ -60,6 +56,4 @@
#define SpinLockRelease(lock) S_UNLOCK(lock)
-#define SpinLockFree(lock) S_LOCK_FREE(lock)
-
#endif /* SPIN_H */
--
2.50.1 (Apple Git-155)
From e757c04f3da5729400d466ffa8a0bf91f8cf272f Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 18 Feb 2026 10:32:26 -0600
Subject: [PATCH v1 3/3] Convert SpinLock* macros to static inline functions.
---
src/include/storage/spin.h | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/src/include/storage/spin.h b/src/include/storage/spin.h
index 78c95ae538b..5b680ff5dd8 100644
--- a/src/include/storage/spin.h
+++ b/src/include/storage/spin.h
@@ -5,22 +5,19 @@
*
*
* The interface to spinlocks is defined by the typedef "slock_t" and
- * these macros:
+ * these functions:
*
- * void SpinLockInit(volatile slock_t *lock)
+ * void SpinLockInit(slock_t *lock)
* Initialize a spinlock (to the unlocked state).
*
- * void SpinLockAcquire(volatile slock_t *lock)
+ * void SpinLockAcquire(slock_t *lock)
* Acquire a spinlock, waiting if necessary.
* Time out and abort() if unable to acquire the lock in a
* "reasonable" amount of time --- typically ~ 1 minute.
*
- * void SpinLockRelease(volatile slock_t *lock)
+ * void SpinLockRelease(slock_t *lock)
* Unlock a previously acquired lock.
*
- * Callers must beware that the macro argument may be evaluated multiple
- * times!
- *
* Load and store operations in calling code are guaranteed not to be
* reordered with respect to these operations, because they include a
* compiler barrier. (Before PostgreSQL 9.5, callers needed to use a
@@ -29,9 +26,9 @@
* Keep in mind the coding rule that spinlocks must not be held for more
* than a few instructions. In particular, we assume it is not possible
* for a CHECK_FOR_INTERRUPTS() to occur while holding a spinlock, and so
- * it is not necessary to do HOLD/RESUME_INTERRUPTS() in these macros.
+ * it is not necessary to do HOLD/RESUME_INTERRUPTS() in these functions.
*
- * These macros are implemented in terms of hardware-dependent macros
+ * These functions are implemented in terms of hardware-dependent macros
* supplied by s_lock.h. There is not currently any extra functionality
* added by this header, but there has been in the past and may someday
* be again.
@@ -49,11 +46,22 @@
#include "storage/s_lock.h"
+static inline void
+SpinLockInit(slock_t *lock)
+{
+ S_INIT_LOCK(lock);
+}
-#define SpinLockInit(lock) S_INIT_LOCK(lock)
-
-#define SpinLockAcquire(lock) S_LOCK(lock)
+static inline void
+SpinLockAcquire(slock_t *lock)
+{
+ S_LOCK(lock);
+}
-#define SpinLockRelease(lock) S_UNLOCK(lock)
+static inline void
+SpinLockRelease(slock_t *lock)
+{
+ S_UNLOCK(lock);
+}
#endif /* SPIN_H */
--
2.50.1 (Apple Git-155)
Attachments:
[text/plain] v1-0001-Remove-unnecessary-volatile-qualifiers.patch (5.2K, ../aZX2oUcKf7IzHnnK@nathan/2-v1-0001-Remove-unnecessary-volatile-qualifiers.patch)
download | inline diff:
From 9917d563397ffac4b41af2e9d58dbf99efbf0924 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 18 Feb 2026 10:31:24 -0600
Subject: [PATCH v1 1/3] Remove unnecessary volatile qualifiers.
---
src/backend/replication/syncrep.c | 19 ++++++++-----------
src/backend/storage/ipc/procsignal.c | 4 ++--
src/backend/storage/lmgr/lock.c | 2 +-
src/test/modules/test_shm_mq/setup.c | 4 ++--
src/test/modules/test_shm_mq/worker.c | 2 +-
5 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index 7ea6001e9ad..045fd35786a 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -473,7 +473,6 @@ SyncRepInitConfig(void)
void
SyncRepReleaseWaiters(void)
{
- volatile WalSndCtlData *walsndctl = WalSndCtl;
XLogRecPtr writePtr;
XLogRecPtr flushPtr;
XLogRecPtr applyPtr;
@@ -548,19 +547,19 @@ SyncRepReleaseWaiters(void)
* Set the lsn first so that when we wake backends they will release up to
* this location.
*/
- if (walsndctl->lsn[SYNC_REP_WAIT_WRITE] < writePtr)
+ if (WalSndCtl->lsn[SYNC_REP_WAIT_WRITE] < writePtr)
{
- walsndctl->lsn[SYNC_REP_WAIT_WRITE] = writePtr;
+ WalSndCtl->lsn[SYNC_REP_WAIT_WRITE] = writePtr;
numwrite = SyncRepWakeQueue(false, SYNC_REP_WAIT_WRITE);
}
- if (walsndctl->lsn[SYNC_REP_WAIT_FLUSH] < flushPtr)
+ if (WalSndCtl->lsn[SYNC_REP_WAIT_FLUSH] < flushPtr)
{
- walsndctl->lsn[SYNC_REP_WAIT_FLUSH] = flushPtr;
+ WalSndCtl->lsn[SYNC_REP_WAIT_FLUSH] = flushPtr;
numflush = SyncRepWakeQueue(false, SYNC_REP_WAIT_FLUSH);
}
- if (walsndctl->lsn[SYNC_REP_WAIT_APPLY] < applyPtr)
+ if (WalSndCtl->lsn[SYNC_REP_WAIT_APPLY] < applyPtr)
{
- walsndctl->lsn[SYNC_REP_WAIT_APPLY] = applyPtr;
+ WalSndCtl->lsn[SYNC_REP_WAIT_APPLY] = applyPtr;
numapply = SyncRepWakeQueue(false, SYNC_REP_WAIT_APPLY);
}
@@ -767,8 +766,7 @@ SyncRepGetCandidateStandbys(SyncRepStandbyData **standbys)
n = 0;
for (i = 0; i < max_wal_senders; i++)
{
- volatile WalSnd *walsnd; /* Use volatile pointer to prevent code
- * rearrangement */
+ WalSnd *walsnd;
SyncRepStandbyData *stby;
WalSndState state; /* not included in SyncRepStandbyData */
@@ -905,7 +903,6 @@ SyncRepGetStandbyPriority(void)
static int
SyncRepWakeQueue(bool all, int mode)
{
- volatile WalSndCtlData *walsndctl = WalSndCtl;
int numprocs = 0;
dlist_mutable_iter iter;
@@ -920,7 +917,7 @@ SyncRepWakeQueue(bool all, int mode)
/*
* Assume the queue is ordered by LSN
*/
- if (!all && walsndctl->lsn[mode] < proc->waitLSN)
+ if (!all && WalSndCtl->lsn[mode] < proc->waitLSN)
return numprocs;
/*
diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index 5d33559926a..97791f979ef 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -283,7 +283,7 @@ CleanupProcSignalState(int status, Datum arg)
int
SendProcSignal(pid_t pid, ProcSignalReason reason, ProcNumber procNumber)
{
- volatile ProcSignalSlot *slot;
+ ProcSignalSlot *slot;
if (procNumber != INVALID_PROC_NUMBER)
{
@@ -394,7 +394,7 @@ EmitProcSignalBarrier(ProcSignalBarrierType type)
*/
for (int i = NumProcSignalSlots - 1; i >= 0; i--)
{
- volatile ProcSignalSlot *slot = &ProcSignal->psh_slot[i];
+ ProcSignalSlot *slot = &ProcSignal->psh_slot[i];
pid_t pid = pg_atomic_read_u32(&slot->pss_pid);
if (pid != 0)
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index e1168ad3837..718de1b5e98 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -309,7 +309,7 @@ typedef struct
uint32 count[FAST_PATH_STRONG_LOCK_HASH_PARTITIONS];
} FastPathStrongRelationLockData;
-static volatile FastPathStrongRelationLockData *FastPathStrongRelationLocks;
+static FastPathStrongRelationLockData *FastPathStrongRelationLocks;
/*
diff --git a/src/test/modules/test_shm_mq/setup.c b/src/test/modules/test_shm_mq/setup.c
index 579e5933d28..a1ec13d66b9 100644
--- a/src/test/modules/test_shm_mq/setup.c
+++ b/src/test/modules/test_shm_mq/setup.c
@@ -36,7 +36,7 @@ static worker_state *setup_background_workers(int nworkers,
dsm_segment *seg);
static void cleanup_background_workers(dsm_segment *seg, Datum arg);
static void wait_for_workers_to_become_ready(worker_state *wstate,
- volatile test_shm_mq_header *hdr);
+ test_shm_mq_header *hdr);
static bool check_worker_status(worker_state *wstate);
/* value cached, fetched from shared memory */
@@ -256,7 +256,7 @@ cleanup_background_workers(dsm_segment *seg, Datum arg)
static void
wait_for_workers_to_become_ready(worker_state *wstate,
- volatile test_shm_mq_header *hdr)
+ test_shm_mq_header *hdr)
{
bool result = false;
diff --git a/src/test/modules/test_shm_mq/worker.c b/src/test/modules/test_shm_mq/worker.c
index 368e4f3f234..14972f7e7cd 100644
--- a/src/test/modules/test_shm_mq/worker.c
+++ b/src/test/modules/test_shm_mq/worker.c
@@ -50,7 +50,7 @@ test_shm_mq_main(Datum main_arg)
shm_toc *toc;
shm_mq_handle *inqh;
shm_mq_handle *outqh;
- volatile test_shm_mq_header *hdr;
+ test_shm_mq_header *hdr;
int myworkernumber;
PGPROC *registrant;
--
2.50.1 (Apple Git-155)
[text/plain] v1-0002-Remove-SpinLockFree-and-S_LOCK_FREE.patch (3.2K, ../aZX2oUcKf7IzHnnK@nathan/3-v1-0002-Remove-SpinLockFree-and-S_LOCK_FREE.patch)
download | inline diff:
From 5033d7108d4203a34fb7eb84f793494ded735803 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 18 Feb 2026 11:03:01 -0600
Subject: [PATCH v1 2/3] Remove SpinLockFree() and S_LOCK_FREE().
---
src/backend/storage/lmgr/s_lock.c | 24 ------------------------
src/include/storage/s_lock.h | 8 --------
src/include/storage/spin.h | 6 ------
3 files changed, 38 deletions(-)
diff --git a/src/backend/storage/lmgr/s_lock.c b/src/backend/storage/lmgr/s_lock.c
index 5b79556bc9c..6df568eccb3 100644
--- a/src/backend/storage/lmgr/s_lock.c
+++ b/src/backend/storage/lmgr/s_lock.c
@@ -262,12 +262,6 @@ main()
return 1;
}
- if (!S_LOCK_FREE(&test_lock.lock))
- {
- printf("S_LOCK_TEST: failed, lock not initialized\n");
- return 1;
- }
-
S_LOCK(&test_lock.lock);
if (test_lock.pad1 != 0x44 || test_lock.pad2 != 0x44)
@@ -276,12 +270,6 @@ main()
return 1;
}
- if (S_LOCK_FREE(&test_lock.lock))
- {
- printf("S_LOCK_TEST: failed, lock not locked\n");
- return 1;
- }
-
S_UNLOCK(&test_lock.lock);
if (test_lock.pad1 != 0x44 || test_lock.pad2 != 0x44)
@@ -290,12 +278,6 @@ main()
return 1;
}
- if (!S_LOCK_FREE(&test_lock.lock))
- {
- printf("S_LOCK_TEST: failed, lock not unlocked\n");
- return 1;
- }
-
S_LOCK(&test_lock.lock);
if (test_lock.pad1 != 0x44 || test_lock.pad2 != 0x44)
@@ -304,12 +286,6 @@ main()
return 1;
}
- if (S_LOCK_FREE(&test_lock.lock))
- {
- printf("S_LOCK_TEST: failed, lock not re-locked\n");
- return 1;
- }
-
printf("S_LOCK_TEST: this will print %d stars and then\n", NUM_DELAYS);
printf(" exit with a 'stuck spinlock' message\n");
printf(" if S_LOCK() and TAS() are working.\n");
diff --git a/src/include/storage/s_lock.h b/src/include/storage/s_lock.h
index 3d9070e79d4..89d28ac40c9 100644
--- a/src/include/storage/s_lock.h
+++ b/src/include/storage/s_lock.h
@@ -21,10 +21,6 @@
* void S_UNLOCK(slock_t *lock)
* Unlock a previously acquired lock.
*
- * bool S_LOCK_FREE(slock_t *lock)
- * Tests if the lock is free. Returns true if free, false if locked.
- * This does *not* change the state of the lock.
- *
* void SPIN_DELAY(void)
* Delay operation to occur inside spinlock wait loop.
*
@@ -671,10 +667,6 @@ spin_delay(void)
(TAS(lock) ? s_lock((lock), __FILE__, __LINE__, __func__) : 0)
#endif /* S_LOCK */
-#if !defined(S_LOCK_FREE)
-#define S_LOCK_FREE(lock) (*(lock) == 0)
-#endif /* S_LOCK_FREE */
-
#if !defined(S_UNLOCK)
/*
* Our default implementation of S_UNLOCK is essentially *(lock) = 0. This
diff --git a/src/include/storage/spin.h b/src/include/storage/spin.h
index 0f160d0e395..78c95ae538b 100644
--- a/src/include/storage/spin.h
+++ b/src/include/storage/spin.h
@@ -18,10 +18,6 @@
* void SpinLockRelease(volatile slock_t *lock)
* Unlock a previously acquired lock.
*
- * bool SpinLockFree(slock_t *lock)
- * Tests if the lock is free. Returns true if free, false if locked.
- * This does *not* change the state of the lock.
- *
* Callers must beware that the macro argument may be evaluated multiple
* times!
*
@@ -60,6 +56,4 @@
#define SpinLockRelease(lock) S_UNLOCK(lock)
-#define SpinLockFree(lock) S_LOCK_FREE(lock)
-
#endif /* SPIN_H */
--
2.50.1 (Apple Git-155)
[text/plain] v1-0003-Convert-SpinLock-macros-to-static-inline-function.patch (2.6K, ../aZX2oUcKf7IzHnnK@nathan/4-v1-0003-Convert-SpinLock-macros-to-static-inline-function.patch)
download | inline diff:
From e757c04f3da5729400d466ffa8a0bf91f8cf272f Mon Sep 17 00:00:00 2001
From: Nathan Bossart <[email protected]>
Date: Wed, 18 Feb 2026 10:32:26 -0600
Subject: [PATCH v1 3/3] Convert SpinLock* macros to static inline functions.
---
src/include/storage/spin.h | 34 +++++++++++++++++++++-------------
1 file changed, 21 insertions(+), 13 deletions(-)
diff --git a/src/include/storage/spin.h b/src/include/storage/spin.h
index 78c95ae538b..5b680ff5dd8 100644
--- a/src/include/storage/spin.h
+++ b/src/include/storage/spin.h
@@ -5,22 +5,19 @@
*
*
* The interface to spinlocks is defined by the typedef "slock_t" and
- * these macros:
+ * these functions:
*
- * void SpinLockInit(volatile slock_t *lock)
+ * void SpinLockInit(slock_t *lock)
* Initialize a spinlock (to the unlocked state).
*
- * void SpinLockAcquire(volatile slock_t *lock)
+ * void SpinLockAcquire(slock_t *lock)
* Acquire a spinlock, waiting if necessary.
* Time out and abort() if unable to acquire the lock in a
* "reasonable" amount of time --- typically ~ 1 minute.
*
- * void SpinLockRelease(volatile slock_t *lock)
+ * void SpinLockRelease(slock_t *lock)
* Unlock a previously acquired lock.
*
- * Callers must beware that the macro argument may be evaluated multiple
- * times!
- *
* Load and store operations in calling code are guaranteed not to be
* reordered with respect to these operations, because they include a
* compiler barrier. (Before PostgreSQL 9.5, callers needed to use a
@@ -29,9 +26,9 @@
* Keep in mind the coding rule that spinlocks must not be held for more
* than a few instructions. In particular, we assume it is not possible
* for a CHECK_FOR_INTERRUPTS() to occur while holding a spinlock, and so
- * it is not necessary to do HOLD/RESUME_INTERRUPTS() in these macros.
+ * it is not necessary to do HOLD/RESUME_INTERRUPTS() in these functions.
*
- * These macros are implemented in terms of hardware-dependent macros
+ * These functions are implemented in terms of hardware-dependent macros
* supplied by s_lock.h. There is not currently any extra functionality
* added by this header, but there has been in the past and may someday
* be again.
@@ -49,11 +46,22 @@
#include "storage/s_lock.h"
+static inline void
+SpinLockInit(slock_t *lock)
+{
+ S_INIT_LOCK(lock);
+}
-#define SpinLockInit(lock) S_INIT_LOCK(lock)
-
-#define SpinLockAcquire(lock) S_LOCK(lock)
+static inline void
+SpinLockAcquire(slock_t *lock)
+{
+ S_LOCK(lock);
+}
-#define SpinLockRelease(lock) S_UNLOCK(lock)
+static inline void
+SpinLockRelease(slock_t *lock)
+{
+ S_UNLOCK(lock);
+}
#endif /* SPIN_H */
--
2.50.1 (Apple Git-155)
view thread (2+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: Re: convert SpinLock* macros to static inline functions
In-Reply-To: <aZX2oUcKf7IzHnnK@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox