]> granicus.if.org Git - postgresql/commitdiff
Remove volatile qualifiers from lwlock.c.
authorRobert Haas <rhaas@postgresql.org>
Mon, 22 Sep 2014 20:42:14 +0000 (16:42 -0400)
committerRobert Haas <rhaas@postgresql.org>
Mon, 22 Sep 2014 20:42:14 +0000 (16:42 -0400)
Now that spinlocks (hopefully!) act as compiler barriers, as of commit
0709b7ee72e4bc71ad07b7120acd117265ab51d0, this should be safe.  This
serves as a demonstration of the new coding style, and may be optimized
better on some machines as well.

src/backend/storage/lmgr/lwlock.c

index 7c96da5c3d8b30ef6274df190f2f6f6b51fffb5c..66fb2e42317ae45d4acc6129460b22af6e875eb0 100644 (file)
@@ -112,7 +112,7 @@ static lwlock_stats lwlock_stats_dummy;
 bool           Trace_lwlocks = false;
 
 inline static void
-PRINT_LWDEBUG(const char *where, const volatile LWLock *lock)
+PRINT_LWDEBUG(const char *where, const LWLock *lock)
 {
        if (Trace_lwlocks)
                elog(LOG, "%s(%s %d): excl %d shared %d head %p rOK %d",
@@ -406,9 +406,7 @@ LWLock *
 LWLockAssign(void)
 {
        LWLock     *result;
-
-       /* use volatile pointer to prevent code rearrangement */
-       volatile int *LWLockCounter;
+       int                *LWLockCounter;
 
        LWLockCounter = (int *) ((char *) MainLWLockArray - 3 * sizeof(int));
        SpinLockAcquire(ShmemLock);
@@ -429,9 +427,7 @@ int
 LWLockNewTrancheId(void)
 {
        int                     result;
-
-       /* use volatile pointer to prevent code rearrangement */
-       volatile int *LWLockCounter;
+       int                *LWLockCounter;
 
        LWLockCounter = (int *) ((char *) MainLWLockArray - 3 * sizeof(int));
        SpinLockAcquire(ShmemLock);
@@ -511,9 +507,8 @@ LWLockAcquireWithVar(LWLock *l, uint64 *valptr, uint64 val)
 
 /* internal function to implement LWLockAcquire and LWLockAcquireWithVar */
 static inline bool
-LWLockAcquireCommon(LWLock *l, LWLockMode mode, uint64 *valptr, uint64 val)
+LWLockAcquireCommon(LWLock *lock, LWLockMode mode, uint64 *valptr, uint64 val)
 {
-       volatile LWLock *lock = l;
        PGPROC     *proc = MyProc;
        bool            retry = false;
        bool            result = true;
@@ -525,7 +520,7 @@ LWLockAcquireCommon(LWLock *l, LWLockMode mode, uint64 *valptr, uint64 val)
        PRINT_LWDEBUG("LWLockAcquire", lock);
 
 #ifdef LWLOCK_STATS
-       lwstats = get_lwlock_stats_entry(l);
+       lwstats = get_lwlock_stats_entry(lock);
 
        /* Count lock acquisition attempts */
        if (mode == LW_EXCLUSIVE)
@@ -642,13 +637,13 @@ LWLockAcquireCommon(LWLock *l, LWLockMode mode, uint64 *valptr, uint64 val)
                 * so that the lock manager or signal manager will see the received
                 * signal when it next waits.
                 */
-               LOG_LWDEBUG("LWLockAcquire", T_NAME(l), T_ID(l), "waiting");
+               LOG_LWDEBUG("LWLockAcquire", T_NAME(lock), T_ID(lock), "waiting");
 
 #ifdef LWLOCK_STATS
                lwstats->block_count++;
 #endif
 
-               TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(l), T_ID(l), mode);
+               TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), T_ID(lock), mode);
 
                for (;;)
                {
@@ -659,9 +654,9 @@ LWLockAcquireCommon(LWLock *l, LWLockMode mode, uint64 *valptr, uint64 val)
                        extraWaits++;
                }
 
-               TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(l), T_ID(l), mode);
+               TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), T_ID(lock), mode);
 
-               LOG_LWDEBUG("LWLockAcquire", T_NAME(l), T_ID(l), "awakened");
+               LOG_LWDEBUG("LWLockAcquire", T_NAME(lock), T_ID(lock), "awakened");
 
                /* Now loop back and try to acquire lock again. */
                retry = true;
@@ -675,10 +670,10 @@ LWLockAcquireCommon(LWLock *l, LWLockMode mode, uint64 *valptr, uint64 val)
        /* We are done updating shared state of the lock itself. */
        SpinLockRelease(&lock->mutex);
 
-       TRACE_POSTGRESQL_LWLOCK_ACQUIRE(T_NAME(l), T_ID(l), mode);
+       TRACE_POSTGRESQL_LWLOCK_ACQUIRE(T_NAME(lock), T_ID(lock), mode);
 
        /* Add lock to list of locks held by this backend */
-       held_lwlocks[num_held_lwlocks++] = l;
+       held_lwlocks[num_held_lwlocks++] = lock;
 
        /*
         * Fix the process wait semaphore's count for any absorbed wakeups.
@@ -697,9 +692,8 @@ LWLockAcquireCommon(LWLock *l, LWLockMode mode, uint64 *valptr, uint64 val)
  * If successful, cancel/die interrupts are held off until lock release.
  */
 bool
-LWLockConditionalAcquire(LWLock *l, LWLockMode mode)
+LWLockConditionalAcquire(LWLock *lock, LWLockMode mode)
 {
-       volatile LWLock *lock = l;
        bool            mustwait;
 
        PRINT_LWDEBUG("LWLockConditionalAcquire", lock);
@@ -747,14 +741,16 @@ LWLockConditionalAcquire(LWLock *l, LWLockMode mode)
        {
                /* Failed to get lock, so release interrupt holdoff */
                RESUME_INTERRUPTS();
-               LOG_LWDEBUG("LWLockConditionalAcquire", T_NAME(l), T_ID(l), "failed");
-               TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(T_NAME(l), T_ID(l), mode);
+               LOG_LWDEBUG("LWLockConditionalAcquire",
+                                       T_NAME(lock), T_ID(lock), "failed");
+               TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE_FAIL(T_NAME(lock),
+                                                                                                T_ID(lock), mode);
        }
        else
        {
                /* Add lock to list of locks held by this backend */
-               held_lwlocks[num_held_lwlocks++] = l;
-               TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE(T_NAME(l), T_ID(l), mode);
+               held_lwlocks[num_held_lwlocks++] = lock;
+               TRACE_POSTGRESQL_LWLOCK_CONDACQUIRE(T_NAME(lock), T_ID(lock), mode);
        }
 
        return !mustwait;
@@ -775,9 +771,8 @@ LWLockConditionalAcquire(LWLock *l, LWLockMode mode)
  * wake up, observe that their records have already been flushed, and return.
  */
 bool
-LWLockAcquireOrWait(LWLock *l, LWLockMode mode)
+LWLockAcquireOrWait(LWLock *lock, LWLockMode mode)
 {
-       volatile LWLock *lock = l;
        PGPROC     *proc = MyProc;
        bool            mustwait;
        int                     extraWaits = 0;
@@ -788,7 +783,7 @@ LWLockAcquireOrWait(LWLock *l, LWLockMode mode)
        PRINT_LWDEBUG("LWLockAcquireOrWait", lock);
 
 #ifdef LWLOCK_STATS
-       lwstats = get_lwlock_stats_entry(l);
+       lwstats = get_lwlock_stats_entry(lock);
 #endif
 
        /* Ensure we will have room to remember the lock */
@@ -855,13 +850,14 @@ LWLockAcquireOrWait(LWLock *l, LWLockMode mode)
                 * Wait until awakened.  Like in LWLockAcquire, be prepared for bogus
                 * wakups, because we share the semaphore with ProcWaitForSignal.
                 */
-               LOG_LWDEBUG("LWLockAcquireOrWait", T_NAME(l), T_ID(l), "waiting");
+               LOG_LWDEBUG("LWLockAcquireOrWait", T_NAME(lock), T_ID(lock),
+                                       "waiting");
 
 #ifdef LWLOCK_STATS
                lwstats->block_count++;
 #endif
 
-               TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(l), T_ID(l), mode);
+               TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), T_ID(lock), mode);
 
                for (;;)
                {
@@ -872,9 +868,10 @@ LWLockAcquireOrWait(LWLock *l, LWLockMode mode)
                        extraWaits++;
                }
 
