From 7fb80c58553812d4485094bbf78e10d8b3dc2c1b Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Mon, 2 Feb 2026 13:24:04 -0500 Subject: [PATCH v12 3/6] bufmgr: Remove the, now obsolete, BM_JUST_DIRTIED Due to the recent changes to use a share-exclusive mode for setting hint bits and for flushing pages, instead of using share mode as before, a buffer cannot be dirtied while the flush is ongoing. The reason we needed JUST_DIRTIED was to handle the case where the buffer was dirtied while IO was ongoing - which is not possible anymore. Author: Reviewed-by: Discussion: https://postgr.es/m/ Backpatch: --- src/include/storage/buf_internals.h | 3 +-- src/backend/storage/buffer/bufmgr.c | 30 ++++++++------------------- src/backend/storage/buffer/localbuf.c | 2 +- 3 files changed, 11 insertions(+), 24 deletions(-) diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h index 27f12502d19..8d1e16b5d51 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -114,8 +114,7 @@ StaticAssertDecl(BUF_REFCOUNT_BITS + BUF_USAGECOUNT_BITS + BUF_FLAG_BITS + BUF_L #define BM_IO_IN_PROGRESS BUF_DEFINE_FLAG( 4) /* previous I/O failed */ #define BM_IO_ERROR BUF_DEFINE_FLAG( 5) -/* dirtied since write started */ -#define BM_JUST_DIRTIED BUF_DEFINE_FLAG( 6) +/* flag bit 6 is not used anymore */ /* have waiter for sole pin */ #define BM_PIN_COUNT_WAITER BUF_DEFINE_FLAG( 7) /* must write for checkpoint */ diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 3b32b4e0ab1..e462fc799fe 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -2886,7 +2886,7 @@ ExtendBufferedRelShared(BufferManagerRelation bmr, buf_state = LockBufHdr(victim_buf_hdr); /* some sanity checks while we hold the buffer header lock */ - Assert(!(buf_state & (BM_VALID | BM_TAG_VALID | BM_DIRTY | BM_JUST_DIRTIED))); + Assert(!(buf_state & (BM_VALID | BM_TAG_VALID | BM_DIRTY))); Assert(BUF_STATE_GET_REFCOUNT(buf_state) == 1); victim_buf_hdr->tag = tag; @@ -3089,7 +3089,7 @@ MarkBufferDirty(Buffer buffer) buf_state = old_buf_state; Assert(BUF_STATE_GET_REFCOUNT(buf_state) > 0); - buf_state |= BM_DIRTY | BM_JUST_DIRTIED; + buf_state |= BM_DIRTY; if (pg_atomic_compare_exchange_u64(&bufHdr->state, &old_buf_state, buf_state)) @@ -4421,7 +4421,6 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, instr_time io_start; Block bufBlock; char *bufToWrite; - uint64 buf_state; Assert(BufferLockHeldByMeInMode(buf, BUFFER_LOCK_EXCLUSIVE) || BufferLockHeldByMeInMode(buf, BUFFER_LOCK_SHARE_EXCLUSIVE)); @@ -4450,19 +4449,12 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, reln->smgr_rlocator.locator.dbOid, reln->smgr_rlocator.locator.relNumber); - buf_state = LockBufHdr(buf); - /* * As we hold at least a share-exclusive lock on the buffer, the LSN * cannot change during the flush (and thus can't be torn). */ recptr = BufferGetLSN(buf); - /* To check if block content changes while flushing. - vadim 01/17/97 */ - UnlockBufHdrExt(buf, buf_state, - 0, BM_JUST_DIRTIED, - 0); - /* * Force XLOG flush up to buffer's LSN. This implements the basic WAL * rule that log updates must hit disk before any of the data-file changes @@ -4480,7 +4472,7 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, * disastrous system-wide consequences. To make sure that can't happen, * skip the flush if the buffer isn't permanent. */ - if (buf_state & BM_PERMANENT) + if (pg_atomic_read_u64(&buf->state) & BM_PERMANENT) XLogFlush(recptr); /* @@ -4533,8 +4525,7 @@ FlushBuffer(BufferDesc *buf, SMgrRelation reln, IOObject io_object, pgBufferUsage.shared_blks_written++; /* - * Mark the buffer as clean (unless BM_JUST_DIRTIED has become set) and - * end the BM_IO_IN_PROGRESS state. + * Mark the buffer as clean and end the BM_IO_IN_PROGRESS state. */ TerminateBufferIO(buf, true, 0, true, false); @@ -5580,8 +5571,7 @@ MarkSharedBufferDirtyHint(Buffer buffer, BufferDesc *bufHdr, uint64 lockstate, * cleaned or dirtied the page concurrently, so we can just rely on the * previously fetched value here without any danger of races. */ - if (unlikely((lockstate & (BM_DIRTY | BM_JUST_DIRTIED)) != - (BM_DIRTY | BM_JUST_DIRTIED))) + if (unlikely(!(lockstate & BM_DIRTY))) { XLogRecPtr lsn = InvalidXLogRecPtr; bool delayChkptFlags = false; @@ -5667,7 +5657,7 @@ MarkSharedBufferDirtyHint(Buffer buffer, BufferDesc *bufHdr, uint64 lockstate, } UnlockBufHdrExt(bufHdr, buf_state, - BM_DIRTY | BM_JUST_DIRTIED, + BM_DIRTY, 0, 0); if (delayChkptFlags) @@ -7131,10 +7121,8 @@ StartBufferIO(BufferDesc *buf, bool forInput, bool nowait) * BM_IO_IN_PROGRESS bit is set for the buffer * The buffer is Pinned * - * If clear_dirty is true and BM_JUST_DIRTIED is not set, we clear the - * buffer's BM_DIRTY flag. This is appropriate when terminating a - * successful write. The check on BM_JUST_DIRTIED is necessary to avoid - * marking the buffer clean if it was re-dirtied while we were writing. + * If clear_dirty is true, we clear the buffer's BM_DIRTY flag. This is + * appropriate when terminating a successful write. * * set_flag_bits gets ORed into the buffer's flags. It must include * BM_IO_ERROR in a failure case. For successful completion it could @@ -7160,7 +7148,7 @@ TerminateBufferIO(BufferDesc *buf, bool clear_dirty, uint64 set_flag_bits, /* Clear earlier errors, if this IO failed, it'll be marked again */ unset_flag_bits |= BM_IO_ERROR; - if (clear_dirty && !(buf_state & BM_JUST_DIRTIED)) + if (clear_dirty) unset_flag_bits |= BM_DIRTY | BM_CHECKPOINT_NEEDED; if (release_aio) diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c index 04a540379a2..404c6bccbdd 100644 --- a/src/backend/storage/buffer/localbuf.c +++ b/src/backend/storage/buffer/localbuf.c @@ -441,7 +441,7 @@ ExtendBufferedRelLocal(BufferManagerRelation bmr, { uint64 buf_state = pg_atomic_read_u64(&victim_buf_hdr->state); - Assert(!(buf_state & (BM_VALID | BM_TAG_VALID | BM_DIRTY | BM_JUST_DIRTIED))); + Assert(!(buf_state & (BM_VALID | BM_TAG_VALID | BM_DIRTY))); victim_buf_hdr->tag = tag; -- 2.48.1.76.g4e746b1a31.dirty