upstream/postgresql
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2026-07-16 05:02:57 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

bufmgr: Fix order of operations in UnlockBufHdrExt

Browse source 
In c75ebc657f I (Andres) introduced UnlockBufHdrExt() which can set and
clear bits in the buffer state using CAS. Unfortunately I added bits before
subtracting them, which means that a bit that was both removed and set would
remain unset.  Fix the order of operations.

The only known case where that is a problem is that BM_IO_ERROR would not
actually remain set.

It's unfortunately not trivial to add a decent, race-free, test to verify that
BM_IO_ERROR remains set. That's therefore left for the 20 cycle.

Reported-by: Yura Sokolov <y.sokolov@postgrespro.ru>
Discussion: https://postgr.es/m/ab0dcc9e-aba0-44e3-ac23-8d74c48888e6@postgrespro.ru
Backpatch-through: 19, where c75ebc657f went in
This commit is contained in:
Andres Freund 2026-07-10 13:25:13 -04:00
parent 133eba078f
commit a1b74b7279
1 changed files with 11 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
src/include/storage/buf_internals.h
View file

@ -463,12 +463,13 @@ UnlockBufHdr(BufferDesc *desc)
}
/*
* Unlock the buffer header, while atomically adding the flags in set_bits,
* unsetting the ones in unset_bits and changing the refcount by
* refcount_change.
* Unlock the buffer header, while atomically unsetting the ones in
* unset_bits, adding the flags in set_bits, and changing the refcount by
* refcount_change. If a flag is both cleared and added, it will end up being
* set.
*
* Note that this approach would not work for usagecount, since we need to cap
* the usagecount at BM_MAX_USAGE_COUNT.
* Note that this approach would not trivially work for usagecount, since we
* need to cap the usagecount at BM_MAX_USAGE_COUNT.
*/
static inline uint64
UnlockBufHdrExt(BufferDesc *desc, uint64 old_buf_state,
@ -481,8 +482,12 @@ UnlockBufHdrExt(BufferDesc *desc, uint64 old_buf_state,
Assert(buf_state & BM_LOCKED);
buf_state |= set_bits;
/*
* Set bits after clearing bits, so that a cleared and re-added flag
* survives.
*/
buf_state &= ~unset_bits;
buf_state |= set_bits;
buf_state &= ~BM_LOCKED;
if (refcount_change != 0)