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

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Matthias van de Meent <boekewurm+postgres@gmail.com>
Cc: pgsql-hackers@postgresql.org, Melanie Plageman <melanieplageman@gmail.com>, Thomas Munro <thomas.munro@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, Noah Misch <noah@leadboat.com>, Robert Haas <robertmhaas@gmail.com>, Michael Paquier <michael.paquier@gmail.com>
Date: 2025-11-20T02:47:49Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Require share-exclusive lock to set hint bits and to flush

  2. lwlock: Remove ForEachLWLockHeldByMe

  3. bufmgr: Implement buffer content locks independently of lwlocks

  4. bufmgr: Change BufferDesc.state to be a 64-bit atomic

  5. heapam: Add batch mode mvcc check and use it in page mode

  6. freespace: Don't modify page without any lock

  7. heapam: Move logic to handle HEAP_MOVED into a helper function

  8. bufmgr: Optimize & harmonize LockBufHdr(), LWLockWaitListLock()

  9. bufmgr: Add one-entry cache for private refcount

  10. bufmgr: Separate keys for private refcount infrastructure

  11. Add pg_atomic_unlocked_write_u64

  12. Rename BUFFERPIN wait event class to BUFFER

  13. bufmgr: Turn BUFFER_LOCK_* into an enum

  14. lwlock: Fix, currently harmless, bug in LWLockWakeup()

  15. bufmgr: Use atomic sub for unpinning buffers

  16. bufmgr: Allow some buffer state modifications while holding header lock

  17. bufmgr: Fix valgrind checking for buffers pinned in StrategyGetBuffer()

  18. bufmgr: Don't lock buffer header in StrategyGetBuffer()

  19. bufmgr: fewer calls to BufferDescriptorGetContentLock

  20. bufmgr: Fix signedness of mask variable in BufferSync()

  21. bufmgr: Introduce FlushUnlockedBuffer

  22. Improve ReadRecentBuffer() scalability

Attachments

Hi,

On 2025-10-09 17:16:49 -0400, Andres Freund wrote:
> On 2025-10-09 16:35:44 -0400, Andres Freund wrote:
> > I pushed a few commits from this patchset after Matthias' review
> > (thanks!). Unfortunately in 5e899859287 I missed that the valgrind annotations
> > would not be done anymore for the buffers returned by
> > StrategyGetBuffer(). Which turned skink red.
> > 
> > The attached 0001 patch centralizes the valgrind initialization in
> > TrackNewBufferPin(), which 5e899859287 had added. The nice side effect of that
> > is that there are fewer VALGRIND_MAKE_MEM_DEFINED() calls than before. The
> > naming isn't the perfect match, but it seems fine to me.
> 
> Forgot to say: I'll push this patch soon, to get skink back to green. Unless
> somebody says something.  We can adjust this later, if the comment and/or
> placement of VALGRIND_MAKE_MEM_DEFINED() isn't to everyones liking.

I have pushed that fix as well as the subsequent buffer header locking changes
a while ago.


Attached is a patchset that actually implements the buffer content locks in
bufmgr.c. This isn't that close to a committable shape yet, but it seemed
useful to get it out there.  The first few patches seem closer, so it'll also
be useful to narrow this down.

0001: A straight-up bugfix in lwlock.c - albeit for a bug that seems currently
	effectively harmless.

0002: Not really required, but seems like an improvement to me

0003: A prerequisite to 0004, pretty boring itself

0004: Use 64bit atomics for BufferDesc.state - at this point nothing uses the
additional bits yet, though.  Some annoying reformatting required to avoid
long lines.

0005: There already was a wait event class for BUFFERPIN. It seems better to
make that more general than to implement them separately.

0006+0007: This is preparatory work for 0008, but also worthwhile on its
own. The private refcount stuff does show up in profiles. The reason it's
related is that without these changes the added information in 0008 makes that
worse.

0008: The main change. Implements buffer content locking independently from
lwlock.c. There's obviously a lot of similarity between lwlock.c code and
this, but I've not found a good way to reduce the duplication without giving
up too much.  This patch does immediately introduce share-exclusive as a new
lock level, mostly because it was too painful to do separately.

0009+0010+0011: Preparatory work for 0012.

0012: One of the main goals of this patchset - use the new share-exclusive
lock level to only allow hint bits to be set while no IO is going on.

0013: Prototype of making UnlockReleaseBuffer() faster and of using it more
widely in nbtree.c

0014: Now that hint bits can't be done while IO is going on, we don't need to
copy pages anymore.  This needs a fair bit more work, as denoted by the FIXMEs
in the code.

I've tried to add detail to the more important commit messages, at least until
0012.


I want to again emphasize that the important commits (i.e. 0008, 0012, 0014)
aren't close to being mergeable. But I think they're in a stage that they
could benefit from "lenient" high-level review.

Greetings,

Andres Freund