From c9d6eff6f2c93c59cfc7232c78bc31808f13ed9b Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 11 Nov 2026 00:04:13 +0200 Subject: [PATCH 1/4] tests: osbench's disk groups as a metal row of their own `osbench_disk` runs `files`, `nvme` and `persist` alone on the bench's NVMe, so a change to the disk's path is measured without the whole suite's boot. It is the row PR #1131 introduced; it lands here first because the forced-write change that follows is measured with it, and its base arm needs the row too. Only a pass that names it runs it: the nightly `osbench` boot measures the same groups. Co-Authored-By: Claude Fable 5.2 --- tests/guest/osbench/README.md | 3 +++ tests/toyos.rs | 26 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/tests/guest/osbench/README.md b/tests/guest/osbench/README.md index dbdd42f614e..d4bb9fa2151 100644 --- a/tests/guest/osbench/README.md +++ b/tests/guest/osbench/README.md @@ -234,6 +234,9 @@ What makes the disk rows one comparison, and what stays each system's own: evicting in insertion order, and the 4 KiB sequential read misses (`issues/a-sequential-read-of-a-file-as-large-as-datas-clean-cache-reads-the-disk.md`). +The `osbench_disk` metal row runs `files`, `nvme` and `persist` alone, on +the benchcase image. + ## What is not a comparison The table also holds rows only one system can measure. They show the diff --git a/tests/toyos.rs b/tests/toyos.rs index 853180bd977..ba80b0e8e90 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -1297,6 +1297,21 @@ const METAL: &[(&str, metal::Metal)] = &[ "clock_holds_over_minutes", metal::Metal { arms: BENCHCASE, judge: |b| clock_holds_over_minutes(b[0]) }, ), + ( + // `osbench files`, `nvme` and `persist` alone: the disk's figures, on + // the bench's NVMe, for a change to the disk's path that the whole + // suite's boot need not measure. Evidence and not a verdict, as + // `osbench` is, whose boot measures the same groups: only a pass + // that names it runs it. The T14 and the AMD bench. + "osbench_disk", + metal::Metal { + arms: OSBENCH_DISK, + judge: |b| { + data_on_the_nvme(b[0])?; + osbench_ran(b[0], &OSBENCH_DISK_GROUPS) + }, + }, + ), ( // `osbench lan` against the `osbench-peer` the other bench's Linux runs, // compared with the same program under this bench's Linux on the same @@ -1866,6 +1881,17 @@ const OSBENCH_ENERGY: &[metal::Arm] = &[metal::Arm { ..metal::once("energybenchcase", "tests/images/benchcase.toml", &[], &["osbench_energy"]) }]; +/// The groups that write the disk, in the suite's order. +const OSBENCH_DISK_GROUPS: [&str; 3] = ["osbench_files", "osbench_nvme", "osbench_persist"]; + +/// No boot has timed the disk's groups apart from the rest of the suite, so +/// they take the whole suite's bound, [`BENCHCASE`]'s, which covers them. +const OSBENCH_DISK: &[metal::Arm] = &[metal::Arm { + adds_ms: 465_000, + named_only: true, + ..metal::once("diskbenchcase", "tests/images/benchcase.toml", &[], &OSBENCH_DISK_GROUPS) +}]; + /// The two groups that need another party share one boot when one pass names /// both. Each adds its own worst span; the 60 s every list has is the /// boot's margin, not either group's. From 8a8ff27b969b8a0339237ac544f21ea4419546fc Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 11 Nov 2026 00:36:45 +0200 Subject: [PATCH 2/4] A journal entry is one flush and a forced write, not two flushes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nothing in the native write path could make one write durable by itself: the block protocol, the NVMe driver, the USB mass-storage driver and bcachefs knew only a write and a flush of the whole device. So every bcachefs journal entry was written between two flushes, and an entry naming itself between three. A write may now be asked with FUA, and is on the medium when it is answered: - `BlockDev::write_durable`. Its default is the fallback, stated once: a device that cannot force one write writes it and then flushes. - The block protocol's write carries `fua` (`toyos_blockring::entry`), and an open's answer says whether the session's device takes it. A server refuses a forced write it did not offer. A forced write acknowledged is recorded under no loss count and kept by no client: no reset can take it, so nothing is issued again for it. The caller's side of that is a contract the crash model checks: a forced write never goes over a write of the same client no flush has covered. - NVMe sets FUA in the Write command (NVM Command Set 1.0 §3.2.6, Command Dword 12 bit 30). Every controller takes the bit, so every NVMe session offers it. - USB mass storage sets FUA in WRITE(10) (SBC-3 §5.32, byte 1 bit 3) where the device's mode parameters set DPOFUA, which the driver now asks with one MODE SENSE(6) as a disk first comes up. A disk that does not say so is offered without, its line says so, and its clients write and flush. A forced write leaves no flush owed when the device leaves. - The file server's cache forces a durable write's blocks onto the disk alone, in the flush's place in line, with no flush of the disk. - The journal writes each entry after one flush and forced. An entry naming itself is forced headless and then its header block forced: no flush between. Since the cache asks the disk for no flush when nothing was written since the last, a commit of names alone is one forced write. Tests, all on the host. The protocol's exhaustive model gains forced writes and the law that one answered done stays on the medium through every reset, death and device error; its control sends the write unforced and reds that law. The crash model takes a forced write as torn until it completes and durable after, and refuses one over an unflushed write. The journal's crash tests judge every state while an entry is still torn and once it is forced; a new test holds a commit to exactly one flush and one forced write, and shows a device that leaves the entry in its cache losing a committed create. Co-Authored-By: Claude Fable 5.2 --- build/ci.rs | 9 +- docs/src/architecture/storage.md | 4 + ...storage-since-the-benches-boot-off-nvme.md | 5 + lib/bcachefs/src/engine/mkfs.rs | 18 +- lib/bcachefs/src/engine/mount/mod.rs | 20 +-- lib/bcachefs/src/engine/tests/crash.rs | 107 ++++++++++- lib/blockdev/src/lib.rs | 15 ++ lib/blockdev/src/model.rs | 56 ++++++ lib/blockring/Cargo.toml | 4 + lib/blockring/src/client.rs | 66 +++++-- lib/blockring/src/entry.rs | 52 ++++-- lib/blockring/src/model.rs | 129 +++++++++++--- lib/blockring/src/server.rs | 68 +++++-- lib/blockring/src/wire.rs | 50 ++++-- lib/blockserve/src/lib.rs | 2 +- lib/blockserve/src/sessions.rs | 45 +++-- lib/scsi/src/lib.rs | 89 ++++++++-- system/diskserver/src/disk.rs | 45 ++++- system/diskserver/src/disk/tests.rs | 52 +++++- system/diskserver/src/session.rs | 15 +- system/fileserver/src/cache.rs | 166 +++++++++++++++--- system/nvme/src/main.rs | 12 +- system/nvme/src/nvme.rs | 45 ++++- system/usbd/src/main.rs | 15 +- system/usbd/xhci/src/bot.rs | 5 +- system/usbd/xhci/src/msc.rs | 54 ++++-- system/usbd/xhci/src/stub/devices.rs | 15 ++ system/usbd/xhci/src/tests.rs | 62 ++++++- 28 files changed, 1003 insertions(+), 222 deletions(-) diff --git a/build/ci.rs b/build/ci.rs index 32a8297bd99..afd84b61758 100644 --- a/build/ci.rs +++ b/build/ci.rs @@ -754,9 +754,9 @@ pub(crate) const CONTROLS: &[Control] = &[ red(KERNEL_SIM, "placement-ignores-staleness", Some("policy"), &[ Fails("a_stopped_cpu_stops_taking_work"), ]), - // The block protocol's three: a completion lost to a session's end, a - // completion given twice after a reset, and a loss nothing is written - // again after. + // The block protocol's four: a completion lost to a session's end, a + // completion given twice after a reset, a loss nothing is written again + // after, and a forced write the device was not told to force. red(BLOCKRING, "mutate-session-end-forgets", None, &[ Fails("model::tests::every_request_is_answered_exactly_once"), ]), @@ -766,6 +766,9 @@ pub(crate) const CONTROLS: &[Control] = &[ red(BLOCKRING, "mutate-no-reissue-after-loss", None, &[ Fails("model::tests::what_a_flush_calls_durable_is_on_the_medium"), ]), + red(BLOCKRING, "mutate-fua-goes-out-unforced", None, &[ + Fails("model::tests::a_forced_write_answered_done_stays_on_the_medium"), + ]), // The link's one: a full ring's want swapped `Relaxed`, a wake lost. red(LINK, "mutate-want-relaxed", Some("ends_loom"), &[Fails("a_full_ring_is_never_left_asleep_on_room_it_has")]), red(SDK, "transport-publish-relaxed", Some("transport_loom"), &[ diff --git a/docs/src/architecture/storage.md b/docs/src/architecture/storage.md index d6c83408bc1..1b8266bf3b3 100644 --- a/docs/src/architecture/storage.md +++ b/docs/src/architecture/storage.md @@ -23,6 +23,10 @@ All four layers run in userland. block service. - A partition has one holder at a time, every request is bounded to it before the device sees it, and a flush answers for its own writer's writes. +- A write may be asked onto the medium by itself, without a flush of the + whole device; a filesystem's journal commits with one. A device that cannot + force a single write is written and then flushed instead, and its driver + says so when it brings the disk up. - Caching belongs to file servers, never to the block layer. - Partition editing and storage initialisation are userland operations shared by disk tools and the installer. Editing the table, reading payloads and diff --git a/issues/no-bench-row-drives-usb-storage-since-the-benches-boot-off-nvme.md b/issues/no-bench-row-drives-usb-storage-since-the-benches-boot-off-nvme.md index 3a1ae8c10e9..7219f7502e7 100644 --- a/issues/no-bench-row-drives-usb-storage-since-the-benches-boot-off-nvme.md +++ b/issues/no-bench-row-drives-usb-storage-since-the-benches-boot-off-nvme.md @@ -31,6 +31,11 @@ the boot stick lost it: - `amdvi_translates_the_boot_controller` and `amdvi_remaps_the_boot_controller` judge the NVMe controller where they judged the stick's xHCI. +- Whether a stick's mode parameters set DPOFUA, and whether it then takes a + WRITE(10) with FUA, is read on no bench: usbd asks MODE SENSE as a disk + first comes up and forces a durable write only where the stick says so + (`system/usbd/xhci/src/msc.rs`), judged against a host model alone. + QEMU's machines still boot off a stick (`the_log_lands_on_the_boot_stick`). No row that writes the scratch disk is built. The owner declined the diff --git a/lib/bcachefs/src/engine/mkfs.rs b/lib/bcachefs/src/engine/mkfs.rs index 18bcd542382..ab64e01c831 100644 --- a/lib/bcachefs/src/engine/mkfs.rs +++ b/lib/bcachefs/src/engine/mkfs.rs @@ -353,12 +353,26 @@ fn fields(geometry: &Geometry, member: &Member, crypt: Option<&Crypt>, roots: &[ out } -pub(crate) async fn write(dev: &impl BlockDev, sector: u64, bytes: &[u8]) -> Result<()> { +/// The device block `sector` is, and `bytes` as whole blocks, zero past +/// their end. +fn blocks(sector: u64, bytes: &[u8]) -> (u64, Vec<[u8; BLOCK]>) { let at = sector * SECTOR as u64; assert!(at.is_multiple_of(BLOCK as u64), "a write off a block boundary"); let mut blocks = alloc::vec![[0u8; BLOCK]; bytes.len().div_ceil(BLOCK)]; blocks.as_flattened_mut()[..bytes.len()].copy_from_slice(bytes); - dev.write(at / BLOCK as u64, &blocks).await?; + (at / BLOCK as u64, blocks) +} + +pub(crate) async fn write(dev: &impl BlockDev, sector: u64, bytes: &[u8]) -> Result<()> { + let (lba, blocks) = blocks(sector, bytes); + dev.write(lba, &blocks).await?; + Ok(()) +} + +/// [`write`], on the medium once it returns ([`BlockDev::write_durable`]). +pub(crate) async fn write_durable(dev: &impl BlockDev, sector: u64, bytes: &[u8]) -> Result<()> { + let (lba, blocks) = blocks(sector, bytes); + dev.write_durable(lba, &blocks).await?; Ok(()) } diff --git a/lib/bcachefs/src/engine/mount/mod.rs b/lib/bcachefs/src/engine/mount/mod.rs index ad288566aad..07716a7a0cc 100644 --- a/lib/bcachefs/src/engine/mount/mod.rs +++ b/lib/bcachefs/src/engine/mount/mod.rs @@ -1441,10 +1441,13 @@ impl Mount { } /// Writes every closed entry through `upto`, in order, each after a - /// flush and followed by one (A2.12), so each is durable once written and - /// so is every node write that completed before it. An entry naming - /// itself has its header sector written last, after a flush. A failed - /// write is reissued with the same bytes, or the mount stops writing. + /// flush and itself durable as it is written (A2.12), so each is durable + /// once written and so is every node write that completed before it. An + /// entry naming itself has its header sector written last, once the rest + /// is durable. The journal's blocks are written no other way, so a + /// durable write of one is never over a write no flush has covered. A + /// failed write is reissued with the same bytes, or the mount stops + /// writing. async fn write_entries(&self, upto: u64) -> Result<()> { self.journal_queue.wait(|| (!self.journal_held.get()).then(|| self.journal_held.set(true))).await; let out = async { @@ -1468,14 +1471,11 @@ impl Mount { if header_last { let mut headless = bytes.clone(); headless[..SECTOR].fill(0); - mkfs::write(&self.dev, sector, &headless).await?; - self.dev.flush().await?; - mkfs::write(&self.dev, sector, &bytes[..BLOCK]).await?; + mkfs::write_durable(&self.dev, sector, &headless).await?; + mkfs::write_durable(&self.dev, sector, &bytes[..BLOCK]).await } else { - mkfs::write(&self.dev, sector, &bytes).await?; + mkfs::write_durable(&self.dev, sector, &bytes).await } - self.dev.flush().await?; - Ok(()) }) .await?; let mut st = self.st.borrow_mut(); diff --git a/lib/bcachefs/src/engine/tests/crash.rs b/lib/bcachefs/src/engine/tests/crash.rs index 9a7d20a15da..0e3445a4730 100644 --- a/lib/bcachefs/src/engine/tests/crash.rs +++ b/lib/bcachefs/src/engine/tests/crash.rs @@ -16,7 +16,8 @@ use super::mount::{random, MEMORY, NOW, OWNER}; use super::*; /// A device that, after each write and flush it passes on, hands the model -/// to `after`. +/// to `after`: a durable write both while it is still torn and once it is on +/// the medium. pub(super) struct Watched<'a> { pub model: &'a Model, pub after: Box, @@ -42,6 +43,14 @@ impl BlockDev for Watched<'_> { (self.after)(self.model); Ok(()) } + + async fn write_durable(&self, lba: u64, data: &[Block]) -> core::result::Result<(), toyos_blockdev::Error> { + self.model.write(lba, data).await?; + (self.after)(self.model); + self.model.forced(); + (self.after)(self.model); + Ok(()) + } } const MIN_LIMITS: toyos_crypto::scrypt::Limits = toyos_crypto::scrypt::Limits { memory: 1 << 20, work: 1 << 20 }; @@ -240,6 +249,102 @@ pub(super) fn ops_survive_a_crash_on(image: Vec, passphrase: Option<&[u8]>, reused } +/// What a mount asked of its device, in order. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum Asked { + Write, + Flush, + /// A durable write at this block. + Durable(u64), +} + +/// A device that records what it is asked, and takes a durable write +/// forced, or, with `forces` off, as a write it leaves in its cache. +struct Recorded<'a> { + model: &'a Model, + forces: bool, + asked: core::cell::RefCell>, +} + +impl BlockDev for Recorded<'_> { + fn blocks(&self) -> u64 { + self.model.blocks() + } + + async fn read(&self, lba: u64, out: &mut [Block]) -> core::result::Result<(), toyos_blockdev::Error> { + self.model.read(lba, out).await + } + + async fn write(&self, lba: u64, data: &[Block]) -> core::result::Result<(), toyos_blockdev::Error> { + self.asked.borrow_mut().push(Asked::Write); + self.model.write(lba, data).await + } + + async fn flush(&self) -> core::result::Result<(), toyos_blockdev::Error> { + self.asked.borrow_mut().push(Asked::Flush); + self.model.flush().await + } + + async fn write_durable(&self, lba: u64, data: &[Block]) -> core::result::Result<(), toyos_blockdev::Error> { + self.asked.borrow_mut().push(Asked::Durable(lba)); + match self.forces { + true => self.model.write_durable(lba, data).await, + false => self.model.write(lba, data).await, + } + } +} + +/// A commit of one create asks the device for a flush and then its journal +/// entry, durable as written, and nothing after: with a file's data written +/// before it, the same two after the data. Every crash once the commit has +/// returned holds the create. The control: on a device that takes the entry +/// without forcing it, some crash after the commit returned has lost it. +#[test] +fn a_commit_is_one_flush_and_its_entry_forced_and_is_lost_where_the_entry_is_not() { + for forces in [true, false] { + let model = Model::new(formatted(64 << 20, None).into_inner(), 512); + let dev = Recorded { model: &model, forces, asked: core::cell::RefCell::default() }; + let mount = block_on_ready(Mount::open(&dev, MEMORY, random())).unwrap(); + let root = mount.root(); + block_on_ready(async { + // The first entry after a clean state names itself, and goes + // header last. + mount.create(root, b"first", New::File, OWNER, NOW).await.unwrap(); + mount.commit().await.unwrap(); + dev.asked.borrow_mut().clear(); + mount.create(root, b"second", New::File, OWNER, NOW).await.unwrap(); + mount.commit().await.unwrap(); + }); + let asked = dev.asked.take(); + let [Asked::Flush, Asked::Durable(entry)] = asked[..] else { panic!("a commit asked {asked:?}") }; + + let held = model.check(1 << 12, 0xf0a_0001, |image| { + let dev = Overlay::new(image); + let mount = block_on_ready(Mount::open(&dev, MEMORY, random())).map_err(|e| format!("mount: {e:?}"))?; + match block_on_ready(mount.lookup(mount.root(), b"second")) { + Ok(Some(_)) => Ok(()), + other => Err(format!("the committed create is {other:?}")), + } + }); + match (forces, held) { + (true, Ok(_)) => {} + (true, Err(lost)) => panic!("{lost}"), + (false, Err(_)) => continue, + (false, Ok(search)) => panic!("an entry left in the cache was held through {search:?}"), + } + + block_on_ready(async { + let file = mount.lookup(root, b"second").await.unwrap().unwrap().ino; + assert_eq!(mount.write(file, 0, &[7; BLOCK], NOW).await.unwrap(), BLOCK); + mount.commit().await.unwrap(); + }); + let asked = dev.asked.take(); + let [Asked::Write, Asked::Flush, Asked::Durable(next)] = asked[..] else { panic!("a commit after data asked {asked:?}") }; + assert!(next > entry, "the second entry at block {next} is not after the first at {entry}"); + block_on_ready(mount.unmount()).unwrap(); + } +} + fn creates(n: usize) -> Vec { (0..n).map(|i| Op::Create(format!("c{i}").into_bytes())).collect() } diff --git a/lib/blockdev/src/lib.rs b/lib/blockdev/src/lib.rs index da3ea21a259..6b646e84edb 100644 --- a/lib/blockdev/src/lib.rs +++ b/lib/blockdev/src/lib.rs @@ -72,6 +72,15 @@ pub trait BlockDev { /// Every write that completed before this was asked, on the medium. async fn flush(&self) -> Result<(), Error>; + /// `data` at `lba`, on the medium once complete, and nothing else made + /// durable by it. It must not overlap a write of this device's no flush + /// has covered. A device that cannot force one write to its medium + /// writes it and then flushes, which is this. + async fn write_durable(&self, lba: u64, data: &[Block]) -> Result<(), Error> { + self.write(lba, data).await?; + self.flush().await + } + /// [`read`](Self::read) of a file's data rather than the filesystem's own /// structures: a device that caches the structures may pass this by. async fn read_data(&self, lba: u64, out: &mut [Block]) -> Result<(), Error> { @@ -105,6 +114,9 @@ impl BlockDev for &D { async fn flush(&self) -> Result<(), Error> { (**self).flush().await } + async fn write_durable(&self, lba: u64, data: &[Block]) -> Result<(), Error> { + (**self).write_durable(lba, data).await + } async fn read_data(&self, lba: u64, out: &mut [Block]) -> Result<(), Error> { (**self).read_data(lba, out).await } @@ -126,6 +138,9 @@ impl BlockDev for Rc { async fn flush(&self) -> Result<(), Error> { (**self).flush().await } + async fn write_durable(&self, lba: u64, data: &[Block]) -> Result<(), Error> { + (**self).write_durable(lba, data).await + } async fn read_data(&self, lba: u64, out: &mut [Block]) -> Result<(), Error> { (**self).read_data(lba, out).await } diff --git a/lib/blockdev/src/model.rs b/lib/blockdev/src/model.rs index 41ae752ea1b..892bfdf7099 100644 --- a/lib/blockdev/src/model.rs +++ b/lib/blockdev/src/model.rs @@ -8,6 +8,9 @@ //! covers holds, after the crash, either what was durable or what any one of //! those writes carried; a write kept in part is a torn write. //! +//! **A forced write is on the medium once it completes** ([`Model::forced`]), +//! and until then is torn like any other. +//! //! **The unit is the device's**, never the format's block: one logical block //! (512 bytes on a 512-byte-LBA device) unless the device reports a larger //! unit it writes whole across a power loss. @@ -96,6 +99,20 @@ impl Model { self.state.borrow().unflushed.len() } + /// The newest write completes forced: it is durable, and every other + /// unflushed write stays as it was. + /// + /// # Panics + /// With no unflushed write, or where the newest overlaps another: the + /// cache would hold an older copy of blocks the medium has newer. + pub fn forced(&self) { + let mut state = self.state.borrow_mut(); + let w = state.unflushed.pop().expect("a forced write completes after it was written"); + let overlapped = state.unflushed.iter().any(|o| o.at < w.at + w.data.len() && w.at < o.at + o.data.len()); + assert!(!overlapped, "a forced write at byte {} over a write no flush has covered", w.at); + state.durable[w.at..w.at + w.data.len()].copy_from_slice(&w.data); + } + /// `judge` asked of every medium a crash now could leave, or of `budget` /// of them drawn from `seed` when there are more; the first it refuses. /// Each medium is built in place over the durable bytes, so the judge @@ -212,6 +229,12 @@ impl BlockDev for Model { } Ok(()) } + + async fn write_durable(&self, lba: u64, data: &[Block]) -> Result<(), Error> { + self.write(lba, data).await?; + self.forced(); + Ok(()) + } } /// A device over a medium [`Model::check`] judges, which leaves it as it is: @@ -323,6 +346,39 @@ mod tests { assert_eq!(search, Search::Every(1 << 8), "the marker's eight units, each kept or not"); } + /// A record written forced is durable with no flush, so no crash keeps + /// the marker without it; and it made nothing else durable: a write + /// before it is still torn. + #[test] + fn a_forced_record_is_whole_under_its_marker_and_covers_no_other_write() { + let dev = Model::new(vec![0; 4 * BLOCK], 512); + block_on_ready(async { + dev.write(3, &[[0x33; BLOCK]]).await.unwrap(); + dev.write_durable(RECORD, &[[0xAA; BLOCK]]).await.unwrap(); + dev.write(MARKER, &[[0x11; BLOCK]]).await.unwrap(); + }); + assert_eq!(dev.unflushed(), 2); + let mut earlier_lost = false; + let search = dev + .check(1 << 20, SEED, |image| { + earlier_lost |= image[3 * BLOCK] == 0; + committed_is_whole(image) + }) + .unwrap_or_else(|v| panic!("{v}")); + assert_eq!(search, Search::Every(1 << 16), "the marker's and the earlier write's units"); + assert!(earlier_lost, "a forced write made an earlier write durable"); + } + + #[test] + #[should_panic(expected = "over a write no flush has covered")] + fn a_forced_write_over_an_unflushed_one_is_refused() { + let dev = Model::new(vec![0; BLOCK], BLOCK); + block_on_ready(async { + dev.write(0, &[[1; BLOCK]]).await.unwrap(); + dev.write_durable(0, &[[2; BLOCK]]).await.unwrap(); + }); + } + /// One block written at one LBA on a device with 512-byte units: a crash /// keeps any subset of its eight sectors, torn ones included. On a device /// that keeps the whole block across a power loss, only none or all. diff --git a/lib/blockring/Cargo.toml b/lib/blockring/Cargo.toml index 47a9b5efbdb..1eb4895c8e6 100644 --- a/lib/blockring/Cargo.toml +++ b/lib/blockring/Cargo.toml @@ -33,6 +33,10 @@ mutate-abort-keeps-inflight = [] # re-issues nothing: writes it had acknowledged are gone and a later flush # still says durable. mutate-no-reissue-after-loss = [] +# `Client` puts a write asked with `fua` on the wire without it, and still +# keeps nothing of it: the device holds it in its cache, and it was called +# durable. +mutate-fua-goes-out-unforced = [] [dependencies] # Whose flush answers for which writes the disk lost, and who holds which span: diff --git a/lib/blockring/src/client.rs b/lib/blockring/src/client.rs index bf8c5012c79..a749500d10c 100644 --- a/lib/blockring/src/client.rs +++ b/lib/blockring/src/client.rs @@ -25,6 +25,12 @@ //! is answered [`Outcome::Durable`] only by a device flush that succeeded //! after every loss it could have been told about. //! +//! **A write with `fua` is on the medium once acknowledged**, so it is not +//! kept and never issued again: [`Outcome::Done`] for it is durable. The +//! caller asks one only of a session that offers it +//! ([`crate::wire::Opened::fua`]), and never over a write of its own that is +//! kept: issued again after a loss, the kept write would replace it. +//! //! **What it keeps is bounded.** Once the acknowledged writes it keeps reach //! the limit it was made with, the caller's next write waits while the client //! asks a flush of its own. If that flush fails past [`MAX_ATTEMPTS`], writes @@ -60,7 +66,7 @@ use crate::layout::DEPTH; #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum Ask

