From: Nathan Bossart <nathandbossart@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Subject: Re: convert SpinLock* macros to static inline functions
Date: Thu, 19 Feb 2026 10:08:20 -0600
Message-ID: <aZc1dO205sbKvkZp@nathan> (raw)
In-Reply-To: <wbbl44cvhsbt5nm2yi7e67mpgztn56j65yznlkkzlcp3sriy6f@t4p5e6knf5hv>
References: <aZX2oUcKf7IzHnnK@nathan>
<wbbl44cvhsbt5nm2yi7e67mpgztn56j65yznlkkzlcp3sriy6f@t4p5e6knf5hv>
Thanks for looking.
On Wed, Feb 18, 2026 at 04:23:28PM -0500, Andres Freund wrote:
> Not quite as sure it's the right thing to remove it from SpinLockAcquire()
> though. I think removing it more widely would likely cause issues, e.g. if you
> removed it from s_lock(), the compiler would be in its right to just turn:
>
> int
> s_lock(volatile slock_t *lock, const char *file, int line, const char *func)
> ...
> while (TAS_SPIN(lock))
> {
> perform_spin_delay(&delayStatus);
> }
> ...
>
> into
> ...
> if (*lock)
> {
> while (true)
> perform_spin_delay(&delayStatus);
> }
> else
> TAS(lock);
> ...
>
> as without the volatile, or a compiler barrier, the compiler can assume that
> nothing will change the the value of *lock (at least theoretically, if it can
> prove that perform_spin_delay() doesn't change the value of *lock).
This crossed my mind, but I couldn't see how it could introduce any issues
that don't already exist. Both tas() and s_lock() have the lock parameter
marked as volatile, and S_LOCK is often expanded into functions where the
lock variable is not marked volatile. Why would those existing cases not
be subject to this problem but the static inline function would? And why
would marking SpinLockAcquire()'s lock parameter volatile fix it if doing
so for tas() and s_lock() doesn't? FWIW I don't see any changes to the
code generated for s_lock() with these patches, although I only looked on a
couple of machines.
Perhaps it's still worth doing out of an abundance of caution, or maybe I
am missing something subtle about the liberties that compilers take in this
area...
--
nathan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, andres@anarazel.de, hlinnaka@iki.fi
Subject: Re: convert SpinLock* macros to static inline functions
In-Reply-To: <aZc1dO205sbKvkZp@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox