From d97475c877337d9e41529a95e2cd0ef63cd8f96e Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Tue, 2 Feb 2021 11:09:45 +1300 Subject: [PATCH v3 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. Discussion: https://postgr.es/m/CA+hUKGLdemy2gBm80kz20GTe6hNVwoErE8KwcJk6-U56oStjtg@mail.gmail.com --- 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.24.3 (Apple Git-128)