From 2a534b23f58f9e9d57252b5bb5e3ef358b87a61b Mon Sep 17 00:00:00 2001 From: Matthew Jackson Date: Sun, 10 May 2026 19:57:27 -0700 Subject: [PATCH] =?UTF-8?q?disc/patch:=20adaptive=20batching=20=E2=80=94?= =?UTF-8?q?=2032=20sectors,=20drop=20to=201=20on=20failure?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pass N now reads at 32 sectors per attempt and drops to 1 only on batch-read failure to probe each sector individually. After 16 consecutive clean single-sector reads it climbs back to 32. Net effect: NonTrimmed regions walk ~32x faster in clean stretches without sacrificing per-sector recovery quality — the drop-to-1 retry from the same cursor position guarantees every sector in a failed batch is individually attempted. Design contract: - A batch-read failure (count > 1) is NOT a recorded failure: no NonTrimmed mark, no consecutive_failures bump, no damage_window push, cursor stays put. We just drop current_batch to 1 and the loop re-attempts the same position at single-sector granularity. - A single-sector failure (count == 1) follows the existing path: NonTrimmed mark, consecutive_failures++, damage_window.push(false), post-failure pause, wedge probes. - Backtrack always at count=1: this path fills a gap that the main loop's damage-window skip jumped over. Using batched reads there would lump good sectors into NonTrimmed marks when the gap contains even one bad sector. State machine adds: - `initial_batch` (from opts.block_sectors, default 32 in patch_internal) - `current_batch` (mutable, starts at initial_batch, drops to 1 on batch failure) - `consecutive_singles_ok` (counter, resets on upscale + failure) - `ADAPTIVE_UPSCALE_THRESHOLD = 16` (matches sweep's pattern for "16 consecutive good = back to fast mode") Tests: - pass_n_size_aware_skip.rs PatternedSectorReader now fills each sector with its OWN LBA byte (not the starting LBA's byte). This matches real drive behavior — the pre-0.18.13 fixture's "fill whole batch with one byte" was a shortcut that only worked when patch read 1 sector at a time. Existing recovery-quality assertions all still pass under adaptive batching. User spec: "try 32, pass, great, fail -> do 1 sector" --- src/disc/mod.rs | 97 ++++++++++++++++++++++++++++++--- tests/pass_n_size_aware_skip.rs | 10 +++- 2 files changed, 97 insertions(+), 10 deletions(-) diff --git a/src/disc/mod.rs b/src/disc/mod.rs index f239b37..e695ff1 100644 --- a/src/disc/mod.rs +++ b/src/disc/mod.rs @@ -1390,7 +1390,15 @@ impl Disc { ) -> Result { let patch_opts = PatchOptions { decrypt: opts.decrypt, - block_sectors: Some(1), + // 0.18.13: adaptive batching. patch() reads at 32 sectors + // when the drive is healthy, drops to 1 on failure to + // probe each sector individually, then climbs back after + // 16 consecutive clean singles. Walks NonTrimmed regions + // ~32x faster in clean stretches without sacrificing any + // per-sector recovery quality — the drop-to-1 retry from + // the same position guarantees every sector in a failed + // batch is individually probed. See Disc::patch body. + block_sectors: Some(32), full_recovery: true, reverse: true, wedged_threshold: 50, @@ -2202,7 +2210,23 @@ impl Disc { ); } - let block_sectors = opts.block_sectors.unwrap_or(1); + // Adaptive batching: read at `current_batch`, drop to 1 on + // batch-read failure, climb back to `initial_batch` after + // ADAPTIVE_UPSCALE_THRESHOLD consecutive single-sector successes. + // Rationale: dense damage scattered through a NonTrimmed range + // is rare — most "bad ranges" in pass N have lots of good + // sectors that swept-by-default landed inside. Batch reads + // walk those at ~32x the speed of singles, dropping to 1 + // only when the drive actually returns an error. Guarantees: + // - no good sector is ever marked NonTrimmed because it + // was bundled in a failed batch — failed batches are + // "split decisions", not recorded failures + // - drop-to-1 retries the SAME starting position, so every + // sector in the failed batch is individually probed + let initial_batch = opts.block_sectors.unwrap_or(1); + let mut current_batch: u16 = initial_batch; + let mut consecutive_singles_ok: u32 = 0; + const ADAPTIVE_UPSCALE_THRESHOLD: u32 = 16; let recovery = opts.full_recovery; let mut halted = false; @@ -2239,7 +2263,7 @@ impl Disc { 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]; + let mut buf = vec![0u8; initial_batch as usize * 2048]; // Pass 2 uses smaller sectors (1 vs 32) but same damage detection logic const PASSN_DAMAGE_WINDOW: usize = 16; @@ -2322,7 +2346,7 @@ impl Disc { tracing::info!( target: "freemkv::disc", phase = "patch_start", - block_sectors, + block_sectors = initial_batch, recovery, reverse = opts.reverse, wedged_threshold = opts.wedged_threshold, @@ -2463,13 +2487,13 @@ impl Disc { if block_end <= *range_pos { break; } - let span = (block_end - *range_pos).min(block_sectors as u64 * 2048); + let span = (block_end - *range_pos).min(current_batch as u64 * 2048); (block_end - span, span) } else { if block_end >= end { break; } - let span = (end - block_end).min(block_sectors as u64 * 2048); + let span = (end - block_end).min(current_batch as u64 * 2048); (block_end, span) }; let lba = (pos / 2048) as u32; @@ -2532,6 +2556,28 @@ impl Disc { if consecutive_good_since_skip >= PASSN_ESCALATION_RESET_GOOD { consecutive_skips_without_recovery = 0; } + // Adaptive batching: track clean single-sector reads to + // decide when to climb back to `initial_batch`. A batch + // read succeeding (count > 1) tells us the drive is healthy + // but doesn't accumulate toward upscale — we got back to + // batch=1 because of a failure here, we need consistent + // health at the slow tempo before scaling up again. + if count == 1 && current_batch < initial_batch { + consecutive_singles_ok += 1; + if consecutive_singles_ok >= ADAPTIVE_UPSCALE_THRESHOLD { + tracing::info!( + target: "freemkv::disc", + phase = "patch_adaptive_upscale", + from = current_batch, + to = initial_batch, + consecutive_singles_ok, + lba, + "adaptive batching: drive stable, climbing back to initial_batch" + ); + current_batch = initial_batch; + consecutive_singles_ok = 0; + } + } damage_window.push(true); if damage_window.len() > PASSN_DAMAGE_WINDOW { damage_window.remove(0); @@ -2629,7 +2675,14 @@ impl Disc { let mut bt_pos = backtrack_start; while bt_pos < backtrack_end { let span = - (backtrack_end - bt_pos).min(block_sectors as u64 * 2048); + // Backtrack always at count=1: this path + // fills a gap that the main loop's damage- + // window skip jumped over. Using batched + // reads here would lump good sectors into + // NonTrimmed marks when the gap contains + // even one bad sector. Backtrack is rare + // enough that the per-sector cost is fine. + (backtrack_end - bt_pos).min(2048); let bt_lba = (bt_pos / 2048) as u32; let bt_count = (span / 2048) as u16; let bt_bytes = bt_count as usize * 2048; @@ -2685,9 +2738,37 @@ impl Disc { } } Err(err) => { + // Adaptive batching split decision: a batch-read + // failure (count > 1) is NOT a recorded failure. + // We don't yet know which sector in the batch was + // actually bad — could be one, could be many. + // Drop to count=1 and retry the SAME starting + // position so every sector gets individually + // probed. Cursor stays put; loop continues. + // Invariants: no good sector ever gets lumped + // into a NonTrimmed mark, no spurious + // consecutive_failures (which drives wedge + // detection), no damage_window pollution from + // batch-level signals. + if count > 1 { + tracing::info!( + target: "freemkv::disc", + phase = "patch_adaptive_split", + lba, + count, + from_batch = current_batch, + err_code = err.code(), + "adaptive batching: batch read failed, dropping to count=1 to probe individually" + ); + current_batch = 1; + consecutive_singles_ok = 0; + continue; + } + blocks_read_failed += 1; consecutive_failures += 1; consecutive_good_since_skip = 0; + consecutive_singles_ok = 0; unreadable_count += 1; tracing::warn!( @@ -3111,7 +3192,7 @@ impl Disc { if let Some(reporter) = opts.progress { let (s, bad_ranges_now) = read_shared(&shared); - let kind = if block_sectors == 1 { + let kind = if initial_batch == 1 { crate::progress::PassKind::Scrape { reverse: opts.reverse, } diff --git a/tests/pass_n_size_aware_skip.rs b/tests/pass_n_size_aware_skip.rs index 6996d83..9b445d9 100644 --- a/tests/pass_n_size_aware_skip.rs +++ b/tests/pass_n_size_aware_skip.rs @@ -70,8 +70,14 @@ impl SectorReader for PatternedSectorReader { }); } } - for chunk in buf.chunks_mut(SECTOR_SIZE) { - chunk.fill((lba & 0xff) as u8); + // Fill each sector with ITS OWN LBA byte, not the starting LBA's + // byte. This matches real drive behavior: a multi-sector READ + // returns per-sector-correct data. Pre-0.18.13 only single-sector + // reads were exercised by patch tests, so the cheaper "fill the + // whole batch with one byte" worked; adaptive batching needs the + // per-sector pattern to verify correct positioning. + for (i, chunk) in buf.chunks_mut(SECTOR_SIZE).enumerate() { + chunk.fill(((lba + i as u32) & 0xff) as u8); } Ok(buf.len()) }