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>

Hi Andres,

So far I’ve only reviewed 0001 and 0002. I’m not very familiar with this area, so the review has been a bit slow.

Overall, 0001 looks good to me. It renames LW_FLAG_RELEASE_OK to LW_FLAG_WAKE_IN_PROGRESS and inverts the meaning, which makes sense. I only have a small nit on naming: the local variable “new_release_in_progress". I see that it’s inherited from the old name and was updated from “_ok" to “_in_progress", but now that the flag itself is renamed, would it make sense to rename the variable as well? Something like “wake_in_progress" or “new_wake_in_progress" might better reflect the new flag name.

In 0002, a bunch of new macros are introduced. My initial impression wasn’t great, mostly due to the amount of line wrapping. Looking a bit closer, I also noticed some duplication, for example, "BUF_REFCOUNT_BITS + BUF_USAGECOUNT_BITS" appears more than once; and a small inconsistency between BUF_STATE_GET_REFCOUNT and BUF_STATE_GET_USAGECOUNT (even though the former doesn’t actually need a shift).

I tried a small refactor of the macro definitions in the attached diff to see if things could be made a bit more regular. It introduces a helper macro MASK() and a BUF_REFCOUNT_SHIFT constant, and removes a bit of duplication. If you like it, feel free to take it; otherwise, please just ignore it. Note that, the diff is based on 0002.

(I actually hesitated to attach a diff, because if you’ve already created a CF entry, the attached diff could break the CI tests. If that happens, sorry about that.)

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