From 4b6df3e3944aea238b4bc329a9d595cda6da608d Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <andreas@proxel.se>
Date: Fri, 21 Nov 2025 10:15:06 +0100
Subject: [PATCH v3 1/2] Do not lock in BufferGetLSNAtomic() on archs with 8
 byte atomic reads

On platforms where we can read or write the whole LSN to a buffer page
we do not need to lock the buffer header to be sure we do not get a torn
LSN. To ahcieve this we need to make sure we always read and write the
while LSN and do not write it field by field.

While working on this I converted PageXLogRecPtr from a struct with two
uint32 fields into an uint64 to make the intent more clear.

Idea by Andres Freund.
---
 src/include/access/gist.h           |  4 +--
 src/include/storage/bufpage.h       | 48 +++++++++++++++++++++--------
 src/backend/access/common/bufmask.c |  2 +-
 src/backend/storage/buffer/bufmgr.c | 12 ++++++--
 4 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/src/include/access/gist.h b/src/include/access/gist.h
index 8fa30126f..9b385b13a 100644
--- a/src/include/access/gist.h
+++ b/src/include/access/gist.h
@@ -186,8 +186,8 @@ typedef struct GISTENTRY
 #define GistMarkFollowRight(page) ( GistPageGetOpaque(page)->flags |= F_FOLLOW_RIGHT)
 #define GistClearFollowRight(page)	( GistPageGetOpaque(page)->flags &= ~F_FOLLOW_RIGHT)
 
-#define GistPageGetNSN(page) ( PageXLogRecPtrGet(GistPageGetOpaque(page)->nsn))
-#define GistPageSetNSN(page, val) ( PageXLogRecPtrSet(GistPageGetOpaque(page)->nsn, val))
+#define GistPageGetNSN(page) ( PageXLogRecPtrGet(&GistPageGetOpaque(page)->nsn))
+#define GistPageSetNSN(page, val) ( PageXLogRecPtrSet(&GistPageGetOpaque(page)->nsn, val))
 
 
 /*
diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h
index ae3725b3b..2dbd9f19e 100644
--- a/src/include/storage/bufpage.h
+++ b/src/include/storage/bufpage.h
@@ -91,23 +91,45 @@ typedef uint16 LocationIndex;
 
 
 /*
- * For historical reasons, the 64-bit LSN value is stored as two 32-bit
- * values.
+ * For historical reasons, the storage of 64-bit LSN values depends on the
+ * endianess because the field used to be stored as two 32-bit values. When
+ * reading and writing the LSN we need to convert between the two formats.
+ *
+ * We are careful to try to treat the LSN as a single uint64 so callers like
+ * BufferGetLSNAtomic() can be sure there are no torn reads or writes.
  */
-typedef struct
-{
-	uint32		xlogid;			/* high bits */
-	uint32		xrecoff;		/* low bits */
-} PageXLogRecPtr;
+typedef uint64 PageXLogRecPtr;
+
+#ifdef WORDS_BIGENDIAN
 
 static inline XLogRecPtr
-PageXLogRecPtrGet(PageXLogRecPtr val)
+PageXLogRecPtrGet(const PageXLogRecPtr *val)
 {
-	return (uint64) val.xlogid << 32 | val.xrecoff;
+	return *val;
 }
 
-#define PageXLogRecPtrSet(ptr, lsn) \
-	((ptr).xlogid = (uint32) ((lsn) >> 32), (ptr).xrecoff = (uint32) (lsn))
+static inline void
+PageXLogRecPtrSet(PageXLogRecPtr *ptr, XLogRecPtr lsn)
+{
+	*ptr = lsn;
+}
+
+#else
+
+static inline XLogRecPtr
+PageXLogRecPtrGet(const volatile PageXLogRecPtr *val)
+{
+	PageXLogRecPtr tmp = *val;
+	return (tmp << 32) | (tmp >> 32);
+}
+
+static inline void
+PageXLogRecPtrSet(volatile PageXLogRecPtr *ptr, XLogRecPtr lsn)
+{
+	*ptr = (lsn << 32) | (lsn >> 32);
+}
+
+#endif
 
 /*
  * disk page organization
@@ -385,12 +407,12 @@ PageGetMaxOffsetNumber(const PageData *page)
 static inline XLogRecPtr
 PageGetLSN(const PageData *page)
 {
-	return PageXLogRecPtrGet(((const PageHeaderData *) page)->pd_lsn);
+	return PageXLogRecPtrGet(&((const PageHeaderData *) page)->pd_lsn);
 }
 static inline void
 PageSetLSN(Page page, XLogRecPtr lsn)
 {
-	PageXLogRecPtrSet(((PageHeader) page)->pd_lsn, lsn);
+	PageXLogRecPtrSet(&((PageHeader) page)->pd_lsn, lsn);
 }
 
 static inline bool
diff --git a/src/backend/access/common/bufmask.c b/src/backend/access/common/bufmask.c
index 1a9e7bea5..8a67bfa1a 100644
--- a/src/backend/access/common/bufmask.c
+++ b/src/backend/access/common/bufmask.c
@@ -32,7 +32,7 @@ mask_page_lsn_and_checksum(Page page)
 {
 	PageHeader	phdr = (PageHeader) page;
 
-	PageXLogRecPtrSet(phdr->pd_lsn, (uint64) MASK_MARKER);
+	PageXLogRecPtrSet(&phdr->pd_lsn, (uint64) MASK_MARKER);
 	phdr->pd_checksum = MASK_MARKER;
 }
 
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 7241477ca..290239484 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -4626,13 +4626,18 @@ BufferIsPermanent(Buffer buffer)
 
 /*
  * BufferGetLSNAtomic
- *		Retrieves the LSN of the buffer atomically using a buffer header lock.
- *		This is necessary for some callers who may not have an exclusive lock
- *		on the buffer.
+ *		Retrieves the LSN of the buffer atomically.
+ *
+ *		On platforms without 8 byte atomic reads/write we need to take a
+ *		header lock. This is necessary for some callers who may not have an
+ *		exclusive lock on the buffer.
  */
 XLogRecPtr
 BufferGetLSNAtomic(Buffer buffer)
 {
+#ifdef PG_HAVE_8BYTE_SINGLE_COPY_ATOMICITY
+	return PageGetLSN(BufferGetPage(buffer));
+#else
 	char	   *page = BufferGetPage(buffer);
 	BufferDesc *bufHdr;
 	XLogRecPtr	lsn;
@@ -4653,6 +4658,7 @@ BufferGetLSNAtomic(Buffer buffer)
 	UnlockBufHdr(bufHdr);
 
 	return lsn;
+#endif
 }
 
 /* ---------------------------------------------------------------------
-- 
2.51.0

