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
