From 8f9dfc9f85fc0169a94838e29b51e910be1fe7b3 Mon Sep 17 00:00:00 2001 From: Alexandre Felipe Date: Tue, 8 Sep 2026 04:32:51 +0100 Subject: [PATCH 8/9] bufmgr: no lookup locks Commented out the LW_SHARED buffer mapping locks in bufmgr. This passes `make check` but not make check-world. make -C src/test/recovery check PROVE_TESTS="t/027_stream_regress.pl" The algorithm is lock-free but if the bucket is repeatedly having concurrent deletions during bucket might trigger a retry if one buffer is repeatedly deleted and inserted we might get blocked. Retrying forever would consume too much CPU, my initial version was with 5 retries, and I thought that would be more than enough, here we have 1000. This version also attempts checking wether the previous entry, remains in the correct chain and repeating only the last link. --- src/backend/storage/buffer/buf_table.c | 16 ++++++++++++---- src/backend/storage/buffer/bufmgr.c | 22 +++++++++++----------- 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/src/backend/storage/buffer/buf_table.c b/src/backend/storage/buffer/buf_table.c index 23c14e6c688..9e0d945b603 100644 --- a/src/backend/storage/buffer/buf_table.c +++ b/src/backend/storage/buffer/buf_table.c @@ -216,11 +216,11 @@ BufTableScan(BufferTag *tagPtr, uint32 hashcode, int buf_id) retry: if(++attempts > 1000) goto die; prev = P_NEW; - id = pg_atomic_fetch_u32(head); + id = pg_atomic_read_u32(head); while(id != P_NEW) { /* this will get a fresh version of entry cache line */ - next = pg_atomic_fetch_u32(&entries[id].next); + next = pg_atomic_read_u32(&entries[id].next); if (BufferTagsEqual(&entries[id].tag, tagPtr)) return id; if(entries[id].bucket != bucket) @@ -229,8 +229,16 @@ retry: { /* step back for a while and try the same link again */ id = prev; - SPIN_DELAY(); - continue; + next = pg_atomic_fetch_u32(&entries[id].next); + if(next != id) + { + /* promising, the oriign link changed + * so maybe we are back on the right chain + * give it another chance */ + id = next; + continue; + } + goto retry; } else goto retry; diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 17f142e4c5b..7e2324c634f 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -701,7 +701,7 @@ PrefetchSharedBuffer(SMgrRelation smgr_reln, PrefetchBufferResult result = {InvalidBuffer, false}; BufferTag newTag; /* identity of requested block */ uint32 newHash; /* hash value for newTag */ - LWLock *newPartitionLock; /* buffer partition lock for it */ + /* LWLock *newPartitionLock; */ /* XXX: no lock used */ int buf_id; Assert(BlockNumberIsValid(blockNum)); @@ -712,12 +712,12 @@ PrefetchSharedBuffer(SMgrRelation smgr_reln, /* determine its hash code and partition lock ID */ newHash = BufTableHashCode(&newTag); - newPartitionLock = BufMappingPartitionLock(newHash); + /* newPartitionLock = BufMappingPartitionLock(newHash); */ /* see if the block is in the buffer pool already */ - LWLockAcquire(newPartitionLock, LW_SHARED); + /* LWLockAcquire(newPartitionLock, LW_SHARED); */ /* XXX: Lookup doesn't require a lock */ buf_id = BufTableLookup(&newTag, newHash); - LWLockRelease(newPartitionLock); + /* LWLockRelease(newPartitionLock); */ /* XXX: Lock not acquired */ /* If not in buffers, initiate prefetch */ if (buf_id < 0) @@ -2220,7 +2220,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, newPartitionLock = BufMappingPartitionLock(newHash); /* see if the block is in the buffer pool already */ - LWLockAcquire(newPartitionLock, LW_SHARED); + /* LWLockAcquire(newPartitionLock, LW_SHARED); */ /* XXX: Lookup doesn't require a lock */ existing_buf_id = BufTableLookup(&newTag, newHash); if (existing_buf_id >= 0) { @@ -2237,7 +2237,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, valid = PinBuffer(buf, strategy, false); /* Can release the mapping lock as soon as we've pinned it */ - LWLockRelease(newPartitionLock); + /* LWLockRelease(newPartitionLock); */ /* XXX: Lock not acquired */ *foundPtr = true; @@ -2258,7 +2258,7 @@ BufferAlloc(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, * Didn't find it in the buffer pool. We'll have to initialize a new * buffer. Remember to unlock the mapping lock while doing the work. */ - LWLockRelease(newPartitionLock); + /* LWLockRelease(newPartitionLock); */ /* XXX: Lock not acquired */ /* * Acquire a victim buffer. Somebody else might try to do the same, we @@ -5085,7 +5085,7 @@ FindAndDropRelationBuffers(RelFileLocator rlocator, ForkNumber forkNum, { uint32 bufHash; /* hash value for tag */ BufferTag bufTag; /* identity of requested block */ - LWLock *bufPartitionLock; /* buffer partition lock for it */ + /* LWLock *bufPartitionLock; */ /* XXX: no lock used */ int buf_id; BufferDesc *bufHdr; @@ -5094,12 +5094,12 @@ FindAndDropRelationBuffers(RelFileLocator rlocator, ForkNumber forkNum, /* determine its hash code and partition lock ID */ bufHash = BufTableHashCode(&bufTag); - bufPartitionLock = BufMappingPartitionLock(bufHash); + /* bufPartitionLock = BufMappingPartitionLock(bufHash); */ /* Check that it is in the buffer pool. If not, do nothing. */ - LWLockAcquire(bufPartitionLock, LW_SHARED); + /* LWLockAcquire(bufPartitionLock, LW_SHARED); */ /* XXX: Lookup doesn't require a lock */ buf_id = BufTableLookup(&bufTag, bufHash); - LWLockRelease(bufPartitionLock); + /* LWLockRelease(bufPartitionLock); */ /* XXX: Lock not acquired */ if (buf_id < 0) continue; -- 2.53.0