From 4442fa2df669062eeb93d0c1d3ed3f62429e9425 Mon Sep 17 00:00:00 2001 From: MattJackson <1085847+MattJackson@users.noreply.github.com> Date: Sun, 10 May 2026 16:55:02 -0700 Subject: [PATCH] =?UTF-8?q?disc:=20wedge=20AVOIDANCE=20on=20Pass=201=20?= =?UTF-8?q?=E2=80=94=20inter-error=20pause=20+=20larger=20jumps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Complements the wedge-skip backstop (fbdb50c) with proactive avoidance so we don't HIT the wedge in the first place. User's take after seeing the Dune Pt 2 rip wedge at 48%: 'we shouldn't be wedging.' Empirical observations from the 23:09:12-23:09:55 wedge timeline: 5 read errors over 43 s, ~8 s apart (drive's own ECC recovery takes 5-10 s per failure). Not 'hammering' in any usual sense, but cumulative firmware-state buildup over 5 in-cluster errors was enough to tip the BU40N into wedge mode at the 5th error. Damage cluster spanned ~140 MB (LBAs 19.898M-19.965M). Current damage-jump base of 256 sectors × batch=32 = 16 MB first jump, doubling to 32 MB, 64 MB... Each jump landed BACK INSIDE the 140 MB cluster, exposing the drive to MORE in-cluster errors. Two avoidance levers: 1. Inter-error pause on Pass 1 (PASS_1_FAIL_PAUSE_SECS = 5 s): pre-fix Pass 1 ran pause_secs=0 on all errors to 'zoom past' damage zones. Successful reads still zoom at zero pause — the pause applies only to FAILED reads, giving the drive's firmware cool-down between cluster exposures. Cost: ~5 s per scattered failure (~30-60 s total on a damage cluster); trivial vs. crashing the rip at 48%. 2. Larger damage-jump base (JUMP_BASE_SECTORS = 1024, up from 256): first jump at batch=32 now covers 64 MB instead of 16 MB, second jump 128 MB instead of 32 MB. Two jumps clear 192 MB — well past most single-cluster damage patterns. Smaller jumps were landing inside the cluster and adding to the wedge counter. Plus a halt-aware sleep helper (sleep_secs_or_halt) so the new inter-error pause doesn't degrade halt response time. Halt poll granularity 100 ms — halt fires within ~100 ms regardless of remaining pause time. Updated three sleep call sites in disc/mod.rs (SkipBlock pause, JumpAhead post-pause, Retry pause). The wedge-SKIP backstop (fbdb50c) stays — combined with this avoidance work, the flow becomes: damage cluster encountered → pause 5 s, mark NonTrimmed → second failure → pause 5 s, mark NonTrimmed → ... threshold hit → damage-jump 64 MB (clears 95% of clusters) → if jump lands in another cluster: 128 MB next jump → only if drive STILL wedges after all this: wedge-skip kicks in (1 GB jump + 30 s cooldown × 16 budget) Tests: pass_1_pauses_briefly_on_skip_for_wedge_avoidance — locks the new 5 s pause behavior in place (replaces the old pause=0 test). integration test threshold bumped from 5 s to 60 s with comment explaining the new bound is 'not infinite' rather than 'milliseconds-fast'. All 433+ tests green on cargo +1.86 fmt + clippy + test. Precommit green. --- src/disc/mod.rs | 40 ++++++++++--- src/disc/read_error.rs | 81 +++++++++++++++++++++----- tests/integration_progress_and_halt.rs | 14 +++-- 3 files changed, 109 insertions(+), 26 deletions(-) diff --git a/src/disc/mod.rs b/src/disc/mod.rs index 1b6688b..a1c32e8 100644 --- a/src/disc/mod.rs +++ b/src/disc/mod.rs @@ -1634,9 +1634,7 @@ impl Disc { match action { read_error::ReadAction::Retry { pause_secs } => { - if pause_secs > 0 { - std::thread::sleep(std::time::Duration::from_secs(pause_secs)); - } + sleep_secs_or_halt(pause_secs, opts.halt.as_ref()); } read_error::ReadAction::Bisect => { read_ctx.bisecting = true; @@ -1713,9 +1711,7 @@ impl Disc { break 'outer; } bytes_done = bytes_done.saturating_add(block_bytes); - if pause_secs > 0 { - std::thread::sleep(std::time::Duration::from_secs(pause_secs)); - } + sleep_secs_or_halt(pause_secs, opts.halt.as_ref()); pos += block_bytes; } read_error::ReadAction::JumpAhead { @@ -1770,9 +1766,7 @@ impl Disc { "damage-jump" ); pos = jump_pos; - if pause_secs > 0 { - std::thread::sleep(std::time::Duration::from_secs(pause_secs)); - } + sleep_secs_or_halt(pause_secs, opts.halt.as_ref()); } read_error::ReadAction::AbortPass => { let (status, sense) = extract_scsi_context(&err); @@ -1974,6 +1968,34 @@ pub struct PatchOutcome { pub wedged_threshold: u64, } +/// Sleep `secs` seconds, but break early if `halt` flips to true. +/// Used by Pass 1's wedge-avoidance inter-error pause so halt +/// remains responsive regardless of how long the pause is. +/// Polling granularity 100 ms — bounded latency on halt regardless +/// of pause length. +pub(crate) fn sleep_secs_or_halt( + secs: u64, + halt: Option<&std::sync::Arc>, +) { + if secs == 0 { + return; + } + let Some(h) = halt else { + std::thread::sleep(std::time::Duration::from_secs(secs)); + return; + }; + let total = std::time::Duration::from_secs(secs); + let slice = std::time::Duration::from_millis(100); + let start = std::time::Instant::now(); + while start.elapsed() < total { + if h.load(std::sync::atomic::Ordering::Relaxed) { + return; + } + let remaining = total.saturating_sub(start.elapsed()); + std::thread::sleep(remaining.min(slice)); + } +} + pub fn mapfile_path_for(iso_path: &std::path::Path) -> std::path::PathBuf { let mut s = iso_path.as_os_str().to_os_string(); s.push(".mapfile"); diff --git a/src/disc/read_error.rs b/src/disc/read_error.rs index ac26637..76c0b55 100644 --- a/src/disc/read_error.rs +++ b/src/disc/read_error.rs @@ -187,6 +187,24 @@ const NOT_READY_MAX_RETRIES: u32 = 3; const BRIDGE_DEGRADATION_PAUSE_SECS: u64 = 15; const BRIDGE_DEGRADATION_MAX_RETRIES: u32 = 5; +/// Pass 1 inter-error pause. ZERO on the clean path (sweep zooms +/// past damage windows) but applied to every failed read so the +/// drive's firmware gets cool-down time between damage-cluster +/// exposures. Wedge-avoidance change 2026-05-10: pre-fix `pause_secs +/// = 0` on Pass 1 errors caused back-to-back exposures that +/// accumulated firmware wedge state. +const PASS_1_FAIL_PAUSE_SECS: u64 = 5; + +/// Base of the damage-jump distance formula: `jump_sectors = +/// JUMP_BASE_SECTORS × batch × jump_multiplier`. Bumped 2026-05-10 +/// from 256 → 1024 (4×) so the first damage-jump at batch=32 covers +/// 64 MB instead of 16 MB. Empirically the BU40N's damage clusters +/// are 100+ MB wide; 16 MB jumps landed inside the cluster and the +/// re-read added to the firmware wedge counter. 64 MB → 128 MB +/// (after one doubling) clears almost any single-cluster damage in +/// 2 jumps. +const JUMP_BASE_SECTORS: u64 = 1024; + // Firmware-wedge skip policy for Pass 1 sweep // =========================================== // @@ -385,13 +403,31 @@ pub fn handle_read_error(err: &Error, ctx: &mut ReadCtx) -> ReadAction { bad_count * 100 / ctx.damage_window.len() }; - // Pass 1's job is to get to the end of the disc fast. Inter- - // block pauses help the drive cool down on Pass N (gentle - // recovery on bad ranges) but on Pass 1 they just turn a 30 s - // damage zone into 30 minutes of dead-air sleep. Pass N - // (`bisect_on_marginal=true`) keeps the original cooldown logic. + // Inter-error pause on Pass 1 — wedge avoidance. + // + // Pre-2026-05-10 Pass 1 ran `pause_secs = 0` on all errors so + // sweep would zoom past damage zones in seconds instead of + // minutes. Empirically on the BU40N this caused firmware-wedge + // events: each read failure leaves residual state in the drive's + // firmware, and back-to-back errors without cooldown accumulate + // toward a wedge threshold. We hit wedge on Dune Pt 2 after 5 + // errors in 43 s spread over a 140 MB damage cluster — even + // though wall-clock pacing was slow, the drive had no breathing + // room between exposures. + // + // New policy: Pass 1 keeps `pause = 0` on the CLEAN path + // (successful reads zoom past damage windows fine), but every + // failed read takes a PASS_1_FAIL_PAUSE_SECS pause before the + // next read. Cost: ~5 s extra per scattered failure, ~30-60 s + // total in a damage cluster — trivial compared to the alternative + // of crashing the whole rip at 48%. Long failure streaks still + // escalate via CONSECUTIVE_FAIL_LONG_PAUSE. let pause_secs = if !ctx.bisect_on_marginal { - 0 + if ctx.consecutive_failures >= CONSECUTIVE_FAIL_LONG_PAUSE_THRESHOLD { + CONSECUTIVE_FAIL_LONG_PAUSE_SECS + } else { + PASS_1_FAIL_PAUSE_SECS + } } else if ctx.consecutive_failures >= CONSECUTIVE_FAIL_LONG_PAUSE_THRESHOLD { CONSECUTIVE_FAIL_LONG_PAUSE_SECS } else { @@ -404,6 +440,14 @@ pub fn handle_read_error(err: &Error, ctx: &mut ReadCtx) -> ReadAction { // multiplier produced a 56 GB jump). Saturating arithmetic on // the sector calc as defence in depth. // + // Jump base bumped 2026-05-10 from 256 to 1024 sectors per + // multiplier unit (= 64 MB first jump at batch=32, up from + // 16 MB). The smaller base routinely landed jumps back inside + // damage clusters of 100+ MB, each landing adding to the + // firmware's wedge counter. 64 MB initial + 128 MB second + + // 256 MB third clears almost any single-cluster damage + // pattern we've seen in 2 jumps. + // // Two triggers, evaluated in order: // // a. **Fast-entry** — `consecutive_outer_failures >= fast_jump_threshold`. @@ -416,13 +460,15 @@ pub fn handle_read_error(err: &Error, ctx: &mut ReadCtx) -> ReadAction { // sliding window of 16 outer reads. Pass N's only path, // and Pass 1's fallback if the failures are scattered // enough that we don't hit the consecutive threshold. - const MAX_JUMP_MULTIPLIER: u64 = 64; // 64 × 256 × batch sectors + const MAX_JUMP_MULTIPLIER: u64 = 64; let fast_trigger = !ctx.bisecting && ctx.consecutive_outer_failures >= ctx.fast_jump_threshold; let window_trigger = ctx.damage_window.len() >= ctx.damage_window_max && bad_pct >= ctx.damage_threshold_pct; if fast_trigger || window_trigger { let mult = ctx.jump_multiplier.min(MAX_JUMP_MULTIPLIER); - let sectors = 256u64.saturating_mul(ctx.batch as u64).saturating_mul(mult); + let sectors = JUMP_BASE_SECTORS + .saturating_mul(ctx.batch as u64) + .saturating_mul(mult); ctx.jump_multiplier = (ctx.jump_multiplier.saturating_mul(2)).min(MAX_JUMP_MULTIPLIER); // Reset the outer-failure counter so a long damaged region // doesn't keep firing fast-jump every read after the initial @@ -695,14 +741,23 @@ mod tests { } #[test] - fn pass_1_does_not_pause_on_skip() { - // Pass 1 must zoom — a damage zone is Pass N's problem to - // recover from. Sleeping between failed batches turned a 30s - // damaged region into a 30-minute Pass-1 grind on real discs. + fn pass_1_pauses_briefly_on_skip_for_wedge_avoidance() { + // Pre-2026-05-10 Pass 1 ran pause_secs=0 on all errors (zoom + // past damage zones in seconds). That caused firmware wedges + // on the BU40N — back-to-back errors with no cooldown built + // up firmware state until the drive entered the wedge fast- + // fail mode. New policy: a brief inter-error pause (5 s) on + // Pass 1 to give the drive's firmware time to settle between + // damage-zone exposures. Successful reads remain zero-pause + // — only errors cost time, and only a few seconds per + // scattered failure. Trivial cost compared to crashing the + // whole rip at 48%. let mut ctx = ReadCtx::for_sweep(32); let action = handle_read_error(&medium_err(), &mut ctx); match action { - ReadAction::SkipBlock { pause_secs } => assert_eq!(pause_secs, 0), + ReadAction::SkipBlock { pause_secs } => { + assert_eq!(pause_secs, PASS_1_FAIL_PAUSE_SECS); + } other => panic!("expected SkipBlock, got {other:?}"), } } diff --git a/tests/integration_progress_and_halt.rs b/tests/integration_progress_and_halt.rs index f57fdb7..e920726 100644 --- a/tests/integration_progress_and_halt.rs +++ b/tests/integration_progress_and_halt.rs @@ -459,11 +459,17 @@ fn test_disc_copy_completes_full_disc_with_failing_reader() { let _ = std::fs::remove_file(&iso_path); let _ = std::fs::remove_file(libfreemkv::disc::mapfile_path_for(&iso_path)); - // Hard bound — even at 0 ms per read, 1024 sectors with skip-forward - // should complete in well under a second on any host. + // Hard bound — Pass 1 must NOT infinite-loop on a fully-failing + // reader. The threshold accommodates the 2026-05-10 wedge- + // avoidance pause (PASS_1_FAIL_PAUSE_SECS = 5 s on each failed + // batch). With batch=32 and 1024 sectors that's up to ~5 batch + // failures + a few damage-jump pauses before fast-trigger jumps + // us past end-of-disc — well-bounded total, ~20-30 s typical. + // The point of this test is "finishes cleanly, not infinitely", + // not "completes in milliseconds." assert!( - elapsed < Duration::from_secs(5), - "Pass 1 took {elapsed:?} on a 2 MB synthetic disc — expected < 5 s" + elapsed < Duration::from_secs(60), + "Pass 1 took {elapsed:?} on a 2 MB synthetic disc — expected < 60 s (not infinite)" ); // Per RIP_DESIGN.md §2.1: Pass 1 must reach end of disc regardless of