diff --git a/src/disc/patch.rs b/src/disc/patch.rs index 5ef94a1..a2cb561 100644 --- a/src/disc/patch.rs +++ b/src/disc/patch.rs @@ -468,160 +468,77 @@ pub(super) fn recovery_read( } } -// --------------------------------------------------------------------------- -// Scatter-recovery: "reset, read good data, come back for one sector." -// --------------------------------------------------------------------------- -// -// A genuinely-damaged sector makes the drive grind its internal C1/C2/L-EC -// re-read loop for the whole recovery timeout (~60 s) and still fail. Worse, -// re-reading consecutive bad LBAs at identical conditions both (a) re-fails -// identically and (b) is exactly the rapid-failure cadence that drops the -// BU40N into a firmware fast-fail wedge (see CLAUDE.md hard-rule #2). -// -// The fix mirrors ddrescue/MakeMKV practice: between attempts on a stuck -// sector, SEEK AWAY and read a run of known-good sectors. That forces a full -// re-seek + servo/focus relock (re-seating the head so it arrives at the bad -// sector on a fresh, tracking-locked approach — which recovers marginal -// sectors a stationary grind can't) AND breaks the consecutive-failure cadence -// that wedges the drive. Each fresh re-read uses the FAST timeout -// (`recovery = false`), not the 60 s deep grind: a recalibrated marginal -// sector reads quickly, and a truly-dead one fails fast instead of burning -// 60 s per fresh attempt. Many fast fresh attempts beat one long grind. - -/// Fresh re-read attempts on a stuck sector before giving up (each preceded -/// by a recalibration read). -const SCATTER_MAX_ATTEMPTS: u32 = 3; -/// Known-good sectors read at the anchor to recalibrate between attempts -/// (~196 KB; a multiple of 3 so AACS units stay aligned, no widening). -const SCATTER_GOOD_SECTORS: u16 = 96; -/// Base anchor LBA. The leading region of a mounted disc is good, and seeking -/// there from a high bad-region LBA is a long stroke that fully re-seats the -/// head. -const SCATTER_ANCHOR_BASE_LBA: u32 = 64; -/// Vary the anchor per attempt so the drive can't satisfy the recalibration -/// read from cache (a cache hit performs no seek = no recalibration). -const SCATTER_ANCHOR_STRIDE: u32 = 8192; - -/// Recalibration primitive: read `count` known-good sectors at `anchor`, -/// discarding the data. The point is the physical SEEK + sustained tracking -/// read that re-seats the head/servo — not the bytes. Routed through -/// [`recovery_read`] (fast path) so an unaligned AACS anchor still issues a -/// real drive read instead of being rejected pre-read by the decrypting -/// source. Best-effort: a failed anchor read (e.g. it landed in another bad -/// range) still performed the seek, so the error is ignored. -pub(super) fn read_good_sectors( - reader: &mut R, - decrypt_is_aacs: bool, - anchor: u32, - count: u16, -) { - let mut buf = vec![0u8; count as usize * 2048]; - let _ = recovery_read(reader, decrypt_is_aacs, anchor, count, &mut buf, false); +/// The still-bad `[pos, len)` sub-ranges of one bad section, in byte offsets +/// (all multiples of 2048), kept sorted and non-overlapping. The per-section +/// recovery rework (#50) threads one of these through the recovery phase +/// helpers: each phase RECOVERS some bytes and calls [`SubRanges::remove`] to +/// shrink the set; whatever remains after all phases is the dead residue that +/// gets recorded NonTrimmed. Pure data structure — no I/O — so each phase +/// helper is unit-testable by asserting the residual `SubRanges`. +/// +/// Foundation for the phased `recover_section` orchestrator; not yet wired +/// into the live loop (see the deferral note in the #50 work). +#[cfg_attr(not(test), allow(dead_code))] +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub(super) struct SubRanges { + /// (pos, len) pairs, sorted by pos, non-overlapping, all non-zero len. + ranges: Vec<(u64, u64)>, } -/// Try to recover a stuck single sector by recalibrating between fresh, fast -/// re-reads (see the module-section comment above). On success `buf[..bytes]` -/// holds the recovered sector and the caller treats it exactly like a normal -/// read success; on failure it returns `false` to fall through to the usual -/// NonTrimmed give-up. -/// -/// Engages ONLY for a genuine single-sector MEDIUM ERROR (sense_key 0x03): -/// transport faults abort the pass, NOT_READY has its own retry path, and -/// wedge-family senses (HARDWARE / ILLEGAL_REQUEST) want a cooldown/eject — -/// scattering on those would just hammer an already-wedged drive. -#[allow(clippy::too_many_arguments)] -pub(super) fn scatter_recover( - reader: &mut R, - err: &Error, - lba: u32, - count: u16, - bytes: usize, - buf: &mut [u8], - decrypt_is_aacs: bool, - halt: Option<&std::sync::Arc>, -) -> bool { - let is_medium = err - .scsi_sense() - .map(|s| s.sense_key == crate::scsi::SENSE_KEY_MEDIUM_ERROR) - .unwrap_or(false); - if count != 1 || !is_medium { - return false; +#[cfg_attr(not(test), allow(dead_code))] +impl SubRanges { + /// One whole bad section. + pub(super) fn from_section(pos: u64, len: u64) -> Self { + let ranges = if len == 0 { + Vec::new() + } else { + vec![(pos, len)] + }; + Self { ranges } } - let halted = |h: Option<&std::sync::Arc>| { - h.map(|h| h.load(std::sync::atomic::Ordering::Relaxed)) - .unwrap_or(false) - }; + pub(super) fn is_empty(&self) -> bool { + self.ranges.is_empty() + } - for attempt in 1..=SCATTER_MAX_ATTEMPTS { - if halted(halt) { - return false; + /// Total still-bad bytes across all sub-ranges. + pub(super) fn total_len(&self) -> u64 { + self.ranges.iter().map(|&(_, l)| l).sum() + } + + pub(super) fn ranges(&self) -> &[(u64, u64)] { + &self.ranges + } + + /// Remove the recovered byte-range `[pos, pos+len)` from the bad set, + /// splitting any sub-range it bisects and trimming any it overlaps. A + /// range fully covered is dropped; a removal landing in a gap is a no-op. + /// This is how a phase helper records "these bytes are no longer bad". + pub(super) fn remove(&mut self, pos: u64, len: u64) { + if len == 0 { + return; } - // Anchor toward disc start, varied per attempt, clamped below the - // target so we never read the bad region itself as the "good" anchor. - let anchor = SCATTER_ANCHOR_BASE_LBA - .saturating_add(attempt.saturating_mul(SCATTER_ANCHOR_STRIDE)) - .min(lba.saturating_sub(SCATTER_GOOD_SECTORS as u32 + 1)); - // Recalibration read (a real seek + sustained good-sector read) IS the - // settle and the cadence-breaker — no extra idle sleep. Time it so the - // live test can see what the recalibration costs. - let anchor_t = std::time::Instant::now(); - read_good_sectors(reader, decrypt_is_aacs, anchor, SCATTER_GOOD_SECTORS); - let anchor_ms = anchor_t.elapsed().as_millis(); - if halted(halt) { - return false; - } - - // Fresh, FAST re-read of the target (recovery=false). Time it: a quick - // success means a marginal sector caught on the recalibrated approach; - // a slow failure means the fast timeout is being spent — both inform - // tuning SCATTER_* without guessing. - let reread_t = std::time::Instant::now(); - let reread = recovery_read( - reader, - decrypt_is_aacs, - lba, - count, - &mut buf[..bytes], - false, - ); - let reread_ms = reread_t.elapsed().as_millis(); - match reread { - Ok(_) => { - tracing::info!( - target: "freemkv::disc", - phase = "patch.scatter.recovered", - lba, - attempt, - anchor, - anchor_ms, - reread_ms, - "scatter-recovery: recalibrated fresh approach recovered the sector" - ); - return true; + let rend = pos + len; + let mut out: Vec<(u64, u64)> = Vec::with_capacity(self.ranges.len() + 1); + for &(rp, rl) in &self.ranges { + let re = rp + rl; + // Disjoint: keep whole. + if rend <= rp || pos >= re { + out.push((rp, rl)); + continue; + } + // Left remainder [rp, pos). + if pos > rp { + out.push((rp, pos - rp)); } - Err(_) => { - tracing::debug!( - target: "freemkv::disc", - phase = "patch.scatter.miss", - lba, - attempt, - anchor, - anchor_ms, - reread_ms, - "scatter-recovery: fresh attempt still failed" - ); + // Right remainder [rend, re). + if rend < re { + out.push((rend, re - rend)); } + // Otherwise the overlap consumed this whole sub-range. } + self.ranges = out; } - tracing::debug!( - target: "freemkv::disc", - phase = "patch.scatter.exhausted", - lba, - attempts = SCATTER_MAX_ATTEMPTS, - "scatter-recovery: all fresh attempts failed; leaving sector NonTrimmed" - ); - false } /// Pre-loop diagnostic dump: emits `patch_mapfile_snapshot` plus the @@ -2072,70 +1989,37 @@ impl PatchCtx<'_, '_, R> { continue; } - // The slow deep-recovery read also failed. Before giving - // up on this single sector, try scatter-recovery: read - // good data far away to recalibrate the head, then re-read - // this one sector fresh and fast (see `scatter_recover`). - // A recovery here is a normal read success — record it and - // advance the cursor exactly like the Ok arm does. - if count == 1 - && scatter_recover( - self.reader, - &err, - lba, - count, - bytes, - &mut self.buf, - self.decrypt_is_aacs, - self.opts.halt.as_ref(), - ) - { - match handle_read_success( - &mut self.state, - &frame, - self.opts, - lba, - count, - pos, - block_bytes, - bytes, - &mut self.buf, - read_duration_ms, - self.pipe, - self.shared, - self.reader, - )? { - // Break == the whole-pass stall guard fired. - OuterAction::Break => return Ok(RegionOutcome::Wedged), - OuterAction::Continue => {} - } - } else { - match handle_read_failure( - &mut self.state, - &frame, - self.opts, - &err, - lba, - count, - pos, - block_bytes, - bytes, - read_duration_ms, - self.pipe, - self.shared, - self.reader, - )? { - FailureAction::Continue => {} - FailureAction::ContinueInner => continue, - // BreakOuter fires for both a transport fault and a - // wedge-family abort; distinguish for the exit reason. - FailureAction::BreakOuter => { - return Ok(if err.is_scsi_transport_failure() { - RegionOutcome::TransportFault - } else { - RegionOutcome::Wedged - }); - } + // The retry also failed. Hand off to handle_read_failure + // (NOT_READY pause/retry, batch bisect, NonTrimmed mark, + // wedge/transport abort). (Scatter-recovery was removed: live + // probing proved recalibration does not change a sector's + // recovery — the drive's per-sector ECC is media-bound, not + // approach-bound — so it only cost time.) + match handle_read_failure( + &mut self.state, + &frame, + self.opts, + &err, + lba, + count, + pos, + block_bytes, + bytes, + read_duration_ms, + self.pipe, + self.shared, + self.reader, + )? { + FailureAction::Continue => {} + FailureAction::ContinueInner => continue, + // BreakOuter fires for both a transport fault and a + // wedge-family abort; distinguish for the exit reason. + FailureAction::BreakOuter => { + return Ok(if err.is_scsi_transport_failure() { + RegionOutcome::TransportFault + } else { + RegionOutcome::Wedged + }); } } } @@ -3483,197 +3367,6 @@ mod tests { ); } - // ---------------------------------------------------------------- - // Scatter-recovery ("reset, read good data, come back for one - // sector") — recalibrate-between-fresh-attempts + the medium-error - // gate. Validated against a synthetic SectorSource, never the live - // drive (CLAUDE.md hard-rule #2: hammering real bad LBAs wedges it). - // ---------------------------------------------------------------- - - /// A medium-error (sense_key 0x03 = UNRECOVERED READ) CHECK CONDITION — - /// the genuine bad-sector case scatter-recovery targets. - fn medium_err() -> Error { - Error::ScsiError { - opcode: 0x28, - status: crate::scsi::SCSI_STATUS_CHECK_CONDITION, - sense: Some(crate::scsi::ScsiSense { - sense_key: crate::scsi::SENSE_KEY_MEDIUM_ERROR, - asc: 0x11, - ascq: 0x05, - }), - } - } - - /// A NOT_READY error — has its OWN retry path; scatter must skip it. - fn not_ready_err() -> Error { - Error::ScsiError { - opcode: 0x28, - status: crate::scsi::SCSI_STATUS_CHECK_CONDITION, - sense: Some(crate::scsi::ScsiSense { - sense_key: crate::scsi::SENSE_KEY_NOT_READY, - asc: 0x04, - ascq: 0x00, - }), - } - } - - /// Reads at `target` fail until `target_reads > fail_until` (a marginal - /// sector that comes back on a later fresh approach), unless - /// `always_fail`. Any other LBA (the recalibration anchor) reads OK. - /// Counts target vs anchor reads so tests can assert the scatter cadence. - struct ScatterFixture { - target: u32, - fail_until: u32, - always_fail: bool, - target_reads: u32, - anchor_reads: u32, - } - - impl SectorSource for ScatterFixture { - fn read_sectors( - &mut self, - lba: u32, - count: u16, - buf: &mut [u8], - _recovery: bool, - ) -> Result { - let bytes = count as usize * 2048; - if lba == self.target { - self.target_reads += 1; - if !self.always_fail && self.target_reads > self.fail_until { - buf[..bytes].fill(0xAB); - return Ok(bytes); - } - return Err(medium_err()); - } - self.anchor_reads += 1; - let n = bytes.min(buf.len()); - buf[..n].fill(0); - Ok(bytes) - } - } - - #[test] - fn scatter_recovers_marginal_sector_after_recalibration() { - let target = 1_000_000u32; - let mut fx = ScatterFixture { - target, - fail_until: 1, - always_fail: false, - target_reads: 0, - anchor_reads: 0, - }; - let mut buf = vec![0u8; 2048]; - let ok = scatter_recover( - &mut fx, - &medium_err(), - target, - 1, - 2048, - &mut buf, - false, - None, - ); - assert!(ok, "a marginal sector should recover on a fresh re-read"); - assert_eq!(buf[0], 0xAB, "recovered bytes are written into buf"); - assert_eq!( - fx.target_reads, 2, - "1st fresh re-read fails, the 2nd (after recalibration) succeeds" - ); - assert_eq!( - fx.anchor_reads, 2, - "one recalibration read precedes each fresh attempt" - ); - } - - #[test] - fn scatter_gives_up_on_dead_sector_after_max_attempts() { - let target = 1_000_000u32; - let mut fx = ScatterFixture { - target, - fail_until: 0, - always_fail: true, - target_reads: 0, - anchor_reads: 0, - }; - let mut buf = vec![0u8; 2048]; - let ok = scatter_recover( - &mut fx, - &medium_err(), - target, - 1, - 2048, - &mut buf, - false, - None, - ); - assert!(!ok, "a truly-dead sector exhausts attempts and gives up"); - assert_eq!( - fx.target_reads, SCATTER_MAX_ATTEMPTS, - "exactly SCATTER_MAX_ATTEMPTS fresh re-reads, no more" - ); - assert_eq!( - fx.anchor_reads, SCATTER_MAX_ATTEMPTS, - "recalibrates before each fresh attempt" - ); - } - - #[test] - fn scatter_skips_non_medium_error() { - let target = 1_000_000u32; - let mut fx = ScatterFixture { - target, - fail_until: 0, - always_fail: true, - target_reads: 0, - anchor_reads: 0, - }; - let mut buf = vec![0u8; 2048]; - let ok = scatter_recover( - &mut fx, - ¬_ready_err(), - target, - 1, - 2048, - &mut buf, - false, - None, - ); - assert!( - !ok, - "NOT_READY is handled by its own retry path, not scatter" - ); - assert_eq!(fx.target_reads, 0, "no reads issued for a non-medium error"); - assert_eq!(fx.anchor_reads, 0); - } - - #[test] - fn scatter_skips_batch_reads() { - // count > 1 means "don't know which sector is bad yet" — the loop - // drops to count=1 elsewhere; scatter only ever runs on singles. - let target = 1_000_000u32; - let mut fx = ScatterFixture { - target, - fail_until: 0, - always_fail: true, - target_reads: 0, - anchor_reads: 0, - }; - let mut buf = vec![0u8; 2 * 2048]; - let ok = scatter_recover( - &mut fx, - &medium_err(), - target, - 2, - 2 * 2048, - &mut buf, - false, - None, - ); - assert!(!ok); - assert_eq!(fx.target_reads, 0, "batch reads are not scattered"); - } - #[test] fn recovery_read_widens_unaligned_aacs_window() { // A mid-unit AACS read must widen to the enclosing 3-sector unit @@ -3715,4 +3408,67 @@ mod tests { "copied back the requested sector (lba 4), not the unit head (lba 3)" ); } + + // ---------------------------------------------------------------- + // SubRanges — the still-bad work-list the per-section recovery + // phases (#50) shrink. Pure data structure; exhaustively tested so + // each future phase helper can assert on its residual ranges. + // ---------------------------------------------------------------- + + #[test] + fn subranges_from_section_and_basics() { + let s = SubRanges::from_section(2048, 10 * 2048); + assert!(!s.is_empty()); + assert_eq!(s.total_len(), 10 * 2048); + assert_eq!(s.ranges(), &[(2048, 10 * 2048)]); + assert!(SubRanges::from_section(2048, 0).is_empty()); + assert!(SubRanges::default().is_empty()); + } + + #[test] + fn subranges_remove_middle_splits() { + // [0,20k) minus [8k,12k) -> [0,8k) + [12k,20k) + let mut s = SubRanges::from_section(0, 20 * 1024); + s.remove(8 * 1024, 4 * 1024); + assert_eq!(s.ranges(), &[(0, 8 * 1024), (12 * 1024, 8 * 1024)]); + assert_eq!(s.total_len(), 16 * 1024); + } + + #[test] + fn subranges_remove_prefix_suffix_and_whole() { + // prefix + let mut s = SubRanges::from_section(1000, 1000); + s.remove(900, 200); // [1000,1100) trimmed off the front + assert_eq!(s.ranges(), &[(1100, 900)]); + // suffix + let mut s = SubRanges::from_section(1000, 1000); + s.remove(1800, 500); // [1800,2000) trimmed off the back + assert_eq!(s.ranges(), &[(1000, 800)]); + // whole (exact + over-cover both clear it) + let mut s = SubRanges::from_section(1000, 1000); + s.remove(1000, 1000); + assert!(s.is_empty()); + let mut s = SubRanges::from_section(1000, 1000); + s.remove(0, 100_000); + assert!(s.is_empty()); + } + + #[test] + fn subranges_remove_gap_and_zero_are_noops() { + let mut s = SubRanges::from_section(1000, 1000); + s.remove(5000, 1000); // disjoint, after + s.remove(0, 500); // disjoint, before + s.remove(1200, 0); // zero-len + assert_eq!(s.ranges(), &[(1000, 1000)]); + } + + #[test] + fn subranges_remove_spanning_two_ranges() { + // two sub-ranges, removal straddling the gap trims the inner edges + let mut s = SubRanges::from_section(0, 4096); + s.remove(1024, 1024); // -> [0,1024) + [2048,4096) + assert_eq!(s.ranges(), &[(0, 1024), (2048, 2048)]); + s.remove(512, 2048); // covers tail of first + head of second + assert_eq!(s.ranges(), &[(0, 512), (2560, 1536)]); + } } diff --git a/tests/passn_handler_ab.rs b/tests/passn_handler_ab.rs index e53c488..5381c6a 100644 --- a/tests/passn_handler_ab.rs +++ b/tests/passn_handler_ab.rs @@ -699,31 +699,32 @@ fn profile_07_medium_then_good() { trace, ); - // GOLDEN: patch's cache-priming (`prime_cache`) issues 3 throwaway - // single-sector reads at lba-3, lba-2, lba-1 (NOT lba itself) before - // each count==1 recovery read. In the default REVERSE patch pass, - // lba 108's prime reads lba 105 (consuming step 0 = fail) and lba 107's - // prime reads lba 105 again (consuming step 1 = fail), so when the real - // recovery read for lba 105 occurs its script step is 2 (= ok). - // prime_cache(105) itself reads 102, 103, 104 — it does NOT advance - // lba 105's own counter. Net effect: patch fully recovers the range in - // one pass thanks to priming, even though the script said "fails on - // first two attempts." - // - // This is the documented cache-prime behavior (`disc/patch.rs` - // ~line 398, "Proven 2026-05-07 with dd-as-oracle: 8/8 sectors - // recoverable when primed vs 6/8 cold"). The golden pins it. + // GOLDEN: cache-priming and scatter-recovery are BOTH gone (ruled out + // by live drive probing — neither improves recovery; the drive's + // per-sector ECC is media-bound, not approach-bound). With the script + // "fail, fail, ok" per bad sector, recovery in ONE pass now hinges on + // bisect re-reading a sector enough times to consume its two failing + // steps: a sector caught in a failed batch is re-read as the batch + // halves (32→16→8→4→2→1), which for most of the cluster reaches the + // Ok step. The 2 sectors bisect reads fewest times stay NonTrimmed and + // recover on the NEXT pass — exactly the multi-pass design this test's + // header describes ("bad sectors stay NonTrimmed in this pass"). So + // 254 of 256 sectors recover here; 2 (4096 B) defer. assert_eq!( stats.bytes_good, - capacity_sectors as u64 * 2048, - "07_medium_then_good bytes_good — cache-prime should consume \ - the failing script steps so the real read sees Ok" + 254 * 2048, + "07_medium_then_good bytes_good (bisect re-reads consume the \ + fail,fail,ok script for most of the cluster; 2 defer to next pass)" ); assert_eq!( stats.bytes_unreadable, 0, - "07_medium_then_good bytes_unreadable" + "07_medium_then_good bytes_unreadable (NonTrimmed, never terminal in one pass)" + ); + assert_eq!( + stats.bytes_pending, + 2 * 2048, + "07_medium_then_good bytes_pending (2 sectors deferred to the next pass)" ); - assert_eq!(stats.bytes_pending, 0, "07_medium_then_good bytes_pending"); assert!(!pr.halted, "07_medium_then_good halted"); assert!( trace_len <= 100,