From 17a7730c282018d02e3d82990f4fd84e0d2f6ce0 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 8 Jun 2020 17:10:26 -0700
Subject: [PATCH v1 1/4] WORKAROUND: Avoid spinlock use in signal handler by
 using 32bit generations.

As 64bit atomic reads aren't aren't guaranteed to be atomic on all
platforms we can end up with spinlocks being used inside signal
handlers. Which obviously is dangerous, as that can easily create self
deadlocks.

32bit atomics don't have that problem. But 32bit generations might not
be enough in all situations (?). So this possibly isn't the right
fix.

The problem can easily be reproduced by running make check in a
--disable-spinlocks --disable-atomics build.

Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/include/storage/procsignal.h     |  4 +--
 src/backend/storage/ipc/procsignal.c | 48 ++++++++++++++--------------
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/src/include/storage/procsignal.h b/src/include/storage/procsignal.h
index a0c0bc3ce55..c5a7e362dd9 100644
--- a/src/include/storage/procsignal.h
+++ b/src/include/storage/procsignal.h
@@ -65,8 +65,8 @@ extern void ProcSignalInit(int pss_idx);
 extern int	SendProcSignal(pid_t pid, ProcSignalReason reason,
 						   BackendId backendId);
 
-extern uint64 EmitProcSignalBarrier(ProcSignalBarrierType type);
-extern void WaitForProcSignalBarrier(uint64 generation);
+extern uint32 EmitProcSignalBarrier(ProcSignalBarrierType type);
+extern void WaitForProcSignalBarrier(uint32 generation);
 extern void ProcessProcSignalBarrier(void);
 
 extern void procsignal_sigusr1_handler(SIGNAL_ARGS);
diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index c809196d06a..eba424e15a7 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -60,7 +60,7 @@ typedef struct
 {
 	pid_t		pss_pid;
 	sig_atomic_t pss_signalFlags[NUM_PROCSIGNALS];
-	pg_atomic_uint64 pss_barrierGeneration;
+	pg_atomic_uint32 pss_barrierGeneration;
 	pg_atomic_uint32 pss_barrierCheckMask;
 } ProcSignalSlot;
 
@@ -72,7 +72,7 @@ typedef struct
  */
 typedef struct
 {
-	pg_atomic_uint64 psh_barrierGeneration;
+	pg_atomic_uint32 psh_barrierGeneration;
 	ProcSignalSlot psh_slot[FLEXIBLE_ARRAY_MEMBER];
 } ProcSignalHeader;
 
@@ -126,7 +126,7 @@ ProcSignalShmemInit(void)
 	{
 		int			i;
 
-		pg_atomic_init_u64(&ProcSignal->psh_barrierGeneration, 0);
+		pg_atomic_init_u32(&ProcSignal->psh_barrierGeneration, 0);
 
 		for (i = 0; i < NumProcSignalSlots; ++i)
 		{
@@ -134,7 +134,7 @@ ProcSignalShmemInit(void)
 
 			slot->pss_pid = 0;
 			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_barrierGeneration, PG_UINT32_MAX);
 			pg_atomic_init_u32(&slot->pss_barrierCheckMask, 0);
 		}
 	}
@@ -151,7 +151,7 @@ void
 ProcSignalInit(int pss_idx)
 {
 	volatile ProcSignalSlot *slot;
-	uint64		barrier_generation;
+	uint32		barrier_generation;
 
 	Assert(pss_idx >= 1 && pss_idx <= NumProcSignalSlots);
 
@@ -178,8 +178,8 @@ ProcSignalInit(int pss_idx)
 	 */
 	pg_atomic_write_u32(&slot->pss_barrierCheckMask, 0);
 	barrier_generation =
-		pg_atomic_read_u64(&ProcSignal->psh_barrierGeneration);
-	pg_atomic_write_u64(&slot->pss_barrierGeneration, barrier_generation);
+		pg_atomic_read_u32(&ProcSignal->psh_barrierGeneration);
+	pg_atomic_write_u32(&slot->pss_barrierGeneration, barrier_generation);
 	pg_memory_barrier();
 
 	/* Mark slot with my PID */
@@ -230,7 +230,7 @@ CleanupProcSignalState(int status, Datum arg)
 	 * Make this slot look like it's absorbed all possible barriers, so that
 	 * no barrier waits block on it.
 	 */
-	pg_atomic_write_u64(&slot->pss_barrierGeneration, PG_UINT64_MAX);
+	pg_atomic_write_u32(&slot->pss_barrierGeneration, PG_UINT32_MAX);
 
 	slot->pss_pid = 0;
 }
