On Wed, Feb 18, 2026 at 2:28 PM Nathan Bossart <[email protected]> wrote: > > Starting a new thread from [0]. > > On Wed, Feb 18, 2026 at 02:17:34AM +0200, Heikki Linnakangas wrote: > > On 18/02/2026 01:11, Andres Freund wrote: > >> I think the spinlock functions should also assert this. > > > > +1 > > While working on adding assertions that we are not in a signal handler to > the spinlock functions, I figured I'd take the opportunity to convert the > spin.h macros to static inline functions. This was previously discussed > [1], but AFAICT there was debate over whether to remove S_LOCK and friends > altogether (which doesn't seem to have happened). But IIUC nobody was > horribly opposed to $subject, which I think makes sense to do as > prerequisite work for the aforementioned assertions. >
+1 > However, as soon as I did this, I got a bunch of build failures because > various parts of the code still use volatile qualifiers quite liberally. > It looks like most of these (e.g., see code from commits 2487d872e0, > 966fb05b58, 4bc15a8bfb, and 4db3744f1f) predate making spinlocks compiler > barriers (commit 0709b7ee72) or were cargo-culted from code that predated > it. So, IIUC, it's probably safe to remove these volatile qualifiers now. > We could alternatively add volatile qualifiers to the new static inline > function parameters, but that seems like it might just encourage continued > unnecessary use. > Just wondering if there's some code-path that uses it inside PG_TRY..PG_CATCH that can use longjump. > I also noticed that SpinLockFree() is unused (and apparently never was). > There seems to have been agreement back in 2020 to remove it [2], but it > still exists. I added a prerequisite patch for this, too. > Yes, let's remove it. Regards, -- Fabrízio de Royes Mello
