diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c index e9ab612717..71b54e0292 100644 --- a/src/backend/access/transam/twophase.c +++ b/src/backend/access/transam/twophase.c @@ -485,7 +485,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid, proc->roleId = owner; proc->tempNamespaceId = InvalidOid; proc->isBackgroundWorker = false; - proc->lwWaiting = false; + proc->lwWaiting = LW_WS_NOT_WAITING; proc->lwWaitMode = 0; proc->waitLock = NULL; proc->waitProcLock = NULL; diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index 6b6e920704..933831edf5 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -913,6 +913,15 @@ LWLockWakeup(LWLock *lock) wokeup_somebody = true; } + /* + * Signal that the process isn't on the wait list anymore. This allows + * LWLockDequeueSelf() to remove itself of the waitlist with a + * proclist_delete(), rather than having to check if it has been + * removed from the list. + */ + Assert(waiter->lwWaiting == LW_WS_WAITING); + waiter->lwWaiting = LW_WS_PENDING_WAKEUP; + /* * Once we've woken up an exclusive lock, there's no point in waking * up anybody else. @@ -970,7 +979,7 @@ LWLockWakeup(LWLock *lock) * another lock. */ pg_write_barrier(); - waiter->lwWaiting = false; + waiter->lwWaiting = LW_WS_NOT_WAITING; PGSemaphoreUnlock(waiter->sem); } } @@ -991,7 +1000,7 @@ LWLockQueueSelf(LWLock *lock, LWLockMode mode) if (MyProc == NULL) elog(PANIC, "cannot wait without a PGPROC structure"); - if (MyProc->lwWaiting) + if (MyProc->lwWaiting != LW_WS_NOT_WAITING) elog(PANIC, "queueing for lock while waiting on another one"); LWLockWaitListLock(lock); @@ -999,7 +1008,7 @@ LWLockQueueSelf(LWLock *lock, LWLockMode mode) /* setting the flag is protected by the spinlock */ pg_atomic_fetch_or_u32(&lock->state, LW_FLAG_HAS_WAITERS); - MyProc->lwWaiting = true; + MyProc->lwWaiting = LW_WS_WAITING; MyProc->lwWaitMode = mode; /* LW_WAIT_UNTIL_FREE waiters are always at the front of the queue */ @@ -1027,8 +1036,7 @@ LWLockQueueSelf(LWLock *lock, LWLockMode mode) static void LWLockDequeueSelf(LWLock *lock) { - bool found = false; - proclist_mutable_iter iter; + bool on_waitlist; #ifdef LWLOCK_STATS lwlock_stats *lwstats; @@ -1041,18 +1049,13 @@ LWLockDequeueSelf(LWLock *lock) LWLockWaitListLock(lock); /* - * Can't just remove ourselves from the list, but we need to iterate over - * all entries as somebody else could have dequeued us. + * Remove ourselves from the waitlist, unless we've already been + * removed. The removal happens with the wait list lock held, so there's + * no race in this check. */ - proclist_foreach_modify(iter, &lock->waiters, lwWaitLink) - { - if (iter.cur == MyProc->pgprocno) - { - found = true; - proclist_delete(&lock->waiters, iter.cur, lwWaitLink); - break; - } - } + on_waitlist = MyProc->lwWaiting == LW_WS_WAITING; + if (on_waitlist) + proclist_delete(&lock->waiters, MyProc->pgprocno, lwWaitLink); if (proclist_is_empty(&lock->waiters) && (pg_atomic_read_u32(&lock->state) & LW_FLAG_HAS_WAITERS) != 0) @@ -1064,8 +1067,8 @@ LWLockDequeueSelf(LWLock *lock) LWLockWaitListUnlock(lock); /* clear waiting state again, nice for debugging */ - if (found) - MyProc->lwWaiting = false; + if (on_waitlist) + MyProc->lwWaiting = LW_WS_NOT_WAITING; else { int extraWaits = 0; @@ -1089,7 +1092,7 @@ LWLockDequeueSelf(LWLock *lock) for (;;) { PGSemaphoreLock(MyProc->sem); - if (!MyProc->lwWaiting) + if (MyProc->lwWaiting == LW_WS_NOT_WAITING) break; extraWaits++; } @@ -1239,7 +1242,7 @@ LWLockAcquire(LWLock *lock, LWLockMode mode) for (;;) { PGSemaphoreLock(proc->sem); - if (!proc->lwWaiting) + if (proc->lwWaiting == LW_WS_NOT_WAITING) break; extraWaits++; } @@ -1399,7 +1402,7 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode) for (;;) { PGSemaphoreLock(proc->sem); - if (!proc->lwWaiting) + if (proc->lwWaiting == LW_WS_NOT_WAITING) break; extraWaits++; } @@ -1611,7 +1614,7 @@ LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval) for (;;) { PGSemaphoreLock(proc->sem); - if (!proc->lwWaiting) + if (proc->lwWaiting == LW_WS_NOT_WAITING) break; extraWaits++; } @@ -1690,6 +1693,10 @@ LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val) proclist_delete(&lock->waiters, iter.cur, lwWaitLink); proclist_push_tail(&wakeup, iter.cur, lwWaitLink); + + /* see LWLockWakeup() */ + Assert(waiter->lwWaiting == LW_WS_WAITING); + waiter->lwWaiting = LW_WS_PENDING_WAKEUP; } /* We are done updating shared state of the lock itself. */ @@ -1705,7 +1712,7 @@ LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val) proclist_delete(&wakeup, iter.cur, lwWaitLink); /* check comment in LWLockWakeup() about this barrier */ pg_write_barrier(); - waiter->lwWaiting = false; + waiter->lwWaiting = LW_WS_NOT_WAITING; PGSemaphoreUnlock(waiter->sem); } } diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 80a8b48c3c..83b0610dce 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -403,7 +403,7 @@ InitProcess(void) /* NB -- autovac launcher intentionally does not set IS_AUTOVACUUM */ if (IsAutoVacuumWorkerProcess()) MyPgXact->vacuumFlags |= PROC_IS_AUTOVACUUM; - MyProc->lwWaiting = false; + MyProc->lwWaiting = LW_WS_NOT_WAITING; MyProc->lwWaitMode = 0; MyProc->waitLock = NULL; MyProc->waitProcLock = NULL; @@ -583,7 +583,7 @@ InitAuxiliaryProcess(void) MyPgXact->delayChkpt = false; MyProc->delayChkptEnd = false; MyPgXact->vacuumFlags = 0; - MyProc->lwWaiting = false; + MyProc->lwWaiting = LW_WS_NOT_WAITING; MyProc->lwWaitMode = 0; MyProc->waitLock = NULL; MyProc->waitProcLock = NULL; diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index 87b1ce083d..a30a7d08e9 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -24,6 +24,14 @@ struct PGPROC; +/* what state of the wait process is a backend in */ +typedef enum LWLockWaitState +{ + LW_WS_NOT_WAITING, /* not currently waiting / woken up */ + LW_WS_WAITING, /* currently waiting */ + LW_WS_PENDING_WAKEUP /* removed from waitlist, but not yet signalled */ +} LWLockWaitState; + /* * Code outside of lwlock.c should not manipulate the contents of this * structure directly, but we have to declare it here to allow LWLocks to be diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 7024df206d..5edb22f49d 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -134,7 +134,7 @@ struct PGPROC bool recoveryConflictPending; /* Info about LWLock the process is currently waiting for, if any. */ - bool lwWaiting; /* true if waiting for an LW lock */ + uint8 lwWaiting; /* see LWLockWaitState */ uint8 lwWaitMode; /* lwlock mode being waited for */ proclist_node lwWaitLink; /* position in LW lock wait list */