From 6021e37bd8c7d09ae2b5ade06cdc9dfd352a1c98 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Sat, 30 Jan 2021 16:32:49 +1300
Subject: [PATCH v2 2/2] Use condition variables for ProcSignalBarriers.

Instead of a poll/sleep loop, use condition variables for precise
wake-up.

Adjust the condition variable sleep loop to support code that runs
inside CFI() code that might cause the current cv_sleep_target to move
under its feet without becoming corrupted.
---
 src/backend/storage/ipc/procsignal.c          | 31 +++++++------------
 src/backend/storage/lmgr/condition_variable.c | 11 +++++--
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index 30082d443f..c9aed86af0 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -23,6 +23,7 @@
 #include "miscadmin.h"
 #include "pgstat.h"
 #include "replication/walsender.h"
+#include "storage/condition_variable.h"
 #include "storage/ipc.h"
 #include "storage/latch.h"
 #include "storage/proc.h"
@@ -60,10 +61,11 @@
  */
 typedef struct
 {
-	pid_t		pss_pid;
-	sig_atomic_t pss_signalFlags[NUM_PROCSIGNALS];
+	volatile pid_t		pss_pid;
+	volatile sig_atomic_t pss_signalFlags[NUM_PROCSIGNALS];
 	pg_atomic_uint64 pss_barrierGeneration;
 	pg_atomic_uint32 pss_barrierCheckMask;
+	ConditionVariable pss_barrierCV;
 } ProcSignalSlot;
 
 /*
@@ -94,7 +96,7 @@ typedef struct
 	((flags) &= ~(((uint32) 1) << (uint32) (type)))
 
 static ProcSignalHeader *ProcSignal = NULL;
-static volatile ProcSignalSlot *MyProcSignalSlot = NULL;
+static ProcSignalSlot *MyProcSignalSlot = NULL;
 
 static bool CheckProcSignal(ProcSignalReason reason);
 static void CleanupProcSignalState(int status, Datum arg);
@@ -143,6 +145,7 @@ ProcSignalShmemInit(void)
 			MemSet(slot->pss_signalFlags, 0, sizeof(slot->pss_signalFlags));
 			pg_atomic_init_u64(&slot->pss_barrierGeneration, PG_UINT64_MAX);
 			pg_atomic_init_u32(&slot->pss_barrierCheckMask, 0);
+			ConditionVariableInit(&slot->pss_barrierCV);
 		}
 	}
 }
@@ -157,7 +160,7 @@ ProcSignalShmemInit(void)
 void
 ProcSignalInit(int pss_idx)
 {
-	volatile ProcSignalSlot *slot;
+	ProcSignalSlot *slot;
 	uint64		barrier_generation;
 
 	Assert(pss_idx >= 1 && pss_idx <= NumProcSignalSlots);
@@ -392,13 +395,11 @@ EmitProcSignalBarrier(ProcSignalBarrierType type)
 void
 WaitForProcSignalBarrier(uint64 generation)
 {
-	long		timeout = 125L;
-
 	Assert(generation <= pg_atomic_read_u64(&ProcSignal->psh_barrierGeneration));
 
 	for (int i = NumProcSignalSlots - 1; i >= 0; i--)
 	{
-		volatile ProcSignalSlot *slot = &ProcSignal->psh_slot[i];
+		ProcSignalSlot *slot = &ProcSignal->psh_slot[i];
 		uint64		oldval;
 
 		/*
@@ -410,20 +411,11 @@ WaitForProcSignalBarrier(uint64 generation)
 		oldval = pg_atomic_read_u64(&slot->pss_barrierGeneration);
 		while (oldval < generation)
 		{
-			int			events;
-
-			CHECK_FOR_INTERRUPTS();
-
-			events =
-				WaitLatch(MyLatch,
-						  WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,
-						  timeout, WAIT_EVENT_PROC_SIGNAL_BARRIER);
-			ResetLatch(MyLatch);
-
+			ConditionVariableSleep(&slot->pss_barrierCV,
+								   WAIT_EVENT_PROC_SIGNAL_BARRIER);
 			oldval = pg_atomic_read_u64(&slot->pss_barrierGeneration);
-			if (events & WL_TIMEOUT)
-				timeout = Min(timeout * 2, 1000L);
 		}
+		ConditionVariableCancelSleep();
 	}
 
 	/*
@@ -590,6 +582,7 @@ ProcessProcSignalBarrier(void)
 	 * next called.
 	 */
 	pg_atomic_write_u64(&MyProcSignalSlot->pss_barrierGeneration, shared_gen);
+	ConditionVariableBroadcast(&MyProcSignalSlot->pss_barrierCV);
 }
 
 /*
diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c
index 0a61ff0031..3b2f4fd8b7 100644
--- a/src/backend/storage/lmgr/condition_variable.c
+++ b/src/backend/storage/lmgr/condition_variable.c
@@ -165,8 +165,6 @@ ConditionVariableTimedSleep(ConditionVariable *cv, long timeout,
 		/* Reset latch before examining the state of the wait list. */
 		ResetLatch(MyLatch);
 
-		CHECK_FOR_INTERRUPTS();
-
 		/*
 		 * If this process has been taken out of the wait list, then we know
 		 * that it has been signaled by ConditionVariableSignal (or
@@ -190,6 +188,15 @@ ConditionVariableTimedSleep(ConditionVariable *cv, long timeout,
 		}
 		SpinLockRelease(&cv->mutex);
 
+		/*
+		 * Check for interrupts, and return spuriously if that caused the
+		 * current sleep target to change incidentally through using other CVs
+		 * (notably ProcSignalBarrier).
+		 */
+		CHECK_FOR_INTERRUPTS();
+		if (cv != cv_sleep_target)
+			done = true;
+
 		/* We were signaled, so return */
 		if (done)
 			return false;
-- 
2.20.1