Hi, On 2024-07-29 12:33:13 -0400, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > On 2024-07-29 11:31:56 -0400, Tom Lane wrote: > >> There was some recent discussion about getting rid of > >> --disable-spinlocks on the grounds that nobody would use > >> hardware that lacked native spinlocks. But now I wonder > >> if there is a testing/debugging reason to keep it. > > > Seems it'd be a lot more straightforward to just add an assertion to the > > x86-64 spinlock implementation verifying that the spinlock isn't already > > free?
FWIW, I quickly hacked that up, and it indeed quickly fails with 0393f542d72^ and passes with 0393f542d72. > I dunno, is that the only extra check that the --disable-spinlocks > implementation is providing? I think it also provides the (valuable!) check that spinlocks were actually initialized. But that again seems like something we'd be better off adding more general infrastructure for - nobody runs --disable-spinlocks locally, we shouldn't need to run this on the buildfarm to find problems like this. > I'm kind of allergic to putting Asserts into spinlocked code segments, > mostly on the grounds that it violates the straight-line-code precept. > I suppose it's not really that bad for tests that you don't expect > to fail, but still ... I don't think the spinlock implementation itself is really affected by that rule - after all, the --disable-spinlocks implementation actually consists out of several layers of external function calls (including syscalls in some cases!). Greetings, Andres Freund
