Re: Buffer locking is special (hints, checksums, AIO writes) - Mailing list 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
significant32bit will be used for buffer locking as stated in buf_internals.h, in other words, there is no room for
liftingthe 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/