5 files changed, 23 insertions, 27 deletions
diff --git a/src/backend/port/unix_latch.c b/src/backend/port/unix_latch.c
index 92ae7801583..147e22cee4e 100644
--- a/src/backend/port/unix_latch.c
+++ b/src/backend/port/unix_latch.c
@@ -51,6 +51,7 @@
 #include "miscadmin.h"
 #include "portability/instr_time.h"
 #include "postmaster/postmaster.h"
+#include "storage/barrier.h"
 #include "storage/latch.h"
 #include "storage/pmsignal.h"
 #include "storage/shmem.h"
@@ -515,12 +516,11 @@ SetLatch(volatile Latch *latch)
 	pid_t		owner_pid;
 
 	/*
-	 * XXX there really ought to be a memory barrier operation right here, to
-	 * ensure that any flag variables we might have changed get flushed to
-	 * main memory before we check/set is_set.  Without that, we have to
-	 * require that callers provide their own synchronization for machines
-	 * with weak memory ordering (see latch.h).
+	 * The memory barrier has be to be placed here to ensure that any flag
+	 * variables possibly changed by this process have been flushed to main
+	 * memory, before we check/set is_set.
 	 */
+	pg_memory_barrier();
 
 	/* Quick exit if already set */
 	if (latch->is_set)
@@ -574,14 +574,12 @@ ResetLatch(volatile Latch *latch)
 	latch->is_set = false;
 
 	/*
-	 * XXX there really ought to be a memory barrier operation right here, to
-	 * ensure that the write to is_set gets flushed to main memory before we
+	 * Ensure that the write to is_set gets flushed to main memory before we
 	 * examine any flag variables.  Otherwise a concurrent SetLatch might
 	 * falsely conclude that it needn't signal us, even though we have missed
 	 * seeing some flag updates that SetLatch was supposed to inform us of.
-	 * For the moment, callers must supply their own synchronization of flag
-	 * variables (see latch.h).
 	 */
+	pg_memory_barrier();
 }
 
 /*
diff --git a/src/backend/port/win32_latch.c b/src/backend/port/win32_latch.c
index da0657c915a..c7d4bdddc21 100644
--- a/src/backend/port/win32_latch.c
+++ b/src/backend/port/win32_latch.c
@@ -27,6 +27,7 @@
 #include "miscadmin.h"
 #include "portability/instr_time.h"
 #include "postmaster/postmaster.h"
+#include "storage/barrier.h"
 #include "storage/latch.h"
 #include "storage/pmsignal.h"
 #include "storage/shmem.h"
@@ -293,6 +294,13 @@ SetLatch(volatile Latch *latch)
 {
 	HANDLE		handle;
 
+	/*
+	 * The memory barrier has be to be placed here to ensure that any flag
+	 * variables possibly changed by this process have been flushed to main
+	 * memory, before we check/set is_set.
+	 */
+	pg_memory_barrier();
+
 	/* Quick exit if already set */
 	if (latch->is_set)
 		return;
@@ -325,4 +333,12 @@ ResetLatch(volatile Latch *latch)
 	Assert(latch->owner_pid == MyProcPid);
 
 	latch->is_set = false;
+
+	/*
+	 * Ensure that the write to is_set gets flushed to main memory before we
+	 * examine any flag variables.  Otherwise a concurrent SetLatch might
+	 * falsely conclude that it needn't signal us, even though we have missed
+	 * seeing some flag updates that SetLatch was supposed to inform us of.
+	 */
+	pg_memory_barrier();
 }
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
index c99c270a7ca..b2b3a81f54c 100644
--- a/src/backend/replication/syncrep.c
+++ b/src/backend/replication/syncrep.c
@@ -172,20 +172,10 @@ SyncRepWaitForLSN(XLogRecPtr XactCommitLSN)
 		 * walsender changes the state to SYNC_REP_WAIT_COMPLETE, it will
 		 * never update it again, so we can't be seeing a stale value in that
 		 * case.
-		 *
-		 * Note: on machines with weak memory ordering, the acquisition of the
-		 * lock is essential to avoid race conditions: we cannot be sure the
-		 * sender's state update has reached main memory until we acquire the
-		 * lock.  We could get rid of this dance if SetLatch/ResetLatch
-		 * contained memory barriers.
 		 */
 		syncRepState = MyProc->syncRepState;
 		if (syncRepState == SYNC_REP_WAITING)
-		{
-			LWLockAcquire(SyncRepLock, LW_SHARED);
 			syncRepState = MyProc->syncRepState;
-			LWLockRelease(SyncRepLock);
-		}
 		if (syncRepState == SYNC_REP_WAIT_COMPLETE)
 			break;
 
diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
index ac647d65458..3add619b5da 100644
--- a/src/backend/storage/buffer/freelist.c
+++ b/src/backend/storage/buffer/freelist.c
@@ -214,7 +214,6 @@ StrategyGetBuffer(BufferAccessStrategy strategy)
 	{
 		/* reset bgwprocno first, before setting the latch */
 		StrategyControl->bgwprocno = -1;
-		pg_write_barrier();
 
 		/*
 		 * Not acquiring ProcArrayLock here which is slightly icky. It's
diff --git a/src/include/storage/latch.h b/src/include/storage/latch.h
index 138a748f624..f1577b00955 100644
--- a/src/include/storage/latch.h
+++ b/src/include/storage/latch.h
@@ -57,13 +57,6 @@
  * SetLatch *after* that. SetLatch is designed to return quickly if the
  * latch is already set.
  *
- * Presently, when using a shared latch for interprocess signalling, the
- * flag variable(s) set by senders and inspected by the wait loop must
- * be protected by spinlocks or LWLocks, else it is possible to miss events
- * on machines with weak memory ordering (such as PPC).  This restriction
- * will be lifted in future by inserting suitable memory barriers into
- * SetLatch and ResetLatch.
- *
  * On some platforms, signals will not interrupt the latch wait primitive
  * by themselves.  Therefore, it is critical that any signal handler that
  * is meant to terminate a WaitLatch wait calls SetLatch.