Re: missing PG_IO_ALIGN_SIZE uses

> On Dec 1, 2025, at 15:55, Peter Eisentraut <peter(at)eisentraut(dot)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 immediately obvious how, since the comment also mentions that it wants alignment on "a full xlog block size boundary". Maybe Max(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 if we 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 a consistent syntax.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/