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-15T00:04:21Z
Lists: pgsql-hackers 

> On Jan 15, 2026, at 07:37, Andres Freund <andres@anarazel.de> wrote:
> 
> Hi,
> 
> On 2026-01-15 07:20:27 +0800, Chao Li wrote:
>>> On Jan 15, 2026, at 00:30, Andres Freund <andres@anarazel.de> wrote:
>>> On 2026-01-14 11:41:19 +0800, Chao Li wrote:
>>>> 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.
>>> 
>>> I'm not following - there's plenty space for more bits if we need that:
>>> 
>>> * State of the buffer itself (in order):
>>> * - 18 bits refcount
>>> * - 4 bits usage count
>>> * - 12 bits of flags
>>> * - 18 bits share-lock count
>>> * - 1 bit share-exclusive locked
>>> * - 1 bit exclusive locked
>>> 
>>> That's 54 bits in total. Which part is in the lower and which in the upper
>>> 32bit isn't relevant for anything afaict?
>> 
>> Because I saw the comment in buf_internals.h:
>> ```
>> * NB: A future commit will use a significant portion of the remaining bits to
>> * implement buffer locking as part of the state variable.
>> ```
>> That seems to indicate all the significant 32 bits will be used for buffer locking.
> 
> A significant portion != all. As the above excerpt from the comment shows, the
> locking uses 20 bits. We could increase max backends by 5 bits without running
> out of bits (we'd need space both in the refcount bitspace as well as the
> share-lock bitspace).

Make sense. I think I misread the comment.

> 
> 
>> Also, there is an assert that concretes the impression:
>> ```
>> StaticAssertDecl(BUF_REFCOUNT_BITS + BUF_USAGECOUNT_BITS + BUF_FLAG_BITS == 32,
>>       "parts of buffer state space need to equal 32");
>> ```
> 
> You can see that being relaxed in the subsequent commit, when we start to use
> more bits.
> 

Sure. I plan to review 0003-0005 today. I believe I will get better understanding.

So, 0001 and 0002 LGTM now.

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