Allow txg_wait_synced_flags() and dmu_tx_assign() to return when the pool suspends

[robn]

[Sponsors: Klara, Inc., Wasabi Technology, Inc.]

Motivation and Context

Under failmode=continue, if the pool suspends we will return EIO on syscalls at the "edge" of OpenZFS, but once they're inside the house, they will usually block. We would like to get to a world where it does what it sounds like it will do: throw out in-flight syscalls rather than blocking them in the kernel. This is the first part of realising that.

Description

There's two parts to this change.

At the pool level, txg_wait_synced_flags() gets a new flag TXG_WAIT_SUSPEND. If provided, the call will be allowed to block, but will return if the pool suspends while waiting. To trigger this, a new function txg_wait_kicked() will throw all waiters out of the condvar so they can reconsider their exit conditions. zio_suspend() now calls this.

Further up, dmu_tx_try_assign() gets changed to make use of this flag. It already had a suspend check on first entry and returns EIO or ERESTART depending on the the presence of DMU_TX_WAIT and the current failmode. However, if it then ended up failing the assign because there is no space left in the tx (usually the write throttle), it would fall back into an infinite wait for the next transaction. That wait is now done with TXG_WAIT_SUSPEND, and if the pool suspends, it loops and reconsiders.

There's a few callers of the form VERIFY0(dmu_tx_assign(tx, DMU_TX_WAIT)), which (obviously) cannot handle a surprise error return if the pool suspends. To support them, DMU_TX_SUSPEND has been added, which lets them get the previous behaviour of blocking until the next txg, even if it has to wait until the heat death of the universe. This is sort of a cop-out, but working out a whole bunch of additional error responses is not what I want for an incremental step.

Future plans

On the surface, this looks like a lot of change with a somewhat dubious justification: make the much-maligned failmode=continue work in slightly more places than it does currently. But, this is also laying the groundwork for some future work:

• being able to break out of txg_wait_synced_flags() on suspend is needed to make syscalls backed by zil_commit() (eg fsync()) also return error on suspend. That code is written, but not shown here because the changes inside the ZIL are very invasive, and I wanted to keep this PR focused.
• being able to break out of txg_wait_synced_flags() and dmu_tx_assign() is also needed for forced export (currently being rebuilt on top of this) and for some other ideas we're playing with (eg failmode=readonly).
• there's general interest in making failmode=continue do what people expect from it when they first discover it: eject everything from the kernel. I want to get us there.

How Has This Been Tested?

Tests included to try to test dmu_tx_assign() behaviour during suspend with different failmodes. It's not perfect, as it's somewhat at-a-distance, but it should be good enough for now.

Full ZTS run completed against Linux 6.1.137. FreeBSD 14.2-p1 in progress (will update when done).

“Success” possibly doesn’t say a lot, as there’s few (no?) tests that change failmode= in any meaningful way, or that take much interest in userspace applications when the pool suspends, but at least it suggests normal operation is not broken.

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Performance enhancement (non-breaking change which improves efficiency)
• Code cleanup (non-breaking change which makes code smaller or more readable)
• Breaking change (fix or feature that would cause existing functionality to change)
• Library ABI change (libzfs, libzfs_core, libnvpair, libuutil and libzfsbootenv)
• Documentation (a change to man pages or other documentation)

Checklist:

• My code follows the OpenZFS code style requirements.
• I have updated the documentation accordingly.
• I have read the contributing document.
• I have added tests to cover my changes.
• I have run the ZFS Test Suite with this change applied.
• All commit messages are properly formatted and contain Signed-off-by.

[amotin]

FreeBSD is unhappy:

ZTS run /usr/local/share/zfs/zfs-tests/tests/functional/failmode/failmode_dmu_tx_wait
  panic: VERIFY3S(zio->io_error, ==, ENXIO) failed (5 == 6)
  
  cpuid = 0
  time = 1747707582
  KDB: stack backtrace:
  #0 0xffffffff80c3dd05 at kdb_backtrace+0x65
  #1 0xffffffff80bf1882 at vpanic+0x152
  #2 0xffffffff82a4250a at spl_panic+0x3a
  #3 0xffffffff82c79220 at zio_vdev_io_done+0x140
  #4 0xffffffff82c70548 at zio_execute+0x78
  #5 0xffffffff80c52322 at taskqueue_run_locked+0x182
  #6 0xffffffff80c535a2 at taskqueue_thread_loop+0xc2
  #7 0xffffffff80bad28d at fork_exit+0x7d
  #8 0xffffffff8108610e at fork_trampoline+0xe

[robn]

ZTS run /usr/local/share/zfs/zfs-tests/tests/functional/failmode/failmode_dmu_tx_wait
panic: VERIFY3S(zio->io_error, ==, ENXIO) failed (5 == 6)

Ahh, I didn't have a debug build. It has to be something to do with the errors I'm injecting but I'm not seeing it. I'll dig further but it'll be nbd.

[robn]

ZTS run /usr/local/share/zfs/zfs-tests/tests/functional/failmode/failmode_dmu_tx_wait
panic: VERIFY3S(zio->io_error, ==, ENXIO) failed (5 == 6)

See top commit. Basically, injecting ENXIO actually injects EIO, which can't beat the assert when injected into a top-level vdev_geom device. It's a whole stupid mess; this patch is the narrowest I could think of to retain the intent while still allowing it to happen. Tell me if its bonkers (my FreeBSD-fu is not strong) and/or if you'd prefer it in a separate PR.

(running my own ZTS run on 14.2 right now).

[pcd1193182]

Despite what the PR intro says, I don't see the DMU_TX_NOSUSPEND flag; is that just outdated?

[robn]

Despite what the PR intro says, I don't see the DMU_TX_NOSUSPEND flag; is that just outdated?

It was changed to DMU_TX_SUSPEND upthread. I hadn't updated the OP or commit messages, have done now, ty.