Horton

Re: Atomic operations within spinlocks

Thomas Munro <thomas.munro@gmail.com>

View thread View on postgr.es
From: Thomas Munro <thomas.munro@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2020-06-03T21:40:31Z
Lists: pgsql-hackers 
On Thu, Jun 4, 2020 at 8:45 AM Andres Freund <andres@anarazel.de> wrote:
> On 2020-06-03 14:19:45 -0400, Tom Lane wrote:
> > In the second place, this coding seems to me to indicate serious
> > confusion about which lock is protecting what.  In the above example,
> > if writtenUpto is only accessed through atomic operations then it
> > seems like we could just move the pg_atomic_read_u64 out of the
> > spinlock section; or if the spinlock is adequate protection then we
> > could just do a normal fetch.  If we actually need both locks then
> > this needs significant re-thinking, IMO.
>
> Yea, it doesn't seem necessary at all that writtenUpto is read with the
> spinlock held. That's very recent:
>
> commit 2c8dd05d6cbc86b7ad21cfd7010e041bb4c3950b
> Author: Michael Paquier <michael@paquier.xyz>
> Date:   2020-05-17 09:22:07 +0900
>
>     Make pg_stat_wal_receiver consistent with the WAL receiver's shmem info
>
> and I assume just was caused by mechanical replacement, rather than
> intentionally doing so while holding the spinlock. As far as I can tell
> none of the other writtenUpto accesses are under the spinlock.

Yeah.  It'd be fine to move that after the spinlock release.  Although
it's really just for informational purposes only, not for any data
integrity purpose, reading it before the spinlock acquisition would
theoretically allow it to appear to be (reportedly) behind
flushedUpto, which would be silly.

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.