{ Read { lba: u64, blocks: u32 }, - Write { lba: u64, blocks: u32, payload: P }, + Write { lba: u64, blocks: u32, payload: P, fua: bool }, Flush, } @@ -85,7 +91,7 @@ pub const MAX_ATTEMPTS: u32 = 4; #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum Outcome { /// A read's data is in its arena blocks; a write was taken by the device, - /// which is not durable yet. + /// which is not durable yet unless it was asked with `fua`. Done, /// A flush: every write acknowledged before it was asked for is on the /// medium. @@ -139,13 +145,13 @@ struct Queued

{ impl

Queued

{ fn reissue(acked: Acked

) -> Self { let Acked { lba, blocks, payload, first, attempts, .. } = acked; - Self { ask: Ask::Write { lba, blocks, payload }, kind: Kind::Reissue { first, attempts } } + Self { ask: Ask::Write { lba, blocks, payload, fua: false }, kind: Kind::Reissue { first, attempts } } } /// The write this reissue is, acknowledged again at `seq`. fn acked(self, seq: u64) -> Acked

{ match (self.ask, self.kind) { - (Ask::Write { lba, blocks, payload }, Kind::Reissue { first, attempts }) => { + (Ask::Write { lba, blocks, payload, .. }, Kind::Reissue { first, attempts }) => { Acked { lba, blocks, payload, first, seq, attempts } } _ => unreachable!("blockring: only a reissue is acknowledged again"), @@ -270,14 +276,14 @@ impl Client { // or a later one of the caller's. (Kind::Reissue { .. }, &Ask::Write { lba, blocks, .. }) => { let blocked = self.wire.values().any(|sent| { - matches!(sent.op, Op::Write { run, lba: at } if at < lba + u64::from(blocks) && lba < at + u64::from(run.count())) + matches!(sent.op, Op::Write { run, lba: at, .. } if at < lba + u64::from(blocks) && lba < at + u64::from(run.count())) }); if blocked { return None; } } (_, _) if self.reissuing() => return None, - (Kind::User(ticket), Ask::Write { .. }) if self.kept >= self.limit => match self.pushback { + (Kind::User(ticket), Ask::Write { fua: false, .. }) if self.kept >= self.limit => match self.pushback { Pushback::Failed => { self.outbox.pop_front(); self.answers.push_back((ticket, Outcome::Device)); @@ -305,7 +311,10 @@ impl Client { let Kind::User(ticket) = front.kind else { unreachable!("blockring: a read is the caller's") }; Op::Read { run: place(Placing::Read { ticket, blocks })?, lba } } - Ask::Write { lba, blocks, payload } => Op::Write { run: place(Placing::Write { payload, blocks: *blocks })?, lba: *lba }, + Ask::Write { lba, blocks, payload, fua } => { + let fua = *fua && cfg!(not(feature = "mutate-fua-goes-out-unforced")); + Op::Write { run: place(Placing::Write { payload, blocks: *blocks })?, lba: *lba, fua } + } }; let queued = self.outbox.pop_front().expect("just seen"); if matches!(queued.kind, Kind::Reissue { .. }) { @@ -326,7 +335,9 @@ impl Client { match (queued.kind, completion.status) { (Kind::User(ticket), Status::Ok) => { match (queued.ask, op) { - (Ask::Write { lba, blocks, payload }, Op::Write { .. }) => { + // On the medium: no loss takes it, and nothing is kept. + (Ask::Write { fua: true, .. }, Op::Write { .. }) => {} + (Ask::Write { lba, blocks, payload, fua: false }, Op::Write { .. }) => { let seq = self.bump(); self.kept += u64::from(blocks); let acked = Acked { lba, blocks, payload, first: seq, seq, attempts: 0 }; @@ -594,7 +605,7 @@ mod tests { } fn write(ticket: Ticket, lba: u64, blocks: u32) -> Ask { - Ask::Write { lba, blocks, payload: ticket } + Ask::Write { lba, blocks, payload: ticket, fua: false } } fn up(limit: u64) -> C { @@ -628,7 +639,7 @@ mod tests { let mut client = up(ROOMY); client.submit(1, write(1, 5, 2)); let w = next(&mut client).unwrap(); - assert_eq!(w.op, Op::Write { run: run_of(1, 2), lba: 5 }); + assert_eq!(w.op, Op::Write { run: run_of(1, 2), lba: 5, fua: false }); answer(&mut client, w, Status::Ok); assert_eq!(client.take_released().collect::>(), [run_of(1, 2)]); assert_eq!(client.kept(), 2, "acknowledged, not yet covered"); @@ -648,7 +659,26 @@ mod tests { assert_eq!(client.next_request(|_| None), None); assert_eq!(client.take_answers().count(), 0); let w = next(&mut client).unwrap(); - assert_eq!(w.op, Op::Write { run: run_of(1, 1), lba: 0 }); + assert_eq!(w.op, Op::Write { run: run_of(1, 1), lba: 0, fua: false }); + } + + /// A write with `fua` goes out saying so, past the limit a kept write + /// would wait at, and is not kept once acknowledged: the session's end + /// after it issues nothing again. + #[test] + fn a_forced_write_is_not_kept_and_a_loss_issues_nothing_again() { + let mut client = up(0); + client.submit(1, Ask::Write { lba: 5, blocks: 2, payload: 1, fua: true }); + let w = next(&mut client).unwrap(); + assert_eq!(w.op, Op::Write { run: run_of(1, 2), lba: 5, fua: true }); + answer(&mut client, w, Status::Ok); + assert_eq!(client.take_answers().collect::>(), [(1, Outcome::Done)]); + assert_eq!(client.kept(), 0); + assert!(client.quiet()); + client.session_ended(); + client.session_started(); + assert_eq!(next(&mut client), None); + assert_eq!(client.reissued(), 0); } /// A write acknowledged, then a flush answered `Lost`: the write goes out @@ -673,7 +703,7 @@ mod tests { }) .unwrap(); assert_eq!(placed, [(1, 2)]); - assert_eq!(again.op, Op::Write { run: run_of(1, 2), lba: 5 }); + assert_eq!(again.op, Op::Write { run: run_of(1, 2), lba: 5, fua: false }); assert_eq!(next(&mut client), None, "the flush waits for the write"); answer(&mut client, again, Status::Ok); let f2 = next(&mut client).unwrap(); @@ -699,11 +729,11 @@ mod tests { client.session_ended(); client.session_started(); let first = next(&mut client).unwrap(); - assert_eq!(first.op, Op::Write { run: run_of(1, 2), lba: 0 }); + assert_eq!(first.op, Op::Write { run: run_of(1, 2), lba: 0, fua: false }); assert_eq!(next(&mut client), None, "the overlapping one waits"); answer(&mut client, first, Status::Ok); let second = next(&mut client).unwrap(); - assert_eq!(second.op, Op::Write { run: run_of(2, 1), lba: 1 }); + assert_eq!(second.op, Op::Write { run: run_of(2, 1), lba: 1, fua: false }); } /// A write the device refused on every reissue is gone, and its caller was @@ -719,7 +749,7 @@ mod tests { for _ in 0..=MAX_ATTEMPTS { client.session_started(); let again = next(&mut client).unwrap(); - assert_eq!(again.op, Op::Write { run: run_of(1, 1), lba: 0 }); + assert_eq!(again.op, Op::Write { run: run_of(1, 1), lba: 0, fua: false }); answer(&mut client, again, Status::Device); } assert_eq!(client.kept(), 0); @@ -804,7 +834,7 @@ mod tests { assert_eq!(next(&mut client), None, "the write goes out, or the client asks its own flush"); answer(&mut client, f, Status::Ok); let w = next(&mut client).unwrap(); - assert_eq!(w.op, Op::Write { run: run_of(3, 1), lba: 4 }); + assert_eq!(w.op, Op::Write { run: run_of(3, 1), lba: 4, fua: false }); } /// The client's own flush, refused past `MAX_ATTEMPTS`, refuses a write @@ -822,7 +852,7 @@ mod tests { assert_eq!(f.op, Op::Flush); answer(&mut client, f, Status::Device); if let Some(again) = next(&mut client) { - assert_eq!(again.op, Op::Write { run: run_of(1, 2), lba: 0 }, "the kept write goes again"); + assert_eq!(again.op, Op::Write { run: run_of(1, 2), lba: 0, fua: false }, "the kept write goes again"); answer(&mut client, again, Status::Ok); } } @@ -834,6 +864,6 @@ mod tests { answer(&mut client, f, Status::Ok); client.submit(4, write(4, 8, 1)); let w = next(&mut client).unwrap(); - assert_eq!(w.op, Op::Write { run: run_of(4, 1), lba: 8 }); + assert_eq!(w.op, Op::Write { run: run_of(4, 1), lba: 8, fua: false }); } } diff --git a/lib/blockring/src/entry.rs b/lib/blockring/src/entry.rs index 67d29627b45..ba4487c1011 100644 --- a/lib/blockring/src/entry.rs +++ b/lib/blockring/src/entry.rs @@ -16,8 +16,10 @@ use crate::layout::{ARENA, CQE_WORDS, MAX_REQUEST_BLOCKS, SQE_WORDS}; pub enum Op { /// The run's blocks from the partition's block `lba` into the arena. Read { run: Run, lba: u64 }, - /// The run's blocks from the arena to the partition's block `lba`. - Write { run: Run, lba: u64 }, + /// The run's blocks from the arena to the partition's block `lba`. With + /// `fua` they are on the medium before it is answered, which only a + /// session opened saying so takes ([`crate::wire::Opened::fua`]). + Write { run: Run, lba: u64, fua: bool }, /// Every write acknowledged before this was submitted, onto the medium. Flush, } @@ -26,6 +28,9 @@ const READ: u32 = 1; const WRITE: u32 = 2; const FLUSH: u32 = 3; +/// A write's flags word: bit 0 is `fua`, and no other is defined. +const FUA: u32 = 1; + /// One request. #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub struct Request { @@ -45,29 +50,31 @@ pub enum Refused { impl Request { pub fn encode(&self) -> [u32; SQE_WORDS] { - let (op, lba, first, count) = match self.op { - Op::Read { run, lba } => (READ, lba, run.first(), run.count()), - Op::Write { run, lba } => (WRITE, lba, run.first(), run.count()), - Op::Flush => (FLUSH, 0, 0, 0), + let (op, lba, first, count, flags) = match self.op { + Op::Read { run, lba } => (READ, lba, run.first(), run.count(), 0), + Op::Write { run, lba, fua } => (WRITE, lba, run.first(), run.count(), if fua { FUA } else { 0 }), + Op::Flush => (FLUSH, 0, 0, 0, 0), }; - [op, self.tag, lba as u32, (lba >> 32) as u32, count, first, 0, 0] + [op, self.tag, lba as u32, (lba >> 32) as u32, count, first, flags, 0] } /// The request these words are, bounded against a partition of /// `partition_blocks`: every block it names is inside both the arena and /// the partition, a transfer moves at least one block and at most - /// [`MAX_REQUEST_BLOCKS`], and a flush names nothing. + /// [`MAX_REQUEST_BLOCKS`], only a write carries flags, and a flush names + /// nothing. pub fn decode(words: [u32; SQE_WORDS], partition_blocks: u64) -> Result { - let [op, tag, lba_low, lba_high, blocks, arena, reserved @ ..] = words; + let [op, tag, lba_low, lba_high, blocks, arena, flags, reserved] = words; let refused = Refused::Malformed { tag }; - if reserved.iter().any(|&word| word != 0) { + if reserved != 0 { return Err(refused); } let lba = u64::from(lba_low) | (u64::from(lba_high) << 32); - let transfer: fn(Run, u64) -> Op = match op { - READ => |run, lba| Op::Read { run, lba }, - WRITE => |run, lba| Op::Write { run, lba }, - FLUSH if lba == 0 && blocks == 0 && arena == 0 => return Ok(Self { op: Op::Flush, tag }), + let transfer: fn(Run, u64) -> Op = match (op, flags) { + (READ, 0) => |run, lba| Op::Read { run, lba }, + (WRITE, 0) => |run, lba| Op::Write { run, lba, fua: false }, + (WRITE, FUA) => |run, lba| Op::Write { run, lba, fua: true }, + (FLUSH, 0) if lba == 0 && blocks == 0 && arena == 0 => return Ok(Self { op: Op::Flush, tag }), _ => return Err(refused), }; let run = Run::decode(arena, blocks, &ARENA).map_err(|_| refused)?; @@ -83,7 +90,7 @@ impl Request { #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum Status { /// Done. For a flush, every write of its writer's acknowledged before it - /// was submitted is on the medium. + /// was submitted is on the medium; for a write with `fua`, that write is. Ok, /// Refused unread: the request was malformed ([`Refused`]). Invalid, @@ -151,7 +158,8 @@ mod tests { let last = ARENA.slots() - MAX_REQUEST_BLOCKS; for request in [ Request { op: Op::Read { run: run(0, 1), lba: 999 }, tag: 7 }, - Request { op: Op::Write { run: run(last, MAX_REQUEST_BLOCKS), lba: 0 }, tag: u32::MAX }, + Request { op: Op::Write { run: run(last, MAX_REQUEST_BLOCKS), lba: 0, fua: false }, tag: u32::MAX }, + Request { op: Op::Write { run: run(2, 3), lba: 5, fua: true }, tag: 3 }, Request { op: Op::Flush, tag: 0 }, ] { assert_eq!(Request::decode(request.encode(), PARTITION), Ok(request)); @@ -164,14 +172,19 @@ mod tests { /// refusal still carries its tag. #[test] fn a_request_outside_its_bounds_is_refused_by_tag() { - let write = |lba| Request { op: Op::Write { run: run(5, 2), lba }, tag: 42 }; + let write = |lba| Request { op: Op::Write { run: run(5, 2), lba, fua: false }, tag: 42 }; + let read = Request { op: Op::Read { run: run(5, 2), lba: 10 }, tag: 42 }; let good = write(10); let with = |at: usize, word: u32| { let mut words = good.encode(); words[at] = word; words }; - let cases: [(&str, [u32; SQE_WORDS]); 10] = [ + let mut forced_read = read.encode(); + forced_read[6] = 1; + let mut forced_flush = Request { op: Op::Flush, tag: 42 }.encode(); + forced_flush[6] = 1; + let cases: [(&str, [u32; SQE_WORDS]); 13] = [ ("op 0", with(0, 0)), ("op 4", with(0, 4)), ("no blocks", with(4, 0)), @@ -182,6 +195,9 @@ mod tests { ("lba wraps", write(u64::MAX).encode()), ("reserved word", with(7, 1)), ("a flush with a range", with(0, 3)), + ("a flag no write defines", with(6, 2)), + ("a read with a write's flag", forced_read), + ("a flush with a write's flag", forced_flush), ]; for (what, words) in cases { assert_eq!( diff --git a/lib/blockring/src/model.rs b/lib/blockring/src/model.rs index 0709f14f4e6..71d0396e49d 100644 --- a/lib/blockring/src/model.rs +++ b/lib/blockring/src/model.rs @@ -1,11 +1,12 @@ //! Every ordering of a client, a server, a device and their failures. //! -//! A scripted caller asks for writes and flushes; the rings between the client -//! and the server are queues, and rings of the transport's own are driven -//! beside them and held to them both ways ([`Queues`]); the server is [`ServerSession`] -//! over a device of two blocks with a volatile cache. [`explore`] runs, depth -//! first and exhaustively, every interleaving of: the caller asking for its -//! next step, the server taking a request, the device completing any one it +//! A scripted caller asks for writes, forced writes and flushes; the rings +//! between the client and the server are queues, and rings of the transport's +//! own are driven beside them and held to them both ways ([`Queues`]); the +//! server is [`ServerSession`] over a device of two blocks with a volatile +//! cache. [`explore`] runs, depth first and exhaustively, every interleaving +//! of: the caller asking for its next step, the server taking a request, the +//! device completing any one it //! holds, **the device failing any one it holds** (not done, answered so), the //! client reading a completion, **the device being reset** under whatever is in //! flight (each command dropped, or run before the stop with its completion @@ -24,7 +25,9 @@ //! write acknowledged before the flush was asked for, or one asked for after //! it; and a flush ends answered durable unless the run spent more failures //! than [`MAX_ATTEMPTS`], which is the fewest that can make the client give -//! a write or a flush up ([`Law::Durable`]). +//! a write or a flush up ([`Law::Durable`]); +//! - a forced write answered done is on the medium from then on, until a +//! write asked for after it replaces it ([`Law::Forced`]). use alloc::collections::{BTreeMap, BTreeSet, VecDeque}; use alloc::format; @@ -80,6 +83,8 @@ pub struct Explored { #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)] pub enum Step { Write { block: u64, value: u8 }, + /// A write with `fua`. + Forced { block: u64, value: u8 }, Flush, } @@ -90,6 +95,7 @@ pub enum Law { Durable, /// A caller's write first goes out while the client keeps its limit. Kept, + Forced, } /// One word of the session page. The model runs on one thread, so every order @@ -297,7 +303,7 @@ fn start(failures: Failures, limit: u64) -> World { fn connect(world: &mut World) -> Result<(), (Law, String)> { let mut holds = Holds::new(); holds.hold(0, BLOCKS as u64, 1).expect("a fresh server holds nothing"); - world.server = Some(Server { session: ServerSession::new(0, BLOCKS as u64, true), holds }); + world.server = Some(Server { session: ServerSession::new(0, BLOCKS as u64, true, true), holds }); world.alive = true; world.losses = 0; world.client.session_started(); @@ -316,7 +322,7 @@ fn pump(world: &mut World) -> Result<(), (Law, String)> { loop { let kept = world.client.kept(); let Some(request) = world.client.next_request(place) else { return Ok(()) }; - if let Op::Write { run, .. } = request.op { + if let Op::Write { run, fua: false, .. } = request.op { let ticket = Ticket::from(run.first()); if world.sent.insert(ticket) && kept >= world.limit { return Err((Law::Kept, format!("write {ticket} went out with {kept} blocks kept"))); @@ -326,21 +332,26 @@ fn pump(world: &mut World) -> Result<(), (Law, String)> { } } -/// A write step's block and value; a write's arena block is its ticket. +/// A write step's block and value, forced or not; a write's arena block is +/// its ticket. fn write_of(script: &[Step], ticket: Ticket) -> Option<(u64, u8)> { match script[ticket as usize] { - Step::Write { block, value } => Some((block, value)), + Step::Write { block, value } | Step::Forced { block, value } => Some((block, value)), Step::Flush => None, } } -/// The device carries out `request`: a write lands in the cache, a flush moves -/// the cache onto the medium. +/// The device carries out `request`: a write lands in the cache, a write with +/// `fua` on the medium in place of what the cache held of its block, and a +/// flush moves the cache onto the medium. fn apply(script: &[Step], cache: &mut [Option; BLOCKS], media: &mut [u8; BLOCKS], request: Request) { match request.op { - Op::Write { run, lba } => { + Op::Write { run, lba, fua } => { let (_, value) = write_of(script, u64::from(run.first())).expect("a write's arena block is its ticket"); - cache[lba as usize] = Some(value); + cache[lba as usize] = (!fua).then_some(value); + if fua { + media[lba as usize] = value; + } } Op::Flush => { for (b, slot) in cache.iter_mut().enumerate() { @@ -408,9 +419,31 @@ fn settle(script: &[Step], world: &mut World) -> Result<(), (Law, String)> { durable(script, world, ticket).map_err(|why| (Law::Durable, why))?; } } + forced(script, world).map_err(|why| (Law::Forced, why))?; pump(world) } +/// Every forced write answered done is on the medium, or a write asked for +/// after it is. +fn forced(script: &[Step], world: &World) -> Result<(), String> { + for (&ticket, answers) in &world.answers { + let Step::Forced { block, value } = script[ticket as usize] else { continue }; + if answers.as_slice() != [Outcome::Done] { + continue; + } + let later = (ticket + 1..script.len() as u64).filter_map(|t| write_of(script, t)).filter(|&(b, _)| b == block); + let allowed: Vec = core::iter::once(value).chain(later.map(|(_, v)| v)).collect(); + let on = world.media[block as usize]; + if !allowed.contains(&on) { + return Err(format!( + "forced write {ticket} was answered done with block {block} holding {on}, not its {value} \ + (or a later one of {allowed:?})" + )); + } + } + Ok(()) +} + /// What the flush `ticket`, just answered durable, promised is on the medium. fn durable(script: &[Step], world: &World, flush: Ticket) -> Result<(), String> { let before = &world.acked_before[&flush]; @@ -432,6 +465,15 @@ fn durable(script: &[Step], world: &World, flush: Ticket) -> Result<(), String> Ok(()) } +/// Whether the write `ticket` was answered done before a flush that was then +/// answered durable was asked for: the client keeps it no longer. +fn covered(script: &[Step], world: &World, ticket: Ticket) -> bool { + (ticket + 1..script.len() as u64).any(|flush| { + world.acked_before.get(&flush).is_some_and(|acked| acked.contains(&ticket)) + && world.answers.get(&flush).map(Vec::as_slice) == Some(&[Outcome::Durable]) + }) +} + /// Nothing more can happen: every ticket was answered, and every flush said /// durable — or, past [`MAX_ATTEMPTS`] failures, said the device refused it. fn end(run: &mut Run, world: &World) { @@ -533,25 +575,45 @@ fn next(script: &[Step], world: &World) -> Vec<(String, After)> { }; // The caller asks for its next step, never a write to a block with a write - // still unanswered: two writes in flight to one block are unordered. + // still unanswered: two writes in flight to one block are unordered. Nor + // a forced write to a block with a write no flush has covered, so the + // flush before a forced write waits for the writes it is to cover. if world.next < script.len() { let ticket = world.next as Ticket; - let busy = write_of(script, ticket).is_some_and(|(block, _)| { + let unanswered = |block: u64| { (0..ticket).any(|t| { write_of(script, t).is_some_and(|(b, _)| b == block) && !world.answers.contains_key(&t) }) - }); + }; + let busy = match (script[world.next], script.get(world.next + 1)) { + (Step::Write { block, .. }, _) => unanswered(block), + (Step::Forced { block, .. }, _) => { + unanswered(block) + || (0..ticket).any(|t| { + matches!(script[t as usize], Step::Write { block: b, .. } if b == block) + && world.answers[&t].as_slice() == [Outcome::Done] + && !covered(script, world, t) + }) + } + (Step::Flush, Some(&Step::Forced { block, .. })) => unanswered(block), + (Step::Flush, _) => false, + }; if !busy { let mut w = world.clone(); match script[world.next] { Step::Write { block, .. } => { - w.client.submit(ticket, Ask::Write { lba: block, blocks: 1, payload: ticket }); + w.client.submit(ticket, Ask::Write { lba: block, blocks: 1, payload: ticket, fua: false }); + } + Step::Forced { block, .. } => { + w.client.submit(ticket, Ask::Write { lba: block, blocks: 1, payload: ticket, fua: true }); } Step::Flush => { let acked = w .answers .iter() - .filter(|(t, a)| write_of(script, **t).is_some() && a.as_slice() == [Outcome::Done]) + .filter(|(t, a)| { + matches!(script[**t as usize], Step::Write { .. }) && a.as_slice() == [Outcome::Done] + }) .map(|(t, _)| *t) .collect(); w.acked_before.insert(ticket, acked); @@ -729,10 +791,21 @@ mod tests { at_most(1, 1, MAX_ATTEMPTS as u8), ]; + /// A forced write over a write a flush has covered, beside a write kept + /// through it: a loss after it issues the kept write again and the + /// covered one never. + const FORCED: [Step; 5] = [ + Step::Write { block: 0, value: 1 }, + Step::Flush, + Step::Forced { block: 0, value: 2 }, + Step::Write { block: 1, value: 3 }, + Step::Flush, + ]; + fn every_bound() -> impl Iterator { BOUNDS .into_iter() - .map(|f| (&SCRIPT[..], f)) + .flat_map(|f| [(&SCRIPT[..], f), (&FORCED[..], f)]) .chain([(&ONE_BLOCK[..], ONE_BLOCK_BOUND)]) .chain(GIVE_UP_BOUNDS.into_iter().map(|f| (&GIVE_UP[..], f))) } @@ -773,6 +846,20 @@ mod tests { } } + /// A forced write answered done is on the medium whatever is lost after + /// it, with no flush asked for it. + #[test] + fn a_forced_write_answered_done_stays_on_the_medium() { + for (script, failures) in every_bound() { + if let Some((Law::Forced, why)) = verdict(script, failures).broken { + panic!("{failures:?}: {why}"); + } + } + let explored = verdict(&FORCED, at_most(0, 0, 0)); + assert_eq!(explored.broken, None); + assert!(explored.ends >= 1, "no run asked the forced write"); + } + /// No caller's write goes out while the client keeps its limit: it waits /// for the client's own flush, under every failure the bounds allow. #[test] diff --git a/lib/blockring/src/server.rs b/lib/blockring/src/server.rs index c2745355485..dc47aae8ccc 100644 --- a/lib/blockring/src/server.rs +++ b/lib/blockring/src/server.rs @@ -13,7 +13,8 @@ //! the device's loss count, and a flush's completion under the same count //! settles every writer and fails for this one if the disk lost writes of its //! own since it last heard. The loss count is the device's, bumped by whoever -//! resets it. +//! resets it. A write with `fua` is on the medium when the device answers it, +//! so no loss can take it and it is recorded under none. use alloc::vec::Vec; @@ -33,6 +34,9 @@ pub struct ServerSession { /// Its grant lets it write; a write on a session whose grant does not is /// answered `ReadOnly` and never reaches the device. writes: bool, + /// Its device forces a write to the medium; a write with `fua` on a + /// session whose device does not is answered `Invalid`. + fua: bool, /// Each request in flight, in the order it was taken: a tag is only ever /// compared for equality. inflight: Vec<(u32, Op)>, @@ -49,9 +53,10 @@ pub enum Taken { impl ServerSession { /// A session over the partition at device block `first`, `blocks` long, - /// which `holds` already holds for it, taking writes if `writes`. - pub fn new(first: u64, blocks: u64, writes: bool) -> Self { - Self { blocks, first, writes, inflight: Vec::new() } + /// which `holds` already holds for it, taking writes if `writes` and + /// writes with `fua` if `fua`, as its open was answered. + pub fn new(first: u64, blocks: u64, writes: bool, fua: bool) -> Self { + Self { blocks, first, writes, fua, inflight: Vec::new() } } /// The writer this session's writes and flushes are accounted to. @@ -76,10 +81,12 @@ impl ServerSession { /// Decide what one entry the client published is. /// - /// A malformed entry, a tag already in flight, and a write its grant does - /// not let it make, are answered at once and never reach the device: the - /// second would make one tag two requests, and the client could not tell - /// which answer was whose. + /// A malformed entry, a tag already in flight, a write its grant does not + /// let it make, and a write with `fua` its open did not offer, are + /// answered at once and never reach the device: the second would make one + /// tag two requests, and the client could not tell which answer was + /// whose; the last would be answered as durable by a device that cannot + /// say so. pub fn take(&mut self, words: [u32; SQE_WORDS]) -> Taken { let request = match Request::decode(words, self.blocks) { Ok(request) => request, @@ -93,6 +100,9 @@ impl ServerSession { if !self.writes && matches!(request.op, Op::Write { .. }) { return Taken::Answer(Completion { tag: request.tag, status: Status::ReadOnly }); } + if !self.fua && matches!(request.op, Op::Write { fua: true, .. }) { + return Taken::Answer(Completion { tag: request.tag, status: Status::Invalid }); + } self.inflight.push((request.tag, request.op)); Taken::Issue(request) } @@ -112,7 +122,8 @@ impl ServerSession { let status = match (op, done) { (_, false) => Status::Device, (Op::Read { .. }, true) => Status::Ok, - (Op::Write { .. }, true) => { + (Op::Write { fua: true, .. }, true) => Status::Ok, + (Op::Write { fua: false, .. }, true) => { holds.wrote(self.writer(), losses); Status::Ok } @@ -141,7 +152,10 @@ mod tests { use crate::layout::ARENA; fn write(tag: u32) -> [u32; SQE_WORDS] { - Request { op: Op::Write { run: ARENA.run(0, 1).unwrap(), lba: 0 }, tag }.encode() + Request { op: Op::Write { run: ARENA.run(0, 1).unwrap(), lba: 0, fua: false }, tag }.encode() + } + fn forced(tag: u32) -> [u32; SQE_WORDS] { + Request { op: Op::Write { run: ARENA.run(0, 1).unwrap(), lba: 0, fua: true }, tag }.encode() } fn flush(tag: u32) -> [u32; SQE_WORDS] { Request { op: Op::Flush, tag }.encode() @@ -151,7 +165,7 @@ mod tests { fn a_reset_answers_once_and_the_late_device_answer_is_dropped() { let mut holds: Holds = Holds::new(); holds.hold(10, 20, 1).unwrap(); - let mut session = ServerSession::new(10, 10, true); + let mut session = ServerSession::new(10, 10, true, true); assert!(matches!(session.take(write(1)), Taken::Issue(_))); assert_eq!(session.abort_all(), [Completion { tag: 1, status: Status::Device }]); assert_eq!(session.complete(1, true, &mut holds, 1), None); @@ -161,7 +175,7 @@ mod tests { /// tags' values: nothing orders a tag but its arrival. #[test] fn a_reset_answers_in_the_order_taken() { - let mut session = ServerSession::new(0, 10, true); + let mut session = ServerSession::new(0, 10, true, true); for tag in [9, 4] { assert!(matches!(session.take(write(tag)), Taken::Issue(_))); } @@ -171,7 +185,7 @@ mod tests { #[test] fn a_tag_in_flight_twice_is_refused_unissued() { - let mut session = ServerSession::new(0, 10, true); + let mut session = ServerSession::new(0, 10, true, true); assert!(matches!(session.take(write(4)), Taken::Issue(_))); assert_eq!(session.take(write(4)), Taken::Answer(Completion { tag: 4, status: Status::Invalid })); assert_eq!(session.inflight().len(), 1); @@ -183,7 +197,7 @@ mod tests { fn a_flush_after_a_loss_answers_lost_once() { let mut holds: Holds = Holds::new(); holds.hold(0, 10, 1).unwrap(); - let mut session = ServerSession::new(0, 10, true); + let mut session = ServerSession::new(0, 10, true, true); session.take(write(1)); assert_eq!(session.complete(1, true, &mut holds, 0).unwrap().status, Status::Ok); session.take(flush(2)); @@ -192,11 +206,35 @@ mod tests { assert_eq!(session.complete(3, true, &mut holds, 1).unwrap().status, Status::Ok); } + /// A write with `fua` acknowledged before a reset is on the medium: the + /// flush after the reset has no loss of it to report. Without `fua` the + /// same write is the loss the test above reports. + #[test] + fn a_forced_write_is_no_loss_to_the_flush_after_a_reset() { + let mut holds: Holds = Holds::new(); + holds.hold(0, 10, 1).unwrap(); + let mut session = ServerSession::new(0, 10, true, true); + session.take(forced(1)); + assert_eq!(session.complete(1, true, &mut holds, 0).unwrap().status, Status::Ok); + session.take(flush(2)); + assert_eq!(session.complete(2, true, &mut holds, 1).unwrap().status, Status::Ok); + } + + /// A session whose device forces no write refuses one with `fua` + /// unissued, and still takes a plain write. + #[test] + fn a_forced_write_on_a_session_that_offered_none_is_refused_unissued() { + let mut session = ServerSession::new(0, 10, true, false); + assert_eq!(session.take(forced(1)), Taken::Answer(Completion { tag: 1, status: Status::Invalid })); + assert_eq!(session.inflight().len(), 0); + assert!(matches!(session.take(write(2)), Taken::Issue(_))); + } + /// A session whose grant does not write answers a write `ReadOnly` and /// holds nothing for the device, and still reads and flushes. #[test] fn a_read_only_session_refuses_a_write_unissued() { - let mut session = ServerSession::new(0, 10, false); + let mut session = ServerSession::new(0, 10, false, true); assert_eq!(session.take(write(1)), Taken::Answer(Completion { tag: 1, status: Status::ReadOnly })); assert_eq!(session.inflight().len(), 0); let read = Request { op: Op::Read { run: ARENA.run(0, 1).unwrap(), lba: 0 }, tag: 2 }; diff --git a/lib/blockring/src/wire.rs b/lib/blockring/src/wire.rs index 1209e973416..5f363d5c150 100644 --- a/lib/blockring/src/wire.rs +++ b/lib/blockring/src/wire.rs @@ -76,8 +76,10 @@ pub const DRIVER_BOUND_NS: u64 = 30_000_000_000; /// long again for the driver's answer to an open it hands over. pub const ANSWER_BOUND_NS: u64 = 2 * DRIVER_BOUND_NS; -/// What an open was answered with: the partition's length in blocks, and its -/// unique GUID as the table stores it. +/// What an open was answered with: the partition's length in blocks, its +/// unique GUID as the table stores it, and whether its device forces a write +/// with `fua` onto the medium. A client of a session that does not offer it +/// writes and then flushes instead. /// /// A server has one from [`Opened::over`], with its ends of the page: /// @@ -85,28 +87,33 @@ pub const ANSWER_BOUND_NS: u64 = 2 * DRIVER_BOUND_NS; /// use core::sync::atomic::AtomicU32; /// use toyos_blockring::{layout::RING_WORDS, wire::Opened}; /// let page: [AtomicU32; RING_WORDS] = core::array::from_fn(|_| AtomicU32::new(0)); -/// let (_ends, _answer) = Opened::over(&page, 1, [0; 16]); +/// let (_ends, _answer) = Opened::over(&page, 1, [0; 16], false); /// ``` /// /// and never without them: /// /// ```compile_fail -/// let _answer = toyos_blockring::wire::Opened { blocks: 1, unique: [0; 16] }; +/// let _answer = toyos_blockring::wire::Opened { blocks: 1, unique: [0; 16], fua: false }; /// ``` #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub struct Opened { blocks: u64, unique: [u8; GUID_BYTES], + fua: bool, } impl Opened { - pub const BYTES: usize = 8 + GUID_BYTES; + pub const BYTES: usize = 8 + GUID_BYTES + 1; /// A server's ends of the session `page` it was sent, every word it owns /// set to 0, and the answer to send after: `blocks` of the partition - /// `unique`. - pub fn over(page: &[W; RING_WORDS], blocks: u64, unique: [u8; GUID_BYTES]) -> (ServerRings, Self) { - (layout::server(page), Self { blocks, unique }) + /// `unique`, on a device that forces a write if `fua`. + pub fn over(page: &[W; RING_WORDS], blocks: u64, unique: [u8; GUID_BYTES], fua: bool) -> (ServerRings, Self) { + (layout::server(page), Self { blocks, unique, fua }) + } + + pub fn fua(&self) -> bool { + self.fua } pub fn blocks(&self) -> u64 { @@ -120,17 +127,21 @@ impl Opened { pub fn encode(&self) -> [u8; Self::BYTES] { let mut out = [0u8; Self::BYTES]; out[..8].copy_from_slice(&self.blocks.to_le_bytes()); - out[8..].copy_from_slice(&self.unique); + out[8..8 + GUID_BYTES].copy_from_slice(&self.unique); + out[8 + GUID_BYTES] = u8::from(self.fua); out } pub fn decode(bytes: &[u8]) -> Option { - if bytes.len() != Self::BYTES { - return None; - } + let bytes: &[u8; Self::BYTES] = bytes.try_into().ok()?; let blocks = u64::from_le_bytes(bytes[..8].try_into().ok()?); - let unique = bytes[8..].try_into().ok()?; - Some(Self { blocks, unique }) + let unique = bytes[8..8 + GUID_BYTES].try_into().ok()?; + let fua = match bytes[8 + GUID_BYTES] { + 0 => false, + 1 => true, + _ => return None, + }; + Some(Self { blocks, unique, fua }) } } @@ -535,9 +546,14 @@ mod tests { let bytes: Vec = listed.iter().flat_map(|l| l.encode()).collect(); assert_eq!(Listed::decode_all(&bytes).map(|all| all.collect::>()), Some(listed.to_vec())); assert!(Listed::decode_all(&bytes[1..]).is_none()); - let opened = Opened { blocks: u64::MAX - 3, unique: [7; GUID_BYTES] }; - assert_eq!(Opened::decode(&opened.encode()), Some(opened)); - assert_eq!(Opened::decode(&opened.encode()[1..]), None); + for fua in [false, true] { + let opened = Opened { blocks: u64::MAX - 3, unique: [7; GUID_BYTES], fua }; + assert_eq!(Opened::decode(&opened.encode()), Some(opened)); + assert_eq!(Opened::decode(&opened.encode()[1..]), None); + let mut neither = opened.encode(); + neither[Opened::BYTES - 1] = 2; + assert_eq!(Opened::decode(&neither), None, "a flag that is neither"); + } for r in [ Refusal::NotFound, Refusal::Held, diff --git a/lib/blockserve/src/lib.rs b/lib/blockserve/src/lib.rs index 78477be870a..2f0d96a570f 100644 --- a/lib/blockserve/src/lib.rs +++ b/lib/blockserve/src/lib.rs @@ -14,7 +14,7 @@ pub mod region; pub mod sessions; pub mod table; -pub use sessions::{Device, Owner, Sessions}; +pub use sessions::{Device, Io, Owner, Sessions}; pub use table::Partition; /// A partition GUID as the lines say it. diff --git a/lib/blockserve/src/sessions.rs b/lib/blockserve/src/sessions.rs index 96f5df35acf..e50d0341e2c 100644 --- a/lib/blockserve/src/sessions.rs +++ b/lib/blockserve/src/sessions.rs @@ -51,13 +51,24 @@ pub struct Owner { pub write: bool, } +/// A transfer a disk is asked for. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum Io { + Read, + /// With `fua`, on the medium before the device answers it. + Write { fua: bool }, +} + /// What a disk's sessions ask of the controller it is on. pub trait Device { /// Whether one more command can be issued now. fn has_room(&self) -> bool; - /// Issue a read or a write of `blocks` blocks at the disk's block - /// `block`, to or from device address `at`, answered for `owner`. - fn submit_io(&mut self, write: bool, block: u64, blocks: u32, at: u64, owner: Owner); + /// Whether the disk forces a write onto its medium: a session is told at + /// its open, and only one told so is issued a write with `fua`. + fn fua(&self) -> bool; + /// Issue `io` of `blocks` blocks at the disk's block `block`, to or from + /// device address `at`, answered for `owner`. + fn submit_io(&mut self, io: Io, block: u64, blocks: u32, at: u64, owner: Owner); /// Issue a flush of the disk's volatile cache for `owner`; `false` for a /// disk with none, whose every write answered is on the medium already. fn submit_flush(&mut self, owner: Owner) -> bool; @@ -203,7 +214,7 @@ impl Sessions { self.holds.release(first); return refuse(&conn, Refusal::Exhausted); }; - let (rings, opened) = Opened::over(region.words(), blocks, unique); + let (rings, opened) = Opened::over(region.words(), blocks, unique, dev.fua()); if conn.try_send_bytes_with(wire::MSG_OPENED, &opened.encode(), [client_ring.into(), answered.into()]).is_err() { // The client went before it heard: nothing of the session reached // the device, and what was taken for it goes back. @@ -214,7 +225,7 @@ impl Sessions { } let id = self.next_id; self.next_id += 1; - let state = ServerSession::new(first, opened.blocks(), writes); + let state = ServerSession::new(first, opened.blocks(), writes, opened.fua()); let served = Served { heard, ring, region, device_addr, rings, state, unique, closing: false, requests: 0, posted: false }; self.sessions.insert(id, served); let access = if writes { "" } else { ", read-only" }; @@ -304,18 +315,19 @@ impl Sessions { Taken::Issue(req) => { let write = matches!(req.op, Op::Write { .. }); let owner = Owner { session: id, tag: req.tag, write }; - match req.op { - Op::Read { run, lba } | Op::Write { run, lba } => { - let at = s.device_addr + run.span().offset as u64; - dev.submit_io(write, s.state.first() + lba, run.count(), at, owner); - } + let (io, run, lba) = match req.op { + Op::Read { run, lba } => (Io::Read, run, lba), + Op::Write { run, lba, fua } => (Io::Write { fua }, run, lba), Op::Flush => { if !dev.submit_flush(owner) { let c = s.state.complete(req.tag, true, &mut self.holds, self.losses); Self::post(s, c.expect("just taken")); } + continue; } - } + }; + let at = s.device_addr + run.span().offset as u64; + dev.submit_io(io, s.state.first() + lba, run.count(), at, owner); } } } @@ -486,8 +498,10 @@ impl Sessions { let owner = |write| Owner { session: JOB, tag: 0, write }; match job.steps.next(&mut job.region, dev.has_room()) { Ask::Wait => return, - Ask::Read { block, count, at } => dev.submit_io(false, job.first + block, count, job.device_addr + at as u64, owner(false)), - Ask::Write { block, at } => dev.submit_io(true, job.first + block, 1, job.device_addr + at as u64, owner(true)), + Ask::Read { block, count, at } => dev.submit_io(Io::Read, job.first + block, count, job.device_addr + at as u64, owner(false)), + Ask::Write { block, at } => { + dev.submit_io(Io::Write { fua: false }, job.first + block, 1, job.device_addr + at as u64, owner(true)) + } Ask::Flush => { if !dev.submit_flush(owner(false)) { job.steps.answered(true); @@ -532,7 +546,10 @@ mod tests { fn has_room(&self) -> bool { unreachable!() } - fn submit_io(&mut self, _: bool, _: u64, _: u32, _: u64, _: Owner) { + fn fua(&self) -> bool { + false + } + fn submit_io(&mut self, _: Io, _: u64, _: u32, _: u64, _: Owner) { unreachable!() } fn submit_flush(&mut self, _: Owner) -> bool { diff --git a/lib/scsi/src/lib.rs b/lib/scsi/src/lib.rs index 75c734a58a3..96b85f5efe7 100644 --- a/lib/scsi/src/lib.rs +++ b/lib/scsi/src/lib.rs @@ -2,7 +2,7 @@ //! each answer means, and the bring-up between a configured interface and a //! disk with a size — as decisions with no transfer in them. //! -//! One logical unit, READ(10)/WRITE(10) only, no MODE SENSE. **Everything a +//! One logical unit, READ(10)/WRITE(10) only. **Everything a //! device answers is checked and never believed**, and a refusal is by name: a //! size this driver cannot address, a block size that does not divide the host //! block, a peripheral that is not a disk. @@ -24,6 +24,16 @@ pub const WRITE_10: u8 = 0x2A; /// INQUIRY's operation code (SPC-4 §6.4). pub const INQUIRY: u8 = 0x12; +/// A transfer between the host and the medium. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum Io { + Read, + /// With `fua`, WRITE(10)'s FUA bit (SBC-3 §5.32, byte 1 bit 3): the + /// device writes the blocks to its medium before it answers. Only a + /// device whose mode parameters say so takes it ([`dpofua`]). + Write { fua: bool }, +} + /// One command descriptor block, with the direction of the data it moves. /// /// Built only here, so no command can name a length a CBW cannot carry or a @@ -54,12 +64,21 @@ impl Cdb { /// (SBC-3 §5.24). pub const SYNCHRONIZE_CACHE: Self = Self::of([0x35, 0, 0, 0, 0, 0, 0, 0, 0, 0], false); + /// Every mode page (SPC-4, MODE SENSE(6): page code 3Fh), of which only + /// the mode parameter header is read. [`MODE_BYTES`] allocated, so a + /// device with pages to send has room to send them. + pub const MODE_SENSE: Self = Self::of([0x1A, 0, 0x3F, 0, MODE_BYTES as u8, 0], true); + /// READ(10) or WRITE(10) of `sectors` at `lba`. - pub fn transfer(write: bool, lba: u32, sectors: u16) -> Self { + pub fn transfer(io: Io, lba: u32, sectors: u16) -> Self { let [a, b, c, d] = lba.to_be_bytes(); let [hi, lo] = sectors.to_be_bytes(); - let opcode = if write { WRITE_10 } else { READ_10 }; - Self::of([opcode, 0, a, b, c, d, 0, hi, lo, 0], !write) + let (opcode, flags) = match io { + Io::Read => (READ_10, 0), + Io::Write { fua: false } => (WRITE_10, 0), + Io::Write { fua: true } => (WRITE_10, 1 << 3), + }; + Self::of([opcode, flags, a, b, c, d, 0, hi, lo, 0], io == Io::Read) } /// The bytes that go into the CBW. @@ -77,6 +96,19 @@ impl Cdb { } } +/// Bytes a MODE SENSE(6) asks for. +pub const MODE_BYTES: usize = 192; + +/// Whether the device takes FUA in a write: DPOFUA, bit 4 of the +/// device-specific parameter (SBC-3, mode parameters, table "DEVICE-SPECIFIC +/// PARAMETER field for direct access block devices"), which is byte 2 of +/// MODE SENSE(6)'s mode parameter header (SPC-4, mode parameter header +/// formats). `header` is the first bytes of what `reply` delivered. A device +/// that refuses MODE SENSE, or answers less than the header, takes none. +pub fn dpofua(reply: Reply, header: &[u8; 4]) -> bool { + matches!(reply, Reply::Ok { delivered } if delivered >= 4) && header[2] & (1 << 4) != 0 +} + /// Bytes a REQUEST SENSE asks for. pub const SENSE_BYTES: usize = 18; @@ -211,7 +243,7 @@ pub struct Transfer { lba: u64, count: u32, done: u32, - write: bool, + io: Io, sectors_per_block: u32, most: u32, } @@ -249,14 +281,14 @@ impl Moved { impl Transfer { /// A transfer on a disk of `geometry`; nothing to do for `count` zero. - pub fn new(lba: u64, count: u32, write: bool, geometry: &Geometry, most: u32) -> Result { + pub fn new(lba: u64, count: u32, io: Io, geometry: &Geometry, most: u32) -> Result { let sectors = u64::from(most) * u64::from(geometry.sectors_per_block); assert!(most > 0 && u16::try_from(sectors).is_ok(), "a batch READ(10) cannot count"); let fits = lba.checked_add(u64::from(count)).is_some_and(|end| end <= geometry.blocks); if count > 0 && !fits { return Err(PastTheEnd { lba, count, blocks: geometry.blocks }); } - Ok(Self { lba, count, done: 0, write, sectors_per_block: geometry.sectors_per_block, most }) + Ok(Self { lba, count, done: 0, io, sectors_per_block: geometry.sectors_per_block, most }) } /// The command owed next, or `None` once every block has moved. @@ -272,7 +304,7 @@ impl Transfer { let sector = u32::try_from(sector).expect("a sector past what READ(10) addresses"); let sectors = (blocks * self.sectors_per_block) as u16; Some(Batch { - cdb: Cdb::transfer(self.write, sector, sectors), + cdb: Cdb::transfer(self.io, sector, sectors), blocks, bytes: blocks as usize * HOST_BLOCK as usize, offset: self.done as usize * HOST_BLOCK as usize, @@ -579,10 +611,10 @@ mod tests { /// 2 to 5, the transfer length big endian in 7 and 8. #[test] fn a_transfer_cdb_is_laid_out_as_sbc_defines_it() { - let read = Cdb::transfer(false, 0x0102_0304, 0x0506); + let read = Cdb::transfer(Io::Read, 0x0102_0304, 0x0506); assert_eq!(read.bytes(), [0x28, 0, 1, 2, 3, 4, 0, 5, 6, 0]); assert!(read.data_in()); - let write = Cdb::transfer(true, 0xFFFF_FFFF, 1); + let write = Cdb::transfer(Io::Write { fua: false }, 0xFFFF_FFFF, 1); assert_eq!(write.bytes(), [0x2A, 0, 0xFF, 0xFF, 0xFF, 0xFF, 0, 0, 1, 0]); assert!(!write.data_in()); } @@ -594,6 +626,12 @@ mod tests { assert_eq!(Cdb::SYNCHRONIZE_CACHE.bytes(), [0x35, 0, 0, 0, 0, 0, 0, 0, 0, 0]); assert!(!Cdb::TEST_UNIT_READY.data_in() && !Cdb::SYNCHRONIZE_CACHE.data_in()); assert!(Cdb::REQUEST_SENSE.data_in()); + assert_eq!(Cdb::MODE_SENSE.bytes(), [0x1A, 0, 0x3F, 0, 192, 0]); + assert!(Cdb::MODE_SENSE.data_in()); + assert_eq!(Cdb::transfer(Io::Read, 0x0102_0304, 0x0506).bytes(), [0x28, 0, 1, 2, 3, 4, 0, 5, 6, 0]); + assert_eq!(Cdb::transfer(Io::Write { fua: false }, 1, 8).bytes(), [0x2A, 0, 0, 0, 0, 1, 0, 0, 8, 0]); + assert_eq!(Cdb::transfer(Io::Write { fua: true }, 1, 8).bytes(), [0x2A, 0x08, 0, 0, 0, 1, 0, 0, 8, 0]); + assert!(Cdb::transfer(Io::Read, 0, 1).data_in() && !Cdb::transfer(Io::Write { fua: true }, 0, 1).data_in()); assert_eq!(Query::Inquiry.cdb().bytes(), [0x12, 0, 0, 0, 36, 0]); assert_eq!(Query::Capacity10.cdb().bytes(), [0x25, 0, 0, 0, 0, 0, 0, 0, 0, 0]); assert_eq!(Query::Capacity16.cdb().bytes(), [0x9E, 0x10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 32, 0, 0]); @@ -606,6 +644,21 @@ mod tests { assert_eq!(usize::from(Query::Capacity16.cdb().bytes()[13]), Query::Capacity16.allocation()); } + /// DPOFUA is read from a header that arrived whole, and from nothing + /// else: not WP beside it, not a refusal, not a short answer. + #[test] + fn dpofua_is_the_headers_bit_and_a_device_that_does_not_say_takes_none() { + let whole = Reply::Ok { delivered: 4 }; + assert!(dpofua(whole, &[3, 0, 0x10, 0])); + assert!(dpofua(Reply::Ok { delivered: 36 }, &[35, 0, 0x90, 8])); + assert!(!dpofua(whole, &[3, 0, 0x80, 0]), "write protect is not DPOFUA"); + assert!(!dpofua(whole, &[3, 0, 0, 0])); + assert!(!dpofua(Reply::Ok { delivered: 3 }, &[3, 0, 0x10, 0]), "a header that did not arrive"); + let illegal = Sense { key: 0x05, asc: 0x20, ..Sense::NONE }; + assert!(!dpofua(Reply::Refused(illegal), &[3, 0, 0x10, 0])); + assert!(!dpofua(Reply::Broken, &[3, 0, 0x10, 0])); + } + fn fixed(code: u8, additional: u8, key: u8, asc: u8, ascq: u8) -> [u8; SENSE_BYTES] { let mut response = [0u8; SENSE_BYTES]; response[0] = code; @@ -661,22 +714,22 @@ mod tests { #[test] fn a_transfer_past_the_disk_is_refused_and_an_empty_one_is_nothing() { - let past = |lba, count| Transfer::new(lba, count, false, &STICK, 8); + let past = |lba, count| Transfer::new(lba, count, Io::Read, &STICK, 8); assert_eq!(past(19, 2), Err(PastTheEnd { lba: 19, count: 2, blocks: 20 })); assert_eq!(past(u64::MAX, 1).map(|_| ()), Err(PastTheEnd { lba: u64::MAX, count: 1, blocks: 20 })); assert!(past(19, 1).is_ok()); let empty = past(u64::MAX, 0).expect("nothing to move"); assert_eq!(empty.next(), None); - let no_size = Transfer::new(0, 1, false, &Geometry::NONE, 8); + let no_size = Transfer::new(0, 1, Io::Read, &Geometry::NONE, 8); assert_eq!(no_size, Err(PastTheEnd { lba: 0, count: 1, blocks: 0 })); - assert_eq!(Transfer::new(0, 0, false, &Geometry::NONE, 8).expect("nothing to move").next(), None); + assert_eq!(Transfer::new(0, 0, Io::Read, &Geometry::NONE, 8).expect("nothing to move").next(), None); } /// Batches of at most `most` blocks, each at its own sector and offset, /// and a batch that did not move whole is not moved past. #[test] fn a_transfer_is_its_batches_in_order_and_only_a_whole_one_advances() { - let mut transfer = Transfer::new(2, 18, true, &STICK, 8).expect("fits"); + let mut transfer = Transfer::new(2, 18, Io::Write { fua: false }, &STICK, 8).expect("fits"); let mut seen = [None; 4]; for slot in &mut seen { let Some(batch) = transfer.next() else { break }; @@ -687,9 +740,9 @@ mod tests { assert_eq!(transfer.answered(&batch, Reply::Ok { delivered: bytes }), Moved::Whole); } assert_eq!(seen, [ - Some((Cdb::transfer(true, 16, 64), 8, 0, 2)), - Some((Cdb::transfer(true, 80, 64), 8, 8 * 4096, 10)), - Some((Cdb::transfer(true, 144, 16), 2, 16 * 4096, 18)), + Some((Cdb::transfer(Io::Write { fua: false }, 16, 64), 8, 0, 2)), + Some((Cdb::transfer(Io::Write { fua: false }, 80, 64), 8, 8 * 4096, 10)), + Some((Cdb::transfer(Io::Write { fua: false }, 144, 16), 2, 16 * 4096, 18)), None, ]); } @@ -698,7 +751,7 @@ mod tests { /// the first batch moved. #[test] fn only_the_first_batch_may_answer_ask_again() { - let mut transfer = Transfer::new(0, 16, false, &STICK, 8).expect("fits"); + let mut transfer = Transfer::new(0, 16, Io::Read, &STICK, 8).expect("fits"); let first = transfer.next().expect("a batch"); assert_eq!(transfer.answered(&first, Reply::Budget), Moved::Ended(Fail::Budget)); assert_eq!(transfer.answered(&first, Reply::Broken), Moved::Ended(Fail::Device)); diff --git a/system/diskserver/src/disk.rs b/system/diskserver/src/disk.rs index 5350b1fba1a..01d1ffc03cc 100644 --- a/system/diskserver/src/disk.rs +++ b/system/diskserver/src/disk.rs @@ -15,6 +15,11 @@ //! [`Error::Gone`], and so is every request once a reconnect fails: what was //! waiting for the next session would otherwise go out under requests asked //! since. +//! +//! **A durable write is one write with `fua` where the session's device +//! forces it, and a write and a flush where it does not** +//! ([`Served::write_shared_durable`]); the disk's driver says which at its +//! bring-up. use std::cell::RefCell; use std::collections::{BTreeMap, BTreeSet}; @@ -33,7 +38,8 @@ use crate::{Outcome, Ticket}; pub trait Link { fn blocks(&self) -> u64; fn submit_read(&mut self, lba: u64, blocks: u32) -> Result; - fn submit_write(&mut self, lba: u64, blocks: &[SharedBlock]) -> Result; + fn fua(&self) -> bool; + fn submit_write(&mut self, lba: u64, blocks: &[SharedBlock], fua: bool) -> Result; fn submit_flush(&mut self) -> Result; fn wait(&mut self, block: bool) -> Waited; fn reconnect(&mut self) -> Result<(), SessionError>; @@ -46,8 +52,11 @@ impl Link for Session { fn submit_read(&mut self, lba: u64, blocks: u32) -> Result { Session::submit_read(self, lba, blocks) } - fn submit_write(&mut self, lba: u64, blocks: &[SharedBlock]) -> Result { - Session::submit_write(self, lba, blocks) + fn fua(&self) -> bool { + Session::fua(self) + } + fn submit_write(&mut self, lba: u64, blocks: &[SharedBlock], fua: bool) -> Result { + Session::submit_write(self, lba, blocks, fua) } fn submit_flush(&mut self) -> Result { Session::submit_flush(self) @@ -178,11 +187,28 @@ impl Served { /// `blocks` written at `lba`, shared: the session keeps them until a /// flush covers the write, rather than a copy of them. pub async fn write_shared(&self, lba: u64, blocks: &[SharedBlock]) -> Result<(), Error> { + self.write_requests(lba, blocks, false).await + } + + /// `blocks` written at `lba` and on the medium once this returns: with + /// `fua` where the session offers it, and otherwise written and then + /// flushed. Not over a write no flush has covered. + pub async fn write_shared_durable(&self, lba: u64, blocks: &[SharedBlock]) -> Result<(), Error> { + // Asked of the session each time: a reconnect may have found the + // disk behind another driver. + if self.link.borrow().fua() { + return self.write_requests(lba, blocks, true).await; + } + self.write_requests(lba, blocks, false).await?; + self.flush().await + } + + async fn write_requests(&self, lba: u64, blocks: &[SharedBlock], fua: bool) -> Result<(), Error> { within(lba, blocks.len(), self.blocks)?; let per = MAX_REQUEST_BLOCKS as usize; for (i, chunk) in blocks.chunks(per).enumerate() { let at = lba + (i * per) as u64; - match self.ask(|link| link.submit_write(at, chunk)).await.map_err(|Gone| Error::Gone)?.outcome { + match self.ask(|link| link.submit_write(at, chunk, fua)).await.map_err(|Gone| Error::Gone)?.outcome { Outcome::Done => {} Outcome::ReadOnly => return Err(Error::ReadOnly), _ => return Err(Error::Device), @@ -192,6 +218,10 @@ impl Served { } } +fn shared(data: &[Block]) -> Vec { + data.iter().map(|block| std::rc::Rc::new(Box::new(*block))).collect() +} + impl BlockDev for Served { fn blocks(&self) -> u64 { self.blocks @@ -214,8 +244,11 @@ impl BlockDev for Served { /// A copy of `data`, which the session keeps until a flush covers it. async fn write(&self, lba: u64, data: &[Block]) -> Result<(), Error> { - let shared: Vec = data.iter().map(|block| std::rc::Rc::new(Box::new(*block))).collect(); - self.write_shared(lba, &shared).await + self.write_shared(lba, &shared(data)).await + } + + async fn write_durable(&self, lba: u64, data: &[Block]) -> Result<(), Error> { + self.write_shared_durable(lba, &shared(data)).await } async fn flush(&self) -> Result<(), Error> { diff --git a/system/diskserver/src/disk/tests.rs b/system/diskserver/src/disk/tests.rs index 4da6d8d514c..25796f057cf 100644 --- a/system/diskserver/src/disk/tests.rs +++ b/system/diskserver/src/disk/tests.rs @@ -15,7 +15,7 @@ use crate::{Outcome, Ticket}; #[derive(Clone, Debug, PartialEq, Eq)] enum Asked { Read { lba: u64, blocks: u32 }, - Write { lba: u64, data: Vec }, + Write { lba: u64, data: Vec, fua: bool }, Flush, } @@ -37,6 +37,8 @@ struct Script { reconnects: VecDeque>, reconnected: u32, read_only: bool, + /// What each session's open says of its device. + fua: bool, } impl Script { @@ -52,6 +54,7 @@ impl Script { reconnects: VecDeque::new(), reconnected: 0, read_only: false, + fua: false, } } @@ -73,7 +76,9 @@ impl Script { (Outcome::Done, Some(self.disk[at..at + blocks as usize * BLOCK].to_vec())) } Asked::Write { .. } if self.read_only => (Outcome::ReadOnly, None), - Asked::Write { lba, data } => { + // A server refuses a forced write it did not offer. + Asked::Write { fua: true, .. } if !self.fua => (Outcome::Invalid, None), + Asked::Write { lba, data, .. } => { let at = lba as usize * BLOCK; self.disk[at..at + data.len()].copy_from_slice(&data); (Outcome::Done, None) @@ -93,8 +98,12 @@ impl Link for Script { self.ask(Asked::Read { lba, blocks }) } - fn submit_write(&mut self, lba: u64, blocks: &[SharedBlock]) -> Result { - self.ask(Asked::Write { lba, data: blocks.iter().flat_map(|b| b.iter().copied()).collect() }) + fn fua(&self) -> bool { + self.fua + } + + fn submit_write(&mut self, lba: u64, blocks: &[SharedBlock], fua: bool) -> Result { + self.ask(Asked::Write { lba, data: blocks.iter().flat_map(|b| b.iter().copied()).collect(), fua }) } fn submit_flush(&mut self) -> Result { @@ -152,12 +161,45 @@ fn a_request_on_the_wire_at_a_session_end_is_asked_again_after_the_reconnect() { let script = disk.link.borrow(); assert_eq!(script.reconnected, 3); let read = Asked::Read { lba: 3, blocks: 1 }; - let write = Asked::Write { lba: 5, data: vec![0x55; BLOCK] }; + let write = Asked::Write { lba: 5, data: vec![0x55; BLOCK], fua: false }; assert_eq!(script.log, [read.clone(), read, write.clone(), write, Asked::Flush, Asked::Flush]); assert!(script.disk[5 * BLOCK..6 * BLOCK].iter().all(|&b| b == 0x55)); assert!(!disk.pending()); } +/// A durable write on a session whose device forces writes is one write with +/// `fua` and no flush, split like any transfer; on one whose device does not +/// it is the write and then a flush. +#[test] +fn a_durable_write_is_forced_where_the_session_offers_it_and_flushed_where_not() { + for fua in [true, false] { + let mut script = Script::new(40); + script.fua = fua; + let disk = Served::new(script); + now(&disk, disk.write_durable(2, &blocks(0x77, 33))).unwrap(); + let script = disk.link.borrow(); + let write = |lba, n| Asked::Write { lba, data: vec![0x77; n * BLOCK], fua }; + let mut want = vec![write(2, 32), write(34, 1)]; + if !fua { + want.push(Asked::Flush); + } + assert_eq!(script.log, want, "fua {fua}"); + assert!(script.disk[2 * BLOCK..35 * BLOCK].iter().all(|&b| b == 0x77)); + } +} + +/// A forced write on the wire when its session ends is asked again, forced. +#[test] +fn a_forced_write_on_the_wire_at_a_session_end_is_asked_again_forced() { + let mut script = Script::new(4); + script.fua = true; + script.ends.push_back(0); + let disk = Served::new(script); + now(&disk, disk.write_durable(1, &blocks(9, 1))).unwrap(); + let forced = Asked::Write { lba: 1, data: vec![9; BLOCK], fua: true }; + assert_eq!(disk.link.borrow().log, [forced.clone(), forced]); +} + /// A request refused at every session end is asked once more than /// [`MAX_RECONNECTS`] allows again, and then the disk is gone to it. #[test] diff --git a/system/diskserver/src/session.rs b/system/diskserver/src/session.rs index 99a3e7f8270..38e4f57896e 100644 --- a/system/diskserver/src/session.rs +++ b/system/diskserver/src/session.rs @@ -188,16 +188,25 @@ impl Session { t } + /// Whether the session's device forces a write asked with `fua` onto its + /// medium, as this session's open was answered: a reconnect may answer + /// otherwise. + pub fn fua(&self) -> bool { + self.opened.fua() + } + /// Ask for `blocks` written at `lba`, which the session keeps a share of /// until a flush covers the write. With [`KEPT_BLOCKS`] kept, it goes out - /// after a flush of the session's own. - pub fn submit_write(&mut self, lba: u64, blocks: &[SharedBlock]) -> Result { + /// after a flush of the session's own. With `fua` it is on the medium + /// once answered [`Outcome::Done`] and nothing of it is kept; one asked + /// of a session that does not offer it is answered [`Outcome::Invalid`]. + pub fn submit_write(&mut self, lba: u64, blocks: &[SharedBlock], fua: bool) -> Result { let count = transfer_blocks(blocks.len()); if self.bells.is_none() { return Err(Unsent::Ended); } let ticket = self.ticket(); - self.client.submit(ticket, Ask::Write { lba, blocks: count, payload: blocks.to_vec() }); + self.client.submit(ticket, Ask::Write { lba, blocks: count, payload: blocks.to_vec(), fua }); self.pending.insert(ticket, Pending::Write); Ok(ticket) } diff --git a/system/fileserver/src/cache.rs b/system/fileserver/src/cache.rs index 85ab0098f18..f89160b0b21 100644 --- a/system/fileserver/src/cache.rs +++ b/system/fileserver/src/cache.rs @@ -13,6 +13,13 @@ //! first, and is refused whole when that flush is, so memory bounds the //! write-back and not the other way round. //! +//! **A durable write goes to the disk at once and alone** +//! ([`BlockDev::write_durable`]): its blocks into the cache and then forced +//! onto the disk's medium ([`Disk::write_shared_durable`]), with no flush of +//! the disk and nothing else written. It runs as a flush does, one at a time +//! with every flush, so no flush carries an older copy of its blocks beside +//! it. +//! //! **Clean blocks are kept up to the limit the cache is made with** and the //! oldest-read goes first; a dirty block is never dropped. A block read in //! is kept only if the allocator gives it and its place in the eviction order @@ -62,6 +69,13 @@ pub trait Disk: BlockDev { let data: Vec = blocks.iter().map(|block| ***block).collect(); self.write(lba, &data).await } + + /// [`BlockDev::write_durable`] of `blocks`, shared or copied as + /// [`write_shared`](Self::write_shared) has it. + async fn write_shared_durable(&self, lba: u64, blocks: &[SharedBlock]) -> Result<(), Error> { + let data: Vec = blocks.iter().map(|block| ***block).collect(); + self.write_durable(lba, &data).await + } } impl Disk for diskserver::disk::Served { @@ -76,6 +90,10 @@ impl Disk for diskserver::disk::Served { async fn write_shared(&self, lba: u64, blocks: &[SharedBlock]) -> Result<(), Error> { diskserver::disk::Served::write_shared(self, lba, blocks).await } + + async fn write_shared_durable(&self, lba: u64, blocks: &[SharedBlock]) -> Result<(), Error> { + diskserver::disk::Served::write_shared_durable(self, lba, blocks).await + } } /// Dirty blocks held before the cache flushes by itself: 8 MiB. @@ -344,28 +362,24 @@ impl BlockDev for Cache { if over { self.flush().await?; } - let mut inner = self.inner.borrow_mut(); - for (block, chunk) in (lba..).zip(data) { - inner.versions += 1; - let version = inner.versions; - let newly = match inner.slots.get_mut(&block) { - Some(slot) => { - match Rc::get_mut(&mut slot.data) { - Some(data) => **data = *chunk, - None => slot.data = Rc::new(Box::new(*chunk)), - } - slot.version = version; - !std::mem::replace(&mut slot.dirty, true) - } - None => { - inner.slots.insert(block, Slot { data: Rc::new(Box::new(*chunk)), dirty: true, version }); - true - } - }; - if newly { - inner.dirty += 1; - } + self.inner.borrow_mut().put(lba, data); + Ok(()) + } + + /// Into the cache, and from it forced onto the disk's medium in runs, + /// with the flush's place held: the blocks are dirty only until their + /// run is answered, so they pass the dirty bound by, and a run the disk + /// refused stays dirty for the next flush. + async fn write_durable(&self, lba: u64, data: &[Block]) -> Result<(), Error> { + within(lba, data.len(), self.disk.blocks())?; + self.until(|inner| !inner.flushing).await; + self.inner.borrow_mut().flushing = true; + let _flushing = Flushing(self); + self.inner.borrow_mut().put(lba, data); + for first in (0..data.len()).step_by(RUN) { + self.write_run(lba + first as u64, RUN.min(data.len() - first), true).await?; } + self.inner.borrow_mut().evict(); Ok(()) } @@ -384,7 +398,7 @@ impl BlockDev for Cache { poll_fn(|cx| loop { while refused.is_ok() && flight.len() < self.disk.depth() { let Some((first, count)) = runs.next() else { break }; - flight.push(Box::pin(self.write_run(first, count))); + flight.push(Box::pin(self.write_run(first, count, false))); } let before = flight.len(); flight.retain_mut(|run| match run.as_mut().poll(cx) { @@ -426,9 +440,10 @@ impl Cache { runs } - /// One run of a flush: written once no fetch is reading it, and each block - /// clean afterwards unless written again meanwhile. - async fn write_run(&self, first: u64, count: usize) -> Result<(), Error> { + /// One run of dirty blocks: written once no fetch is reading it, forced + /// onto the medium if `durable`, and each block clean afterwards unless + /// written again meanwhile. + async fn write_run(&self, first: u64, count: usize, durable: bool) -> Result<(), Error> { let end = first + count as u64; self.until(|inner| !inner.fetches.values().any(|f| f.first < end && first < f.first + f.count as u64)).await; let (blocks, versions): (Vec, Vec) = { @@ -441,9 +456,13 @@ impl Cache { }) .unzip() }; - let wrote = self.disk.write_shared(first, &blocks).await; - self.inner.borrow_mut().written += 1; - wrote?; + if durable { + self.disk.write_shared_durable(first, &blocks).await?; + } else { + let wrote = self.disk.write_shared(first, &blocks).await; + self.inner.borrow_mut().written += 1; + wrote?; + } let mut inner = self.inner.borrow_mut(); for (block, version) in (first..).zip(versions) { // A block written again meanwhile stays dirty. @@ -465,6 +484,31 @@ impl Cache { } impl Inner { + /// `data` at `lba`, each block dirty at a version of its own. + fn put(&mut self, lba: u64, data: &[Block]) { + for (block, chunk) in (lba..).zip(data) { + self.versions += 1; + let version = self.versions; + let newly = match self.slots.get_mut(&block) { + Some(slot) => { + match Rc::get_mut(&mut slot.data) { + Some(data) => **data = *chunk, + None => slot.data = Rc::new(Box::new(*chunk)), + } + slot.version = version; + !std::mem::replace(&mut slot.dirty, true) + } + None => { + self.slots.insert(block, Slot { data: Rc::new(Box::new(*chunk)), dirty: true, version }); + true + } + }; + if newly { + self.dirty += 1; + } + } + } + /// Drop the oldest clean blocks past the limit. fn evict(&mut self) { while self.slots.len() - self.dirty > self.clean_limit { @@ -508,6 +552,9 @@ mod tests { reads: Cell, writes: Cell, flushes: Cell, + /// Writes forced onto the medium, and whether they are refused. + forced: Cell, + refusing_forced: Cell, refusing_flush: Cell, } @@ -518,6 +565,8 @@ mod tests { reads: Cell::new(0), writes: Cell::new(0), flushes: Cell::new(0), + forced: Cell::new(0), + refusing_forced: Cell::new(false), refusing_flush: Cell::new(false), } } @@ -542,6 +591,13 @@ mod tests { } self.ram.flush().await } + async fn write_durable(&self, lba: u64, data: &[Block]) -> Result<(), Error> { + self.forced.set(self.forced.get() + 1); + if self.refusing_forced.get() { + return Err(Error::Device); + } + self.ram.write(lba, data).await + } } impl Disk for Counting {} @@ -582,6 +638,41 @@ mod tests { assert!(c.inner.borrow().order.len() <= 4 * (2 + DIRTY_LIMIT) + 2); } + /// A durable write is on the disk when it returns, forced and alone: no + /// flush of the disk, and a dirty block beside it still unwritten. It + /// owes the disk no flush afterwards either. + #[test] + fn a_durable_write_is_forced_onto_the_disk_alone_and_owes_no_flush() { + let c = cache(); + block_on_ready(c.write(3, &[[3; BLOCK]])).unwrap(); + block_on_ready(c.write_durable(7, &[[0xAB; BLOCK]; 40])).unwrap(); + assert_eq!((c.disk.forced.get(), c.disk.writes.get(), c.disk.flushes.get()), (2, 0, 0), "40 blocks are two runs"); + assert_eq!(c.counts().dirty, 1, "the block beside it is still the cache's alone"); + let mut on_disk = [[0; BLOCK]; 40]; + block_on_ready(c.disk.ram.read(7, &mut on_disk)).unwrap(); + assert_eq!(on_disk, [[0xAB; BLOCK]; 40]); + assert_eq!(read_one(&c, 46), Ok([0xAB; BLOCK])); + assert_eq!(c.disk.reads.get(), 0, "a durable write's blocks are read from the cache"); + + block_on_ready(c.flush()).unwrap(); + assert_eq!((c.disk.writes.get(), c.disk.flushes.get()), (1, 1), "the flush is for the block beside it"); + block_on_ready(c.write_durable(7, &[[0xCD; BLOCK]])).unwrap(); + block_on_ready(c.flush()).unwrap(); + assert_eq!((c.disk.forced.get(), c.disk.flushes.get()), (3, 1), "a forced write was flushed for"); + } + + /// A durable write the disk refused is an error, and its blocks stay + /// dirty: the next flush writes them and asks the disk to keep them. + #[test] + fn a_durable_write_the_disk_refused_stays_dirty_for_the_next_flush() { + let c = cache(); + c.disk.refusing_forced.set(true); + assert_eq!(block_on_ready(c.write_durable(7, &[[0xAB; BLOCK]])), Err(Error::Device)); + assert_eq!(c.counts().dirty, 1); + block_on_ready(c.flush()).unwrap(); + assert_eq!((c.disk.writes.get(), c.disk.flushes.get(), c.counts().dirty), (1, 1, 0)); + } + #[test] fn a_write_reaches_the_disk_at_the_flush_and_not_before() { let c = cache(); @@ -897,6 +988,25 @@ mod tests { } } + /// A durable write of a block a flush is carrying waits for that flush: + /// the disk never has the older copy in flight beside the newer, and + /// holds the newer afterwards. + #[test] + fn a_durable_write_waits_for_the_flush_carrying_its_block() { + let c = Cache::new(Held::new(16), CLEAN_LIMIT); + block_on_ready(c.write(1, &[[1; BLOCK]])).unwrap(); + c.disk.open.set(false); + let exec = Executor::new(); + exec.spawn(async { c.flush().await.unwrap() }); + exec.spawn(async { c.write_durable(1, &[[2; BLOCK]]).await.unwrap() }); + exec.run_until_stalled(); + assert_eq!(c.disk.writes_in_flight(), 1, "the durable write went out beside the flush"); + c.disk.release(); + assert_eq!(exec.run_until_stalled(), 0); + assert_eq!(c.disk.overlaps.get(), 0); + assert_eq!(on_disk(&c.disk, 1), [2; BLOCK]); + } + /// A flush keeps as many runs in flight as the disk takes, and no more. #[test] fn a_flush_keeps_as_many_runs_in_flight_as_the_disk_takes() { diff --git a/system/nvme/src/main.rs b/system/nvme/src/main.rs index 2883d508a7d..c87cbc5e91f 100644 --- a/system/nvme/src/main.rs +++ b/system/nvme/src/main.rs @@ -24,7 +24,7 @@ use toyos_blockring::disk::Settled; use toyos_blockring::BLOCK_BYTES; use toyos_blockserve::link::{Handed, Link}; use toyos_blockserve::sessions::MAX_SESSIONS; -use toyos_blockserve::{Device, Sessions}; +use toyos_blockserve::{Device, Io, Sessions}; mod nvme; @@ -82,8 +82,11 @@ impl Device for Controller { fn has_room(&self) -> bool { Controller::has_room(self) } - fn submit_io(&mut self, write: bool, block: u64, blocks: u32, at: u64, owner: toyos_blockserve::Owner) { - Controller::submit_io(self, write, block, blocks, at, Owner::Session(owner)); + fn fua(&self) -> bool { + true + } + fn submit_io(&mut self, io: Io, block: u64, blocks: u32, at: u64, owner: toyos_blockserve::Owner) { + Controller::submit_io(self, io, block, blocks, at, Owner::Session(owner)); } fn submit_flush(&mut self, owner: toyos_blockserve::Owner) -> bool { // No volatile cache: every write answered is on the medium already. @@ -126,7 +129,8 @@ fn main() { } }; println!( - "{name}: up: {} I/O queues of {} commands, volatile write cache {}, {}-byte sectors, {} sectors", + "{name}: up: {} I/O queues of {} commands, volatile write cache {}, a forced write is a Write with FUA, \ + {}-byte sectors, {} sectors", ctrl.queues(), nvme::COMMANDS_PER_QUEUE, if ctrl.vwc { "present, so a flush issues Flush" } else { "absent, so a flush issues nothing" }, diff --git a/system/nvme/src/nvme.rs b/system/nvme/src/nvme.rs index ea6ddf3d314..ac2d0becd49 100644 --- a/system/nvme/src/nvme.rs +++ b/system/nvme/src/nvme.rs @@ -21,6 +21,11 @@ //! volatile cache a completed write is on the medium, and a flush has nothing //! to issue. //! +//! **A write asked with `fua` sets FUA** (NVM Command Set 1.0 §3.2.6, Command +//! Dword 12 bit 30): the controller writes the data to non-volatile medium +//! before it completes the command. Every controller takes the bit, so every +//! session is offered it. +//! //! **A command the controller does not answer is reclaimed by a controller //! reset** (§3.7.2): `CC.EN` cleared, every command it held is gone, the queues //! are made again. Whoever owned one is told it was not done. @@ -32,6 +37,7 @@ use toyos::poller::{Poller, READABLE}; use toyos::shm::SharedMemory; use toyos::{DmaRegion, PciDev}; use toyos_abi::syscall::SyscallError; +use toyos_blockserve::Io; const REG_CAP: usize = 0x00; const REG_CC: usize = 0x14; @@ -73,6 +79,8 @@ const FEATURE_NUMBER_OF_QUEUES: u32 = 0x07; const IO_FLUSH: u8 = 0x00; const IO_WRITE: u8 = 0x01; const IO_READ: u8 = 0x02; +/// Write's Force Unit Access, in Command Dword 12. +const WRITE_FUA: u32 = 1 << 30; /// Where everything this driver itself owns is in its one grant. const OFF_ADMIN_SQ: usize = 0x0000; @@ -90,6 +98,17 @@ const _: () = assert!(OFF_PRP + MAX_IO_QUEUES as usize * IO_DEPTH as usize * PAG const _: () = assert!(ADMIN_DEPTH as usize * SQE_BYTES <= PAGE); const _: () = assert!(IO_DEPTH as usize * SQE_BYTES <= PAGE); +/// The opcode and Command Dword 12 of `io` over `sectors` sectors: the count, +/// zero-based, and FUA for a write asked with it. +fn transfer(io: Io, sectors: u64) -> (u8, u32) { + let count = (sectors - 1) as u32; + match io { + Io::Read => (IO_READ, count), + Io::Write { fua: false } => (IO_WRITE, count), + Io::Write { fua: true } => (IO_WRITE, count | WRITE_FUA), + } +} + /// Whose a command is. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum Owner { @@ -603,26 +622,27 @@ impl Controller { (0..self.io.len()).filter(|&i| !self.io[i].free.is_empty()).max_by_key(|&i| self.io[i].free.len()) } - /// Issue a read or a write of `blocks` 4 KiB blocks at device block - /// `block`, to or from device address `at`, for `owner`. + /// Issue `io` of `blocks` 4 KiB blocks at device block `block`, to or + /// from device address `at`, for `owner`. /// /// # Panics /// With no room: the caller asks [`Self::has_room`] first. - pub fn submit_io(&mut self, write: bool, block: u64, blocks: u32, at: u64, owner: Owner) { + pub fn submit_io(&mut self, io: Io, block: u64, blocks: u32, at: u64, owner: Owner) { let per = (toyos_blockring::BLOCK_BYTES as u32 / self.lba_bytes) as u64; - self.submit_sectors(write, block * per, blocks as u64 * per, at, owner); + self.submit_sectors(io, block * per, blocks as u64 * per, at, owner); } /// [`Self::submit_io`] in the namespace's own sectors: `sectors` of them /// from `lba`, which a table at the device's end needs when its sector /// count is not whole blocks. - fn submit_sectors(&mut self, write: bool, lba: u64, sectors: u64, at: u64, owner: Owner) { + fn submit_sectors(&mut self, io: Io, lba: u64, sectors: u64, at: u64, owner: Owner) { let queue = self.roomiest().expect("nvme: an I/O submission with no queue room"); let pages = (sectors * self.lba_bytes as u64).div_ceil(PAGE as u64) as usize; let (dma, dma_addr) = (self.dma, self.dma_addr); let list_base = OFF_PRP + queue * IO_DEPTH as usize * PAGE; + let (opcode, cdw12) = transfer(io, sectors); let cmd = Command { - opcode: if write { IO_WRITE } else { IO_READ }, + opcode, nsid: 1, prp1: at, prp2: match pages { @@ -632,7 +652,7 @@ impl Controller { }, cdw10: lba as u32, cdw11: (lba >> 32) as u32, - cdw12: (sectors - 1) as u32, + cdw12, }; // Every page after the first, in this slot's list page (§4.1.1). self.submit(Some(queue), cmd, owner, |cid| { @@ -661,7 +681,7 @@ impl Controller { assert!(out.len() <= PAGE && out.len().is_multiple_of(self.lba_bytes as usize) && !out.is_empty()); assert_eq!(self.busy(), 0, "nvme: a waited read beside other commands"); let sectors = (out.len() / self.lba_bytes as usize) as u64; - self.submit_sectors(false, lba, sectors, self.dma_addr + OFF_SCRATCH as u64, Owner::Driver); + self.submit_sectors(Io::Read, lba, sectors, self.dma_addr + OFF_SCRATCH as u64, Owner::Driver); let start = Instant::now(); let mut done = Vec::new(); let ok = loop { @@ -702,7 +722,7 @@ impl Controller { #[cfg(test)] mod tests { - use super::{page_size, sector_shift}; + use super::{page_size, sector_shift, transfer, Io}; /// An Identify Namespace page: `formats` LBA formats, each `(MS, LBADS)`. fn namespace(formats: &[(u16, u8)]) -> impl Fn(usize) -> u32 { @@ -713,6 +733,13 @@ mod tests { } } + #[test] + fn only_a_write_asked_with_fua_sets_it() { + assert_eq!(transfer(Io::Read, 8), (0x02, 7)); + assert_eq!(transfer(Io::Write { fua: false }, 8), (0x01, 7)); + assert_eq!(transfer(Io::Write { fua: true }, 256), (0x01, 255 | 1 << 30)); + } + #[test] fn a_namespace_past_sixteen_formats_takes_the_index_high_bits() { let mut formats = vec![(0, 9); 64]; diff --git a/system/usbd/src/main.rs b/system/usbd/src/main.rs index 046dd6ad9bc..3d34444a5f4 100644 --- a/system/usbd/src/main.rs +++ b/system/usbd/src/main.rs @@ -31,14 +31,14 @@ use toyos_abi::syscall::{SyscallError, DEV_PREFIX}; use toyos_blockring::disk::Settled; use toyos_blockserve::link::{Handed, Link}; use toyos_blockserve::sessions::{Owner as Request, MAX_SESSIONS}; -use toyos_blockserve::{table, Device, Sessions}; +use toyos_blockserve::{table, Device, Io, Sessions}; use toyos_input::wire::{self, Record, MAX_PAYLOAD}; use toyos_inspect::{Snapshot, MSG_INSPECT, MSG_SNAPSHOT}; use toyos_pci_claim::{Bar, Grant}; use toyos_xhci::exec::{Dead, Host, Nanos, Tasks}; use toyos_xhci::hc::Reset; use toyos_xhci::input::{Opened, Owner, Sent}; -use toyos_xhci::msc::{Event, Work}; +use toyos_xhci::msc::{self, Event, Work}; use toyos_xhci::usb::{self, Config, Usb}; /// Connections accepted and not yet asked, and how long one may stay so. @@ -116,8 +116,15 @@ impl Device for Disk<'_> { fn has_room(&self) -> bool { self.usb.disks.has_room(self.disk) } - fn submit_io(&mut self, write: bool, block: u64, blocks: u32, at: u64, owner: Request) { - self.usb.disks.submit(self.disk, Work::Io { write, block, blocks, at, token: (owner.session, owner.tag) }); + fn fua(&self) -> bool { + self.usb.disks.fua(self.disk) + } + fn submit_io(&mut self, io: Io, block: u64, blocks: u32, at: u64, owner: Request) { + let io = match io { + Io::Read => msc::Io::Read, + Io::Write { fua } => msc::Io::Write { fua }, + }; + self.usb.disks.submit(self.disk, Work::Io { io, block, blocks, at, token: (owner.session, owner.tag) }); } fn submit_flush(&mut self, owner: Request) -> bool { let cached = self.usb.disks.cached(self.disk); diff --git a/system/usbd/xhci/src/bot.rs b/system/usbd/xhci/src/bot.rs index 00b3418553c..48f3b90ccf8 100644 --- a/system/usbd/xhci/src/bot.rs +++ b/system/usbd/xhci/src/bot.rs @@ -343,6 +343,7 @@ impl CswDue { #[cfg(test)] mod tests { use super::*; + use toyos_scsi::Io; const EVERY: [Phase; 6] = [ Phase::Closed, @@ -372,11 +373,11 @@ mod tests { const WHOLE: Answer = Answer::Moved { code: CC_SUCCESS, residue: 0 }; fn read(sectors: u16) -> Cdb { - Cdb::transfer(false, 0, sectors) + Cdb::transfer(Io::Read, 0, sectors) } fn write(sectors: u16) -> Cdb { - Cdb::transfer(true, 0, sectors) + Cdb::transfer(Io::Write { fua: false }, 0, sectors) } /// A CSW as §5.2 lays it out. diff --git a/system/usbd/xhci/src/msc.rs b/system/usbd/xhci/src/msc.rs index 02aa2ec36e2..0d9b04dac44 100644 --- a/system/usbd/xhci/src/msc.rs +++ b/system/usbd/xhci/src/msc.rs @@ -9,6 +9,11 @@ //! again, the disk held for its device's return ([`crate::identity`]); then //! offline, the last thing it is sent a reset. //! +//! **A write is forced with FUA where the device says it takes it** (DPOFUA +//! in its mode parameters, `toyos_scsi::dpofua`), asked once as the disk +//! first comes up and said in its line. A disk that does not is offered +//! without, and its sessions' clients write and then flush. +//! //! **A command's bound is policy.** BOT and SBC give a command no time //! limit, and a stick has answered later than two seconds //! (`issues/a-stick-that-answers-late-is-broken-by-the-two-second-abandon.md`): @@ -22,6 +27,7 @@ use core::cell::{Cell, RefCell}; use core::num::NonZeroU16; use toyos_device_memory::{DmaBuffers, Registers}; +pub use toyos_scsi::Io; use toyos_scsi::{self as scsi, Cdb, Geometry, Reply, Sense, HOST_BLOCK, SENSE_BYTES}; use crate::bot::{self, Answer, Bot, Broke, Pipe, RoundTrip, CBW_LEN, CSW_LEN}; @@ -56,7 +62,7 @@ pub struct Silent; pub enum Work { /// `blocks` host blocks at host block `block`, to or from device address /// `at`, which the controller's domain maps. - Io { write: bool, block: u64, blocks: u32, at: u64, token: (u64, u32) }, + Io { io: Io, block: u64, blocks: u32, at: u64, token: (u64, u32) }, /// The device's volatile cache to the medium. Flush { token: (u64, u32) }, } @@ -94,6 +100,8 @@ struct Disk { /// When this driver last reset its port. reset_at: Option, no_cache: bool, + /// The device said it takes FUA when the disk first came up. + fua: bool, debt: Debt, } @@ -148,6 +156,11 @@ impl Disks { self.disks.borrow().get(&disk).is_some_and(|d| !d.no_cache) } + /// Whether disk `disk` forces a write asked with FUA onto its medium. + pub fn fua(&self, disk: u32) -> bool { + self.disks.borrow().get(&disk).is_some_and(|d| d.fua) + } + /// Queue `work` on disk `disk`, which its sessions were offered and which /// has not gone. pub fn submit(&self, disk: u32, work: Work) { @@ -172,7 +185,7 @@ impl Disks { fn end(&self, disk: u32) { let Some(d) = self.disks.borrow_mut().remove(&disk) else { return }; for work in d.work { - let write = matches!(work, Work::Io { write: true, .. }); + let write = matches!(work, Work::Io { io: Io::Write { .. }, .. }); self.event(Event::Answered { disk, token: work.token(), write, ok: false, status: 0 }); } self.event(Event::Gone { disk }); @@ -291,15 +304,18 @@ pub async fn serve( Some(disk) => disk, None => { let table = msc.table(&geometry).await?; + let fua = msc.takes_fua().await?; if msc.asked { return Ok(()); } let disk = disks.next.get(); disks.next.set(disk + 1); - let held = Disk { identity, geometry, work: VecDeque::new(), away: None, reset_at: None, no_cache: false, debt: Debt::NONE }; + let held = + Disk { identity, geometry, work: VecDeque::new(), away: None, reset_at: None, no_cache: false, fua, debt: Debt::NONE }; disks.disks.borrow_mut().insert(disk, held); disks.event(Event::Arrived { disk, name: format!("usb disk {disk}"), table }); - say(format_args!("serving it as disk {disk}")); + let forced = if fua { "DPOFUA set, so a forced write is a WRITE(10) with FUA" } else { "no DPOFUA, so a forced write is a write and a flush" }; + say(format_args!("serving it as disk {disk}: {forced}")); disk } }; @@ -437,6 +453,14 @@ impl<'d, 'u, R: Registers, D: DmaBuffers, C: Host, T> Msc<'d, 'u, R, D, C, T> { } } + /// Whether the device's mode parameters say it takes FUA. + async fn takes_fua(&mut self) -> Result { + let at = self.addr(self.io + AT_SMALL); + let reply = self.command(&Cdb::MODE_SENSE, Data::In { at, len: scsi::MODE_BYTES as u32 }).await?; + let header = core::array::from_fn(|i| self.dev.read8(self.io + AT_SMALL + i)); + Ok(scsi::dpofua(reply, &header)) + } + /// The table, as `Usb::table` reads it, its sectors fetched by replaying /// the read until it asks for none it has not been given. async fn table(&mut self, geometry: &Geometry) -> Result { @@ -462,7 +486,7 @@ impl<'d, 'u, R: Registers, D: DmaBuffers, C: Host, T> Msc<'d, 'u, R, D, C, T> { let start = block * u64::from(per); let sectors = u64::from(per).min(geometry.sectors() - start) as u32; let len = sectors * geometry.sector_bytes(); - let cdb = Cdb::transfer(false, start as u32, sectors as u16); + let cdb = Cdb::transfer(Io::Read, start as u32, sectors as u16); let read = self.command(&cdb, Data::In { at, len }).await?; for s in 0..u64::from(sectors) { let first = (s as usize) * geometry.sector_bytes() as usize; @@ -496,10 +520,10 @@ impl<'d, 'u, R: Registers, D: DmaBuffers, C: Host, T> Msc<'d, 'u, R, D, C, T> { .await; let Some(work) = next else { return Ok(()) }; let geometry = disks.disks.borrow()[&disk].geometry; - let (ok, status, write) = match work { - Work::Io { write, block, blocks, at, .. } => { - let (ok, status) = self.io(&geometry, write, block, blocks, at).await?; - (ok, status, write) + let (ok, status, io) = match work { + Work::Io { io, block, blocks, at, .. } => { + let (ok, status) = self.io(&geometry, io, block, blocks, at).await?; + (ok, status, Some(io)) } Work::Flush { .. } => { let (ok, status, no_cache) = match scsi::flushed(self.command(&Cdb::SYNCHRONIZE_CACHE, Data::None).await?) { @@ -514,7 +538,7 @@ impl<'d, 'u, R: Registers, D: DmaBuffers, C: Host, T> Msc<'d, 'u, R, D, C, T> { } d.no_cache |= no_cache; } - (ok, status, false) + (ok, status, None) } }; if self.asked || self.dev.leaving.get() { @@ -530,17 +554,19 @@ impl<'d, 'u, R: Registers, D: DmaBuffers, C: Host, T> Msc<'d, 'u, R, D, C, T> { let mut all = disks.disks.borrow_mut(); let d = all.get_mut(&disk).expect("served"); d.work.pop_front(); - if write && ok { + // A write the device forced is on its medium: no flush is owed. + if ok && io == Some(Io::Write { fua: false }) { d.debt.wrote(); } drop(all); + let write = matches!(io, Some(Io::Write { .. })); disks.event(Event::Answered { disk, token: work.token(), write, ok, status }); } } /// One read or write, in as many READ(10)s or WRITE(10)s as it takes. - async fn io(&mut self, geometry: &Geometry, write: bool, block: u64, blocks: u32, at: u64) -> Result<(bool, u32), Dead> { - let mut transfer = match scsi::Transfer::new(block, blocks, write, geometry, toyos_blockring::MAX_REQUEST_BLOCKS) { + async fn io(&mut self, geometry: &Geometry, io: Io, block: u64, blocks: u32, at: u64) -> Result<(bool, u32), Dead> { + let mut transfer = match scsi::Transfer::new(block, blocks, io, geometry, toyos_blockring::MAX_REQUEST_BLOCKS) { Ok(t) => t, Err(past) => { self.dev.usb.say(format_args!("a transfer past the disk's end refused: {past:?}")); @@ -550,7 +576,7 @@ impl<'d, 'u, R: Registers, D: DmaBuffers, C: Host, T> Msc<'d, 'u, R, D, C, T> { while let Some(batch) = transfer.next() { let at = at + batch.offset as u64; let len = batch.bytes as u32; - let data = if write { Data::Out { at, len } } else { Data::In { at, len } }; + let data = if io == Io::Read { Data::In { at, len } } else { Data::Out { at, len } }; let reply = self.command(&batch.cdb, data).await?; match transfer.answered(&batch, reply) { scsi::Moved::Whole => {} diff --git a/system/usbd/xhci/src/stub/devices.rs b/system/usbd/xhci/src/stub/devices.rs index a3b7b998eda..1d5ff661f37 100644 --- a/system/usbd/xhci/src/stub/devices.rs +++ b/system/usbd/xhci/src/stub/devices.rs @@ -129,6 +129,11 @@ pub struct Stick { pub serial: String, /// CBWs it took, by operation code. pub commands: Rc>>, + /// Whether its mode parameters set DPOFUA; a write with FUA is refused + /// where they do not. + pub dpofua: bool, + /// WRITE(10)s it took with FUA. + pub forced: Rc>, state: Bot, sense: (u8, u8, u8), } @@ -142,6 +147,8 @@ impl Stick { pipes: Rc::default(), serial: serial.into(), commands: Rc::default(), + dpofua: false, + forced: Rc::default(), state: Bot::Idle, sense: (0, 0, 0), } @@ -210,7 +217,15 @@ impl Stick { let data = self.disk.borrow()[lba * SECTOR..(lba + count) * SECTOR].to_vec(); reply_in(data, &mut self.sense) } + // Mode parameter header alone: no block descriptor, no page. + 0x1A => reply_in(vec![3, 0, u8::from(self.dpofua) << 4, 0], &mut self.sense), + 0x2A if cdb[1] & 0x08 != 0 && !self.dpofua => { + // ILLEGAL REQUEST, INVALID FIELD IN CDB. + self.sense = (0x05, 0x24, 0x00); + Bot::Status(Self::csw(tag, length as u32, 1)) + } 0x2A => { + self.forced.set(self.forced.get() + u32::from(cdb[1] & 0x08 != 0)); let count = usize::from(u16::from_be_bytes([cdb[7], cdb[8]])); Bot::DataOut { lba: u64::from(be32(2)), len: count * SECTOR, csw: Self::csw(tag, 0, 0) } } diff --git a/system/usbd/xhci/src/tests.rs b/system/usbd/xhci/src/tests.rs index 5033f83d5ed..ba4ba61ac15 100644 --- a/system/usbd/xhci/src/tests.rs +++ b/system/usbd/xhci/src/tests.rs @@ -10,7 +10,7 @@ use toyos_input::wire::Record; use crate::exec::{Dead, Host, Nanos, Tasks}; use crate::hc::{self, Reset}; use crate::input::{Opened, Owner, Sent}; -use crate::msc::{Event, Work}; +use crate::msc::{Event, Io, Work}; use crate::stub::devices::{gpt_disk, Keyboard, Pipes, Stick}; use crate::stub::port::{FakePort, ResetBehaviour}; use crate::stub::{Answers, Dma, Function, Regs, Xhc, LENT_BASE}; @@ -355,7 +355,7 @@ fn the_stop_resets_every_connected_port_then_halts_resets_and_unpowers_the_contr assert!(w.said("collection 0: a Keyboard"), "{:#?}", w.host.lines.borrow()); // The stick's next command goes unanswered, so the stop finds it open. pipes.set(Pipes::SilentIn); - usb.disks.submit(0, Work::Io { write: false, block: 0, blocks: 1, at: LENT_BASE, token: (0, 1) }); + usb.disks.submit(0, Work::Io { io: Io::Read, block: 0, blocks: 1, at: LENT_BASE, token: (0, 1) }); let now = w.host.now.get(); w.run(&usb, &mut tasks, &mut owner, now + 50 * MS).unwrap(); @@ -524,9 +524,9 @@ fn a_disks_work_moves_between_the_stick_and_the_lent_region() { let pattern: Vec = (0..8192u32).map(|i| (i * 7) as u8).collect(); w.xhc().mem.borrow_mut().write(LENT_BASE + 0xF000, &pattern); // Two blocks from 60 KiB into the region: the data crosses 64 KiB. - usb.disks.submit(0, Work::Io { write: true, block: 300, blocks: 2, at: LENT_BASE + 0xF000, token: (1, 1) }); + usb.disks.submit(0, Work::Io { io: Io::Write { fua: false }, block: 300, blocks: 2, at: LENT_BASE + 0xF000, token: (1, 1) }); usb.disks.submit(0, Work::Flush { token: (1, 2) }); - usb.disks.submit(0, Work::Io { write: false, block: 300, blocks: 2, at: LENT_BASE + 0x1_8000, token: (1, 3) }); + usb.disks.submit(0, Work::Io { io: Io::Read, block: 300, blocks: 2, at: LENT_BASE + 0x1_8000, token: (1, 3) }); w.run(&usb, &mut tasks, &mut owner, 700 * MS).unwrap(); let answered: Vec<((u64, u32), bool)> = usb .disks @@ -546,6 +546,49 @@ fn a_disks_work_moves_between_the_stick_and_the_lent_region() { assert_eq!(w.xhc().mem.borrow().faults, 0); } +/// A stick whose mode parameters set DPOFUA is offered as forcing writes, and +/// a write asked with FUA reaches it as a WRITE(10) carrying the bit. One +/// whose do not is offered without. +#[test] +fn a_stick_that_says_dpofua_is_sent_fua_and_one_that_does_not_is_offered_without() { + for dpofua in [true, false] { + let (mut stick, disk, _) = stick("SERIAL1"); + stick.dpofua = dpofua; + let (commands, forced) = (stick.commands.clone(), stick.forced.clone()); + let w = World::new(vec![(FakePort::occupied(QUICK), Some(Box::new(stick)))], 1); + w.xhc().mem.borrow_mut().lend(LENT_BASE, 128 * 1024); + let usb = w.start(BOTH); + let mut tasks = usb.tasks(); + let _end = End(&usb); + let mut owner = Mock::default(); + w.run(&usb, &mut tasks, &mut owner, 600 * MS).unwrap(); + assert!(matches!(usb.disks.take_events()[..], [Event::Arrived { disk: 0, .. }])); + assert!(commands.borrow().contains(&0x1A), "MODE SENSE was never asked"); + assert_eq!(usb.disks.fua(0), dpofua); + if !dpofua { + assert!(w.said("no DPOFUA, so a forced write is a write and a flush"), "{:#?}", w.host.lines.borrow()); + continue; + } + let pattern = [0x5Au8; 4096]; + w.xhc().mem.borrow_mut().write(LENT_BASE, &pattern); + usb.disks.submit(0, Work::Io { io: Io::Write { fua: true }, block: 300, blocks: 1, at: LENT_BASE, token: (1, 1) }); + usb.disks.submit(0, Work::Io { io: Io::Write { fua: false }, block: 301, blocks: 1, at: LENT_BASE, token: (1, 2) }); + w.run(&usb, &mut tasks, &mut owner, 700 * MS).unwrap(); + let answered: Vec = usb + .disks + .take_events() + .into_iter() + .filter_map(|e| match e { + Event::Answered { ok, write: true, .. } => Some(ok), + _ => None, + }) + .collect(); + assert_eq!(answered, [true, true]); + assert_eq!(forced.get(), 1, "only the write asked with FUA carries it"); + assert_eq!(&disk.borrow()[300 * 4096..301 * 4096], &pattern[..]); + } +} + /// Writes that each cross 64 KiB, so their data phase is a TD of two TRBs, /// interleaved with reads, whose bulk-out share is their CBW alone: the /// bulk-out ring laps with TDs at every offset from its Link, and the stub @@ -569,7 +612,8 @@ fn a_td_that_would_cross_the_rings_link_starts_behind_it() { while queued < works && usb.disks.has_room(0) { let write = queued % 2 == 0; let (block, blocks, at) = if write { (300, 2, LENT_BASE + 0xF000) } else { (310, 1, LENT_BASE + 0x1_8000) }; - usb.disks.submit(0, Work::Io { write, block, blocks, at, token: (1, queued) }); + let io = if write { Io::Write { fua: false } } else { Io::Read }; + usb.disks.submit(0, Work::Io { io, block, blocks, at, token: (1, queued) }); queued += 1; } until += 50 * MS; @@ -787,7 +831,7 @@ fn a_stick_back_from_its_port_reset_keeps_its_disk_and_its_work() { assert!(matches!(answered(&usb).1[..], [Event::Arrived { disk: 0, .. }])); pipes.set(Pipes::SilentOut); let queued = w.host.now.get(); - usb.disks.submit(0, Work::Io { write: true, block: 10, blocks: 1, at: LENT_BASE, token: (1, 1) }); + usb.disks.submit(0, Work::Io { io: Io::Write { fua: false }, block: 10, blocks: 1, at: LENT_BASE, token: (1, 1) }); w.run(&usb, &mut tasks, &mut owner, queued + crate::msc::COMMAND_NS + MS / 2).unwrap(); assert!(w.said("the ladder climbs to its port reset"), "{:#?}", w.host.lines.borrow()); pipes.set(Pipes::Answer); @@ -812,7 +856,7 @@ fn a_different_stick_after_the_port_reset_ends_the_disk_that_left() { answered(&usb); pipes.set(Pipes::SilentOut); let queued = w.host.now.get(); - usb.disks.submit(0, Work::Io { write: true, block: 10, blocks: 1, at: LENT_BASE, token: (1, 1) }); + usb.disks.submit(0, Work::Io { io: Io::Write { fua: false }, block: 10, blocks: 1, at: LENT_BASE, token: (1, 1) }); w.run(&usb, &mut tasks, &mut owner, queued + crate::msc::COMMAND_NS + MS / 2).unwrap(); let (other, other_disk, _) = stick("OTHER"); w.xhc().devices[0] = Some(Box::new(other)); @@ -899,8 +943,8 @@ fn a_stick_that_answers_late_once_is_served_again() { answered(&usb); pipes.set(Pipes::SilentIn); let queued = w.host.now.get(); - usb.disks.submit(0, Work::Io { write: true, block: 10, blocks: 1, at: LENT_BASE, token: (1, 1) }); - usb.disks.submit(0, Work::Io { write: false, block: 10, blocks: 1, at: LENT_BASE + 4096, token: (1, 2) }); + usb.disks.submit(0, Work::Io { io: Io::Write { fua: false }, block: 10, blocks: 1, at: LENT_BASE, token: (1, 1) }); + usb.disks.submit(0, Work::Io { io: Io::Read, block: 10, blocks: 1, at: LENT_BASE + 4096, token: (1, 2) }); w.run(&usb, &mut tasks, &mut owner, queued + crate::msc::COMMAND_NS + 2_000 * MS).unwrap(); pipes.set(Pipes::Answer); w.run(&usb, &mut tasks, &mut owner, queued + 6 * crate::msc::COMMAND_NS).unwrap(); From b8e7fe9214fca25f50ee61dd9ce0b301d931e383 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 11 Nov 2026 01:09:16 +0200 Subject: [PATCH 3/4] blockring: the header says which completion is durable The protocol's header still said a completion never means durable. A write asked with `fua` is the exception the forced-write change made. A comment alone. Co-Authored-By: Claude Fable 5.2 --- lib/blockring/src/lib.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/blockring/src/lib.rs b/lib/blockring/src/lib.rs index a8d94ed35e4..9e7e331a3ae 100644 --- a/lib/blockring/src/lib.rs +++ b/lib/blockring/src/lib.rs @@ -15,9 +15,11 @@ //! the open's answer ([`wire::OPENED_HANDLES`]); each end's bell is also what //! tells it the other has gone: its waker's end is the session's end. //! -//! **A completion means the device took the request, never that it is -//! durable.** Durability is a flush's answer, and a flush answers for its own -//! writer ([`server`], over `toyos-blockhold`): a flush whose writer's writes +//! **A completion means the device took the request, and for a write only +//! one asked with `fua` that it is durable** ([`entry::Op::Write`]), on a +//! session whose open offered it. Durability is otherwise a flush's answer, +//! and a flush answers for its own writer ([`server`], over +//! `toyos-blockhold`): a flush whose writer's writes //! the disk lost since they were acknowledged is answered [`Status::Lost`]. //! The client keeps every acknowledged write no flush has covered and issues //! it again after a loss — a `Lost` flush, or a session that ended — before From fea712a167f2b89356bfacbd5e724e5029cd1d77 Mon Sep 17 00:00:00 2001 From: japabu Date: Wed, 11 Nov 2026 01:37:05 +0200 Subject: [PATCH 4/4] Review r1 of #1202: the client refuses a forced write over a kept one 1. The rule that a forced write never goes over a kept write was prose and a panic in the crash model. A violation is silent: after a loss the kept bytes are issued again over the forced ones and a later flush calls them durable. The block client, which alone holds the kept set, now panics when a write with `fua` is next and a kept write overlaps it. The journal's comment gave an incomplete reason for holding the rule: a refused durable write can leave its blocks written and unflushed, and it is each attempt's own flush that covers them. A device double that refuses the first forced write holds that flush inside the retry: Flush, Durable, Flush, Durable. 2. A reconnect compared the whole open's answer, `fua` included, so a device that answered MODE SENSE differently after a driver restart lost its volume. A reconnect is held to the partition's length and GUID and adopts the new answer for forced writes. Both comments that claimed as much are now true, and the refusal of a forced write the new server does not offer is reached by a test. 3. The USB record's forced-write gap has its own section and its own exit, and says what a stick that sets DPOFUA and then refuses or ignores FUA does. Suggestions taken: a mode header whose MODE DATA LENGTH ends inside it is not believed; the MODE SENSE comment says why all pages and 192 bytes; `the_log_lands_on_the_boot_stick` prints the stick's bring-up line; `write_durable` promises nothing else durable. Co-Authored-By: Claude Fable 5.2 --- ...storage-since-the-benches-boot-off-nvme.md | 33 +++++++++++--- lib/bcachefs/src/engine/mount/mod.rs | 9 ++-- lib/bcachefs/src/engine/tests/crash.rs | 36 +++++++++++++++- lib/blockdev/src/lib.rs | 8 ++-- lib/blockring/src/client.rs | 43 ++++++++++++++++++- lib/scsi/src/lib.rs | 14 ++++-- system/diskserver/src/disk.rs | 4 +- system/diskserver/src/disk/tests.rs | 36 +++++++++++++++- system/diskserver/src/session.rs | 38 ++++++++++++++-- tests/toyos.rs | 3 ++ 10 files changed, 197 insertions(+), 27 deletions(-) diff --git a/issues/no-bench-row-drives-usb-storage-since-the-benches-boot-off-nvme.md b/issues/no-bench-row-drives-usb-storage-since-the-benches-boot-off-nvme.md index 7219f7502e7..c1ec5848c50 100644 --- a/issues/no-bench-row-drives-usb-storage-since-the-benches-boot-off-nvme.md +++ b/issues/no-bench-row-drives-usb-storage-since-the-benches-boot-off-nvme.md @@ -31,11 +31,6 @@ the boot stick lost it: - `amdvi_translates_the_boot_controller` and `amdvi_remaps_the_boot_controller` judge the NVMe controller where they judged the stick's xHCI. -- Whether a stick's mode parameters set DPOFUA, and whether it then takes a - WRITE(10) with FUA, is read on no bench: usbd asks MODE SENSE as a disk - first comes up and forces a durable write only where the stick says so - (`system/usbd/xhci/src/msc.rs`), judged against a host model alone. - QEMU's machines still boot off a stick (`the_log_lands_on_the_boot_stick`). No row that writes the scratch disk is built. The owner declined the @@ -43,9 +38,35 @@ comparison of ToyOS's resets with Linux's restart (`issues/a-reset-can-leave-a-usb-stick-hung-until-its-power-is-removed.md`), and that does not discharge this exit. +**A stick's forced writes have never met a real stick.** This is not a +judgement the boot stick lost: it is new with the forced write. usbd asks +MODE SENSE(6) as a disk first comes up, reads DPOFUA from its header, and +sends a durable write as WRITE(10) with FUA only where the stick set it +(`system/usbd/xhci/src/msc.rs`, `lib/scsi`). A host model and QEMU's stick +are all that have answered either command. What a real stick can do that +they do not: + +- **Break on the MODE SENSE.** The command climbs the recovery ladder like + any broken command, and since it runs at every first bring-up, a stick + that hangs on it is never served. +- **Set DPOFUA and refuse a write with FUA.** The write is answered not + done; the journal issues its entry again twice more and then the mount stops + writing. Nothing acknowledged is lost, and the volume takes no more + writes until it is mounted again. +- **Set DPOFUA and ignore FUA.** The write is answered done from the + stick's cache. The commit is acknowledged without being on the medium, a + power cut loses it, and usbd counts no flush owed for it, so a stick that + leaves then is counted as having lost nothing. Nothing on the host can + see this. + **Exit**: a metal row drives a USB stick that is not the boot disk — the scratch disk (`issues/the-scratch-disk-adopt-scratch-makes-is-not-served.md`) — and judges usbd serving it, a reset under a write to it, and a tone beside -that write. +that write. And for forced writes: on a real stick, usbd's bring-up line +shows the MODE SENSE answered with no Bulk-Only break before it; and on a +real device that sets DPOFUA, carrying a bcachefs volume, a commit's +WRITE(10) with FUA is answered done. Whether such a device honours FUA is +shown only by cutting its power after an acknowledged commit; that is +evidence to record if it is ever taken, and no part of this exit. **Owner**: the orchestrator, which runs metal. diff --git a/lib/bcachefs/src/engine/mount/mod.rs b/lib/bcachefs/src/engine/mount/mod.rs index 07716a7a0cc..65059bbaa09 100644 --- a/lib/bcachefs/src/engine/mount/mod.rs +++ b/lib/bcachefs/src/engine/mount/mod.rs @@ -1444,10 +1444,11 @@ impl Mount { /// flush and itself durable as it is written (A2.12), so each is durable /// once written and so is every node write that completed before it. An /// entry naming itself has its header sector written last, once the rest - /// is durable. The journal's blocks are written no other way, so a - /// durable write of one is never over a write no flush has covered. A - /// failed write is reissued with the same bytes, or the mount stops - /// writing. + /// is durable. A durable write is never over a write no flush has + /// covered: an attempt that failed may have left the entry's blocks + /// written and unflushed, and each attempt's own flush covers them + /// before it writes again. A failed write is reissued with the same + /// bytes, or the mount stops writing. async fn write_entries(&self, upto: u64) -> Result<()> { self.journal_queue.wait(|| (!self.journal_held.get()).then(|| self.journal_held.set(true))).await; let out = async { diff --git a/lib/bcachefs/src/engine/tests/crash.rs b/lib/bcachefs/src/engine/tests/crash.rs index 0e3445a4730..ea317378e25 100644 --- a/lib/bcachefs/src/engine/tests/crash.rs +++ b/lib/bcachefs/src/engine/tests/crash.rs @@ -259,10 +259,12 @@ enum Asked { } /// A device that records what it is asked, and takes a durable write -/// forced, or, with `forces` off, as a write it leaves in its cache. +/// forced, or, with `forces` off, as a write it leaves in its cache. It +/// refuses its next `refusing` durable writes, each left in its cache. struct Recorded<'a> { model: &'a Model, forces: bool, + refusing: Cell, asked: core::cell::RefCell>, } @@ -287,6 +289,11 @@ impl BlockDev for Recorded<'_> { async fn write_durable(&self, lba: u64, data: &[Block]) -> core::result::Result<(), toyos_blockdev::Error> { self.asked.borrow_mut().push(Asked::Durable(lba)); + if self.refusing.get() > 0 { + self.refusing.set(self.refusing.get() - 1); + self.model.write(lba, data).await?; + return Err(toyos_blockdev::Error::Device); + } match self.forces { true => self.model.write_durable(lba, data).await, false => self.model.write(lba, data).await, @@ -303,7 +310,7 @@ impl BlockDev for Recorded<'_> { fn a_commit_is_one_flush_and_its_entry_forced_and_is_lost_where_the_entry_is_not() { for forces in [true, false] { let model = Model::new(formatted(64 << 20, None).into_inner(), 512); - let dev = Recorded { model: &model, forces, asked: core::cell::RefCell::default() }; + let dev = Recorded { model: &model, forces, refusing: Cell::new(0), asked: core::cell::RefCell::default() }; let mount = block_on_ready(Mount::open(&dev, MEMORY, random())).unwrap(); let root = mount.root(); block_on_ready(async { @@ -345,6 +352,31 @@ fn a_commit_is_one_flush_and_its_entry_forced_and_is_lost_where_the_entry_is_not } } +/// A durable write the device refused may be in its cache, so the attempt +/// after it flushes before it writes the entry again: the model refuses a +/// forced write over what no flush has covered, and the commit is asked as +/// flush, entry, flush, entry. +#[test] +fn an_entry_written_again_after_a_refusal_follows_a_flush_of_its_own() { + let model = Model::new(formatted(64 << 20, None).into_inner(), 512); + let dev = Recorded { model: &model, forces: true, refusing: Cell::new(0), asked: core::cell::RefCell::default() }; + let mount = block_on_ready(Mount::open(&dev, MEMORY, random())).unwrap(); + block_on_ready(async { + mount.create(mount.root(), b"first", New::File, OWNER, NOW).await.unwrap(); + mount.commit().await.unwrap(); + dev.asked.borrow_mut().clear(); + dev.refusing.set(1); + mount.create(mount.root(), b"second", New::File, OWNER, NOW).await.unwrap(); + mount.commit().await.unwrap(); + }); + let asked = dev.asked.take(); + let [Asked::Flush, Asked::Durable(first), Asked::Flush, Asked::Durable(again)] = asked[..] else { + panic!("a commit whose entry was refused once asked {asked:?}") + }; + assert_eq!(first, again); + block_on_ready(mount.unmount()).unwrap(); +} + fn creates(n: usize) -> Vec { (0..n).map(|i| Op::Create(format!("c{i}").into_bytes())).collect() } diff --git a/lib/blockdev/src/lib.rs b/lib/blockdev/src/lib.rs index 6b646e84edb..0f38365ade3 100644 --- a/lib/blockdev/src/lib.rs +++ b/lib/blockdev/src/lib.rs @@ -72,10 +72,10 @@ pub trait BlockDev { /// Every write that completed before this was asked, on the medium. async fn flush(&self) -> Result<(), Error>; - /// `data` at `lba`, on the medium once complete, and nothing else made - /// durable by it. It must not overlap a write of this device's no flush - /// has covered. A device that cannot force one write to its medium - /// writes it and then flushes, which is this. + /// `data` at `lba`, on the medium once complete; nothing else is promised + /// durable. It must not overlap a write of this device's no flush has + /// covered. A device that cannot force one write to its medium writes it + /// and then flushes, which is this. async fn write_durable(&self, lba: u64, data: &[Block]) -> Result<(), Error> { self.write(lba, data).await?; self.flush().await diff --git a/lib/blockring/src/client.rs b/lib/blockring/src/client.rs index a749500d10c..67263e9d8fd 100644 --- a/lib/blockring/src/client.rs +++ b/lib/blockring/src/client.rs @@ -29,7 +29,9 @@ //! kept and never issued again: [`Outcome::Done`] for it is durable. The //! caller asks one only of a session that offers it //! ([`crate::wire::Opened::fua`]), and never over a write of its own that is -//! kept: issued again after a loss, the kept write would replace it. +//! kept: issued again after a loss, the kept write would replace it, and a +//! later flush would call the older bytes durable. One that is, is the +//! caller's bug, and [`Client::next_request`] panics rather than send it. //! //! **What it keeps is bounded.** Once the acknowledged writes it keeps reach //! the limit it was made with, the caller's next write waits while the client @@ -263,6 +265,9 @@ impl Client { /// `D` on the wire, what is next must wait for writes being issued again /// or for the client's own flush, or `place` has no room for it, which a /// request on the wire answered makes. + /// + /// # Panics + /// When a write with `fua` is next and a kept write overlaps it. pub fn next_request(&mut self, mut place: impl FnMut(Placing<'_, P>) -> Option) -> Option { if !self.up || self.wire.values().count() == D { return None; @@ -312,6 +317,16 @@ impl Client { Op::Read { run: place(Placing::Read { ticket, blocks })?, lba } } Ask::Write { lba, blocks, payload, fua } => { + // Nothing is being issued again here, so every kept write is + // in `acked`. + let over = self.acked.iter().find(|a| a.lba < lba + u64::from(*blocks) && *lba < a.lba + u64::from(a.blocks)); + if let (true, Some(kept)) = (*fua, over) { + panic!( + "blockring: a forced write of {blocks} blocks at {lba} over the kept write of {} at {}, \ + which no flush has covered", + kept.blocks, kept.lba + ); + } let fua = *fua && cfg!(not(feature = "mutate-fua-goes-out-unforced")); Op::Write { run: place(Placing::Write { payload, blocks: *blocks })?, lba: *lba, fua } } @@ -681,6 +696,32 @@ mod tests { assert_eq!(client.reissued(), 0); } + /// A forced write over a kept write is refused before it goes out, and + /// goes once a flush has covered the kept one. + #[test] + fn a_forced_write_goes_over_a_write_only_once_a_flush_covered_it() { + let kept = |client: &mut C| { + client.submit(1, write(1, 4, 2)); + let w = next(client).unwrap(); + answer(client, w, Status::Ok); + }; + let mut client = up(ROOMY); + kept(&mut client); + client.submit(2, Ask::Flush); + let f = next(&mut client).unwrap(); + answer(&mut client, f, Status::Ok); + client.submit(3, Ask::Write { lba: 5, blocks: 2, payload: 3, fua: true }); + assert!(matches!(next(&mut client).unwrap().op, Op::Write { fua: true, .. })); + + let mut client = up(ROOMY); + kept(&mut client); + client.submit(3, Ask::Write { lba: 5, blocks: 2, payload: 3, fua: true }); + let refused = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| next(&mut client))); + let why = refused.expect_err("a forced write went out over a kept write"); + let why = why.downcast_ref::().expect("a message"); + assert!(why.contains("a forced write of 2 blocks at 5 over the kept write of 2 at 4"), "{why}"); + } + /// A write acknowledged, then a flush answered `Lost`: the write goes out /// again with its own payload before the flush is asked again, and the /// flush's caller hears `Durable` only from the second. diff --git a/lib/scsi/src/lib.rs b/lib/scsi/src/lib.rs index 96b85f5efe7..c56ec027081 100644 --- a/lib/scsi/src/lib.rs +++ b/lib/scsi/src/lib.rs @@ -65,8 +65,10 @@ impl Cdb { pub const SYNCHRONIZE_CACHE: Self = Self::of([0x35, 0, 0, 0, 0, 0, 0, 0, 0, 0], false); /// Every mode page (SPC-4, MODE SENSE(6): page code 3Fh), of which only - /// the mode parameter header is read. [`MODE_BYTES`] allocated, so a - /// device with pages to send has room to send them. + /// the mode parameter header is read. The standard lets the allocation + /// be the header's four bytes; all pages with [`MODE_BYTES`] allocated is + /// asked instead as the form a device is likeliest to have been sent + /// before, which is a choice and no requirement. pub const MODE_SENSE: Self = Self::of([0x1A, 0, 0x3F, 0, MODE_BYTES as u8, 0], true); /// READ(10) or WRITE(10) of `sectors` at `lba`. @@ -104,9 +106,11 @@ pub const MODE_BYTES: usize = 192; /// PARAMETER field for direct access block devices"), which is byte 2 of /// MODE SENSE(6)'s mode parameter header (SPC-4, mode parameter header /// formats). `header` is the first bytes of what `reply` delivered. A device -/// that refuses MODE SENSE, or answers less than the header, takes none. +/// that refuses MODE SENSE, answers less than the header, or states a MODE +/// DATA LENGTH (byte 0, the bytes after itself) that ends before the +/// header does, takes none. pub fn dpofua(reply: Reply, header: &[u8; 4]) -> bool { - matches!(reply, Reply::Ok { delivered } if delivered >= 4) && header[2] & (1 << 4) != 0 + matches!(reply, Reply::Ok { delivered } if delivered >= 4) && header[0] >= 3 && header[2] & (1 << 4) != 0 } /// Bytes a REQUEST SENSE asks for. @@ -654,6 +658,8 @@ mod tests { assert!(!dpofua(whole, &[3, 0, 0x80, 0]), "write protect is not DPOFUA"); assert!(!dpofua(whole, &[3, 0, 0, 0])); assert!(!dpofua(Reply::Ok { delivered: 3 }, &[3, 0, 0x10, 0]), "a header that did not arrive"); + assert!(!dpofua(whole, &[2, 0, 0x10, 0]), "mode data that ends inside the header"); + assert!(!dpofua(whole, &[0, 0, 0x10, 0]), "no mode data"); let illegal = Sense { key: 0x05, asc: 0x20, ..Sense::NONE }; assert!(!dpofua(Reply::Refused(illegal), &[3, 0, 0x10, 0])); assert!(!dpofua(Reply::Broken, &[3, 0, 0x10, 0])); diff --git a/system/diskserver/src/disk.rs b/system/diskserver/src/disk.rs index 01d1ffc03cc..d8773a6ee57 100644 --- a/system/diskserver/src/disk.rs +++ b/system/diskserver/src/disk.rs @@ -194,8 +194,8 @@ impl Served { /// `fua` where the session offers it, and otherwise written and then /// flushed. Not over a write no flush has covered. pub async fn write_shared_durable(&self, lba: u64, blocks: &[SharedBlock]) -> Result<(), Error> { - // Asked of the session each time: a reconnect may have found the - // disk behind another driver. + // Asked of the session each time: a reconnect adopts what its server + // says of the device. if self.link.borrow().fua() { return self.write_requests(lba, blocks, true).await; } diff --git a/system/diskserver/src/disk/tests.rs b/system/diskserver/src/disk/tests.rs index 25796f057cf..51f0a995620 100644 --- a/system/diskserver/src/disk/tests.rs +++ b/system/diskserver/src/disk/tests.rs @@ -37,8 +37,10 @@ struct Script { reconnects: VecDeque>, reconnected: u32, read_only: bool, - /// What each session's open says of its device. + /// What the session's open says of its device, and what each + /// reconnect's says in turn; as before once they run out. fua: bool, + fua_after: VecDeque, } impl Script { @@ -55,6 +57,7 @@ impl Script { reconnected: 0, read_only: false, fua: false, + fua_after: VecDeque::new(), } } @@ -131,6 +134,9 @@ impl Link for Script { self.reconnected += 1; let answer = self.reconnects.pop_front().unwrap_or(Ok(())); self.up = answer.is_ok(); + if self.up { + self.fua = self.fua_after.pop_front().unwrap_or(self.fua); + } answer } } @@ -200,6 +206,34 @@ fn a_forced_write_on_the_wire_at_a_session_end_is_asked_again_forced() { assert_eq!(disk.link.borrow().log, [forced.clone(), forced]); } +/// A reconnect's answer for the device is the one that holds from then on, +/// either way. A forced write on the wire when its session ended is asked +/// again as it was; the server that no longer offers one refuses it, and the +/// durable write after that is a write and a flush. The other way, the +/// durable write after the reconnect is forced. +#[test] +fn a_reconnect_s_answer_for_forced_writes_is_adopted_both_ways() { + let mut script = Script::new(4); + script.fua = true; + script.ends.push_back(0); + script.fua_after.push_back(false); + let disk = Served::new(script); + assert_eq!(now(&disk, disk.write_durable(1, &blocks(9, 1))), Err(Error::Device)); + now(&disk, disk.write_durable(1, &blocks(9, 1))).unwrap(); + let write = |fua| Asked::Write { lba: 1, data: vec![9; BLOCK], fua }; + assert_eq!(disk.link.borrow().log, [write(true), write(true), write(false), Asked::Flush]); + assert!(disk.link.borrow().disk[BLOCK..2 * BLOCK].iter().all(|&b| b == 9)); + + let mut script = Script::new(4); + script.ends.push_back(0); + script.fua_after.push_back(true); + let disk = Served::new(script); + now(&disk, disk.write(2, &blocks(7, 1))).unwrap(); + now(&disk, disk.write_durable(1, &blocks(9, 1))).unwrap(); + let plain = Asked::Write { lba: 2, data: vec![7; BLOCK], fua: false }; + assert_eq!(disk.link.borrow().log, [plain.clone(), plain, write(true)]); +} + /// A request refused at every session end is asked once more than /// [`MAX_RECONNECTS`] allows again, and then the disk is gone to it. #[test] diff --git a/system/diskserver/src/session.rs b/system/diskserver/src/session.rs index 38e4f57896e..5dd4b71f2d7 100644 --- a/system/diskserver/src/session.rs +++ b/system/diskserver/src/session.rs @@ -189,8 +189,7 @@ impl Session { } /// Whether the session's device forces a write asked with `fua` onto its - /// medium, as this session's open was answered: a reconnect may answer - /// otherwise. + /// medium, as the open or the last reconnect was answered. pub fn fua(&self) -> bool { self.opened.fua() } @@ -372,6 +371,9 @@ impl Session { /// Open the same partition over the same region through the same name, /// after the service ended; writes the client holds go out again first. + /// The partition must answer as it did; whether its device forces writes + /// is taken as the new server says it, since a driver asks its device + /// again each time it comes up. /// /// Blocks until the service answers, within [`wire::ANSWER_BOUND_NS`]: a /// name whose server is being restarted queues the connection until the @@ -382,9 +384,10 @@ impl Session { // two indices at zero, as it will set its own. self.rings = layout::client(self.region.words()); let (bells, opened) = handshake(&self.names, &self.service, self.guid, &self.region)?; - if opened != self.opened { + if !same_partition(&opened, &self.opened) { return Err(Error::Protocol); } + self.opened = opened; self.bells = Some(bells); self.client.session_started(); self.pump(); @@ -392,6 +395,12 @@ impl Session { } } +/// Whether two opens answered for one partition: its length and its GUID, +/// and not what its device does with a forced write. +fn same_partition(a: &Opened, b: &Opened) -> bool { + (a.blocks(), a.unique()) == (b.blocks(), b.unique()) +} + fn transfer_blocks(blocks: usize) -> u32 { assert!((1..=MAX_REQUEST_BLOCKS as usize).contains(&blocks), "diskserver: a transfer of {blocks} blocks"); blocks as u32 @@ -462,3 +471,26 @@ pub fn list(names: &Namespace, service: &str) -> Result, Error _ => Err(Error::Protocol), } } + +#[cfg(test)] +mod tests { + use super::*; + + fn opened(blocks: u64, unique: u8, fua: bool) -> Opened { + let mut bytes = Vec::from(blocks.to_le_bytes()); + bytes.extend([unique; wire::GUID_BYTES]); + bytes.push(u8::from(fua)); + Opened::decode(&bytes).expect("an open's answer") + } + + /// A reconnect is refused for another partition and for another length, + /// and never for what the device does with a forced write. + #[test] + fn a_reconnect_is_held_to_the_partition_and_not_to_forced_writes() { + let first = opened(100, 7, false); + assert!(same_partition(&opened(100, 7, true), &first)); + assert!(same_partition(&first, &opened(100, 7, true))); + assert!(!same_partition(&opened(101, 7, false), &first)); + assert!(!same_partition(&opened(100, 8, false), &first)); + } +} diff --git a/tests/toyos.rs b/tests/toyos.rs index ba80b0e8e90..a823dab9dcb 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -3321,6 +3321,9 @@ fn the_log_lands_on_the_boot_stick(test_config: &Path) -> Result<(), String> { let mut qemu = QemuInstance::boot_with_options(test_config, &[], &[], options); let mut console = qemu.boot_log().to_string(); qemu::await_marker(&mut qemu, &mut console, ": serving it as disk 0", "usbd to serve the boot stick")?; + // Which way the stick takes a forced write, as its MODE SENSE answered. + let served = console.lines().find(|l| l.contains(": serving it as disk 0")).unwrap_or_default(); + eprintln!(" [usbd] {}", served.trim()); qemu::await_marker(&mut qemu, &mut console, toyos_logstream::LOG_OPENED, "logkeeper to open the log")?; // The stop is asked for under load: the stick is being written when the // power button is pressed, as the metal row's deadline finds it.