Use atomic access for SlruShared->latest_page_number

The new concurrency model proposed for slru.c to improve performance
does not include any single lock that would coordinate processes
doing concurrent reads/writes on SlruShared->latest_page_number.
We can instead use atomic reads and writes for that variable.

Author: Dilip Kumar <dilipbalaut@gmail.com>
Reviewed-by: Andrey M. Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/CAFiTN-vzDvNz=ExGXz6gdyjtzGixKSqs0mKHMmaQ8sOSEFZ33A@mail.gmail.com

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index f6e7da7ffc9563185eaf28d4711ff832d3ca30ec..06fc2989bab189bc4227a90e4c29d0852ed15ab8 100644 (file)
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -766,14 +766,10 @@ StartupCLOG(void)
        TransactionId xid = XidFromFullTransactionId(TransamVariables->nextXid);
        int64           pageno = TransactionIdToPage(xid);
 
-       LWLockAcquire(XactSLRULock, LW_EXCLUSIVE);
-
        /*
         * Initialize our idea of the latest page number.
         */
-       XactCtl->shared->latest_page_number = pageno;
-
-       LWLockRelease(XactSLRULock);
+       pg_atomic_write_u64(&XactCtl->shared->latest_page_number, pageno);
 }
 
 /*

diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 61b82385f359426df200c4db7bb32e8c62b80742..6bfe60343e636018399856555fafd3653c5d6549 100644 (file)
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -689,9 +689,7 @@ ActivateCommitTs(void)
        /*
         * Re-Initialize our idea of the latest page number.
         */
-       LWLockAcquire(CommitTsSLRULock, LW_EXCLUSIVE);
-       CommitTsCtl->shared->latest_page_number = pageno;
-       LWLockRelease(CommitTsSLRULock);
+       pg_atomic_write_u64(&CommitTsCtl->shared->latest_page_number, pageno);
 
        /*
         * If CommitTs is enabled, but it wasn't in the previous server run, we
@@ -1006,7 +1004,8 @@ commit_ts_redo(XLogReaderState *record)
                 * During XLOG replay, latest_page_number isn't set up yet; insert a
                 * suitable value to bypass the sanity test in SimpleLruTruncate.
                 */
-               CommitTsCtl->shared->latest_page_number = trunc->pageno;
+               pg_atomic_write_u64(&CommitTsCtl->shared->latest_page_number,
+                                                       trunc->pageno);
 
                SimpleLruTruncate(CommitTsCtl, trunc->pageno);
        }

diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 59523be90132856c1a895cfadb62877cd153d4da..febc429f72415242594e6917e3a3746498a037d2 100644 (file)
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -2017,13 +2017,15 @@ StartupMultiXact(void)
         * Initialize offset's idea of the latest page number.
         */
        pageno = MultiXactIdToOffsetPage(multi);
-       MultiXactOffsetCtl->shared->latest_page_number = pageno;
+       pg_atomic_write_u64(&MultiXactOffsetCtl->shared->latest_page_number,
+                                               pageno);
 
        /*
         * Initialize member's idea of the latest page number.
         */
        pageno = MXOffsetToMemberPage(offset);
-       MultiXactMemberCtl->shared->latest_page_number = pageno;
+       pg_atomic_write_u64(&MultiXactMemberCtl->shared->latest_page_number,
+                                               pageno);
 }
 
 /*
@@ -2047,14 +2049,15 @@ TrimMultiXact(void)
        oldestMXactDB = MultiXactState->oldestMultiXactDB;
        LWLockRelease(MultiXactGenLock);
 
-       /* Clean up offsets state */
-       LWLockAcquire(MultiXactOffsetSLRULock, LW_EXCLUSIVE);
-
        /*
         * (Re-)Initialize our idea of the latest page number for offsets.
         */
        pageno = MultiXactIdToOffsetPage(nextMXact);
-       MultiXactOffsetCtl->shared->latest_page_number = pageno;
+       pg_atomic_write_u64(&MultiXactOffsetCtl->shared->latest_page_number,
+                                               pageno);
+
+       /* Clean up offsets state */
+       LWLockAcquire(MultiXactOffsetSLRULock, LW_EXCLUSIVE);
 
        /*
         * Zero out the remainder of the current offsets page.  See notes in
@@ -2081,14 +2084,16 @@ TrimMultiXact(void)
 
        LWLockRelease(MultiXactOffsetSLRULock);
 
-       /* And the same for members */
-       LWLockAcquire(MultiXactMemberSLRULock, LW_EXCLUSIVE);
-
        /*
+        * And the same for members.
+        *
         * (Re-)Initialize our idea of the latest page number for members.
         */
        pageno = MXOffsetToMemberPage(offset);
-       MultiXactMemberCtl->shared->latest_page_number = pageno;
+       pg_atomic_write_u64(&MultiXactMemberCtl->shared->latest_page_number,
+                                               pageno);
+
+       LWLockAcquire(MultiXactMemberSLRULock, LW_EXCLUSIVE);
 
        /*
         * Zero out the remainder of the current members page.  See notes in
@@ -3333,7 +3338,8 @@ multixact_redo(XLogReaderState *record)
                 * SimpleLruTruncate.
                 */
                pageno = MultiXactIdToOffsetPage(xlrec.endTruncOff);
