I posted some bufmgr cleanup a few weeks ago, but it conflicted with

some concurrent changes Jan was making to the bufmgr. Here's an
updated version of the patch -- it should apply cleanly to CVS
HEAD and passes the regression tests.

This patch makes the following changes:

     - remove the UnlockAndReleaseBuffer() and UnlockAndWriteBuffer()
       macros, and replace uses of them with calls to the appropriate
       functions.

     - remove a bunch of #ifdef BMTRACE code: it is ugly & broken
       (i.e. it doesn't compile)

     - make BufferReplace() return a bool, not an int

     - cleanup some logic in bufmgr.c; should be functionality
       equivalent to the previous code, just cleaner now

     - remove the BM_PRIVATE flag as it is unused

     - improve a few comments, etc.

src/backend/access/heap/heapam.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.158 2003/11/29 19:51:40 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.159 2003/12/14 00:34:47 neilc Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -2110,7 +2110,8 @@ heap_xlog_clean(bool redo, XLogRecPtr lsn, XLogRecord *record)
 
 	if (XLByteLE(lsn, PageGetLSN(page)))
 	{
-		UnlockAndReleaseBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		ReleaseBuffer(buffer);
 		return;
 	}
 
@@ -2135,7 +2136,8 @@ heap_xlog_clean(bool redo, XLogRecPtr lsn, XLogRecord *record)
 
 	PageSetLSN(page, lsn);
 	PageSetSUI(page, ThisStartUpID);	/* prev sui */
-	UnlockAndWriteBuffer(buffer);
+	LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+	WriteBuffer(buffer);
 }
 
 static void
@@ -2170,7 +2172,8 @@ heap_xlog_delete(bool redo, XLogRecPtr lsn, XLogRecord *record)
 	{
 		if (XLByteLE(lsn, PageGetLSN(page)))	/* changes are applied */
 		{
-			UnlockAndReleaseBuffer(buffer);
+			LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+			ReleaseBuffer(buffer);
 			return;
 		}
 	}
@@ -2199,7 +2202,8 @@ heap_xlog_delete(bool redo, XLogRecPtr lsn, XLogRecord *record)
 		htup->t_ctid = xlrec->target.tid;
 		PageSetLSN(page, lsn);
 		PageSetSUI(page, ThisStartUpID);
-		UnlockAndWriteBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		WriteBuffer(buffer);
 		return;
 	}
 
@@ -2249,7 +2253,8 @@ heap_xlog_insert(bool redo, XLogRecPtr lsn, XLogRecord *record)
 
 		if (XLByteLE(lsn, PageGetLSN(page)))	/* changes are applied */
 		{
-			UnlockAndReleaseBuffer(buffer);
+			LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+			ReleaseBuffer(buffer);
 			return;
 		}
 
@@ -2282,7 +2287,8 @@ heap_xlog_insert(bool redo, XLogRecPtr lsn, XLogRecord *record)
 			elog(PANIC, "heap_insert_redo: failed to add tuple");
 		PageSetLSN(page, lsn);
 		PageSetSUI(page, ThisStartUpID);		/* prev sui */
-		UnlockAndWriteBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		WriteBuffer(buffer);
 		return;
 	}
 
@@ -2332,7 +2338,8 @@ heap_xlog_update(bool redo, XLogRecPtr lsn, XLogRecord *record, bool move)
 	{
 		if (XLByteLE(lsn, PageGetLSN(page)))	/* changes are applied */
 		{
-			UnlockAndReleaseBuffer(buffer);
+			LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+			ReleaseBuffer(buffer);
 			if (samepage)
 				return;
 			goto newt;
@@ -2378,7 +2385,8 @@ heap_xlog_update(bool redo, XLogRecPtr lsn, XLogRecord *record, bool move)
 			goto newsame;
 		PageSetLSN(page, lsn);
 		PageSetSUI(page, ThisStartUpID);
-		UnlockAndWriteBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		WriteBuffer(buffer);
 		goto newt;
 	}
 
@@ -2421,7 +2429,8 @@ newsame:;
 
 		if (XLByteLE(lsn, PageGetLSN(page)))	/* changes are applied */
 		{
-			UnlockAndReleaseBuffer(buffer);
+			LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+			ReleaseBuffer(buffer);
 			return;
 		}
 
@@ -2474,7 +2483,8 @@ newsame:;
 			elog(PANIC, "heap_update_redo: failed to add tuple");
 		PageSetLSN(page, lsn);
 		PageSetSUI(page, ThisStartUpID);		/* prev sui */
-		UnlockAndWriteBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		WriteBuffer(buffer);
 		return;
 	}
 
