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.

Discussion: https://postgr.es/m/5ubipyssiju5twkb7zgqwdr7q2vhpkpmuelxfpanetlk6ofnop@hvxb4g2amb2d

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 8084858726fddad8ce887c093bd6a7040aa70205..65ba4c5d8d4bbb8bc3f76724806e0f828e6d1fa4 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -2882,7 +2882,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;
@@ -3085,7 +3085,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))
@@ -4417,7 +4417,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));
@@ -4446,19 +4445,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
@@ -4476,7 +4468,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);
 
        /*
@@ -4529,8 +4521,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);
 
@@ -5587,8 +5578,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;
@@ -5674,7 +5664,7 @@ MarkSharedBufferDirtyHint(Buffer buffer, BufferDesc *bufHdr, uint64 lockstate,
                }
 
                UnlockBufHdrExt(bufHdr, buf_state,
-                                               BM_DIRTY | BM_JUST_DIRTIED,
+                                               BM_DIRTY,
                                                0, 0);
 
                if (delayChkptFlags)
@@ -7143,10 +7133,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
@@ -7172,7 +7160,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 04a540379a22f15758a6ca4fdec85faa1ea0c7e1..404c6bccbdd7e3742120876cde644ef576ebc88b 100644 (file)
--- 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;
 

diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h
index 27f12502d1965bb44356bf9669341c1580785133..8d1e16b5d519133ff1a6371002aea6b38581c2a4 100644 (file)
--- 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 */