Re: Non-reproducible AIO failure

On 26/08/2025 3:37 AM, Andres Freund wrote:
> Hi,
>
> I'm a bit confused by this focus on bitfields - both Alexander and Konstantin
> stated they could reproduce the issue without the bitfields.


Sorry if I am not correct, but it seems that the problem was never 
reproduced with replaced bitfields.
I have once again looked through all this thread (it is really long;) 
and find just one place when you wrote, that you was able to reproduce 
the problem without bitfields. But as far as understand it was the 
different problem which was fixed by your patch 
v1-0001-aio-Add-missing-memory-barrier-when-waiting-for-I.patch 
<https://www.postgresql.org/message-id/attachment/177619/v1-0001-aio-Add-missing-memory-barrier-when-waiting-for-I.patch>

Still it is not quite clear to me how bitfields can cause this issue.
Yes, fitfields are updated all together - when you update one field, all 
other bitfields are also rewritten.
But actually compiler (at least some versions) does quite 
straightforward thing (especially without optimization):
1. Loads all bitfields.
2. Update required bitfield.
3. Stores all bitfields.

It is very unlikely that some stale version is stored in registers - 
without -O optimizer is using registers only locally and doesn't try to 
keep some variables in registers. Also not clear how we can read some 
stale value from memory if we perform write barrier before updating 
status (in pgaio_io_update_state).
It it impossible that `op` and `state` fields belongs to different cache 
lines because PgAioHandle is aligned on 8. So doesn;t matter whether 
there are bitfields or bytes, this fields should be propagated between 
cores all together.
Certainly, extraction of bitfields is not an atomic operations (as we 
see in assembler, it is done using two loads).
So we can observe inconsistent state of this two fields.

Let's consider fields `op` and `state`. Let's their initial state is 
`op=1`m `state=1`.
The we do update `op=2` and call pgaio_io_update_state(2). Some other 
task can see (op=1, state=1)  or (op=2, state=1) or (op=2, state=2) but 
not (op=1, state=2).

The problem can happen only if some other task tries to update `op` 
without prior checking for `state`.
In this case we can get (op=3,state=1) as well as (op=2,state=2).
But `op` field is updated in just two places: pgaio_io_reclaim and 
pgaio_io_stage. And in both cases we first check 'state' value.

What makes me concern is what happen in case of branch misprediction.
Assume that we have code:

if (state == 1) {
     op = 2;
} else {
     op = 3;
}

It seems to be absolutely legal, but what will happen in case if with 
speculative execution processor start executing first branch and does 
assignment op=2, but then state is loaded and used to be not equal to 1, 
so processor has to undone this execution. But for some moment we we can 
have state (state=0 and op=2) which can be observed by other task and 
cause assertion failure.
Is it really possible?

> But we have observed the generated code being pretty grotty and it's caused
> more than enough confusion - so let's just replace them with plain uint8's and
> cast in switches.

+1

May be I am wrong, but it seems to me that after 
add-moissing-memory-barrier patch was applied nobody reproduced 
assertion failure with replaced bitfields.