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 <exclusion@gmail.com>
Reported-by: Konstantin Knizhnik <knizhnik@garret.ru>
Discussion: https://postgr.es/m/1500090.1745443021@sss.pgh.pa.us
Backpatch-through: 18

1 files changed, 10 insertions, 4 deletions

diff --git a/src/include/storage/aio_internal.h b/src/include/storage/aio_internal.h
index 2d37a243abe..b4de30f2ec1 100644
--- a/src/include/storage/aio_internal.h
+++ b/src/include/storage/aio_internal.h
@@ -92,17 +92,23 @@ typedef enum PgAioHandleState
 
 struct ResourceOwnerData;
 
-/* typedef is in aio_types.h */
+/*
+ * Typedef is in aio_types.h
+ *
+ * We don't use the underlying enums for state, target and op to avoid wasting
+ * space. We tried using bitfields, but several compilers generate rather
+ * horrid code for that.
+ */
 struct PgAioHandle
 {
 	/* all state updates should go through pgaio_io_update_state() */
-	PgAioHandleState state:8;
+	uint8		state;
 
 	/* what are we operating on */
-	PgAioTargetID target:8;
+	uint8		target;
 
 	/* which IO operation */
-	PgAioOp		op:8;
+	uint8		op;
 
 	/* bitfield of PgAioHandleFlags */
 	uint8		flags;