Re: Use PGAlignedBlock instead of "char buf[BLCKSZ]" in more places

Hi,

On 2023-12-04 15:53:44 +0100, Peter Eisentraut wrote:
> On 04.12.23 06:46, Michael Paquier wrote:
> > On Mon, Dec 04, 2023 at 06:59:13AM +0530, Bharath Rupireddy wrote:
> > > The commit 44cac934 replaced "char buf[BLCKSZ]" with PGAlignedBlock to
> > > avoid issues on alignment-picky hardware. While it replaced most of the
> > > instances, there are still some more left. How about we use PGAlignedBlock
> > > there too, something like the attached patch? A note [2] in the commit
> > > 44cac934 says that ensuring proper alignment makes kernel data transfers
> > > fasters and the left-over "char buf[BLCKSZ]" either do read() or write()
> > > system calls, so it might be worth to align them with PGAlignedBlock.
> > >
> > > Thoughts?
> >
> > The buffers used to write the lock file and the TLI history file are
> > not page buffers, and this could make code readers think that these
> > are pages.
>
> The type is called "aligned block", not "aligned buffer" or "aligned page",
> so I don't think it's incorrect to try to use it.

Block is a type defined in bufmgr.h...

> So I am honestly not sure if there's a point in changing
> > them because the current code is not incorrect, isn't it? It looks
> > like 2042b3428d39 for the TLI history file and 52948169bcdd for the
> > lock file began using BLCKSZ because that was just a handy thing to
> > do, and because we know they would never get beyond that.
>
> Yeah, it's not clear why these need to be block-sized. We shouldn't
> perpetuate this without more clarity about this.

If we change something, we should consider making buffers like these aligned
to page sizes, rather than just MAXALIGNED.

Greetings,

Andres Freund