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

disc/patch: keep MAX speed on recovery reads + geometric batch re-grow

Browse Source 
Live probing on the BU40N/UHD testbed (rprobe/dprobe) settled the speed
question empirically: a marginal sector reads ~12x FASTER at MAX speed
than at slow, and slow NEVER recovered one MAX didn't. Cleanly-tested
(one uncached read per sector) the recovery RATE is identical across
speed/direction/window/batch — the drive's per-sector ECC is media-bound,
not approach-bound. The only host lever that matters is read SPEED.

So the old 'drop to 0x0000 on first failure and hold it for the whole
range' only slowed the GOOD sectors of a bad range — measured ~3x slower
overall. Now the range stays at MAX; the first failure just re-attempts
once (stochastic media) and falls through. Live A/B: 11 KB/s -> 33 KB/s,
bytes_lost unchanged.

Also: batch adaptation is now halve-on-failure / double-on-success
(geometric), so a bisected batch climbs back through clean runs from any
size (the old count==1-only upscale left it stuck at mid sizes). Removed
the dead consecutive_singles_ok counter and the inert batch-retry probe.

25 patch tests green; precommit clean on Rust 1.86.
This commit is contained in:
Matthew Jackson
2026-06-30 13:25:07 -07:00
parent 9cc422bd00
commit caf1b03fd4
1 changed files with 38 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/disc/patch.rs
+38 -49
View File
@@ -320,7 +320,6 @@ const CONSECUTIVE_FAIL_LONG_PAUSE: u64 = 5;
const CONSECUTIVE_FAIL_LONG_PAUSE_THRESHOLD: u64 = 10; const CONSECUTIVE_FAIL_LONG_PAUSE_THRESHOLD: u64 = 10;
// Adaptive batching: climb back to `initial_batch` after this many // Adaptive batching: climb back to `initial_batch` after this many
// consecutive clean single-sector successes. // consecutive clean single-sector successes.
const ADAPTIVE_UPSCALE_THRESHOLD: u32 = 16;
// Wedge-family (HARDWARE_ERROR / ILLEGAL_REQUEST) cooldown and abort // Wedge-family (HARDWARE_ERROR / ILLEGAL_REQUEST) cooldown and abort
// thresholds — see `handle_read_failure` below for context. // thresholds — see `handle_read_failure` below for context.
// Single source of truth lives in `disc::read_error` so this cannot // Single source of truth lives in `disc::read_error` so this cannot
@@ -782,7 +781,6 @@ pub(super) struct PatchLoopState {
pub now: fn() -> std::time::Instant, pub now: fn() -> std::time::Instant,
// Adaptive batch // Adaptive batch
pub current_batch: u16, pub current_batch: u16,
pub consecutive_singles_ok: u32,
// Snapshot at construction — these stay constant for the whole pass // Snapshot at construction — these stay constant for the whole pass
pub bytes_good_before: u64, pub bytes_good_before: u64,
pub bytes_good_start: u64, pub bytes_good_start: u64,
@@ -848,7 +846,6 @@ impl PatchLoopState {
range_bytes_good: bytes_good_before, range_bytes_good: bytes_good_before,
now, now,
current_batch: initial_batch, current_batch: initial_batch,
consecutive_singles_ok: 0,
bytes_good_before, bytes_good_before,
bytes_good_start: bytes_good_before, bytes_good_start: bytes_good_before,
total_bytes, total_bytes,
@@ -900,26 +897,28 @@ pub(super) fn handle_read_success<R: SectorSource + ?Sized>(
if state.consecutive_good_since_skip >= PASSN_ESCALATION_RESET_GOOD { if state.consecutive_good_since_skip >= PASSN_ESCALATION_RESET_GOOD {
state.consecutive_skips_without_recovery = 0; state.consecutive_skips_without_recovery = 0;
} }
// Adaptive batching: track clean single-sector reads to decide // Adaptive batch re-grow: the partner to handle_read_failure's
// when to climb back to `state.initial_batch`. A batch read // halve-on-failure (bisect). On ANY successful read below
// succeeding (count > 1) tells us the drive is healthy but doesn't // initial_batch, double the batch — so it converges on the right
// accumulate toward upscale — we got back to batch=1 because of a // granularity (tiny across damage, climbing back through clean runs)
// failure here, we need consistent health at the slow tempo // and a mid-size batch left by a bisect (8/4/2) still climbs back,
// before scaling up again. // not just count==1. Mirrors the sweep's read_error adaptive batch:
if count == 1 && state.current_batch < state.initial_batch { // halve down, double up.
state.consecutive_singles_ok += 1; if state.current_batch < state.initial_batch {
if state.consecutive_singles_ok >= ADAPTIVE_UPSCALE_THRESHOLD { let grown = state
tracing::info!( .current_batch
.saturating_mul(2)
.min(state.initial_batch);
if grown != state.current_batch {
tracing::debug!(
target: "freemkv::disc", target: "freemkv::disc",
phase = "patch.batch.upscale", phase = "patch.batch.upscale",
from = state.current_batch, from = state.current_batch,
to = state.initial_batch, to = grown,
consecutive_singles_ok = state.consecutive_singles_ok,
lba, lba,
"adaptive batching: drive stable, climbing back to initial_batch" "adaptive batching: clean read, doubling batch toward initial_batch"
); );
state.current_batch = state.initial_batch; state.current_batch = grown;
state.consecutive_singles_ok = 0;
} }
} }
state.damage_window.push(true); state.damage_window.push(true);
@@ -1173,16 +1172,14 @@ pub(super) fn handle_read_failure<R: SectorSource + ?Sized>(
from_batch = state.current_batch, from_batch = state.current_batch,
to_batch = halved, to_batch = halved,
err_code = err.code(), err_code = err.code(),
"adaptive batching: batch read failed, bisecting (halving) to isolate the bad sector" "batch read failed, bisecting (halving) to isolate the bad sector"
); );
state.current_batch = halved; state.current_batch = halved;
state.consecutive_singles_ok = 0;
return Ok(FailureAction::ContinueInner); return Ok(FailureAction::ContinueInner);
} }
state.blocks_read_failed += 1; state.blocks_read_failed += 1;
state.consecutive_good_since_skip = 0; state.consecutive_good_since_skip = 0;
state.consecutive_singles_ok = 0;
state.unreadable_count += 1; state.unreadable_count += 1;
// Reset the per-LBA NOT_READY counter whenever the LBA changes. // Reset the per-LBA NOT_READY counter whenever the LBA changes.
@@ -1956,7 +1953,7 @@ impl<R: SectorSource + ?Sized> PatchCtx<'_, '_, R> {
); );
// Enter at MAX speed + the full initial batch: read the clean // Enter at MAX speed + the full initial batch: read the clean
// overshoot fast. `range_slowed` flips on the first read failure // overshoot fast. `retried_once` flips on the first read failure
// (below), dropping to the slow recovery speed for the rest of // (below), dropping to the slow recovery speed for the rest of
// the range and arming the inter-range cooldown. // the range and arming the inter-range cooldown.
self.reader.set_speed(0xFFFF); self.reader.set_speed(0xFFFF);
@@ -1969,7 +1966,7 @@ impl<R: SectorSource + ?Sized> PatchCtx<'_, '_, R> {
"range entering at MAX read speed (drops to slow recovery on first failure)" "range entering at MAX read speed (drops to slow recovery on first failure)"
); );
self.state.current_batch = self.state.initial_batch; self.state.current_batch = self.state.initial_batch;
let mut range_slowed = false; let mut retried_once = false;
loop { loop {
if let Some(ref h) = self.opts.halt { if let Some(ref h) = self.opts.halt {
@@ -2058,28 +2055,20 @@ impl<R: SectorSource + ?Sized> PatchCtx<'_, '_, R> {
} }
} }
Err(err) => { Err(err) => {
// First failure in this range: the fast-batched pass // First failure in this range: the fast-batched pass over
// over the clean overshoot is done. A genuine transport // the clean overshoot is done. A genuine transport fault
// fault (bridge crash) is NOT a recoverable bad sector — // (bridge crash) is NOT a recoverable bad sector — let
// skip the slow re-read and let handle_read_failure abort // handle_read_failure abort immediately. Otherwise re-attempt
// immediately. Drop to the slow recovery speed, arm the // the read ONCE (stochastic media: a marginal sector often
// cooldown, and RE-ATTEMPT the same position once at slow // reads on a retry) and fall through. Stay at MAX speed:
// speed before marking it: the drive's deep ECC recovery // direct probing on the live BU40N/UHD proved MAX reads a
// only engages slow, and the failure so far is a fast-read // marginal sector ~12x faster than slow AND slow never
// miss. Hold the cursor (don't advance, don't count // recovered one MAX didn't — so the old drop-to-0x0000 only
// damage); only a slow-speed result reaches // wasted time on the GOOD sectors of a bad range (~3x slower
// handle_read_failure. `range_slowed` gates this to once. // overall). `retried_once` gates the retry to once per range.
if !range_slowed && !err.is_scsi_transport_failure() { if !retried_once && !err.is_scsi_transport_failure() {
self.reader.set_speed(0x0000); self.reader.set_speed(0xFFFF);
tracing::info!( retried_once = true;
target: "freemkv::disc",
phase = "patch.speed",
lba,
speed = "0x0000",
"range dropped to slow recovery speed; retrying the failing read at slow speed before marking"
);
range_slowed = true;
self.cooldown_pending = true;
continue; continue;
} }
@@ -2392,10 +2381,10 @@ impl Disc {
); );
} }
// Adaptive batching: read at `state.current_batch`, drop to 1 // Adaptive batching: read at `state.current_batch`, HALVE on a
// on batch-read failure, climb back to `state.initial_batch` // batch-read failure (bisect to isolate the bad sector), and
// after ADAPTIVE_UPSCALE_THRESHOLD consecutive single-sector // DOUBLE back toward `state.initial_batch` on each clean read.
// successes. Rationale: dense damage scattered through a // Rationale: dense damage scattered through a
// NonTrimmed range is rare — most "bad ranges" in pass N have // NonTrimmed range is rare — most "bad ranges" in pass N have
// lots of good sectors that swept-by-default landed inside. // lots of good sectors that swept-by-default landed inside.
// Batch reads walk those at ~32x the speed of singles, // Batch reads walk those at ~32x the speed of singles,