Skip to content

Commit 8f6bb85

Browse files
robertmhaasrobertmhaas
committed
Remove more volatile qualifiers.
Prior to commit 0709b7e, access to variables within a spinlock-protected critical section had to be done through a volatile pointer, but that should no longer be necessary. This continues work begun in df4077c and 6ba4ecb. Thomas Munro and Michael Paquier
1 parent b943f50 commit 8f6bb85

File tree

6 files changed

+108
-182
lines changed

6 files changed

+108
-182
lines changed

‎src/backend/postmaster/checkpointer.c

+27-37
Original file line numberDiff line numberDiff line change
@@ -288,13 +288,10 @@ CheckpointerMain(void)
288288
/* Warn any waiting backends that the checkpoint failed. */
289289
if (ckpt_active)
290290
{
291-
/* use volatile pointer to prevent code rearrangement */
292-
volatile CheckpointerShmemStruct *cps = CheckpointerShmem;
293-
294-
SpinLockAcquire(&cps->ckpt_lck);
295-
cps->ckpt_failed++;
296-
cps->ckpt_done = cps->ckpt_started;
297-
SpinLockRelease(&cps->ckpt_lck);
291+
SpinLockAcquire(&CheckpointerShmem->ckpt_lck);
292+
CheckpointerShmem->ckpt_failed++;
293+
CheckpointerShmem->ckpt_done = CheckpointerShmem->ckpt_started;
294+
SpinLockRelease(&CheckpointerShmem->ckpt_lck);
298295

299296
ckpt_active = false;
300297
}
@@ -428,9 +425,6 @@ CheckpointerMain(void)
428425
bool ckpt_performed = false;
429426
bool do_restartpoint;
430427

431-
/* use volatile pointer to prevent code rearrangement */
432-
volatile CheckpointerShmemStruct *cps = CheckpointerShmem;
433-
434428
/*
435429
* Check if we should perform a checkpoint or a restartpoint. As a
436430
* side-effect, RecoveryInProgress() initializes TimeLineID if
@@ -443,11 +437,11 @@ CheckpointerMain(void)
443437
* checkpoint we should perform, and increase the started-counter
444438
* to acknowledge that we've started a new checkpoint.
445439
*/
446-
SpinLockAcquire(&cps->ckpt_lck);
447-
flags |= cps->ckpt_flags;
448-
cps->ckpt_flags = 0;
449-
cps->ckpt_started++;
450-
SpinLockRelease(&cps->ckpt_lck);
440+
SpinLockAcquire(&CheckpointerShmem->ckpt_lck);
441+
flags |= CheckpointerShmem->ckpt_flags;
442+
CheckpointerShmem->ckpt_flags = 0;
443+
CheckpointerShmem->ckpt_started++;
444+
SpinLockRelease(&CheckpointerShmem->ckpt_lck);
451445

452446
/*
453447
* The end-of-recovery checkpoint is a real checkpoint that's
@@ -505,9 +499,9 @@ CheckpointerMain(void)
505499
/*
506500
* Indicate checkpoint completion to any waiting backends.
507501
*/
508-
SpinLockAcquire(&cps->ckpt_lck);
509-
cps->ckpt_done = cps->ckpt_started;
510-
SpinLockRelease(&cps->ckpt_lck);
502+
SpinLockAcquire(&CheckpointerShmem->ckpt_lck);
503+
CheckpointerShmem->ckpt_done = CheckpointerShmem->ckpt_started;
504+
SpinLockRelease(&CheckpointerShmem->ckpt_lck);
511505

