From dbf345ac5a857a11801d07291ecd71eb224050af Mon Sep 17 00:00:00 2001 From: Andreas Karlsson Date: Fri, 21 Nov 2025 10:15:06 +0100 Subject: [PATCH v1 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/backend/storage/buffer/bufmgr.c | 12 ++++++++--- src/include/storage/bufpage.h | 33 +++++++++++++++++++++-------- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 327ddb7adc8..34c4c4bd925 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -4491,13 +4491,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; @@ -4518,6 +4523,7 @@ BufferGetLSNAtomic(Buffer buffer) UnlockBufHdr(bufHdr); return lsn; +#endif } /* --------------------------------------------------------------------- diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h index abc2cf2a020..1b54967c70c 100644 --- a/src/include/storage/bufpage.h +++ b/src/include/storage/bufpage.h @@ -91,23 +91,38 @@ 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) { - return (uint64) val.xlogid << 32 | val.xrecoff; + return val; } #define PageXLogRecPtrSet(ptr, lsn) \ - ((ptr).xlogid = (uint32) ((lsn) >> 32), (ptr).xrecoff = (uint32) (lsn)) + ((ptr) = (lsn)) + +#else + +static inline XLogRecPtr +PageXLogRecPtrGet(volatile PageXLogRecPtr val) +{ + return (val << 32) | (val >> 32); +} + +#define PageXLogRecPtrSet(ptr, lsn) \ + ((ptr) = ((lsn) << 32) | ((lsn) >> 32)) + +#endif /* * disk page organization -- 2.47.3