v0.13.11: revert SgIoTransport timeout path — keep transport alive

v0.13.10's 'fd=-1 on first poll timeout' was too aggressive: a single
transient killed the entire transport, Pass 1 finished in 45ms with
0 GB good on Dune 2.

Revert to spawn-close + main-thread-reopen (the v0.13.5/8 pattern).
Per-timeout cost is up to ~60s while the kernel completes the
abandoned command, but the v0.13.9 Disc::copy stall guard caps
catastrophic stalls at 120s of bytes_good non-advance. Pass 1 bails
cleanly with NonTrimmed ranges; Pass 2 has a working Drive for
retries with recovery=true + 30s timeouts.
This commit is contained in:
MattJackson
2026-04-25 08:47:10 -07:00
parent 36b65f526a
commit 92dc145864
3 changed files with 63 additions and 26 deletions
+32
View File
@@ -1,5 +1,37 @@
# Changelog
## 0.13.11 (2026-04-25)
### Fix: revert SgIoTransport timeout path to keep transport alive
v0.13.10 changed `SgIoTransport::execute` to set `fd = -1` on a poll
timeout (no reopen on the main thread, since that would serialize
against the spawned close()). The intent was to escape the 60-s
blocking reopen.
The cost was too high: a single transient poll timeout permanently
killed the transport. Live test on Dune 2 (post-replug):
- Pass 1 ran for **45 ms** then returned with 0 GB good and 80 GB
pending.
- The first SCSI READ timed out, fd went to -1, every subsequent
read returned `DeviceNotFound` instantly, Disc::copy raced through
the entire disc skip-forwarding in milliseconds.
- Pass 2 inherited the dead Drive and was equally useless.
Revert: spawn close + reopen on main thread (the v0.13.5/8
behavior). Yes the main-thread open() may block up to ~60 s while
the kernel completes the abandoned command — but the v0.13.9
`Disc::copy` stall guard already caps catastrophic stalls at 120 s
of `bytes_good` non-advance. Net: per-timeout cost is ~60 s, but
Pass 1 cleanly bails out within 120 s of any wedge, and Pass 2 has
a working Drive to retry NonTrimmed ranges with `recovery=true` +
30 s timeouts.
The integration test for the stall guard
(`test_disc_copy_stall_detection_triggers_skip_forward`) continues
to pass — the guard fires regardless of which transport-recovery
strategy is in play.
## 0.13.10 (2026-04-25)
### Version sync — no functional changes
+1 -1
View File
@@ -1,6 +1,6 @@
[package]
name = "libfreemkv"
version = "0.13.10"
version = "0.13.11"
edition = "2024"
rust-version = "1.86"
license = "AGPL-3.0-only"
+30 -25
View File
@@ -321,38 +321,43 @@ impl ScsiTransport for SgIoTransport {
if pr <= 0 {
// Timeout (0) or fatal poll error (-1).
// Command is still pending in the kernel. Abandon the fd by
// spawning a background close (which will block until the
// kernel completes/aborts the pending command), and mark
// this transport invalid by setting `self.fd = -1`.
// Command is still pending in the kernel. Spawn a background
// close of the old fd (which blocks until the kernel
// completes/aborts the pending command) and open a fresh fd
// on the main thread. The main-thread open() can serialize
// against the in-flight close via the kernel's per-device
// state lock — so this call may block up to ~60 s while
// the kernel finishes the abandoned command. That's the
// cost of keeping the Drive alive across a timeout. The
// Disc::copy stall guard (v0.13.9, default 120 s of
// bytes_good non-advance) is the upper bound that prevents
// a catastrophic grind on a wedged read region.
//
// Why we no longer reopen on the main thread: opening the
// SAME /dev/sg* device while the prior fd is mid-close
// serializes via the kernel's per-device state lock, so
// `libc::open()` on the main thread blocks for the same
// duration that close() does — defeating the userspace
// timeout. Observed in v0.13.8 live test on Dune 2: each
// timed-out read added 60+ s to the next iteration of
// Disc::copy, leaving the rip stuck without surfacing an
// error or wedging the drive.
//
// Net effect of the fix: a single read timeout invalidates
// the SgIoTransport. The Drive is now "dead" until the
// consumer (autorip's rip thread) catches the failure and
// reopens. Disc::copy's `skip_on_error=true` path will see
// the Err and skip-forward, advancing pos, and the next
// read on this fd returns Err(DeviceNotFound) immediately —
// which Disc::copy continues to skip-forward through until
// the NonTried region is exhausted. Pass 1 then ends with
// bytes_pending > 0 and the rip thread reopens the Drive
// for Pass 2 (Disc::patch with recovery=true and 30 s
// timeouts).
// History:
// - 0.13.5 and earlier: same as this — but with no upper
// bound, hence 45-min hangs.
// - 0.13.10: tried "set fd=-1, no reopen" — too aggressive,
// one transient timeout killed the whole transport, Pass
// 1 finished in 45 ms with everything NonTrimmed.
// - 0.13.11 (this): same close+reopen as 0.13.5/8 BUT with
// the v0.13.9 stall guard ensuring Disc::copy bails out
// cleanly within 120 s of zero forward progress.
let old_fd = self.fd;
self.fd = -1;
std::thread::spawn(move || {
unsafe { libc::close(old_fd) };
});
let c_path = Self::to_c_path(&self.device_path);
let new_fd = unsafe {
libc::open(
c_path.as_ptr() as *const libc::c_char,
libc::O_RDWR | libc::O_NONBLOCK | libc::O_CLOEXEC,
)
};
self.fd = if new_fd >= 0 { new_fd } else { -1 };
return Err(Error::ScsiError {
opcode: cdb[0],
status: 0xFF,