Re: Throwing away unnecessary spin-locks

Hi,

On 2026-10-01 20:50:35 +0100, Alexandre Felipe wrote:
> Please try, if you want
> $ grep -A 2 -rn SpinLockAcquire src/backend | grep SpinLockRelease -B 2
>
> Story
> =====
> A common pattern that I have been repeatedly warned against is using
> spin-locks unnecessarily. I was surprised to see in checkpointer.c
> FirstCallSinceLastCheckpoint.
>
> int new_done;
> SpinLockAcquire(&CheckpointerShmem->ckpt_lck);
> new_done = CheckpointerShmem->ckpt_done;
> SpinLockRelease(&CheckpointerShmem->ckpt_lck);
>
> I think could certainly be replaced by something like
>
> + pg_compiler_barrier()
> + new_done = CheckpointerShmem->ckpt_done;
> + pg_compiler_barrier()

That's not a correct transformation. A compiler barrier does not guarantee
cache coherency. You actually have to use correct memory barrier pairings. It
might suffice to use a read memory barrier in this case, but obviously the
easiest transformation is to use full memory barriers on both sides.

The main reason for not having changed all of these over is that you actually
have to carefully think about the changes...

> Grepping the codebase we get 105 matches, in 26 files.
>
> One interesting case is xlog.c that uses the lock to protect 64-bit
> assignments I saw conversations about pg_atomic_u64 for that, but then in
> 32-bit platforms we get this weird 3-field structure everywhere.

What are you referring to with "weird 3-field structure"?

Greetings,

Andres Freund