From 430008b5a7ac395ab3057377104148e80718045c Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 16 Oct 2015 14:12:20 -0400 Subject: [PATCH] Remove volatile qualifiers from dynahash.c, shmem.c, and sinvaladt.c Prior to commit 0709b7ee72e4bc71ad07b7120acd117265ab51d0, access to variables within a spinlock-protected critical section had to be done through a volatile pointer, but that should no longer be necessary. Thomas Munro --- src/backend/storage/ipc/shmem.c | 11 ++---- src/backend/storage/ipc/sinvaladt.c | 22 +++-------- src/backend/utils/hash/dynahash.c | 59 +++++++++++++---------------- 3 files changed, 37 insertions(+), 55 deletions(-) diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c index 250e31255e..78f15f0b00 100644 --- a/src/backend/storage/ipc/shmem.c +++ b/src/backend/storage/ipc/shmem.c @@ -170,29 +170,26 @@ ShmemAlloc(Size size) Size newFree; void *newSpace; - /* use volatile pointer to prevent code rearrangement */ - volatile PGShmemHeader *shmemseghdr = ShmemSegHdr; - /* * ensure all space is adequately aligned. */ size = MAXALIGN(size); - Assert(shmemseghdr != NULL); + Assert(ShmemSegHdr != NULL); SpinLockAcquire(ShmemLock); - newStart = shmemseghdr->freeoffset; + newStart = ShmemSegHdr->freeoffset; /* extra alignment for large requests, since they are probably buffers */ if (size >= BLCKSZ) newStart = BUFFERALIGN(newStart); newFree = newStart + size; - if (newFree <= shmemseghdr->totalsize) + if (newFree <= ShmemSegHdr->totalsize) { newSpace = (void *) ((char *) ShmemBase + newStart); - shmemseghdr->freeoffset = newFree; + ShmemSegHdr->freeoffset = newFree; } else newSpace = NULL; diff --git a/src/backend/storage/ipc/sinvaladt.c b/src/backend/storage/ipc/sinvaladt.c index a2fde89b52..3cc15e0429 100644 --- a/src/backend/storage/ipc/sinvaladt.c +++ b/src/backend/storage/ipc/sinvaladt.c @@ -485,14 +485,9 @@ SIInsertDataEntries(const SharedInvalidationMessage *data, int n) } /* Update current value of maxMsgNum using spinlock */ - { - /* use volatile pointer to prevent code rearrangement */ - volatile SISeg *vsegP = segP; - - SpinLockAcquire(&vsegP->msgnumLock); - vsegP->maxMsgNum = max; - SpinLockRelease(&vsegP->msgnumLock); - } + SpinLockAcquire(&segP->msgnumLock); + segP->maxMsgNum = max; + SpinLockRelease(&segP->msgnumLock); /* * Now that the maxMsgNum change is globally visible, we give everyone @@ -579,14 +574,9 @@ SIGetDataEntries(SharedInvalidationMessage *data, int datasize) stateP->hasMessages = false; /* Fetch current value of maxMsgNum using spinlock */ - { - /* use volatile pointer to prevent code rearrangement */ - volatile SISeg *vsegP = segP; - - SpinLockAcquire(&vsegP->msgnumLock); - max = vsegP->maxMsgNum; - SpinLockRelease(&vsegP->msgnumLock); - } + SpinLockAcquire(&segP->msgnumLock); + max = segP->maxMsgNum; + SpinLockRelease(&segP->msgnumLock); if (stateP->resetState) { diff --git a/src/backend/utils/hash/dynahash.c b/src/backend/utils/hash/dynahash.c index 27580af397..eacffc4468 100644 --- a/src/backend/utils/hash/dynahash.c +++ b/src/backend/utils/hash/dynahash.c @@ -941,25 +941,22 @@ hash_search_with_hash_value(HTAB *hashp, case HASH_REMOVE: if (currBucket != NULL) { - /* use volatile pointer to prevent code rearrangement */ - volatile HASHHDR *hctlv = hctl; - /* if partitioned, must lock to touch nentries and freeList */ - if (IS_PARTITIONED(hctlv)) - SpinLockAcquire(&hctlv->mutex); + if (IS_PARTITIONED(hctl)) + SpinLockAcquire(&hctl->mutex); - Assert(hctlv->nentries > 0); - hctlv->nentries--; + Assert(hctl->nentries > 0); + hctl->nentries--; /* remove record from hash bucket's chain. */ *prevBucketPtr = currBucket->link; /* add the record to the freelist for this table. */ - currBucket->link = hctlv->freeList; - hctlv->freeList = currBucket; + currBucket->link = hctl->freeList; + hctl->freeList = currBucket; - if (IS_PARTITIONED(hctlv)) - SpinLockRelease(&hctlv->mutex); + if (IS_PARTITIONED(hctl)) + SpinLockRelease(&hctl->mutex); /* * better hope the caller is synchronizing access to this @@ -1180,26 +1177,25 @@ hash_update_hash_key(HTAB *hashp, static HASHBUCKET get_hash_entry(HTAB *hashp) { - /* use volatile pointer to prevent code rearrangement */ - volatile HASHHDR *hctlv = hashp->hctl; + HASHHDR *hctl = hashp->hctl; HASHBUCKET newElement; for (;;) { /* if partitioned, must lock to touch nentries and freeList */ - if (IS_PARTITIONED(hctlv)) - SpinLockAcquire(&hctlv->mutex); + if (IS_PARTITIONED(hctl)) + SpinLockAcquire(&hctl->mutex); /* try to get an entry from the freelist */ - newElement = hctlv->freeList; + newElement = hctl->freeList; if (newElement != NULL) break; /* no free elements. allocate another chunk of buckets */ - if (IS_PARTITIONED(hctlv)) - SpinLockRelease(&hctlv->mutex); + if (IS_PARTITIONED(hctl)) + SpinLockRelease(&hctl->mutex); - if (!element_alloc(hashp, hctlv->nelem_alloc)) + if (!element_alloc(hashp, hctl->nelem_alloc)) { /* out of memory */ return NULL; @@ -1207,11 +1203,11 @@ get_hash_entry(HTAB *hashp) } /* remove entry from freelist, bump nentries */ - hctlv->freeList = newElement->link; - hctlv->nentries++; + hctl->freeList = newElement->link; + hctl->nentries++; - if (IS_PARTITIONED(hctlv)) - SpinLockRelease(&hctlv->mutex); + if (IS_PARTITIONED(hctl)) + SpinLockRelease(&hctl->mutex); return newElement; } @@ -1536,8 +1532,7 @@ seg_alloc(HTAB *hashp) static bool element_alloc(HTAB *hashp, int nelem) { - /* use volatile pointer to prevent code rearrangement */ - volatile HASHHDR *hctlv = hashp->hctl; + HASHHDR *hctl = hashp->hctl; Size elementSize; HASHELEMENT *firstElement; HASHELEMENT *tmpElement; @@ -1548,7 +1543,7 @@ element_alloc(HTAB *hashp, int nelem) return false; /* Each element has a HASHELEMENT header plus user data. */ - elementSize = MAXALIGN(sizeof(HASHELEMENT)) + MAXALIGN(hctlv->entrysize); + elementSize = MAXALIGN(sizeof(HASHELEMENT)) + MAXALIGN(hctl->entrysize); CurrentDynaHashCxt = hashp->hcxt; firstElement = (HASHELEMENT *) hashp->alloc(nelem * elementSize); @@ -1567,15 +1562,15 @@ element_alloc(HTAB *hashp, int nelem) } /* if partitioned, must lock to touch freeList */ - if (IS_PARTITIONED(hctlv)) - SpinLockAcquire(&hctlv->mutex); + if (IS_PARTITIONED(hctl)) + SpinLockAcquire(&hctl->mutex); /* freelist could be nonempty if two backends did this concurrently */ - firstElement->link = hctlv->freeList; - hctlv->freeList = prevElement; + firstElement->link = hctl->freeList; + hctl->freeList = prevElement; - if (IS_PARTITIONED(hctlv)) - SpinLockRelease(&hctlv->mutex); + if (IS_PARTITIONED(hctl)) + SpinLockRelease(&hctl->mutex); return true; }