On Thu, Aug 1, 2024 at 10:38 AM Andres Freund <andres@anarazel.de> wrote:
> On 2024-08-01 10:09:07 +1200, Thomas Munro wrote:
> > On Thu, Aug 1, 2024 at 7:07 AM Andres Freund <andres@anarazel.de> wrote:
> > > Note that I would like to add a user for S_LOCK_FREE(), to detect repeated
> > > SpinLockRelease():
> > > https://postgr.es/m/20240729182952.hua325647e2ggbsy%40awork3.anarazel.de
> >
> > What about adding a "magic" member in assertion builds? Here is my
> > attempt at that, in 0002.
>
> That changes the ABI, which we don't want, because it breaks using
> extensions against a differently built postgres.
Yeah, right, bad idea. Let me think about how to do something like
what you showed, but with the atomics patch...
Hmm. One of the interesting things about the atomic_flag interface is
that it completely hides the contents of memory. (Guess: its weird
minimal interface was designed to help weird architectures like
PA-RISC, staying on topic for $SUBJECT; I doubt we'll see such a
system again but it's useful for this trick). So I guess we could
push the check down to that layer, and choose arbitrary non-zero
values for the arch-x86.h implementation of pg_atomic_flag . See
attached. Is this on the right track?
(Looking ahead, if we eventually move to using <stdatomic.h>, we won't
be able to use atomic_flag due to lack of relaxed load anyway, so we
could generalise this to atomic_char (rather than atomic_bool), and
keep using non-zero values. Presumably at that point we could also
decree that zero-initialised memory is valid for initialising our
spinlocks, but it seems useful as a defence against uninitialised
objects anyway.)
> I don't really see a reason to avoid having S_LOCK_FREE(), am I missing
> something? Previously the semaphore fallback was a reason, but that's gone
> now...
Sure, but if it's just for assertions, we don't need it. Or any of
the S_XXX stuff.
