Remove volatile qualifiers from dynahash.c, shmem.c, and sinvaladt.c

Prior to commit 0709b7ee72, 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

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

src/backend/storage/ipc/sinvaladt.c

@@ -485,14 +485,9 @@ SIInsertDataEntries(const SharedInvalidationMessage *data, int n)
 		}
 
 		/* Update current value of maxMsgNum using spinlock */
-		{
+		SpinLockAcquire(&segP->msgnumLock);
-			/* use volatile pointer to prevent code rearrangement */
+		segP->maxMsgNum = max;
-			volatile SISeg *vsegP = segP;
+		SpinLockRelease(&segP->msgnumLock);
-
-			SpinLockAcquire(&vsegP->msgnumLock);
-			vsegP->maxMsgNum = max;
-			SpinLockRelease(&vsegP->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 */
-	{
+	SpinLockAcquire(&segP->msgnumLock);
-		/* use volatile pointer to prevent code rearrangement */
+	max = segP->maxMsgNum;
-		volatile SISeg *vsegP = segP;
+	SpinLockRelease(&segP->msgnumLock);
-
-		SpinLockAcquire(&vsegP->msgnumLock);
-		max = vsegP->maxMsgNum;
-		SpinLockRelease(&vsegP->msgnumLock);
-	}
 
 	if (stateP->resetState)
 	{

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))
+				if (IS_PARTITIONED(hctl))
-					SpinLockAcquire(&hctlv->mutex);
+					SpinLockAcquire(&hctl->mutex);
 
-				Assert(hctlv->nentries > 0);
+				Assert(hctl->nentries > 0);
-				hctlv->nentries--;
+				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;
+				currBucket->link = hctl->freeList;
-				hctlv->freeList = currBucket;
+				hctl->freeList = currBucket;
 
-				if (IS_PARTITIONED(hctlv))
+				if (IS_PARTITIONED(hctl))
-					SpinLockRelease(&hctlv->mutex);
+					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 */
+	HASHHDR *hctl = hashp->hctl;
-	volatile HASHHDR *hctlv = hashp->hctl;
 	HASHBUCKET	newElement;
 
 	for (;;)
 	{
 		/* if partitioned, must lock to touch nentries and freeList */
-		if (IS_PARTITIONED(hctlv))
+		if (IS_PARTITIONED(hctl))
-			SpinLockAcquire(&hctlv->mutex);
+			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))
+		if (IS_PARTITIONED(hctl))
-			SpinLockRelease(&hctlv->mutex);
+			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;
+	hctl->freeList = newElement->link;
-	hctlv->nentries++;
+	hctl->nentries++;
 
-	if (IS_PARTITIONED(hctlv))
+	if (IS_PARTITIONED(hctl))
-		SpinLockRelease(&hctlv->mutex);
+		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 */
+	HASHHDR *hctl = hashp->hctl;
-	volatile HASHHDR *hctlv = 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))
+	if (IS_PARTITIONED(hctl))
-		SpinLockAcquire(&hctlv->mutex);
+		SpinLockAcquire(&hctl->mutex);
 
 	/* freelist could be nonempty if two backends did this concurrently */
-	firstElement->link = hctlv->freeList;
+	firstElement->link = hctl->freeList;
-	hctlv->freeList = prevElement;
+	hctl->freeList = prevElement;
 
-	if (IS_PARTITIONED(hctlv))
+	if (IS_PARTITIONED(hctl))
-		SpinLockRelease(&hctlv->mutex);
+		SpinLockRelease(&hctl->mutex);
 
 	return true;
 }