Horton

Re: global barrier & atomics in signal handlers (Re: Atomic operations within spinlocks)

Andres Freund <andres@anarazel.de>

View thread View on postgr.es
From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Tom Lane <tgl@sss.pgh.pa.us>
Date: 2020-06-17T18:33:54Z
Lists: pgsql-hackers 
Hi,

On 2020-06-17 10:34:31 -0400, Robert Haas wrote:
> On Tue, Jun 16, 2020 at 3:28 PM Andres Freund <andres@anarazel.de> wrote:
> > > I think 0003 looks a little strange: it seems to be
> > > testing things that might be implementation details of other things,
> > > and I'm not sure that's really correct. In particular:
> > Hm? Isn't s_lock the, as its comment says, "platform-independent portion
> > of waiting for a spinlock."?  I also don't think we need to purely
> > follow external APIs in internal tests.
> 
> I feel like we at least didn't use to use that on all platforms, but I
> might be misremembering.

There's only one definition of S_LOCK, and s_lock is the only spinlock
related user of perform_spin_delay(). So I don't think so?


> It seems odd and confusing that we have  both
> S_LOCK() and s_lock(), anyway. Differentiating functions based on case
> is not great practice.

It's a terrible idea, yes.  Since we don't actually have any non-default
implementations of S_LOCK, perhaps we should just rip it out? It'd
probably be clearer if SpinLockAcquire() would be what uses TAS() and
falls back to s_lock (best renamed to s_lock_slowpath or such).

It'd perhaps also be good to make SpinLockAcquire() a static inline
instead of a #define, so it can be properly attributed in debuggers and
profilers.

Greetings,

Andres Freund

Commits

  1. Convert SpinLock* macros to static inline functions.

  2. Clean up includes of s_lock.h.

  3. Fix deadlock danger when atomic ops are done under spinlock.

  4. Add basic spinlock tests to regression tests.

  5. spinlock emulation: Fix bug when more than INT_MAX spinlocks are initialized.

  6. Avoid potential spinlock in a signal handler as part of global barriers.

  7. Make pg_stat_wal_receiver consistent with the WAL receiver's shmem info

  8. Improve 64bit atomics support.