pgsql: Fix fallback implementation of pg_atomic_write_u32().

From: Andres Freund <andres(at)anarazel(dot)de>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Fix fallback implementation of pg_atomic_write_u32().
Date: 2016-10-08 00:07:23
Message-ID: E1bsfAh-0001b8-NE@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Fix fallback implementation of pg_atomic_write_u32().

I somehow had assumed that in the spinlock (in turn possibly using
semaphores) based fallback atomics implementation 32 bit writes could be
done without a lock. As far as the write goes that's correct, since
postgres supports only platforms with single-copy atomicity for aligned
32bit writes. But writing without holding the spinlock breaks
read-modify-write operations like pg_atomic_compare_exchange_u32(),
since they'll potentially "miss" a concurrent write, which can't happen
in actual hardware implementations.

In 9.6+ when using the fallback atomics implementation this could lead
to buffer header locks not being properly marked as released, and
potentially some related state corruption. I don't see a related danger
in 9.5 (earliest release with the API), because pg_atomic_write_u32()
wasn't used in a concurrent manner there.

The state variable of local buffers, before this change, were
manipulated using pg_atomic_write_u32(), to avoid unnecessary
synchronization overhead. As that'd not be the case anymore, introduce
and use pg_atomic_unlocked_write_u32(), which does not correctly
interact with RMW operations.

This bug only caused issues when postgres is compiled on platforms
without atomics support (i.e. no common new platform), or when compiled
with --disable-atomics, which explains why this wasn't noticed in
testing.

Reported-By: Tom Lane
Discussion: <14947(dot)1475690465(at)sss(dot)pgh(dot)pa(dot)us>
Backpatch: 9.5-, where the atomic operations API was introduced.

Branch
------
master

Details
-------
http://git.postgresql.org/pg/commitdiff/b0779abb3add11d4dd745779dd81ea8ecdd00a1d

Modified Files
--------------
src/backend/port/atomics.c | 13 +++++++++++++
src/backend/storage/buffer/bufmgr.c | 6 +++---
src/backend/storage/buffer/localbuf.c | 16 ++++++++--------
src/include/port/atomics.h | 25 +++++++++++++++++++++++--
src/include/port/atomics/fallback.h | 3 +++
src/include/port/atomics/generic.h | 9 +++++++++
src/include/storage/buf_internals.h | 3 ++-
7 files changed, 61 insertions(+), 14 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Andres Freund 2016-10-08 00:07:24 pgsql: Fix fallback implementation of pg_atomic_write_u32().
Previous Message Tom Lane 2016-10-07 23:58:58 Re: [COMMITTERS] pgsql: Remove -Wl,-undefined,dynamic_lookup in macOS build.