> On Dec 1, 2025, at 15:55, Peter Eisentraut <peter@eisentraut.org> wrote:
>
> Commit faeedbcefd4 changed the alignment of WAL buffers from XLOG_BLCKSZ to PG_IO_ALIGN_SIZE.
>
> While looking around for places to apply alignas, I think I found at least two places that were forgotten, namely in
BootStrapXLOG()and in pg_test_fsync.c. Patches attached for those.
>
> I also suspect that the TYPEALIGN call in XLOGShmemInit() should take PG_IO_ALIGN_SIZE into account, but it's not
immediatelyobvious how, since the comment also mentions that it wants alignment on "a full xlog block size boundary".
MaybeMax(XLOG_BLCKSZ, PG_IO_ALIGN_SIZE)?
Agreed, in order to keep both full xlog block and direct-I/O alignment the same.
>
> I also wonder whether the check in check_debug_io_direct() for #if XLOG_BLCKSZ < PG_IO_ALIGN_SIZE would be required
ifwe fixed all those places?
Also agreed. Once every WAL buffer aligns by PG_IO_SIZE, the check will become redundant.
>
>
<0001-Use-PG_IO_ALIGN_SIZE-for-aligning-WAL-buffers.patch><0002-Use-PGAlignedXLogBlock-in-BootStrapXLOG.patch><0003-pg_test_fsync-Align-test-data-using-PGAlignedXLogBlo.patch>
Overall the patch looks good to me:
0001 fixes two overlooked alignment to PG_IO_ALIGN_SIZE
0002 switches BootStrapXLOG to use PGAlignedXLogBlock, which is aligned to PG_IO_ALIGN_SIZE
0003 does the same for pg_test_fsync
My only nit comment is in 0002:
```
- memset(page, 0, XLOG_BLCKSZ);
+ memset(&buffer, 0, sizeof buffer);
```
I know “sizeof” is an operator instead of a function, “sizeof buffer” is grammatically correct. However, most of places
do“sizeof(buffer)”, so unless we want to prompt the syntax of “sizeof buffer” (without the braces), it’s better to keep
aconsistent syntax.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
