On 2019-02-22 21:31, Andres Freund wrote: > On 2019-02-22 12:38:35 +0100, Peter Eisentraut wrote: >> On 2019-02-19 18:02, Andres Freund wrote: >>> But even if we were to decide we'd want to keep a volatile in SetLatch() >>> - which I think really would only serve to hide bugs - that'd not mean >>> it's a good idea to keep it on all the other functions in latch.c.
> Right. But we should ever look/write into the contents of a latch > outside of latch.c, so I don't think that'd really be a problem, even if > we relied on volatiles. So how about this patch? -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 8afb8bc4e01e32be532fefccfd39f4d7294d2f32 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pe...@eisentraut.org> Date: Mon, 25 Feb 2019 09:24:15 +0100 Subject: [PATCH] Remove volatile from latch API This was no longer useful since the latch functions use memory barriers already, which are also compiler barriers, and volatile does not help with cross-process access. Discussion: https://www.postgresql.org/message-id/flat/20190218202511.qsfpuj5sy4dbezcw%40alap3.anarazel.de#18783c27d73e9e40009c82f6e0df0974 --- src/backend/storage/ipc/latch.c | 18 +++++++++--------- src/include/storage/latch.h | 16 ++++++++-------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c index 7da337d11f..59fa917ae0 100644 --- a/src/backend/storage/ipc/latch.c +++ b/src/backend/storage/ipc/latch.c @@ -225,7 +225,7 @@ InitializeLatchSupport(void) * Initialize a process-local latch. */ void -InitLatch(volatile Latch *latch) +InitLatch(Latch *latch) { latch->is_set = false; latch->owner_pid = MyProcPid; @@ -257,7 +257,7 @@ InitLatch(volatile Latch *latch) * process references to postmaster-private latches or WaitEventSets. */ void -InitSharedLatch(volatile Latch *latch) +InitSharedLatch(Latch *latch) { #ifdef WIN32 SECURITY_ATTRIBUTES sa; @@ -293,7 +293,7 @@ InitSharedLatch(volatile Latch *latch) * as shared latches use SIGUSR1 for inter-process communication. */ void -OwnLatch(volatile Latch *latch) +OwnLatch(Latch *latch) { /* Sanity checks */ Assert(latch->is_shared); @@ -313,7 +313,7 @@ OwnLatch(volatile Latch *latch) * Disown a shared latch currently owned by the current process. */ void -DisownLatch(volatile Latch *latch) +DisownLatch(Latch *latch) { Assert(latch->is_shared); Assert(latch->owner_pid == MyProcPid); @@ -341,7 +341,7 @@ DisownLatch(volatile Latch *latch) * we return all of them in one call, but we will return at least one. */ int -WaitLatch(volatile Latch *latch, int wakeEvents, long timeout, +WaitLatch(Latch *latch, int wakeEvents, long timeout, uint32 wait_event_info) { return WaitLatchOrSocket(latch, wakeEvents, PGINVALID_SOCKET, timeout, @@ -366,7 +366,7 @@ WaitLatch(volatile Latch *latch, int wakeEvents, long timeout, * WaitEventSet instead; that's more efficient. */ int -WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, +WaitLatchOrSocket(Latch *latch, int wakeEvents, pgsocket sock, long timeout, uint32 wait_event_info) { int ret = 0; @@ -381,7 +381,7 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, if (wakeEvents & WL_LATCH_SET) AddWaitEventToSet(set, WL_LATCH_SET, PGINVALID_SOCKET, - (Latch *) latch, NULL); + latch, NULL); /* Postmaster-managed callers must handle postmaster death somehow. */ Assert(!IsUnderPostmaster || @@ -433,7 +433,7 @@ WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, pgsocket sock, * throwing an error is not a good idea. */ void -SetLatch(volatile Latch *latch) +SetLatch(Latch *latch) { #ifndef WIN32 pid_t owner_pid; @@ -516,7 +516,7 @@ SetLatch(volatile Latch *latch) * the latch is set again before the WaitLatch call. */ void -ResetLatch(volatile Latch *latch) +ResetLatch(Latch *latch) { /* Only the owner should reset the latch */ Assert(latch->owner_pid == MyProcPid); diff --git a/src/include/storage/latch.h b/src/include/storage/latch.h index 5b48709c8a..fc995819d3 100644 --- a/src/include/storage/latch.h +++ b/src/include/storage/latch.h @@ -156,12 +156,12 @@ typedef struct WaitEventSet WaitEventSet; * prototypes for functions in latch.c */ extern void InitializeLatchSupport(void); -extern void InitLatch(volatile Latch *latch); -extern void InitSharedLatch(volatile Latch *latch); -extern void OwnLatch(volatile Latch *latch); -extern void DisownLatch(volatile Latch *latch); -extern void SetLatch(volatile Latch *latch); -extern void ResetLatch(volatile Latch *latch); +extern void InitLatch(Latch *latch); +extern void InitSharedLatch(Latch *latch); +extern void OwnLatch(Latch *latch); +extern void DisownLatch(Latch *latch); +extern void SetLatch(Latch *latch); +extern void ResetLatch(Latch *latch); extern WaitEventSet *CreateWaitEventSet(MemoryContext context, int nevents); extern void FreeWaitEventSet(WaitEventSet *set); @@ -172,9 +172,9 @@ extern void ModifyWaitEvent(WaitEventSet *set, int pos, uint32 events, Latch *la extern int WaitEventSetWait(WaitEventSet *set, long timeout, WaitEvent *occurred_events, int nevents, uint32 wait_event_info); -extern int WaitLatch(volatile Latch *latch, int wakeEvents, long timeout, +extern int WaitLatch(Latch *latch, int wakeEvents, long timeout, uint32 wait_event_info); -extern int WaitLatchOrSocket(volatile Latch *latch, int wakeEvents, +extern int WaitLatchOrSocket(Latch *latch, int wakeEvents, pgsocket sock, long timeout, uint32 wait_event_info); /* -- 2.20.1
