From db26450da5814f2c73180c3b5f553971985b2ffa Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 2 Oct 2015 11:59:55 +0900 Subject: [PATCH 2/2] Remove use of volatile for spinlock operations in more places --- src/backend/postmaster/checkpointer.c | 64 +++++++++++++------------------ src/backend/replication/logical/logical.c | 49 +++++++++++------------ 2 files changed, 49 insertions(+), 64 deletions(-) diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c index 3b3a09e..dc5b856 100644 --- a/src/backend/postmaster/checkpointer.c +++ b/src/backend/postmaster/checkpointer.c @@ -288,13 +288,10 @@ CheckpointerMain(void) /* Warn any waiting backends that the checkpoint failed. */ if (ckpt_active) { - /* use volatile pointer to prevent code rearrangement */ - volatile CheckpointerShmemStruct *cps = CheckpointerShmem; - - SpinLockAcquire(&cps->ckpt_lck); - cps->ckpt_failed++; - cps->ckpt_done = cps->ckpt_started; - SpinLockRelease(&cps->ckpt_lck); + SpinLockAcquire(&CheckpointerShmem->ckpt_lck); + CheckpointerShmem->ckpt_failed++; + CheckpointerShmem->ckpt_done = CheckpointerShmem->ckpt_started; + SpinLockRelease(&CheckpointerShmem->ckpt_lck); ckpt_active = false; } @@ -428,9 +425,6 @@ CheckpointerMain(void) bool ckpt_performed = false; bool do_restartpoint; - /* use volatile pointer to prevent code rearrangement */ - volatile CheckpointerShmemStruct *cps = CheckpointerShmem; - /* * Check if we should perform a checkpoint or a restartpoint. As a * side-effect, RecoveryInProgress() initializes TimeLineID if @@ -443,11 +437,11 @@ CheckpointerMain(void) * checkpoint we should perform, and increase the started-counter * to acknowledge that we've started a new checkpoint. */ - SpinLockAcquire(&cps->ckpt_lck); - flags |= cps->ckpt_flags; - cps->ckpt_flags = 0; - cps->ckpt_started++; - SpinLockRelease(&cps->ckpt_lck); + SpinLockAcquire(&CheckpointerShmem->ckpt_lck); + flags |= CheckpointerShmem->ckpt_flags; + CheckpointerShmem->ckpt_flags = 0; + CheckpointerShmem->ckpt_started++; + SpinLockRelease(&CheckpointerShmem->ckpt_lck); /* * The end-of-recovery checkpoint is a real checkpoint that's @@ -505,9 +499,9 @@ CheckpointerMain(void) /* * Indicate checkpoint completion to any waiting backends. */ - SpinLockAcquire(&cps->ckpt_lck); - cps->ckpt_done = cps->ckpt_started; - SpinLockRelease(&cps->ckpt_lck); + SpinLockAcquire(&CheckpointerShmem->ckpt_lck); + CheckpointerShmem->ckpt_done = CheckpointerShmem->ckpt_started; + SpinLockRelease(&CheckpointerShmem->ckpt_lck); if (ckpt_performed) { @@ -957,8 +951,6 @@ CheckpointerShmemInit(void) void RequestCheckpoint(int flags) { - /* use volatile pointer to prevent code rearrangement */ - volatile CheckpointerShmemStruct *cps = CheckpointerShmem; int ntries; int old_failed, old_started; @@ -992,13 +984,13 @@ RequestCheckpoint(int flags) * a "stronger" request by another backend. The flag senses must be * chosen to make this work! */ - SpinLockAcquire(&cps->ckpt_lck); + SpinLockAcquire(&CheckpointerShmem->ckpt_lck); - old_failed = cps->ckpt_failed; - old_started = cps->ckpt_started; - cps->ckpt_flags |= flags; + old_failed = CheckpointerShmem->ckpt_failed; + old_started = CheckpointerShmem->ckpt_started; + CheckpointerShmem->ckpt_flags |= flags; - SpinLockRelease(&cps->ckpt_lck); + SpinLockRelease(&CheckpointerShmem->ckpt_lck); /* * Send signal to request checkpoint. It's possible that the checkpointer @@ -1046,9 +1038,9 @@ RequestCheckpoint(int flags) /* Wait for a new checkpoint to start. */ for (;;) { - SpinLockAcquire(&cps->ckpt_lck); - new_started = cps->ckpt_started; - SpinLockRelease(&cps->ckpt_lck); + SpinLockAcquire(&CheckpointerShmem->ckpt_lck); + new_started = CheckpointerShmem->ckpt_started; + SpinLockRelease(&CheckpointerShmem->ckpt_lck); if (new_started != old_started) break; @@ -1064,10 +1056,10 @@ RequestCheckpoint(int flags) { int new_done; - SpinLockAcquire(&cps->ckpt_lck); - new_done = cps->ckpt_done; - new_failed = cps->ckpt_failed; - SpinLockRelease(&cps->ckpt_lck); + SpinLockAcquire(&CheckpointerShmem->ckpt_lck); + new_done = CheckpointerShmem->ckpt_done; + new_failed = CheckpointerShmem->ckpt_failed; + SpinLockRelease(&CheckpointerShmem->ckpt_lck); if (new_done - new_started >= 0) break; @@ -1368,15 +1360,13 @@ UpdateSharedMemoryConfig(void) bool FirstCallSinceLastCheckpoint(void) { - /* use volatile pointer to prevent code rearrangement */ - volatile CheckpointerShmemStruct *cps = CheckpointerShmem; static int ckpt_done = 0; int new_done; bool FirstCall = false; - SpinLockAcquire(&cps->ckpt_lck); - new_done = cps->ckpt_done; - SpinLockRelease(&cps->ckpt_lck); + SpinLockAcquire(&CheckpointerShmem->ckpt_lck); + new_done = CheckpointerShmem->ckpt_done; + SpinLockRelease(&CheckpointerShmem->ckpt_lck); if (new_done != ckpt_done) FirstCall = true; diff --git a/src/backend/replication/logical/logical.c b/src/backend/replication/logical/logical.c index 5a07e1d..1ce9081 100644 --- a/src/backend/replication/logical/logical.c +++ b/src/backend/replication/logical/logical.c @@ -848,16 +848,13 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn) bool updated_xmin = false; bool updated_restart = false; - /* use volatile pointer to prevent code rearrangement */ - volatile ReplicationSlot *slot = MyReplicationSlot; + SpinLockAcquire(&MyReplicationSlot->mutex); - SpinLockAcquire(&slot->mutex); - - slot->data.confirmed_flush = lsn; + MyReplicationSlot->data.confirmed_flush = lsn; /* if were past the location required for bumping xmin, do so */ - if (slot->candidate_xmin_lsn != InvalidXLogRecPtr && - slot->candidate_xmin_lsn <= lsn) + if (MyReplicationSlot->candidate_xmin_lsn != InvalidXLogRecPtr && + MyReplicationSlot->candidate_xmin_lsn <= lsn) { /* * We have to write the changed xmin to disk *before* we change @@ -868,28 +865,28 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn) * ->effective_xmin once the new state is synced to disk. After a * crash ->effective_xmin is set to ->xmin. */ - if (TransactionIdIsValid(slot->candidate_catalog_xmin) && - slot->data.catalog_xmin != slot->candidate_catalog_xmin) + if (TransactionIdIsValid(MyReplicationSlot->candidate_catalog_xmin) && + MyReplicationSlot->data.catalog_xmin != MyReplicationSlot->candidate_catalog_xmin) { - slot->data.catalog_xmin = slot->candidate_catalog_xmin; - slot->candidate_catalog_xmin = InvalidTransactionId; - slot->candidate_xmin_lsn = InvalidXLogRecPtr; + MyReplicationSlot->data.catalog_xmin = MyReplicationSlot->candidate_catalog_xmin; + MyReplicationSlot->candidate_catalog_xmin = InvalidTransactionId; + MyReplicationSlot->candidate_xmin_lsn = InvalidXLogRecPtr; updated_xmin = true; } } - if (slot->candidate_restart_valid != InvalidXLogRecPtr && - slot->candidate_restart_valid <= lsn) + if (MyReplicationSlot->candidate_restart_valid != InvalidXLogRecPtr && + MyReplicationSlot->candidate_restart_valid <= lsn) { - Assert(slot->candidate_restart_lsn != InvalidXLogRecPtr); + Assert(MyReplicationSlot->candidate_restart_lsn != InvalidXLogRecPtr); - slot->data.restart_lsn = slot->candidate_restart_lsn; - slot->candidate_restart_lsn = InvalidXLogRecPtr; - slot->candidate_restart_valid = InvalidXLogRecPtr; + MyReplicationSlot->data.restart_lsn = MyReplicationSlot->candidate_restart_lsn; + MyReplicationSlot->candidate_restart_lsn = InvalidXLogRecPtr; + MyReplicationSlot->candidate_restart_valid = InvalidXLogRecPtr; updated_restart = true; } - SpinLockRelease(&slot->mutex); + SpinLockRelease(&MyReplicationSlot->mutex); /* first write new xmin to disk, so we know whats up after a crash */ if (updated_xmin || updated_restart) @@ -907,9 +904,9 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn) */ if (updated_xmin) { - SpinLockAcquire(&slot->mutex); - slot->effective_catalog_xmin = slot->data.catalog_xmin; - SpinLockRelease(&slot->mutex); + SpinLockAcquire(&MyReplicationSlot->mutex); + MyReplicationSlot->effective_catalog_xmin = MyReplicationSlot->data.catalog_xmin; + SpinLockRelease(&MyReplicationSlot->mutex); ReplicationSlotsComputeRequiredXmin(false); ReplicationSlotsComputeRequiredLSN(); @@ -917,10 +914,8 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn) } else { - volatile ReplicationSlot *slot = MyReplicationSlot; - - SpinLockAcquire(&slot->mutex); - slot->data.confirmed_flush = lsn; - SpinLockRelease(&slot->mutex); + SpinLockAcquire(&MyReplicationSlot->mutex); + MyReplicationSlot->data.confirmed_flush = lsn; + SpinLockRelease(&MyReplicationSlot->mutex); } } -- 2.6.0