-               TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(l), T_ID(l), mode);
+               TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), T_ID(lock), mode);
 
-               LOG_LWDEBUG("LWLockAcquireOrWait", T_NAME(l), T_ID(l), "awakened");
+               LOG_LWDEBUG("LWLockAcquireOrWait", T_NAME(lock), T_ID(lock),
+                                       "awakened");
        }
        else
        {
@@ -892,14 +889,16 @@ LWLockAcquireOrWait(LWLock *l, LWLockMode mode)
        {
                /* Failed to get lock, so release interrupt holdoff */
                RESUME_INTERRUPTS();
-               LOG_LWDEBUG("LWLockAcquireOrWait", T_NAME(l), T_ID(l), "failed");
-               TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT_FAIL(T_NAME(l), T_ID(l), mode);
+               LOG_LWDEBUG("LWLockAcquireOrWait", T_NAME(lock), T_ID(lock), "failed");
+               TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT_FAIL(T_NAME(lock), T_ID(lock),
+                                                                                                        mode);
        }
        else
        {
                /* Add lock to list of locks held by this backend */
-               held_lwlocks[num_held_lwlocks++] = l;
-               TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT(T_NAME(l), T_ID(l), mode);
+               held_lwlocks[num_held_lwlocks++] = lock;
+               TRACE_POSTGRESQL_LWLOCK_ACQUIRE_OR_WAIT(T_NAME(lock), T_ID(lock),
+                                                                                               mode);
        }
 
        return !mustwait;
@@ -924,10 +923,8 @@ LWLockAcquireOrWait(LWLock *l, LWLockMode mode)
  * in shared mode, returns 'true'.
  */
 bool
-LWLockWaitForVar(LWLock *l, uint64 *valptr, uint64 oldval, uint64 *newval)
+LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval)
 {
-       volatile LWLock *lock = l;
-       volatile uint64 *valp = valptr;
        PGPROC     *proc = MyProc;
        int                     extraWaits = 0;
        bool            result = false;
@@ -938,7 +935,7 @@ LWLockWaitForVar(LWLock *l, uint64 *valptr, uint64 oldval, uint64 *newval)
        PRINT_LWDEBUG("LWLockWaitForVar", lock);
 
 #ifdef LWLOCK_STATS
-       lwstats = get_lwlock_stats_entry(l);
+       lwstats = get_lwlock_stats_entry(lock);
 #endif   /* LWLOCK_STATS */
 
        /*
@@ -981,7 +978,7 @@ LWLockWaitForVar(LWLock *l, uint64 *valptr, uint64 oldval, uint64 *newval)
                }
                else
                {
-                       value = *valp;
+                       value = *valptr;
                        if (value != oldval)
                        {
                                result = false;
@@ -1023,13 +1020,14 @@ LWLockWaitForVar(LWLock *l, uint64 *valptr, uint64 oldval, uint64 *newval)
                 * so that the lock manager or signal manager will see the received
                 * signal when it next waits.
                 */
-               LOG_LWDEBUG("LWLockWaitForVar", T_NAME(l), T_ID(l), "waiting");
+               LOG_LWDEBUG("LWLockWaitForVar", T_NAME(lock), T_ID(lock), "waiting");
 
 #ifdef LWLOCK_STATS
                lwstats->block_count++;
 #endif
 
-               TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(l), T_ID(l), LW_EXCLUSIVE);
+               TRACE_POSTGRESQL_LWLOCK_WAIT_START(T_NAME(lock), T_ID(lock),
+                                                                                  LW_EXCLUSIVE);
 
                for (;;)
                {
@@ -1040,9 +1038,10 @@ LWLockWaitForVar(LWLock *l, uint64 *valptr, uint64 oldval, uint64 *newval)
                        extraWaits++;
                }
 
