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

diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index 250e31255e9feaf380980338dafb1d5de2f05e5f..78f15f0b00a05f1e8854ab692b666e7c05adb27a 100644 (file)
--- 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 a2fde89b526b96e926b23f34a1092d77a3bb155b..3cc15e042991f7c515730bf389e839a5b8e28f44 100644 (file)
--- 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 27580af397131abaaf3fc6f0f74b50b9b69fa03e..eacffc44687e465eb9348da91feef00df416b017 100644 (file)
--- 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;
 }