From 819b69a81d307e2ad8e9752cf062ebb57813960d Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 5 Apr 2023 10:42:17 -0700 Subject: [PATCH] bufmgr: Add some more error checking [infrastructure] around pinning This adds a few more assertions against a buffer being local in places we don't expect, and extracts the check for a buffer being pinned exactly once from LockBufferForCleanup() into its own function. Later commits will use this function. Reviewed-by: Heikki Linnakangas Reviewed-by: Melanie Plageman Discussion: http://postgr.es/m/419312fd-9255-078c-c3e3-f0525f911d7f@iki.fi --- src/backend/storage/buffer/bufmgr.c | 46 +++++++++++++++++++++-------- src/include/storage/bufmgr.h | 1 + 2 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 7f119cd4b0..2362423b89 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -1735,6 +1735,8 @@ PinBuffer(BufferDesc *buf, BufferAccessStrategy strategy) bool result; PrivateRefCountEntry *ref; + Assert(!BufferIsLocal(b)); + ref = GetPrivateRefCountEntry(b, true); if (ref == NULL) @@ -1880,6 +1882,8 @@ UnpinBuffer(BufferDesc *buf) PrivateRefCountEntry *ref; Buffer b = BufferDescriptorGetBuffer(buf); + Assert(!BufferIsLocal(b)); + /* not moving as we're likely deleting it soon anyway */ ref = GetPrivateRefCountEntry(b, false); Assert(ref != NULL); @@ -4253,6 +4257,29 @@ ConditionalLockBuffer(Buffer buffer) LW_EXCLUSIVE); } +/* + * Verify that this backend is pinning the buffer exactly once. + * + * NOTE: Like in BufferIsPinned(), what we check here is that *this* backend + * holds a pin on the buffer. We do not care whether some other backend does. + */ +void +CheckBufferIsPinnedOnce(Buffer buffer) +{ + if (BufferIsLocal(buffer)) + { + if (LocalRefCount[-buffer - 1] != 1) + elog(ERROR, "incorrect local pin count: %d", + LocalRefCount[-buffer - 1]); + } + else + { + if (GetPrivateRefCount(buffer) != 1) + elog(ERROR, "incorrect local pin count: %d", + GetPrivateRefCount(buffer)); + } +} + /* * LockBufferForCleanup - lock a buffer in preparation for deleting items * @@ -4280,20 +4307,11 @@ LockBufferForCleanup(Buffer buffer) Assert(BufferIsPinned(buffer)); Assert(PinCountWaitBuf == NULL); - if (BufferIsLocal(buffer)) - { - /* There should be exactly one pin */ - if (LocalRefCount[-buffer - 1] != 1) - elog(ERROR, "incorrect local pin count: %d", - LocalRefCount[-buffer - 1]); - /* Nobody else to wait for */ - return; - } + CheckBufferIsPinnedOnce(buffer); - /* There should be exactly one local pin */ - if (GetPrivateRefCount(buffer) != 1) - elog(ERROR, "incorrect local pin count: %d", - GetPrivateRefCount(buffer)); + /* Nobody else to wait for */ + if (BufferIsLocal(buffer)) + return; bufHdr = GetBufferDescriptor(buffer - 1); @@ -4794,6 +4812,8 @@ LockBufHdr(BufferDesc *desc) SpinDelayStatus delayStatus; uint32 old_buf_state; + Assert(!BufferIsLocal(BufferDescriptorGetBuffer(desc))); + init_local_spin_delay(&delayStatus); while (true) diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h index 73762cb1ec..f96dc08021 100644 --- a/src/include/storage/bufmgr.h +++ b/src/include/storage/bufmgr.h @@ -134,6 +134,7 @@ extern void ReleaseBuffer(Buffer buffer); extern void UnlockReleaseBuffer(Buffer buffer); extern void MarkBufferDirty(Buffer buffer); extern void IncrBufferRefCount(Buffer buffer); +extern void CheckBufferIsPinnedOnce(Buffer buffer); extern Buffer ReleaseAndReadBuffer(Buffer buffer, Relation relation, BlockNumber blockNum);