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

> On Jan 13, 2026, at 08:33, Andres Freund <andres(at)anarazel(dot)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/