Horton
Menu

Re: Buffer locking is special (hints, checksums, AIO writes)

Yura Sokolov <y.sokolov@postgrespro.ru>

View thread
From: Yura Sokolov <y.sokolov@postgrespro.ru>
To: Andres Freund <andres@anarazel.de>, Melanie Plageman <melanieplageman@gmail.com>
Cc: Noah Misch <noah@leadboat.com>, Heikki Linnakangas <hlinnaka@iki.fi>, Kirill Reshke <reshkekirill@gmail.com>, Matthias van de Meent <boekewurm+postgres@gmail.com>, pgsql-hackers@postgresql.org, Thomas Munro <thomas.munro@gmail.com>, Robert Haas <robertmhaas@gmail.com>, Michael Paquier <michael.paquier@gmail.com>
Date: 2026-03-31T16:02:33Z
Lists: pgsql-hackers 
27.03.2026 23:00, Andres Freund wrote:
> Hi,
> 
> On 2026-03-25 18:35:55 -0400, Andres Freund wrote:
>> Running it through valgrind and then will work on reading through one more
>> time and pushing them.
> 
> And done.
> 
> Phew, this project took way longer than I'd though it'd take.

In addition to bug with BM_IO_ERROR [1] , I found race condition in
PinBuffer in this lines of code:

	if (unlikely(skip_if_not_valid && !(old_buf_state & BM_VALID)))
		return false;

	/*
	 * We're not allowed to increase the refcount while the buffer
	 * header spinlock is held. Wait for the lock to be released.
	 */
	if (old_buf_state & BM_LOCKED)
		old_buf_state = WaitBufHdrUnlocked(buf);

While we waited for buffer header for being unlocked, it may become
invalid, isn't it?
Therefore, check related to skip_if_not_valid have to happen after waiting.

....

Another question: previously we had to wait for buffer for being unlocked
because UnlockBufHdr wrote to buf->state unconditionally, therefore our pin
increment could be lost.
Now UnlockBufHdr and UnlockBufHdrExt does proper atomic operations and
preserves concurrent changes. Are we still need to wait?

Most of time PinBuffer is called protected by BufTable's partition LWLock,
therefore buffer may not be changed in dramatic way.

But call in ReadRecentBuffer is the exception. It is not protected by
partition lock and have to make additional checks. That is why you
introduced skip_if_not_valid.

Does optimization of ReadRecentBuffer pays for WaitBufHdrUnlocked?

[1]
https://www.postgresql.org/message-id/57a8ea70-32a8-4596-bb68-5d2990b83380%40postgrespro.ru

-- 
regards
Yura Sokolov aka funny-falcon

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. bufmgr: Fix ordering of checks in PinBuffer()

  2. Use UnlockReleaseBuffer() in more places

  3. bufmgr: Make UnlockReleaseBuffer() more efficient

  4. bufmgr: Don't copy pages while writing out

  5. Fix use of wrong variable in _hash_kill_items()

  6. Fix bug due to confusion about what IsMVCCSnapshot means

  7. bufmgr: Switch to standard order in MarkBufferDirtyHint()

  8. bufmgr: Remove the, now obsolete, BM_JUST_DIRTIED

  9. Require share-exclusive lock to set hint bits and to flush

  10. heapam: Don't mimic MarkBufferDirtyHint() in inplace updates

  11. bufmgr: Allow conditionally locking of already locked buffer

  12. bufmgr: Avoid spurious compiler warning after fcb9c977aa5

  13. lwlock: Remove ForEachLWLockHeldByMe

  14. lwlock: Remove support for disowned lwlwocks

  15. bufmgr: Implement buffer content locks independently of lwlocks

  16. bufmgr: Change BufferDesc.state to be a 64-bit atomic

  17. lwlock: Improve local variable name

  18. lwlock: Invert meaning of LW_FLAG_RELEASE_OK

  19. bufmgr: Make definitions related to buffer descriptor easier to modify

  20. heapam: Add batch mode mvcc check and use it in page mode

  21. freespace: Don't modify page without any lock

  22. heapam: Move logic to handle HEAP_MOVED into a helper function

  23. bufmgr: Optimize & harmonize LockBufHdr(), LWLockWaitListLock()

  24. bufmgr: Add one-entry cache for private refcount

  25. bufmgr: Separate keys for private refcount infrastructure

  26. Add pg_atomic_unlocked_write_u64

  27. Rename BUFFERPIN wait event class to BUFFER

  28. bufmgr: Turn BUFFER_LOCK_* into an enum

  29. lwlock: Fix, currently harmless, bug in LWLockWakeup()

  30. bufmgr: Use atomic sub for unpinning buffers

  31. bufmgr: Allow some buffer state modifications while holding header lock

  32. bufmgr: Fix valgrind checking for buffers pinned in StrategyGetBuffer()

  33. bufmgr: Don't lock buffer header in StrategyGetBuffer()

  34. bufmgr: fewer calls to BufferDescriptorGetContentLock

  35. bufmgr: Fix signedness of mask variable in BufferSync()

  36. bufmgr: Introduce FlushUnlockedBuffer

  37. Improve ReadRecentBuffer() scalability