On 2016-10-05 15:02:09 -0400, Tom Lane wrote:
> Andres Freund <and...@anarazel.de> writes:
> > Without yet having analyzed this deeply, could it actually be that the
> > reason is that sem_post/wait aren't proper memory barriers?  On a glance
> > the symptoms look like values have been modified without proper locks...
>
> Hmm, possible ...

Hm. After a long battle of head vs. wall I think I see what the problem
is.  For the fallback atomics implementation I somehow had assumed that
pg_atomic_write_u32() doesn't need to lock, as it's just an unlocked
write.  But that's not true, because it has to cause
pg_atomic_compare_exchange_u32 to fail.  The lack of this can cause a
"leftover" lockbit, when UnlockBufHdr() occurs concurrently an operation
using compare_exchange.

For me the problem often takes a lot longer to reproduce (once only
after 40min), could you run with the attached patch, and see whether
that fixes things for you?

Andres
diff --git a/src/backend/port/atomics.c b/src/backend/port/atomics.c
index 42169a3..aa649b7 100644
--- a/src/backend/port/atomics.c
+++ b/src/backend/port/atomics.c
@@ -104,6 +104,19 @@ pg_atomic_init_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val_)
 	ptr->value = val_;
 }
 
+void
+pg_atomic_write_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val)
+{
+	/*
+	 * One might think that an unlocked write doesn't need a lock, but one
+	 * would be wrong. Even an unlocked write has to cause
+	 * pg_atomic_compare_exchange_u32() (et al) to fail.
+	 */
+	SpinLockAcquire((slock_t *) &ptr->sema);
+	ptr->value = val;
+	SpinLockRelease((slock_t *) &ptr->sema);
+}
+
 bool
 pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr,
 									uint32 *expected, uint32 newval)
diff --git a/src/include/port/atomics/fallback.h b/src/include/port/atomics/fallback.h
index bdaa795..2290fff 100644
--- a/src/include/port/atomics/fallback.h
+++ b/src/include/port/atomics/fallback.h
@@ -133,6 +133,9 @@ pg_atomic_unlocked_test_flag_impl(volatile pg_atomic_flag *ptr)
 #define PG_HAVE_ATOMIC_INIT_U32
 extern void pg_atomic_init_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val_);
 
+#define PG_HAVE_ATOMIC_WRITE_U32
+extern void pg_atomic_write_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val);
+
 #define PG_HAVE_ATOMIC_COMPARE_EXCHANGE_U32
 extern bool pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr,
 												uint32 *expected, uint32 newval);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to