-               TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(l), T_ID(l), LW_EXCLUSIVE);
+               TRACE_POSTGRESQL_LWLOCK_WAIT_DONE(T_NAME(lock), T_ID(lock),
+                                                                                 LW_EXCLUSIVE);
 
-               LOG_LWDEBUG("LWLockWaitForVar", T_NAME(l), T_ID(l), "awakened");
+               LOG_LWDEBUG("LWLockWaitForVar", T_NAME(lock), T_ID(lock), "awakened");
 
                /* Now loop back and check the status of the lock again. */
        }
@@ -1050,7 +1049,7 @@ LWLockWaitForVar(LWLock *l, uint64 *valptr, uint64 oldval, uint64 *newval)
        /* We are done updating shared state of the lock itself. */
        SpinLockRelease(&lock->mutex);
 
-       TRACE_POSTGRESQL_LWLOCK_ACQUIRE(T_NAME(l), T_ID(l), LW_EXCLUSIVE);
+       TRACE_POSTGRESQL_LWLOCK_ACQUIRE(T_NAME(lock), T_ID(lock), LW_EXCLUSIVE);
 
        /*
         * Fix the process wait semaphore's count for any absorbed wakeups.
@@ -1078,10 +1077,8 @@ LWLockWaitForVar(LWLock *l, uint64 *valptr, uint64 oldval, uint64 *newval)
  * The caller must be holding the lock in exclusive mode.
  */
 void
-LWLockUpdateVar(LWLock *l, uint64 *valptr, uint64 val)
+LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val)
 {
-       volatile LWLock *lock = l;
-       volatile uint64 *valp = valptr;
        PGPROC     *head;
        PGPROC     *proc;
        PGPROC     *next;
@@ -1093,7 +1090,7 @@ LWLockUpdateVar(LWLock *l, uint64 *valptr, uint64 val)
        Assert(lock->exclusive == 1);
 
        /* Update the lock's value */
-       *valp = val;
+       *valptr = val;
 
        /*
         * See if there are any LW_WAIT_UNTIL_FREE waiters that need to be woken
@@ -1139,9 +1136,8 @@ LWLockUpdateVar(LWLock *l, uint64 *valptr, uint64 val)
  * LWLockRelease - release a previously acquired lock
  */
 void
-LWLockRelease(LWLock *l)
+LWLockRelease(LWLock *lock)
 {
-       volatile LWLock *lock = l;
        PGPROC     *head;
        PGPROC     *proc;
        int                     i;
@@ -1154,11 +1150,11 @@ LWLockRelease(LWLock *l)
         */
        for (i = num_held_lwlocks; --i >= 0;)
        {
-               if (l == held_lwlocks[i])
+               if (lock == held_lwlocks[i])
                        break;
        }
        if (i < 0)
-               elog(ERROR, "lock %s %d is not held", T_NAME(l), T_ID(l));
+               elog(ERROR, "lock %s %d is not held", T_NAME(lock), T_ID(lock));
        num_held_lwlocks--;
        for (; i < num_held_lwlocks; i++)
                held_lwlocks[i] = held_lwlocks[i + 1];
@@ -1238,14 +1234,15 @@ LWLockRelease(LWLock *l)
        /* We are done updating shared state of the lock itself. */
        SpinLockRelease(&lock->mutex);
 
-       TRACE_POSTGRESQL_LWLOCK_RELEASE(T_NAME(l), T_ID(l));
+       TRACE_POSTGRESQL_LWLOCK_RELEASE(T_NAME(lock), T_ID(lock));
 
        /*
         * Awaken any waiters I removed from the queue.
         */
        while (head != NULL)
        {
-               LOG_LWDEBUG("LWLockRelease", T_NAME(l), T_ID(l), "release waiter");
+               LOG_LWDEBUG("LWLockRelease", T_NAME(lock), T_ID(lock),
+                                       "release waiter");
                proc = head;
                head = proc->lwWaitLink;
                proc->lwWaitLink = NULL;