Remove unnecessary volatile qualifiers.

This commit cleans up volatile qualifiers that fit the below
criteria:

* Accesses to shared memory protected by a spinlock or LWLock.
Before commit 0709b7ee72, callers had to use volatile when
accessing spinlock-protected shared memory.  Since spinlock
acquire/release became compiler barriers, and because LWLocks
provide the same guarantee, that is no longer necessary.  These
either predate that change or were cargo-culted from code that did.

* Pointers used only to find the address of a member.  The volatile
qualifier only affects accesses made by dereferencing the pointer,
so it is unnecessary there.

* Accesses to struct members that are marked volatile in the struct
definition.  There's no need to mark these pointers volatile,
either.

* Leftovers from removed PG_TRY blocks.  These were marked volatile
to protect a value that is modified inside a PG_TRY block, but the
PG_TRY has since been removed.

Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://postgr.es/m/akQ5eJR1tCCXme8e%40nathan

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 75012d4b8f01c8ea278abcac4c9c3b270f678d9a..6f7f6b86eb62bbe558fe10dcc87354734019ba59 100644 (file)
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -450,7 +450,6 @@ static bool
 TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
                                                                XLogRecPtr lsn, int64 pageno)
 {
-       volatile PROC_HDR *procglobal = ProcGlobal;
        PGPROC     *proc = MyProc;
        uint32          nextidx;
        uint32          wakeidx;
@@ -493,7 +492,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
         * different from ours.  If another group starts to update a page in the
         * same bank as ours, they wait until we release the lock.
         */
-       nextidx = pg_atomic_read_u32(&procglobal->clogGroupFirst);
+       nextidx = pg_atomic_read_u32(&ProcGlobal->clogGroupFirst);
 
        while (true)
        {
@@ -524,7 +523,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
 
                pg_atomic_write_u32(&proc->clogGroupNext, nextidx);
 
-               if (pg_atomic_compare_exchange_u32(&procglobal->clogGroupFirst,
+               if (pg_atomic_compare_exchange_u32(&ProcGlobal->clogGroupFirst,
                                                                                   &nextidx,
                                                                                   (uint32) MyProcNumber))
                        break;
@@ -576,7 +575,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
         * At this point, any processes trying to do this would create a separate
         * group.
         */
-       nextidx = pg_atomic_exchange_u32(&procglobal->clogGroupFirst,
+       nextidx = pg_atomic_exchange_u32(&ProcGlobal->clogGroupFirst,
                                                                         INVALID_PROC_NUMBER);
 
        /* Remember head of list so we can perform wakeups after dropping lock. */

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 9407c357f2716e3fb0d2564b939587b31810c2d5..81bba4beac7cb5bd67357c8bec2323ce9de0e5ec 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3637,7 +3637,7 @@ reindex_index(const ReindexStmt *stmt, Oid indexId,
        int                     save_sec_context;
        int                     save_nestlevel;
        IndexInfo  *indexInfo;
-       volatile bool skipped_constraint = false;
+       bool            skipped_constraint = false;
        PGRUsage        ru0;
        bool            progress = ((params->options & REINDEXOPT_REPORT_PROGRESS) != 0);
        bool            set_tablespace = false;

diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index eee8bc29f38d40b1ff1d696f1673199babf43d17..2799f989b96dc45213654ac03381ecbfe3e5acc0 100644 (file)
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -605,7 +605,7 @@ static void CleanupListenersOnExit(void);
 static bool IsListeningOn(const char *channel);
 static void asyncQueueUnregister(void);
 static bool asyncQueueIsFull(void);
-static bool asyncQueueAdvance(volatile QueuePosition *position, int entryLength);
+static bool asyncQueueAdvance(QueuePosition *position, int entryLength);
 static void asyncQueueNotificationToEntry(Notification *n, AsyncQueueEntry *qe);
 static ListCell *asyncQueueAddEntries(ListCell *nextNotify);
 static double asyncQueueUsage(void);
@@ -1968,7 +1968,7 @@ asyncQueueIsFull(void)
  * returns true, else false.
  */
 static bool
-asyncQueueAdvance(volatile QueuePosition *position, int entryLength)
+asyncQueueAdvance(QueuePosition *position, int entryLength)
 {
        int64           pageno = QUEUE_POS_PAGE(*position);
        int                     offset = QUEUE_POS_OFFSET(*position);

diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index e0e30579c590224b54abc5fff211316262423414..d870f09e0a0a0543fd99640b59352b101f1815b6 100644 (file)
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -483,7 +483,6 @@ SyncRepInitConfig(void)
 void
 SyncRepReleaseWaiters(void)
 {
-       volatile WalSndCtlData *walsndctl = WalSndCtl;
        XLogRecPtr      writePtr;
        XLogRecPtr      flushPtr;
        XLogRecPtr      applyPtr;
@@ -558,19 +557,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);
        }
 
@@ -777,8 +776,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 */
 
@@ -915,7 +913,6 @@ SyncRepGetStandbyPriority(void)
 static int
 SyncRepWakeQueue(bool all, int mode)
 {
-       volatile WalSndCtlData *walsndctl = WalSndCtl;
        int                     numprocs = 0;
        dlist_mutable_iter iter;
 
@@ -930,7 +927,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 1397f65f67b6d9ce64d2c88f8d76d709d0fb312f..21a77f98c1df8e8b59fa919d96dc96defe9c8fc5 100644 (file)
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -295,7 +295,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)
        {
@@ -380,7 +380,7 @@ EmitProcSignalBarrier(ProcSignalBarrierType type)
         */
        for (int i = 0; i < NumProcSignalSlots; i++)
        {
-               volatile ProcSignalSlot *slot = &ProcSignal->psh_slot[i];
+               ProcSignalSlot *slot = &ProcSignal->psh_slot[i];
 
                pg_atomic_fetch_or_u32(&slot->pss_barrierCheckMask, flagbit);
        }
@@ -406,7 +406,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)
@@ -670,7 +670,7 @@ ResetProcSignalBarrierBits(uint32 flags)
 static bool
 CheckProcSignal(ProcSignalReason reason)
 {
-       volatile ProcSignalSlot *slot = MyProcSignalSlot;
+       ProcSignalSlot *slot = MyProcSignalSlot;
 
        if (slot != NULL)
        {

diff --git a/src/backend/storage/ipc/shm_toc.c b/src/backend/storage/ipc/shm_toc.c
index 2f9fbb0a519dd7e81b9f3fa234145e50e16ed248..2217d48c3d9b3ab5f257ee8484de8ad4b76d8e0e 100644 (file)
--- a/src/backend/storage/ipc/shm_toc.c
+++ b/src/backend/storage/ipc/shm_toc.c
@@ -87,7 +87,6 @@ shm_toc_attach(uint64 magic, void *address)
 void *
 shm_toc_allocate(shm_toc *toc, Size nbytes)
 {
-       volatile shm_toc *vtoc = toc;
        Size            total_bytes;
        Size            allocated_bytes;
        Size            nentry;
@@ -103,9 +102,9 @@ shm_toc_allocate(shm_toc *toc, Size nbytes)
 
        SpinLockAcquire(&toc->toc_mutex);
 
-       total_bytes = vtoc->toc_total_bytes;
-       allocated_bytes = vtoc->toc_allocated_bytes;
-       nentry = vtoc->toc_nentry;
+       total_bytes = toc->toc_total_bytes;
+       allocated_bytes = toc->toc_allocated_bytes;
+       nentry = toc->toc_nentry;
        toc_bytes = offsetof(shm_toc, toc_entry) + nentry * sizeof(shm_toc_entry)
                + allocated_bytes;
 
@@ -117,7 +116,7 @@ shm_toc_allocate(shm_toc *toc, Size nbytes)
                                (errcode(ERRCODE_OUT_OF_MEMORY),
                                 errmsg("out of shared memory")));
        }
-       vtoc->toc_allocated_bytes += nbytes;
+       toc->toc_allocated_bytes += nbytes;
 
        SpinLockRelease(&toc->toc_mutex);
 
@@ -130,16 +129,15 @@ shm_toc_allocate(shm_toc *toc, Size nbytes)
 Size
 shm_toc_freespace(shm_toc *toc)
 {
-       volatile shm_toc *vtoc = toc;
        Size            total_bytes;
        Size            allocated_bytes;
        Size            nentry;
        Size            toc_bytes;
 
        SpinLockAcquire(&toc->toc_mutex);
-       total_bytes = vtoc->toc_total_bytes;
-       allocated_bytes = vtoc->toc_allocated_bytes;
-       nentry = vtoc->toc_nentry;
+       total_bytes = toc->toc_total_bytes;
+       allocated_bytes = toc->toc_allocated_bytes;
+       nentry = toc->toc_nentry;
        SpinLockRelease(&toc->toc_mutex);
 
        toc_bytes = offsetof(shm_toc, toc_entry) + nentry * sizeof(shm_toc_entry);
@@ -170,7 +168,6 @@ shm_toc_freespace(shm_toc *toc)
 void
 shm_toc_insert(shm_toc *toc, uint64 key, void *address)
 {
-       volatile shm_toc *vtoc = toc;
        Size            total_bytes;
        Size            allocated_bytes;
        Size            nentry;
@@ -183,14 +180,14 @@ shm_toc_insert(shm_toc *toc, uint64 key, void *address)
 
        SpinLockAcquire(&toc->toc_mutex);
 
-       total_bytes = vtoc->toc_total_bytes;
-       allocated_bytes = vtoc->toc_allocated_bytes;
-       nentry = vtoc->toc_nentry;
+       total_bytes = toc->toc_total_bytes;
+       allocated_bytes = toc->toc_allocated_bytes;
+       nentry = toc->toc_nentry;
 
 #ifdef USE_ASSERT_CHECKING
        /* Verify no duplicate keys */
        for (Size i = 0; i < nentry; i++)
-               Assert(vtoc->toc_entry[i].key != key);
+               Assert(toc->toc_entry[i].key != key);
 #endif
 
        toc_bytes = offsetof(shm_toc, toc_entry) + nentry * sizeof(shm_toc_entry)
@@ -208,8 +205,8 @@ shm_toc_insert(shm_toc *toc, uint64 key, void *address)
        }
 
        Assert(offset < total_bytes);
-       vtoc->toc_entry[nentry].key = key;
-       vtoc->toc_entry[nentry].offset = offset;
+       toc->toc_entry[nentry].key = key;
+       toc->toc_entry[nentry].offset = offset;
 
        /*
         * By placing a write barrier after filling in the entry and before
@@ -218,7 +215,7 @@ shm_toc_insert(shm_toc *toc, uint64 key, void *address)
         */
        pg_write_barrier();
 
-       vtoc->toc_nentry++;
+       toc->toc_nentry++;
 
        SpinLockRelease(&toc->toc_mutex);
 }

diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 8d246ed5a4e3206a67f7c3d2b7d047c5ee8ef8ee..5ee80c7632efcc39c842759cfe4de0de0ae2be72 100644 (file)
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -312,7 +312,7 @@ typedef struct
        uint32          count[FAST_PATH_STRONG_LOCK_HASH_PARTITIONS];
 } FastPathStrongRelationLockData;
 
-static volatile FastPathStrongRelationLockData *FastPathStrongRelationLocks;
+static FastPathStrongRelationLockData *FastPathStrongRelationLocks;
 
 static void LockManagerShmemRequest(void *arg);
 static void LockManagerShmemInit(void *arg);

diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 7d01c981a1f68ae88029c46ce421da0e19118916..87dd289f6268199a6bd348c49595b0e33f87bfb2 100644 (file)
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -660,8 +660,7 @@ InitAuxiliaryProcess(void)
        }
 
        /* Mark auxiliary proc as in use by me */
-       /* use volatile pointer to prevent code rearrangement */
-       ((volatile PGPROC *) auxproc)->pid = MyProcPid;
+       auxproc->pid = MyProcPid;
 
        SpinLockRelease(&ProcGlobal->freeProcsLock);
 

diff --git a/src/test/modules/test_shm_mq/setup.c b/src/test/modules/test_shm_mq/setup.c
index 4f40a61e3d928c4ae43580dcc6a8f863613ea042..991fe27a7fad520d3f9eb3cec283fe5d64269dff 100644 (file)
--- a/src/test/modules/test_shm_mq/setup.c
+++ b/src/test/modules/test_shm_mq/setup.c
@@ -38,7 +38,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 */
@@ -258,7 +258,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 e13c05ae5c7cfc21371be7e83a06307bd11130ae..0ba1cfcac47b81c038527365cc9f99bc274592fc 100644 (file)
--- a/src/test/modules/test_shm_mq/worker.c
+++ b/src/test/modules/test_shm_mq/worker.c
@@ -52,7 +52,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;