pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feedIO in wrong state on riscv64
22+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2025-11-07 c5d34f4a55 f8ccab0e97 03d9140cbd 2f76ffe5e4 1c7cba4c52 d8ba910b09 77b5b2c6f4 Fix generic read and write barriers for Clang.
* IO in wrong state on riscv64
@ 2025-10-11 13:00 Alexander Lakhin <exclusion@gmail.com>
0 siblings, 2 replies; 22+ messages in thread
From: Alexander Lakhin @ 2025-10-11 13:00 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; pgsql-hackers
Hello Andres,
I've spotted one more interesting AIO-related failure on a riscv64 machine
[1]:
=== dumping /home/gburd/build/HEAD/pgsql.build/testrun/recovery/027_stream_regress/data/regression.diffs ===
diff -U3 /home/gburd/build/HEAD/pgsql/src/test/regress/expected/tsearch.out
/home/gburd/build/HEAD/pgsql.build/testrun/recovery/027_stream_regress/data/results/tsearch.out
--- /home/gburd/build/HEAD/pgsql/src/test/regress/expected/tsearch.out 2025-10-09 00:06:10.612959611 +0000
+++ /home/gburd/build/HEAD/pgsql.build/testrun/recovery/027_stream_regress/data/results/tsearch.out 2025-10-09
00:27:46.697588198 +0000
@@ -163,11 +163,7 @@
(1 row)
SELECT count(*) FROM test_tsvector WHERE a @@ '!qe <2> qt';
- count
--------
- 6
-(1 row)
-
+ERROR: IO in wrong state: 0
SELECT count(*) FROM test_tsvector WHERE a @@ '!(pl <-> yh)';
count
-------
I've managed to reproduce it using qemu-system-riscv64 with Debian trixie
(6.11.10-riscv64, clang 19.1.4):
$ CC=clang-19 ./configure --enable-debug --enable-cassert --enable-tap-tests && time make -j8 -s
...
configure: using CFLAGS=-Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla
-Werror=unguarded-availability-new -Wendif-labels -Wmissing-format-attribute -Wcast-function-type -Wformat-security
-Wmissing-variable-declarations -fno-strict-aliasing -fwrapv -fexcess-precision=standard
-Wno-unused-command-line-argument -Wno-compound-token-split-by-macro -Wno-format-truncation
-Wno-cast-function-type-strict -g -O2
when running `make check` in a loop (it fails within 10 iterations for me)
with the reduced parallel_schedule:
test: test_setup
test: boolean char name varchar text int2 int4 int8 oid float8
x10
and extra.config:
log_line_prefix = '%m [%p:%l] %q%a '
log_connections = 'true'
log_disconnections = 'true'
log_statement = 'all'
fsync = off
restart_after_crash = off
debug_parallel_query = regress
shared_buffers = 512kB
backtrace_functions = 'pgaio_io_wait'
# mostly reflects greenfly's config, plus shared_buffers decreased
# below 027_stream_regress's 1MB.
With the following debug logging added:
inline patch 1: src/backend/storage/aio/aio.c (+18/-0 lines) download inline-1.patch
--- a/src/backend/storage/aio/aio.c
+++ b/src/backend/storage/aio/aio.c
@@ -578,6 +578,20 @@ pgaio_io_wait(PgAioHandle *ioh, uint64 ref_generation)
PgAioHandleState state;
bool am_owner;
+int inistate = ioh->state;
+if (inistate == PGAIO_HS_IDLE)
+{
+ fprintf(stderr, "!!!pgaio_io_wait[%d]| initial ioh->state: %d\n", getpid(), inistate);
+}
+for (int i = 0; i < 10; i++)
+{
+ int curstate = ioh->state;
+ if (curstate != inistate)
+ {
+ elog(PANIC, "!!!pgaio_io_wait| ioh->state changed from %d to %d at iteration %d", inistate, curstate, i);
+ break;
+ }
+}
am_owner = ioh->owner_procno == MyProcNumber;
if (pgaio_io_was_recycled(ioh, ref_generation, &state))
@@ -599,6 +613,10 @@ pgaio_io_wait(PgAioHandle *ioh, uint64 ref_generation)
{
if (pgaio_io_was_recycled(ioh, ref_generation, &state))
return;
+if (ioh->state == PGAIO_HS_IDLE)
+{
+ fprintf(stderr, "!!!pgaio_io_wait[%d]| ioh->state: %d (initial state: %d)\n", getpid(), ioh->state, inistate);
+}
switch ((PgAioHandleState) state)
{
* Re: IO in wrong state on riscv64
@ 2025-10-12 03:35 Thomas Munro <thomas.munro@gmail.com>
parent: Alexander Lakhin <exclusion@gmail.com>
1 sibling, 1 reply; 22+ messages in thread
From: Thomas Munro @ 2025-10-12 03:35 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers
On Sun, Oct 12, 2025 at 2:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> 2025-10-11 11:34:46.793 GMT [1169773:1] PANIC: !!!pgaio_io_wait| ioh->state changed from 0 to 1 at iteration 0
> # no other iteration number observed
Can you please disassemble pgaio_io_update_state() and
pgaio_io_was_recycled()? I wonder if the memory barriers are not
being generated correctly, causing the state and generation to be
loaded out of order, or something like that...
The previous failure on greenfly was a TIMEOUT in the same test, as if
a query was hanging.
I've also been trying to understand two failures on CI on x86 with the
same symptom, but that might be something else, hard to say yet...
https://cirrus-ci.com/task/5551651043409920
https://www.postgresql.org/message-id/CA%2BhUKG%2BHh7u3YpPLKQS%2BB7KJu0Bw99EjhuPuvBRVccDfgod95g%40ma...
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-10-12 03:58 Thomas Munro <thomas.munro@gmail.com>
parent: Alexander Lakhin <exclusion@gmail.com>
1 sibling, 0 replies; 22+ messages in thread
From: Thomas Munro @ 2025-10-12 03:58 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers
On Sun, Oct 12, 2025 at 2:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
> I've managed to reproduce it using qemu-system-riscv64 with Debian trixie
Huh, that's interesting. What is the host architecture? When I saw
that error myself and wondered about memory order, I dismissed the
idea of trying with qemu, figuring that my x86 host's TSO would affect
the coherency, but thinking again about that... I guess the compiler
might still reorder during riscv codegen if there is something wrong
with the barrier support, and even if it doesn't, the binary
translation to x86 might also feel free to reorder stuff if there are
no barrier instructions to prevent it? Or maybe that doesn't happen
but your host is ARM?
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-10-12 05:00 Alexander Lakhin <exclusion@gmail.com>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 2 replies; 22+ messages in thread
From: Alexander Lakhin @ 2025-10-12 05:00 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
Hello Thomas,
12.10.2025 06:35, Thomas Munro wrote:
> On Sun, Oct 12, 2025 at 2:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
>> 2025-10-11 11:34:46.793 GMT [1169773:1] PANIC: !!!pgaio_io_wait| ioh->state changed from 0 to 1 at iteration 0
>> # no other iteration number observed
> Can you please disassemble pgaio_io_update_state() and
> pgaio_io_was_recycled()? I wonder if the memory barriers are not
> being generated correctly, causing the state and generation to be
> loaded out of order, or something like that...
Please find those attached (gdb "disass/m pgaio_io_update_state" misses
the start of the function (but it's still disassembled below), so I
decided to share the whole output).
This is from clean master, without any modifications, but with the issue
confirmed for this build:
2025-10-11 20:29:11.724 UTC [1679534:1] [unknown] LOG: connection received: host=[local]
2025-10-11 20:29:11.725 UTC [1679536:1] [unknown] LOG: connection received: host=[local]
2025-10-11 20:29:11.726 UTC [1679538:1] [unknown] LOG: connection received: host=[local]
2025-10-11 20:29:11.724 UTC [1679533:1] [unknown] LOG: connection received: host=[local]
2025-10-11 20:29:11.724 UTC [1679537:1] [unknown] LOG: connection received: host=[local]
2025-10-11 20:29:11.724 UTC [1679535:1] [unknown] LOG: connection received: host=[local]
2025-10-11 20:29:11.729 UTC [1679539:1] [unknown] LOG: connection received: host=[local
...
2025-10-11 20:29:11.778 UTC [1679537:3] [unknown] LOG: connection authorized: user=debian database=regression
application_name=pg_regress/create_schema
2025-10-11 20:29:11.778 UTC [1679533:3] [unknown] LOG: connection authorized: user=debian database=regression
application_name=pg_regress/create_type
2025-10-11 20:29:11.778 UTC [1679539:3] [unknown] LOG: connection authorized: user=debian database=regression
application_name=pg_regress/create_procedure
2025-10-11 20:29:11.778 UTC [1679536:3] [unknown] LOG: connection authorized: user=debian database=regression
application_name=pg_regress/create_misc
2025-10-11 20:29:11.778 UTC [1679538:3] [unknown] LOG: connection authorized: user=debian database=regression
application_name=pg_regress/create_table
2025-10-11 20:29:11.778 UTC [1679535:3] [unknown] LOG: connection authorized: user=debian database=regression
application_name=pg_regress/create_function_c
2025-10-11 20:29:11.790 UTC [1679534:2] [unknown] FATAL: IO in wrong state: 0
2025-10-11 20:29:11.790 UTC [1679534:3] [unknown] BACKTRACE:
postgres: debian regression [local] authentication(+0x4371ec) [0x555565cd61ec]
postgres: debian regression [local] authentication(+0x442bb2) [0x555565ce1bb2]
postgres: debian regression [local] authentication(StartBufferIO+0x66) [0x555565ce1a10]
postgres: debian regression [local] authentication(+0x43df60) [0x555565cdcf60]
postgres: debian regression [local] authentication(StartReadBuffer+0x308) [0x555565cdc8a4]
postgres: debian regression [local] authentication(ReadBufferExtended+0x64) [0x555565cdaace]
...
postgres: debian regression [local] authentication(postmaster_child_launch+0x132) [0x555565c7787c]
postgres: debian regression [local] authentication(+0x3dcd52) [0x555565c7bd52]
postgres: debian regression [local] authentication(InitProcessGlobals+0) [0x555565c79c96]
postgres: debian regression [local] authentication(+0x3194b2) [0x555565bb84b2]
/lib/riscv64-linux-gnu/libc.so.6(+0x2791c) [0x7fffa690891c]
/lib/riscv64-linux-gnu/libc.so.6(__libc_start_main+0x74) [0x7fffa69089c4]
postgres: debian regression [local] authentication(_start+0x20) [0x5555659842c8]
> The previous failure on greenfly was a TIMEOUT in the same test, as if
> a query was hanging.
Yeah, I'll try to reproduce it too...
> On Sun, Oct 12, 2025 at 2:00 AM Alexander Lakhin <exclusion@gmail.com> wrote:
>> I've managed to reproduce it using qemu-system-riscv64 with Debian trixie
> Huh, that's interesting. What is the host architecture? When I saw
> that error myself and wondered about memory order, I dismissed the
> idea of trying with qemu, figuring that my x86 host's TSO would affect
> the coherency, but thinking again about that... I guess the compiler
> might still reorder during riscv codegen if there is something wrong
> with the barrier support, and even if it doesn't, the binary
> translation to x86 might also feel free to reorder stuff if there are
> no barrier instructions to prevent it? Or maybe that doesn't happen
> but your host is ARM?
I use AMD Ryzen 9 7900X, Ubuntu 24.04 and run the risc machine with:
qemu-system-riscv64 -machine virt -m 1G -smp 8 -cpu rv64 -device virtio-blk-device,drive=hd \
-drive file=.../dqib_riscv64-virt/image.qcow2,if=none,id=hd -device virtio-net-device,netdev=net \
-netdev user,id=net,hostfwd=tcp::22222-:22 -bios /usr/lib/riscv64-linux-gnu/opensbi/generic/fw_jump.elf \
-kernel /usr/lib/u-boot/qemu-riscv64_smode/uboot.elf -object rng-random,filename=/dev/urandom,id=rng \
-device virtio-rng-device,rng=rng -nographic -append "root=LABEL=rootfs console=ttyS0"
I don't know what hardware greenfly uses (CC'ing Greg in case he'd like to
share some info on this), but timings are similar to what I'm seeing:
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=greenfly&dt=2025-10-09%2000%3A06%3...
ok 160 - select_parallel 5019 ms
`make check` on mine, with select_parallel repeated:
ok 160 - select_parallel 6042 ms
ok 161 - select_parallel 6089 ms
ok 162 - select_parallel 6116 ms
copperhead and boomslang, which are using real RISC hardware [1], show
better timings:
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=copperhead&dt=2025-10-11%2019%3A36...
ok 160 - select_parallel 2677 ms
https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=boomslang&dt=2025-10-12%2002%3A17%...
ok 160 - select_parallel 3651 ms
Thank you for looking into this!
[1] https://www.postgresql.org/message-id/3db97903-884f-4b0c-b1cd-d7442e71ea75%40app.fastmail.com
Best regards,
Alexander
Attachments:
pgaio_io_update_state.asm (text/plain, 24.4K) download
GNU gdb (Debian 15.2-1) 15.2
Copyright (C) 2024 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "riscv64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from tmp_install/usr/local/pgsql/bin/postgres...
(gdb) Dump of assembler code for function pgaio_io_update_state:
341 Assert(ioh >= pgaio_ctl->io_handles &&
0x0000000000436490 <+90>: auipc a0,0x4c3
0x0000000000436494 <+94>: ld a1,-616(a0) # 0x8f9228 <pgaio_ctl>
0x0000000000436498 <+98>: ld a0,48(a1)
0x000000000043649a <+100>: bltu s1,a0,0x436552 <pgaio_io_update_state+284>
0x000000000043649e <+104>: lwu a1,40(a1)
0x00000000004364a2 <+108>: li a2,144
0x00000000004364a6 <+112>: mul a1,a1,a2
0x00000000004364aa <+116>: add a1,a1,a0
0x00000000004364ac <+118>: bgeu s1,a1,0x436552 <pgaio_io_update_state+284>
0x00000000004364b0 <+122>: auipc a1,0x4c1
0x00000000004364b4 <+126>: ld s3,280(a1) # 0x8f75c8
0x0000000000436552 <+284>: auipc a0,0x242
0x0000000000436556 <+288>: addi a0,a0,549 # 0x678777
0x000000000043655a <+292>: auipc a1,0x242
0x000000000043655e <+296>: addi a1,a1,163 # 0x6785fd
0x0000000000436562 <+300>: li a2,342
0x0000000000436566 <+304>: auipc ra,0x160
0x000000000043656a <+308>: jalr 994(ra) # 0x596948 <ExceptionalCondition>
342 ioh < (pgaio_ctl->io_handles + pgaio_ctl->io_handle_count));
343 return ioh - pgaio_ctl->io_handles;
0x00000000004364b8 <+130>: sub s0,s1,a0
0x00000000004364bc <+134>: srai s0,s0,0x4
344 }
345
346 /*
347 * Return the ProcNumber for the process that can use an IO handle. The
348 * mapping from IO handles to PGPROCs is static, therefore this even works
349 * when the corresponding PGPROC is not in use.
350 */
351 ProcNumber
352 pgaio_io_get_owner(PgAioHandle *ioh)
353 {
354 return ioh->owner_procno;
355 }
356
357 /*
358 * Return a wait reference for the IO. Only wait references can be used to
359 * wait for an IOs completion, as handles themselves can be reused after
360 * completion. See also the comment above pgaio_io_acquire().
361 */
362 void
363 pgaio_io_get_wref(PgAioHandle *ioh, PgAioWaitRef *iow)
364 {
365 Assert(ioh->state == PGAIO_HS_HANDED_OUT ||
366 ioh->state == PGAIO_HS_DEFINED ||
367 ioh->state == PGAIO_HS_STAGED);
368 Assert(ioh->generation != 0);
369
370 iow->aio_index = ioh - pgaio_ctl->io_handles;
371 iow->generation_upper = (uint32) (ioh->generation >> 32);
372 iow->generation_lower = (uint32) ioh->generation;
373 }
374
375
376
377 /* --------------------------------------------------------------------------------
378 * Internal Functions related to PgAioHandle
379 * --------------------------------------------------------------------------------
380 */
381
382 static inline void
383 pgaio_io_update_state(PgAioHandle *ioh, PgAioHandleState new_state)
384 {
0x0000000000436436 <+0>: addi sp,sp,-48
0x0000000000436438 <+2>: sd ra,40(sp)
0x000000000043643a <+4>: sd s0,32(sp)
0x000000000043643c <+6>: sd s1,24(sp)
0x000000000043643e <+8>: sd s2,16(sp)
0x0000000000436440 <+10>: sd s3,8(sp)
0x0000000000436442 <+12>: sd s4,0(sp)
385 /*
386 * All callers need to have held interrupts in some form, otherwise
387 * interrupt processing could wait for the IO to complete, while in an
388 * intermediary state.
389 */
390 Assert(!INTERRUPTS_CAN_BE_PROCESSED());
0x0000000000436444 <+14>: auipc a2,0x4a6
0x0000000000436448 <+18>: ld a2,564(a2) # 0x8dc678
0x000000000043644c <+22>: lw a2,0(a2)
0x000000000043644e <+24>: mv s4,a1
0x0000000000436450 <+26>: mv s1,a0
0x0000000000436452 <+28>: bnez a2,0x43646e <pgaio_io_update_state+56>
0x0000000000436454 <+30>: auipc a0,0x4a6
0x0000000000436458 <+34>: ld a0,508(a0) # 0x8dc650
0x000000000043645c <+38>: lw a0,0(a0)
0x000000000043645e <+40>: bnez a0,0x43646e <pgaio_io_update_state+56>
0x0000000000436460 <+42>: auipc a0,0x4a6
0x0000000000436464 <+46>: ld a0,-1616(a0) # 0x8dbe10
0x0000000000436468 <+50>: lw a0,0(a0)
0x000000000043646a <+52>: beqz a0,0x43656e <pgaio_io_update_state+312>
0x000000000043656e <+312>: auipc a0,0x242
0x0000000000436572 <+316>: addi a0,a0,1670 # 0x678bf4
0x0000000000436576 <+320>: auipc a1,0x242
0x000000000043657a <+324>: addi a1,a1,135 # 0x6785fd
0x000000000043657e <+328>: li a2,390
0x0000000000436582 <+332>: auipc ra,0x160
0x0000000000436586 <+336>: jalr 966(ra) # 0x596948 <ExceptionalCondition>
391
392 pgaio_debug_io(DEBUG5, ioh,
0x000000000043646e <+56>: li a0,10
0x0000000000436470 <+58>: li a1,0
0x0000000000436472 <+60>: auipc ra,0x161
0x0000000000436476 <+64>: jalr -1200(ra) # 0x596fc2 <errstart>
0x000000000043647a <+68>: beqz a0,0x43653a <pgaio_io_update_state+260>
0x000000000043647c <+70>: li a0,1
0x000000000043647e <+72>: auipc ra,0x163
0x0000000000436482 <+76>: jalr 1264(ra) # 0x59996e <errhidestmt>
0x0000000000436486 <+80>: li a0,1
0x0000000000436488 <+82>: auipc ra,0x163
0x000000000043648c <+86>: jalr 1356(ra) # 0x5999d4 <errhidecontext>
0x00000000004364be <+136>: mv a0,s1
0x00000000004364c0 <+138>: jal 0x438f90 <pgaio_io_get_op_name>
0x00000000004364c4 <+142>: mv s2,a0
0x00000000004364c6 <+144>: mv a0,s1
0x00000000004364c8 <+146>: jal 0x439076 <pgaio_io_get_target_name>
0x00000000004364d2 <+156>: mv a3,a0
0x00000000004364e6 <+176>: mulw a1,s0,s3
0x0000000000436502 <+204>: mulw a1,s0,s3
0x000000000043650c <+214>: auipc a0,0x242
0x0000000000436510 <+218>: addi a0,a0,1799 # 0x678c13
0x0000000000436514 <+222>: mv a2,s2
0x0000000000436516 <+224>: auipc ra,0x161
0x000000000043651a <+228>: jalr -52(ra) # 0x5974e2 <errmsg_internal>
0x000000000043651e <+232>: auipc a0,0x242
0x0000000000436522 <+236>: addi a0,a0,223 # 0x6785fd
0x0000000000436526 <+240>: auipc a1,0x242
0x000000000043652a <+244>: addi a2,a1,1836 # 0x678c52
0x000000000043652e <+248>: li a1,394
0x0000000000436532 <+252>: auipc ra,0x161
0x0000000000436536 <+256>: jalr -694(ra) # 0x59727c <errfinish>
393 "updating state to %s",
394 pgaio_io_state_get_name(new_state));
395
396 /*
397 * Ensure the changes signified by the new state are visible before the
398 * new state becomes visible.
399 */
400 pg_write_barrier();
0x000000000043653a <+260>: fence rw,w
401
402 ioh->state = new_state;
0x000000000043653e <+264>: sb s4,0(s1)
0x0000000000436542 <+268>: ld ra,40(sp)
0x0000000000436544 <+270>: ld s0,32(sp)
0x0000000000436546 <+272>: ld s1,24(sp)
0x0000000000436548 <+274>: ld s2,16(sp)
0x000000000043654a <+276>: ld s3,8(sp)
0x000000000043654c <+278>: ld s4,0(sp)
403 }
0x000000000043654e <+280>: addi sp,sp,48
0x0000000000436550 <+282>: ret
404
405 static void
406 pgaio_io_resowner_register(PgAioHandle *ioh)
407 {
408 Assert(!ioh->resowner);
409 Assert(CurrentResourceOwner);
410
411 ResourceOwnerRememberAioHandle(CurrentResourceOwner, &ioh->resowner_node);
412 ioh->resowner = CurrentResourceOwner;
413 }
414
415 /*
416 * Stage IO for execution and, if appropriate, submit it immediately.
417 *
418 * Should only be called from pgaio_io_start_*().
419 */
420 void
421 pgaio_io_stage(PgAioHandle *ioh, PgAioOp op)
422 {
423 bool needs_synchronous;
424
425 Assert(ioh->state == PGAIO_HS_HANDED_OUT);
426 Assert(pgaio_my_backend->handed_out_io == ioh);
427 Assert(pgaio_io_has_target(ioh));
428
429 /*
430 * Otherwise an interrupt, in the middle of staging and possibly executing
431 * the IO, could end up trying to wait for the IO, leading to state
432 * confusion.
433 */
434 HOLD_INTERRUPTS();
435
436 ioh->op = op;
437 ioh->result = 0;
438
439 pgaio_io_update_state(ioh, PGAIO_HS_DEFINED);
440
441 /* allow a new IO to be staged */
442 pgaio_my_backend->handed_out_io = NULL;
443
444 pgaio_io_call_stage(ioh);
445
446 pgaio_io_update_state(ioh, PGAIO_HS_STAGED);
447
448 /*
449 * Synchronous execution has to be executed, well, synchronously, so check
450 * that first.
451 */
452 needs_synchronous = pgaio_io_needs_synchronous_execution(ioh);
453
454 pgaio_debug_io(DEBUG3, ioh,
455 "staged (synchronous: %d, in_batch: %d)",
456 needs_synchronous, pgaio_my_backend->in_batchmode);
457
458 if (!needs_synchronous)
459 {
460 pgaio_my_backend->staged_ios[pgaio_my_backend->num_staged_ios++] = ioh;
461 Assert(pgaio_my_backend->num_staged_ios <= PGAIO_SUBMIT_BATCH_SIZE);
462
463 /*
464 * Unless code explicitly opted into batching IOs, submit the IO
465 * immediately.
466 */
467 if (!pgaio_my_backend->in_batchmode)
468 pgaio_submit_staged();
469 }
470 else
471 {
472 pgaio_io_prepare_submit(ioh);
473 pgaio_io_perform_synchronously(ioh);
474 }
475
476 RESUME_INTERRUPTS();
477 }
478
479 bool
480 pgaio_io_needs_synchronous_execution(PgAioHandle *ioh)
481 {
482 /*
483 * If the caller said to execute the IO synchronously, do so.
484 *
485 * XXX: We could optimize the logic when to execute synchronously by first
486 * checking if there are other IOs in flight and only synchronously
487 * executing if not. Unclear whether that'll be sufficiently common to be
488 * worth worrying about.
489 */
490 if (ioh->flags & PGAIO_HF_SYNCHRONOUS)
491 return true;
492
493 /* Check if the IO method requires synchronous execution of IO */
494 if (pgaio_method_ops->needs_synchronous_execution)
495 return pgaio_method_ops->needs_synchronous_execution(ioh);
496
497 return false;
498 }
499
500 /*
501 * Handle IO being processed by IO method.
502 *
503 * Should be called by IO methods / synchronous IO execution, just before the
504 * IO is performed.
505 */
506 void
507 pgaio_io_prepare_submit(PgAioHandle *ioh)
508 {
509 pgaio_io_update_state(ioh, PGAIO_HS_SUBMITTED);
510
511 dclist_push_tail(&pgaio_my_backend->in_flight_ios, &ioh->node);
512 }
513
514 /*
515 * Handle IO getting completed by a method.
516 *
517 * Should be called by IO methods / synchronous IO execution, just after the
518 * IO has been performed.
519 *
520 * Expects to be called in a critical section. We expect IOs to be usable for
521 * WAL etc, which requires being able to execute completion callbacks in a
522 * critical section.
523 */
524 void
525 pgaio_io_process_completion(PgAioHandle *ioh, int result)
526 {
527 Assert(ioh->state == PGAIO_HS_SUBMITTED);
528
529 Assert(CritSectionCount > 0);
530
531 ioh->result = result;
532
533 pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_IO);
534
535 INJECTION_POINT("aio-process-completion-before-shared", ioh);
536
537 pgaio_io_call_complete_shared(ioh);
538
539 pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_SHARED);
540
541 /* condition variable broadcast ensures state is visible before wakeup */
542 ConditionVariableBroadcast(&ioh->cv);
543
544 /* contains call to pgaio_io_call_complete_local() */
545 if (ioh->owner_procno == MyProcNumber)
546 pgaio_io_reclaim(ioh);
547 }
548
549 /*
550 * Has the IO completed and thus the IO handle been reused?
551 *
552 * This is useful when waiting for IO completion at a low level (e.g. in an IO
553 * method's ->wait_one() callback).
554 */
555 bool
556 pgaio_io_was_recycled(PgAioHandle *ioh, uint64 ref_generation, PgAioHandleState *state)
557 {
558 *state = ioh->state;
559
560 /*
561 * Ensure that we don't see an earlier state of the handle than ioh->state
562 * due to compiler or CPU reordering. This protects both ->generation as
563 * directly used here, and other fields in the handle accessed in the
564 * caller if the handle was not reused.
565 */
566 pg_read_barrier();
567
568 return ioh->generation != ref_generation;
569 }
570
571 /*
572 * Wait for IO to complete. External code should never use this, outside of
573 * the AIO subsystem waits are only allowed via pgaio_wref_wait().
574 */
575 static void
576 pgaio_io_wait(PgAioHandle *ioh, uint64 ref_generation)
577 {
578 PgAioHandleState state;
579 bool am_owner;
580
581 am_owner = ioh->owner_procno == MyProcNumber;
582
583 if (pgaio_io_was_recycled(ioh, ref_generation, &state))
584 return;
585
586 if (am_owner)
587 {
588 if (state != PGAIO_HS_SUBMITTED
589 && state != PGAIO_HS_COMPLETED_IO
590 && state != PGAIO_HS_COMPLETED_SHARED
591 && state != PGAIO_HS_COMPLETED_LOCAL)
592 {
593 elog(PANIC, "waiting for own IO %d in wrong state: %s",
594 pgaio_io_get_id(ioh), pgaio_io_get_state_name(ioh));
595 }
596 }
597
598 while (true)
599 {
600 if (pgaio_io_was_recycled(ioh, ref_generation, &state))
601 return;
602
603 switch ((PgAioHandleState) state)
604 {
605 case PGAIO_HS_IDLE:
606 case PGAIO_HS_HANDED_OUT:
607 elog(ERROR, "IO in wrong state: %d", state);
608 break;
609
610 case PGAIO_HS_SUBMITTED:
611
612 /*
613 * If we need to wait via the IO method, do so now. Don't
614 * check via the IO method if the issuing backend is executing
615 * the IO synchronously.
616 */
617 if (pgaio_method_ops->wait_one && !(ioh->flags & PGAIO_HF_SYNCHRONOUS))
618 {
619 pgaio_method_ops->wait_one(ioh, ref_generation);
620 continue;
621 }
622 /* fallthrough */
623
624 /* waiting for owner to submit */
625 case PGAIO_HS_DEFINED:
626 case PGAIO_HS_STAGED:
627 /* waiting for reaper to complete */
628 /* fallthrough */
629 case PGAIO_HS_COMPLETED_IO:
630 /* shouldn't be able to hit this otherwise */
631 Assert(IsUnderPostmaster);
632 /* ensure we're going to get woken up */
633 ConditionVariablePrepareToSleep(&ioh->cv);
634
635 while (!pgaio_io_was_recycled(ioh, ref_generation, &state))
636 {
637 if (state == PGAIO_HS_COMPLETED_SHARED ||
638 state == PGAIO_HS_COMPLETED_LOCAL)
639 break;
640 ConditionVariableSleep(&ioh->cv, WAIT_EVENT_AIO_IO_COMPLETION);
641 }
642
643 ConditionVariableCancelSleep();
644 break;
645
646 case PGAIO_HS_COMPLETED_SHARED:
647 case PGAIO_HS_COMPLETED_LOCAL:
648
649 /*
650 * Note that no interrupts are processed between
651 * pgaio_io_was_recycled() and this check - that's important
652 * as otherwise an interrupt could have already reclaimed the
653 * handle.
654 */
655 if (am_owner)
656 pgaio_io_reclaim(ioh);
657 return;
658 }
659 }
660 }
661
662 /*
663 * Make IO handle ready to be reused after IO has completed or after the
664 * handle has been released without being used.
665 *
666 * Note that callers need to be careful about only calling this in the right
667 * state and that no interrupts can be processed between the state check and
668 * the call to pgaio_io_reclaim(). Otherwise interrupt processing could
669 * already have reclaimed the handle.
670 */
671 static void
672 pgaio_io_reclaim(PgAioHandle *ioh)
673 {
674 /* This is only ok if it's our IO */
675 Assert(ioh->owner_procno == MyProcNumber);
676 Assert(ioh->state != PGAIO_HS_IDLE);
677
678 /* see comment in function header */
679 HOLD_INTERRUPTS();
680
681 /*
682 * It's a bit ugly, but right now the easiest place to put the execution
683 * of local completion callbacks is this function, as we need to execute
684 * local callbacks just before reclaiming at multiple callsites.
685 */
686 if (ioh->state == PGAIO_HS_COMPLETED_SHARED)
687 {
688 PgAioResult local_result;
689
690 local_result = pgaio_io_call_complete_local(ioh);
691 pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_LOCAL);
692
693 if (ioh->report_return)
694 {
695 ioh->report_return->result = local_result;
696 ioh->report_return->target_data = ioh->target_data;
697 }
698 }
699
700 pgaio_debug_io(DEBUG4, ioh,
701 "reclaiming: distilled_result: (status %s, id %u, error_data %d), raw_result: %d",
702 pgaio_result_status_string(ioh->distilled_result.status),
703 ioh->distilled_result.id,
704 ioh->distilled_result.error_data,
705 ioh->result);
706
707 /* if the IO has been defined, it's on the in-flight list, remove */
708 if (ioh->state != PGAIO_HS_HANDED_OUT)
709 dclist_delete_from(&pgaio_my_backend->in_flight_ios, &ioh->node);
710
711 if (ioh->resowner)
712 {
713 ResourceOwnerForgetAioHandle(ioh->resowner, &ioh->resowner_node);
714 ioh->resowner = NULL;
715 }
716
717 Assert(!ioh->resowner);
718
719 /*
720 * Update generation & state first, before resetting the IO's fields,
721 * otherwise a concurrent "viewer" could think the fields are valid, even
722 * though they are being reset. Increment the generation first, so that
723 * we can assert elsewhere that we never wait for an IDLE IO. While it's
724 * a bit weird for the state to go backwards for a generation, it's OK
725 * here, as there cannot be references to the "reborn" IO yet. Can't
726 * update both at once, so something has to give.
727 */
728 ioh->generation++;
729 pgaio_io_update_state(ioh, PGAIO_HS_IDLE);
730
731 /* ensure the state update is visible before we reset fields */
732 pg_write_barrier();
733
734 ioh->op = PGAIO_OP_INVALID;
735 ioh->target = PGAIO_TID_INVALID;
736 ioh->flags = 0;
737 ioh->num_callbacks = 0;
738 ioh->handle_data_len = 0;
739 ioh->report_return = NULL;
740 ioh->result = 0;
741 ioh->distilled_result.status = PGAIO_RS_UNKNOWN;
742
743 /*
744 * We push the IO to the head of the idle IO list, that seems more cache
745 * efficient in cases where only a few IOs are used.
746 */
747 dclist_push_head(&pgaio_my_backend->idle_ios, &ioh->node);
748
749 RESUME_INTERRUPTS();
750 }
751
752 /*
753 * Wait for an IO handle to become usable.
754 *
755 * This only really is useful for pgaio_io_acquire().
756 */
757 static void
758 pgaio_io_wait_for_free(void)
759 {
760 int reclaimed = 0;
761
762 pgaio_debug(DEBUG2, "waiting for free IO with %d pending, %u in-flight, %u idle IOs",
763 pgaio_my_backend->num_staged_ios,
764 dclist_count(&pgaio_my_backend->in_flight_ios),
765 dclist_count(&pgaio_my_backend->idle_ios));
766
767 /*
768 * First check if any of our IOs actually have completed - when using
769 * worker, that'll often be the case. We could do so as part of the loop
770 * below, but that'd potentially lead us to wait for some IO submitted
771 * before.
772 */
773 for (int i = 0; i < io_max_concurrency; i++)
774 {
775 PgAioHandle *ioh = &pgaio_ctl->io_handles[pgaio_my_backend->io_handle_off + i];
776
777 if (ioh->state == PGAIO_HS_COMPLETED_SHARED)
778 {
779 /*
780 * Note that no interrupts are processed between the state check
781 * and the call to reclaim - that's important as otherwise an
782 * interrupt could have already reclaimed the handle.
783 *
784 * Need to ensure that there's no reordering, in the more common
785 * paths, where we wait for IO, that's done by
786 * pgaio_io_was_recycled().
787 */
788 pg_read_barrier();
789 pgaio_io_reclaim(ioh);
790 reclaimed++;
791 }
792 }
793
794 if (reclaimed > 0)
795 return;
796
797 /*
798 * If we have any unsubmitted IOs, submit them now. We'll start waiting in
799 * a second, so it's better they're in flight. This also addresses the
800 * edge-case that all IOs are unsubmitted.
801 */
802 if (pgaio_my_backend->num_staged_ios > 0)
803 pgaio_submit_staged();
804
805 /* possibly some IOs finished during submission */
806 if (!dclist_is_empty(&pgaio_my_backend->idle_ios))
807 return;
808
809 if (dclist_count(&pgaio_my_backend->in_flight_ios) == 0)
810 ereport(ERROR,
811 errmsg_internal("no free IOs despite no in-flight IOs"),
812 errdetail_internal("%d pending, %u in-flight, %u idle IOs",
813 pgaio_my_backend->num_staged_ios,
814 dclist_count(&pgaio_my_backend->in_flight_ios),
815 dclist_count(&pgaio_my_backend->idle_ios)));
816
817 /*
818 * Wait for the oldest in-flight IO to complete.
819 *
820 * XXX: Reusing the general IO wait is suboptimal, we don't need to wait
821 * for that specific IO to complete, we just need *any* IO to complete.
822 */
823 {
824 PgAioHandle *ioh = dclist_head_element(PgAioHandle, node,
825 &pgaio_my_backend->in_flight_ios);
826 uint64 generation = ioh->generation;
827
828 switch ((PgAioHandleState) ioh->state)
829 {
830 /* should not be in in-flight list */
831 case PGAIO_HS_IDLE:
832 case PGAIO_HS_DEFINED:
833 case PGAIO_HS_HANDED_OUT:
834 case PGAIO_HS_STAGED:
835 case PGAIO_HS_COMPLETED_LOCAL:
836 elog(ERROR, "shouldn't get here with io:%d in state %d",
837 pgaio_io_get_id(ioh), ioh->state);
838 break;
839
840 case PGAIO_HS_COMPLETED_IO:
841 case PGAIO_HS_SUBMITTED:
842 pgaio_debug_io(DEBUG2, ioh,
843 "waiting for free io with %u in flight",
844 dclist_count(&pgaio_my_backend->in_flight_ios));
845
846 /*
847 * In a more general case this would be racy, because the
848 * generation could increase after we read ioh->state above.
849 * But we are only looking at IOs by the current backend and
850 * the IO can only be recycled by this backend. Even this is
851 * only OK because we get the handle's generation before
852 * potentially processing interrupts, e.g. as part of
853 * pgaio_debug_io().
854 */
855 pgaio_io_wait(ioh, generation);
856 break;
857
858 case PGAIO_HS_COMPLETED_SHARED:
859
860 /*
861 * It's possible that another backend just finished this IO.
862 *
863 * Note that no interrupts are processed between the state
864 * check and the call to reclaim - that's important as
865 * otherwise an interrupt could have already reclaimed the
866 * handle.
867 *
868 * Need to ensure that there's no reordering, in the more
869 * common paths, where we wait for IO, that's done by
870 * pgaio_io_was_recycled().
871 */
872 pg_read_barrier();
873 pgaio_io_reclaim(ioh);
874 break;
875 }
876
877 if (dclist_count(&pgaio_my_backend->idle_ios) == 0)
878 elog(PANIC, "no idle IO after waiting for IO to terminate");
879 return;
880 }
881 }
882
883 /*
884 * Internal - code outside of AIO should never need this and it'd be hard for
885 * such code to be safe.
886 */
887 static PgAioHandle *
888 pgaio_io_from_wref(PgAioWaitRef *iow, uint64 *ref_generation)
889 {
890 PgAioHandle *ioh;
891
892 Assert(iow->aio_index < pgaio_ctl->io_handle_count);
893
894 ioh = &pgaio_ctl->io_handles[iow->aio_index];
895
896 *ref_generation = ((uint64) iow->generation_upper) << 32 |
897 iow->generation_lower;
898
899 Assert(*ref_generation != 0);
900
901 return ioh;
902 }
903
904 static const char *
905 pgaio_io_state_get_name(PgAioHandleState s)
906 {
907 #define PGAIO_HS_TOSTR_CASE(sym) case PGAIO_HS_##sym: return #sym
908 switch ((PgAioHandleState) s)
0x00000000004364d4 <+158>: bltu a2,a1,0x436500 <pgaio_io_update_state+202>
0x00000000004364d8 <+162>: slli a1,a1,0x3
0x00000000004364da <+164>: auipc a0,0x485
0x00000000004364de <+168>: addi a0,a0,-530 # 0x8bb2c8
0x00000000004364e2 <+172>: add a0,a0,a1
0x00000000004364e4 <+174>: ld a4,0(a0)
0x00000000004364ea <+180>: bltu a2,s4,0x43650a <pgaio_io_update_state+212>
0x00000000004364ee <+184>: slli a0,s4,0x3
0x00000000004364f2 <+188>: auipc a2,0x485
0x00000000004364f6 <+192>: addi a2,a2,-554 # 0x8bb2c8
0x00000000004364fa <+196>: add a0,a0,a2
0x00000000004364fc <+198>: ld a5,0(a0)
0x00000000004364fe <+200>: j 0x43650c <pgaio_io_update_state+214>
0x0000000000436500 <+202>: li a4,0
0x0000000000436506 <+208>: bgeu a2,s4,0x4364ee <pgaio_io_update_state+184>
0x000000000043650a <+212>: li a5,0
909 {
910 PGAIO_HS_TOSTR_CASE(IDLE);
911 PGAIO_HS_TOSTR_CASE(HANDED_OUT);
912 PGAIO_HS_TOSTR_CASE(DEFINED);
913 PGAIO_HS_TOSTR_CASE(STAGED);
914 PGAIO_HS_TOSTR_CASE(SUBMITTED);
915 PGAIO_HS_TOSTR_CASE(COMPLETED_IO);
916 PGAIO_HS_TOSTR_CASE(COMPLETED_SHARED);
917 PGAIO_HS_TOSTR_CASE(COMPLETED_LOCAL);
918 }
919 #undef PGAIO_HS_TOSTR_CASE
920
921 return NULL; /* silence compiler */
922 }
923
924 const char *
925 pgaio_io_get_state_name(PgAioHandle *ioh)
926 {
927 return pgaio_io_state_get_name(ioh->state);
0x00000000004364cc <+150>: lbu a1,0(s1)
0x00000000004364d0 <+154>: li a2,7
End of assembler dump.
(gdb)
pgaio_io_was_recycled.asm (text/plain, 1.5K) download
GNU gdb (Debian 15.2-1) 15.2
Copyright (C) 2024 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "riscv64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from tmp_install/usr/local/pgsql/bin/postgres...
(gdb) Dump of assembler code for function pgaio_io_was_recycled:
558 *state = ioh->state;
0x0000000000436f98 <+0>: lbu a3,0(a0)
0x0000000000436f9c <+4>: sw a3,0(a2)
559
560 /*
561 * Ensure that we don't see an earlier state of the handle than ioh->state
562 * due to compiler or CPU reordering. This protects both ->generation as
563 * directly used here, and other fields in the handle accessed in the
564 * caller if the handle was not reused.
565 */
566 pg_read_barrier();
0x0000000000436f9e <+6>: fence r,rw
567
568 return ioh->generation != ref_generation;
0x0000000000436fa2 <+10>: ld a0,64(a0)
0x0000000000436fa4 <+12>: xor a0,a0,a1
0x0000000000436fa6 <+14>: snez a0,a0
0x0000000000436faa <+18>: ret
End of assembler dump.
(gdb)
* Re: IO in wrong state on riscv64
@ 2025-10-12 13:10 Greg Burd <greg@burd.me>
parent: Alexander Lakhin <exclusion@gmail.com>
1 sibling, 0 replies; 22+ messages in thread
From: Greg Burd @ 2025-10-12 13:10 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; Thomas Munro <thomas.munro@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers
On Sun, Oct 12, 2025, at 1:00 AM, Alexander Lakhin wrote:
> Hello Thomas,
>
> 12.10.2025 06:35, Thomas Munro wrote:
>> On Sun, Oct 12, 2025 at 2:00 AM Alexander Lakhin
>> <exclusion@gmail.com> wrote:
>>> 2025-10-11 11:34:46.793 GMT [1169773:1] PANIC: !!!pgaio_io_wait|
>>> ioh->state changed from 0 to 1 at iteration 0
>>> # no other iteration number observed
>> Can you please disassemble pgaio_io_update_state() and
>> pgaio_io_was_recycled()? I wonder if the memory barriers are not
>> being generated correctly, causing the state and generation to be
>> loaded out of order, or something like that...
>
> Please find those attached (gdb "disass/m pgaio_io_update_state" misses
> the start of the function (but it's still disassembled below), so I
> decided to share the whole output).
> This is from clean master, without any modifications, but with the issue
> confirmed for this build:
> 2025-10-11 20:29:11.724 UTC [1679534:1] [unknown] LOG: connection
> received: host=[local]
> 2025-10-11 20:29:11.725 UTC [1679536:1] [unknown] LOG: connection
> received: host=[local]
> 2025-10-11 20:29:11.726 UTC [1679538:1] [unknown] LOG: connection
> received: host=[local]
> 2025-10-11 20:29:11.724 UTC [1679533:1] [unknown] LOG: connection
> received: host=[local]
> 2025-10-11 20:29:11.724 UTC [1679537:1] [unknown] LOG: connection
> received: host=[local]
> 2025-10-11 20:29:11.724 UTC [1679535:1] [unknown] LOG: connection
> received: host=[local]
> 2025-10-11 20:29:11.729 UTC [1679539:1] [unknown] LOG: connection
> received: host=[local
> ...
> 2025-10-11 20:29:11.778 UTC [1679537:3] [unknown] LOG: connection
> authorized: user=debian database=regression
> application_name=pg_regress/create_schema
> 2025-10-11 20:29:11.778 UTC [1679533:3] [unknown] LOG: connection
> authorized: user=debian database=regression
> application_name=pg_regress/create_type
> 2025-10-11 20:29:11.778 UTC [1679539:3] [unknown] LOG: connection
> authorized: user=debian database=regression
> application_name=pg_regress/create_procedure
> 2025-10-11 20:29:11.778 UTC [1679536:3] [unknown] LOG: connection
> authorized: user=debian database=regression
> application_name=pg_regress/create_misc
> 2025-10-11 20:29:11.778 UTC [1679538:3] [unknown] LOG: connection
> authorized: user=debian database=regression
> application_name=pg_regress/create_table
> 2025-10-11 20:29:11.778 UTC [1679535:3] [unknown] LOG: connection
> authorized: user=debian database=regression
> application_name=pg_regress/create_function_c
> 2025-10-11 20:29:11.790 UTC [1679534:2] [unknown] FATAL: IO in wrong
> state: 0
> 2025-10-11 20:29:11.790 UTC [1679534:3] [unknown] BACKTRACE:
> postgres: debian regression [local] authentication(+0x4371ec)
> [0x555565cd61ec]
> postgres: debian regression [local] authentication(+0x442bb2)
> [0x555565ce1bb2]
> postgres: debian regression [local]
> authentication(StartBufferIO+0x66) [0x555565ce1a10]
> postgres: debian regression [local] authentication(+0x43df60)
> [0x555565cdcf60]
> postgres: debian regression [local]
> authentication(StartReadBuffer+0x308) [0x555565cdc8a4]
> postgres: debian regression [local]
> authentication(ReadBufferExtended+0x64) [0x555565cdaace]
> ...
> postgres: debian regression [local]
> authentication(postmaster_child_launch+0x132) [0x555565c7787c]
> postgres: debian regression [local] authentication(+0x3dcd52)
> [0x555565c7bd52]
> postgres: debian regression [local]
> authentication(InitProcessGlobals+0) [0x555565c79c96]
> postgres: debian regression [local] authentication(+0x3194b2)
> [0x555565bb84b2]
> /lib/riscv64-linux-gnu/libc.so.6(+0x2791c) [0x7fffa690891c]
> /lib/riscv64-linux-gnu/libc.so.6(__libc_start_main+0x74)
> [0x7fffa69089c4]
> postgres: debian regression [local]
> authentication(_start+0x20) [0x5555659842c8]
>
>> The previous failure on greenfly was a TIMEOUT in the same test, as if
>> a query was hanging.
>
> Yeah, I'll try to reproduce it too...
>
>> On Sun, Oct 12, 2025 at 2:00 AM Alexander Lakhin
>> <exclusion@gmail.com> wrote:
>>> I've managed to reproduce it using qemu-system-riscv64 with Debian trixie
>> Huh, that's interesting. What is the host architecture? When I saw
>> that error myself and wondered about memory order, I dismissed the
>> idea of trying with qemu, figuring that my x86 host's TSO would affect
>> the coherency, but thinking again about that... I guess the compiler
>> might still reorder during riscv codegen if there is something wrong
>> with the barrier support, and even if it doesn't, the binary
>> translation to x86 might also feel free to reorder stuff if there are
>> no barrier instructions to prevent it? Or maybe that doesn't happen
>> but your host is ARM?
>
> I use AMD Ryzen 9 7900X, Ubuntu 24.04 and run the risc machine with:
> qemu-system-riscv64 -machine virt -m 1G -smp 8 -cpu rv64 -device
> virtio-blk-device,drive=hd \
> -drive file=.../dqib_riscv64-virt/image.qcow2,if=none,id=hd -device
> virtio-net-device,netdev=net \
> -netdev user,id=net,hostfwd=tcp::22222-:22 -bios
> /usr/lib/riscv64-linux-gnu/opensbi/generic/fw_jump.elf \
> -kernel /usr/lib/u-boot/qemu-riscv64_smode/uboot.elf -object
> rng-random,filename=/dev/urandom,id=rng \
> -device virtio-rng-device,rng=rng -nographic -append
> "root=LABEL=rootfs
> console=ttyS0"
>
> I don't know what hardware greenfly uses (CC'ing Greg in case he'd
> like to share some info on this),
Hello Alexander, thanks for digging into this issue.
The animal "greenfly" is an OrangePi RV2 [1] with 8GB of RAM. It has
the optional 256GB eMMC module and uses that for the root filesystem.
It's running Ubuntu 24.04.3 and I'm compiling with clang 18-1.3. I
think the rest of the details are in the animal registration or in the
logs, but let me know if you have questions or if there are things that
you'd like me to test.
> but timings are similar to what
> I'm seeing:
> https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=greenfly&dt=2025-10-09%2000%3A06%3...
> ok 160 - select_parallel 5019 ms
>
> `make check` on mine, with select_parallel repeated:
> ok 160 - select_parallel 6042 ms
> ok 161 - select_parallel 6089 ms
> ok 162 - select_parallel 6116 ms
>
> copperhead and boomslang, which are using real RISC hardware [1], show
> better timings:
> https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=copperhead&dt=2025-10-11%2019%3A36...
> ok 160 - select_parallel 2677 ms
>
> https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=boomslang&dt=2025-10-12%2002%3A17%...
> ok 160 - select_parallel 3651 ms
>
> Thank you for looking into this!
>
> [1]
> https://www.postgresql.org/message-id/3db97903-884f-4b0c-b1cd-d7442e71ea75%40app.fastmail.com
>
> Best regards,
> Alexander
> Attachments:
> * pgaio_io_update_state.asm
> * pgaio_io_was_recycled.asm
best.
-greg
[1]
http://www.orangepi.org/html/hardWare/computerAndMicrocontrollers/details/Orange-Pi-RV2.html
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-10-12 22:44 Thomas Munro <thomas.munro@gmail.com>
parent: Alexander Lakhin <exclusion@gmail.com>
1 sibling, 1 reply; 22+ messages in thread
From: Thomas Munro @ 2025-10-12 22:44 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
On Sun, Oct 12, 2025 at 6:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
> Please find those attached (gdb "disass/m pgaio_io_update_state" misses
> the start of the function (but it's still disassembled below), so I
> decided to share the whole output).
Could you please also disassemble pgaio_io_reclaim()?
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-10-13 04:00 Alexander Lakhin <exclusion@gmail.com>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Alexander Lakhin @ 2025-10-13 04:00 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
13.10.2025 01:44, Thomas Munro wrote:
> On Sun, Oct 12, 2025 at 6:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
>> Please find those attached (gdb "disass/m pgaio_io_update_state" misses
>> the start of the function (but it's still disassembled below), so I
>> decided to share the whole output).
> Could you please also disassemble pgaio_io_reclaim()?
Sure, the output of disass/m pgaio_io_reclaim is attached.
A side note: I could not reproduce the hang so far — 8 out of 30
iterations of 027_stream_regress failed, but all completed within
700-800 seconds.
Best regards,
Alexander
Attachments:
pgaio_io_reclaim.asm (text/plain, 28.2K) download
GNU gdb (Debian 15.2-1) 15.2
Copyright (C) 2024 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "riscv64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from tmp_install/usr/local/pgsql/bin/postgres...
(gdb) Dump of assembler code for function pgaio_io_reclaim:
341 Assert(ioh >= pgaio_ctl->io_handles &&
0x00000000004366d0 <+162>: auipc a0,0x4c3
0x00000000004366d4 <+166>: ld a1,-1192(a0) # 0x8f9228 <pgaio_ctl>
0x00000000004366d8 <+170>: ld a0,48(a1)
0x00000000004366da <+172>: bltu s0,a0,0x436886 <pgaio_io_reclaim+600>
0x00000000004366de <+176>: lwu a1,40(a1)
0x00000000004366e2 <+180>: li a2,144
0x00000000004366e6 <+184>: mul a1,a1,a2
0x00000000004366ea <+188>: add a1,a1,a0
0x00000000004366ec <+190>: bgeu s0,a1,0x436886 <pgaio_io_reclaim+600>
0x00000000004366f0 <+194>: auipc a1,0x4c1
0x00000000004366f4 <+198>: ld s4,-296(a1) # 0x8f75c8
0x0000000000436886 <+600>: auipc a0,0x242
0x000000000043688a <+604>: addi a0,a0,-271 # 0x678777
0x000000000043688e <+608>: auipc a1,0x242
0x0000000000436892 <+612>: addi a1,a1,-657 # 0x6785fd
0x0000000000436896 <+616>: li a2,342
0x000000000043689a <+620>: auipc ra,0x160
0x000000000043689e <+624>: jalr 174(ra) # 0x596948 <ExceptionalCondition>
342 ioh < (pgaio_ctl->io_handles + pgaio_ctl->io_handle_count));
343 return ioh - pgaio_ctl->io_handles;
0x00000000004366f8 <+202>: sub s1,s0,a0
0x00000000004366fc <+206>: srai s1,s1,0x4
344 }
345
346 /*
347 * Return the ProcNumber for the process that can use an IO handle. The
348 * mapping from IO handles to PGPROCs is static, therefore this even works
349 * when the corresponding PGPROC is not in use.
350 */
351 ProcNumber
352 pgaio_io_get_owner(PgAioHandle *ioh)
353 {
354 return ioh->owner_procno;
355 }
356
357 /*
358 * Return a wait reference for the IO. Only wait references can be used to
359 * wait for an IOs completion, as handles themselves can be reused after
360 * completion. See also the comment above pgaio_io_acquire().
361 */
362 void
363 pgaio_io_get_wref(PgAioHandle *ioh, PgAioWaitRef *iow)
364 {
365 Assert(ioh->state == PGAIO_HS_HANDED_OUT ||
366 ioh->state == PGAIO_HS_DEFINED ||
367 ioh->state == PGAIO_HS_STAGED);
368 Assert(ioh->generation != 0);
369
370 iow->aio_index = ioh - pgaio_ctl->io_handles;
371 iow->generation_upper = (uint32) (ioh->generation >> 32);
372 iow->generation_lower = (uint32) ioh->generation;
373 }
374
375
376
377 /* --------------------------------------------------------------------------------
378 * Internal Functions related to PgAioHandle
379 * --------------------------------------------------------------------------------
380 */
381
382 static inline void
383 pgaio_io_update_state(PgAioHandle *ioh, PgAioHandleState new_state)
384 {
385 /*
386 * All callers need to have held interrupts in some form, otherwise
387 * interrupt processing could wait for the IO to complete, while in an
388 * intermediary state.
389 */
390 Assert(!INTERRUPTS_CAN_BE_PROCESSED());
391
392 pgaio_debug_io(DEBUG5, ioh,
393 "updating state to %s",
394 pgaio_io_state_get_name(new_state));
395
396 /*
397 * Ensure the changes signified by the new state are visible before the
398 * new state becomes visible.
399 */
400 pg_write_barrier();
401
402 ioh->state = new_state;
403 }
404
405 static void
406 pgaio_io_resowner_register(PgAioHandle *ioh)
407 {
408 Assert(!ioh->resowner);
409 Assert(CurrentResourceOwner);
410
411 ResourceOwnerRememberAioHandle(CurrentResourceOwner, &ioh->resowner_node);
412 ioh->resowner = CurrentResourceOwner;
413 }
414
415 /*
416 * Stage IO for execution and, if appropriate, submit it immediately.
417 *
418 * Should only be called from pgaio_io_start_*().
419 */
420 void
421 pgaio_io_stage(PgAioHandle *ioh, PgAioOp op)
422 {
423 bool needs_synchronous;
424
425 Assert(ioh->state == PGAIO_HS_HANDED_OUT);
426 Assert(pgaio_my_backend->handed_out_io == ioh);
427 Assert(pgaio_io_has_target(ioh));
428
429 /*
430 * Otherwise an interrupt, in the middle of staging and possibly executing
431 * the IO, could end up trying to wait for the IO, leading to state
432 * confusion.
433 */
434 HOLD_INTERRUPTS();
435
436 ioh->op = op;
437 ioh->result = 0;
438
439 pgaio_io_update_state(ioh, PGAIO_HS_DEFINED);
440
441 /* allow a new IO to be staged */
442 pgaio_my_backend->handed_out_io = NULL;
443
444 pgaio_io_call_stage(ioh);
445
446 pgaio_io_update_state(ioh, PGAIO_HS_STAGED);
447
448 /*
449 * Synchronous execution has to be executed, well, synchronously, so check
450 * that first.
451 */
452 needs_synchronous = pgaio_io_needs_synchronous_execution(ioh);
453
454 pgaio_debug_io(DEBUG3, ioh,
455 "staged (synchronous: %d, in_batch: %d)",
456 needs_synchronous, pgaio_my_backend->in_batchmode);
457
458 if (!needs_synchronous)
459 {
460 pgaio_my_backend->staged_ios[pgaio_my_backend->num_staged_ios++] = ioh;
461 Assert(pgaio_my_backend->num_staged_ios <= PGAIO_SUBMIT_BATCH_SIZE);
462
463 /*
464 * Unless code explicitly opted into batching IOs, submit the IO
465 * immediately.
466 */
467 if (!pgaio_my_backend->in_batchmode)
468 pgaio_submit_staged();
469 }
470 else
471 {
472 pgaio_io_prepare_submit(ioh);
473 pgaio_io_perform_synchronously(ioh);
474 }
475
476 RESUME_INTERRUPTS();
477 }
478
479 bool
480 pgaio_io_needs_synchronous_execution(PgAioHandle *ioh)
481 {
482 /*
483 * If the caller said to execute the IO synchronously, do so.
484 *
485 * XXX: We could optimize the logic when to execute synchronously by first
486 * checking if there are other IOs in flight and only synchronously
487 * executing if not. Unclear whether that'll be sufficiently common to be
488 * worth worrying about.
489 */
490 if (ioh->flags & PGAIO_HF_SYNCHRONOUS)
491 return true;
492
493 /* Check if the IO method requires synchronous execution of IO */
494 if (pgaio_method_ops->needs_synchronous_execution)
495 return pgaio_method_ops->needs_synchronous_execution(ioh);
496
497 return false;
498 }
499
500 /*
501 * Handle IO being processed by IO method.
502 *
503 * Should be called by IO methods / synchronous IO execution, just before the
504 * IO is performed.
505 */
506 void
507 pgaio_io_prepare_submit(PgAioHandle *ioh)
508 {
509 pgaio_io_update_state(ioh, PGAIO_HS_SUBMITTED);
510
511 dclist_push_tail(&pgaio_my_backend->in_flight_ios, &ioh->node);
512 }
513
514 /*
515 * Handle IO getting completed by a method.
516 *
517 * Should be called by IO methods / synchronous IO execution, just after the
518 * IO has been performed.
519 *
520 * Expects to be called in a critical section. We expect IOs to be usable for
521 * WAL etc, which requires being able to execute completion callbacks in a
522 * critical section.
523 */
524 void
525 pgaio_io_process_completion(PgAioHandle *ioh, int result)
526 {
527 Assert(ioh->state == PGAIO_HS_SUBMITTED);
528
529 Assert(CritSectionCount > 0);
530
531 ioh->result = result;
532
533 pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_IO);
534
535 INJECTION_POINT("aio-process-completion-before-shared", ioh);
536
537 pgaio_io_call_complete_shared(ioh);
538
539 pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_SHARED);
540
541 /* condition variable broadcast ensures state is visible before wakeup */
542 ConditionVariableBroadcast(&ioh->cv);
543
544 /* contains call to pgaio_io_call_complete_local() */
545 if (ioh->owner_procno == MyProcNumber)
546 pgaio_io_reclaim(ioh);
547 }
548
549 /*
550 * Has the IO completed and thus the IO handle been reused?
551 *
552 * This is useful when waiting for IO completion at a low level (e.g. in an IO
553 * method's ->wait_one() callback).
554 */
555 bool
556 pgaio_io_was_recycled(PgAioHandle *ioh, uint64 ref_generation, PgAioHandleState *state)
557 {
558 *state = ioh->state;
559
560 /*
561 * Ensure that we don't see an earlier state of the handle than ioh->state
562 * due to compiler or CPU reordering. This protects both ->generation as
563 * directly used here, and other fields in the handle accessed in the
564 * caller if the handle was not reused.
565 */
566 pg_read_barrier();
567
568 return ioh->generation != ref_generation;
569 }
570
571 /*
572 * Wait for IO to complete. External code should never use this, outside of
573 * the AIO subsystem waits are only allowed via pgaio_wref_wait().
574 */
575 static void
576 pgaio_io_wait(PgAioHandle *ioh, uint64 ref_generation)
577 {
578 PgAioHandleState state;
579 bool am_owner;
580
581 am_owner = ioh->owner_procno == MyProcNumber;
582
583 if (pgaio_io_was_recycled(ioh, ref_generation, &state))
584 return;
585
586 if (am_owner)
587 {
588 if (state != PGAIO_HS_SUBMITTED
589 && state != PGAIO_HS_COMPLETED_IO
590 && state != PGAIO_HS_COMPLETED_SHARED
591 && state != PGAIO_HS_COMPLETED_LOCAL)
592 {
593 elog(PANIC, "waiting for own IO %d in wrong state: %s",
594 pgaio_io_get_id(ioh), pgaio_io_get_state_name(ioh));
595 }
596 }
597
598 while (true)
599 {
600 if (pgaio_io_was_recycled(ioh, ref_generation, &state))
601 return;
602
603 switch ((PgAioHandleState) state)
604 {
605 case PGAIO_HS_IDLE:
606 case PGAIO_HS_HANDED_OUT:
607 elog(ERROR, "IO in wrong state: %d", state);
608 break;
609
610 case PGAIO_HS_SUBMITTED:
611
612 /*
613 * If we need to wait via the IO method, do so now. Don't
614 * check via the IO method if the issuing backend is executing
615 * the IO synchronously.
616 */
617 if (pgaio_method_ops->wait_one && !(ioh->flags & PGAIO_HF_SYNCHRONOUS))
618 {
619 pgaio_method_ops->wait_one(ioh, ref_generation);
620 continue;
621 }
622 /* fallthrough */
623
624 /* waiting for owner to submit */
625 case PGAIO_HS_DEFINED:
626 case PGAIO_HS_STAGED:
627 /* waiting for reaper to complete */
628 /* fallthrough */
629 case PGAIO_HS_COMPLETED_IO:
630 /* shouldn't be able to hit this otherwise */
631 Assert(IsUnderPostmaster);
632 /* ensure we're going to get woken up */
633 ConditionVariablePrepareToSleep(&ioh->cv);
634
635 while (!pgaio_io_was_recycled(ioh, ref_generation, &state))
636 {
637 if (state == PGAIO_HS_COMPLETED_SHARED ||
638 state == PGAIO_HS_COMPLETED_LOCAL)
639 break;
640 ConditionVariableSleep(&ioh->cv, WAIT_EVENT_AIO_IO_COMPLETION);
641 }
642
643 ConditionVariableCancelSleep();
644 break;
645
646 case PGAIO_HS_COMPLETED_SHARED:
647 case PGAIO_HS_COMPLETED_LOCAL:
648
649 /*
650 * Note that no interrupts are processed between
651 * pgaio_io_was_recycled() and this check - that's important
652 * as otherwise an interrupt could have already reclaimed the
653 * handle.
654 */
655 if (am_owner)
656 pgaio_io_reclaim(ioh);
657 return;
658 }
659 }
660 }
661
662 /*
663 * Make IO handle ready to be reused after IO has completed or after the
664 * handle has been released without being used.
665 *
666 * Note that callers need to be careful about only calling this in the right
667 * state and that no interrupts can be processed between the state check and
668 * the call to pgaio_io_reclaim(). Otherwise interrupt processing could
669 * already have reclaimed the handle.
670 */
671 static void
672 pgaio_io_reclaim(PgAioHandle *ioh)
673 {
0x000000000043662e <+0>: addi sp,sp,-64
0x0000000000436630 <+2>: sd ra,56(sp)
0x0000000000436632 <+4>: sd s0,48(sp)
0x0000000000436634 <+6>: sd s1,40(sp)
0x0000000000436636 <+8>: sd s2,32(sp)
0x0000000000436638 <+10>: sd s3,24(sp)
0x000000000043663a <+12>: sd s4,16(sp)
674 /* This is only ok if it's our IO */
675 Assert(ioh->owner_procno == MyProcNumber);
0x000000000043663c <+14>: auipc a1,0x4a6
0x0000000000436640 <+18>: ld a1,-2020(a1) # 0x8dbe58
0x0000000000436644 <+22>: mv s0,a0
0x0000000000436646 <+24>: lw a0,16(a0)
0x0000000000436648 <+26>: lw a1,0(a1)
0x000000000043664a <+28>: bne a0,a1,0x43684e <pgaio_io_reclaim+544>
0x000000000043684e <+544>: auipc a0,0x242
0x0000000000436852 <+548>: addi a0,a0,-494 # 0x678660
0x0000000000436856 <+552>: auipc a1,0x242
0x000000000043685a <+556>: addi a1,a1,-601 # 0x6785fd
0x000000000043685e <+560>: li a2,675
0x0000000000436862 <+564>: auipc ra,0x160
0x0000000000436866 <+568>: jalr 230(ra) # 0x596948 <ExceptionalCondition>
676 Assert(ioh->state != PGAIO_HS_IDLE);
0x000000000043664e <+32>: lbu a0,0(s0)
0x0000000000436652 <+36>: beqz a0,0x43686a <pgaio_io_reclaim+572>
0x000000000043686a <+572>: auipc a0,0x242
0x000000000043686e <+576>: addi a0,a0,1037 # 0x678c77
0x0000000000436872 <+580>: auipc a1,0x242
0x0000000000436876 <+584>: addi a1,a1,-629 # 0x6785fd
0x000000000043687a <+588>: li a2,676
0x000000000043687e <+592>: auipc ra,0x160
0x0000000000436882 <+596>: jalr 202(ra) # 0x596948 <ExceptionalCondition>
677
678 /* see comment in function header */
679 HOLD_INTERRUPTS();
0x0000000000436656 <+40>: auipc a0,0x4a6
0x000000000043665a <+44>: ld s2,34(a0) # 0x8dc678
0x000000000043665e <+48>: lw a0,0(s2)
0x0000000000436662 <+52>: addi a0,a0,1
0x0000000000436664 <+54>: sw a0,0(s2)
680
681 /*
682 * It's a bit ugly, but right now the easiest place to put the execution
683 * of local completion callbacks is this function, as we need to execute
684 * local callbacks just before reclaiming at multiple callsites.
685 */
686 if (ioh->state == PGAIO_HS_COMPLETED_SHARED)
0x0000000000436668 <+58>: lbu a0,0(s0)
0x000000000043666c <+62>: li a1,6
0x000000000043666e <+64>: bne a0,a1,0x4366ae <pgaio_io_reclaim+128>
687 {
688 PgAioResult local_result;
689
690 local_result = pgaio_io_call_complete_local(ioh);
0x0000000000436672 <+68>: mv a0,s0
0x0000000000436674 <+70>: jal 0x4381de <pgaio_io_call_complete_local>
0x0000000000436678 <+74>: mv s1,a0
691 pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_LOCAL);
0x000000000043667a <+76>: li a1,7
0x000000000043667c <+78>: mv a0,s0
0x000000000043667e <+80>: jal 0x436436 <pgaio_io_update_state>
692
693 if (ioh->report_return)
0x0000000000436682 <+84>: ld a0,96(s0)
0x0000000000436684 <+86>: beqz a0,0x4366ae <pgaio_io_reclaim+128>
694 {
695 ioh->report_return->result = local_result;
0x0000000000436686 <+88>: sw s1,0(a0)
0x0000000000436688 <+90>: srli s1,s1,0x20
0x000000000043668a <+92>: sw s1,4(a0)
696 ioh->report_return->target_data = ioh->target_data;
0x000000000043668c <+94>: ld a0,96(s0)
0x000000000043668e <+96>: lw a1,140(s0)
0x0000000000436692 <+100>: sw a1,28(a0)
0x0000000000436694 <+102>: lw a1,136(s0)
0x0000000000436698 <+106>: sw a1,24(a0)
0x000000000043669a <+108>: lw a1,132(s0)
0x000000000043669e <+112>: sw a1,20(a0)
0x00000000004366a0 <+114>: lw a1,128(s0)
0x00000000004366a4 <+118>: sw a1,16(a0)
0x00000000004366a6 <+120>: lw a1,124(s0)
0x00000000004366a8 <+122>: sw a1,12(a0)
0x00000000004366aa <+124>: lw a1,120(s0)
0x00000000004366ac <+126>: sw a1,8(a0)
697 }
698 }
699
700 pgaio_debug_io(DEBUG4, ioh,
0x00000000004366ae <+128>: li a0,11
0x00000000004366b0 <+130>: li a1,0
0x00000000004366b2 <+132>: auipc ra,0x161
0x00000000004366b6 <+136>: jalr -1776(ra) # 0x596fc2 <errstart>
0x00000000004366ba <+140>: beqz a0,0x436788 <pgaio_io_reclaim+346>
0x00000000004366bc <+142>: li a0,1
0x00000000004366be <+144>: auipc ra,0x163
0x00000000004366c2 <+148>: jalr 688(ra) # 0x59996e <errhidestmt>
0x00000000004366c6 <+152>: li a0,1
0x00000000004366c8 <+154>: auipc ra,0x163
0x00000000004366cc <+158>: jalr 780(ra) # 0x5999d4 <errhidecontext>
0x00000000004366fe <+208>: mv a0,s0
0x0000000000436700 <+210>: jal 0x438f90 <pgaio_io_get_op_name>
0x0000000000436704 <+214>: mv s3,a0
0x0000000000436706 <+216>: mv a0,s0
0x0000000000436708 <+218>: jal 0x439076 <pgaio_io_get_target_name>
0x0000000000436712 <+228>: mv a3,a0
0x000000000043672a <+252>: lw a0,84(s0)
0x000000000043672c <+254>: slli a2,a0,0x37
0x0000000000436730 <+258>: srli a2,a2,0x3d
0x0000000000436732 <+260>: li a5,4
0x0000000000436734 <+262>: mulw a1,s1,s4
0x000000000043674e <+288>: lw a2,20(s0)
0x0000000000436750 <+290>: andi a6,a0,63
0x0000000000436754 <+294>: srliw a7,a0,0x9
0x0000000000436758 <+298>: auipc a0,0x242
0x000000000043675c <+302>: addi a0,a0,1339 # 0x678c93
0x0000000000436760 <+306>: sd a2,0(sp)
0x0000000000436762 <+308>: mv a2,s3
0x0000000000436764 <+310>: auipc ra,0x161
0x0000000000436768 <+314>: jalr -642(ra) # 0x5974e2 <errmsg_internal>
0x000000000043676c <+318>: auipc a0,0x242
0x0000000000436770 <+322>: addi a0,a0,-367 # 0x6785fd
0x0000000000436774 <+326>: auipc a1,0x242
0x0000000000436778 <+330>: addi a2,a1,1433 # 0x678d0d
0x000000000043677c <+334>: li a1,705
0x0000000000436780 <+338>: auipc ra,0x161
0x0000000000436784 <+342>: jalr -1284(ra) # 0x59727c <errfinish>
701 "reclaiming: distilled_result: (status %s, id %u, error_data %d), raw_result: %d",
702 pgaio_result_status_string(ioh->distilled_result.status),
703 ioh->distilled_result.id,
704 ioh->distilled_result.error_data,
705 ioh->result);
706
707 /* if the IO has been defined, it's on the in-flight list, remove */
708 if (ioh->state != PGAIO_HS_HANDED_OUT)
0x0000000000436788 <+346>: lbu a0,0(s0)
0x000000000043678c <+350>: li a1,1
0x000000000043678e <+352>: auipc s1,0x4c3
0x0000000000436792 <+356>: beq a0,a1,0x4367b6 <pgaio_io_reclaim+392>
709 dclist_delete_from(&pgaio_my_backend->in_flight_ios, &ioh->node);
0x0000000000436796 <+360>: ld a0,-1390(s1)
710
711 if (ioh->resowner)
0x00000000004367b6 <+392>: ld a0,40(s0)
0x00000000004367b8 <+394>: beqz a0,0x4367ca <pgaio_io_reclaim+412>
712 {
713 ResourceOwnerForgetAioHandle(ioh->resowner, &ioh->resowner_node);
0x00000000004367ba <+396>: addi a1,s0,48
0x00000000004367be <+400>: auipc ra,0x18d
0x00000000004367c2 <+404>: jalr 1566(ra) # 0x5c3ddc <ResourceOwnerForgetAioHandle>
714 ioh->resowner = NULL;
0x00000000004367c6 <+408>: sd zero,40(s0)
715 }
716
717 Assert(!ioh->resowner);
718
719 /*
720 * Update generation & state first, before resetting the IO's fields,
721 * otherwise a concurrent "viewer" could think the fields are valid, even
722 * though they are being reset. Increment the generation first, so that
723 * we can assert elsewhere that we never wait for an IDLE IO. While it's
724 * a bit weird for the state to go backwards for a generation, it's OK
725 * here, as there cannot be references to the "reborn" IO yet. Can't
726 * update both at once, so something has to give.
727 */
728 ioh->generation++;
0x00000000004367ca <+412>: ld a0,64(s0)
0x00000000004367cc <+414>: addi a0,a0,1
0x00000000004367ce <+416>: sd a0,64(s0)
729 pgaio_io_update_state(ioh, PGAIO_HS_IDLE);
0x00000000004367d0 <+418>: mv a0,s0
0x00000000004367d2 <+420>: li a1,0
0x00000000004367d4 <+422>: jal 0x436436 <pgaio_io_update_state>
730
731 /* ensure the state update is visible before we reset fields */
732 pg_write_barrier();
0x00000000004367d8 <+426>: fence rw,w
733
734 ioh->op = PGAIO_OP_INVALID;
735 ioh->target = PGAIO_TID_INVALID;
0x00000000004367e4 <+438>: sb zero,4(s0)
0x00000000004367e8 <+442>: sb zero,3(s0)
0x00000000004367ec <+446>: sb zero,2(s0)
0x00000000004367f4 <+454>: sb zero,1(s0)
736 ioh->flags = 0;
737 ioh->num_callbacks = 0;
738 ioh->handle_data_len = 0;
0x00000000004367dc <+430>: sb zero,13(s0)
739 ioh->report_return = NULL;
0x00000000004367e0 <+434>: sd zero,96(s0)
740 ioh->result = 0;
0x00000000004367fc <+462>: sw zero,20(s0)
741 ioh->distilled_result.status = PGAIO_RS_UNKNOWN;
0x00000000004367f0 <+450>: lwu a1,84(s0)
0x0000000000436800 <+466>: andi a1,a1,-449
0x0000000000436804 <+470>: sw a1,84(s0)
742
743 /*
744 * We push the IO to the head of the idle IO list, that seems more cache
745 * efficient in cases where only a few IOs are used.
746 */
747 dclist_push_head(&pgaio_my_backend->idle_ios, &ioh->node);
0x00000000004367f8 <+458>: ld a0,-1390(s1)
0x0000000000436808 <+474>: addi a2,a0,8
0x000000000043680c <+478>: addi a1,s0,24
748
749 RESUME_INTERRUPTS();
0x000000000043682e <+512>: lw a0,0(s2)
0x0000000000436832 <+516>: beqz a0,0x4368be <pgaio_io_reclaim+656>
0x0000000000436834 <+518>: lw a0,0(s2)
0x0000000000436838 <+522>: addi a0,a0,-1
0x000000000043683a <+524>: sw a0,0(s2)
0x000000000043683e <+528>: ld ra,56(sp)
0x0000000000436840 <+530>: ld s0,48(sp)
0x0000000000436842 <+532>: ld s1,40(sp)
0x0000000000436844 <+534>: ld s2,32(sp)
0x0000000000436846 <+536>: ld s3,24(sp)
0x0000000000436848 <+538>: ld s4,16(sp)
0x00000000004368be <+656>: auipc a0,0x1cd
0x00000000004368c2 <+660>: addi a0,a0,-716 # 0x6035f2
0x00000000004368c6 <+664>: auipc a1,0x242
0x00000000004368ca <+668>: addi a1,a1,-713 # 0x6785fd
0x00000000004368ce <+672>: li a2,749
0x00000000004368d2 <+676>: auipc ra,0x160
0x00000000004368d6 <+680>: jalr 118(ra) # 0x596948 <ExceptionalCondition>
750 }
0x000000000043684a <+540>: addi sp,sp,64
0x000000000043684c <+542>: ret
751
752 /*
753 * Wait for an IO handle to become usable.
754 *
755 * This only really is useful for pgaio_io_acquire().
756 */
757 static void
758 pgaio_io_wait_for_free(void)
759 {
760 int reclaimed = 0;
761
762 pgaio_debug(DEBUG2, "waiting for free IO with %d pending, %u in-flight, %u idle IOs",
763 pgaio_my_backend->num_staged_ios,
764 dclist_count(&pgaio_my_backend->in_flight_ios),
765 dclist_count(&pgaio_my_backend->idle_ios));
766
767 /*
768 * First check if any of our IOs actually have completed - when using
769 * worker, that'll often be the case. We could do so as part of the loop
770 * below, but that'd potentially lead us to wait for some IO submitted
771 * before.
772 */
773 for (int i = 0; i < io_max_concurrency; i++)
774 {
775 PgAioHandle *ioh = &pgaio_ctl->io_handles[pgaio_my_backend->io_handle_off + i];
776
777 if (ioh->state == PGAIO_HS_COMPLETED_SHARED)
778 {
779 /*
780 * Note that no interrupts are processed between the state check
781 * and the call to reclaim - that's important as otherwise an
782 * interrupt could have already reclaimed the handle.
783 *
784 * Need to ensure that there's no reordering, in the more common
785 * paths, where we wait for IO, that's done by
786 * pgaio_io_was_recycled().
787 */
788 pg_read_barrier();
789 pgaio_io_reclaim(ioh);
790 reclaimed++;
791 }
792 }
793
794 if (reclaimed > 0)
795 return;
796
797 /*
798 * If we have any unsubmitted IOs, submit them now. We'll start waiting in
799 * a second, so it's better they're in flight. This also addresses the
800 * edge-case that all IOs are unsubmitted.
801 */
802 if (pgaio_my_backend->num_staged_ios > 0)
803 pgaio_submit_staged();
804
805 /* possibly some IOs finished during submission */
806 if (!dclist_is_empty(&pgaio_my_backend->idle_ios))
807 return;
808
809 if (dclist_count(&pgaio_my_backend->in_flight_ios) == 0)
810 ereport(ERROR,
811 errmsg_internal("no free IOs despite no in-flight IOs"),
812 errdetail_internal("%d pending, %u in-flight, %u idle IOs",
813 pgaio_my_backend->num_staged_ios,
814 dclist_count(&pgaio_my_backend->in_flight_ios),
815 dclist_count(&pgaio_my_backend->idle_ios)));
816
817 /*
818 * Wait for the oldest in-flight IO to complete.
819 *
820 * XXX: Reusing the general IO wait is suboptimal, we don't need to wait
821 * for that specific IO to complete, we just need *any* IO to complete.
822 */
823 {
824 PgAioHandle *ioh = dclist_head_element(PgAioHandle, node,
825 &pgaio_my_backend->in_flight_ios);
826 uint64 generation = ioh->generation;
827
828 switch ((PgAioHandleState) ioh->state)
829 {
830 /* should not be in in-flight list */
831 case PGAIO_HS_IDLE:
832 case PGAIO_HS_DEFINED:
833 case PGAIO_HS_HANDED_OUT:
834 case PGAIO_HS_STAGED:
835 case PGAIO_HS_COMPLETED_LOCAL:
836 elog(ERROR, "shouldn't get here with io:%d in state %d",
837 pgaio_io_get_id(ioh), ioh->state);
838 break;
839
840 case PGAIO_HS_COMPLETED_IO:
841 case PGAIO_HS_SUBMITTED:
842 pgaio_debug_io(DEBUG2, ioh,
843 "waiting for free io with %u in flight",
844 dclist_count(&pgaio_my_backend->in_flight_ios));
845
846 /*
847 * In a more general case this would be racy, because the
848 * generation could increase after we read ioh->state above.
849 * But we are only looking at IOs by the current backend and
850 * the IO can only be recycled by this backend. Even this is
851 * only OK because we get the handle's generation before
852 * potentially processing interrupts, e.g. as part of
853 * pgaio_debug_io().
854 */
855 pgaio_io_wait(ioh, generation);
856 break;
857
858 case PGAIO_HS_COMPLETED_SHARED:
859
860 /*
861 * It's possible that another backend just finished this IO.
862 *
863 * Note that no interrupts are processed between the state
864 * check and the call to reclaim - that's important as
865 * otherwise an interrupt could have already reclaimed the
866 * handle.
867 *
868 * Need to ensure that there's no reordering, in the more
869 * common paths, where we wait for IO, that's done by
870 * pgaio_io_was_recycled().
871 */
872 pg_read_barrier();
873 pgaio_io_reclaim(ioh);
874 break;
875 }
876
877 if (dclist_count(&pgaio_my_backend->idle_ios) == 0)
878 elog(PANIC, "no idle IO after waiting for IO to terminate");
879 return;
880 }
881 }
882
883 /*
884 * Internal - code outside of AIO should never need this and it'd be hard for
885 * such code to be safe.
886 */
887 static PgAioHandle *
888 pgaio_io_from_wref(PgAioWaitRef *iow, uint64 *ref_generation)
889 {
890 PgAioHandle *ioh;
891
892 Assert(iow->aio_index < pgaio_ctl->io_handle_count);
893
894 ioh = &pgaio_ctl->io_handles[iow->aio_index];
895
896 *ref_generation = ((uint64) iow->generation_upper) << 32 |
897 iow->generation_lower;
898
899 Assert(*ref_generation != 0);
900
901 return ioh;
902 }
903
904 static const char *
905 pgaio_io_state_get_name(PgAioHandleState s)
906 {
907 #define PGAIO_HS_TOSTR_CASE(sym) case PGAIO_HS_##sym: return #sym
908 switch ((PgAioHandleState) s)
0x0000000000436714 <+230>: bltu a2,a1,0x436728 <pgaio_io_reclaim+250>
0x0000000000436718 <+234>: slli a1,a1,0x3
0x000000000043671a <+236>: auipc a0,0x485
0x000000000043671e <+240>: addi a0,a0,-1106 # 0x8bb2c8
0x0000000000436722 <+244>: add a0,a0,a1
0x0000000000436724 <+246>: ld a4,0(a0)
0x0000000000436726 <+248>: j 0x43672a <pgaio_io_reclaim+252>
0x0000000000436728 <+250>: li a4,0
909 {
910 PGAIO_HS_TOSTR_CASE(IDLE);
911 PGAIO_HS_TOSTR_CASE(HANDED_OUT);
912 PGAIO_HS_TOSTR_CASE(DEFINED);
913 PGAIO_HS_TOSTR_CASE(STAGED);
914 PGAIO_HS_TOSTR_CASE(SUBMITTED);
915 PGAIO_HS_TOSTR_CASE(COMPLETED_IO);
916 PGAIO_HS_TOSTR_CASE(COMPLETED_SHARED);
917 PGAIO_HS_TOSTR_CASE(COMPLETED_LOCAL);
918 }
919 #undef PGAIO_HS_TOSTR_CASE
920
921 return NULL; /* silence compiler */
922 }
923
924 const char *
925 pgaio_io_get_state_name(PgAioHandle *ioh)
926 {
927 return pgaio_io_state_get_name(ioh->state);
0x000000000043670c <+222>: lbu a1,0(s0)
0x0000000000436710 <+226>: li a2,7
928 }
929
930 const char *
931 pgaio_result_status_string(PgAioResultStatus rs)
932 {
933 switch ((PgAioResultStatus) rs)
0x0000000000436738 <+266>: bltu a5,a2,0x43674c <pgaio_io_reclaim+286>
0x000000000043673c <+270>: slli a2,a2,0x3
0x000000000043673e <+272>: auipc a5,0x485
0x0000000000436742 <+276>: addi a5,a5,-1182 # 0x8bb2a0
0x0000000000436746 <+280>: add a2,a2,a5
0x0000000000436748 <+282>: ld a5,0(a2)
0x000000000043674a <+284>: j 0x43674e <pgaio_io_reclaim+288>
0x000000000043674c <+286>: li a5,0
End of assembler dump.
(gdb)
* Re: IO in wrong state on riscv64
@ 2025-10-13 05:40 Thomas Munro <thomas.munro@gmail.com>
parent: Alexander Lakhin <exclusion@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Munro @ 2025-10-13 05:40 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
On Mon, Oct 13, 2025 at 5:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
> 13.10.2025 01:44, Thomas Munro wrote:
> > On Sun, Oct 12, 2025 at 6:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
> >> Please find those attached (gdb "disass/m pgaio_io_update_state" misses
> >> the start of the function (but it's still disassembled below), so I
> >> decided to share the whole output).
> > Could you please also disassemble pgaio_io_reclaim()?
>
> Sure, the output of disass/m pgaio_io_reclaim is attached.
Thanks. All seems to have something plausible in the right places,
but I know nothing about RISC-V... hmm, what happens if you replace
pg_{read,write}_barrier() with pg_memory_barrier(), in those three
functions? And if it happens to help, perhaps you could try to figure
out which one(s) help? Not that it should be necessary but as a clue
or to rule out this line of enquiry... I guess that should generate
FENCE RW,RW, meaning wait for all preceding reads and writes to
complete and don't let any following reads or writes begin, but that's
just from googling...
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-10-14 04:00 Alexander Lakhin <exclusion@gmail.com>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Alexander Lakhin @ 2025-10-14 04:00 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
Hello Thomas,
13.10.2025 08:40, Thomas Munro wrote:
> Thanks. All seems to have something plausible in the right places,
> but I know nothing about RISC-V... hmm, what happens if you replace
> pg_{read,write}_barrier() with pg_memory_barrier(), in those three
> functions? And if it happens to help, perhaps you could try to figure
> out which one(s) help? Not that it should be necessary but as a clue
> or to rule out this line of enquiry... I guess that should generate
> FENCE RW,RW, meaning wait for all preceding reads and writes to
> complete and don't let any following reads or writes begin, but that's
> just from googling...
The replacements doesn't work for me, unfortunately: I have 3 out of 30
027_stream_regress test runs failed:
2025-10-13 21:27:26.161 UTC [4181290:5] pg_regress/brin ERROR: IO in wrong state: 0
2025-10-13 21:27:26.161 UTC [4181290:6] pg_regress/brin STATEMENT: CREATE TABLE brintest (byteacol bytea,
Disassembly of those three functions is attached.
I'll try to find some simple C programs to test memory barriers outside of
postgres...
Best regards,
Alexander
Attachments:
pgaio_io_reclaim.asm (text/plain, 28.2K) download
GNU gdb (Debian 15.2-1) 15.2
Copyright (C) 2024 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "riscv64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from tmp_install/usr/local/pgsql/bin/postgres...
(gdb) Dump of assembler code for function pgaio_io_reclaim:
341 Assert(ioh >= pgaio_ctl->io_handles &&
0x00000000004366d0 <+162>: auipc a0,0x4c3
0x00000000004366d4 <+166>: ld a1,-1192(a0) # 0x8f9228 <pgaio_ctl>
0x00000000004366d8 <+170>: ld a0,48(a1)
0x00000000004366da <+172>: bltu s0,a0,0x436886 <pgaio_io_reclaim+600>
0x00000000004366de <+176>: lwu a1,40(a1)
0x00000000004366e2 <+180>: li a2,144
0x00000000004366e6 <+184>: mul a1,a1,a2
0x00000000004366ea <+188>: add a1,a1,a0
0x00000000004366ec <+190>: bgeu s0,a1,0x436886 <pgaio_io_reclaim+600>
0x00000000004366f0 <+194>: auipc a1,0x4c1
0x00000000004366f4 <+198>: ld s4,-296(a1) # 0x8f75c8
0x0000000000436886 <+600>: auipc a0,0x242
0x000000000043688a <+604>: addi a0,a0,-271 # 0x678777
0x000000000043688e <+608>: auipc a1,0x242
0x0000000000436892 <+612>: addi a1,a1,-657 # 0x6785fd
0x0000000000436896 <+616>: li a2,342
0x000000000043689a <+620>: auipc ra,0x160
0x000000000043689e <+624>: jalr 174(ra) # 0x596948 <ExceptionalCondition>
342 ioh < (pgaio_ctl->io_handles + pgaio_ctl->io_handle_count));
343 return ioh - pgaio_ctl->io_handles;
0x00000000004366f8 <+202>: sub s1,s0,a0
0x00000000004366fc <+206>: srai s1,s1,0x4
344 }
345
346 /*
347 * Return the ProcNumber for the process that can use an IO handle. The
348 * mapping from IO handles to PGPROCs is static, therefore this even works
349 * when the corresponding PGPROC is not in use.
350 */
351 ProcNumber
352 pgaio_io_get_owner(PgAioHandle *ioh)
353 {
354 return ioh->owner_procno;
355 }
356
357 /*
358 * Return a wait reference for the IO. Only wait references can be used to
359 * wait for an IOs completion, as handles themselves can be reused after
360 * completion. See also the comment above pgaio_io_acquire().
361 */
362 void
363 pgaio_io_get_wref(PgAioHandle *ioh, PgAioWaitRef *iow)
364 {
365 Assert(ioh->state == PGAIO_HS_HANDED_OUT ||
366 ioh->state == PGAIO_HS_DEFINED ||
367 ioh->state == PGAIO_HS_STAGED);
368 Assert(ioh->generation != 0);
369
370 iow->aio_index = ioh - pgaio_ctl->io_handles;
371 iow->generation_upper = (uint32) (ioh->generation >> 32);
372 iow->generation_lower = (uint32) ioh->generation;
373 }
374
375
376
377 /* --------------------------------------------------------------------------------
378 * Internal Functions related to PgAioHandle
379 * --------------------------------------------------------------------------------
380 */
381
382 static inline void
383 pgaio_io_update_state(PgAioHandle *ioh, PgAioHandleState new_state)
384 {
385 /*
386 * All callers need to have held interrupts in some form, otherwise
387 * interrupt processing could wait for the IO to complete, while in an
388 * intermediary state.
389 */
390 Assert(!INTERRUPTS_CAN_BE_PROCESSED());
391
392 pgaio_debug_io(DEBUG5, ioh,
393 "updating state to %s",
394 pgaio_io_state_get_name(new_state));
395
396 /*
397 * Ensure the changes signified by the new state are visible before the
398 * new state becomes visible.
399 */
400 pg_memory_barrier();
401
402 ioh->state = new_state;
403 }
404
405 static void
406 pgaio_io_resowner_register(PgAioHandle *ioh)
407 {
408 Assert(!ioh->resowner);
409 Assert(CurrentResourceOwner);
410
411 ResourceOwnerRememberAioHandle(CurrentResourceOwner, &ioh->resowner_node);
412 ioh->resowner = CurrentResourceOwner;
413 }
414
415 /*
416 * Stage IO for execution and, if appropriate, submit it immediately.
417 *
418 * Should only be called from pgaio_io_start_*().
419 */
420 void
421 pgaio_io_stage(PgAioHandle *ioh, PgAioOp op)
422 {
423 bool needs_synchronous;
424
425 Assert(ioh->state == PGAIO_HS_HANDED_OUT);
426 Assert(pgaio_my_backend->handed_out_io == ioh);
427 Assert(pgaio_io_has_target(ioh));
428
429 /*
430 * Otherwise an interrupt, in the middle of staging and possibly executing
431 * the IO, could end up trying to wait for the IO, leading to state
432 * confusion.
433 */
434 HOLD_INTERRUPTS();
435
436 ioh->op = op;
437 ioh->result = 0;
438
439 pgaio_io_update_state(ioh, PGAIO_HS_DEFINED);
440
441 /* allow a new IO to be staged */
442 pgaio_my_backend->handed_out_io = NULL;
443
444 pgaio_io_call_stage(ioh);
445
446 pgaio_io_update_state(ioh, PGAIO_HS_STAGED);
447
448 /*
449 * Synchronous execution has to be executed, well, synchronously, so check
450 * that first.
451 */
452 needs_synchronous = pgaio_io_needs_synchronous_execution(ioh);
453
454 pgaio_debug_io(DEBUG3, ioh,
455 "staged (synchronous: %d, in_batch: %d)",
456 needs_synchronous, pgaio_my_backend->in_batchmode);
457
458 if (!needs_synchronous)
459 {
460 pgaio_my_backend->staged_ios[pgaio_my_backend->num_staged_ios++] = ioh;
461 Assert(pgaio_my_backend->num_staged_ios <= PGAIO_SUBMIT_BATCH_SIZE);
462
463 /*
464 * Unless code explicitly opted into batching IOs, submit the IO
465 * immediately.
466 */
467 if (!pgaio_my_backend->in_batchmode)
468 pgaio_submit_staged();
469 }
470 else
471 {
472 pgaio_io_prepare_submit(ioh);
473 pgaio_io_perform_synchronously(ioh);
474 }
475
476 RESUME_INTERRUPTS();
477 }
478
479 bool
480 pgaio_io_needs_synchronous_execution(PgAioHandle *ioh)
481 {
482 /*
483 * If the caller said to execute the IO synchronously, do so.
484 *
485 * XXX: We could optimize the logic when to execute synchronously by first
486 * checking if there are other IOs in flight and only synchronously
487 * executing if not. Unclear whether that'll be sufficiently common to be
488 * worth worrying about.
489 */
490 if (ioh->flags & PGAIO_HF_SYNCHRONOUS)
491 return true;
492
493 /* Check if the IO method requires synchronous execution of IO */
494 if (pgaio_method_ops->needs_synchronous_execution)
495 return pgaio_method_ops->needs_synchronous_execution(ioh);
496
497 return false;
498 }
499
500 /*
501 * Handle IO being processed by IO method.
502 *
503 * Should be called by IO methods / synchronous IO execution, just before the
504 * IO is performed.
505 */
506 void
507 pgaio_io_prepare_submit(PgAioHandle *ioh)
508 {
509 pgaio_io_update_state(ioh, PGAIO_HS_SUBMITTED);
510
511 dclist_push_tail(&pgaio_my_backend->in_flight_ios, &ioh->node);
512 }
513
514 /*
515 * Handle IO getting completed by a method.
516 *
517 * Should be called by IO methods / synchronous IO execution, just after the
518 * IO has been performed.
519 *
520 * Expects to be called in a critical section. We expect IOs to be usable for
521 * WAL etc, which requires being able to execute completion callbacks in a
522 * critical section.
523 */
524 void
525 pgaio_io_process_completion(PgAioHandle *ioh, int result)
526 {
527 Assert(ioh->state == PGAIO_HS_SUBMITTED);
528
529 Assert(CritSectionCount > 0);
530
531 ioh->result = result;
532
533 pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_IO);
534
535 INJECTION_POINT("aio-process-completion-before-shared", ioh);
536
537 pgaio_io_call_complete_shared(ioh);
538
539 pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_SHARED);
540
541 /* condition variable broadcast ensures state is visible before wakeup */
542 ConditionVariableBroadcast(&ioh->cv);
543
544 /* contains call to pgaio_io_call_complete_local() */
545 if (ioh->owner_procno == MyProcNumber)
546 pgaio_io_reclaim(ioh);
547 }
548
549 /*
550 * Has the IO completed and thus the IO handle been reused?
551 *
552 * This is useful when waiting for IO completion at a low level (e.g. in an IO
553 * method's ->wait_one() callback).
554 */
555 bool
556 pgaio_io_was_recycled(PgAioHandle *ioh, uint64 ref_generation, PgAioHandleState *state)
557 {
558 *state = ioh->state;
559
560 /*
561 * Ensure that we don't see an earlier state of the handle than ioh->state
562 * due to compiler or CPU reordering. This protects both ->generation as
563 * directly used here, and other fields in the handle accessed in the
564 * caller if the handle was not reused.
565 */
566 pg_memory_barrier();
567
568 return ioh->generation != ref_generation;
569 }
570
571 /*
572 * Wait for IO to complete. External code should never use this, outside of
573 * the AIO subsystem waits are only allowed via pgaio_wref_wait().
574 */
575 static void
576 pgaio_io_wait(PgAioHandle *ioh, uint64 ref_generation)
577 {
578 PgAioHandleState state;
579 bool am_owner;
580
581 am_owner = ioh->owner_procno == MyProcNumber;
582
583 if (pgaio_io_was_recycled(ioh, ref_generation, &state))
584 return;
585
586 if (am_owner)
587 {
588 if (state != PGAIO_HS_SUBMITTED
589 && state != PGAIO_HS_COMPLETED_IO
590 && state != PGAIO_HS_COMPLETED_SHARED
591 && state != PGAIO_HS_COMPLETED_LOCAL)
592 {
593 elog(PANIC, "waiting for own IO %d in wrong state: %s",
594 pgaio_io_get_id(ioh), pgaio_io_get_state_name(ioh));
595 }
596 }
597
598 while (true)
599 {
600 if (pgaio_io_was_recycled(ioh, ref_generation, &state))
601 return;
602
603 switch ((PgAioHandleState) state)
604 {
605 case PGAIO_HS_IDLE:
606 case PGAIO_HS_HANDED_OUT:
607 elog(ERROR, "IO in wrong state: %d", state);
608 break;
609
610 case PGAIO_HS_SUBMITTED:
611
612 /*
613 * If we need to wait via the IO method, do so now. Don't
614 * check via the IO method if the issuing backend is executing
615 * the IO synchronously.
616 */
617 if (pgaio_method_ops->wait_one && !(ioh->flags & PGAIO_HF_SYNCHRONOUS))
618 {
619 pgaio_method_ops->wait_one(ioh, ref_generation);
620 continue;
621 }
622 /* fallthrough */
623
624 /* waiting for owner to submit */
625 case PGAIO_HS_DEFINED:
626 case PGAIO_HS_STAGED:
627 /* waiting for reaper to complete */
628 /* fallthrough */
629 case PGAIO_HS_COMPLETED_IO:
630 /* shouldn't be able to hit this otherwise */
631 Assert(IsUnderPostmaster);
632 /* ensure we're going to get woken up */
633 ConditionVariablePrepareToSleep(&ioh->cv);
634
635 while (!pgaio_io_was_recycled(ioh, ref_generation, &state))
636 {
637 if (state == PGAIO_HS_COMPLETED_SHARED ||
638 state == PGAIO_HS_COMPLETED_LOCAL)
639 break;
640 ConditionVariableSleep(&ioh->cv, WAIT_EVENT_AIO_IO_COMPLETION);
641 }
642
643 ConditionVariableCancelSleep();
644 break;
645
646 case PGAIO_HS_COMPLETED_SHARED:
647 case PGAIO_HS_COMPLETED_LOCAL:
648
649 /*
650 * Note that no interrupts are processed between
651 * pgaio_io_was_recycled() and this check - that's important
652 * as otherwise an interrupt could have already reclaimed the
653 * handle.
654 */
655 if (am_owner)
656 pgaio_io_reclaim(ioh);
657 return;
658 }
659 }
660 }
661
662 /*
663 * Make IO handle ready to be reused after IO has completed or after the
664 * handle has been released without being used.
665 *
666 * Note that callers need to be careful about only calling this in the right
667 * state and that no interrupts can be processed between the state check and
668 * the call to pgaio_io_reclaim(). Otherwise interrupt processing could
669 * already have reclaimed the handle.
670 */
671 static void
672 pgaio_io_reclaim(PgAioHandle *ioh)
673 {
0x000000000043662e <+0>: addi sp,sp,-64
0x0000000000436630 <+2>: sd ra,56(sp)
0x0000000000436632 <+4>: sd s0,48(sp)
0x0000000000436634 <+6>: sd s1,40(sp)
0x0000000000436636 <+8>: sd s2,32(sp)
0x0000000000436638 <+10>: sd s3,24(sp)
0x000000000043663a <+12>: sd s4,16(sp)
674 /* This is only ok if it's our IO */
675 Assert(ioh->owner_procno == MyProcNumber);
0x000000000043663c <+14>: auipc a1,0x4a6
0x0000000000436640 <+18>: ld a1,-2020(a1) # 0x8dbe58
0x0000000000436644 <+22>: mv s0,a0
0x0000000000436646 <+24>: lw a0,16(a0)
0x0000000000436648 <+26>: lw a1,0(a1)
0x000000000043664a <+28>: bne a0,a1,0x43684e <pgaio_io_reclaim+544>
0x000000000043684e <+544>: auipc a0,0x242
0x0000000000436852 <+548>: addi a0,a0,-494 # 0x678660
0x0000000000436856 <+552>: auipc a1,0x242
0x000000000043685a <+556>: addi a1,a1,-601 # 0x6785fd
0x000000000043685e <+560>: li a2,675
0x0000000000436862 <+564>: auipc ra,0x160
0x0000000000436866 <+568>: jalr 230(ra) # 0x596948 <ExceptionalCondition>
676 Assert(ioh->state != PGAIO_HS_IDLE);
0x000000000043664e <+32>: lbu a0,0(s0)
0x0000000000436652 <+36>: beqz a0,0x43686a <pgaio_io_reclaim+572>
0x000000000043686a <+572>: auipc a0,0x242
0x000000000043686e <+576>: addi a0,a0,1037 # 0x678c77
0x0000000000436872 <+580>: auipc a1,0x242
0x0000000000436876 <+584>: addi a1,a1,-629 # 0x6785fd
0x000000000043687a <+588>: li a2,676
0x000000000043687e <+592>: auipc ra,0x160
0x0000000000436882 <+596>: jalr 202(ra) # 0x596948 <ExceptionalCondition>
677
678 /* see comment in function header */
679 HOLD_INTERRUPTS();
0x0000000000436656 <+40>: auipc a0,0x4a6
0x000000000043665a <+44>: ld s2,34(a0) # 0x8dc678
0x000000000043665e <+48>: lw a0,0(s2)
0x0000000000436662 <+52>: addi a0,a0,1
0x0000000000436664 <+54>: sw a0,0(s2)
680
681 /*
682 * It's a bit ugly, but right now the easiest place to put the execution
683 * of local completion callbacks is this function, as we need to execute
684 * local callbacks just before reclaiming at multiple callsites.
685 */
686 if (ioh->state == PGAIO_HS_COMPLETED_SHARED)
0x0000000000436668 <+58>: lbu a0,0(s0)
0x000000000043666c <+62>: li a1,6
0x000000000043666e <+64>: bne a0,a1,0x4366ae <pgaio_io_reclaim+128>
687 {
688 PgAioResult local_result;
689
690 local_result = pgaio_io_call_complete_local(ioh);
0x0000000000436672 <+68>: mv a0,s0
0x0000000000436674 <+70>: jal 0x4381de <pgaio_io_call_complete_local>
0x0000000000436678 <+74>: mv s1,a0
691 pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_LOCAL);
0x000000000043667a <+76>: li a1,7
0x000000000043667c <+78>: mv a0,s0
0x000000000043667e <+80>: jal 0x436436 <pgaio_io_update_state>
692
693 if (ioh->report_return)
0x0000000000436682 <+84>: ld a0,96(s0)
0x0000000000436684 <+86>: beqz a0,0x4366ae <pgaio_io_reclaim+128>
694 {
695 ioh->report_return->result = local_result;
0x0000000000436686 <+88>: sw s1,0(a0)
0x0000000000436688 <+90>: srli s1,s1,0x20
0x000000000043668a <+92>: sw s1,4(a0)
696 ioh->report_return->target_data = ioh->target_data;
0x000000000043668c <+94>: ld a0,96(s0)
0x000000000043668e <+96>: lw a1,140(s0)
0x0000000000436692 <+100>: sw a1,28(a0)
0x0000000000436694 <+102>: lw a1,136(s0)
0x0000000000436698 <+106>: sw a1,24(a0)
0x000000000043669a <+108>: lw a1,132(s0)
0x000000000043669e <+112>: sw a1,20(a0)
0x00000000004366a0 <+114>: lw a1,128(s0)
0x00000000004366a4 <+118>: sw a1,16(a0)
0x00000000004366a6 <+120>: lw a1,124(s0)
0x00000000004366a8 <+122>: sw a1,12(a0)
0x00000000004366aa <+124>: lw a1,120(s0)
0x00000000004366ac <+126>: sw a1,8(a0)
697 }
698 }
699
700 pgaio_debug_io(DEBUG4, ioh,
0x00000000004366ae <+128>: li a0,11
0x00000000004366b0 <+130>: li a1,0
0x00000000004366b2 <+132>: auipc ra,0x161
0x00000000004366b6 <+136>: jalr -1776(ra) # 0x596fc2 <errstart>
0x00000000004366ba <+140>: beqz a0,0x436788 <pgaio_io_reclaim+346>
0x00000000004366bc <+142>: li a0,1
0x00000000004366be <+144>: auipc ra,0x163
0x00000000004366c2 <+148>: jalr 688(ra) # 0x59996e <errhidestmt>
0x00000000004366c6 <+152>: li a0,1
0x00000000004366c8 <+154>: auipc ra,0x163
0x00000000004366cc <+158>: jalr 780(ra) # 0x5999d4 <errhidecontext>
0x00000000004366fe <+208>: mv a0,s0
0x0000000000436700 <+210>: jal 0x438f90 <pgaio_io_get_op_name>
0x0000000000436704 <+214>: mv s3,a0
0x0000000000436706 <+216>: mv a0,s0
0x0000000000436708 <+218>: jal 0x439076 <pgaio_io_get_target_name>
0x0000000000436712 <+228>: mv a3,a0
0x000000000043672a <+252>: lw a0,84(s0)
0x000000000043672c <+254>: slli a2,a0,0x37
0x0000000000436730 <+258>: srli a2,a2,0x3d
0x0000000000436732 <+260>: li a5,4
0x0000000000436734 <+262>: mulw a1,s1,s4
0x000000000043674e <+288>: lw a2,20(s0)
0x0000000000436750 <+290>: andi a6,a0,63
0x0000000000436754 <+294>: srliw a7,a0,0x9
0x0000000000436758 <+298>: auipc a0,0x242
0x000000000043675c <+302>: addi a0,a0,1339 # 0x678c93
0x0000000000436760 <+306>: sd a2,0(sp)
0x0000000000436762 <+308>: mv a2,s3
0x0000000000436764 <+310>: auipc ra,0x161
0x0000000000436768 <+314>: jalr -642(ra) # 0x5974e2 <errmsg_internal>
0x000000000043676c <+318>: auipc a0,0x242
0x0000000000436770 <+322>: addi a0,a0,-367 # 0x6785fd
0x0000000000436774 <+326>: auipc a1,0x242
0x0000000000436778 <+330>: addi a2,a1,1433 # 0x678d0d
0x000000000043677c <+334>: li a1,705
0x0000000000436780 <+338>: auipc ra,0x161
0x0000000000436784 <+342>: jalr -1284(ra) # 0x59727c <errfinish>
701 "reclaiming: distilled_result: (status %s, id %u, error_data %d), raw_result: %d",
702 pgaio_result_status_string(ioh->distilled_result.status),
703 ioh->distilled_result.id,
704 ioh->distilled_result.error_data,
705 ioh->result);
706
707 /* if the IO has been defined, it's on the in-flight list, remove */
708 if (ioh->state != PGAIO_HS_HANDED_OUT)
0x0000000000436788 <+346>: lbu a0,0(s0)
0x000000000043678c <+350>: li a1,1
0x000000000043678e <+352>: auipc s1,0x4c3
0x0000000000436792 <+356>: beq a0,a1,0x4367b6 <pgaio_io_reclaim+392>
709 dclist_delete_from(&pgaio_my_backend->in_flight_ios, &ioh->node);
0x0000000000436796 <+360>: ld a0,-1390(s1)
710
711 if (ioh->resowner)
0x00000000004367b6 <+392>: ld a0,40(s0)
0x00000000004367b8 <+394>: beqz a0,0x4367ca <pgaio_io_reclaim+412>
712 {
713 ResourceOwnerForgetAioHandle(ioh->resowner, &ioh->resowner_node);
0x00000000004367ba <+396>: addi a1,s0,48
0x00000000004367be <+400>: auipc ra,0x18d
0x00000000004367c2 <+404>: jalr 1566(ra) # 0x5c3ddc <ResourceOwnerForgetAioHandle>
714 ioh->resowner = NULL;
0x00000000004367c6 <+408>: sd zero,40(s0)
715 }
716
717 Assert(!ioh->resowner);
718
719 /*
720 * Update generation & state first, before resetting the IO's fields,
721 * otherwise a concurrent "viewer" could think the fields are valid, even
722 * though they are being reset. Increment the generation first, so that
723 * we can assert elsewhere that we never wait for an IDLE IO. While it's
724 * a bit weird for the state to go backwards for a generation, it's OK
725 * here, as there cannot be references to the "reborn" IO yet. Can't
726 * update both at once, so something has to give.
727 */
728 ioh->generation++;
0x00000000004367ca <+412>: ld a0,64(s0)
0x00000000004367cc <+414>: addi a0,a0,1
0x00000000004367ce <+416>: sd a0,64(s0)
729 pgaio_io_update_state(ioh, PGAIO_HS_IDLE);
0x00000000004367d0 <+418>: mv a0,s0
0x00000000004367d2 <+420>: li a1,0
0x00000000004367d4 <+422>: jal 0x436436 <pgaio_io_update_state>
730
731 /* ensure the state update is visible before we reset fields */
732 pg_memory_barrier();
0x00000000004367d8 <+426>: fence rw,rw
733
734 ioh->op = PGAIO_OP_INVALID;
735 ioh->target = PGAIO_TID_INVALID;
0x00000000004367e4 <+438>: sb zero,4(s0)
0x00000000004367e8 <+442>: sb zero,3(s0)
0x00000000004367ec <+446>: sb zero,2(s0)
0x00000000004367f4 <+454>: sb zero,1(s0)
736 ioh->flags = 0;
737 ioh->num_callbacks = 0;
738 ioh->handle_data_len = 0;
0x00000000004367dc <+430>: sb zero,13(s0)
739 ioh->report_return = NULL;
0x00000000004367e0 <+434>: sd zero,96(s0)
740 ioh->result = 0;
0x00000000004367fc <+462>: sw zero,20(s0)
741 ioh->distilled_result.status = PGAIO_RS_UNKNOWN;
0x00000000004367f0 <+450>: lwu a1,84(s0)
0x0000000000436800 <+466>: andi a1,a1,-449
0x0000000000436804 <+470>: sw a1,84(s0)
742
743 /*
744 * We push the IO to the head of the idle IO list, that seems more cache
745 * efficient in cases where only a few IOs are used.
746 */
747 dclist_push_head(&pgaio_my_backend->idle_ios, &ioh->node);
0x00000000004367f8 <+458>: ld a0,-1390(s1)
0x0000000000436808 <+474>: addi a2,a0,8
0x000000000043680c <+478>: addi a1,s0,24
748
749 RESUME_INTERRUPTS();
0x000000000043682e <+512>: lw a0,0(s2)
0x0000000000436832 <+516>: beqz a0,0x4368be <pgaio_io_reclaim+656>
0x0000000000436834 <+518>: lw a0,0(s2)
0x0000000000436838 <+522>: addi a0,a0,-1
0x000000000043683a <+524>: sw a0,0(s2)
0x000000000043683e <+528>: ld ra,56(sp)
0x0000000000436840 <+530>: ld s0,48(sp)
0x0000000000436842 <+532>: ld s1,40(sp)
0x0000000000436844 <+534>: ld s2,32(sp)
0x0000000000436846 <+536>: ld s3,24(sp)
0x0000000000436848 <+538>: ld s4,16(sp)
0x00000000004368be <+656>: auipc a0,0x1cd
0x00000000004368c2 <+660>: addi a0,a0,-716 # 0x6035f2
0x00000000004368c6 <+664>: auipc a1,0x242
0x00000000004368ca <+668>: addi a1,a1,-713 # 0x6785fd
0x00000000004368ce <+672>: li a2,749
0x00000000004368d2 <+676>: auipc ra,0x160
0x00000000004368d6 <+680>: jalr 118(ra) # 0x596948 <ExceptionalCondition>
750 }
0x000000000043684a <+540>: addi sp,sp,64
0x000000000043684c <+542>: ret
751
752 /*
753 * Wait for an IO handle to become usable.
754 *
755 * This only really is useful for pgaio_io_acquire().
756 */
757 static void
758 pgaio_io_wait_for_free(void)
759 {
760 int reclaimed = 0;
761
762 pgaio_debug(DEBUG2, "waiting for free IO with %d pending, %u in-flight, %u idle IOs",
763 pgaio_my_backend->num_staged_ios,
764 dclist_count(&pgaio_my_backend->in_flight_ios),
765 dclist_count(&pgaio_my_backend->idle_ios));
766
767 /*
768 * First check if any of our IOs actually have completed - when using
769 * worker, that'll often be the case. We could do so as part of the loop
770 * below, but that'd potentially lead us to wait for some IO submitted
771 * before.
772 */
773 for (int i = 0; i < io_max_concurrency; i++)
774 {
775 PgAioHandle *ioh = &pgaio_ctl->io_handles[pgaio_my_backend->io_handle_off + i];
776
777 if (ioh->state == PGAIO_HS_COMPLETED_SHARED)
778 {
779 /*
780 * Note that no interrupts are processed between the state check
781 * and the call to reclaim - that's important as otherwise an
782 * interrupt could have already reclaimed the handle.
783 *
784 * Need to ensure that there's no reordering, in the more common
785 * paths, where we wait for IO, that's done by
786 * pgaio_io_was_recycled().
787 */
788 pg_read_barrier();
789 pgaio_io_reclaim(ioh);
790 reclaimed++;
791 }
792 }
793
794 if (reclaimed > 0)
795 return;
796
797 /*
798 * If we have any unsubmitted IOs, submit them now. We'll start waiting in
799 * a second, so it's better they're in flight. This also addresses the
800 * edge-case that all IOs are unsubmitted.
801 */
802 if (pgaio_my_backend->num_staged_ios > 0)
803 pgaio_submit_staged();
804
805 /* possibly some IOs finished during submission */
806 if (!dclist_is_empty(&pgaio_my_backend->idle_ios))
807 return;
808
809 if (dclist_count(&pgaio_my_backend->in_flight_ios) == 0)
810 ereport(ERROR,
811 errmsg_internal("no free IOs despite no in-flight IOs"),
812 errdetail_internal("%d pending, %u in-flight, %u idle IOs",
813 pgaio_my_backend->num_staged_ios,
814 dclist_count(&pgaio_my_backend->in_flight_ios),
815 dclist_count(&pgaio_my_backend->idle_ios)));
816
817 /*
818 * Wait for the oldest in-flight IO to complete.
819 *
820 * XXX: Reusing the general IO wait is suboptimal, we don't need to wait
821 * for that specific IO to complete, we just need *any* IO to complete.
822 */
823 {
824 PgAioHandle *ioh = dclist_head_element(PgAioHandle, node,
825 &pgaio_my_backend->in_flight_ios);
826 uint64 generation = ioh->generation;
827
828 switch ((PgAioHandleState) ioh->state)
829 {
830 /* should not be in in-flight list */
831 case PGAIO_HS_IDLE:
832 case PGAIO_HS_DEFINED:
833 case PGAIO_HS_HANDED_OUT:
834 case PGAIO_HS_STAGED:
835 case PGAIO_HS_COMPLETED_LOCAL:
836 elog(ERROR, "shouldn't get here with io:%d in state %d",
837 pgaio_io_get_id(ioh), ioh->state);
838 break;
839
840 case PGAIO_HS_COMPLETED_IO:
841 case PGAIO_HS_SUBMITTED:
842 pgaio_debug_io(DEBUG2, ioh,
843 "waiting for free io with %u in flight",
844 dclist_count(&pgaio_my_backend->in_flight_ios));
845
846 /*
847 * In a more general case this would be racy, because the
848 * generation could increase after we read ioh->state above.
849 * But we are only looking at IOs by the current backend and
850 * the IO can only be recycled by this backend. Even this is
851 * only OK because we get the handle's generation before
852 * potentially processing interrupts, e.g. as part of
853 * pgaio_debug_io().
854 */
855 pgaio_io_wait(ioh, generation);
856 break;
857
858 case PGAIO_HS_COMPLETED_SHARED:
859
860 /*
861 * It's possible that another backend just finished this IO.
862 *
863 * Note that no interrupts are processed between the state
864 * check and the call to reclaim - that's important as
865 * otherwise an interrupt could have already reclaimed the
866 * handle.
867 *
868 * Need to ensure that there's no reordering, in the more
869 * common paths, where we wait for IO, that's done by
870 * pgaio_io_was_recycled().
871 */
872 pg_read_barrier();
873 pgaio_io_reclaim(ioh);
874 break;
875 }
876
877 if (dclist_count(&pgaio_my_backend->idle_ios) == 0)
878 elog(PANIC, "no idle IO after waiting for IO to terminate");
879 return;
880 }
881 }
882
883 /*
884 * Internal - code outside of AIO should never need this and it'd be hard for
885 * such code to be safe.
886 */
887 static PgAioHandle *
888 pgaio_io_from_wref(PgAioWaitRef *iow, uint64 *ref_generation)
889 {
890 PgAioHandle *ioh;
891
892 Assert(iow->aio_index < pgaio_ctl->io_handle_count);
893
894 ioh = &pgaio_ctl->io_handles[iow->aio_index];
895
896 *ref_generation = ((uint64) iow->generation_upper) << 32 |
897 iow->generation_lower;
898
899 Assert(*ref_generation != 0);
900
901 return ioh;
902 }
903
904 static const char *
905 pgaio_io_state_get_name(PgAioHandleState s)
906 {
907 #define PGAIO_HS_TOSTR_CASE(sym) case PGAIO_HS_##sym: return #sym
908 switch ((PgAioHandleState) s)
0x0000000000436714 <+230>: bltu a2,a1,0x436728 <pgaio_io_reclaim+250>
0x0000000000436718 <+234>: slli a1,a1,0x3
0x000000000043671a <+236>: auipc a0,0x485
0x000000000043671e <+240>: addi a0,a0,-1106 # 0x8bb2c8
0x0000000000436722 <+244>: add a0,a0,a1
0x0000000000436724 <+246>: ld a4,0(a0)
0x0000000000436726 <+248>: j 0x43672a <pgaio_io_reclaim+252>
0x0000000000436728 <+250>: li a4,0
909 {
910 PGAIO_HS_TOSTR_CASE(IDLE);
911 PGAIO_HS_TOSTR_CASE(HANDED_OUT);
912 PGAIO_HS_TOSTR_CASE(DEFINED);
913 PGAIO_HS_TOSTR_CASE(STAGED);
914 PGAIO_HS_TOSTR_CASE(SUBMITTED);
915 PGAIO_HS_TOSTR_CASE(COMPLETED_IO);
916 PGAIO_HS_TOSTR_CASE(COMPLETED_SHARED);
917 PGAIO_HS_TOSTR_CASE(COMPLETED_LOCAL);
918 }
919 #undef PGAIO_HS_TOSTR_CASE
920
921 return NULL; /* silence compiler */
922 }
923
924 const char *
925 pgaio_io_get_state_name(PgAioHandle *ioh)
926 {
927 return pgaio_io_state_get_name(ioh->state);
0x000000000043670c <+222>: lbu a1,0(s0)
0x0000000000436710 <+226>: li a2,7
928 }
929
930 const char *
931 pgaio_result_status_string(PgAioResultStatus rs)
932 {
933 switch ((PgAioResultStatus) rs)
0x0000000000436738 <+266>: bltu a5,a2,0x43674c <pgaio_io_reclaim+286>
0x000000000043673c <+270>: slli a2,a2,0x3
0x000000000043673e <+272>: auipc a5,0x485
0x0000000000436742 <+276>: addi a5,a5,-1182 # 0x8bb2a0
0x0000000000436746 <+280>: add a2,a2,a5
0x0000000000436748 <+282>: ld a5,0(a2)
0x000000000043674a <+284>: j 0x43674e <pgaio_io_reclaim+288>
0x000000000043674c <+286>: li a5,0
End of assembler dump.
(gdb)
pgaio_io_update_state.asm (text/plain, 24.4K) download
GNU gdb (Debian 15.2-1) 15.2
Copyright (C) 2024 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "riscv64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from tmp_install/usr/local/pgsql/bin/postgres...
(gdb) Dump of assembler code for function pgaio_io_update_state:
341 Assert(ioh >= pgaio_ctl->io_handles &&
0x0000000000436490 <+90>: auipc a0,0x4c3
0x0000000000436494 <+94>: ld a1,-616(a0) # 0x8f9228 <pgaio_ctl>
0x0000000000436498 <+98>: ld a0,48(a1)
0x000000000043649a <+100>: bltu s1,a0,0x436552 <pgaio_io_update_state+284>
0x000000000043649e <+104>: lwu a1,40(a1)
0x00000000004364a2 <+108>: li a2,144
0x00000000004364a6 <+112>: mul a1,a1,a2
0x00000000004364aa <+116>: add a1,a1,a0
0x00000000004364ac <+118>: bgeu s1,a1,0x436552 <pgaio_io_update_state+284>
0x00000000004364b0 <+122>: auipc a1,0x4c1
0x00000000004364b4 <+126>: ld s3,280(a1) # 0x8f75c8
0x0000000000436552 <+284>: auipc a0,0x242
0x0000000000436556 <+288>: addi a0,a0,549 # 0x678777
0x000000000043655a <+292>: auipc a1,0x242
0x000000000043655e <+296>: addi a1,a1,163 # 0x6785fd
0x0000000000436562 <+300>: li a2,342
0x0000000000436566 <+304>: auipc ra,0x160
0x000000000043656a <+308>: jalr 994(ra) # 0x596948 <ExceptionalCondition>
342 ioh < (pgaio_ctl->io_handles + pgaio_ctl->io_handle_count));
343 return ioh - pgaio_ctl->io_handles;
0x00000000004364b8 <+130>: sub s0,s1,a0
0x00000000004364bc <+134>: srai s0,s0,0x4
344 }
345
346 /*
347 * Return the ProcNumber for the process that can use an IO handle. The
348 * mapping from IO handles to PGPROCs is static, therefore this even works
349 * when the corresponding PGPROC is not in use.
350 */
351 ProcNumber
352 pgaio_io_get_owner(PgAioHandle *ioh)
353 {
354 return ioh->owner_procno;
355 }
356
357 /*
358 * Return a wait reference for the IO. Only wait references can be used to
359 * wait for an IOs completion, as handles themselves can be reused after
360 * completion. See also the comment above pgaio_io_acquire().
361 */
362 void
363 pgaio_io_get_wref(PgAioHandle *ioh, PgAioWaitRef *iow)
364 {
365 Assert(ioh->state == PGAIO_HS_HANDED_OUT ||
366 ioh->state == PGAIO_HS_DEFINED ||
367 ioh->state == PGAIO_HS_STAGED);
368 Assert(ioh->generation != 0);
369
370 iow->aio_index = ioh - pgaio_ctl->io_handles;
371 iow->generation_upper = (uint32) (ioh->generation >> 32);
372 iow->generation_lower = (uint32) ioh->generation;
373 }
374
375
376
377 /* --------------------------------------------------------------------------------
378 * Internal Functions related to PgAioHandle
379 * --------------------------------------------------------------------------------
380 */
381
382 static inline void
383 pgaio_io_update_state(PgAioHandle *ioh, PgAioHandleState new_state)
384 {
0x0000000000436436 <+0>: addi sp,sp,-48
0x0000000000436438 <+2>: sd ra,40(sp)
0x000000000043643a <+4>: sd s0,32(sp)
0x000000000043643c <+6>: sd s1,24(sp)
0x000000000043643e <+8>: sd s2,16(sp)
0x0000000000436440 <+10>: sd s3,8(sp)
0x0000000000436442 <+12>: sd s4,0(sp)
385 /*
386 * All callers need to have held interrupts in some form, otherwise
387 * interrupt processing could wait for the IO to complete, while in an
388 * intermediary state.
389 */
390 Assert(!INTERRUPTS_CAN_BE_PROCESSED());
0x0000000000436444 <+14>: auipc a2,0x4a6
0x0000000000436448 <+18>: ld a2,564(a2) # 0x8dc678
0x000000000043644c <+22>: lw a2,0(a2)
0x000000000043644e <+24>: mv s4,a1
0x0000000000436450 <+26>: mv s1,a0
0x0000000000436452 <+28>: bnez a2,0x43646e <pgaio_io_update_state+56>
0x0000000000436454 <+30>: auipc a0,0x4a6
0x0000000000436458 <+34>: ld a0,508(a0) # 0x8dc650
0x000000000043645c <+38>: lw a0,0(a0)
0x000000000043645e <+40>: bnez a0,0x43646e <pgaio_io_update_state+56>
0x0000000000436460 <+42>: auipc a0,0x4a6
0x0000000000436464 <+46>: ld a0,-1616(a0) # 0x8dbe10
0x0000000000436468 <+50>: lw a0,0(a0)
0x000000000043646a <+52>: beqz a0,0x43656e <pgaio_io_update_state+312>
0x000000000043656e <+312>: auipc a0,0x242
0x0000000000436572 <+316>: addi a0,a0,1670 # 0x678bf4
0x0000000000436576 <+320>: auipc a1,0x242
0x000000000043657a <+324>: addi a1,a1,135 # 0x6785fd
0x000000000043657e <+328>: li a2,390
0x0000000000436582 <+332>: auipc ra,0x160
0x0000000000436586 <+336>: jalr 966(ra) # 0x596948 <ExceptionalCondition>
391
392 pgaio_debug_io(DEBUG5, ioh,
0x000000000043646e <+56>: li a0,10
0x0000000000436470 <+58>: li a1,0
0x0000000000436472 <+60>: auipc ra,0x161
0x0000000000436476 <+64>: jalr -1200(ra) # 0x596fc2 <errstart>
0x000000000043647a <+68>: beqz a0,0x43653a <pgaio_io_update_state+260>
0x000000000043647c <+70>: li a0,1
0x000000000043647e <+72>: auipc ra,0x163
0x0000000000436482 <+76>: jalr 1264(ra) # 0x59996e <errhidestmt>
0x0000000000436486 <+80>: li a0,1
0x0000000000436488 <+82>: auipc ra,0x163
0x000000000043648c <+86>: jalr 1356(ra) # 0x5999d4 <errhidecontext>
0x00000000004364be <+136>: mv a0,s1
0x00000000004364c0 <+138>: jal 0x438f90 <pgaio_io_get_op_name>
0x00000000004364c4 <+142>: mv s2,a0
0x00000000004364c6 <+144>: mv a0,s1
0x00000000004364c8 <+146>: jal 0x439076 <pgaio_io_get_target_name>
0x00000000004364d2 <+156>: mv a3,a0
0x00000000004364e6 <+176>: mulw a1,s0,s3
0x0000000000436502 <+204>: mulw a1,s0,s3
0x000000000043650c <+214>: auipc a0,0x242
0x0000000000436510 <+218>: addi a0,a0,1799 # 0x678c13
0x0000000000436514 <+222>: mv a2,s2
0x0000000000436516 <+224>: auipc ra,0x161
0x000000000043651a <+228>: jalr -52(ra) # 0x5974e2 <errmsg_internal>
0x000000000043651e <+232>: auipc a0,0x242
0x0000000000436522 <+236>: addi a0,a0,223 # 0x6785fd
0x0000000000436526 <+240>: auipc a1,0x242
0x000000000043652a <+244>: addi a2,a1,1836 # 0x678c52
0x000000000043652e <+248>: li a1,394
0x0000000000436532 <+252>: auipc ra,0x161
0x0000000000436536 <+256>: jalr -694(ra) # 0x59727c <errfinish>
393 "updating state to %s",
394 pgaio_io_state_get_name(new_state));
395
396 /*
397 * Ensure the changes signified by the new state are visible before the
398 * new state becomes visible.
399 */
400 pg_memory_barrier();
0x000000000043653a <+260>: fence rw,rw
401
402 ioh->state = new_state;
0x000000000043653e <+264>: sb s4,0(s1)
0x0000000000436542 <+268>: ld ra,40(sp)
0x0000000000436544 <+270>: ld s0,32(sp)
0x0000000000436546 <+272>: ld s1,24(sp)
0x0000000000436548 <+274>: ld s2,16(sp)
0x000000000043654a <+276>: ld s3,8(sp)
0x000000000043654c <+278>: ld s4,0(sp)
403 }
0x000000000043654e <+280>: addi sp,sp,48
0x0000000000436550 <+282>: ret
404
405 static void
406 pgaio_io_resowner_register(PgAioHandle *ioh)
407 {
408 Assert(!ioh->resowner);
409 Assert(CurrentResourceOwner);
410
411 ResourceOwnerRememberAioHandle(CurrentResourceOwner, &ioh->resowner_node);
412 ioh->resowner = CurrentResourceOwner;
413 }
414
415 /*
416 * Stage IO for execution and, if appropriate, submit it immediately.
417 *
418 * Should only be called from pgaio_io_start_*().
419 */
420 void
421 pgaio_io_stage(PgAioHandle *ioh, PgAioOp op)
422 {
423 bool needs_synchronous;
424
425 Assert(ioh->state == PGAIO_HS_HANDED_OUT);
426 Assert(pgaio_my_backend->handed_out_io == ioh);
427 Assert(pgaio_io_has_target(ioh));
428
429 /*
430 * Otherwise an interrupt, in the middle of staging and possibly executing
431 * the IO, could end up trying to wait for the IO, leading to state
432 * confusion.
433 */
434 HOLD_INTERRUPTS();
435
436 ioh->op = op;
437 ioh->result = 0;
438
439 pgaio_io_update_state(ioh, PGAIO_HS_DEFINED);
440
441 /* allow a new IO to be staged */
442 pgaio_my_backend->handed_out_io = NULL;
443
444 pgaio_io_call_stage(ioh);
445
446 pgaio_io_update_state(ioh, PGAIO_HS_STAGED);
447
448 /*
449 * Synchronous execution has to be executed, well, synchronously, so check
450 * that first.
451 */
452 needs_synchronous = pgaio_io_needs_synchronous_execution(ioh);
453
454 pgaio_debug_io(DEBUG3, ioh,
455 "staged (synchronous: %d, in_batch: %d)",
456 needs_synchronous, pgaio_my_backend->in_batchmode);
457
458 if (!needs_synchronous)
459 {
460 pgaio_my_backend->staged_ios[pgaio_my_backend->num_staged_ios++] = ioh;
461 Assert(pgaio_my_backend->num_staged_ios <= PGAIO_SUBMIT_BATCH_SIZE);
462
463 /*
464 * Unless code explicitly opted into batching IOs, submit the IO
465 * immediately.
466 */
467 if (!pgaio_my_backend->in_batchmode)
468 pgaio_submit_staged();
469 }
470 else
471 {
472 pgaio_io_prepare_submit(ioh);
473 pgaio_io_perform_synchronously(ioh);
474 }
475
476 RESUME_INTERRUPTS();
477 }
478
479 bool
480 pgaio_io_needs_synchronous_execution(PgAioHandle *ioh)
481 {
482 /*
483 * If the caller said to execute the IO synchronously, do so.
484 *
485 * XXX: We could optimize the logic when to execute synchronously by first
486 * checking if there are other IOs in flight and only synchronously
487 * executing if not. Unclear whether that'll be sufficiently common to be
488 * worth worrying about.
489 */
490 if (ioh->flags & PGAIO_HF_SYNCHRONOUS)
491 return true;
492
493 /* Check if the IO method requires synchronous execution of IO */
494 if (pgaio_method_ops->needs_synchronous_execution)
495 return pgaio_method_ops->needs_synchronous_execution(ioh);
496
497 return false;
498 }
499
500 /*
501 * Handle IO being processed by IO method.
502 *
503 * Should be called by IO methods / synchronous IO execution, just before the
504 * IO is performed.
505 */
506 void
507 pgaio_io_prepare_submit(PgAioHandle *ioh)
508 {
509 pgaio_io_update_state(ioh, PGAIO_HS_SUBMITTED);
510
511 dclist_push_tail(&pgaio_my_backend->in_flight_ios, &ioh->node);
512 }
513
514 /*
515 * Handle IO getting completed by a method.
516 *
517 * Should be called by IO methods / synchronous IO execution, just after the
518 * IO has been performed.
519 *
520 * Expects to be called in a critical section. We expect IOs to be usable for
521 * WAL etc, which requires being able to execute completion callbacks in a
522 * critical section.
523 */
524 void
525 pgaio_io_process_completion(PgAioHandle *ioh, int result)
526 {
527 Assert(ioh->state == PGAIO_HS_SUBMITTED);
528
529 Assert(CritSectionCount > 0);
530
531 ioh->result = result;
532
533 pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_IO);
534
535 INJECTION_POINT("aio-process-completion-before-shared", ioh);
536
537 pgaio_io_call_complete_shared(ioh);
538
539 pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_SHARED);
540
541 /* condition variable broadcast ensures state is visible before wakeup */
542 ConditionVariableBroadcast(&ioh->cv);
543
544 /* contains call to pgaio_io_call_complete_local() */
545 if (ioh->owner_procno == MyProcNumber)
546 pgaio_io_reclaim(ioh);
547 }
548
549 /*
550 * Has the IO completed and thus the IO handle been reused?
551 *
552 * This is useful when waiting for IO completion at a low level (e.g. in an IO
553 * method's ->wait_one() callback).
554 */
555 bool
556 pgaio_io_was_recycled(PgAioHandle *ioh, uint64 ref_generation, PgAioHandleState *state)
557 {
558 *state = ioh->state;
559
560 /*
561 * Ensure that we don't see an earlier state of the handle than ioh->state
562 * due to compiler or CPU reordering. This protects both ->generation as
563 * directly used here, and other fields in the handle accessed in the
564 * caller if the handle was not reused.
565 */
566 pg_memory_barrier();
567
568 return ioh->generation != ref_generation;
569 }
570
571 /*
572 * Wait for IO to complete. External code should never use this, outside of
573 * the AIO subsystem waits are only allowed via pgaio_wref_wait().
574 */
575 static void
576 pgaio_io_wait(PgAioHandle *ioh, uint64 ref_generation)
577 {
578 PgAioHandleState state;
579 bool am_owner;
580
581 am_owner = ioh->owner_procno == MyProcNumber;
582
583 if (pgaio_io_was_recycled(ioh, ref_generation, &state))
584 return;
585
586 if (am_owner)
587 {
588 if (state != PGAIO_HS_SUBMITTED
589 && state != PGAIO_HS_COMPLETED_IO
590 && state != PGAIO_HS_COMPLETED_SHARED
591 && state != PGAIO_HS_COMPLETED_LOCAL)
592 {
593 elog(PANIC, "waiting for own IO %d in wrong state: %s",
594 pgaio_io_get_id(ioh), pgaio_io_get_state_name(ioh));
595 }
596 }
597
598 while (true)
599 {
600 if (pgaio_io_was_recycled(ioh, ref_generation, &state))
601 return;
602
603 switch ((PgAioHandleState) state)
604 {
605 case PGAIO_HS_IDLE:
606 case PGAIO_HS_HANDED_OUT:
607 elog(ERROR, "IO in wrong state: %d", state);
608 break;
609
610 case PGAIO_HS_SUBMITTED:
611
612 /*
613 * If we need to wait via the IO method, do so now. Don't
614 * check via the IO method if the issuing backend is executing
615 * the IO synchronously.
616 */
617 if (pgaio_method_ops->wait_one && !(ioh->flags & PGAIO_HF_SYNCHRONOUS))
618 {
619 pgaio_method_ops->wait_one(ioh, ref_generation);
620 continue;
621 }
622 /* fallthrough */
623
624 /* waiting for owner to submit */
625 case PGAIO_HS_DEFINED:
626 case PGAIO_HS_STAGED:
627 /* waiting for reaper to complete */
628 /* fallthrough */
629 case PGAIO_HS_COMPLETED_IO:
630 /* shouldn't be able to hit this otherwise */
631 Assert(IsUnderPostmaster);
632 /* ensure we're going to get woken up */
633 ConditionVariablePrepareToSleep(&ioh->cv);
634
635 while (!pgaio_io_was_recycled(ioh, ref_generation, &state))
636 {
637 if (state == PGAIO_HS_COMPLETED_SHARED ||
638 state == PGAIO_HS_COMPLETED_LOCAL)
639 break;
640 ConditionVariableSleep(&ioh->cv, WAIT_EVENT_AIO_IO_COMPLETION);
641 }
642
643 ConditionVariableCancelSleep();
644 break;
645
646 case PGAIO_HS_COMPLETED_SHARED:
647 case PGAIO_HS_COMPLETED_LOCAL:
648
649 /*
650 * Note that no interrupts are processed between
651 * pgaio_io_was_recycled() and this check - that's important
652 * as otherwise an interrupt could have already reclaimed the
653 * handle.
654 */
655 if (am_owner)
656 pgaio_io_reclaim(ioh);
657 return;
658 }
659 }
660 }
661
662 /*
663 * Make IO handle ready to be reused after IO has completed or after the
664 * handle has been released without being used.
665 *
666 * Note that callers need to be careful about only calling this in the right
667 * state and that no interrupts can be processed between the state check and
668 * the call to pgaio_io_reclaim(). Otherwise interrupt processing could
669 * already have reclaimed the handle.
670 */
671 static void
672 pgaio_io_reclaim(PgAioHandle *ioh)
673 {
674 /* This is only ok if it's our IO */
675 Assert(ioh->owner_procno == MyProcNumber);
676 Assert(ioh->state != PGAIO_HS_IDLE);
677
678 /* see comment in function header */
679 HOLD_INTERRUPTS();
680
681 /*
682 * It's a bit ugly, but right now the easiest place to put the execution
683 * of local completion callbacks is this function, as we need to execute
684 * local callbacks just before reclaiming at multiple callsites.
685 */
686 if (ioh->state == PGAIO_HS_COMPLETED_SHARED)
687 {
688 PgAioResult local_result;
689
690 local_result = pgaio_io_call_complete_local(ioh);
691 pgaio_io_update_state(ioh, PGAIO_HS_COMPLETED_LOCAL);
692
693 if (ioh->report_return)
694 {
695 ioh->report_return->result = local_result;
696 ioh->report_return->target_data = ioh->target_data;
697 }
698 }
699
700 pgaio_debug_io(DEBUG4, ioh,
701 "reclaiming: distilled_result: (status %s, id %u, error_data %d), raw_result: %d",
702 pgaio_result_status_string(ioh->distilled_result.status),
703 ioh->distilled_result.id,
704 ioh->distilled_result.error_data,
705 ioh->result);
706
707 /* if the IO has been defined, it's on the in-flight list, remove */
708 if (ioh->state != PGAIO_HS_HANDED_OUT)
709 dclist_delete_from(&pgaio_my_backend->in_flight_ios, &ioh->node);
710
711 if (ioh->resowner)
712 {
713 ResourceOwnerForgetAioHandle(ioh->resowner, &ioh->resowner_node);
714 ioh->resowner = NULL;
715 }
716
717 Assert(!ioh->resowner);
718
719 /*
720 * Update generation & state first, before resetting the IO's fields,
721 * otherwise a concurrent "viewer" could think the fields are valid, even
722 * though they are being reset. Increment the generation first, so that
723 * we can assert elsewhere that we never wait for an IDLE IO. While it's
724 * a bit weird for the state to go backwards for a generation, it's OK
725 * here, as there cannot be references to the "reborn" IO yet. Can't
726 * update both at once, so something has to give.
727 */
728 ioh->generation++;
729 pgaio_io_update_state(ioh, PGAIO_HS_IDLE);
730
731 /* ensure the state update is visible before we reset fields */
732 pg_memory_barrier();
733
734 ioh->op = PGAIO_OP_INVALID;
735 ioh->target = PGAIO_TID_INVALID;
736 ioh->flags = 0;
737 ioh->num_callbacks = 0;
738 ioh->handle_data_len = 0;
739 ioh->report_return = NULL;
740 ioh->result = 0;
741 ioh->distilled_result.status = PGAIO_RS_UNKNOWN;
742
743 /*
744 * We push the IO to the head of the idle IO list, that seems more cache
745 * efficient in cases where only a few IOs are used.
746 */
747 dclist_push_head(&pgaio_my_backend->idle_ios, &ioh->node);
748
749 RESUME_INTERRUPTS();
750 }
751
752 /*
753 * Wait for an IO handle to become usable.
754 *
755 * This only really is useful for pgaio_io_acquire().
756 */
757 static void
758 pgaio_io_wait_for_free(void)
759 {
760 int reclaimed = 0;
761
762 pgaio_debug(DEBUG2, "waiting for free IO with %d pending, %u in-flight, %u idle IOs",
763 pgaio_my_backend->num_staged_ios,
764 dclist_count(&pgaio_my_backend->in_flight_ios),
765 dclist_count(&pgaio_my_backend->idle_ios));
766
767 /*
768 * First check if any of our IOs actually have completed - when using
769 * worker, that'll often be the case. We could do so as part of the loop
770 * below, but that'd potentially lead us to wait for some IO submitted
771 * before.
772 */
773 for (int i = 0; i < io_max_concurrency; i++)
774 {
775 PgAioHandle *ioh = &pgaio_ctl->io_handles[pgaio_my_backend->io_handle_off + i];
776
777 if (ioh->state == PGAIO_HS_COMPLETED_SHARED)
778 {
779 /*
780 * Note that no interrupts are processed between the state check
781 * and the call to reclaim - that's important as otherwise an
782 * interrupt could have already reclaimed the handle.
783 *
784 * Need to ensure that there's no reordering, in the more common
785 * paths, where we wait for IO, that's done by
786 * pgaio_io_was_recycled().
787 */
788 pg_read_barrier();
789 pgaio_io_reclaim(ioh);
790 reclaimed++;
791 }
792 }
793
794 if (reclaimed > 0)
795 return;
796
797 /*
798 * If we have any unsubmitted IOs, submit them now. We'll start waiting in
799 * a second, so it's better they're in flight. This also addresses the
800 * edge-case that all IOs are unsubmitted.
801 */
802 if (pgaio_my_backend->num_staged_ios > 0)
803 pgaio_submit_staged();
804
805 /* possibly some IOs finished during submission */
806 if (!dclist_is_empty(&pgaio_my_backend->idle_ios))
807 return;
808
809 if (dclist_count(&pgaio_my_backend->in_flight_ios) == 0)
810 ereport(ERROR,
811 errmsg_internal("no free IOs despite no in-flight IOs"),
812 errdetail_internal("%d pending, %u in-flight, %u idle IOs",
813 pgaio_my_backend->num_staged_ios,
814 dclist_count(&pgaio_my_backend->in_flight_ios),
815 dclist_count(&pgaio_my_backend->idle_ios)));
816
817 /*
818 * Wait for the oldest in-flight IO to complete.
819 *
820 * XXX: Reusing the general IO wait is suboptimal, we don't need to wait
821 * for that specific IO to complete, we just need *any* IO to complete.
822 */
823 {
824 PgAioHandle *ioh = dclist_head_element(PgAioHandle, node,
825 &pgaio_my_backend->in_flight_ios);
826 uint64 generation = ioh->generation;
827
828 switch ((PgAioHandleState) ioh->state)
829 {
830 /* should not be in in-flight list */
831 case PGAIO_HS_IDLE:
832 case PGAIO_HS_DEFINED:
833 case PGAIO_HS_HANDED_OUT:
834 case PGAIO_HS_STAGED:
835 case PGAIO_HS_COMPLETED_LOCAL:
836 elog(ERROR, "shouldn't get here with io:%d in state %d",
837 pgaio_io_get_id(ioh), ioh->state);
838 break;
839
840 case PGAIO_HS_COMPLETED_IO:
841 case PGAIO_HS_SUBMITTED:
842 pgaio_debug_io(DEBUG2, ioh,
843 "waiting for free io with %u in flight",
844 dclist_count(&pgaio_my_backend->in_flight_ios));
845
846 /*
847 * In a more general case this would be racy, because the
848 * generation could increase after we read ioh->state above.
849 * But we are only looking at IOs by the current backend and
850 * the IO can only be recycled by this backend. Even this is
851 * only OK because we get the handle's generation before
852 * potentially processing interrupts, e.g. as part of
853 * pgaio_debug_io().
854 */
855 pgaio_io_wait(ioh, generation);
856 break;
857
858 case PGAIO_HS_COMPLETED_SHARED:
859
860 /*
861 * It's possible that another backend just finished this IO.
862 *
863 * Note that no interrupts are processed between the state
864 * check and the call to reclaim - that's important as
865 * otherwise an interrupt could have already reclaimed the
866 * handle.
867 *
868 * Need to ensure that there's no reordering, in the more
869 * common paths, where we wait for IO, that's done by
870 * pgaio_io_was_recycled().
871 */
872 pg_read_barrier();
873 pgaio_io_reclaim(ioh);
874 break;
875 }
876
877 if (dclist_count(&pgaio_my_backend->idle_ios) == 0)
878 elog(PANIC, "no idle IO after waiting for IO to terminate");
879 return;
880 }
881 }
882
883 /*
884 * Internal - code outside of AIO should never need this and it'd be hard for
885 * such code to be safe.
886 */
887 static PgAioHandle *
888 pgaio_io_from_wref(PgAioWaitRef *iow, uint64 *ref_generation)
889 {
890 PgAioHandle *ioh;
891
892 Assert(iow->aio_index < pgaio_ctl->io_handle_count);
893
894 ioh = &pgaio_ctl->io_handles[iow->aio_index];
895
896 *ref_generation = ((uint64) iow->generation_upper) << 32 |
897 iow->generation_lower;
898
899 Assert(*ref_generation != 0);
900
901 return ioh;
902 }
903
904 static const char *
905 pgaio_io_state_get_name(PgAioHandleState s)
906 {
907 #define PGAIO_HS_TOSTR_CASE(sym) case PGAIO_HS_##sym: return #sym
908 switch ((PgAioHandleState) s)
0x00000000004364d4 <+158>: bltu a2,a1,0x436500 <pgaio_io_update_state+202>
0x00000000004364d8 <+162>: slli a1,a1,0x3
0x00000000004364da <+164>: auipc a0,0x485
0x00000000004364de <+168>: addi a0,a0,-530 # 0x8bb2c8
0x00000000004364e2 <+172>: add a0,a0,a1
0x00000000004364e4 <+174>: ld a4,0(a0)
0x00000000004364ea <+180>: bltu a2,s4,0x43650a <pgaio_io_update_state+212>
0x00000000004364ee <+184>: slli a0,s4,0x3
0x00000000004364f2 <+188>: auipc a2,0x485
0x00000000004364f6 <+192>: addi a2,a2,-554 # 0x8bb2c8
0x00000000004364fa <+196>: add a0,a0,a2
0x00000000004364fc <+198>: ld a5,0(a0)
0x00000000004364fe <+200>: j 0x43650c <pgaio_io_update_state+214>
0x0000000000436500 <+202>: li a4,0
0x0000000000436506 <+208>: bgeu a2,s4,0x4364ee <pgaio_io_update_state+184>
0x000000000043650a <+212>: li a5,0
909 {
910 PGAIO_HS_TOSTR_CASE(IDLE);
911 PGAIO_HS_TOSTR_CASE(HANDED_OUT);
912 PGAIO_HS_TOSTR_CASE(DEFINED);
913 PGAIO_HS_TOSTR_CASE(STAGED);
914 PGAIO_HS_TOSTR_CASE(SUBMITTED);
915 PGAIO_HS_TOSTR_CASE(COMPLETED_IO);
916 PGAIO_HS_TOSTR_CASE(COMPLETED_SHARED);
917 PGAIO_HS_TOSTR_CASE(COMPLETED_LOCAL);
918 }
919 #undef PGAIO_HS_TOSTR_CASE
920
921 return NULL; /* silence compiler */
922 }
923
924 const char *
925 pgaio_io_get_state_name(PgAioHandle *ioh)
926 {
927 return pgaio_io_state_get_name(ioh->state);
0x00000000004364cc <+150>: lbu a1,0(s1)
0x00000000004364d0 <+154>: li a2,7
End of assembler dump.
(gdb)
pgaio_io_was_recycled.asm (text/plain, 1.5K) download
GNU gdb (Debian 15.2-1) 15.2
Copyright (C) 2024 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "riscv64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from tmp_install/usr/local/pgsql/bin/postgres...
(gdb) Dump of assembler code for function pgaio_io_was_recycled:
558 *state = ioh->state;
0x0000000000436f98 <+0>: lbu a3,0(a0)
0x0000000000436f9c <+4>: sw a3,0(a2)
559
560 /*
561 * Ensure that we don't see an earlier state of the handle than ioh->state
562 * due to compiler or CPU reordering. This protects both ->generation as
563 * directly used here, and other fields in the handle accessed in the
564 * caller if the handle was not reused.
565 */
566 pg_memory_barrier();
0x0000000000436f9e <+6>: fence rw,rw
567
568 return ioh->generation != ref_generation;
0x0000000000436fa2 <+10>: ld a0,64(a0)
0x0000000000436fa4 <+12>: xor a0,a0,a1
0x0000000000436fa6 <+14>: snez a0,a0
0x0000000000436faa <+18>: ret
End of assembler dump.
(gdb)
* Re: IO in wrong state on riscv64
@ 2025-10-15 04:00 Thomas Munro <thomas.munro@gmail.com>
parent: Alexander Lakhin <exclusion@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Munro @ 2025-10-15 04:00 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
On Tue, Oct 14, 2025 at 5:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
> The replacements doesn't work for me, unfortunately: I have 3 out of 30
> 027_stream_regress test runs failed:
> 2025-10-13 21:27:26.161 UTC [4181290:5] pg_regress/brin ERROR: IO in wrong state: 0
> 2025-10-13 21:27:26.161 UTC [4181290:6] pg_regress/brin STATEMENT: CREATE TABLE brintest (byteacol bytea,
I wonder if the problem could be with buf->io_wref. In WaitIO() we do:
iow = buf->io_wref;
UnlockBufHdr(buf, buf_state);
...
pgaio_wref_wait(&iow);
... but UnlockBufHdr() is only concerned with write ordering:
static inline void
UnlockBufHdr(BufferDesc *desc, uint32 buf_state)
{
pg_write_barrier();
pg_atomic_write_u32(&desc->state, buf_state & (~BM_LOCKED));
}
What happens if you insert pg_memory_barrier() after the line that
reads buf->io_wref?
Does this reproduce on REL_18_STABLE?
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-10-20 02:05 Thomas Munro <thomas.munro@gmail.com>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Munro @ 2025-10-20 02:05 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
Updating the list with some progress: Alexander narrowed it down to
pgaio_io_wait() being inlined into pgaio_io_was_recycled(). It seems
to have some instructions in the wrong order, and that can be
reproduced standalone, so I've asked about it here:
https://github.com/llvm/llvm-project/issues/164194
I'm not seeing how that could be our fault, but let's see...
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-10-22 10:51 Andres Freund <andres@anarazel.de>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Andres Freund @ 2025-10-22 10:51 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers; greg@burd.me
Hi,
On 2025-10-20 13:05:53 +1100, Thomas Munro wrote:
> Updating the list with some progress: Alexander narrowed it down to
> pgaio_io_wait() being inlined into pgaio_io_was_recycled(). It seems
> to have some instructions in the wrong order, and that can be
> reproduced standalone, so I've asked about it here:
>
> https://github.com/llvm/llvm-project/issues/164194
>
> I'm not seeing how that could be our fault, but let's see...
This is some awesome work Alexander and Thomas. Thanks for doing all this
investigation.
Have you figured out if the LLVM bug is specific to riscv or is more general?
Afaict most of the MachineSink stuff mentioned in the llvm bug is generic, not
arch specific. But I'd be surprised if such a problem in a more widely used
arch hadn't been hit long ago...
It's also somewhat surprising that this hasn't caused issues more widely in PG
on riscv...
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-10-22 21:51 Thomas Munro <thomas.munro@gmail.com>
parent: Andres Freund <andres@anarazel.de>
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Munro @ 2025-10-22 21:51 UTC (permalink / raw)
To: Andres Freund <andres@anarazel.de>; +Cc: Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers; greg@burd.me
On Wed, Oct 22, 2025 at 11:51 PM Andres Freund <andres@anarazel.de> wrote:
> Have you figured out if the LLVM bug is specific to riscv or is more general?
> Afaict most of the MachineSink stuff mentioned in the llvm bug is generic, not
> arch specific. But I'd be surprised if such a problem in a more widely used
> arch hadn't been hit long ago...
It also happens on MIPS, Loongarch and x86 (I think?), but not POWER
with that godbolt link I posted.
I wonder if the direction this will go is that it's actually UB: the
C11 memory model only defines fences for atomic operations in section
7.17.4. I think that's basically what this says, and perhaps it's
also showing what that mention of mayStore is about:
https://llvm.org/docs/Atomics.html#optimization-outside-atomic
Surprisingly, even __sync_synchronize() doesn't seem to prevent Clang
from sinking that load instruction. That led me to an interesting old
discussion:
https://discourse.llvm.org/t/intended-semantics-for-fence-seq-cst/28591/4
"... it is not an explicit goal of LLVM to
support or continue supporting legacy code that did what it had to to
express functional concurrent code. It may happen to work now, but
unless enough LLVM users express interest this may break one day, and
__sync_synchronize may not order anything (it may just emit a fence
without forcing any ordering)."
"... trying to make user code that doesn't use
volatile or atomic, but does use __sync_synchronize, work as intended
isn't one of LLVM's goals."
Hngghhghh...
On a more practical note, Alexander, does this work?
#if !defined(pg_read_barrier_impl) && defined(HAVE_GCC__ATOMIC_INT32_CAS)
/* acquire semantics include read barrier semantics */
-# define pg_read_barrier_impl()
__atomic_thread_fence(__ATOMIC_ACQUIRE)
+# define pg_read_barrier_impl() do {
__atomic_signal_fence(__ATOMIC_ACQUIRE);
__atomic_thread_fence(__ATOMIC_ACQUIRE); } while (0)
#endif
Someone might want to write an arch-riscv.h that spits out the asm
instead (should be just FENCE R, R and FENCE W, W as far as I can see,
which might be slightly better than what acquire/release generate, on
paper at least), but apparently this problem is wider than RISCV so we
need a general solution anyway.
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-10-23 07:00 Alexander Lakhin <exclusion@gmail.com>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Alexander Lakhin @ 2025-10-23 07:00 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; Andres Freund <andres@anarazel.de>; +Cc: pgsql-hackers; greg@burd.me
Hello Thomas,
23.10.2025 00:51, Thomas Munro wrote:
> On a more practical note, Alexander, does this work?
>
> #if !defined(pg_read_barrier_impl) && defined(HAVE_GCC__ATOMIC_INT32_CAS)
> /* acquire semantics include read barrier semantics */
> -# define pg_read_barrier_impl()
> __atomic_thread_fence(__ATOMIC_ACQUIRE)
> +# define pg_read_barrier_impl() do {
> __atomic_signal_fence(__ATOMIC_ACQUIRE);
> __atomic_thread_fence(__ATOMIC_ACQUIRE); } while (0)
> #endif
>
> Someone might want to write an arch-riscv.h that spits out the asm
> instead (should be just FENCE R, R and FENCE W, W as far as I can see,
> which might be slightly better than what acquire/release generate, on
> paper at least), but apparently this problem is wider than RISCV so we
> need a general solution anyway.
Thank you for working on this!
Unfortunately, this change doesn't lead to change in disassembly of
pgaio_io_wait(), produced with clang-19 -O1. That is, I'm getting the same
disassembly as the one I sent you before (error/pgaio_io_wait.asm).
I tried:
#if !defined(pg_read_barrier_impl) && defined(HAVE_GCC__ATOMIC_INT32_CAS)
/* acquire semantics include read barrier semantics */
-# define pg_read_barrier_impl() __atomic_thread_fence(__ATOMIC_ACQUIRE)
+# define pg_read_barrier_impl() do { } while (0)
#endif
to make sure that the change of pg_read_barrier_impl() affects
pgaio_io_wait() and really saw a difference in the disassembly.
I haven't run tests with the modified version yet, going to test later
just to be sure, but I suppose we want to see the binary code changed.
Best regards,
Alexander
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-10-23 07:52 Thomas Munro <thomas.munro@gmail.com>
parent: Alexander Lakhin <exclusion@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Munro @ 2025-10-23 07:52 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
On Thu, Oct 23, 2025 at 8:00 PM Alexander Lakhin <exclusion@gmail.com> wrote:
> Unfortunately, this change doesn't lead to change in disassembly of
> pgaio_io_wait(), produced with clang-19 -O1. That is, I'm getting the same
> disassembly as the one I sent you before (error/pgaio_io_wait.asm).
Oh, I think this should work better:
#define pg_read_barrier_impl() \
do { pg_compiler_barrier_impl();
__atomic_thread_fence(__ATOMIC_ACQUIRE); } while (0)
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-11-07 23:47 Thomas Munro <thomas.munro@gmail.com>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 1 reply; 22+ messages in thread
From: Thomas Munro @ 2025-11-07 23:47 UTC (permalink / raw)
To: Alexander Lakhin <exclusion@gmail.com>; +Cc: Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
On Thu, Oct 23, 2025 at 8:52 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> Oh, I think this should work better:
>
> #define pg_read_barrier_impl() \
> do { pg_compiler_barrier_impl();
> __atomic_thread_fence(__ATOMIC_ACQUIRE); } while (0)
Alexander did some extensive testing and we stared at the codegen on a
lot of architectures to confirm that this prevents the reordering.
Pushed and back-patched like that.
Someone should probably write an arch-riscv.h file that generates
FENCE R, R and FENCE W, W (these are a bit weaker than what the
compiler is generating but strong enough for our "traditional
pre-C11-style" barrier semantics) and it's probably safe to declare
that RISC-V has atomic single copy for int64_t (hard to imagine this
isn't try, but we would need to cite the chapter and verse of the
technical manual). But RISC-V was really just the messenger here, so
we had to address the generic version anyway.
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-11-08 00:03 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <thomas.munro@gmail.com>
0 siblings, 2 replies; 22+ messages in thread
From: Tom Lane @ 2025-11-08 00:03 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Alexander Lakhin <exclusion@gmail.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
Thomas Munro <thomas.munro@gmail.com> writes:
> Alexander did some extensive testing and we stared at the codegen on a
> lot of architectures to confirm that this prevents the reordering.
> Pushed and back-patched like that.
You're a brave man to be pushing this into the last-ever release
of v13 with all of 12 hours remaining till code freeze. I don't
mind so much for the newer branches, but I'm feeling nervous
about the risk/reward ratio for v13.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-11-08 00:10 Andres Freund <andres@anarazel.de>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 0 replies; 22+ messages in thread
From: Andres Freund @ 2025-11-08 00:10 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Thomas Munro <thomas.munro@gmail.com>; Alexander Lakhin <exclusion@gmail.com>; pgsql-hackers; greg@burd.me
Hi,
On 2025-11-07 19:03:46 -0500, Tom Lane wrote:
> Thomas Munro <thomas.munro@gmail.com> writes:
> > Alexander did some extensive testing and we stared at the codegen on a
> > lot of architectures to confirm that this prevents the reordering.
> > Pushed and back-patched like that.
>
> You're a brave man to be pushing this into the last-ever release
> of v13 with all of 12 hours remaining till code freeze. I don't
> mind so much for the newer branches, but I'm feeling nervous
> about the risk/reward ratio for v13.
The patch doesn't do anything complicated, it just adds a compiler barrier to
the read/write barriers. As both the compiler barrier and read/write barrier
code is old I don't see a whole lot of potential for problems here. If this
were changing inline assembly or such, it'd be a different story...
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-11-08 00:13 Thomas Munro <thomas.munro@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
1 sibling, 2 replies; 22+ messages in thread
From: Thomas Munro @ 2025-11-08 00:13 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alexander Lakhin <exclusion@gmail.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
On Sat, Nov 8, 2025 at 1:03 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> You're a brave man to be pushing this into the last-ever release
> of v13 with all of 12 hours remaining till code freeze. I don't
> mind so much for the newer branches, but I'm feeling nervous
> about the risk/reward ratio for v13.
I figured "adding a no-op compiler barrier where we already believed
there to be a compiler barrier and rely on it crucially" was pretty
safe as these things go, but that's a good point about v13 and I can
revert it in that one branch in a couple of hours if that's the
consensus.
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-11-08 00:24 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <thomas.munro@gmail.com>
1 sibling, 0 replies; 22+ messages in thread
From: Tom Lane @ 2025-11-08 00:24 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Alexander Lakhin <exclusion@gmail.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
Thomas Munro <thomas.munro@gmail.com> writes:
> On Sat, Nov 8, 2025 at 1:03 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> You're a brave man to be pushing this into the last-ever release
>> of v13 with all of 12 hours remaining till code freeze. I don't
>> mind so much for the newer branches, but I'm feeling nervous
>> about the risk/reward ratio for v13.
> I figured "adding a no-op compiler barrier where we already believed
> there to be a compiler barrier and rely on it crucially" was pretty
> safe as these things go, but that's a good point about v13 and I can
> revert it in that one branch in a couple of hours if that's the
> consensus.
It looks fairly safe to me too, or I'd be pushing back harder.
Still, we routinely get burnt by things we push into a branch's
final release, so I think it's worth having a discussion about
whether this has any possible downsides.
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-11-08 19:26 Tom Lane <tgl@sss.pgh.pa.us>
parent: Thomas Munro <thomas.munro@gmail.com>
1 sibling, 1 reply; 22+ messages in thread
From: Tom Lane @ 2025-11-08 19:26 UTC (permalink / raw)
To: Thomas Munro <thomas.munro@gmail.com>; +Cc: Alexander Lakhin <exclusion@gmail.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
... BTW, I wonder why you did not add pg_compiler_barrier_impl()
to our other use of __atomic_thread_fence:
#if !defined(pg_memory_barrier_impl)
# if defined(HAVE_GCC__ATOMIC_INT32_CAS)
# define pg_memory_barrier_impl() __atomic_thread_fence(__ATOMIC_SEQ_CST)
# elif defined(__GNUC__)
# define pg_memory_barrier_impl() __sync_synchronize()
# endif
#endif /* !defined(pg_memory_barrier_impl) */
If the problem is that Clang doesn't treat __atomic_thread_fence
as a compiler barrier, why is this usage safer than the other two?
regards, tom lane
^ permalink raw reply [nested|flat] 22+ messages in thread
* Re: IO in wrong state on riscv64
@ 2025-11-08 22:50 Thomas Munro <thomas.munro@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 22+ messages in thread
From: Thomas Munro @ 2025-11-08 22:50 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Alexander Lakhin <exclusion@gmail.com>; Andres Freund <andres@anarazel.de>; pgsql-hackers; greg@burd.me
On Sun, Nov 9, 2025 at 8:26 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> ... BTW, I wonder why you did not add pg_compiler_barrier_impl()
> to our other use of __atomic_thread_fence:
>
> #if !defined(pg_memory_barrier_impl)
> # if defined(HAVE_GCC__ATOMIC_INT32_CAS)
> # define pg_memory_barrier_impl() __atomic_thread_fence(__ATOMIC_SEQ_CST)
> # elif defined(__GNUC__)
> # define pg_memory_barrier_impl() __sync_synchronize()
> # endif
> #endif /* !defined(pg_memory_barrier_impl) */
>
> If the problem is that Clang doesn't treat __atomic_thread_fence
> as a compiler barrier, why is this usage safer than the other two?
__sync_synchronize() actually has the same problem, more surprisingly
if you ask me, but I suppose it doesn't seem very likely that they
ever shipped a compiler with the C11-reinterpretation retrofitted
underneath the traditional GCC __sync workalikes that didn't qualify
for the top branch instead. You're quite right about that one though,
and I'm annoyed at myself for missing it. I suspect it might be less
prone to actual problems due to details of the contexts it's used in
(ie luck), a bit like pg_write_barrier() as far I know so far. I'm
looking into the codegen across our branches in practice, but I
imagine that in any case it should be OK as a follow-up fix for the
next release, it's just an additional problem and hopefully only a
hypothetical one... I'm looking into the codegen in practice across
branches, compilers and architectures to see if I can confirm that...
^ permalink raw reply [nested|flat] 22+ messages in thread
end of thread, other threads:[~2025-11-08 22:50 UTC | newest]
Thread overview: 22+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-10-11 13:00 IO in wrong state on riscv64 Alexander Lakhin <exclusion@gmail.com>
2025-10-12 03:35 ` Thomas Munro <thomas.munro@gmail.com>
2025-10-12 05:00 ` Alexander Lakhin <exclusion@gmail.com>
2025-10-12 13:10 ` Greg Burd <greg@burd.me>
2025-10-12 22:44 ` Thomas Munro <thomas.munro@gmail.com>
2025-10-13 04:00 ` Alexander Lakhin <exclusion@gmail.com>
2025-10-13 05:40 ` Thomas Munro <thomas.munro@gmail.com>
2025-10-14 04:00 ` Alexander Lakhin <exclusion@gmail.com>
2025-10-15 04:00 ` Thomas Munro <thomas.munro@gmail.com>
2025-10-20 02:05 ` Thomas Munro <thomas.munro@gmail.com>
2025-10-22 10:51 ` Andres Freund <andres@anarazel.de>
2025-10-22 21:51 ` Thomas Munro <thomas.munro@gmail.com>
2025-10-23 07:00 ` Alexander Lakhin <exclusion@gmail.com>
2025-10-23 07:52 ` Thomas Munro <thomas.munro@gmail.com>
2025-11-07 23:47 ` Thomas Munro <thomas.munro@gmail.com>
2025-11-08 00:03 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-11-08 00:10 ` Andres Freund <andres@anarazel.de>
2025-11-08 00:13 ` Thomas Munro <thomas.munro@gmail.com>
2025-11-08 00:24 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-11-08 19:26 ` Tom Lane <tgl@sss.pgh.pa.us>
2025-11-08 22:50 ` Thomas Munro <thomas.munro@gmail.com>
2025-10-12 03:58 ` Thomas Munro <thomas.munro@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox