On Tue, Sep 08, 2026 at 12:00:47PM -0500, Nathan Bossart wrote: > * v2-0001: We are changing a variable from signed to unsigned, but the code > goes out of its way to avoid negative values and signed integer overflow, > so I don't think there are any real problems here. The only atomic > arithmetic operation is in SICleanupQueue() where we subtract > MSGNUMWRAPAROUND, which IIUC should never produce a negative value. That > being said, I don't think it would be too disruptive to switch all relevant > variables to uint32 as a prerequisite patch. I don't see any particular > reason for those variables to be signed, anyway.
v3-0001 is the prerequisite patch. This requires some new clamping logic in SICleanupQueue() for minsig and lowbound, since the subtractions can produce negative values. I believe this retains the existing behavior, but need to double-check. > * v2-0006: The variables in this one are only ever incremented by 1, and > they track the number of workers for a given operation, which I can't > imagine approaches anything even close to overflowing an integer. Not to > mention that we're using signed integers for all the relevant variables > today... I don't see any risk here, but I'll try to switch the relevant > variables to unsigned as a prerequisite and see how it looks. If it's too > invasive, it's probably not worth worrying about. Yeah, this looks far too invasive. I left it alone. > * v2-0007: I think this one already does all the work to avoid any signed > versus unsigned mismatches. The Assert() in SharedFileSetOnDetach() looks > bogus, though, so I'll fix that. I guess there could be some risk of > overflow in the "refcnt + 1" in SharedFileSetAttach(), but we don't handle > that at all today, so I don't think we need to worry about it. (In theory > this patch actually reduces the overflow risk by switching to unsigned, > anyway.) Upon closer inspection, the Assert() looks fine. I'm not sure why I thought it was bogus. -- nathan
>From e9ea728d9378e7abc9d85d39874b535c378bb7c3 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Tue, 8 Sep 2026 12:40:05 -0500 Subject: [PATCH v3 1/6] Use unsigned integers for sinval message numbers. Currently, the message numbers in sinvaladt.c are ints, but they are never negative, and the code already takes pains to keep them from overflowing. This commit changes them to uint32. The only wrinkle is that SICleanupQueue() computes two thresholds by subtracting from maxMsgNum, and those could previously go negative. They are now clamped at zero, which disables the corresponding checks just as a negative threshold did. This is preparatory work for a follow-up commit that will convert maxMsgNum to an unsigned atomic variable. --- src/backend/storage/ipc/sinvaladt.c | 33 +++++++++++++++++------------ 1 file changed, 19 insertions(+), 14 deletions(-) diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c index 37a21ffaf1a..4e6f9a84375 100644 --- a/src/backend/storage/ipc/sinvaladt.c +++ b/src/backend/storage/ipc/sinvaladt.c @@ -93,7 +93,7 @@ * read maxMsgNum if you are not holding SInvalWriteLock, and you need the * spinlock to write maxMsgNum unless you are holding both locks.) * - * Note: since maxMsgNum is an int and hence presumably atomically readable/ + * Note: since maxMsgNum is a uint32 and hence presumably atomically readable/ * writable, the spinlock might seem unnecessary. The reason it is needed * is to provide a memory barrier: we need to be sure that messages written * to the array are actually there before maxMsgNum is increased, and that @@ -140,7 +140,7 @@ typedef struct ProcState /* procPid is zero in an inactive ProcState array entry. */ pid_t procPid; /* PID of backend, for signaling */ /* nextMsgNum is meaningless if procPid == 0 or resetState is true. */ - int nextMsgNum; /* next message number to read */ + uint32 nextMsgNum; /* next message number to read */ bool resetState; /* backend needs to reset its state */ bool signaled; /* backend has been sent catchup signal */ bool hasMessages; /* backend has unread messages */ @@ -168,9 +168,9 @@ typedef struct SISeg /* * General state information */ - int minMsgNum; /* oldest message still needed */ - int maxMsgNum; /* next message number to be assigned */ - int nextThreshold; /* # of messages to call SICleanupQueue */ + uint32 minMsgNum; /* oldest message still needed */ + uint32 maxMsgNum; /* next message number to be assigned */ + uint32 nextThreshold; /* # of messages to call SICleanupQueue */ slock_t msgnumLock; /* spinlock protecting maxMsgNum */ @@ -385,8 +385,8 @@ SIInsertDataEntries(const SharedInvalidationMessage *data, int n) while (n > 0) { int nthistime = Min(n, WRITE_QUANTUM); - int numMsgs; - int max; + uint32 numMsgs; + uint32 max; int i; n -= nthistime; @@ -476,7 +476,7 @@ SIGetDataEntries(SharedInvalidationMessage *data, int datasize) { SISeg *segP; ProcState *stateP; - int max; + uint32 max; int n; segP = shmInvalBuffer; @@ -579,11 +579,11 @@ void SICleanupQueue(bool callerHasWriteLock, int minFree) { SISeg *segP = shmInvalBuffer; - int min, + uint32 min, minsig, lowbound, - numMsgs, - i; + numMsgs; + int i; ProcState *needSig = NULL; /* Lock out all writers and readers */ @@ -597,15 +597,20 @@ SICleanupQueue(bool callerHasWriteLock, int minFree) * backends that are too far back. Note that because we ignore sendOnly * backends here it is possible for them to keep sending messages without * a problem even when they are the only active backend. + * + * Note that the thresholds are clamped at zero rather than allowed to + * wrap around. A threshold of zero disables its check, just as a + * negative one would. */ min = segP->maxMsgNum; - minsig = min - SIG_THRESHOLD; - lowbound = min - MAXNUMMESSAGES + minFree; + minsig = (min > SIG_THRESHOLD) ? min - SIG_THRESHOLD : 0; + lowbound = (min + minFree > MAXNUMMESSAGES) ? + min + minFree - MAXNUMMESSAGES : 0; for (i = 0; i < segP->numProcs; i++) { ProcState *stateP = &segP->procState[segP->pgprocnos[i]]; - int n = stateP->nextMsgNum; + uint32 n = stateP->nextMsgNum; /* Ignore if already in reset state */ Assert(stateP->procPid != 0); -- 2.55.0
>From 6d62b5e5ee4899fd7f979063f471be079951cfa4 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Tue, 8 Sep 2026 12:40:05 -0500 Subject: [PATCH v3 2/6] Convert SISeg->maxMsgNum to an atomic variable. Currently, this variable is a uint32 protected by a spinlock. The spinlock exists only to provide memory barriers, so by converting the variable to an atomic and using the barrier-providing accessors in the spinlock's place, we can remove the spinlock. --- src/backend/storage/ipc/sinvaladt.c | 51 ++++++++++------------------- 1 file changed, 17 insertions(+), 34 deletions(-) diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c index 4e6f9a84375..5c0cb317101 100644 --- a/src/backend/storage/ipc/sinvaladt.c +++ b/src/backend/storage/ipc/sinvaladt.c @@ -24,7 +24,6 @@ #include "storage/procsignal.h" #include "storage/shmem.h" #include "storage/sinvaladt.h" -#include "storage/spin.h" #include "storage/subsystems.h" /* @@ -87,19 +86,10 @@ * has no need to touch anyone's ProcState, except in the infrequent cases * when SICleanupQueue is needed. The only point of overlap is that * the writer wants to change maxMsgNum while readers need to read it. - * We deal with that by having a spinlock that readers must take for just - * long enough to read maxMsgNum, while writers take it for just long enough - * to write maxMsgNum. (The exact rule is that you need the spinlock to - * read maxMsgNum if you are not holding SInvalWriteLock, and you need the - * spinlock to write maxMsgNum unless you are holding both locks.) - * - * Note: since maxMsgNum is a uint32 and hence presumably atomically readable/ - * writable, the spinlock might seem unnecessary. The reason it is needed - * is to provide a memory barrier: we need to be sure that messages written - * to the array are actually there before maxMsgNum is increased, and that - * readers will see that data after fetching maxMsgNum. Multiprocessors - * that have weak memory-ordering guarantees can fail without the memory - * barrier instructions that are included in the spinlock sequences. + * We deal with that by making maxMsgNum an atomic variable. (The exact rule + * is that you need to use a barrier-providing accessor to read maxMsgNum if + * you are not holding SInvalWriteLock, and you need a barrier-providing + * accessor to write maxMsgNum unless you are holding both locks.) */ @@ -169,11 +159,9 @@ typedef struct SISeg * General state information */ uint32 minMsgNum; /* oldest message still needed */ - uint32 maxMsgNum; /* next message number to be assigned */ + pg_atomic_uint32 maxMsgNum; /* next message number to be assigned */ uint32 nextThreshold; /* # of messages to call SICleanupQueue */ - slock_t msgnumLock; /* spinlock protecting maxMsgNum */ - /* * Circular buffer holding shared-inval messages */ @@ -244,11 +232,10 @@ SharedInvalShmemInit(void *arg) { int i; - /* Clear message counters, init spinlock */ + /* Clear message counters */ shmInvalBuffer->minMsgNum = 0; - shmInvalBuffer->maxMsgNum = 0; + pg_atomic_init_u32(&shmInvalBuffer->maxMsgNum, 0); shmInvalBuffer->nextThreshold = CLEANUP_MIN; - SpinLockInit(&shmInvalBuffer->msgnumLock); /* The buffer[] array is initially all unused, so we need not fill it */ @@ -306,7 +293,7 @@ SharedInvalBackendInit(bool sendOnly) /* mark myself active, with all extant messages already read */ stateP->procPid = MyProcPid; - stateP->nextMsgNum = segP->maxMsgNum; + stateP->nextMsgNum = pg_atomic_read_u32(&segP->maxMsgNum); stateP->resetState = false; stateP->signaled = false; stateP->hasMessages = false; @@ -402,7 +389,7 @@ SIInsertDataEntries(const SharedInvalidationMessage *data, int n) */ for (;;) { - numMsgs = segP->maxMsgNum - segP->minMsgNum; + numMsgs = pg_atomic_read_u32(&segP->maxMsgNum) - segP->minMsgNum; if (numMsgs + nthistime > MAXNUMMESSAGES || numMsgs >= segP->nextThreshold) SICleanupQueue(true, nthistime); @@ -413,17 +400,15 @@ SIInsertDataEntries(const SharedInvalidationMessage *data, int n) /* * Insert new message(s) into proper slot of circular buffer */ - max = segP->maxMsgNum; + max = pg_atomic_read_u32(&segP->maxMsgNum); while (nthistime-- > 0) { segP->buffer[max % MAXNUMMESSAGES] = *data++; max++; } - /* Update current value of maxMsgNum using spinlock */ - SpinLockAcquire(&segP->msgnumLock); - segP->maxMsgNum = max; - SpinLockRelease(&segP->msgnumLock); + /* Update current value of maxMsgNum using barrier */ + pg_atomic_write_membarrier_u32(&segP->maxMsgNum, max); /* * Now that the maxMsgNum change is globally visible, we give everyone @@ -509,10 +494,8 @@ SIGetDataEntries(SharedInvalidationMessage *data, int datasize) */ stateP->hasMessages = false; - /* Fetch current value of maxMsgNum using spinlock */ - SpinLockAcquire(&segP->msgnumLock); - max = segP->maxMsgNum; - SpinLockRelease(&segP->msgnumLock); + /* Fetch current value of maxMsgNum using barrier */ + max = pg_atomic_read_membarrier_u32(&segP->maxMsgNum); if (stateP->resetState) { @@ -602,7 +585,7 @@ SICleanupQueue(bool callerHasWriteLock, int minFree) * wrap around. A threshold of zero disables its check, just as a * negative one would. */ - min = segP->maxMsgNum; + min = pg_atomic_read_u32(&segP->maxMsgNum); minsig = (min > SIG_THRESHOLD) ? min - SIG_THRESHOLD : 0; lowbound = (min + minFree > MAXNUMMESSAGES) ? min + minFree - MAXNUMMESSAGES : 0; @@ -649,7 +632,7 @@ SICleanupQueue(bool callerHasWriteLock, int minFree) if (min >= MSGNUMWRAPAROUND) { segP->minMsgNum -= MSGNUMWRAPAROUND; - segP->maxMsgNum -= MSGNUMWRAPAROUND; + pg_atomic_fetch_sub_u32(&segP->maxMsgNum, MSGNUMWRAPAROUND); for (i = 0; i < segP->numProcs; i++) segP->procState[segP->pgprocnos[i]].nextMsgNum -= MSGNUMWRAPAROUND; } @@ -658,7 +641,7 @@ SICleanupQueue(bool callerHasWriteLock, int minFree) * Determine how many messages are still in the queue, and set the * threshold at which we should repeat SICleanupQueue(). */ - numMsgs = segP->maxMsgNum - segP->minMsgNum; + numMsgs = pg_atomic_read_u32(&segP->maxMsgNum) - segP->minMsgNum; if (numMsgs < CLEANUP_MIN) segP->nextThreshold = CLEANUP_MIN; else -- 2.55.0
>From a131db20b366f83587358b7272746b6ca061cbae Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Tue, 8 Sep 2026 12:40:05 -0500 Subject: [PATCH v3 3/6] Convert ParallelBitmapHeapState->state to an atomic variable. Currently, this variable is a SharedBitmapState protected by a spinlock. By converting it to an atomic variable, we can remove the spinlock. --- src/backend/executor/nodeBitmapHeapscan.c | 23 +++++++---------------- 1 file changed, 7 insertions(+), 16 deletions(-) diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index 83d6478bc2b..a9e83f30687 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -79,7 +79,6 @@ typedef enum /* ---------------- * ParallelBitmapHeapState information * tbmiterator iterator for scanning current pages - * mutex mutual exclusion for state * state current state of the TIDBitmap * cv conditional wait variable * ---------------- @@ -87,8 +86,7 @@ typedef enum typedef struct ParallelBitmapHeapState { dsa_pointer tbmiterator; - slock_t mutex; - SharedBitmapState state; + pg_atomic_uint32 state; ConditionVariable cv; } ParallelBitmapHeapState; @@ -228,9 +226,7 @@ BitmapHeapNext(BitmapHeapScanState *node) static inline void BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate) { - SpinLockAcquire(&pstate->mutex); - pstate->state = BM_FINISHED; - SpinLockRelease(&pstate->mutex); + pg_atomic_write_membarrier_u32(&pstate->state, BM_FINISHED); ConditionVariableBroadcast(&pstate->cv); } @@ -476,15 +472,12 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) static bool BitmapShouldInitializeSharedState(ParallelBitmapHeapState *pstate) { - SharedBitmapState state; + uint32 state; while (1) { - SpinLockAcquire(&pstate->mutex); - state = pstate->state; - if (pstate->state == BM_INITIAL) - pstate->state = BM_INPROGRESS; - SpinLockRelease(&pstate->mutex); + state = BM_INITIAL; + pg_atomic_compare_exchange_u32(&pstate->state, &state, BM_INPROGRESS); /* Exit if bitmap is done, or if we're the leader. */ if (state != BM_INPROGRESS) @@ -538,9 +531,7 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node, pstate->tbmiterator = 0; - /* Initialize the mutex */ - SpinLockInit(&pstate->mutex); - pstate->state = BM_INITIAL; + pg_atomic_init_u32(&pstate->state, BM_INITIAL); ConditionVariableInit(&pstate->cv); @@ -565,7 +556,7 @@ ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node, if (dsa == NULL) return; - pstate->state = BM_INITIAL; + pg_atomic_write_u32(&pstate->state, BM_INITIAL); if (DsaPointerIsValid(pstate->tbmiterator)) tbm_free_shared_area(dsa, pstate->tbmiterator); -- 2.55.0
>From 7499e0df53f100ea281f4ccc68a43039fd0e41d4 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Tue, 8 Sep 2026 12:40:06 -0500 Subject: [PATCH v3 4/6] Convert PROC_HDR->startupBufferPinWaitBuf to an atomic variable. Currently, this variable is a Buffer that is accessed via a volatile pointer. By converting it to an atomic variable, we can remove the volatile qualifiers. No barriers are needed; as the comment there notes, the value is published before the backends that read it are signaled. --- src/backend/storage/lmgr/proc.c | 12 +++--------- src/include/storage/proc.h | 2 +- 2 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index ab65a6dbcc9..91fe2766640 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -238,7 +238,7 @@ ProcGlobalShmemInit(void *arg) dlist_init(&ProcGlobal->autovacFreeProcs); dlist_init(&ProcGlobal->bgworkerFreeProcs); dlist_init(&ProcGlobal->walsenderFreeProcs); - ProcGlobal->startupBufferPinWaitBuf = InvalidBuffer; + pg_atomic_init_u32(&ProcGlobal->startupBufferPinWaitBuf, InvalidBuffer); pg_atomic_init_u32(&ProcGlobal->avLauncherProc, INVALID_PROC_NUMBER); pg_atomic_init_u32(&ProcGlobal->walwriterProc, INVALID_PROC_NUMBER); pg_atomic_init_u32(&ProcGlobal->checkpointerProc, INVALID_PROC_NUMBER); @@ -767,10 +767,7 @@ InitAuxiliaryProcess(void) void SetStartupBufferPinWaitBuf(Buffer buffer) { - /* use volatile pointer to prevent code rearrangement */ - volatile PROC_HDR *procglobal = ProcGlobal; - - procglobal->startupBufferPinWaitBuf = buffer; + pg_atomic_write_u32(&ProcGlobal->startupBufferPinWaitBuf, buffer); } /* @@ -779,10 +776,7 @@ SetStartupBufferPinWaitBuf(Buffer buffer) Buffer GetStartupBufferPinWaitBuf(void) { - /* use volatile pointer to prevent code rearrangement */ - volatile PROC_HDR *procglobal = ProcGlobal; - - return procglobal->startupBufferPinWaitBuf; + return pg_atomic_read_u32(&ProcGlobal->startupBufferPinWaitBuf); } /* diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 4c3f431b4eb..abe40001d9a 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -500,7 +500,7 @@ typedef struct PROC_HDR /* Current shared estimate of appropriate spins_per_delay value */ int spins_per_delay; /* Buffer that Startup process waits for pin on, or InvalidBuffer */ - Buffer startupBufferPinWaitBuf; + pg_atomic_uint32 startupBufferPinWaitBuf; } PROC_HDR; extern PGDLLIMPORT PROC_HDR *ProcGlobal; -- 2.55.0
>From baa8667d3cdffc14dcab7858734e70fff06cfb75 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Tue, 8 Sep 2026 12:40:06 -0500 Subject: [PATCH v3 5/6] Convert Sharedsort's worker counters to atomic variables. Currently, currentWorker and workersFinished are ints protected by a spinlock. By converting them to atomic variables, we can remove the spinlock. Note that worker_freeze_result_tape() also stores the worker's tape metadata in shared memory within the spinlock's critical section, but each worker writes only its own slot, and the fetch-and-add that follows provides the barrier that the leader depends on when it reads the tapes. --- src/backend/utils/sort/tuplesort.c | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/src/backend/utils/sort/tuplesort.c b/src/backend/utils/sort/tuplesort.c index f67651483f8..37c40763ee0 100644 --- a/src/backend/utils/sort/tuplesort.c +++ b/src/backend/utils/sort/tuplesort.c @@ -104,6 +104,7 @@ #include "commands/tablespace.h" #include "miscadmin.h" #include "pg_trace.h" +#include "port/atomics.h" #include "port/pg_bitutils.h" #include "storage/shmem.h" #include "utils/guc.h" @@ -340,9 +341,6 @@ struct Tuplesortstate */ struct Sharedsort { - /* mutex protects all fields prior to tapes */ - slock_t mutex; - /* * currentWorker generates ordinal identifier numbers for parallel sort * workers. These start from 0, and are always gapless. @@ -351,8 +349,8 @@ struct Sharedsort * is equal to state.nParticipants within the leader, leader is ready to * merge worker runs. */ - int currentWorker; - int workersFinished; + pg_atomic_uint32 currentWorker; + pg_atomic_uint32 workersFinished; /* Temporary file space */ SharedFileSet fileset; @@ -3254,9 +3252,8 @@ tuplesort_initialize_shared(Sharedsort *shared, int nWorkers, dsm_segment *seg) Assert(nWorkers > 0); - SpinLockInit(&shared->mutex); - shared->currentWorker = 0; - shared->workersFinished = 0; + pg_atomic_init_u32(&shared->currentWorker, 0); + pg_atomic_init_u32(&shared->workersFinished, 0); SharedFileSetInit(&shared->fileset, seg); shared->nTapes = nWorkers; for (i = 0; i < nWorkers; i++) @@ -3293,16 +3290,9 @@ tuplesort_attach_shared(Sharedsort *shared, dsm_segment *seg) static int worker_get_identifier(Tuplesortstate *state) { - Sharedsort *shared = state->shared; - int worker; - Assert(WORKER(state)); - SpinLockAcquire(&shared->mutex); - worker = shared->currentWorker++; - SpinLockRelease(&shared->mutex); - - return worker; + return pg_atomic_fetch_add_u32(&state->shared->currentWorker, 1); } /* @@ -3344,10 +3334,8 @@ worker_freeze_result_tape(Tuplesortstate *state) LogicalTapeFreeze(state->result_tape, &output); /* Store properties of output tape, and update finished worker count */ - SpinLockAcquire(&shared->mutex); shared->tapes[state->worker] = output; - shared->workersFinished++; - SpinLockRelease(&shared->mutex); + pg_atomic_fetch_add_u32(&shared->workersFinished, 1); } /* @@ -3389,9 +3377,7 @@ leader_takeover_tapes(Tuplesortstate *state) Assert(LEADER(state)); Assert(nParticipants >= 1); - SpinLockAcquire(&shared->mutex); - workersFinished = shared->workersFinished; - SpinLockRelease(&shared->mutex); + workersFinished = pg_atomic_read_membarrier_u32(&shared->workersFinished); if (nParticipants != workersFinished) elog(ERROR, "cannot take over tapes before all workers finish"); -- 2.55.0
>From 62473354494adaea6064c1c75a0afd8b7b46f634 Mon Sep 17 00:00:00 2001 From: Nathan Bossart <[email protected]> Date: Tue, 8 Sep 2026 12:40:06 -0500 Subject: [PATCH v3 6/6] Convert SharedFileSet->refcnt to an atomic variable. Currently, this variable is an int protected by a spinlock. By converting it to an atomic variable, we can remove the spinlock. Detaching becomes an atomic subtract, and attaching becomes a compare-and-exchange loop, since it must not resurrect a fileset whose reference count has already reached zero. --- src/backend/storage/file/sharedfileset.c | 27 +++++++++--------------- src/include/storage/sharedfileset.h | 5 ++--- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/src/backend/storage/file/sharedfileset.c b/src/backend/storage/file/sharedfileset.c index d76bd72dc63..4f12f92beae 100644 --- a/src/backend/storage/file/sharedfileset.c +++ b/src/backend/storage/file/sharedfileset.c @@ -38,8 +38,7 @@ void SharedFileSetInit(SharedFileSet *fileset, dsm_segment *seg) { /* Initialize the shared fileset specific members. */ - SpinLockInit(&fileset->mutex); - fileset->refcnt = 1; + pg_atomic_init_u32(&fileset->refcnt, 1); /* Initialize the fileset. */ FileSetInit(&fileset->fs); @@ -55,19 +54,15 @@ SharedFileSetInit(SharedFileSet *fileset, dsm_segment *seg) void SharedFileSetAttach(SharedFileSet *fileset, dsm_segment *seg) { - bool success; + uint32 refcnt; - SpinLockAcquire(&fileset->mutex); - if (fileset->refcnt == 0) - success = false; - else - { - ++fileset->refcnt; - success = true; - } - SpinLockRelease(&fileset->mutex); + refcnt = pg_atomic_read_u32(&fileset->refcnt); + while (refcnt != 0 && + !pg_atomic_compare_exchange_u32(&fileset->refcnt, &refcnt, + refcnt + 1)) + ; - if (!success) + if (refcnt == 0) ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("could not attach to a SharedFileSet that is already destroyed"))); @@ -98,11 +93,9 @@ SharedFileSetOnDetach(dsm_segment *segment, Datum datum) bool unlink_all = false; SharedFileSet *fileset = (SharedFileSet *) DatumGetPointer(datum); - SpinLockAcquire(&fileset->mutex); - Assert(fileset->refcnt > 0); - if (--fileset->refcnt == 0) + Assert(pg_atomic_read_u32(&fileset->refcnt) > 0); + if (pg_atomic_sub_fetch_u32(&fileset->refcnt, 1) == 0) unlink_all = true; - SpinLockRelease(&fileset->mutex); /* * If we are the last to detach, we delete the directory in all diff --git a/src/include/storage/sharedfileset.h b/src/include/storage/sharedfileset.h index 904396e7173..d89626ae64b 100644 --- a/src/include/storage/sharedfileset.h +++ b/src/include/storage/sharedfileset.h @@ -15,10 +15,10 @@ #ifndef SHAREDFILESET_H #define SHAREDFILESET_H +#include "port/atomics.h" #include "storage/dsm.h" #include "storage/fd.h" #include "storage/fileset.h" -#include "storage/spin.h" /* * A set of temporary files that can be shared by multiple backends. @@ -26,8 +26,7 @@ typedef struct SharedFileSet { FileSet fs; - slock_t mutex; /* mutex protecting the reference count */ - int refcnt; /* number of attached backends */ + pg_atomic_uint32 refcnt; /* number of attached backends */ } SharedFileSet; extern void SharedFileSetInit(SharedFileSet *fileset, dsm_segment *seg); -- 2.55.0
