Horton
Menu

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

Chao Li <li.evan.chao@gmail.com>

View thread
From: Chao Li <li.evan.chao@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Kirill Reshke <reshkekirill@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, Melanie Plageman <melanieplageman@gmail.com>, Matthias van de Meent <boekewurm+postgres@gmail.com>, pgsql-hackers@postgresql.org, Thomas Munro <thomas.munro@gmail.com>, Noah Misch <noah@leadboat.com>, Robert Haas <robertmhaas@gmail.com>, Michael Paquier <michael.paquier@gmail.com>
Date: 2026-01-14T03:41:19Z
Lists: pgsql-hackers 

> On Jan 13, 2026, at 08:33, Andres Freund <andres@anarazel.de> wrote:
> 
> Hi,
> 
> On 2026-01-12 12:45:03 -0500, Andres Freund wrote:
>> I'm doing another pass through 0003 and will push that if I don't find
>> anything significant.
> 
> Done, after adjust two comments in minor ways.
> 
> 
>> Also working on doing comment polishing of the later patches, found a few
>> things, but not quite enough to be worth reposting yet.
> 
> Here are the remaining commits, with a bit of polish:
> 
> - fixed references to old names in some places (lwlocks, release_ok)
> 
> - Aded an assert that we don't already hold a lock in BufferLockConditional()
> 
> - typo and grammar fixes
> 
> - updated the commit message of the LW_FLAG_RELEASE_OK, as "requested" by
>  Melanie. I hope this explains the situation better.
> 
> - added a commit that renames ResOwnerReleaseBufferPin to
>  ResOwnerReleaseBuffer (et al), as it now also releases content locks if held
> 
>  I kept this separate as I'm not yet sure about the new name, partially due
>  to there also being a "buffer io" resowner.  I tried "buffer ownership" for
>  the resowner that tracks pins and locks, but that was long and not clearly
>  better.
> 
> Greetings,
> 
> Andres Freund
> <v10-0001-lwlock-Invert-meaning-of-LW_FLAG_RELEASE_OK.patch><v10-0002-bufmgr-Make-definitions-related-to-buffer-descri.patch><v10-0003-bufmgr-Change-BufferDesc.state-to-be-a-64-bit-at.patch><v10-0004-bufmgr-Implement-buffer-content-locks-independen.patch><v10-0005-Require-share-exclusive-lock-to-set-hint-bits-an.patch><v10-0006-WIP-Make-UnlockReleaseBuffer-more-efficient.patch><v10-0007-WIP-bufmgr-Don-t-copy-pages-while-writing-out.patch><v10-0008-WIP-bufmgr-Rename-ResOwnerReleaseBufferPin.patch>

A couple of comments on v10-0003, I just noticed 0001 and 0002 have been pushed.

Basically, code changes in 0003 is straightforward, just a couple of small comments:

1
```
- * refcounts in buf_internals.h.  This limitation could be lifted by using a
- * 64bit state; but it's unlikely to be worthwhile as 2^18-1 backends exceed
- * currently realistic configurations. Even if that limitation were removed,
- * we still could not a) exceed 2^23-1 because inval.c stores the ProcNumber
- * as a 3-byte signed integer, b) INT_MAX/4 because some places compute
- * 4*MaxBackends without any overflow check.  We check that the configured
- * number of backends does not exceed MAX_BACKENDS in InitializeMaxBackends().
+ * refcounts in buf_internals.h.  This limitation could be lifted, but it's
```

Before this patch, there was room for lifting the limitation. With this patch, state is 64bit already, but the significant 32bit will be used for buffer locking as stated in buf_internals.h, in other words, there is no room for lifting the limitation now. If that’s true, then I think we can remove the statements about lifting limitation.

2. By searching for “LockBufHdr”, I found one place missed to update in contrib/pg_prewarm/autoprewarm.c at line 706:
```
	for (num_blocks = 0, i = 0; i < NBuffers; i++)
	{
		uint32		buf_state; <=== line 706, should change to uint64

		CHECK_FOR_INTERRUPTS();

		bufHdr = GetBufferDescriptor(i);

		/* Lock each buffer header before inspecting. */
		buf_state = LockBufHdr(bufHdr);
```

I will continue reviewing 0004 tomorrow.


Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

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