Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81.1.4\))
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: <jtg5cu4n6h5lib3kzx66ju4yhh6kmviaud7oq6dtut6c4q4rdi@xwsfoagt3c2b>
Date: Wed, 14 Jan 2026 11:41:19 +0800
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>
Content-Transfer-Encoding: quoted-printable
Message-Id: <9C2494E1-F446-42DF-B070-7E231B8E6221@gmail.com>
References: <lneuyxqxamqoayd2ntau3lqjblzdckw6tjgeu4574ezwh4tzlg@noioxkquezdw>
 <c00d4a9f-c0f8-49ca-bf27-29c6641f2fc6@iki.fi>
 <q3ybyaiownp4ivt6xfy55s5llvtq6vqsd4zbt5kn6hacxdcpp4@2ngd4h4bev5d>
 <dsbacri4xldlobl3jbqqlksefsyrwj7lzljbohxsbqjus3z4o7@otb6rge4w7az>
 <1108f18d-cf7c-4f17-b29c-a119fe42f7e5@iki.fi>
 <5dwlfu2jyzkyf3nrlzxxblxctb6xio5es73ptgsahjnmfu5miu@772rc764hfhi>
 <ossv2eistssmubfsir6xjll76tynvxv5lup4zkrfzjkud7fycw@rf5vii6l6cha>
 <4csodkvvfbfloxxjlkgsnl2lgfv2mtzdl7phqzd4jxjadxm4o5@usw7feyb5bzf>
 <CALdSSPgyc7VuMLUZ8J7v2G-PebBa_vEi+mp-cLkcOwNycB56Hw@mail.gmail.com>
 <kjdgvws34gpnz4y6xu5aaeul5mspgy3ahvyiw4lndb3ecsacdb@b2ccyowotpqh>
 <jtg5cu4n6h5lib3kzx66ju4yhh6kmviaud7oq6dtut6c4q4rdi@xwsfoagt3c2b>
To: Andres Freund <andres@anarazel.de>
Archived-At: <https://www.postgresql.org/message-id/9C2494E1-F446-42DF-B070-7E231B8E6221%40gmail.com>
Precedence: bulk


> On Jan 13, 2026, at 08:33, Andres Freund <andres@anarazel.de> wrote:
>=20
> Hi,
>=20
> 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.
>=20
> Done, after adjust two comments in minor ways.
>=20
>=20
>> Also working on doing comment polishing of the later patches, found a =
few
>> things, but not quite enough to be worth reposting yet.
>=20
> Here are the remaining commits, with a bit of polish:
>=20
> - fixed references to old names in some places (lwlocks, release_ok)
>=20
> - Aded an assert that we don't already hold a lock in =
BufferLockConditional()
>=20
> - typo and grammar fixes
>=20
> - updated the commit message of the LW_FLAG_RELEASE_OK, as "requested" =
by
>  Melanie. I hope this explains the situation better.
>=20
> - added a commit that renames ResOwnerReleaseBufferPin to
>  ResOwnerReleaseBuffer (et al), as it now also releases content locks =
if held
>=20
>  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.
>=20
> Greetings,
>=20
> Andres Freund
> =
<v10-0001-lwlock-Invert-meaning-of-LW_FLAG_RELEASE_OK.patch><v10-0002-bufm=
gr-Make-definitions-related-to-buffer-descri.patch><v10-0003-bufmgr-Change=
-BufferDesc.state-to-be-a-64-bit-at.patch><v10-0004-bufmgr-Implement-buffe=
r-content-locks-independen.patch><v10-0005-Require-share-exclusive-lock-to=
-set-hint-bits-an.patch><v10-0006-WIP-Make-UnlockReleaseBuffer-more-effici=
ent.patch><v10-0007-WIP-bufmgr-Don-t-copy-pages-while-writing-out.patch><v=
10-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=E2=80=99s true, then =
I think we can remove the statements about lifting limitation.

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

		CHECK_FOR_INTERRUPTS();

		bufHdr =3D GetBufferDescriptor(i);

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

I will continue reviewing 0004 tomorrow.


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