From 5338f805d513761f9a7ff1812013a173d0ab3b56 Mon Sep 17 00:00:00 2001 From: Matthew Jackson Date: Fri, 8 May 2026 12:58:48 -0700 Subject: [PATCH] v0.17.5: Pass N kernel block-device fallback + per-range fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Direct-SATA BU40N + Dune Part Two UHD live testing exposed that the v0.17.3 single-shot SCSI READ path matched 0/22 of the small bad- sector LBAs that dd if=/dev/sr0 recovers on the same drive. This release closes that gap and fixes adjacent bugs silently capping recovery. - /dev/sr0 pread fallback in Drive::read (Linux only): on SCSI READ Err, fall back to posix_fadvise(DONTNEED) + pread() against the corresponding block device. Kernel sr_mod runs ~5 internal retries with no per-attempt mid-layer escalation overhead — the mechanism behind dd's recovery advantage. End-to-end byte verification confirms the fallback path returns real disc data. - Disc::patch per-range watchdog fix: MAX_RANGE_SECS was breaking 'outer (one slow range killed the entire patch). Now skips to the next range. Pre-fix patch died after 4 sectors of range 1 of 47. - Per-sector range budget: range_budget = sectors × 25 s, capped at 1800 s. Replaces the flat 180 s/range that was unfair to medium ranges and pointlessly generous to single-sector ones. - consecutive_failures resets per range. The wedge-exit detector is for stuck-on-one-range, not many-small-ranges-with-one-fail-each. - Reverted inline 5× retry experiment (was hurting: each retry paid kernel SCSI escalation overhead). Restored READ_RECOVERY_TIMEOUT_MS to 60 s. The kernel-auto-retry pattern is now provided by sr0 fallback. Empirical: pass 1 recovered 94.6 MB / 11 s of main title (33 sr0 saves). Pass 2 added 0.6 MB. Remaining ~233 MB on the test disc appears physically unrecoverable on this hardware. --- CHANGELOG.md | 62 +++++++++++++++++++++++++ Cargo.toml | 2 +- src/disc/mod.rs | 87 +++++++++++++++++++++++++++------- src/drive/mod.rs | 118 +++++++++++++++++++++++++++++++++++++++++++++++ src/scsi/mod.rs | 23 +++++++-- 5 files changed, 271 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c435089..8b08429 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,67 @@ # Changelog +## 0.17.5 (2026-05-08) + +### Pass N recovery — kernel block-device fallback + per-range fixes + +Live testing on direct-SATA BU40N + AACS-encrypted UHD disc revealed that +the v0.17.3 single-shot SCSI READ path matched 0/22 of the small bad-sector +LBAs that `dd if=/dev/sr0` recovers on the same drive. This release closes +that gap and fixes several adjacent bugs that were silently capping +recovery. + +- **`/dev/sr0` pread fallback in `Drive::read` (Linux)**: when a SCSI READ + via `/dev/sg*` returns Err, fall back to `posix_fadvise(POSIX_FADV_DONTNEED) + + pread()` against the corresponding block device. The kernel `sr_mod` + driver runs ~5 internal retries per command without the per-attempt + error-escalation overhead that userspace SG_IO retries pay, which is the + source of dd's recovery advantage. End-to-end byte-verification confirms + the fallback path returns real disc data (md5-equivalent to fresh dd from + the same drive session). The block fd is opened in `Drive::open` by + resolving `/sys/class/scsi_generic/sgN/device/block`; best-effort, with no + fallback if open fails. + +- **`Disc::patch` per-range watchdog fix**: when a range hit `MAX_RANGE_SECS`, + the old code did `wedged_exit = true; break 'outer;` — a single slow range + killed the entire patch. Now `break;` (skip this range, advance the outer + for loop). Pre-fix, patch was dying after 4 sectors of the first slow + range and never reaching the other 46. + +- **Per-sector range budget**: replaced flat `MAX_RANGE_SECS = 180` with + `range_budget_secs = (range_sectors × SECONDS_PER_SECTOR).min(RANGE_BUDGET_CAP_SECS)`. + Tiny ranges exit fast (1-sector range = 25 s); medium ranges get + proportional time (51-sector range ≈ 1275 s); large ranges still bounded + by the 1800 s cap so they cannot monopolise pass 1. + +- **`consecutive_failures` resets per range**: the wedge-exit detector + (`>= 50 consecutive failures`) is for "stuck on the same range"; pre-fix + the counter persisted across ranges, so 50 small post-bisection ranges + with one failure each falsely tripped the wedge mid-pass. Reset at every + range boundary. + +- **Reverted inline 5× retry in patch**: a brief experiment that was + measurably harmful — each "2 s" SCSI timeout paid ~1.5 s kernel SCSI + mid-layer error-escalation overhead, so 5× retry took ~17 s per LBA and + triggered the per-range watchdog in 4 sectors. Restored + `READ_RECOVERY_TIMEOUT_MS = 60_000` (the v0.17.3 baseline; the kernel- + auto-retry pattern is now provided by the `/dev/sr0` fallback above). + +### Empirical results (Dune Part Two UHD, BU40N direct SATA) + +- Pass 1: **94.6 MB recovered** (28% of formerly-bad data, 33 sr0 fallback + saves), **11 s of main-title content** restored. Patch completed all 47 + retryable ranges naturally (was wedging at range 1 of 47 in v0.17.3). +- Pass 2 cumulative: 95.2 MB (+0.6 MB; diminishing returns curve). +- Remaining ~233 MB on the test disc appears physically unrecoverable on + this hardware (kernel auto-retry can't decode it either). + +### Behavioural notes + +- The `/dev/sr0` fallback is Linux only; macOS and Windows fall back to + the existing single-shot SCSI behaviour. The fallback is gated to + `recovery=true` reads (only fires from the patch path, not the sweep + path) to avoid page-cache pressure during multi-GB sequential ripping. + ## 0.17.0 (2026-05-04) ### Code quality: unwrap safety, clippy compliance, test coverage diff --git a/Cargo.toml b/Cargo.toml index 54bda76..ac6e6d1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "libfreemkv" -version = "0.17.3" +version = "0.17.5" edition = "2024" rust-version = "1.86" license = "AGPL-3.0-only" diff --git a/src/disc/mod.rs b/src/disc/mod.rs index ce54ff7..3792d54 100644 --- a/src/disc/mod.rs +++ b/src/disc/mod.rs @@ -1995,14 +1995,33 @@ impl Disc { let mut blocks_attempted: u64 = 0; let mut blocks_read_ok: u64 = 0; let mut blocks_read_failed: u64 = 0; - let mut consecutive_failures: u64 = 0; + // Reset to 0 at the start of every range; declared without init + // because the per-range reset (below) always runs before any read. + let mut consecutive_failures: u64; let mut unreadable_count: u64 = 0; let mut bytes_good_last = bytes_good_before; let mut stall_start = std::time::Instant::now(); let mut range_start; let mut range_bytes_good; const STALL_SECS: u64 = 3600; - const MAX_RANGE_SECS: u64 = 180; + // Per-range budget = sectors_in_range × SECONDS_PER_SECTOR, capped + // at RANGE_BUDGET_CAP. Replaces the old flat 180 s/range — that + // was unfair to medium ranges (a 51-sector range got the same + // 180 s as a 1-sector range, so multi-sector ranges couldn't + // even attempt every sector inside their budget) and pointlessly + // generous to single-sector ranges (180 s when ~5 s would do). + // The cap keeps catastrophic ranges (10s of MB) bounded so they + // can't consume the entire patch run; multi-pass orchestration + // raises the cap on later passes for the genuinely-stuck ones. + // Empirical per-failed-sector cost on direct-SATA BU40N (2026-05-08): + // ~3 s SCSI READ failure + ~15 s sr0 pread fallback (kernel sr_mod + // does ~5 internal retries) ≈ 18-25 s total. SECONDS_PER_SECTOR=25 + // lets a small range fully sample within budget instead of bailing + // after one slow read. Previous value of 5 was too tight: a + // 3-sector range got 15 s budget but the first failed read alone + // took ~20 s, so the watchdog fired before sector 2 could be tried. + const SECONDS_PER_SECTOR: u64 = 25; + const RANGE_BUDGET_CAP_SECS: u64 = 1800; const MAX_SKIPS_PER_RANGE: u32 = 10; let mut skip_count: u32; let mut buf = vec![0u8; block_sectors as usize * 2048]; @@ -2132,6 +2151,24 @@ impl Disc { range_start = std::time::Instant::now(); range_bytes_good = bytes_good_before; skip_count = 0; + // Reset consecutive_failures at each range boundary. The + // wedge-exit detector is for "stuck on the same range" — many + // tiny ranges that each fail their one sampled sector should + // NOT trigger it. Pre-fix: pass 2 hit 134 small post-pass-1 + // ranges, each contributing a single failure, and tripped + // wedged_threshold=50 around range 27/134 — a false positive + // that aborted the rest of the pass. + consecutive_failures = 0; + let range_sectors = *range_size / 2048; + let range_budget_secs = (range_sectors * SECONDS_PER_SECTOR).min(RANGE_BUDGET_CAP_SECS); + tracing::debug!( + target: "freemkv::disc", + phase = "patch_range_budget", + range_lba = *range_pos / 2048, + range_sectors, + range_budget_secs, + "Per-range time budget computed" + ); loop { if let Some(ref h) = opts.halt { if h.load(std::sync::atomic::Ordering::Relaxed) { @@ -2140,39 +2177,47 @@ impl Disc { } } - // Test 1: Range timeout - max 3 minutes per range - if range_start.elapsed().as_secs() > MAX_RANGE_SECS { + // Per-range watchdog: budget = range_sectors × 5 s, capped + // at RANGE_BUDGET_CAP_SECS. Tiny ranges exit fast (1-sector + // range = 5 s budget); medium ranges get proportional time + // (51-sector range = 255 s); huge ranges still bounded by + // the cap so they can't monopolise pass 1. + // + // Both the absolute-elapsed and no-progress checks share + // the same per-range budget. The progress check resets + // range_start on every byte gained, so a steadily-recovering + // range can run as long as it makes progress. + if range_start.elapsed().as_secs() > range_budget_secs { tracing::warn!( target: "freemkv::disc", phase = "patch_range_timeout", range_lba = range_pos / 2048, + range_sectors, elapsed_secs = range_start.elapsed().as_secs(), - bytes_recovered = bytes_good_before - range_bytes_good, - "Range timeout - no progress in {}s, aborting", - MAX_RANGE_SECS + budget_secs = range_budget_secs, + bytes_recovered = range_bytes_good.saturating_sub(bytes_good_before), + "Range timeout - moving to next range" ); - wedged_exit = true; - break 'outer; + break; } - // Test 2: Range progress - must recover bytes in 60 seconds let bytes_good_now = map.stats().bytes_good; if bytes_good_now > range_bytes_good { range_bytes_good = bytes_good_now; range_start = std::time::Instant::now(); } - if range_start.elapsed().as_secs() > MAX_RANGE_SECS { + if range_start.elapsed().as_secs() > range_budget_secs { tracing::warn!( target: "freemkv::disc", phase = "patch_range_stall", range_lba = range_pos / 2048, + range_sectors, elapsed_secs = range_start.elapsed().as_secs(), - bytes_recovered = bytes_good_before - range_bytes_good, - "Range stalled - no recovery in {}s, aborting", - MAX_RANGE_SECS + budget_secs = range_budget_secs, + bytes_recovered = range_bytes_good.saturating_sub(bytes_good_before), + "Range stalled - moving to next range" ); - wedged_exit = true; - break 'outer; + break; } // Test 3: Skip count - max 10 skips per range @@ -2264,6 +2309,16 @@ impl Disc { } } + // Single-shot read. Inline retry was tried 2026-05-08 and + // actively hurt: each timeout pays kernel SCSI mid-layer + // error-escalation overhead (~1.5 s per attempt on top of + // the SCSI timeout), so 5× retry made each LBA take ~17 s + // and forced MAX_RANGE_SECS to fire after 4 sectors. The + // win that motivated the experiment (matching dd via + // /dev/sr0) is being pursued instead through a /dev/sr0 + // pread-based fallback layer that lets the kernel + // sr_mod driver run its own auto-retries (which don't + // pay per-attempt escalation in the same way). let read_start = std::time::Instant::now(); let read_result = reader.read_sectors(lba, count, &mut buf[..bytes], recovery); let read_duration_ms = read_start.elapsed().as_millis(); diff --git a/src/drive/mod.rs b/src/drive/mod.rs index 727b897..6a92586 100644 --- a/src/drive/mod.rs +++ b/src/drive/mod.rs @@ -69,6 +69,16 @@ pub struct Drive { halt: Arc, /// Event handler — fires for read errors and library-level state changes. event_fn: Option>, + /// Linux only: raw fd for the corresponding block device (`/dev/sr*`) + /// used as a recovery fallback when SCSI READ via `/dev/sg*` returns + /// an error. The kernel `sr_mod` driver auto-retries failed reads + /// (~5× per command) — historically the reason `dd if=/dev/sr0` + /// recovers ~50% of bad sectors that single-shot `SG_IO` READ + /// misses on the same drive. `None` when the block device couldn't + /// be resolved or opened (no fallback in that case; SCSI read + /// errors propagate as before). + #[cfg(target_os = "linux")] + block_dev_fd: Option, } impl Drive { @@ -87,6 +97,9 @@ impl Drive { None => (None, None, None), }; + #[cfg(target_os = "linux")] + let block_dev_fd = open_block_device_for_sg(device); + Ok(Drive { scsi: transport, driver, @@ -96,6 +109,8 @@ impl Drive { device_path: device.to_string_lossy().to_string(), halt: Arc::new(AtomicBool::new(false)), event_fn: None, + #[cfg(target_os = "linux")] + block_dev_fd, }) } @@ -483,6 +498,54 @@ impl Drive { scsi_status = status, "Drive::read checked_exec failed" ); + + // /dev/sr0 pread fallback (Linux only). The kernel + // sr_mod driver auto-retries failed reads (~5× per + // command). Empirically (BU40N + Dune Part 2 UHD, + // 2026-05-08) dd via /dev/sr0 recovers ~50% of bad + // sectors that a single-shot SG_IO READ misses. + #[cfg(target_os = "linux")] + if recovery { + if let Some(fd) = self.block_dev_fd { + let len = count as usize * 2048; + if buf.len() >= len { + let offset = lba as i64 * 2048; + // Drop kernel cache for this region so we get + // a fresh device read, not stale page-cache + // data from a prior successful neighbour read. + let _ = unsafe { + libc::posix_fadvise( + fd, + offset, + len as i64, + libc::POSIX_FADV_DONTNEED, + ) + }; + let n = unsafe { + libc::pread(fd, buf.as_mut_ptr() as *mut libc::c_void, len, offset) + }; + if n == len as isize { + tracing::info!( + target: "freemkv::drive", + lba, + count, + bytes = len, + "Drive::read recovered via /dev/sr0 pread fallback" + ); + return Ok(len); + } + tracing::debug!( + target: "freemkv::drive", + lba, + count, + pread_ret = n as i64, + errno = std::io::Error::last_os_error().raw_os_error().unwrap_or(0), + "/dev/sr0 pread fallback also failed" + ); + } + } + } + Err(Error::DiscRead { sector: lba as u64, status: Some(status), @@ -586,6 +649,61 @@ impl Drop for Drive { fn drop(&mut self) { self.cleanup(); // SgIoTransport::drop() runs next, calling libc::close(fd) + #[cfg(target_os = "linux")] + if let Some(fd) = self.block_dev_fd.take() { + unsafe { libc::close(fd) }; + } + } +} + +/// Resolve a `/dev/sg*` path to the corresponding `/dev/sr*` block +/// device by walking sysfs, then open it for read (no `O_DIRECT` — +/// `posix_fadvise(POSIX_FADV_DONTNEED)` flushes the cache before each +/// pread, which avoids buffer-alignment requirements while still +/// forcing fresh device reads). +/// +/// Returns `None` on any error (sysfs not present, no matching block +/// device, open failed). Callers treat that as "no fallback available" +/// and propagate the original SCSI READ error. +#[cfg(target_os = "linux")] +fn open_block_device_for_sg(sg_path: &Path) -> Option { + let basename = sg_path.file_name()?.to_str()?; + if !basename.starts_with("sg") { + return None; + } + let sysfs_dir = format!("/sys/class/scsi_generic/{}/device/block", basename); + let entries = std::fs::read_dir(&sysfs_dir).ok()?; + let block_name = entries + .flatten() + .find_map(|e| e.file_name().into_string().ok())?; + let block_path = format!("/dev/{}", block_name); + + let mut bytes = block_path.as_bytes().to_vec(); + bytes.push(0); + let fd = unsafe { + libc::open( + bytes.as_ptr() as *const libc::c_char, + libc::O_RDONLY | libc::O_CLOEXEC, + ) + }; + if fd < 0 { + tracing::debug!( + target: "freemkv::drive", + sg = basename, + block_path, + errno = std::io::Error::last_os_error().raw_os_error().unwrap_or(0), + "Failed to open block device for fallback; sr0 fallback disabled" + ); + None + } else { + tracing::info!( + target: "freemkv::drive", + sg = basename, + block_path, + fd, + "Opened /dev/sr* as recovery fallback for failed SCSI reads" + ); + Some(fd) } } diff --git a/src/scsi/mod.rs b/src/scsi/mod.rs index b2b0f67..769fafd 100644 --- a/src/scsi/mod.rs +++ b/src/scsi/mod.rs @@ -68,10 +68,25 @@ pub(crate) const TUR_TIMEOUT_MS: u32 = 5_000; pub(crate) const READ_TIMEOUT_MS: u32 = 10_000; /// Timeout for content READ commands on the recovery path — -/// [`disc::Disc::patch`]'s targeted retries on bad ranges. Doubles -/// the fast-path budget so a sector that fails at 30 s gets one more -/// honest attempt. Matches sg_dd's default per-command timeout -/// (`DEF_TIMEOUT = 60000`). +/// [`disc::Disc::patch`]'s targeted retries on bad ranges. Matches +/// `sg_dd`'s 60 s ceiling: long enough that any sector the drive can +/// recover at all gets the time to do so, short enough that an +/// unresponsive bus is detected before the per-range watchdog fires. +/// +/// In practice failed reads return in 1–4 s (the drive itself gives up +/// on uncorrectable ECC before the timeout); the 60 s value is a +/// safety ceiling, not a steady-state cost. +/// +/// Historical note (2026-05-08): briefly lowered to 2 s with a 5× +/// inline retry loop in `Disc::patch` to mimic the kernel `sr_mod` +/// driver's auto-retry pattern. The synthetic logic worked but on the +/// live drive each "2 s" read paid ~1.5 s of kernel SCSI mid-layer +/// error escalation on top, so 5× retries took ~17 s per LBA and +/// triggered MAX_RANGE_SECS after 4 sectors — pushing recovery to +/// 0/22 ranges (worse than the 0/22 baseline of v0.17.3 single-shot +/// at 60 s, since that at least visited every range). Reverted; the +/// kernel-auto-retry approach is being pursued via a `/dev/sr0` pread +/// fallback instead. pub(crate) const READ_RECOVERY_TIMEOUT_MS: u32 = 60_000; // ── SCSI status bytes (SPC-4 §4.5.5) ────────────────────────────────────────