agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Nathan Bossart <nathandbossart@gmail.com>
To: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Andres Freund <andres@anarazel.de>
Cc: Peter Eisentraut <peter@eisentraut.org>
Cc: pgsql-hackers@postgresql.org
Subject: Re: convert various variables to atomics
Date: Tue, 22 Sep 2026 14:50:19 -0500
Message-ID: <arLb-wxKaDHcvSCA@nathan> (raw)
In-Reply-To: <aqBmuOO26ZtG_BgX@nathan>
References: <9d8c317d-d933-46c7-b675-4b9308eaca2b@eisentraut.org>
<amDAZAVob4DJjUfW@nathan>
<ycvruij7554tlhw6w7bg4lqtibd52qguo52iojyou7ejdv5jrm@qyyr5f3hdsue>
<amDGRZxnlmTVjkCe@nathan>
<amJx4Lwx4nuuExYT@nathan>
<207c0bfb-6e06-4358-bb2f-c961915efc36@eisentraut.org>
<hi4rsxwu3ioas5rmuwfnu2gisqb2rd6g2uq56r3pvzk7clismo@oyndzrrylnir>
<3856d1cf-53a8-414b-98d9-829d5a455a86@iki.fi>
<aqA_P7Uwub-MDXOO@nathan>
<aqBmuOO26ZtG_BgX@nathan>
I've now committed everything except for these last two patches, which I'm
planning to commit tomorrow.
--
nathan
From 8438305333b38a31c101f0051cc8b86aaec1e087 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 22 Sep 2026 14:31:14 -0500
Subject: [PATCH v4 1/2] 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.
Author: Yura Sokolov <y.sokolov@postgrespro.ru>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/30aa0030-f694-44ef-a19d-6ef7ddb69374%40postgrespro.ru
Discussion: https://postgr.es/m/alAJeRRzehDjLaF1%40nathan
---
src/backend/storage/ipc/sinvaladt.c | 32 ++++++++++++++++-------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c
index 37a21ffaf1a..b29b4bcc5be 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,19 @@ 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.
*/
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 9903d1cdc01f12acb510ffa8f22ddb1048ec1f48 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 22 Sep 2026 14:36:42 -0500
Subject: [PATCH v4 2/2] 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.
Author: Yura Sokolov <y.sokolov@postgrespro.ru>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Tested-by: solai v <solai.cdac@gmail.com>
Discussion: https://postgr.es/m/30aa0030-f694-44ef-a19d-6ef7ddb69374%40postgrespro.ru
Discussion: https://postgr.es/m/alAJeRRzehDjLaF1%40nathan
---
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 b29b4bcc5be..bc5f9537710 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)
{
@@ -601,7 +584,7 @@ SICleanupQueue(bool callerHasWriteLock, int minFree)
* Note that the thresholds are clamped at zero rather than allowed to
* wrap around.
*/
- 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;
@@ -648,7 +631,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;
}
@@ -657,7 +640,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
Attachments:
[text/plain] v4-0001-Use-unsigned-integers-for-sinval-message-numbers.patch (4.6K, ../arLb-wxKaDHcvSCA@nathan/2-v4-0001-Use-unsigned-integers-for-sinval-message-numbers.patch)
download | inline diff:
From 8438305333b38a31c101f0051cc8b86aaec1e087 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 22 Sep 2026 14:31:14 -0500
Subject: [PATCH v4 1/2] 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.
Author: Yura Sokolov <y.sokolov@postgrespro.ru>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Discussion: https://postgr.es/m/30aa0030-f694-44ef-a19d-6ef7ddb69374%40postgrespro.ru
Discussion: https://postgr.es/m/alAJeRRzehDjLaF1%40nathan
---
src/backend/storage/ipc/sinvaladt.c | 32 ++++++++++++++++-------------
1 file changed, 18 insertions(+), 14 deletions(-)
diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c
index 37a21ffaf1a..b29b4bcc5be 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,19 @@ 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.
*/
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
[text/plain] v4-0002-Convert-SISeg-maxMsgNum-to-an-atomic-variable.patch (6.6K, ../arLb-wxKaDHcvSCA@nathan/3-v4-0002-Convert-SISeg-maxMsgNum-to-an-atomic-variable.patch)
download | inline diff:
From 9903d1cdc01f12acb510ffa8f22ddb1048ec1f48 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Tue, 22 Sep 2026 14:36:42 -0500
Subject: [PATCH v4 2/2] 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.
Author: Yura Sokolov <y.sokolov@postgrespro.ru>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Peter Eisentraut <peter@eisentraut.org>
Reviewed-by: Andres Freund <andres@anarazel.de>
Reviewed-by: Zsolt Parragi <zsolt.parragi@percona.com>
Tested-by: solai v <solai.cdac@gmail.com>
Discussion: https://postgr.es/m/30aa0030-f694-44ef-a19d-6ef7ddb69374%40postgrespro.ru
Discussion: https://postgr.es/m/alAJeRRzehDjLaF1%40nathan
---
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 b29b4bcc5be..bc5f9537710 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)
{
@@ -601,7 +584,7 @@ SICleanupQueue(bool callerHasWriteLock, int minFree)
* Note that the thresholds are clamped at zero rather than allowed to
* wrap around.
*/
- 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;
@@ -648,7 +631,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;
}
@@ -657,7 +640,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
view thread (22+ messages) latest in thread
Message-ID: <arLb-wxKaDHcvSCA@nathan>
Permalink: ../arLb-wxKaDHcvSCA@nathan/
Also on: postgresql.org/message-id/arLb-wxKaDHcvSCA@nathan
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: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, hlinnaka@iki.fi, andres@anarazel.de, peter@eisentraut.org
Subject: Re: convert various variables to atomics
In-Reply-To: <arLb-wxKaDHcvSCA@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