diff --git a/CHANGELOG.md b/CHANGELOG.md index 4901cbb..de0d94a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,51 @@ # Changelog +## 0.13.15 (2026-04-26) + +### Breaking: `on_progress` callback gains `pos` parameter + +Both `CopyOptions::on_progress` and `PatchOptions::on_progress` now take +`Fn(bytes_good: u64, pos: u64, total_bytes: u64)`. The new `pos` parameter +is the current sweep / retry position. Pass 1 callers should display +`pos / total_bytes` for the "% swept" UI bar — `bytes_good` only counts +clean reads (Finished sectors) and freezes during skip-forward bad zones, +which made every previous version's UI look hung at the bad-zone boundary. +This was the v0.13.9 stall-guard origin bug. + +Live trace from v0.13.14: Pass 1 hit a Dune 2 bad zone at 24 GB and +appeared "stuck" for 14 minutes per autorip's UI (`bytes_good = 23.97 GB` +unchanged). Disc trace events showed `pos` actually advanced from 25.8 GB +to 70 GB during that window — Pass 1 was 83 % through the disc, marking +the post-bad-zone NonTrimmed via skip-forward exactly as designed. The +display lied. Now consumers can show the truth. + +### Feature: `PatchOptions::reverse` for reverse-direction retry passes + +When set, `Disc::patch` walks bad ranges from highest LBA to lowest, and +within each range reads sectors back-to-front. Hypothesis (per the live +v0.13.14 test): drives that wedge after a forward read of a bad sector +read fine when approached from end-of-disc backward — most of the +post-bad-zone NonTrimmed range is actually clean data the drive could +have read on Pass 1 had it not been wedged. autorip alternates F/R +across retry passes (Pass 2 = reverse half-batch, Pass 3 = forward +quarter-batch, ...). + +### Feature: `PatchOptions::wedged_threshold` early-exit + +When > 0, `Disc::patch` exits early if it sees this many consecutive +read failures with zero successful reads in the same pass. Saves the +wallclock budget for productive grinding when the drive has clearly +wedged on the bad zone for this pass — a future pass with a different +direction or block size may still recover. Reported via new +`PatchResult::wedged_exit: bool`. + +### Trace: `patch_start` and `patch_done` events + +`freemkv::disc` target now emits `patch_start` (block_sectors, recovery, +reverse, wedged_threshold, num_ranges) and `patch_done` +(blocks_attempted, blocks_read_ok, blocks_read_failed, wedged_exit, +halted, bytes_recovered) at Disc::patch boundaries. + ## 0.13.14 (2026-04-25) ### Sync release — no functional changes in libfreemkv diff --git a/Cargo.toml b/Cargo.toml index 94f9b5b..cabc425 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "libfreemkv" -version = "0.13.14" +version = "0.13.15" edition = "2024" rust-version = "1.86" license = "AGPL-3.0-only" diff --git a/src/disc/mod.rs b/src/disc/mod.rs index 4f9a52d..d5e6485 100644 --- a/src/disc/mod.rs +++ b/src/disc/mod.rs @@ -1420,7 +1420,7 @@ impl Disc { if let Some(cb) = opts.on_progress { let stats = map.stats(); - cb(stats.bytes_good, total_bytes); + cb(stats.bytes_good, pos, total_bytes); } } } @@ -1470,7 +1470,13 @@ pub struct CopyOptions<'a> { /// `skip_on_error`. The skipped region is marked `non-trimmed` for later /// trimming/scraping by `Disc::patch`. pub skip_forward: bool, - pub on_progress: Option<&'a dyn Fn(u64, u64)>, + /// Callback fired per inner-loop iteration with + /// `(bytes_good, pos, total_bytes)`. `pos` is the current sweep + /// position — true Pass 1 progress, including skipped-forward NonTrimmed + /// ranges. `bytes_good` is the count of `Finished` (clean) sectors, + /// which doesn't advance through bad zones. UI should display `pos` + /// for "swept" progress and `bytes_good` for "real data recovered". + pub on_progress: Option<&'a dyn Fn(u64, u64, u64)>, pub halt: Option>, } @@ -1504,7 +1510,20 @@ pub struct PatchOptions<'a> { /// Use full drive-level recovery on each read (slow but thorough). Defaults /// to true — patch is the pass where we *want* the drive to try hard. pub full_recovery: bool, - pub on_progress: Option<&'a dyn Fn(u64, u64)>, + /// Walk bad ranges in reverse order, and within each range walk sectors + /// from high to low LBA. Useful for drives that wedge after a forward + /// read of a bad sector — approaching the post-bad-zone from end-of-disc + /// reads good sectors before the drive sees a bad one. + pub reverse: bool, + /// Bail out early if this many consecutive read failures occur with zero + /// successful reads in the same pass — i.e. the drive is wedged on the + /// bad zone and won't recover during this attempt. `0` disables the + /// guard (run to completion or halt). + pub wedged_threshold: u64, + /// Callback fired per inner-loop iteration with + /// `(bytes_good, pos, total_bytes)`. `pos` is the current LBA-byte + /// position within the patch walk; for reverse passes it counts down. + pub on_progress: Option<&'a dyn Fn(u64, u64, u64)>, pub halt: Option>, } @@ -1524,6 +1543,10 @@ pub struct PatchResult { pub blocks_read_ok: u64, /// Reads that returned Err and were marked `Unreadable`. pub blocks_read_failed: u64, + /// Pass exited early because `wedged_threshold` consecutive failures + /// occurred with zero successful reads — drive appears wedged on the + /// bad zone for this pass. + pub wedged_exit: bool, } impl Disc { @@ -1561,32 +1584,64 @@ impl Disc { let bytes_good_before = map.stats().bytes_good; let mut halted = false; + let mut wedged_exit = false; let mut blocks_attempted: u64 = 0; let mut blocks_read_ok: u64 = 0; let mut blocks_read_failed: u64 = 0; + let mut consecutive_failures: u64 = 0; let mut buf = vec![0u8; block_sectors as usize * 2048]; // Collect bad ranges up front. Iterating while mutating is fragile; // each recorded change is persisted, so resume works even if we crash // mid-loop. - let bad_ranges = map.ranges_with(&[ + let mut bad_ranges = map.ranges_with(&[ mapfile::SectorStatus::NonTried, mapfile::SectorStatus::NonTrimmed, mapfile::SectorStatus::NonScraped, mapfile::SectorStatus::Unreadable, ]); + // Reverse mode: walk ranges from highest LBA to lowest. + if opts.reverse { + bad_ranges.reverse(); + } + tracing::trace!( + target: "freemkv::disc", + phase = "patch_start", + block_sectors, + recovery, + reverse = opts.reverse, + wedged_threshold = opts.wedged_threshold, + num_ranges = bad_ranges.len(), + "Disc::patch entered" + ); 'outer: for (range_pos, range_size) in bad_ranges { - let mut pos = range_pos; let end = range_pos + range_size; - while pos < end { + // In reverse mode, walk this range from end - block_bytes back to range_pos. + // Each iteration emits the block ending at `block_end` (so reads land on + // increasing LBAs internally; we just choose blocks back-to-front). + let mut block_end = if opts.reverse { end } else { range_pos }; + loop { if let Some(ref h) = opts.halt { if h.load(std::sync::atomic::Ordering::Relaxed) { halted = true; break 'outer; } } - let block_bytes = (end - pos).min(block_sectors as u64 * 2048); + // Compute block boundaries based on direction. + let (pos, block_bytes) = if opts.reverse { + if block_end <= range_pos { + break; + } + let span = (block_end - range_pos).min(block_sectors as u64 * 2048); + (block_end - span, span) + } else { + if block_end >= end { + break; + } + let span = (end - block_end).min(block_sectors as u64 * 2048); + (block_end, span) + }; let lba = (pos / 2048) as u32; let count = (block_bytes / 2048) as u16; let bytes = count as usize * 2048; @@ -1596,6 +1651,7 @@ impl Disc { .is_ok(); if read_ok { blocks_read_ok += 1; + consecutive_failures = 0; if opts.decrypt { crate::decrypt::decrypt_sectors(&mut buf[..bytes], &keys, 0)?; } @@ -1607,20 +1663,57 @@ impl Disc { .map_err(|e| Error::IoError { source: e })?; } else { blocks_read_failed += 1; + consecutive_failures += 1; map.record(pos, block_bytes, mapfile::SectorStatus::Unreadable) .map_err(|e| Error::IoError { source: e })?; } - pos += block_bytes; + // Advance block_end in chosen direction. + if opts.reverse { + block_end = block_end.saturating_sub(block_bytes); + } else { + block_end += block_bytes; + } + + // Wedged-drive early-exit: many consecutive failures with zero + // recovered bytes this pass means the drive is stuck and won't + // produce data this pass. Save the wallclock budget for productive + // grinding; future passes (with smaller block size, reverse, or + // after settle) may still recover. + if opts.wedged_threshold > 0 + && consecutive_failures >= opts.wedged_threshold + && blocks_read_ok == 0 + { + tracing::trace!( + target: "freemkv::disc", + phase = "patch_wedged_exit", + consecutive_failures, + blocks_read_failed, + "Disc::patch giving up — drive appears wedged" + ); + wedged_exit = true; + break 'outer; + } if let Some(cb) = opts.on_progress { let s = map.stats(); - cb(s.bytes_good, total_bytes); + cb(s.bytes_good, pos, total_bytes); } } } file.sync_all().map_err(|e| Error::IoError { source: e })?; let stats = map.stats(); + tracing::trace!( + target: "freemkv::disc", + phase = "patch_done", + blocks_attempted, + blocks_read_ok, + blocks_read_failed, + wedged_exit, + halted, + bytes_recovered = stats.bytes_good.saturating_sub(bytes_good_before), + "Disc::patch returning" + ); Ok(PatchResult { bytes_total: total_bytes, bytes_good: stats.bytes_good, @@ -1631,6 +1724,7 @@ impl Disc { blocks_attempted, blocks_read_ok, blocks_read_failed, + wedged_exit, }) } } diff --git a/tests/integration_progress_and_halt.rs b/tests/integration_progress_and_halt.rs index 0cc3c70..ef2ff9e 100644 --- a/tests/integration_progress_and_halt.rs +++ b/tests/integration_progress_and_halt.rs @@ -178,7 +178,7 @@ fn test_disc_copy_progress_callback_fires() { let calls_cb = calls.clone(); let last_bytes_cb = last_bytes.clone(); - let progress = move |bytes: u64, _total: u64| { + let progress = move |bytes: u64, _pos: u64, _total: u64| { calls_cb.fetch_add(1, Ordering::Relaxed); last_bytes_cb.store(bytes, Ordering::Relaxed); };