Re: Throwing away unnecessary spin-locks

Hi Alexandre,

> #define SLOCK_DEFINE_SCALAR_ACCESSORS(typename) \

> static inline typename \

> slock_read_barrier_##typename(volatile slock_t *lock, volatile typename
*p) \

> { \

> typename val; \

> \

> (void) lock; \

> AssertPointerAlignment(p, alignof(typename)); \

> val = *p; \

> pg_read_barrier(); \

> return val; \

> } \

> static inline void \

> slock_write_barrier_##typename(volatile slock_t *lock, volatile typename
*p, typename v) \

> { \

> (void) lock; \

> AssertPointerAlignment(p, alignof(typename)); \

> *p = v; \

> pg_write_barrier(); \

>

> }

On x86 pg_read_barrier() and pg_write_barrier() are only compiler
barriers, so v1 ends up as a plain load and a plain store there. That
hides problems. ARM, RISC-V and POWER are weakly ordered, and on those
a write barrier placed after the store does not order it against the
stores before it. The spinlock version does not have that problem.

I think the safer route is what recent commits like df3978c2340 did,
convert the field to pg_atomic and use pg_atomic_read_membarrier_u32()
and pg_atomic_write_membarrier_u32().

Thanks,
Shihao