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