512506
if (ckpt_performed)
513507
{
@@ -957,8 +951,6 @@ CheckpointerShmemInit(void)
957951
void
958952
RequestCheckpoint(int flags)
959953
{
960-
/* use volatile pointer to prevent code rearrangement */
961-
volatile CheckpointerShmemStruct *cps = CheckpointerShmem;
962954
int ntries;
963955
int old_failed,
964956
old_started;
@@ -992,13 +984,13 @@ RequestCheckpoint(int flags)
992984
* a "stronger" request by another backend. The flag senses must be
993985
* chosen to make this work!
994986
*/
995-
SpinLockAcquire(&cps->ckpt_lck);
987+
SpinLockAcquire(&CheckpointerShmem->ckpt_lck);
996988

997-
old_failed = cps->ckpt_failed;
998-
old_started = cps->ckpt_started;
999-
cps->ckpt_flags |= flags;
989+
old_failed = CheckpointerShmem->ckpt_failed;
990+
old_started = CheckpointerShmem->ckpt_started;
991+
CheckpointerShmem->ckpt_flags |= flags;
1000992

1001-
SpinLockRelease(&cps->ckpt_lck);
993+
SpinLockRelease(&CheckpointerShmem->ckpt_lck);
1002994

1003995
/*
1004996
* Send signal to request checkpoint. It's possible that the checkpointer
@@ -1046,9 +1038,9 @@ RequestCheckpoint(int flags)
10461038
/* Wait for a new checkpoint to start. */
10471039
for (;;)
10481040
{
1049-
SpinLockAcquire(&cps->ckpt_lck);
1050-
new_started = cps->ckpt_started;
1051-
SpinLockRelease(&cps->ckpt_lck);
1041+
SpinLockAcquire(&CheckpointerShmem->ckpt_lck);
1042+
new_started = CheckpointerShmem->ckpt_started;
1043+
SpinLockRelease(&CheckpointerShmem->ckpt_lck);
10521044

10531045
if (new_started != old_started)
10541046
break;
@@ -1064,10 +1056,10 @@ RequestCheckpoint(int flags)
10641056
{
10651057
int new_done;
10661058

1067-
SpinLockAcquire(&cps->ckpt_lck);
1068-
new_done = cps->ckpt_done;
1069-
new_failed = cps->ckpt_failed;
1070-
SpinLockRelease(&cps->ckpt_lck);
1059+
SpinLockAcquire(&CheckpointerShmem->ckpt_lck);
1060+
new_done = CheckpointerShmem->ckpt_done;
1061+
new_failed = CheckpointerShmem->ckpt_failed;
1062+
SpinLockRelease(&CheckpointerShmem->ckpt_lck);
10711063

10721064
if (new_done - new_started >= 0)
10731065
break;
@@ -1368,15 +1360,13 @@ UpdateSharedMemoryConfig(void)
13681360
bool
13691361
FirstCallSinceLastCheckpoint(void)
13701362
{
1371-
/* use volatile pointer to prevent code rearrangement */
1372-
volatile CheckpointerShmemStruct *cps = CheckpointerShmem;
13731363
static int ckpt_done = 0;
13741364
int new_done;
13751365
bool FirstCall = false;
13761366

1377-
SpinLockAcquire(&cps->ckpt_lck);
1378-
new_done = cps->ckpt_done;
1379-
SpinLockRelease(&cps->ckpt_lck);
1367+
SpinLockAcquire(&CheckpointerShmem->ckpt_lck);
1368+
new_done = CheckpointerShmem->ckpt_done;
1369+
SpinLockRelease(&CheckpointerShmem->ckpt_lck);
13801370

13811371
if (new_done != ckpt_done)
13821372
FirstCall = true;

‎src/backend/replication/logical/logical.c

+22-27
Original file line numberDiff line numberDiff line change
@@ -848,16 +848,13 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn)
848848
bool updated_xmin = false;
849849
bool updated_restart = false;
850850

851-
/* use volatile pointer to prevent code rearrangement */
852-
volatile ReplicationSlot *slot = MyReplicationSlot;
851+
SpinLockAcquire(&MyReplicationSlot->mutex);
853852

854-
SpinLockAcquire(&slot->mutex);
855-
856-
slot->data.confirmed_flush = lsn;
853+
MyReplicationSlot->data.confirmed_flush = lsn;
857854

858855
/* if were past the location required for bumping xmin, do so */
859-
if (slot->candidate_xmin_lsn != InvalidXLogRecPtr &&
860-
slot->candidate_xmin_lsn <= lsn)
856+
if (MyReplicationSlot->candidate_xmin_lsn != InvalidXLogRecPtr &&
857+
MyReplicationSlot->candidate_xmin_lsn <= lsn)
861858
{
862859
/*
863860
* We have to write the changed xmin to disk *before* we change
@@ -868,28 +865,28 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn)
868865
* ->effective_xmin once the new state is synced to disk. After a
869866
* crash ->effective_xmin is set to ->xmin.
870867
*/
871-
if (TransactionIdIsValid(slot->candidate_catalog_xmin) &&
872-
slot->data.catalog_xmin != slot->candidate_catalog_xmin)
868+
if (TransactionIdIsValid(MyReplicationSlot->candidate_catalog_xmin) &&
869+
MyReplicationSlot->data.catalog_xmin != MyReplicationSlot->candidate_catalog_xmin)
873870
{
874-
slot->data.catalog_xmin = slot->candidate_catalog_xmin;
875-
slot->candidate_catalog_xmin = InvalidTransactionId;
876-
slot->candidate_xmin_lsn = InvalidXLogRecPtr;
871+
MyReplicationSlot->data.catalog_xmin = MyReplicationSlot->candidate_catalog_xmin;
872+
MyReplicationSlot->candidate_catalog_xmin = InvalidTransactionId;
873+
MyReplicationSlot->candidate_xmin_lsn = InvalidXLogRecPtr;
877874
updated_xmin = true;
878875
}
879876
}
880877

881-
if (slot->candidate_restart_valid != InvalidXLogRecPtr &&
882-
slot->candidate_restart_valid <= lsn)
878+
if (MyReplicationSlot->candidate_restart_valid != InvalidXLogRecPtr &&
879+
MyReplicationSlot->candidate_restart_valid <= lsn)
883880
{
884-
Assert(slot->candidate_restart_lsn != InvalidXLogRecPtr);
881+
Assert(MyReplicationSlot->candidate_restart_lsn != InvalidXLogRecPtr);
885882

886-
slot->data.restart_lsn = slot->candidate_restart_lsn;
887-
slot->candidate_restart_lsn = InvalidXLogRecPtr;
888-
slot->candidate_restart_valid = InvalidXLogRecPtr;
883+
MyReplicationSlot->data.restart_lsn = MyReplicationSlot->candidate_restart_lsn;
884+
MyReplicationSlot->candidate_restart_lsn = InvalidXLogRecPtr;
885+
MyReplicationSlot->candidate_restart_valid = InvalidXLogRecPtr;
889886
updated_restart = true;
890887
}
891888

892-
SpinLockRelease(&slot->mutex);
889+
SpinLockRelease(&MyReplicationSlot->mutex);
893890

894891
/* first write new xmin to disk, so we know whats up after a crash */
895892
if (updated_xmin || updated_restart)
@@ -907,20 +904,18 @@ LogicalConfirmReceivedLocation(XLogRecPtr lsn)
907904
*/
908905
if (updated_xmin)
909906
{
910-
SpinLockAcquire(&slot->mutex);
911-
slot->effective_catalog_xmin = slot->data.catalog_xmin;
912-
SpinLockRelease(&slot->mutex);
907+
SpinLockAcquire(&MyReplicationSlot->mutex);
908+
MyReplicationSlot->effective_catalog_xmin = MyReplicationSlot->data.catalog_xmin;
909+
SpinLockRelease(&MyReplicationSlot->mutex);
913910

914911
ReplicationSlotsComputeRequiredXmin(false);
915912
ReplicationSlotsComputeRequiredLSN();
916913
}
917914
}
918915
else
919916
{
920-
volatile ReplicationSlot *slot = MyReplicationSlot;
921-
922-
SpinLockAcquire(&slot->mutex);
923-
slot->data.confirmed_flush = lsn;
924-
SpinLockRelease(&slot->mutex);
917+
SpinLockAcquire(&MyReplicationSlot->mutex);
918+
MyReplicationSlot->data.confirmed_flush = lsn;
919+
SpinLockRelease(&MyReplicationSlot->mutex);
925920
}
926921
}

0 commit comments

Comments
 (0)