On Fri, Oct 16, 2015 at 9:07 AM, Thomas Munro
<thomas.mu...@enterprisedb.com> wrote:
> On Wed, Oct 7, 2015 at 8:52 AM, Robert Haas <robertmh...@gmail.com> wrote:
>> On Thu, Oct 1, 2015 at 11:01 PM, Michael Paquier
>> <michael.paqu...@gmail.com> wrote:
>>>> Right, see attached.
>>>
>>> It seems to me that we could as well simplify checkpoint.c and
>>> logical.c. In those files volatile casts are used as well to protect
>>> from reordering for spinlock operations. See for example 0002 on top
>>> of 0001 that is Thomas' patch.
>>
>> These patches look good to me, so I have committed them.
>
> Thanks. Also, spin.h's comment contains an out of date warning about
> this. Here's a suggested fix for that, and a couple more volatrivia
> patches.
I have looked at the rest of the code, and it seems that we can get
rid of volatile in a couple of extra places like in the attached as
those are used with spin locks. This applies on top of Thomas' set.
--
Michael
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index b13fe03..0dc4117 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -3718,8 +3718,6 @@ static int
KnownAssignedXidsGetAndSetXmin(TransactionId *xarray, TransactionId *xmin,
TransactionId xmax)
{
- /* use volatile pointer to prevent code rearrangement */
- volatile ProcArrayStruct *pArray = procArray;
int count = 0;
int head,
tail;
@@ -3734,10 +3732,10 @@ KnownAssignedXidsGetAndSetXmin(TransactionId *xarray, TransactionId *xmin,
*
* Must take spinlock to ensure we see up-to-date array contents.
*/
- SpinLockAcquire(&pArray->known_assigned_xids_lck);
- tail = pArray->tailKnownAssignedXids;
- head = pArray->headKnownAssignedXids;
- SpinLockRelease(&pArray->known_assigned_xids_lck);
+ SpinLockAcquire(&procArray->known_assigned_xids_lck);
+ tail = procArray->tailKnownAssignedXids;
+ head = procArray->headKnownAssignedXids;
+ SpinLockRelease(&procArray->known_assigned_xids_lck);
for (i = tail; i < head; i++)
{
@@ -3777,8 +3775,6 @@ KnownAssignedXidsGetAndSetXmin(TransactionId *xarray, TransactionId *xmin,
static TransactionId
KnownAssignedXidsGetOldestXmin(void)
{
- /* use volatile pointer to prevent code rearrangement */
- volatile ProcArrayStruct *pArray = procArray;
int head,
tail;
int i;
@@ -3786,10 +3782,10 @@ KnownAssignedXidsGetOldestXmin(void)
/*
* Fetch head just once, since it may change while we loop.
*/
- SpinLockAcquire(&pArray->known_assigned_xids_lck);
- tail = pArray->tailKnownAssignedXids;
- head = pArray->headKnownAssignedXids;
- SpinLockRelease(&pArray->known_assigned_xids_lck);
+ SpinLockAcquire(&procArray->known_assigned_xids_lck);
+ tail = procArray->tailKnownAssignedXids;
+ head = procArray->headKnownAssignedXids;
+ SpinLockRelease(&procArray->known_assigned_xids_lck);
for (i = tail; i < head; i++)
{
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 2c2535b..bb10c1b 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -283,15 +283,13 @@ InitProcGlobal(void)
void
InitProcess(void)
{
- /* use volatile pointer to prevent code rearrangement */
- volatile PROC_HDR *procglobal = ProcGlobal;
PGPROC * volatile * procgloballist;
/*
* ProcGlobal should be set up already (if we are a backend, we inherit
* this by fork() or EXEC_BACKEND mechanism from the postmaster).
*/
- if (procglobal == NULL)
+ if (ProcGlobal == NULL)
elog(PANIC, "proc header uninitialized");
if (MyProc != NULL)
@@ -299,11 +297,11 @@ InitProcess(void)
/* Decide which list should supply our PGPROC. */
if (IsAnyAutoVacuumProcess())
- procgloballist = &procglobal->autovacFreeProcs;
+ procgloballist = &ProcGlobal->autovacFreeProcs;
else if (IsBackgroundWorker)
- procgloballist = &procglobal->bgworkerFreeProcs;
+ procgloballist = &ProcGlobal->bgworkerFreeProcs;
else
- procgloballist = &procglobal->freeProcs;
+ procgloballist = &ProcGlobal->freeProcs;
/*
* Try to get a proc struct from the appropriate free list. If this
@@ -314,7 +312,7 @@ InitProcess(void)
*/
SpinLockAcquire(ProcStructLock);
- set_spins_per_delay(procglobal->spins_per_delay);
+ set_spins_per_delay(ProcGlobal->spins_per_delay);
MyProc = *procgloballist;
@@ -578,13 +576,10 @@ InitAuxiliaryProcess(void)
void
PublishStartupProcessInformation(void)
{
- /* use volatile pointer to prevent code rearrangement */
- volatile PROC_HDR *procglobal = ProcGlobal;
-
SpinLockAcquire(ProcStructLock);
- procglobal->startupProc = MyProc;
- procglobal->startupProcPid = MyProcPid;
+ ProcGlobal->startupProc = MyProc;
+ ProcGlobal->startupProcPid = MyProcPid;
SpinLockRelease(ProcStructLock);
}
@@ -627,12 +622,9 @@ HaveNFreeProcs(int n)
{
PGPROC *proc;
- /* use volatile pointer to prevent code rearrangement */
- volatile PROC_HDR *procglobal = ProcGlobal;
-
SpinLockAcquire(ProcStructLock);
- proc = procglobal->freeProcs;
+ proc = ProcGlobal->freeProcs;
while (n > 0 && proc != NULL)
{
@@ -772,8 +764,6 @@ RemoveProcFromArray(int code, Datum arg)
static void
ProcKill(int code, Datum arg)
{
- /* use volatile pointer to prevent code rearrangement */
- volatile PROC_HDR *procglobal = ProcGlobal;
PGPROC *proc;
PGPROC * volatile * procgloballist;
@@ -822,7 +812,7 @@ ProcKill(int code, Datum arg)
*procgloballist = proc;
/* Update shared estimate of spins_per_delay */
- procglobal->spins_per_delay = update_spins_per_delay(procglobal->spins_per_delay);
+ ProcGlobal->spins_per_delay = update_spins_per_delay(ProcGlobal->spins_per_delay);
SpinLockRelease(ProcStructLock);
@@ -1644,9 +1634,6 @@ ProcSendSignal(int pid)
if (RecoveryInProgress())
{
- /* use volatile pointer to prevent code rearrangement */
- volatile PROC_HDR *procglobal = ProcGlobal;
-
SpinLockAcquire(ProcStructLock);
/*
@@ -1657,8 +1644,8 @@ ProcSendSignal(int pid)
* backend, so BackendPidGetProc() will not return any pid at all. So
* we remember the information for this special case.
*/
- if (pid == procglobal->startupProcPid)
- proc = procglobal->startupProc;
+ if (pid == ProcGlobal->startupProcPid)
+ proc = ProcGlobal->startupProc;
SpinLockRelease(ProcStructLock);
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
