pq/libfreemkv
1
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

disc/patch: remove scatter-recovery (ruled out by live probing); add SubRanges

Browse Source 
Live drive probing (cold-single vs in-stream batch vs centered window,
one clean uncached read per sector) showed recovery RATE is identical
across approach: 3/5 every mode. The drive's per-sector ECC is
media-bound and partly stochastic, NOT approach-bound. So scatter
(seek-away recalibration + far-anchor re-read) does not improve recovery
and is dead weight — removed (function, read_good_sectors, SCATTER_*
consts, and its tests).

Add SubRanges: the still-bad (pos,len) sub-ranges of a section with
remove()/split, the work-list foundation for the upcoming per-section
recovery decomposition (recover_section -> stream/bisect/retry phase
helpers). Unit-tested; unused until the decomposition lands.

25 patch + 8 AB profile tests green; precommit clean on Rust 1.86.
This commit is contained in:
Matthew Jackson
2026-06-30 13:56:49 -07:00
parent caf1b03fd4
commit 923b9edbf4
2 changed files with 175 additions and 418 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/disc/patch.rs
+130 -374
View File
@@ -468,160 +468,77 @@ pub(super) fn recovery_read<R: SectorSource + ?Sized>(
} }
} }
// --------------------------------------------------------------------------- /// The still-bad `[pos, len)` sub-ranges of one bad section, in byte offsets
// Scatter-recovery: "reset, read good data, come back for one sector." /// (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
// A genuinely-damaged sector makes the drive grind its internal C1/C2/L-EC /// shrink the set; whatever remains after all phases is the dead residue that
// re-read loop for the whole recovery timeout (~60 s) and still fail. Worse, /// gets recorded NonTrimmed. Pure data structure — no I/O — so each phase
// re-reading consecutive bad LBAs at identical conditions both (a) re-fails /// helper is unit-testable by asserting the residual `SubRanges`.
// 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<R: SectorSource + ?Sized>(
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);
}
/// 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): /// Foundation for the phased `recover_section` orchestrator; not yet wired
/// transport faults abort the pass, NOT_READY has its own retry path, and /// into the live loop (see the deferral note in the #50 work).
/// wedge-family senses (HARDWARE / ILLEGAL_REQUEST) want a cooldown/eject — #[cfg_attr(not(test), allow(dead_code))]
/// scattering on those would just hammer an already-wedged drive. #[derive(Debug, Clone, PartialEq, Eq, Default)]
#[allow(clippy::too_many_arguments)] pub(super) struct SubRanges {
pub(super) fn scatter_recover<R: SectorSource + ?Sized>( /// (pos, len) pairs, sorted by pos, non-overlapping, all non-zero len.
reader: &mut R, ranges: Vec<(u64, u64)>,
err: &Error,
lba: u32,
count: u16,
bytes: usize,
buf: &mut [u8],
decrypt_is_aacs: bool,
halt: Option<&std::sync::Arc<std::sync::atomic::AtomicBool>>,
) -> 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;
} }
let halted = |h: Option<&std::sync::Arc<std::sync::atomic::AtomicBool>>| { #[cfg_attr(not(test), allow(dead_code))]
h.map(|h| h.load(std::sync::atomic::Ordering::Relaxed)) impl SubRanges {
.unwrap_or(false) /// 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 }
for attempt in 1..=SCATTER_MAX_ATTEMPTS {
if halted(halt) {
return false;
}
// 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 pub(super) fn is_empty(&self) -> bool {
// success means a marginal sector caught on the recalibrated approach; self.ranges.is_empty()
// 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;
} }
Err(_) => {
tracing::debug!( /// Total still-bad bytes across all sub-ranges.
target: "freemkv::disc", pub(super) fn total_len(&self) -> u64 {
phase = "patch.scatter.miss", self.ranges.iter().map(|&(_, l)| l).sum()
lba,
attempt,
anchor,
anchor_ms,
reread_ms,
"scatter-recovery: fresh attempt still failed"
);
} }
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;
}
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));
}
// 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 /// Pre-loop diagnostic dump: emits `patch_mapfile_snapshot` plus the
@@ -2072,44 +1989,12 @@ impl<R: SectorSource + ?Sized> PatchCtx<'_, '_, R> {
continue; continue;
} }
// The slow deep-recovery read also failed. Before giving // The retry also failed. Hand off to handle_read_failure
// up on this single sector, try scatter-recovery: read // (NOT_READY pause/retry, batch bisect, NonTrimmed mark,
// good data far away to recalibrate the head, then re-read // wedge/transport abort). (Scatter-recovery was removed: live
// this one sector fresh and fast (see `scatter_recover`). // probing proved recalibration does not change a sector's
// A recovery here is a normal read success — record it and // recovery — the drive's per-sector ECC is media-bound, not
// advance the cursor exactly like the Ok arm does. // approach-bound — so it only cost time.)
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( match handle_read_failure(
&mut self.state, &mut self.state,
&frame, &frame,
@@ -2139,7 +2024,6 @@ impl<R: SectorSource + ?Sized> PatchCtx<'_, '_, R> {
} }
} }
} }
}
let did_skip = let did_skip =
compute_damage_skip(&mut self.state, &mut frame, self.opts, lba, block_bytes); compute_damage_skip(&mut self.state, &mut frame, self.opts, lba, block_bytes);
@@ -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<usize> {
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,
&not_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] #[test]
fn recovery_read_widens_unaligned_aacs_window() { fn recovery_read_widens_unaligned_aacs_window() {
// A mid-unit AACS read must widen to the enclosing 3-sector unit // 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)" "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)]);
}
} }
tests/passn_handler_ab.rs
+20 -19
View File
@@ -699,31 +699,32 @@ fn profile_07_medium_then_good() {
trace, trace,
); );
// GOLDEN: patch's cache-priming (`prime_cache`) issues 3 throwaway // GOLDEN: cache-priming and scatter-recovery are BOTH gone (ruled out
// single-sector reads at lba-3, lba-2, lba-1 (NOT lba itself) before // by live drive probing — neither improves recovery; the drive's
// each count==1 recovery read. In the default REVERSE patch pass, // per-sector ECC is media-bound, not approach-bound). With the script
// lba 108's prime reads lba 105 (consuming step 0 = fail) and lba 107's // "fail, fail, ok" per bad sector, recovery in ONE pass now hinges on
// prime reads lba 105 again (consuming step 1 = fail), so when the real // bisect re-reading a sector enough times to consume its two failing
// recovery read for lba 105 occurs its script step is 2 (= ok). // steps: a sector caught in a failed batch is re-read as the batch
// prime_cache(105) itself reads 102, 103, 104 — it does NOT advance // halves (32→16→8→4→2→1), which for most of the cluster reaches the
// lba 105's own counter. Net effect: patch fully recovers the range in // Ok step. The 2 sectors bisect reads fewest times stay NonTrimmed and
// one pass thanks to priming, even though the script said "fails on // recover on the NEXT pass — exactly the multi-pass design this test's
// first two attempts." // header describes ("bad sectors stay NonTrimmed in this pass"). So
// // 254 of 256 sectors recover here; 2 (4096 B) defer.
// 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.
assert_eq!( assert_eq!(
stats.bytes_good, stats.bytes_good,
capacity_sectors as u64 * 2048, 254 * 2048,
"07_medium_then_good bytes_good — cache-prime should consume \ "07_medium_then_good bytes_good (bisect re-reads consume the \
the failing script steps so the real read sees Ok" fail,fail,ok script for most of the cluster; 2 defer to next pass)"
); );
assert_eq!( assert_eq!(
stats.bytes_unreadable, 0, 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!(!pr.halted, "07_medium_then_good halted");
assert!( assert!(
trace_len <= 100, trace_len <= 100,