bufmgr: Introduce FlushUnlockedBuffer

There were several copies of code locking a buffer, flushing its contents, and
unlocking the buffer. It seems worth centralizing that into a helper function.

Reviewed-by: Matthias van de Meent <boekewurm+postgres@gmail.com>
Discussion: https://postgr.es/m/fvfmkr5kk4nyex56ejgxj3uzi63isfxovp2biecb4bspbjrze7@az2pljabhnff

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 10232a333d823815e7c7fd641fca890df5c4d75c..fd462a25beb820c426c2f989b5539bf1d4eb1330 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -534,6 +534,8 @@ static inline BufferDesc *BufferAlloc(SMgrRelation smgr,
 static bool AsyncReadBuffers(ReadBuffersOperation *operation, int *nblocks_progress);
 static void CheckReadBuffersOperation(ReadBuffersOperation *operation, bool is_complete);
 static Buffer GetVictimBuffer(BufferAccessStrategy strategy, IOContext io_context);
+static void FlushUnlockedBuffer(BufferDesc *buf, SMgrRelation reln,
+                               IOObject io_object, IOContext io_context);
 static void FlushBuffer(BufferDesc *buf, SMgrRelation reln,
                        IOObject io_object, IOContext io_context);
 static void FindAndDropRelationBuffers(RelFileLocator rlocator,
@@ -3927,11 +3929,8 @@ SyncOneBuffer(int buf_id, bool skip_recently_used, WritebackContext *wb_context)
     * buffer is clean by the time we've locked it.)
     */
    PinBuffer_Locked(bufHdr);
-   LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
 
-   FlushBuffer(bufHdr, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
-
-   LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
+   FlushUnlockedBuffer(bufHdr, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
 
    tag = bufHdr->tag;
 
@@ -4378,6 +4377,19 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object,
    error_context_stack = errcallback.previous;
 }
 
+/*
+ * Convenience wrapper around FlushBuffer() that locks/unlocks the buffer
+ * before/after calling FlushBuffer().
+ */
+static void
+FlushUnlockedBuffer(BufferDesc *buf, SMgrRelation reln,
+                   IOObject io_object, IOContext io_context)
+{
+   LWLockAcquire(BufferDescriptorGetContentLock(buf), LW_SHARED);
+   FlushBuffer(buf, reln, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
+   LWLockRelease(BufferDescriptorGetContentLock(buf));
+}
+
 /*
  * RelationGetNumberOfBlocksInFork
  *     Determines the current number of pages in the specified relation fork.
@@ -4967,9 +4979,7 @@ FlushRelationBuffers(Relation rel)
            (buf_state & (BM_VALID | BM_DIRTY)) == (BM_VALID | BM_DIRTY))
        {
            PinBuffer_Locked(bufHdr);
-           LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-           FlushBuffer(bufHdr, srel, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
-           LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
+           FlushUnlockedBuffer(bufHdr, srel, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
            UnpinBuffer(bufHdr);
        }
        else
@@ -5064,9 +5074,7 @@ FlushRelationsAllBuffers(SMgrRelation *smgrs, int nrels)
            (buf_state & (BM_VALID | BM_DIRTY)) == (BM_VALID | BM_DIRTY))
        {
            PinBuffer_Locked(bufHdr);
-           LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-           FlushBuffer(bufHdr, srelent->srel, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
-           LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
+           FlushUnlockedBuffer(bufHdr, srelent->srel, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
            UnpinBuffer(bufHdr);
        }
        else
@@ -5292,9 +5300,7 @@ FlushDatabaseBuffers(Oid dbid)
            (buf_state & (BM_VALID | BM_DIRTY)) == (BM_VALID | BM_DIRTY))
        {
            PinBuffer_Locked(bufHdr);
-           LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
-           FlushBuffer(bufHdr, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
-           LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
+           FlushUnlockedBuffer(bufHdr, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
            UnpinBuffer(bufHdr);
        }
        else
@@ -6569,10 +6575,8 @@ EvictUnpinnedBufferInternal(BufferDesc *desc, bool *buffer_flushed)
    /* If it was dirty, try to clean it once. */
    if (buf_state & BM_DIRTY)
    {
-       LWLockAcquire(BufferDescriptorGetContentLock(desc), LW_SHARED);
-       FlushBuffer(desc, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
+       FlushUnlockedBuffer(desc, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
        *buffer_flushed = true;
-       LWLockRelease(BufferDescriptorGetContentLock(desc));
    }
 
    /* This will return false if it becomes dirty or someone else pins it. */