diff --git a/src/backend/port/atomics.c b/src/backend/port/atomics.c index 42169a33cf..d5970e42b0 100644 --- a/src/backend/port/atomics.c +++ b/src/backend/port/atomics.c @@ -104,6 +104,19 @@ pg_atomic_init_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val_) ptr->value = val_; } +void +pg_atomic_write_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val) +{ + /* + * One might think that an unlocked write doesn't need to acquire the + * spinlock, but one would be wrong. Even an unlocked write has to cause a + * concurrent pg_atomic_compare_exchange_u32() (et al) to fail. + */ + SpinLockAcquire((slock_t *) &ptr->sema); + ptr->value = val; + SpinLockRelease((slock_t *) &ptr->sema); +} + bool pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 *expected, uint32 newval) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 2b63cd3ef1..df4c9d7109 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -821,7 +821,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, Assert(buf_state & BM_VALID); buf_state &= ~BM_VALID; - pg_atomic_write_u32(&bufHdr->state, buf_state); + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); } else { @@ -941,7 +941,7 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum, uint32 buf_state = pg_atomic_read_u32(&bufHdr->state); buf_state |= BM_VALID; - pg_atomic_write_u32(&bufHdr->state, buf_state); + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); } else { @@ -3167,7 +3167,7 @@ FlushRelationBuffers(Relation rel) false); buf_state &= ~(BM_DIRTY | BM_JUST_DIRTIED); - pg_atomic_write_u32(&bufHdr->state, buf_state); + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); /* Pop the error context stack */ error_context_stack = errcallback.previous; diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c index ca2388789d..fa961ad9c8 100644 --- a/src/backend/storage/buffer/localbuf.c +++ b/src/backend/storage/buffer/localbuf.c @@ -138,7 +138,7 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum, if (BUF_STATE_GET_USAGECOUNT(buf_state) < BM_MAX_USAGE_COUNT) { buf_state += BUF_USAGECOUNT_ONE; - pg_atomic_write_u32(&bufHdr->state, buf_state); + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); } } LocalRefCount[b]++; @@ -181,7 +181,7 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum, if (BUF_STATE_GET_USAGECOUNT(buf_state) > 0) { buf_state -= BUF_USAGECOUNT_ONE; - pg_atomic_write_u32(&bufHdr->state, buf_state); + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); trycounter = NLocBuffer; } else @@ -222,7 +222,7 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum, /* Mark not-dirty now in case we error out below */ buf_state &= ~BM_DIRTY; - pg_atomic_write_u32(&bufHdr->state, buf_state); + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); pgBufferUsage.local_blks_written++; } @@ -249,7 +249,7 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum, /* mark buffer invalid just in case hash insert fails */ CLEAR_BUFFERTAG(bufHdr->tag); buf_state &= ~(BM_VALID | BM_TAG_VALID); - pg_atomic_write_u32(&bufHdr->state, buf_state); + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); } hresult = (LocalBufferLookupEnt *) @@ -266,7 +266,7 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum, buf_state |= BM_TAG_VALID; buf_state &= ~BUF_USAGECOUNT_MASK; buf_state += BUF_USAGECOUNT_ONE; - pg_atomic_write_u32(&bufHdr->state, buf_state); + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); *foundPtr = FALSE; return bufHdr; @@ -302,7 +302,7 @@ MarkLocalBufferDirty(Buffer buffer) buf_state |= BM_DIRTY; - pg_atomic_write_u32(&bufHdr->state, buf_state); + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); } /* @@ -351,7 +351,7 @@ DropRelFileNodeLocalBuffers(RelFileNode rnode, ForkNumber forkNum, CLEAR_BUFFERTAG(bufHdr->tag); buf_state &= ~BUF_FLAG_MASK; buf_state &= ~BUF_USAGECOUNT_MASK; - pg_atomic_write_u32(&bufHdr->state, buf_state); + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); } } } @@ -395,7 +395,7 @@ DropRelFileNodeAllLocalBuffers(RelFileNode rnode) CLEAR_BUFFERTAG(bufHdr->tag); buf_state &= ~BUF_FLAG_MASK; buf_state &= ~BUF_USAGECOUNT_MASK; - pg_atomic_write_u32(&bufHdr->state, buf_state); + pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state); } } } diff --git a/src/include/port/atomics.h b/src/include/port/atomics.h index f7884d72c6..4e15ff8764 100644 --- a/src/include/port/atomics.h +++ b/src/include/port/atomics.h @@ -255,10 +255,12 @@ pg_atomic_read_u32(volatile pg_atomic_uint32 *ptr) } /* - * pg_atomic_write_u32 - unlocked write to atomic variable. + * pg_atomic_write_u32 - write to atomic variable. * * The write is guaranteed to succeed as a whole, i.e. it's not possible to - * observe a partial write for any reader. + * observe a partial write for any reader. Note that this correctly interacts + * with pg_atomic_compare_exchange_u32, in contrast to + * pg_atomic_unlocked_write_u32(). * * No barrier semantics. */ @@ -270,6 +272,25 @@ pg_atomic_write_u32(volatile pg_atomic_uint32 *ptr, uint32 val) pg_atomic_write_u32_impl(ptr, val); } +/* + * pg_atomic_unlocked_write_u32 - unlocked write to atomic variable. + * + * The write is guaranteed to succeed as a whole, i.e. it's not possible to + * observe a partial write for any reader. But note that writing this way is + * not guaranteed to correctly interact with read-modify-write operations like + * pg_atomic_compare_exchange_u32. This should only be used in cases where + * minor performance regressions due to atomics emulation are unacceptable. + * + * No barrier semantics. + */ +static inline void +pg_atomic_unlocked_write_u32(volatile pg_atomic_uint32 *ptr, uint32 val) +{ + AssertPointerAlignment(ptr, 4); + + pg_atomic_unlocked_write_u32_impl(ptr, val); +} + /* * pg_atomic_exchange_u32 - exchange newval with current value * diff --git a/src/include/port/atomics/fallback.h b/src/include/port/atomics/fallback.h index bdaa795abe..2290fff196 100644 --- a/src/include/port/atomics/fallback.h +++ b/src/include/port/atomics/fallback.h @@ -133,6 +133,9 @@ pg_atomic_unlocked_test_flag_impl(volatile pg_atomic_flag *ptr) #define PG_HAVE_ATOMIC_INIT_U32 extern void pg_atomic_init_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val_); +#define PG_HAVE_ATOMIC_WRITE_U32 +extern void pg_atomic_write_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val); + #define PG_HAVE_ATOMIC_COMPARE_EXCHANGE_U32 extern bool pg_atomic_compare_exchange_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 *expected, uint32 newval); diff --git a/src/include/port/atomics/generic.h b/src/include/port/atomics/generic.h index 32a01136e6..1acd19242b 100644 --- a/src/include/port/atomics/generic.h +++ b/src/include/port/atomics/generic.h @@ -58,6 +58,15 @@ pg_atomic_write_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val) } #endif +#ifndef PG_HAVE_ATOMIC_UNLOCKED_WRITE_U32 +#define PG_HAVE_ATOMIC_UNLOCKED_WRITE_U32 +static inline void +pg_atomic_unlocked_write_u32_impl(volatile pg_atomic_uint32 *ptr, uint32 val) +{ + ptr->value = val; +} +#endif + /* * provide fallback for test_and_set using atomic_exchange if available */ diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h index e0dfb2f5b0..c7da9f6ac2 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -168,7 +168,8 @@ typedef struct buftag * We use this same struct for local buffer headers, but the locks are not * used and not all of the flag bits are useful either. To avoid unnecessary * overhead, manipulations of the state field should be done without actual - * atomic operations (i.e. only pg_atomic_read/write). + * atomic operations (i.e. only pg_atomic_read_u32() and + * pg_atomic_unlocked_write_u32()). * * Be careful to avoid increasing the size of the struct when adding or * reordering members. Keeping it below 64 bytes (the most common CPU