From 5865150b6d535ecea241e9ad3038564cb7768b9a Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 27 Aug 2025 19:12:11 -0400 Subject: aio: Stop using enum bitfields due to bad code generation During an investigation into rather odd aio related errors on macos, observed by Alexander and Konstantin, we started to wonder if bitfield access is related to the error. At the moment it looks like it is related, we cannot reproduce the failures when replacing the bitfields. In addition, the problem can only be reproduced with some compiler [versions] and not everyone has been able to reproduce the issue. The observed problem is that, very rarely, PgAioHandle->{state,target} are in an inconsistent state, after having been checked to be in a valid state not long before, triggering an assertion failure. Unfortunately, this could be caused by wrong compiler code generation or somehow of missing memory barriers - we don't really know. In theory there should not be any concurrent write access to the handle in the state the bug is triggered, as the handle was idle and is just being initialized. Separately from the bug, we observed that at least gcc and clang generate rather terrible code for the bitfield access. Even if it's not clear if the observed assertion failure is actually caused by the bitfield somehow, the bad code generation alone is sufficient reason to stop using bitfields. Therefore, replace the enum bitfields with uint8s and instead cast in each switch statement. Reported-by: Alexander Lakhin Reported-by: Konstantin Knizhnik Discussion: https://postgr.es/m/1500090.1745443021@sss.pgh.pa.us Backpatch-through: 18 --- src/backend/storage/aio/aio_io.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/backend/storage/aio/aio_io.c') diff --git a/src/backend/storage/aio/aio_io.c b/src/backend/storage/aio/aio_io.c index 520b5077df2..7d11d40284a 100644 --- a/src/backend/storage/aio/aio_io.c +++ b/src/backend/storage/aio/aio_io.c @@ -121,7 +121,7 @@ pgaio_io_perform_synchronously(PgAioHandle *ioh) START_CRIT_SECTION(); /* Perform IO. */ - switch (ioh->op) + switch ((PgAioOp) ioh->op) { case PGAIO_OP_READV: pgstat_report_wait_start(WAIT_EVENT_DATA_FILE_READ); @@ -176,7 +176,7 @@ pgaio_io_get_op_name(PgAioHandle *ioh) { Assert(ioh->op >= 0 && ioh->op < PGAIO_OP_COUNT); - switch (ioh->op) + switch ((PgAioOp) ioh->op) { case PGAIO_OP_INVALID: return "invalid"; @@ -198,7 +198,7 @@ pgaio_io_uses_fd(PgAioHandle *ioh, int fd) { Assert(ioh->state >= PGAIO_HS_DEFINED); - switch (ioh->op) + switch ((PgAioOp) ioh->op) { case PGAIO_OP_READV: return ioh->op_data.read.fd == fd; @@ -222,7 +222,7 @@ pgaio_io_get_iovec_length(PgAioHandle *ioh, struct iovec **iov) *iov = &pgaio_ctl->iovecs[ioh->iovec_off]; - switch (ioh->op) + switch ((PgAioOp) ioh->op) { case PGAIO_OP_READV: return ioh->op_data.read.iov_length; -- cgit v1.2.3