Re: Throwing away unnecessary spin-locks

From: Alexandre Felipe <o(dot)alexandre(dot)felipe(at)gmail(dot)com>
To: shihao zhong <zhong950419(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Throwing away unnecessary spin-locks
Date: 2026-10-02 07:22:10
Message-ID: CAE8JnxMyR_6YOU_nhVCop0S96A3kEYK6w8O5U_xzXsGorU4dVg@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Oct 2, 2026 at 6:30 AM shihao zhong <zhong950419(at)gmail(dot)com> wrote:

> 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.
>

pg_write_barrier(), pg_read_barrier(), are compiler barriers, and they
are the same as pg_compiler_barrier()?

Those barriers have no cost at run time right? So it wouldn't hurt
having two.

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().
>

I see that pg_atomic_write_membarrier_u32 uses an exchange,
In the patch above I used __atomic_store (that AFAIK will not be reordered)
and might block other processes trying to write on the same
cache line.

Trying to make it portable, to handle at least bool, uint32, uint64, Pointer
in a regular pattern.

Full barrier
pg_memory_barrier();
*out = *in; /* where only one side is shared */
pg_memory_barrier();

Release barrier
pg_compiler_barrier();
*p_shared = v_local;
pg_memory_barrier();

Acquire barrier
pg_memory_barrier();
v_local = *p_shared;
pg_compiler_barrier();

Does it make sense?

Can release/acquire semantics be used or we MUST have a full barrier?

Regards,
Alexandre

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Kukushkin 2026-10-02 07:25:42 Re: pg_dump: assert failure sorting casts/transforms
Previous Message Álvaro Herrera 2026-10-02 07:21:17 Re: wiki upgrade