Thread: Non-volatile variables used for spinlock manipulation

Since SpinLockAcquire() / SpinLockRelease() macros usually reference variables
declared as volatile, I wonder if the following changes should be applied.

--
Antonin Houska
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt
Web: http://www.postgresql-support.de, http://www.cybertec.at

diff --git a/src/backend/storage/ipc/shm_mq.c b/src/backend/storage/ipc/shm_mq.c
index 0e60dbc..256d09d 100644
--- a/src/backend/storage/ipc/shm_mq.c
+++ b/src/backend/storage/ipc/shm_mq.c
@@ -195,11 +195,11 @@ shm_mq_set_receiver(shm_mq *mq, PGPROC *proc)
     volatile shm_mq *vmq = mq;
     PGPROC       *sender;

-    SpinLockAcquire(&mq->mq_mutex);
+    SpinLockAcquire(&vmq->mq_mutex);
     Assert(vmq->mq_receiver == NULL);
     vmq->mq_receiver = proc;
     sender = vmq->mq_sender;
-    SpinLockRelease(&mq->mq_mutex);
+    SpinLockRelease(&vmq->mq_mutex);

     if (sender != NULL)
         SetLatch(&sender->procLatch);
@@ -214,11 +214,11 @@ shm_mq_set_sender(shm_mq *mq, PGPROC *proc)
     volatile shm_mq *vmq = mq;
     PGPROC       *receiver;

-    SpinLockAcquire(&mq->mq_mutex);
+    SpinLockAcquire(&vmq->mq_mutex);
     Assert(vmq->mq_sender == NULL);
     vmq->mq_sender = proc;
     receiver = vmq->mq_receiver;
-    SpinLockRelease(&mq->mq_mutex);
+    SpinLockRelease(&vmq->mq_mutex);

     if (receiver != NULL)
         SetLatch(&receiver->procLatch);
@@ -233,9 +233,9 @@ shm_mq_get_receiver(shm_mq *mq)
     volatile shm_mq *vmq = mq;
     PGPROC       *receiver;

-    SpinLockAcquire(&mq->mq_mutex);
+    SpinLockAcquire(&vmq->mq_mutex);
     receiver = vmq->mq_receiver;
-    SpinLockRelease(&mq->mq_mutex);
+    SpinLockRelease(&vmq->mq_mutex);

     return receiver;
 }
@@ -249,9 +249,9 @@ shm_mq_get_sender(shm_mq *mq)
     volatile shm_mq *vmq = mq;
     PGPROC       *sender;

-    SpinLockAcquire(&mq->mq_mutex);
+    SpinLockAcquire(&vmq->mq_mutex);
     sender = vmq->mq_sender;
-    SpinLockRelease(&mq->mq_mutex);
+    SpinLockRelease(&vmq->mq_mutex);

     return sender;
 }

Antonin Houska <ah@cybertec.at> writes:
> Since SpinLockAcquire() / SpinLockRelease() macros usually reference variables
> declared as volatile, I wonder if the following changes should be applied.

We've been making changes to remove that requirement, so I think that at
least in 9.5/HEAD this isn't an issue anymore.
        regards, tom lane

On 2015-09-04 09:42:20 -0400, Tom Lane wrote:
> Antonin Houska <ah@cybertec.at> writes:
> > Since SpinLockAcquire() / SpinLockRelease() macros usually reference variables
> > declared as volatile, I wonder if the following changes should be applied.
> 
> We've been making changes to remove that requirement, so I think that at
> least in 9.5/HEAD this isn't an issue anymore.

And even before that the pointer passed to SpinLockAcquire/Release()
didn't have to be volatile. The important thing was that read/writes
that are protected by the spinlock were only done via volatile
variables, to prevent them from being moved outside the critical
section. All the tas()/unlock section already force the pointer to the
lock to be volatile anyway.