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