From 443221e84b1360b0b20159c84e2f5bf5c787d6a8 Mon Sep 17 00:00:00 2001
From: Asim R P <apraveen@pivotal.io>
Date: Fri, 3 Nov 2017 16:48:32 -0700
Subject: [PATCH 2/2] PageGetLSN: assert that locks are properly held

To help developers find issues, assert that the proper locks are held
for shared pages inside PageGetLSN. The implementation for this check is
added in a helper function, AssertPageIsLockedForLSN, which is called
from PageGetLSN whenever backend code is built with assertions enabled.
This introduces a slight source incompatibility: previously, both Pages
and PageHeaders could be passed to the macro, but now callers must
explicitly pass a Page.
---
 contrib/pageinspect/rawpage.c       |  2 +-
 src/backend/storage/buffer/bufmgr.c | 35 +++++++++++++++++++++++++++++++++++
 src/include/storage/bufpage.h       | 17 +++++++++++++++--
 3 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/contrib/pageinspect/rawpage.c b/contrib/pageinspect/rawpage.c
index 25af22f453..e3a1ca0e3b 100644
--- a/contrib/pageinspect/rawpage.c
+++ b/contrib/pageinspect/rawpage.c
@@ -250,7 +250,7 @@ page_header(PG_FUNCTION_ARGS)
 
 	/* Extract information from the page header */
 
-	lsn = PageGetLSN(page);
+	lsn = PageGetLSN((Page) page);
 
 	/* pageinspect >= 1.2 uses pg_lsn instead of text for the LSN field. */
 	if (TupleDescAttr(tupdesc, 0)->atttypid == TEXTOID)
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 15795b0c5a..8fbb8e79b3 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -4355,3 +4355,38 @@ TestForOldSnapshot_impl(Snapshot snapshot, Relation relation)
 				(errcode(ERRCODE_SNAPSHOT_TOO_OLD),
 				 errmsg("snapshot too old")));
 }
+
+#if defined(USE_ASSERT_CHECKING) && !defined(FRONTEND)
+void
+AssertPageIsLockedForLSN(Page page)
+{
+	char *pagePtr = page;
+
+	/*
+	 * We only want to assert that we hold a lock on the page contents if the
+	 * page is shared (i.e. it is one of the BufferBlocks).
+	 */
+	if (BufferBlocks <= pagePtr
+		&& pagePtr < (BufferBlocks + NBuffers * BLCKSZ))
+	{
+		ptrdiff_t bufId = (pagePtr - BufferBlocks) / BLCKSZ;
+		BufferDesc *buf = GetBufferDescriptor(bufId);
+		LWLock *content_lock = BufferDescriptorGetContentLock(buf);
+		uint32 buf_state = pg_atomic_read_u32(&buf->state);
+
+		/*
+		 * Either:
+		 * 1) we hold the exclusive lock for the buffer contents, so reading is
+		 *    always safe, or
+		 * 2) we hold the shared lock and the header spinlock, in which case we
+		 *    are protected against other exclusive lock holders and other WAL
+		 *    log hint writers, or
+		 * 3) XLog hint bits aren't enabled (so there are no WAL log hint
+		 *    writers) and we just need the shared lock by itself.
+		 */
+		Assert(LWLockHeldByMeInMode(content_lock, LW_EXCLUSIVE)
+			   || (LWLockHeldByMeInMode(content_lock, LW_SHARED)
+				   && (!XLogHintBitIsNeeded() || (buf_state & BM_LOCKED))));
+	}
+}
+#endif
diff --git a/src/include/storage/bufpage.h b/src/include/storage/bufpage.h
index 50c72a3c8d..db94310586 100644
--- a/src/include/storage/bufpage.h
+++ b/src/include/storage/bufpage.h
@@ -359,8 +359,21 @@ PageValidateSpecialPointer(Page page)
  * Additional macros for access to page headers. (Beware multiple evaluation
  * of the arguments!)
  */
-#define PageGetLSN(page) \
-	PageXLogRecPtrGet(((PageHeader) (page))->pd_lsn)
+
+#if defined(USE_ASSERT_CHECKING) && !defined(FRONTEND)
+/* in bufmgr.c; used in PageGetLSN */
+extern void AssertPageIsLockedForLSN(Page page);
+#else
+#define AssertPageIsLockedForLSN(page)
+#endif
+
+static inline XLogRecPtr
+PageGetLSN(Page page)
+{
+	AssertPageIsLockedForLSN(page);
+	return PageXLogRecPtrGet(((PageHeader) page)->pd_lsn);
+}
+
 #define PageSetLSN(page, lsn) \
 	PageXLogRecPtrSet(((PageHeader) (page))->pd_lsn, lsn)
 
-- 
2.13.5 (Apple Git-94)