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;
 }