public inbox for [email protected]  
help / color / mirror / Atom feed
From: Greg Burd <[email protected]>
To: Andres Freund <[email protected]>
Cc: Matthias van de Meent <[email protected]>
Cc: =?utf-8?Q?pgsql-hackers=40postgresql.org?= <[email protected]>
Cc: Melanie Plageman <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: Noah Misch <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Michael Paquier <[email protected]>
Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
Date: Thu, 20 Nov 2025 14:08:57 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <6rgb2nvhyvnszz4ul3wfzlf5rheb2kkwrglthnna7qhe24onwr@vw27225tkyar>
References: <6rgb2nvhyvnszz4ul3wfzlf5rheb2kkwrglthnna7qhe24onwr@vw27225tkyar>


On Nov 19 2025, at 9:47 pm, Andres Freund <[email protected]> wrote:

> 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.

Hello Andres,

After talking to you about these ideas at PGConf in NYC I've been
anxiously awaiting this patch set. Thanks for dedicating the energy and
time to get it to this stage.

High level feedback after reading the patches/email/commit messages is
that it looks to get you to where you wanted to be, unblocking AIO
writes. I think they'll end up being faster than what's in place now,
even before you get to the AIO piece.  Certainly removing the copy of
each page to do a checksum will help.  Opening the door to a future
where we can have super-pinned/locked pages is also a net win.

Everything before/after 0008 was rather easy to digest and understand
and I found nothing really to call out at this stage

0008 is understandable too, it's just sizable. While it is large, I find
it well laid out and more readable than before.  I gave the locking code
a good look, it seems correct AFAICT.

Keep going, I'll be happy to dedicate time to testing and digging into
the commits as you get this into a final state.  I look forward to
extending/enhancing this code once integrated.

> 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

cheers.

-greg





view thread (57+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Buffer locking is special (hints, checksums, AIO writes)
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox