Re: BM_IO_ERROR flag is lost in TerminateBufferIO due to order of operations in UnlockBufHdrExt

26.03.2026 23:29, Andres Freund wrote:
> Hi,
>
> On 2026-03-25 17:51:30 +0300, Yura Sokolov wrote:
>> UnlockBufHdrExt does:
>>
>> buf_state |= set_bits;
>> buf_state &= ~unset_bits;
>> buf_state &= ~BM_LOCKED;
>>
>> TerminateBufferIO unconditionally does:
>>
>> unset_flag_bits |= BM_IO_ERROR;
>>
>> Due to this, AbortBufferIO and buffer_readv_complete_one are failed
>> to set BM_IO_ERROR with call to TerminateBufferIO.
>>
>> It was found with proprietary code that was triggered on
>> PGAIO_RS_ERROR and made assertion on BM_IO_ERROR presence.
>
> That's clearly not right. Care to write a patch? I think we should add a
> test for this in src/test/modules/test_aio too. As we don't rely on things
> like BM_IO_ERROR this is quite easy to not notice.
I thought, fix is too small to go the way of patches.
I don't mind if you just push it.

But if I can save your time on writing test, I'll try.

...

I believe, proper way is to add assertion in UnlockBufHdrExt:

Assert(!(set_bits & unset_bits));

And make TerminateBufferIO to exclude BM_IO_ERROR if it is among
set_flag_bits.

Is it ok?

--
regards
Yura Sokolov aka funny-falcon