From 38fea11563137df6d6bcfcce1e4cd14fee9cc818 Mon Sep 17 00:00:00 2001 From: Alexandre Felipe Date: Sun, 6 Sep 2026 01:20:31 +0100 Subject: [PATCH] lwlock: LW_EXCLUSIVE Rooms make clean; this patch changes LWLockMode enum. Currently LW_LOCK supports LW_SHARED and LW_EXCLUSIVE mode, LW_SHARED has no practical limits as it simply count holders and the counter range is [0, MAX_BACKENDS]. For LW_EXCLUSIVE mode it accepts exactly one holder at a time. This patch includes the concept of rooms, is the analogy of a hotel where EXCLUSIVE access is granted to each room individually. The implementation consists in tracking looks to 16 rooms. The backward compatible behaviour is locking all the rooms, when acquiring the LWLock in exclusive mode. Shared mode doesn't support rooms (as the same bits will be used to tracke count holders), and the behaviour is as if every shared lock acquires a LW_SHARED lock for every room. This doesn't change exclusive-shared contention, but can be employed to reduce exclusive-exclusive contention. --- src/backend/storage/lmgr/lwlock.c | 164 +++++++++++++++++++++--------- src/include/storage/lwlock.h | 93 +++++++++++++++-- src/include/storage/proc.h | 1 + 3 files changed, 202 insertions(+), 56 deletions(-) diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index 82a1d4d2e26..2b54f6e49c4 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -46,7 +46,7 @@ * * For lock acquisition we use an atomic compare-and-exchange on the lockcount * variable. For exclusive lock we swap in a sentinel value - * (LW_VAL_EXCLUSIVE), for shared locks we count the number of holders. + * (LW_EXCLUSIVE), for shared locks we count the number of holders. * * To release the lock we use an atomic decrement to release the lock. If the * new value is zero (we get that atomically), we know we can/have to release @@ -100,13 +100,9 @@ #define LW_FLAG_MASK (((1<nwaiters), @@ -765,7 +764,7 @@ LWLockAttemptLock(LWLock *lock, LWLockMode mode) { uint32 old_state; - Assert(mode == LW_EXCLUSIVE || mode == LW_SHARED); + Assert(mode == LW_SHARED || (mode & LW_LOCK_MODE_MASK) == LW_EXCLUSIVE); /* * Read once outside the loop, later iterations will get the newer value @@ -781,17 +780,24 @@ LWLockAttemptLock(LWLock *lock, LWLockMode mode) desired_state = old_state; - if (mode == LW_EXCLUSIVE) + if ((mode & LW_LOCK_MODE_MASK) == LW_EXCLUSIVE) { - lock_free = (old_state & LW_LOCK_MASK) == 0; + uint32 rooms = mode & LW_LOCK_ROOM_MASK; + Assert(rooms != 0); + + if (old_state & LW_EXCLUSIVE) + lock_free = (old_state & rooms) == 0; /* rooms vs rooms */ + else + lock_free = (old_state & LW_SHARED_MASK) == 0; /* any shared count blocks */ + if (lock_free) - desired_state += LW_VAL_EXCLUSIVE; + desired_state |= LW_EXCLUSIVE | rooms; } else { - lock_free = (old_state & LW_VAL_EXCLUSIVE) == 0; + lock_free = (old_state & LW_EXCLUSIVE) == 0; if (lock_free) - desired_state += LW_VAL_SHARED; + desired_state += LW_SHARED; } /* @@ -811,7 +817,7 @@ LWLockAttemptLock(LWLock *lock, LWLockMode mode) { /* Great! Got the lock. */ #ifdef LOCK_DEBUG - if (mode == LW_EXCLUSIVE) + if ((mode & LW_LOCK_MODE_MASK) == LW_EXCLUSIVE) lock->owner = MyProc; #endif return false; @@ -903,8 +909,10 @@ LWLockWaitListUnlock(LWLock *lock) static void LWLockWakeup(LWLock *lock) { + /* will be set to LW_EXCLUSIVE or LW_SHARED later */ + LWLockMode awaking_mode = LW_WAIT_UNTIL_FREE; bool new_wake_in_progress = false; - bool wokeup_somebody = false; + uint32 held_rooms = 0; proclist_head wakeup; proclist_mutable_iter iter; @@ -913,11 +921,25 @@ LWLockWakeup(LWLock *lock) /* lock wait list while collecting backends to wake up */ LWLockWaitListLock(lock); + { + uint32 state = pg_atomic_read_u32(&lock->state); + + if (state & LW_EXCLUSIVE) + held_rooms = state & LW_LOCK_ROOM_MASK; + } + proclist_foreach_modify(iter, &lock->waiters, lwWaitLink) { PGPROC *waiter = GetPGProcByNumber(iter.cur); - if (wokeup_somebody && waiter->lwWaitMode == LW_EXCLUSIVE) + /* + * Don't mix LW_SHARED and LW_EXCLUSIVE + */ + if (new_wake_in_progress && waiter->lwWaitMode != awaking_mode) + continue; + + if (waiter->lwWaitMode == LW_EXCLUSIVE && + (waiter->lwLockRoom & held_rooms) != 0) continue; proclist_delete(&lock->waiters, iter.cur, lwWaitLink); @@ -931,11 +953,7 @@ LWLockWakeup(LWLock *lock) * automatically. */ new_wake_in_progress = true; - - /* - * Don't wakeup (further) exclusive locks. - */ - wokeup_somebody = true; + awaking_mode = waiter->lwWaitMode; } /* @@ -947,12 +965,17 @@ LWLockWakeup(LWLock *lock) 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. - */ if (waiter->lwWaitMode == LW_EXCLUSIVE) - break; + { + held_rooms |= waiter->lwLockRoom; + + /* + * Once we've woken up an exclusive lock that covers all remaining + * rooms, there's no point in waking up anybody else. + */ + if (held_rooms == LW_LOCK_ROOM_MASK) + break; + } } Assert(proclist_is_empty(&wakeup) || pg_atomic_read_u32(&lock->state) & LW_FLAG_HAS_WAITERS); @@ -1025,7 +1048,20 @@ LWLockQueueSelf(LWLock *lock, LWLockMode mode) if (MyProc == NULL) elog(PANIC, "cannot wait without a PGPROC structure"); - if (MyProc->lwWaiting != LW_WS_NOT_WAITING) + if (MyProc->lwWaiting == LW_WS_PENDING_WAKEUP) + { + /* + * Woken but not yet signalled; wait like LWLockDequeueSelf() so we + * never re-queue while still in a wakeup handoff. + */ + for (;;) + { + PGSemaphoreLock(MyProc->sem); + if (MyProc->lwWaiting == LW_WS_NOT_WAITING) + break; + } + } + else if (MyProc->lwWaiting != LW_WS_NOT_WAITING) elog(PANIC, "queueing for lock while waiting on another one"); LWLockWaitListLock(lock); @@ -1034,10 +1070,11 @@ LWLockQueueSelf(LWLock *lock, LWLockMode mode) pg_atomic_fetch_or_u32(&lock->state, LW_FLAG_HAS_WAITERS); MyProc->lwWaiting = LW_WS_WAITING; - MyProc->lwWaitMode = mode; + MyProc->lwWaitMode = mode & LW_LOCK_MODE_MASK; + MyProc->lwLockRoom = mode & LW_LOCK_ROOM_MASK; /* LW_WAIT_UNTIL_FREE waiters are always at the front of the queue */ - if (mode == LW_WAIT_UNTIL_FREE) + if ((mode & LW_LOCK_MODE_MASK) == LW_WAIT_UNTIL_FREE) proclist_push_head(&lock->waiters, MyProcNumber, lwWaitLink); else proclist_push_tail(&lock->waiters, MyProcNumber, lwWaitLink); @@ -1158,13 +1195,15 @@ LWLockAcquire(LWLock *lock, LWLockMode mode) lwstats = get_lwlock_stats_entry(lock); #endif - Assert(mode == LW_SHARED || mode == LW_EXCLUSIVE); - + Assert(mode == LW_SHARED || (mode & LW_LOCK_MODE_MASK) == LW_EXCLUSIVE); + /* assume all rooms on an exclusive lock if omitted */ + if (mode == LW_EXCLUSIVE) + mode |= LW_LOCK_ROOM_MASK; PRINT_LWDEBUG("LWLockAcquire", lock, mode); #ifdef LWLOCK_STATS /* Count lock acquisition attempts */ - if (mode == LW_EXCLUSIVE) + if ((mode & LW_LOCK_MODE_MASK) == LW_EXCLUSIVE) lwstats->ex_acquire_count++; else lwstats->sh_acquire_count++; @@ -1322,7 +1361,11 @@ LWLockConditionalAcquire(LWLock *lock, LWLockMode mode) { bool mustwait; - Assert(mode == LW_SHARED || mode == LW_EXCLUSIVE); + Assert(mode == LW_SHARED || (mode & LW_LOCK_MODE_MASK) == LW_EXCLUSIVE); + + /* assume all rooms on an exclusive lock if omitted */ + if (mode == LW_EXCLUSIVE) + mode |= LW_LOCK_ROOM_MASK; PRINT_LWDEBUG("LWLockConditionalAcquire", lock, mode); @@ -1386,7 +1429,11 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode) lwstats = get_lwlock_stats_entry(lock); #endif - Assert(mode == LW_SHARED || mode == LW_EXCLUSIVE); + Assert(mode == LW_SHARED || (mode & LW_LOCK_MODE_MASK) == LW_EXCLUSIVE); + + /* assume all rooms on an exclusive lock if omitted */ + if (mode == LW_EXCLUSIVE) + mode |= LW_LOCK_ROOM_MASK; PRINT_LWDEBUG("LWLockAcquireOrWait", lock, mode); @@ -1516,7 +1563,7 @@ LWLockConflictsWithVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval, * this, so we don't need a memory barrier here as far as the current * usage is concerned. But that might not be safe in general. */ - mustwait = (pg_atomic_read_u32(&lock->state) & LW_VAL_EXCLUSIVE) != 0; + mustwait = (pg_atomic_read_u32(&lock->state) & LW_EXCLUSIVE) != 0; if (!mustwait) { @@ -1716,7 +1763,7 @@ LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val) LWLockWaitListLock(lock); - Assert(pg_atomic_read_u32(&lock->state) & LW_VAL_EXCLUSIVE); + Assert(pg_atomic_read_u32(&lock->state) & LW_EXCLUSIVE); /* * See if there are any LW_WAIT_UNTIL_FREE waiters that need to be woken @@ -1794,24 +1841,44 @@ LWLockRelease(LWLock *lock) * Release my hold on lock, after that it can immediately be acquired by * others, even if we still have to wakeup other waiters. */ - if (mode == LW_EXCLUSIVE) - oldstate = pg_atomic_sub_fetch_u32(&lock->state, LW_VAL_EXCLUSIVE); + if ((mode & LW_LOCK_MODE_MASK) == LW_EXCLUSIVE) + { + /* + * Fast path: release the lock without looping if + * - no other holders + * - wait list not locked + * - no waiters + * - no wake in progress + */ + uint32 clear = (mode & LW_LOCK_ROOM_MASK) | LW_EXCLUSIVE; + oldstate = mode | LW_EXCLUSIVE; + while (!pg_atomic_compare_exchange_u32(&lock->state, &oldstate, + oldstate & ~clear)) + { + clear = mode & LW_LOCK_ROOM_MASK; + /* + * Release the exclusive lock if there are no remaining + * locked rooms. + */ + if ((oldstate & ((~mode & LW_LOCK_ROOM_MASK) | LW_FLAG_WAKE_IN_PROGRESS)) == 0) + clear |= LW_EXCLUSIVE; + } + } else - oldstate = pg_atomic_sub_fetch_u32(&lock->state, LW_VAL_SHARED); - - /* nobody else can have that kind of lock */ - Assert(!(oldstate & LW_VAL_EXCLUSIVE)); + oldstate = pg_atomic_sub_fetch_u32(&lock->state, LW_SHARED); if (TRACE_POSTGRESQL_LWLOCK_RELEASE_ENABLED()) TRACE_POSTGRESQL_LWLOCK_RELEASE(T_NAME(lock)); /* * Check if we're still waiting for backends to get scheduled, if so, - * don't wake them up again. + * don't wake them up again. Exclusive room release must wake even + * when other rooms remain; LWLockWakeup filters by waiter rooms. */ if ((oldstate & LW_FLAG_HAS_WAITERS) && !(oldstate & LW_FLAG_WAKE_IN_PROGRESS) && - (oldstate & LW_LOCK_MASK) == 0) + ((oldstate & LW_LOCK_MASK) == 0 || + (mode & LW_LOCK_MODE_MASK) == LW_EXCLUSIVE)) check_waiters = true; else check_waiters = false; @@ -1932,7 +1999,8 @@ LWLockHeldByMeInMode(LWLock *lock, LWLockMode mode) for (i = 0; i < num_held_lwlocks; i++) { - if (held_lwlocks[i].lock == lock && held_lwlocks[i].mode == mode) + if (held_lwlocks[i].lock == lock && + (held_lwlocks[i].mode & LW_LOCK_MODE_MASK) == (mode & LW_LOCK_MODE_MASK)) return true; } return false; diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index efa5b427e9f..cdc5195c158 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -31,7 +31,8 @@ typedef enum LWLockWaitState LW_WS_WAITING, /* currently waiting */ LW_WS_PENDING_WAKEUP, /* removed from waitlist, but not yet * signalled */ -} LWLockWaitState; +} LWLockWaitState; + /* * Code outside of lwlock.c should not manipulate the contents of this @@ -80,8 +81,8 @@ extern PGDLLIMPORT LWLockPadded *MainLWLockArray; */ /* Number of partitions of the shared buffer mapping hashtable */ -#define NUM_BUFFER_PARTITIONS 128 - +#define LOG2_NUM_BUFFER_PARTITIONS 7 +#define NUM_BUFFER_PARTITIONS (1 << LOG2_NUM_BUFFER_PARTITIONS) /* Number of partitions the shared lock tables are divided into */ #define LOG2_NUM_LOCK_PARTITIONS 4 #define NUM_LOCK_PARTITIONS (1 << LOG2_NUM_LOCK_PARTITIONS) @@ -101,14 +102,90 @@ extern PGDLLIMPORT LWLockPadded *MainLWLockArray; typedef enum LWLockMode { - LW_EXCLUSIVE, - LW_SHARED, - LW_WAIT_UNTIL_FREE, /* A special mode used in PGPROC->lwWaitMode, - * when waiting for lock to become free. Not - * to be used as LWLockAcquire argument */ + LW_EXCLUSIVE = 1, + LW_SHARED = 2, + /* A special mode used in PGPROC->lwWaitMode, + * when waiting for lock to become free. Not + * to be used as LWLockAcquire argument */ + LW_WAIT_UNTIL_FREE = 4, + /* For backward compatibility LW_EXCLUSIVE must + * act as LW_EXCLUSIVE + LW_LOCK_ROOM_MASK + */ + /* 16 rooms in two internal bytes */ + LW_LOCK_ROOM_0 = 0x00000100, + LW_LOCK_ROOM_1 = 0x00000200, + LW_LOCK_ROOM_2 = 0x00000400, + LW_LOCK_ROOM_3 = 0x00000800, + LW_LOCK_ROOM_4 = 0x00001000, + LW_LOCK_ROOM_5 = 0x00002000, + LW_LOCK_ROOM_6 = 0x00004000, + LW_LOCK_ROOM_7 = 0x00008000, + + LW_LOCK_ROOM_8 = 0x00010000, + LW_LOCK_ROOM_9 = 0x00020000, + LW_LOCK_ROOM_10 = 0x00040000, + LW_LOCK_ROOM_11 = 0x00080000, + LW_LOCK_ROOM_12 = 0x00100000, + LW_LOCK_ROOM_13 = 0x00200000, + LW_LOCK_ROOM_14 = 0x00400000, + LW_LOCK_ROOM_15 = 0x00800000, + /* + * Alternatively use A-Z, where 5+5 rooms are bits of the + * least and the most significant bytes. + * F-U coincide with 1-15. + */ + LW_LOCK_ROOM_A = 0x00000008, + LW_LOCK_ROOM_B = 0x00000010, + LW_LOCK_ROOM_C = 0x00000020, + LW_LOCK_ROOM_D = 0x00000040, + LW_LOCK_ROOM_E = 0x00000080, + + LW_LOCK_ROOM_F = 0x00000100, + LW_LOCK_ROOM_G = 0x00000200, + LW_LOCK_ROOM_H = 0x00000400, + LW_LOCK_ROOM_I = 0x00000800, + LW_LOCK_ROOM_J = 0x00001000, + LW_LOCK_ROOM_K = 0x00002000, + LW_LOCK_ROOM_L = 0x00004000, + LW_LOCK_ROOM_M = 0x00008000, + + LW_LOCK_ROOM_N = 0x00010000, + LW_LOCK_ROOM_O = 0x00020000, + LW_LOCK_ROOM_P = 0x00040000, + LW_LOCK_ROOM_Q = 0x00080000, + LW_LOCK_ROOM_R = 0x00100000, + LW_LOCK_ROOM_S = 0x00200000, + LW_LOCK_ROOM_T = 0x00400000, + LW_LOCK_ROOM_U = 0x00800000, + + LW_LOCK_ROOM_V = 0x01000000, + LW_LOCK_ROOM_W = 0x02000000, + LW_LOCK_ROOM_X = 0x04000000, + LW_LOCK_ROOM_Y = 0x08000000, + LW_LOCK_ROOM_Z = 0x10000000, + + /* + * 3 high bits reserved for flags in LWLock state + * 3 low bits reserved for modes in LWLockMode + * 26 bits usable as room mask. + */ + LW_LOCK_ROOM_MASK = 0x1ffffff8, + LW_LOCK_MODE_MASK = 0x00000007, } LWLockMode; +/* + * A helper to acquire the lock for one room out of 16 + * LW_EXCLUSIVE + LW_LOCK_ROOM(3) + * LW_EXCLUSIVE + LW_ROOM_3 + */ +#define LW_LOCK_ROOM(i) ((LWLockMode) (1 << (8 + ((i) & 0xF)))) +/* Alternatively one can lock one room out of 26 + * LW_EXCLUSIVE + LW_LOCK_ROOM_ALPHA('A') + * LW_EXCLUSIVE + LW_LOCK_ROOM_ALPHA(12345) + * LW_EXCLUSIVE + LW_LOCK_ROOM_A + */ +#define LW_LOCK_ROOM_ALPHA(i) ((LWLockMode) (1 << (3 + ((i) - 65) % 26))) #ifdef LOCK_DEBUG extern PGDLLIMPORT bool Trace_lwlocks; #endif diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 03a1a466fa8..47232082d61 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -286,6 +286,7 @@ typedef struct PGPROC * is acceptable, although not pretty, because a backend can't wait for * both types of locks at the same time. */ + uint32 lwLockRoom; /* lock room being waited for */ uint8 lwWaiting; /* see LWLockWaitState */ uint8 lwWaitMode; /* lwlock mode being waited for */ proclist_node lwWaitLink; /* position in LW lock wait list */ -- 2.53.0