drive: disable FUA on READ(10) — the ~10x sequential-read regression

Forcing FUA (Force Unit Access) on EVERY READ(10) bypassed the drive's
readahead/streaming cache on the bulk Pass-1 sweep, collapsing sequential
throughput ~10x (UHD 15-25 → ~2 MB/s, DVD → ~0.5 MB/s), disc-type-
agnostic — the cache IS the streaming throughput. This was the real speed
regression (not unlock/riplock, which the earlier chase suspected). Clear
byte-1 bit 0x08. FUA will return as a dedicated Pass-N recovery handler
that sets/clears it per marginal-sector re-read, where cache-masking of a
stochastic sector actually matters — never blanket on the bulk path (#55).
This commit is contained in:
Matthew Jackson
2026-07-01 10:26:56 -07:00
parent c781fb7193
commit d5d72c4e22
+18 -16
View File
@@ -719,22 +719,23 @@ impl Drive {
count: u16, count: u16,
buf: &mut [u8], buf: &mut [u8],
timeout_ms: u32, timeout_ms: u32,
// `recovery` only gates the Linux /dev/sr0 pread fallback below; on // `recovery` gates only the Linux /dev/sr0 pread fallback below; on
// other platforms it is intentionally unused. // other platforms it is intentionally unused.
#[cfg_attr(not(target_os = "linux"), allow(unused_variables))] recovery: bool, #[cfg_attr(not(target_os = "linux"), allow(unused_variables))] recovery: bool,
) -> Result<usize> { ) -> Result<usize> {
// FUA (Force Unit Access) is DISABLED for now — byte-1 bit 0x08 cleared.
// Unconditionally forcing every READ(10) past the drive cache disabled
// the drive's readahead/streaming cache on the bulk sequential sweep and
// collapsed throughput ~10x (UHD 15-25 → ~2 MB/s, DVD → ~0.5 MB/s),
// disc-type-agnostic — the cache IS the streaming throughput.
//
// TODO(#55): reintroduce FUA as a dedicated Pass-N recovery HANDLER that
// sets/clears it per marginal-sector re-read (where cache-masking of a
// stochastic sector actually matters), never blanket-applied to the bulk
// read path.
let cdb = [ let cdb = [
crate::scsi::SCSI_READ_10, crate::scsi::SCSI_READ_10,
// FUA (Force Unit Access, bit 3 = 0x08): every read comes from the 0x00,
// physical MEDIA, never the drive's cache. A recovery tool must not
// trust the cache — the BU40N caches aggressively (a re-read of a
// good sector returns in ~4 ms vs ~250 ms from media), so without
// FUA a retry of a marginal sector could return a cached result
// instead of giving the surface a fresh physical attempt. A
// stochastic sector that would read on a real media hit is otherwise
// masked by a cached miss. Cache-bypass is validated on the
// BU40N/Initio bridge (FUA reads execute, media 40 ms vs cache 4 ms).
0x08,
(lba >> 24) as u8, (lba >> 24) as u8,
(lba >> 16) as u8, (lba >> 16) as u8,
(lba >> 8) as u8, (lba >> 8) as u8,
@@ -1414,9 +1415,10 @@ mod command_tests {
#[test] #[test]
fn read_builds_read10_cdb_with_be_lba_and_count() { fn read_builds_read10_cdb_with_be_lba_and_count() {
// Drive::read issues READ(10) (0x28). LBA bytes 2..5 big-endian, // Drive::read issues READ(10) (0x28). LBA bytes 2..5 big-endian,
// transfer length bytes 7..8 big-endian (MMC-6). FUA is set (byte 1 == // transfer length bytes 7..8 big-endian (MMC-6). FUA is DISABLED (byte 1
// 0x08) so every read bypasses the drive cache and hits physical media. // == 0x00) so the drive cache/readahead is allowed — forcing FUA on the
// Distinct nibbles catch a swapped shift. // bulk sweep collapsed throughput ~10x. Distinct nibbles catch a swapped
// shift.
let RecordingHarness { let RecordingHarness {
drive: mut d, drive: mut d,
cdb, cdb,
@@ -1428,8 +1430,8 @@ mod command_tests {
let c = cdb.lock().unwrap(); let c = cdb.lock().unwrap();
assert_eq!(c[0], crate::scsi::SCSI_READ_10); assert_eq!(c[0], crate::scsi::SCSI_READ_10);
assert_eq!( assert_eq!(
c[1], 0x08, c[1], 0x00,
"Drive::read sets FUA (force media, bypass cache)" "FUA disabled — cache/readahead allowed on the bulk read path"
); );
assert_eq!(&c[2..6], &[0x00, 0xAB, 0xCD, 0xEF], "LBA big-endian"); assert_eq!(&c[2..6], &[0x00, 0xAB, 0xCD, 0xEF], "LBA big-endian");
assert_eq!(&c[7..9], &[0x00, 0x02], "transfer length big-endian"); assert_eq!(&c[7..9], &[0x00, 0x02], "transfer length big-endian");