From 0762a151b0e018944694ccac07e521adcdf7a06f Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Sat, 15 Mar 2025 12:30:07 -0400 Subject: [PATCH] localbuf: Introduce InvalidateLocalBuffer() Previously, there were three copies of this code, two of them identical. There's no good reason for that. This change is nice on its own, but the main motivation is the AIO patchset, which needs to add extra checks the deduplicated code, which of course is easier if there is only one version. Reviewed-by: Melanie Plageman Discussion: https://postgr.es/m/CAAKRu_b9anbWzEs5AAF9WCvcEVmgz-1AkHSQ-CLLy-p7WHzvFw@mail.gmail.com --- src/backend/storage/buffer/localbuf.c | 92 +++++++++++++-------------- 1 file changed, 44 insertions(+), 48 deletions(-) diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c index 456a2232f22..5331091132d 100644 --- a/src/backend/storage/buffer/localbuf.c +++ b/src/backend/storage/buffer/localbuf.c @@ -56,6 +56,7 @@ static int NLocalPinnedBuffers = 0; static void InitLocalBuffers(void); static Block GetLocalBufferStorage(void); static Buffer GetLocalVictimBuffer(void); +static void InvalidateLocalBuffer(BufferDesc *bufHdr, bool check_unreferenced); /* @@ -269,17 +270,7 @@ GetLocalVictimBuffer(void) */ if (pg_atomic_read_u32(&bufHdr->state) & BM_TAG_VALID) { - uint32 buf_state = pg_atomic_read_u32(&bufHdr->state); - LocalBufferLookupEnt *hresult; - - hresult = (LocalBufferLookupEnt *) - hash_search(LocalBufHash, &bufHdr->tag, HASH_REMOVE, NULL); - if (!hresult) /* shouldn't happen */ - elog(ERROR, "local buffer hash table corrupted"); - /* mark buffer invalid just in case hash insert fails */ - ClearBufferTag(&bufHdr->tag); - buf_state &= ~(BUF_FLAG_MASK | BUF_USAGECOUNT_MASK); - pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); + InvalidateLocalBuffer(bufHdr, false); pgstat_count_io_op(IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL, IOOP_EVICT, 1, 0); } @@ -492,6 +483,46 @@ MarkLocalBufferDirty(Buffer buffer) pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); } +/* + * InvalidateLocalBuffer -- mark a local buffer invalid. + * + * If check_unreferenced is true, error out if the buffer is still + * pinned. Passing false is appropriate when calling InvalidateLocalBuffer() + * as part of changing the identity of a buffer, instead of just dropping the + * buffer. + * + * See also InvalidateBuffer(). + */ +static void +InvalidateLocalBuffer(BufferDesc *bufHdr, bool check_unreferenced) +{ + Buffer buffer = BufferDescriptorGetBuffer(bufHdr); + int bufid = -buffer - 1; + uint32 buf_state; + LocalBufferLookupEnt *hresult; + + buf_state = pg_atomic_read_u32(&bufHdr->state); + + if (check_unreferenced && LocalRefCount[bufid] != 0) + elog(ERROR, "block %u of %s is still referenced (local %u)", + bufHdr->tag.blockNum, + relpathbackend(BufTagGetRelFileLocator(&bufHdr->tag), + MyProcNumber, + BufTagGetForkNum(&bufHdr->tag)).str, + LocalRefCount[bufid]); + + /* Remove entry from hashtable */ + hresult = (LocalBufferLookupEnt *) + hash_search(LocalBufHash, &bufHdr->tag, HASH_REMOVE, NULL); + if (!hresult) /* shouldn't happen */ + elog(ERROR, "local buffer hash table corrupted"); + /* Mark buffer invalid */ + ClearBufferTag(&bufHdr->tag); + buf_state &= ~BUF_FLAG_MASK; + buf_state &= ~BUF_USAGECOUNT_MASK; + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); +} + /* * DropRelationLocalBuffers * This function removes from the buffer pool all the pages of the @@ -512,7 +543,6 @@ DropRelationLocalBuffers(RelFileLocator rlocator, ForkNumber forkNum, for (i = 0; i < NLocBuffer; i++) { BufferDesc *bufHdr = GetLocalBufferDescriptor(i); - LocalBufferLookupEnt *hresult; uint32 buf_state; buf_state = pg_atomic_read_u32(&bufHdr->state); @@ -522,24 +552,7 @@ DropRelationLocalBuffers(RelFileLocator rlocator, ForkNumber forkNum, BufTagGetForkNum(&bufHdr->tag) == forkNum && bufHdr->tag.blockNum >= firstDelBlock) { - if (LocalRefCount[i] != 0) - elog(ERROR, "block %u of %s is still referenced (local %u)", - bufHdr->tag.blockNum, - relpathbackend(BufTagGetRelFileLocator(&bufHdr->tag), - MyProcNumber, - BufTagGetForkNum(&bufHdr->tag)).str, - LocalRefCount[i]); - - /* Remove entry from hashtable */ - hresult = (LocalBufferLookupEnt *) - hash_search(LocalBufHash, &bufHdr->tag, HASH_REMOVE, NULL); - if (!hresult) /* shouldn't happen */ - elog(ERROR, "local buffer hash table corrupted"); - /* Mark buffer invalid */ - ClearBufferTag(&bufHdr->tag); - buf_state &= ~BUF_FLAG_MASK; - buf_state &= ~BUF_USAGECOUNT_MASK; - pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); + InvalidateLocalBuffer(bufHdr, true); } } } @@ -559,7 +572,6 @@ DropRelationAllLocalBuffers(RelFileLocator rlocator) for (i = 0; i < NLocBuffer; i++) { BufferDesc *bufHdr = GetLocalBufferDescriptor(i); - LocalBufferLookupEnt *hresult; uint32 buf_state; buf_state = pg_atomic_read_u32(&bufHdr->state); @@ -567,23 +579,7 @@ DropRelationAllLocalBuffers(RelFileLocator rlocator) if ((buf_state & BM_TAG_VALID) && BufTagMatchesRelFileLocator(&bufHdr->tag, &rlocator)) { - if (LocalRefCount[i] != 0) - elog(ERROR, "block %u of %s is still referenced (local %u)", - bufHdr->tag.blockNum, - relpathbackend(BufTagGetRelFileLocator(&bufHdr->tag), - MyProcNumber, - BufTagGetForkNum(&bufHdr->tag)).str, - LocalRefCount[i]); - /* Remove entry from hashtable */ - hresult = (LocalBufferLookupEnt *) - hash_search(LocalBufHash, &bufHdr->tag, HASH_REMOVE, NULL); - if (!hresult) /* shouldn't happen */ - elog(ERROR, "local buffer hash table corrupted"); - /* Mark buffer invalid */ - ClearBufferTag(&bufHdr->tag); - buf_state &= ~BUF_FLAG_MASK; - buf_state &= ~BUF_USAGECOUNT_MASK; - pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); + InvalidateLocalBuffer(bufHdr, true); } } }