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