@@ -2523,7 +2533,8 @@ _heap_unlock_tuple(void *data)
 		elog(PANIC, "_heap_unlock_tuple: invalid xmax in rollback");
 	htup->t_infomask &= ~HEAP_XMAX_UNLOGGED;
 	htup->t_infomask |= HEAP_XMAX_INVALID;
-	UnlockAndWriteBuffer(buffer);
+	LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+	WriteBuffer(buffer);
 	return;
 }
 

src/backend/access/nbtree/nbtxlog.c

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtxlog.c,v 1.8 2003/11/29 19:51:40 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/nbtree/nbtxlog.c,v 1.9 2003/12/14 00:34:47 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -69,7 +69,8 @@ forget_matching_split(Relation reln, RelFileNode node,
 	btitem = (BTItem) PageGetItem(page, PageGetItemId(page, offnum));
 	rightblk = ItemPointerGetBlockNumber(&(btitem->bti_itup.t_tid));
 	Assert(ItemPointerGetOffsetNumber(&(btitem->bti_itup.t_tid)) == P_HIKEY);
-	UnlockAndReleaseBuffer(buffer);
+	LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+	ReleaseBuffer(buffer);
 
 	foreach(l, incomplete_splits)
 	{
@@ -137,7 +138,8 @@ _bt_restore_meta(Relation reln, XLogRecPtr lsn,
 
 	PageSetLSN(metapg, lsn);
 	PageSetSUI(metapg, ThisStartUpID);
-	UnlockAndWriteBuffer(metabuf);
+	LockBuffer(metabuf, BUFFER_LOCK_UNLOCK);
+	WriteBuffer(metabuf);
 }
 
 static void
@@ -184,7 +186,10 @@ btree_xlog_insert(bool redo, bool isleaf, bool ismeta,
 		if (redo)
 		{
 			if (XLByteLE(lsn, PageGetLSN(page)))
-				UnlockAndReleaseBuffer(buffer);
+			{
+				LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+				ReleaseBuffer(buffer);
+			}
 			else
 			{
 				if (PageAddItem(page, (Item) datapos, datalen,
@@ -194,7 +199,8 @@ btree_xlog_insert(bool redo, bool isleaf, bool ismeta,
 
 				PageSetLSN(page, lsn);
 				PageSetSUI(page, ThisStartUpID);
-				UnlockAndWriteBuffer(buffer);
+				LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+				WriteBuffer(buffer);
 			}
 		}
 		else
@@ -203,7 +209,10 @@ btree_xlog_insert(bool redo, bool isleaf, bool ismeta,
 				elog(PANIC, "btree_insert_undo: bad page LSN");
 
 			if (!P_ISLEAF(pageop))
-				UnlockAndReleaseBuffer(buffer);
+			{
+				LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+				ReleaseBuffer(buffer);
+			}
 			else
 				elog(PANIC, "btree_insert_undo: unimplemented");
 		}
@@ -275,7 +284,8 @@ btree_xlog_split(bool redo, bool onleft, bool isroot,
 
 		PageSetLSN(page, lsn);
 		PageSetSUI(page, ThisStartUpID);
-		UnlockAndWriteBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		WriteBuffer(buffer);
 	}
 	else
 	{
@@ -310,7 +320,8 @@ btree_xlog_split(bool redo, bool onleft, bool isroot,
 
 		PageSetLSN(page, lsn);
 		PageSetSUI(page, ThisStartUpID);
-		UnlockAndWriteBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		WriteBuffer(buffer);
 	}
 	else
 	{
@@ -334,7 +345,10 @@ btree_xlog_split(bool redo, bool onleft, bool isroot,
 				elog(PANIC, "btree_split_redo: uninitialized next right page");
 
 			if (XLByteLE(lsn, PageGetLSN(page)))
-				UnlockAndReleaseBuffer(buffer);
+			{
+				LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+				ReleaseBuffer(buffer);
+			}
 			else
 			{
 				pageop = (BTPageOpaque) PageGetSpecialPointer(page);
@@ -342,7 +356,8 @@ btree_xlog_split(bool redo, bool onleft, bool isroot,
 
 				PageSetLSN(page, lsn);
 				PageSetSUI(page, ThisStartUpID);
-				UnlockAndWriteBuffer(buffer);
+				LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+				WriteBuffer(buffer);
 			}
 		}
 	}
@@ -385,7 +400,8 @@ btree_xlog_delete(bool redo, XLogRecPtr lsn, XLogRecord *record)
 
 	if (XLByteLE(lsn, PageGetLSN(page)))
 	{
-		UnlockAndReleaseBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		ReleaseBuffer(buffer);
 		return;
 	}
 
@@ -407,7 +423,8 @@ btree_xlog_delete(bool redo, XLogRecPtr lsn, XLogRecord *record)
 
 	PageSetLSN(page, lsn);
 	PageSetSUI(page, ThisStartUpID);
-	UnlockAndWriteBuffer(buffer);
+	LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+	WriteBuffer(buffer);
 }
 
 static void
@@ -445,7 +462,10 @@ btree_xlog_delete_page(bool redo, bool ismeta,
 		if (PageIsNew((PageHeader) page))
 			elog(PANIC, "btree_delete_page_redo: uninitialized parent page");
 		if (XLByteLE(lsn, PageGetLSN(page)))
-			UnlockAndReleaseBuffer(buffer);
+		{
+			LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+			ReleaseBuffer(buffer);
+		}
 		else
 		{
 			OffsetNumber poffset;
@@ -472,7 +492,8 @@ btree_xlog_delete_page(bool redo, bool ismeta,
 
 			PageSetLSN(page, lsn);
 			PageSetSUI(page, ThisStartUpID);
-			UnlockAndWriteBuffer(buffer);
+			LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+			WriteBuffer(buffer);
 		}
 	}
 
@@ -486,7 +507,10 @@ btree_xlog_delete_page(bool redo, bool ismeta,
 		if (PageIsNew((PageHeader) page))
 			elog(PANIC, "btree_delete_page_redo: uninitialized right sibling");
 		if (XLByteLE(lsn, PageGetLSN(page)))
-			UnlockAndReleaseBuffer(buffer);
+		{
+			LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+			ReleaseBuffer(buffer);
+		}
 		else
 		{
 			pageop = (BTPageOpaque) PageGetSpecialPointer(page);
@@ -494,7 +518,8 @@ btree_xlog_delete_page(bool redo, bool ismeta,
 
 			PageSetLSN(page, lsn);
 			PageSetSUI(page, ThisStartUpID);
-			UnlockAndWriteBuffer(buffer);
+			LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+			WriteBuffer(buffer);
 		}
 	}
 
@@ -510,7 +535,10 @@ btree_xlog_delete_page(bool redo, bool ismeta,
 			if (PageIsNew((PageHeader) page))
 				elog(PANIC, "btree_delete_page_redo: uninitialized left sibling");
 			if (XLByteLE(lsn, PageGetLSN(page)))
-				UnlockAndReleaseBuffer(buffer);
+			{
+				LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+				ReleaseBuffer(buffer);
+			}
 			else
 			{
 				pageop = (BTPageOpaque) PageGetSpecialPointer(page);
@@ -518,7 +546,8 @@ btree_xlog_delete_page(bool redo, bool ismeta,
 
 				PageSetLSN(page, lsn);
 				PageSetSUI(page, ThisStartUpID);
-				UnlockAndWriteBuffer(buffer);
+				LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+				WriteBuffer(buffer);
 			}
 		}
 	}
@@ -543,7 +572,8 @@ btree_xlog_delete_page(bool redo, bool ismeta,
 
 		PageSetLSN(page, lsn);
 		PageSetSUI(page, ThisStartUpID);
-		UnlockAndWriteBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		WriteBuffer(buffer);
 	}
 	else
 	{
@@ -606,7 +636,8 @@ btree_xlog_newroot(bool redo, XLogRecPtr lsn, XLogRecord *record)
 
 	PageSetLSN(page, lsn);
 	PageSetSUI(page, ThisStartUpID);
-	UnlockAndWriteBuffer(buffer);
+	LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+	WriteBuffer(buffer);
 
 	_bt_restore_meta(reln, lsn,
 					 xlrec->rootblk, xlrec->level,
@@ -668,7 +699,8 @@ btree_xlog_newpage(bool redo, XLogRecPtr lsn, XLogRecord *record)
 
 	PageSetLSN(page, lsn);
 	PageSetSUI(page, ThisStartUpID);
-	UnlockAndWriteBuffer(buffer);
+	LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+	WriteBuffer(buffer);
 }
 
 

src/backend/access/transam/xlog.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.127 2003/12/12 18:45:08 petere Exp $
+ * $PostgreSQL: pgsql/src/backend/access/transam/xlog.c,v 1.128 2003/12/14 00:34:47 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1712,7 +1712,8 @@ RestoreBkpBlocks(XLogRecord *record, XLogRecPtr lsn)
 				memcpy((char *) page, blk, BLCKSZ);
 				PageSetLSN(page, lsn);
 				PageSetSUI(page, ThisStartUpID);
-				UnlockAndWriteBuffer(buffer);
+				LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+				WriteBuffer(buffer);
 			}
 		}
 

src/backend/access/transam/xlogutils.c

@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/access/transam/xlogutils.c,v 1.27 2003/11/29 19:51:40 pgsql Exp $
+ * $PostgreSQL: pgsql/src/backend/access/transam/xlogutils.c,v 1.28 2003/12/14 00:34:47 neilc Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -60,13 +60,15 @@ XLogIsOwnerOfTuple(RelFileNode hnode, ItemPointer iptr,
 	if (PageIsNew((PageHeader) page) ||
 		ItemPointerGetOffsetNumber(iptr) > PageGetMaxOffsetNumber(page))
 	{
-		UnlockAndReleaseBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		ReleaseBuffer(buffer);
 		return (0);
 	}
 	lp = PageGetItemId(page, ItemPointerGetOffsetNumber(iptr));
 	if (!ItemIdIsUsed(lp) || ItemIdDeleted(lp))
 	{
-		UnlockAndReleaseBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		ReleaseBuffer(buffer);
 		return (0);
 	}
 
@@ -76,11 +78,13 @@ XLogIsOwnerOfTuple(RelFileNode hnode, ItemPointer iptr,
 	if (!TransactionIdEquals(HeapTupleHeaderGetXmin(htup), xid) ||
 		HeapTupleHeaderGetCmin(htup) != cid)
 	{
-		UnlockAndReleaseBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		ReleaseBuffer(buffer);
 		return (-1);
 	}
 
-	UnlockAndReleaseBuffer(buffer);
+	LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+	ReleaseBuffer(buffer);
 	return (1);
 }
 
@@ -112,21 +116,24 @@ XLogIsValidTuple(RelFileNode hnode, ItemPointer iptr)
 	if (PageIsNew((PageHeader) page) ||
 		ItemPointerGetOffsetNumber(iptr) > PageGetMaxOffsetNumber(page))
 	{
-		UnlockAndReleaseBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		ReleaseBuffer(buffer);
 		return (false);
 	}
 
 	if (PageGetSUI(page) != ThisStartUpID)
 	{
 		Assert(PageGetSUI(page) < ThisStartUpID);
-		UnlockAndReleaseBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		ReleaseBuffer(buffer);
 		return (true);
 	}
 
 	lp = PageGetItemId(page, ItemPointerGetOffsetNumber(iptr));
 	if (!ItemIdIsUsed(lp) || ItemIdDeleted(lp))
 	{
-		UnlockAndReleaseBuffer(buffer);
+		LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+		ReleaseBuffer(buffer);
 		return (false);
 	}
 
@@ -141,12 +148,14 @@ XLogIsValidTuple(RelFileNode hnode, ItemPointer iptr)
 			 TransactionIdDidAbort(HeapTupleHeaderGetXvac(htup))) ||
 			TransactionIdDidAbort(HeapTupleHeaderGetXmin(htup)))
 		{
-			UnlockAndReleaseBuffer(buffer);
+			LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+			ReleaseBuffer(buffer);
 			return (false);
 		}
 	}
 
-	UnlockAndReleaseBuffer(buffer);
+	LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
+	ReleaseBuffer(buffer);
 	return (true);
 }