@@ -317,11 +317,11 @@ SendProcSignal(pid_t pid, ProcSignalReason reason, BackendId backendId)
  * Callers are entitled to assume that this function will not throw ERROR
  * or FATAL.
  */
-uint64
+uint32
 EmitProcSignalBarrier(ProcSignalBarrierType type)
 {
-	uint64		flagbit = UINT64CONST(1) << (uint64) type;
-	uint64		generation;
+	uint32		flagbit = 1 << (uint32) type;
+	uint32		generation;
 
 	/*
 	 * Set all the flags.
@@ -329,7 +329,7 @@ EmitProcSignalBarrier(ProcSignalBarrierType type)
 	 * Note that pg_atomic_fetch_or_u32 has full barrier semantics, so this is
 	 * totally ordered with respect to anything the caller did before, and
 	 * anything that we do afterwards. (This is also true of the later call to
-	 * pg_atomic_add_fetch_u64.)
+	 * pg_atomic_add_fetch_u32.)
 	 */
 	for (int i = 0; i < NumProcSignalSlots; i++)
 	{
@@ -342,7 +342,7 @@ EmitProcSignalBarrier(ProcSignalBarrierType type)
 	 * Increment the generation counter.
 	 */
 	generation =
-		pg_atomic_add_fetch_u64(&ProcSignal->psh_barrierGeneration, 1);
+		pg_atomic_add_fetch_u32(&ProcSignal->psh_barrierGeneration, 1);
 
 	/*
 	 * Signal all the processes, so that they update their advertised barrier
@@ -379,16 +379,16 @@ EmitProcSignalBarrier(ProcSignalBarrierType type)
  * 1 second.
  */
 void
-WaitForProcSignalBarrier(uint64 generation)
+WaitForProcSignalBarrier(uint32 generation)
 {
 	long		timeout = 125L;
 
 	for (int i = NumProcSignalSlots - 1; i >= 0; i--)
 	{
 		volatile ProcSignalSlot *slot = &ProcSignal->psh_slot[i];
-		uint64		oldval;
+		uint32		oldval;
 
-		oldval = pg_atomic_read_u64(&slot->pss_barrierGeneration);
+		oldval = pg_atomic_read_u32(&slot->pss_barrierGeneration);
 		while (oldval < generation)
 		{
 			int			events;
@@ -401,7 +401,7 @@ WaitForProcSignalBarrier(uint64 generation)
 						  timeout, WAIT_EVENT_PROC_SIGNAL_BARRIER);
 			ResetLatch(MyLatch);
 
-			oldval = pg_atomic_read_u64(&slot->pss_barrierGeneration);
+			oldval = pg_atomic_read_u32(&slot->pss_barrierGeneration);
 			if (events & WL_TIMEOUT)
 				timeout = Min(timeout * 2, 1000L);
 		}
@@ -428,7 +428,7 @@ WaitForProcSignalBarrier(uint64 generation)
 void
 ProcessProcSignalBarrier(void)
 {
-	uint64		generation;
+	uint32		generation;
 	uint32		flags;
 
 	/* Exit quickly if there's no work to do. */
@@ -443,7 +443,7 @@ ProcessProcSignalBarrier(void)
 	 * happens before we atomically extract the flags, and that any subsequent
 	 * state changes happen afterward.
 	 */
-	generation = pg_atomic_read_u64(&ProcSignal->psh_barrierGeneration);
+	generation = pg_atomic_read_u32(&ProcSignal->psh_barrierGeneration);
 	flags = pg_atomic_exchange_u32(&MyProcSignalSlot->pss_barrierCheckMask, 0);
 
 	/*
@@ -466,7 +466,7 @@ ProcessProcSignalBarrier(void)
 	 * things have changed further, it'll get fixed up when this function is
 	 * next called.
 	 */
-	pg_atomic_write_u64(&MyProcSignalSlot->pss_barrierGeneration, generation);
+	pg_atomic_write_u32(&MyProcSignalSlot->pss_barrierGeneration, generation);
 }
 
 static void
@@ -515,11 +515,11 @@ CheckProcSignalBarrier(void)
 
 	if (slot != NULL)
 	{
-		uint64		mygen;
-		uint64		curgen;
+		uint32		mygen;
+		uint32		curgen;
 
-		mygen = pg_atomic_read_u64(&slot->pss_barrierGeneration);
-		curgen = pg_atomic_read_u64(&ProcSignal->psh_barrierGeneration);
+		mygen = pg_atomic_read_u32(&slot->pss_barrierGeneration);
+		curgen = pg_atomic_read_u32(&ProcSignal->psh_barrierGeneration);
 		return (mygen != curgen);
 	}
 
-- 
2.25.0.114.g5b0ca878e0