-               MultiXactOffsetCtl->shared->latest_page_number = pageno;
+               pg_atomic_write_u64(&MultiXactOffsetCtl->shared->latest_page_number,
+                                                       pageno);
                PerformOffsetsTruncation(xlrec.startTruncOff, xlrec.endTruncOff);
 
                LWLockRelease(MultiXactTruncationLock);

diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 9ac4790f1647c4a94272feb0a17538b5526e82eb..556d1c15867300d06cfbd52431370464dfbd0ccf 100644 (file)
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -17,7 +17,8 @@
  * per-buffer LWLocks that synchronize I/O for each buffer.  The control lock
  * must be held to examine or modify any shared state.  A process that is
  * reading in or writing out a page buffer does not hold the control lock,
- * only the per-buffer lock for the buffer it is working on.
+ * only the per-buffer lock for the buffer it is working on.  One exception
+ * is latest_page_number, which is read and written using atomic ops.
  *
  * "Holding the control lock" means exclusive lock in all cases except for
  * SimpleLruReadPage_ReadOnly(); see comments for SlruRecentlyUsed() for
@@ -239,8 +240,7 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
                shared->lsn_groups_per_page = nlsns;
 
                shared->cur_lru_count = 0;
-
-               /* shared->latest_page_number will be set later */
+               pg_atomic_write_u64(&shared->latest_page_number, 0);
 
                shared->slru_stats_idx = pgstat_get_slru_index(name);
 
@@ -329,8 +329,15 @@ SimpleLruZeroPage(SlruCtl ctl, int64 pageno)
        /* Set the LSNs for this new page to zero */
        SimpleLruZeroLSNs(ctl, slotno);
 
-       /* Assume this page is now the latest active page */
-       shared->latest_page_number = pageno;
+       /*
+        * Assume this page is now the latest active page.
+        *
+        * Note that because both this routine and SlruSelectLRUPage run with
+        * ControlLock held, it is not possible for this to be zeroing a page that
+        * SlruSelectLRUPage is going to evict simultaneously.  Therefore, there's
+        * no memory barrier here.
+        */
+       pg_atomic_write_u64(&shared->latest_page_number, pageno);
 
        /* update the stats counter of zeroed pages */
        pgstat_count_slru_page_zeroed(shared->slru_stats_idx);
@@ -1113,9 +1120,17 @@ SlruSelectLRUPage(SlruCtl ctl, int64 pageno)
                                shared->page_lru_count[slotno] = cur_count;
                                this_delta = 0;
                        }
+
+                       /*
+                        * If this page is the one most recently zeroed, don't consider it
+                        * an eviction candidate. See comments in SimpleLruZeroPage for an
+                        * explanation about the lack of a memory barrier here.
+                        */
                        this_page_number = shared->page_number[slotno];
-                       if (this_page_number == shared->latest_page_number)
+                       if (this_page_number ==
+                               pg_atomic_read_u64(&shared->latest_page_number))
                                continue;
+
                        if (shared->page_status[slotno] == SLRU_PAGE_VALID)
                        {
                                if (this_delta > best_valid_delta ||
@@ -1254,7 +1269,6 @@ void
 SimpleLruTruncate(SlruCtl ctl, int64 cutoffPage)
 {
        SlruShared      shared = ctl->shared;
-       int                     slotno;
 
        /* update the stats counter of truncates */
        pgstat_count_slru_truncate(shared->slru_stats_idx);
@@ -1270,10 +1284,13 @@ SimpleLruTruncate(SlruCtl ctl, int64 cutoffPage)
 restart:
 
        /*
-        * While we are holding the lock, make an important safety check: the
-        * current endpoint page must not be eligible for removal.
+        * An important safety check: the current endpoint page must not be
+        * eligible for removal.  This check is just a backstop against wraparound
+        * bugs elsewhere in SLRU handling, so we don't care if we read a slightly
+        * outdated value; therefore we don't add a memory barrier.
         */
-       if (ctl->PagePrecedes(shared->latest_page_number, cutoffPage))
+       if (ctl->PagePrecedes(pg_atomic_read_u64(&shared->latest_page_number),
+                                                 cutoffPage))
        {
                LWLockRelease(shared->ControlLock);
                ereport(LOG,
@@ -1282,7 +1299,7 @@ restart:
                return;
        }
 
-       for (slotno = 0; slotno < shared->num_slots; slotno++)
+       for (int slotno = 0; slotno < shared->num_slots; slotno++)
        {
                if (shared->page_status[slotno] == SLRU_PAGE_EMPTY)
                        continue;

diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index b05f6bc71d912c500c222b4e67c8d27c62febd36..210948865471d421f7f8adbf8ccb3a7302c631f0 100644 (file)
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -49,6 +49,9 @@ typedef enum
 
 /*
  * Shared-memory state
+ *
+ * ControlLock is used to protect access to the other fields, except
+ * latest_page_number, which uses atomics; see comment in slru.c.
  */
 typedef struct SlruSharedData
 {
@@ -95,7 +98,7 @@ typedef struct SlruSharedData
         * this is not critical data, since we use it only to avoid swapping out
         * the latest page.
         */
-       int64           latest_page_number;
+       pg_atomic_uint64 latest_page_number;
 
        /* SLRU's index for statistics purposes (might not be unique) */
        int                     slru_stats_idx;