diff --git a/issues/a-read-and-a-write-of-a-paged-file-can-tear-across-pages.md b/issues/a-read-and-a-write-of-a-paged-file-can-tear-across-pages.md deleted file mode 100644 index a8b6236d61..0000000000 --- a/issues/a-read-and-a-write-of-a-paged-file-can-tear-across-pages.md +++ /dev/null @@ -1,23 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-31 ---- - -# A read and a write of a paged file can tear across pages - -POSIX XSH 2.9.7 makes a `read` and a `write` of a regular file atomic with -respect to each other. A holder's `SYS_PAGED_READ` copies a pinned frame at a -time (`kernel/src/object/paged.rs`), and the file server's write lands page -by page under write pins, outside the object's lock. A read concurrent with a -write of several pages can return some pages before the write and some after -it, and a page can be torn at the bytes being copied. - -This is the contract memory stage 7a states (writes are visible at once, with -no atomicity beyond the hardware's), and it is not what POSIX asks. - -**Exit**: a read and a write of one file are atomic with respect to each -other, or each is atomic per call by a rule the file server and the kernel -state together, with a guest test that reds on a torn read. - -**Owner**: the storage track (`issues/storage-is-layers-and-a-role-is-a-filesystem.md`). diff --git a/issues/a-refused-carried-file-costs-every-other-files-fsync-or-leaks-its-blocks.md b/issues/a-refused-carried-file-costs-every-other-files-fsync-or-leaks-its-blocks.md new file mode 100644 index 0000000000..d1779ab1bb --- /dev/null +++ b/issues/a-refused-carried-file-costs-every-other-files-fsync-or-leaks-its-blocks.md @@ -0,0 +1,37 @@ +--- +status: open +kind: defect +opened: 2026-11-01 +--- + +# A carried file that is refused costs every other file's fsync, or leaks its blocks + +An fsync on DATA commits its own file and every file the commit carries +(`Volume::carried`): one whose entry was written, or whose reserve took +blocks, since the last commit (`system/fileserver/src/pages.rs`, +`Files::fsync`; `system/fileserver/src/data.rs`, `DataVolume::commit`). A +refusal on a carried file is paid by the files beside it: + +- **Its pages refused.** `Files::fsync` refuses the commit when a carried + file's write-back is refused, since its entry would name blocks without + its bytes. While that write-back stays refused, every fsync of every + file on the volume is refused with it. +- **Its entry refused.** `DataVolume::commit` says a carried entry whose + write was refused, and commits anyway. Where the file took blocks since + the last commit, the commit makes the allocator's take durable with no + entry naming them, and after a stop those blocks are leaked: not readable + by any file, and not free. This is not a disclosure. + +A file carried only by blocks taken (its entry unwritten since the last +commit) could be left out of the commit at the cost of that leak, rather +than refusing everyone's fsync. + +Found by review of #1141; no test drives either refusal into another file's +fsync or a stop. + +**Exit**: a refusal on one carried file costs only that file: an fsync of +another lands whole, and after a stop the volume holds as free every block +no entry names. A host test refuses one carried file's write-back, and +another's entry, fsyncs a third, stops, and counts the free blocks. + +**Owner**: the storage track (`issues/storage-is-layers-and-a-role-is-a-filesystem.md`). diff --git a/issues/a-sync-commits-the-entry-of-a-file-whose-pages-were-refused.md b/issues/a-sync-commits-the-entry-of-a-file-whose-pages-were-refused.md new file mode 100644 index 0000000000..bcca2298d2 --- /dev/null +++ b/issues/a-sync-commits-the-entry-of-a-file-whose-pages-were-refused.md @@ -0,0 +1,29 @@ +--- +status: open +kind: defect +opened: 2026-11-01 +--- + +# A sync commits the entry of a file whose pages the volume refused + +`Files::sync` (`system/fileserver/src/pages.rs`) writes every dirty file's +pages back and then calls the volume's `sync` even where a file's write-back +was refused. DATA's `sync` writes every open file's entry, that file's +among them, so the commit makes durable an entry whose new extents name +blocks that never received its pages. In the same boot the file still reads +right: its pages stay dirty in its object and the next sync tries them +again. After a stop before that, it reads what those blocks last held, +which may be a deleted file's bytes +(`issues/a-reallocated-extent-on-data-keeps-the-deleted-files-bytes.md`). + +An fsync does not: a carried file whose pages are refused refuses the +commit (`Files::fsync`). + +Found by reading, while making an fsync commit one file; no test drives a +refused write-back into a stop. + +**Exit**: a sync's commit carries no entry of a file whose pages were +refused, with a host test that refuses one file's write-back, stops the +volume after the sync and reads that file as its last commit left it. + +**Owner**: the storage track (`issues/storage-is-layers-and-a-role-is-a-filesystem.md`). diff --git a/issues/an-fsync-of-a-served-file-syncs-its-whole-volume.md b/issues/an-fsync-of-a-served-file-syncs-its-whole-volume.md index e955882008..c47c615840 100644 --- a/issues/an-fsync-of-a-served-file-syncs-its-whole-volume.md +++ b/issues/an-fsync-of-a-served-file-syncs-its-whole-volume.md @@ -6,10 +6,14 @@ opened: 2026-09-27 # An fsync of a served file syncs its whole volume -The file server answers `FSYNC` by syncing the volume, every open file's +**Built, unmeasured.** `FSYNC` now commits its own file and what the +volume's commit carries (`Files::fsync`, `Volume::carried`), and no longer +syncs the volume. What follows is how it was. + +The file server answered `FSYNC` by syncing the volume, every open file's entry and every dirty block of the cache (`system/fileserver/src/main.rs`, -`FSYNC`, which calls `sync` with the file's node and syncs the whole volume). So an -`fsync` or `File::sync_all` of one file on `/home` syncs all of DATA, and a +`FSYNC`, which called `sync` with the file's node and synced the whole volume). So an +`fsync` or `File::sync_all` of one file on `/home` synced all of DATA, and a 4 KiB create and fsync cost 3.5–3.6 ms p50 on QEMU TCG against the kernel's 1.0–1.3 ms, measured on the branch that moved DATA to the file server. @@ -20,3 +24,5 @@ for no durability. **Exit**: `FSYNC` makes durable the file it names and what that file's entry depends on, with a guest measurement of one file's fsync beside the volume's. +What is owed is that measurement: osbench's +`persist.write_4k_fsync_beside_1m`, base against head on metal. diff --git a/kernel/pure/paged.rs b/kernel/pure/paged.rs index d3194de8fe..b6129f9d22 100644 --- a/kernel/pure/paged.rs +++ b/kernel/pure/paged.rs @@ -17,6 +17,12 @@ //! it in the same step, so the size never covers bytes not yet in the object; //! a shrink removes every page past it, dirty or not. //! +//! **A holder's read is atomic against every write** (POSIX XSH 2.9.7): it +//! begins only while no write is under way ([`Object::read_begin`]), and its +//! bytes count only where no write has begun since ([`Object::unchanged`]). +//! A write is one call, or, where a call does not take all it was given, every +//! call until [`Object::end_write`]: the pager's whole write. +//! //! What a removed frame owes (the last pin's free, a zeroed tail) is the //! caller's: every removal answers its frames. @@ -204,15 +210,25 @@ struct Slot { } /// One object's part of the table: its size, whether its pager ended, the -/// list of its slots. +/// list of its slots, and its writes under way. pub struct Object { id: u32, size: u64, gone: bool, head: u32, pages: usize, + /// Moved at every write's begin and end, and at every resize. + version: u64, + /// Writes under way: each call's, and the held one. + writing: u32, + /// A call that did not take all it was given left its write under way. + held: bool, } +/// What a holder's read began at ([`Object::read_begin`]). +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Version(u64); + impl Object { pub fn size(&self) -> u64 { self.size @@ -230,6 +246,43 @@ impl Object { fn page_end(&self) -> u64 { self.size.div_ceil(PAGE) } + + /// Where a holder's read begins: `None` while a write is under way. + pub fn read_begin(&self) -> Option { + (self.writing == 0).then_some(Version(self.version)) + } + + /// Whether no write or resize has begun since the read began at `v`. + pub fn unchanged(&self, v: Version) -> bool { + self.version == v.0 + } + + /// A write call begins, before its first byte lands. + pub fn write_begin(&mut self) { + self.writing = self.writing.checked_add(1).expect("an object's writes overflowed"); + self.version += 1; + } + + /// A write call ends, after its last byte landed. One that did not take + /// all it was given leaves the write under way until + /// [`end_write`](Self::end_write), unless an earlier call's already is. + pub fn write_end(&mut self, whole: bool) { + if !whole && !self.held { + self.held = true; + return; + } + self.writing = self.writing.checked_sub(1).expect("a write ended that never began"); + self.version += 1; + } + + /// The pager ends the write a short call left under way. + pub fn end_write(&mut self) -> Result<(), Refused> { + if !core::mem::take(&mut self.held) { + return Err(Refused::InvalidArgument); + } + self.write_end(true); + Ok(()) + } } /// A pager's table: `capacity` slots and a hash of them, both made here. @@ -307,7 +360,7 @@ impl Pages { /// [`release`](Self::release) takes every page it held. pub fn object(&mut self, id: u32, size: u64) -> Object { assert!(size <= LAST_END, "an object of {size} bytes past the last page"); - Object { id, size, gone: false, head: NONE, pages: 0 } + Object { id, size, gone: false, head: NONE, pages: 0, version: 0, writing: 0, held: false } } fn hash(&self, object: u32, page: u64) -> usize { @@ -617,6 +670,7 @@ impl Pages { /// answers the frame of a partial last page, whose tail past the size the /// caller zeroes. pub fn resize(&mut self, o: &mut Object, size: u64, out: &mut impl FnMut(F)) -> Option { + o.version += 1; let keep = size.div_ceil(PAGE); if size < o.size { let mut at = o.head; @@ -677,6 +731,47 @@ mod tests { |f| freed.push(f) } + /// XSH 2.9.7: a read that began before a write, or while one was under + /// way, never counts; one that began after it does. + #[test] + fn a_read_counts_only_where_no_write_began_or_was_under_way() { + let mut p = Pages::::new(8, 4).unwrap(); + let mut o = p.object(1, PAGE); + let before = o.read_begin().expect("nothing writes yet"); + o.write_begin(); + assert_eq!(o.read_begin(), None, "a read meets the write in flight"); + assert!(!o.unchanged(before), "a read that began before it meets it"); + o.write_end(true); + assert!(!o.unchanged(before), "and still does once it ended"); + let after = o.read_begin().expect("the write ended"); + assert!(o.unchanged(after)); + p.resize(&mut o, 0, &mut none); + assert!(!o.unchanged(after), "a shrink is a change a read meets"); + } + + /// A pager's write that takes several calls is one write: the first short + /// call leaves it under way, later ones nest in it, and only its end lets + /// a read begin. + #[test] + fn a_short_call_holds_the_write_until_its_end() { + let mut p = Pages::::new(8, 4).unwrap(); + let mut o = p.object(2, PAGE); + assert_eq!(o.end_write(), Err(Refused::InvalidArgument), "nothing is held"); + o.write_begin(); + o.write_end(false); + assert_eq!(o.read_begin(), None, "a short call's write is under way"); + o.write_begin(); + o.write_end(false); + o.write_begin(); + o.write_end(true); + assert_eq!(o.read_begin(), None, "a later call, short or whole, ends no held write"); + let held = o.version; + assert_eq!(o.end_write(), Ok(())); + let v = o.read_begin().expect("the end lets a read begin"); + assert!(o.unchanged(v) && v != Version(held), "the end moved the version"); + assert_eq!(o.end_write(), Err(Refused::InvalidArgument), "a write ends once"); + } + #[test] fn a_supplied_page_is_found_and_a_second_supply_changes_nothing() { let mut p = Pages::new(8, 4).unwrap(); diff --git a/kernel/src/object/paged.rs b/kernel/src/object/paged.rs index a7b74d6b77..e0c3cfc4f5 100644 --- a/kernel/src/object/paged.rs +++ b/kernel/src/object/paged.rs @@ -22,6 +22,11 @@ //! A pager's own process is refused a holder's read, so no pager waits on a //! pager. //! +//! **A holder's read is atomic against its pager's writes** +//! ([`kernel::paged::Object::read_begin`]): one that meets a write is answered +//! `WouldBlock`, and the holder asks the pager, whose own reads its writes +//! order. +//! //! **Lock order.** The pager's lock is innermost but for the allocator's. A //! copy to or from user memory holds pins, never the lock; no wait holds a //! pin. @@ -41,7 +46,7 @@ use core::ptr; use core::sync::atomic::{AtomicU32, Ordering}; use kernel::chunked::{Chunked, Stack}; -use kernel::paged::{Core, Draw, Leave, Lookup, Object, Pages, Refused, Supplied, PAGE}; +use kernel::paged::{Core, Draw, Leave, Lookup, Object, Pages, Refused, Supplied, Version, PAGE}; use kernel::sched::task::WaitClass; use toyos_abi::syscall::{PagedNotice, SyscallError, NOTICE_NO_HOLDERS, NOTICE_REQUESTED}; @@ -450,21 +455,26 @@ impl PagerObject { let mut done = 0usize; let waiting = core::pin::pin!(Waiting::new(self, slot)); let mut pieces = [Piece::default(); CHUNK]; + // A holder's: what its bytes count against. + let mut since: Option = None; loop { let at = offset.saturating_add(done as u64); let step = { let mut state = self.state.lock(); + let size = state.entry(slot).object.size(); if state.gone || state.entry(slot).object.gone() { Step::Gone + } else if done == want || (done == 0 && at >= size) { + // Whole, each copy counted as it ended; or nothing to copy. + Step::Done + } else if waits && !Self::unwritten(&state.entry(slot).object, &mut since) { + Step::Written + } else if at >= size { + Step::Done } else { - let size = state.entry(slot).object.size(); - if done == want || at >= size { - Step::Done - } else { - let end = size.min(offset.saturating_add(want as u64)); - let waiter = waits.then_some(waiting.as_ref()); - self.pin_chunk(&mut state, slot, at..end, &mut pieces, waiter) - } + let end = size.min(offset.saturating_add(want as u64)); + let waiter = waits.then_some(waiting.as_ref()); + self.pin_chunk(&mut state, slot, at..end, &mut pieces, waiter) } }; match step { @@ -478,15 +488,20 @@ impl PagerObject { for piece in &pieces[..count] { pmm::unpin_range(piece.phys, PAGE as usize); } - let gone = { + let (gone, written) = { let state = self.state.lock(); - state.gone || state.entry(slot).object.gone() + let object = &state.entry(slot).object; + (state.gone || object.gone(), waits && !Self::unwritten(object, &mut since)) }; // A frame copied as its pager ended is no byte to report. if gone { return SyscallError::Io.to_u64(); } + if written { + return SyscallError::WouldBlock.to_u64(); + } } + Step::Written => return SyscallError::WouldBlock.to_u64(), Step::Done => return done as u64, Step::Gone if done > 0 => return done as u64, Step::Gone => return SyscallError::Gone.to_u64(), @@ -519,6 +534,18 @@ impl PagerObject { } } + /// Under the lock: whether no write met a holder's read of `object` that + /// began at `since`, or begins now where it is `None`. + fn unwritten(object: &Object, since: &mut Option) -> bool { + match *since { + Some(v) => object.unchanged(v), + None => { + *since = object.read_begin(); + since.is_some() + } + } + } + /// Under the lock: pin the present pages of `range`, at most [`CHUNK`], /// into `pieces`, or else say what the first page is, recording a /// waiter's request for every page from an absent one to the range's end. @@ -561,13 +588,55 @@ impl PagerObject { Step::Copy(count) } + /// A write call on object `id` begins: its slot, held by a reference until + /// [`Self::write_end`], so no release can hand it to another object. + fn write_begin(&self, id: u64) -> Result { + let mut state = self.state.lock(); + let slot = state.live(id)?; + let entry = state.entry_mut(slot); + entry.refs += 1; + entry.object.write_begin(); + Ok(slot) + } + + /// The write call [`Self::write_begin`] began ends, `whole` where it took + /// everything it was given: its reference goes. + fn write_end(&self, slot: u32, whole: bool) { + let mut state = self.state.lock(); + state.entry_mut(slot).object.write_end(whole); + self.unref_locked(&mut state, slot); + } + + /// The pager ends the write of object `id` a short call left under way. + pub fn end_write(&self, id: u64) -> u64 { + let mut state = self.state.lock(); + let ended = state.live(id).and_then(|slot| state.entry_mut(slot).object.end_write().map_err(word)); + match ended { + Ok(()) => 0, + Err(e) => e.to_u64(), + } + } + /// Copy `src` into fresh pages of object `id` from page-aligned `offset`, - /// clean or dirty: the pages installed. The caller checked the span is in - /// reach. + /// clean or dirty: the pages installed. A dirty supply is a write. The + /// caller checked the span is in reach. pub fn supply(&self, id: u64, offset: u64, src: &UserBytes, dirty: bool) -> u64 { if !offset.is_multiple_of(PAGE) || src.len() == 0 { return SyscallError::InvalidArgument.to_u64(); } + if !dirty { + return self.supply_pages(id, offset, src, false); + } + let slot = match self.write_begin(id) { + Ok(slot) => slot, + Err(e) => return e.to_u64(), + }; + let answer = self.supply_pages(id, offset, src, true); + self.write_end(slot, answer == (src.len() as u64).div_ceil(PAGE)); + answer + } + + fn supply_pages(&self, id: u64, offset: u64, src: &UserBytes, dirty: bool) -> u64 { let mut installed = 0u64; let mut done = 0usize; while done < src.len() { @@ -663,6 +732,16 @@ impl PagerObject { /// the bytes written, up to the first absent page. The caller checked the /// span is in reach. pub fn write(&self, id: u64, offset: u64, src: &UserBytes) -> u64 { + let slot = match self.write_begin(id) { + Ok(slot) => slot, + Err(e) => return e.to_u64(), + }; + let answer = self.write_pages(id, offset, src); + self.write_end(slot, answer == src.len() as u64); + answer + } + + fn write_pages(&self, id: u64, offset: u64, src: &UserBytes) -> u64 { let mut done = 0usize; while done < src.len() { let mut pinned = [(0u64, 0u64, 0usize, 0usize); CHUNK]; @@ -799,6 +878,8 @@ enum Step { /// This many pieces were pinned. Copy(usize), Absent(u64), + /// A write met a holder's read. + Written, Failed, Gone, Done, diff --git a/kernel/src/syscall/paged.rs b/kernel/src/syscall/paged.rs index cd7f1e5f78..0a8aaa0f16 100644 --- a/kernel/src/syscall/paged.rs +++ b/kernel/src/syscall/paged.rs @@ -132,6 +132,7 @@ pub(super) fn sys_paged_op(ctx: &SyscallContext<'_>, h: RawHandle, op: u64, args x if x == PagedOp::Fail as u64 => pager.fail(id, args.offset, args.len), x if x == PagedOp::Resize as u64 => pager.resize(id, args.offset), x if x == PagedOp::Release as u64 => pager.release(id), + x if x == PagedOp::EndWrite as u64 => pager.end_write(id), x if x == PagedOp::WritebackBegin as u64 || x == PagedOp::WritebackEnd as u64 => { let end = (x == PagedOp::WritebackEnd as u64).then_some(args.flags & PAGED_WRITTEN != 0); if args.flags & !PAGED_WRITTEN != 0 || (end.is_none() && args.flags != 0) || !range_ok(args.len) { diff --git a/sdk/abi/src/syscall.rs b/sdk/abi/src/syscall.rs index d4c2f98e57..dd0911e4bb 100644 --- a/sdk/abi/src/syscall.rs +++ b/sdk/abi/src/syscall.rs @@ -3311,15 +3311,17 @@ pub const PAGED_RANGE_PAGES: u64 = 512; pub enum PagedOp { /// Copy `len` bytes from `buf` into fresh pages at `offset`, a whole page /// in: clean, or dirty with [`PAGED_DIRTY`], which also grows the size to - /// the end of the bytes. A page already present is left as it is. - /// Answers the pages installed; `ResourceExhausted` where no page could - /// be had, `WouldBlock` at the dirty ceiling, `InvalidArgument` for a - /// clean page past the size or a dirty one past its end. + /// the end of the bytes and is a write ([`PagedOp::Write`]). A page + /// already present is left as it is. Answers the pages installed; + /// `ResourceExhausted` where no page could be had, `WouldBlock` at the + /// dirty ceiling, `InvalidArgument` for a clean page past the size or a + /// dirty one past its end. Supply = 0, /// Copy `len` bytes from `buf` into the present pages at `offset`, dirty, /// growing the size to their end as each page's bytes land. Answers the /// bytes written, stopping at the first absent page; `NotFound` when the - /// first is absent, `WouldBlock` at the dirty ceiling. + /// first is absent, `WouldBlock` at the dirty ceiling. A write that does + /// not take all it was given stays under way until [`PagedOp::EndWrite`]. Write = 1, /// Remove the clean pages of `len` pages from page `offset` that no /// write-back or write holds. Answers how many went. @@ -3345,6 +3347,9 @@ pub enum PagedOp { /// Let the object go: its pages go, its holders answer `Gone`, and its id /// names another object once its last holder has gone. Release = 8, + /// End the write a short [`PagedOp::Write`] or dirty supply left under + /// way; `InvalidArgument` where none is. + EndWrite = 9, } /// [`PagedOp::Supply`]'s pages are dirty. @@ -3416,7 +3421,9 @@ pub fn paged_holder(pager: RawHandle, object: u64) -> Result Result<(), Refused> { + self.op(PagedOp::EndWrite, PagedArgs { object: id, ..PagedArgs::default() }, None).map(drop) + } + /// Drop the clean pages of `count` from page `first` that nothing holds: /// how many went. pub fn withdraw(&self, id: u64, first: u64, count: u64) -> Result { @@ -200,7 +207,8 @@ impl Paged { /// Up to `out.len()` bytes at `offset`, 0 at or past the size, waiting for /// every absent page until it is supplied, failed (`Io`, or a short count /// after some bytes), cut off by a shrink, or its object or pager ends - /// (`Gone`). + /// (`Gone`). A read a write of the pager's met is `WouldBlock`: ask the + /// pager, whose writes order its own reads. pub fn read(&self, out: &mut [u8], offset: u64) -> Result { syscall::paged_read(self.0.raw(), out, offset) } diff --git a/system/fileserver/src/absent.rs b/system/fileserver/src/absent.rs index 219d9a77f0..04f5df1241 100644 --- a/system/fileserver/src/absent.rs +++ b/system/fileserver/src/absent.rs @@ -110,6 +110,10 @@ impl Volume for Absent { Ok(Vec::new()) } + fn commit(&mut self, _node: Option) -> Result<(), Error> { + Ok(()) + } + fn room(&mut self) -> Result { Ok(toyos::fs::Room { free: 0, unit: 1 }) } diff --git a/system/fileserver/src/data.rs b/system/fileserver/src/data.rs index 763409b815..75389f9902 100644 --- a/system/fileserver/src/data.rs +++ b/system/fileserver/src/data.rs @@ -17,7 +17,9 @@ //! must) the block of each page a write will touch and moves the length; the //! write-back puts the pages; the new length and extents reach the file's //! entry at its close, an fsync, or the volume's sync, and a sync writes every -//! such entry before the cache is flushed. A length that +//! such entry before the cache is flushed. An fsync's commit writes its own +//! file's and those of the files it carries ([`Volume::carried`]): any whose +//! entry is written, or whose blocks were taken, since the last. A length that //! shrinks is recorded before the blocks past it are freed, so a failure //! between the two leaks blocks rather than leaving an entry naming freed ones. //! @@ -58,6 +60,9 @@ struct Open { mtime: u64, /// Length or extents changed since the entry was last written. dirty: bool, + /// The entry was written, or blocks taken, since the last commit + /// ([`Volume::carried`]). + carried: bool, /// Unlinked or renamed over: its blocks are no longer its own. gone: bool, holders: u32, @@ -336,12 +341,27 @@ impl DataVolume { } mapped("the entry", &open.path, self.fs.update_metadata(&open.path, &open.extents, open.size, open.mtime))?; open.dirty = false; + open.carried = true; if let Some(meta) = self.names.get_mut(&open.path) { (meta.size, meta.mtime) = (open.size, open.mtime); } Ok(()) } + /// The disk as a stop now leaves it: what the last flush wrote. + #[cfg(test)] + pub(crate) fn stopped(self) -> D { + drop(self.fs); + Rc::try_unwrap(self.cache).ok().expect("one owner").into_disk() + } + + /// A commit landed: no entry it made durable is owed to the next. + fn committed(&mut self) { + for open in self.open.values_mut().filter(|open| !open.dirty) { + open.carried = false; + } + } + /// Forget a node nobody holds and whose entry is written. fn release(&mut self, node: Node) { let open = self.open.remove(&node).expect("an open node"); @@ -362,7 +382,7 @@ impl DataVolume { fn new_node(&mut self, path: &str, extents: Vec, size: u64, mtime: u64) -> Node { let node = self.next; self.next += 1; - self.open.insert(node, Open { path: path.to_string(), extents, size, mtime, dirty: false, gone: false, holders: 0 }); + self.open.insert(node, Open { path: path.to_string(), extents, size, mtime, dirty: false, carried: false, gone: false, holders: 0 }); self.by_path.insert(path.to_string(), node); node } @@ -580,6 +600,7 @@ impl Volume for DataVolume { open.size = open.size.max(end); open.mtime = now; open.dirty = true; + open.carried |= open.extents.iter().map(|e| e.block_count as u64).sum::() > covered; noted(&mut self.names, open); Ok(reserved) } @@ -659,6 +680,7 @@ impl Volume for DataVolume { // Recorded first, freed second. mapped("a truncate", &open.path, self.fs.update_metadata(&open.path, &open.extents, size, now))?; open.dirty = false; + open.carried = true; if let Some(meta) = self.names.get_mut(&open.path) { (meta.size, meta.mtime) = (size, now); } @@ -828,9 +850,35 @@ impl Volume for DataVolume { } } mapped("sync", "", self.fs.sync())?; + self.committed(); Ok(unwritten) } + fn carried(&self, node: Node) -> bool { + self.open.get(&node).is_some_and(|open| open.carried && !open.gone) + } + + fn owes(&self) -> bool { + self.open.values().any(|open| open.dirty && !open.gone) + } + + fn commit(&mut self, node: Option) -> Result<(), Error> { + if let Some(node) = node { + self.persist(node)?; + } + let carried: Vec = self.open.keys().copied().filter(|&n| self.carried(n)).collect(); + for n in carried { + match self.persist(n) { + Err(e) => println!("fileserver: node {n}'s entry was not written at a commit ({e:?}); the next one writes it"), + Ok(()) if self.open[&n].holders == 0 => self.release(n), + Ok(()) => {} + } + } + mapped("sync", "", self.fs.sync())?; + self.committed(); + Ok(()) + } + fn room(&mut self) -> Result { Ok(toyos::fs::Room { free: self.fs.free_bytes(), unit: BLOCK as u64 }) } @@ -1142,6 +1190,24 @@ mod tests { 1_717_245_296_123_456_789 } + /// A commit leaves owed the entry of a file it did not carry, an overwrite + /// within the file's own blocks, and nothing once that entry is committed. + #[test] + fn a_commit_owes_the_entry_of_a_file_it_did_not_carry() { + let mut v = vol(); + let a = v.open("home/a", CREATE).unwrap(); + let b = v.open("home/b", CREATE).unwrap(); + v.write(a, 0, b"a").unwrap(); + v.write(b, 0, b"b").unwrap(); + v.commit(Some(a)).unwrap(); + assert!(!v.owes(), "b took blocks, so the commit carried its entry"); + v.write(b, 0, b"B").unwrap(); + v.commit(Some(a)).unwrap(); + assert!(v.owes(), "b's overwrite took no block and was not carried"); + v.commit(Some(b)).unwrap(); + assert!(!v.owes()); + } + /// DATA stamps a write with the nanosecond its clock read, and the entry /// keeps it whole across a remount: a stamp of whole seconds would round /// it away. @@ -1160,9 +1226,7 @@ mod tests { } fn remount(v: DataVolume) -> DataVolume { - let DataVolume { fs, cache, .. } = v; - drop(fs); - let disk = Rc::try_unwrap(cache).ok().expect("one owner").into_disk(); + let disk = v.stopped(); let Probed::Mounted(again) = DataVolume::probe(disk, CLEAN_LIMIT, &["home"], clock) else { panic!("remount") }; again } diff --git a/system/fileserver/src/fat.rs b/system/fileserver/src/fat.rs index b72741c92e..ec75dbf9f1 100644 --- a/system/fileserver/src/fat.rs +++ b/system/fileserver/src/fat.rs @@ -433,6 +433,34 @@ impl Volume for FatVolume { Ok(unlevel) } + /// A flush writes every chain the cache holds, whichever file asked: an + /// open file changed since its entry was level is carried. + fn carried(&self, node: Node) -> bool { + self.open.get(&node).is_some_and(|open| !open.gone && (open.dirty || open.file.needs_reconcile())) + } + + fn owes(&self) -> bool { + self.open.keys().any(|&node| self.carried(node)) + } + + fn commit(&mut self, node: Option) -> Result<(), fs::Error> { + if !self.writable { + return Ok(()); + } + if let Some(node) = node { + self.level(node)?; + } + let carried: Vec = self.open.keys().copied().filter(|&n| self.carried(n)).collect(); + for n in carried { + match self.level(n) { + Err(e) => println!("fileserver: node {n} was not brought level at a commit ({e:?}); the next one does it"), + Ok(()) if self.open[&n].holders == 0 => self.release(n), + Ok(()) => {} + } + } + self.cache.block_on(self.fs.sync()).map_err(|e| logged("sync", "", e)) + } + fn room(&mut self) -> Result { let unit = u64::from(self.fs.geometry().bytes_per_cluster()); if !self.writable { @@ -549,6 +577,20 @@ mod tests { assert_eq!(v.lstat("a.txt").unwrap().mtime, 1_717_245_300 * NANOS_PER_SEC); } + /// A write is owed until a commit, and a commit naming another file + /// carries it: FAT's flush writes every chain it holds. + #[test] + fn a_commit_of_one_file_leaves_no_other_s_write_owed() { + const CREATE: OpenHow = OpenHow { create: true, create_new: false, truncate: false }; + let (mut v, _, _) = spec_fat(); + let a = v.open("a.txt", CREATE).unwrap(); + let b = v.open("b.txt", CREATE).unwrap(); + v.write(b, 0, &[2; 3000]).unwrap(); + assert!(v.owes()); + v.commit(Some(a)).unwrap(); + assert!(!v.owes()); + } + /// What [`a_sync_of_files_nothing_changed_asks_the_disk_nothing`]'s volume /// reads as the time, which only that test sets. static LATER: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); diff --git a/system/fileserver/src/main.rs b/system/fileserver/src/main.rs index 92133ef966..e4d46290ec 100644 --- a/system/fileserver/src/main.rs +++ b/system/fileserver/src/main.rs @@ -40,10 +40,11 @@ //! client lends is read once per request into this process's memory, and //! what is acted on is that copy. //! -//! **What is written reaches the disk at a sync**: an fsync, a client's -//! `SYNC`, or [`fileserver::writeback::WRITEBACK`] after the first unsynced write when nothing asked -//! sooner. A kill loses what no sync covered, which is POSIX's promise and no -//! more. +//! **What is written reaches the disk at a sync**: a client's `SYNC`, or +//! [`fileserver::writeback::WRITEBACK`] after the first unsynced write when +//! nothing asked sooner; an fsync makes its own file durable, with what the +//! volume's commit carries beside it ([`fileserver::pages::Files::fsync`]). A +//! kill loses what neither covered, which is POSIX's promise and no more. use std::cell::RefCell; use std::collections::BTreeMap; @@ -544,7 +545,7 @@ impl Server { self.drop_client(id, "it never lent its window"); } if self.writeback.take_due(now) { - if let Err(e) = self.sync(None) { + if let Err(e) = self.sync() { println!("fileserver: the write-back sync failed: {e:?}"); } } @@ -928,14 +929,18 @@ impl Server { SETATTR => self.set_attr(id, &r), FSYNC => { let file = self.settled(id, r.fid)?; - let node = match file.borrow().held { - Held::File(node) => Some(node), - // A directory's names are made durable with the volume. + let named = match file.borrow().held { + Held::File(node) => Some((file.borrow().identity, node)), Held::Dir { .. } => None, }; - self.sync(node) + self.files.fsync(&mut *self.volume, named)?; + // One that left nothing to make durable did the write-back's work. + if !self.files.owes(&*self.volume) { + self.writeback.synced(false, Instant::now()); + } + Ok(Answer::Reply(Reply::ok())) } - SYNC => self.sync(None), + SYNC => self.sync(), ROOM => { let room = self.volume.room()?; Ok(Answer::Reply(Reply { value: room.free, value2: room.unit, ..Reply::ok() })) @@ -1257,13 +1262,12 @@ impl Server { Ok(Answer::Reply(Reply::ok())) } - /// The volume synced, answered as `node`'s file fared, or with none as - /// every file did. A file whose entry was refused keeps the write-back - /// due, so the next one tries it again. - fn sync(&mut self, node: Option) -> Result { + /// The volume synced, answered as every file fared. A file whose entry + /// was refused keeps the write-back due, so the next one tries it again. + fn sync(&mut self) -> Result { let unwritten = self.files.sync(&mut *self.volume)?; self.writeback.synced(!unwritten.is_empty(), Instant::now()); - match unwritten.into_iter().find(|&(n, _)| node.is_none_or(|node| n == node)) { + match unwritten.into_iter().next() { Some((_, e)) => Err(e), None => Ok(Answer::Reply(Reply::ok())), } diff --git a/system/fileserver/src/pages.rs b/system/fileserver/src/pages.rs index b93579abb6..490439887f 100644 --- a/system/fileserver/src/pages.rs +++ b/system/fileserver/src/pages.rs @@ -11,6 +11,10 @@ //! sake until room is wanted: then a file nobody holds and nothing dirties //! goes first, oldest first. //! +//! **A write is one write of the object's**, however many calls it takes: +//! a call that stops short leaves it under way until its end, so no holder +//! reads the file half written (`toyos::paged::Pager::end_write`). +//! //! **The volume's size is the object's.** A write reserves on the volume //! first, so the volume's size runs past the object's only for the bytes the //! write then puts in before it returns; nothing reads the volume for them @@ -36,6 +40,7 @@ pub trait Pager { fn read(&self, id: u64, out: Bytes<'_>, offset: u64) -> Result; fn supply(&self, id: u64, offset: u64, bytes: &[u8], dirty: bool) -> Result; fn write(&self, id: u64, offset: u64, bytes: &[u8]) -> Result; + fn end_write(&self, id: u64) -> Result<(), Refused>; fn withdraw(&self, id: u64, first: u64, count: u64) -> Result; fn fail(&self, id: u64, first: u64, count: u64) -> Result<(), Refused>; fn resize(&self, id: u64, size: u64) -> Result<(), Refused>; @@ -57,6 +62,9 @@ impl Pager for toyos::paged::Pager { fn write(&self, id: u64, offset: u64, bytes: &[u8]) -> Result { toyos::paged::Pager::write(self, id, offset, bytes) } + fn end_write(&self, id: u64) -> Result<(), Refused> { + toyos::paged::Pager::end_write(self, id) + } fn withdraw(&self, id: u64, first: u64, count: u64) -> Result { toyos::paged::Pager::withdraw(self, id, first, count) } @@ -369,31 +377,42 @@ impl Files

{ let reserved = volume.reserve(node, offset, data.len() as u64)?; let old = reserved.size; self.touch(identity); - let mut done = 0; - let put = self.put(volume, &Writing { identity, node, old, contents: reserved.contents(), offset, data }, &mut done); let id = self.files[&identity].object; - if let Err(e) = put { + let mut put = Put { done: 0, held: false }; + let written = self.put(volume, &Writing { identity, node, old, contents: reserved.contents(), offset, data }, &mut put); + if let Err(e) = &written { // What was reserved and not put would read as the blocks' old bytes. - let landed = if done > 0 { offset + done as u64 } else { 0 }; + let landed = if put.done > 0 { offset + put.done as u64 } else { 0 }; let cut = old.max(landed); let resized = self.pg().resize(id, cut).map_err(word).and(volume.unreserve(node, reserved, landed)); if let Err(again) = resized { - println!("fileserver: a refused write of file {identity} was not cut back to {cut} ({again:?})"); + println!("fileserver: a refused write of file {identity} was not cut back to {cut} ({again:?}), after {e:?}"); } - return Err(e); } - Ok(()) + if put.held { + match self.pg().end_write(id) { + Ok(()) | Err(Refused::Gone) => {} + Err(why) => println!("fileserver: file {identity}'s write would not end ({why:?}); its holders ask this server"), + } + } + written } /// The bytes of a write into the object, each absent page supplied dirty, - /// counting into `done` what landed. - fn put(&mut self, volume: &mut dyn Volume, w: &Writing<'_>, done: &mut usize) -> Result<(), Error> { + /// counting into `put` what landed and whether a call left the write + /// under way. + fn put(&mut self, volume: &mut dyn Volume, w: &Writing<'_>, put: &mut Put) -> Result<(), Error> { let Writing { identity, node, old, contents, offset, data } = *w; let end = offset + data.len() as u64; + let done = &mut put.done; while *done < data.len() { let at = offset + *done as u64; let id = self.files.get(&identity).ok_or(Error::Stale)?.object; - match self.pg().write(id, at, &data[*done..]) { + let wrote = self.pg().write(id, at, &data[*done..]); + // A first call that takes everything is the whole write; any later + // call is under the write a short first one left under way. + put.held |= wrote != Ok(data.len() - *done); + match wrote { Ok(n) => { self.landed(identity, at, n); *done += n; @@ -530,6 +549,34 @@ impl Files

{ Ok(unwritten) } + /// An fsync: `file`'s pages to the volume, and every file's the volume's + /// commit carries, then the commit. A carried file whose pages would not + /// go refuses it, since its entry would name blocks without its bytes. + pub fn fsync(&mut self, volume: &mut dyn Volume, file: Option<(u64, Node)>) -> Result<(), Error> { + let named = file.map(|(identity, _)| identity); + let owed: Vec = self + .files + .iter() + .filter(|(id, f)| f.dirty.is_some() && (Some(**id) == named || volume.carried(f.node))) + .map(|(id, _)| *id) + .collect(); + for id in owed { + if let Err(e) = self.write_back(volume, id) { + if Some(id) != named { + println!("fileserver: file {id}'s pages would not go to the volume ({e:?}); an fsync of another is refused"); + } + return Err(e); + } + } + volume.commit(file.map(|(_, node)| node)) + } + + /// Whether a change is not yet durable: a dirty page, or an entry the + /// volume owes ([`Volume::owes`]). + pub fn owes(&self, volume: &dyn Volume) -> bool { + self.files.values().any(|f| f.dirty.is_some()) || volume.owes() + } + /// What the pager's objects asked. pub fn notice(&mut self, volume: &mut dyn Volume, notice: Notice) { match notice { @@ -558,6 +605,13 @@ impl Files

{ } } +/// How far a write got: the bytes that landed, and whether a call left the +/// object's write under way. +struct Put { + done: usize, + held: bool, +} + /// A write under way: its file, at `offset`, which was `old` bytes long. #[derive(Clone, Copy)] struct Writing<'a> { diff --git a/system/fileserver/src/pages/tests.rs b/system/fileserver/src/pages/tests.rs index 4919978901..a35bdceb39 100644 --- a/system/fileserver/src/pages/tests.rs +++ b/system/fileserver/src/pages/tests.rs @@ -1,5 +1,5 @@ use std::cell::RefCell; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; use std::rc::Rc; use toyos_abi::syscall::SyscallError; @@ -23,6 +23,13 @@ struct Model { supplies: u32, /// Every supply from here is refused. refusing: bool, + /// This object's write-backs are refused. + unwritable: Option, + /// Objects a call that did not take all it was given left written. + held: BTreeSet, + /// Each object's pages as a holder could read them after each change + /// that left no write under way. + views: Vec<(u64, BTreeMap>)>, } struct Page { @@ -35,6 +42,18 @@ impl Model { fn held(&self) -> usize { self.pages.values().filter(|p| p.dirty || p.writeback).count() } + + /// A change to `id` ended, `whole` where the call took all it was given: + /// the kernel's rule (`kernel::paged::Object::write_end`). + fn changed(&mut self, id: u64, whole: bool) { + if !whole { + self.held.insert(id); + } + if !self.held.contains(&id) { + let view = self.pages.iter().filter(|((c, _), _)| *c == id).map(|(&(_, p), page)| (p, page.bytes.clone())).collect(); + self.views.push((id, view)); + } + } } #[derive(Clone)] @@ -48,45 +67,8 @@ impl Fake { fn pages_of(&self, object: u64) -> usize { self.0.borrow().pages.keys().filter(|(c, _)| *c == object).count() } -} - -impl Pager for Fake { - fn object(&self, size: u64) -> Result { - let mut m = self.0.borrow_mut(); - if m.sizes.len() >= m.objects { - return Err(Refused::NoRoom); - } - m.next += 1; - let id = m.next; - m.sizes.insert(id, size); - Ok(id) - } - fn release(&self, id: u64) -> Result<(), Refused> { - let mut m = self.0.borrow_mut(); - m.pages.retain(|(c, _), _| *c != id); - m.sizes.remove(&id).map(drop).ok_or(Refused::Gone) - } - - fn read(&self, id: u64, out: Bytes<'_>, offset: u64) -> Result { - let m = self.0.borrow(); - let size = m.sizes[&id]; - let end = size.min(offset + out.len() as u64); - let mut at = offset; - while at < end { - let Some(page) = m.pages.get(&(id, at / PAGE)) else { break }; - let within = (at % PAGE) as usize; - let n = ((PAGE as usize - within) as u64).min(end - at) as usize; - out.write((at - offset) as usize, &page.bytes[within..within + n]); - at += n as u64; - } - match at - offset { - 0 if offset < end => Err(Refused::Absent), - n => Ok(n as usize), - } - } - - fn supply(&self, id: u64, offset: u64, bytes: &[u8], dirty: bool) -> Result { + fn supply_pages(&self, id: u64, offset: u64, bytes: &[u8], dirty: bool) -> Result { let mut m = self.0.borrow_mut(); m.supplies += 1; if m.refusing { @@ -123,7 +105,7 @@ impl Pager for Fake { Ok(installed) } - fn write(&self, id: u64, offset: u64, bytes: &[u8]) -> Result { + fn write_pages(&self, id: u64, offset: u64, bytes: &[u8]) -> Result { let mut m = self.0.borrow_mut(); let mut done = 0; while done < bytes.len() { @@ -144,6 +126,64 @@ impl Pager for Fake { } if done == 0 { Err(Refused::Absent) } else { Ok(done) } } +} + +impl Pager for Fake { + fn object(&self, size: u64) -> Result { + let mut m = self.0.borrow_mut(); + if m.sizes.len() >= m.objects { + return Err(Refused::NoRoom); + } + m.next += 1; + let id = m.next; + m.sizes.insert(id, size); + Ok(id) + } + + fn release(&self, id: u64) -> Result<(), Refused> { + let mut m = self.0.borrow_mut(); + m.pages.retain(|(c, _), _| *c != id); + m.sizes.remove(&id).map(drop).ok_or(Refused::Gone) + } + + fn read(&self, id: u64, out: Bytes<'_>, offset: u64) -> Result { + let m = self.0.borrow(); + let size = m.sizes[&id]; + let end = size.min(offset + out.len() as u64); + let mut at = offset; + while at < end { + let Some(page) = m.pages.get(&(id, at / PAGE)) else { break }; + let within = (at % PAGE) as usize; + let n = ((PAGE as usize - within) as u64).min(end - at) as usize; + out.write((at - offset) as usize, &page.bytes[within..within + n]); + at += n as u64; + } + match at - offset { + 0 if offset < end => Err(Refused::Absent), + n => Ok(n as usize), + } + } + + fn supply(&self, id: u64, offset: u64, bytes: &[u8], dirty: bool) -> Result { + let answer = self.supply_pages(id, offset, bytes, dirty); + if dirty { + self.0.borrow_mut().changed(id, answer == Ok(bytes.len().div_ceil(PAGE as usize) as u64)); + } + answer + } + + fn write(&self, id: u64, offset: u64, bytes: &[u8]) -> Result { + let answer = self.write_pages(id, offset, bytes); + self.0.borrow_mut().changed(id, answer == Ok(bytes.len())); + answer + } + + fn end_write(&self, id: u64) -> Result<(), Refused> { + let mut m = self.0.borrow_mut(); + assert!(m.held.remove(&id), "a write ended that no short call left under way"); + m.changed(id, true); + Ok(()) + } fn withdraw(&self, id: u64, first: u64, count: u64) -> Result { let mut m = self.0.borrow_mut(); @@ -165,10 +205,14 @@ impl Pager for Fake { page.bytes[(size % PAGE) as usize..].fill(0); } m.sizes.insert(id, size); + m.changed(id, true); Ok(()) } fn writeback_begin(&self, id: u64, first: u64, count: u64) -> Result<[u64; 8], Refused> { + if self.0.borrow().unwritable == Some(id) { + return Err(Refused::Kernel(SyscallError::Io)); + } let mut bits = [0u64; 8]; for (&(c, p), page) in self.0.borrow_mut().pages.iter_mut() { if c == id && (first..first + count).contains(&p) && page.dirty { @@ -475,3 +519,136 @@ fn an_object_refused_for_room_is_made_by_a_closed_files_going() { let identity = v.identity("home/d").unwrap(); assert_eq!(files.opened(&mut v, identity, n), Err(Error::NoSpace), "both held open: refused, by name"); } + +/// XSH 2.9.7, the server's half: a write that takes many calls — an absent +/// page supplied, the dirty ceiling written back, a last page in part — is +/// one write of the object's, so every state a holder can read between calls +/// is the file before it or after it, and the write ends whether it lands +/// or is refused. +#[test] +fn a_write_of_many_calls_is_read_whole_or_not_at_all() { + const PAGES: usize = 8; + let page = PAGE as usize; + let old = pattern(PAGES * page, 10); + let mut v = volume(); + let n = v.open("home/f", CREATE).unwrap(); + v.write(n, 0, &old).unwrap(); + v.close(n).unwrap(); + let fake = Fake::new(64, 4); + let mut files = Files::new(Some(fake.clone())); + let at = open(&mut files, &mut v, "home/f"); + read(&mut files, &mut v, at, 0, PAGES * page); + let object = files.object(at.1).unwrap(); + assert_eq!(fake.withdraw(object, 3, 1), Ok(1)); + fake.0.borrow_mut().views.clear(); + let supplies = fake.0.borrow().supplies; + + let new = pattern(6 * page + 100, 11); + files.write(&mut v, at.1, at.0, 0, &new).unwrap(); + let mut after = old.clone(); + after[..new.len()].copy_from_slice(&new); + let m = fake.0.borrow(); + assert!(m.held.is_empty(), "the write was left under way"); + assert!(m.supplies > supplies, "the write supplied no absent page: one call, nothing to tear"); + for (k, (_, view)) in m.views.iter().filter(|(id, _)| *id == object).enumerate() { + let as_of = |file: &[u8]| view.iter().all(|(&p, bytes)| p as usize >= PAGES || bytes[..] == file[p as usize * page..][..page]); + assert!(as_of(&old) || as_of(&after), "view {k} of {} is neither the file before the write nor after it", m.views.len()); + } + drop(m); + + fake.0.borrow_mut().refusing = true; + assert!(files.write(&mut v, at.1, at.0, 7 * page as u64 + 50, &pattern(3 * page, 12)).is_err()); + assert!(fake.0.borrow().held.is_empty(), "a refused write was left under way"); +} + +/// A volume whose every block holds what an earlier tenant left, 0xAB. +fn stale_volume() -> DataVolume { + let disk = Ram::new(4096); + let stale = vec![[0xAB; toyos_blockdev::BLOCK]; 4096]; + toyos_blockdev::block_on(toyos_blockdev::BlockDev::write(&disk, 0, &stale), || ()).unwrap(); + DataVolume::format(disk, 256, &["home"], clock).unwrap() +} + +/// `v` stopped now and mounted again: what its last commit left. +fn after_a_stop(v: DataVolume) -> DataVolume { + match DataVolume::probe(v.stopped(), 256, &["home"], clock) { + crate::data::Probed::Mounted(v) => v, + _ => panic!("the volume does not mount after a stop"), + } +} + +/// An fsync makes its own file durable, and leaves another file's write +/// over blocks the volume already named in the object, unwritten: nothing +/// the commit makes durable names those blocks anew. +#[test] +fn an_fsync_makes_its_file_durable_and_no_other_files_overwrite() { + let page = PAGE as usize; + let mut v = stale_volume(); + let mut files = Files::new(Some(Fake::new(64, 16))); + let b = open(&mut files, &mut v, "home/b"); + files.write(&mut v, b.1, b.0, 0, &pattern(2 * page, 1)).unwrap(); + assert_eq!(files.sync(&mut v).unwrap(), []); + let a = open(&mut files, &mut v, "home/a"); + files.write(&mut v, a.1, a.0, 0, &pattern(2 * page, 2)).unwrap(); + files.write(&mut v, b.1, b.0, 0, &pattern(2 * page, 3)).unwrap(); + + files.fsync(&mut v, Some((a.1, a.0))).unwrap(); + assert!(files.files[&b.1].dirty.is_some(), "the other file's overwrite was written back"); + assert!(files.owes(&v), "the other file's overwrite is owed to a later sync"); + let mut v = after_a_stop(v); + assert!(on_volume(&mut v, "home/a") == pattern(2 * page, 2), "the fsynced file is not as written"); + assert!(on_volume(&mut v, "home/b") == pattern(2 * page, 1), "the other file's overwrite reached the volume"); +} + +/// A file whose blocks the commit holds as taken is carried by an fsync of +/// another: its pages reach the volume first and its entry goes with them, +/// so after a stop it reads its own bytes, never an earlier tenant's. +#[test] +fn an_fsync_writes_first_the_pages_of_every_file_its_commit_carries() { + let page = PAGE as usize; + let mut v = stale_volume(); + let mut files = Files::new(Some(Fake::new(64, 16))); + let a = open(&mut files, &mut v, "home/a"); + let c = open(&mut files, &mut v, "home/c"); + files.write(&mut v, a.1, a.0, 0, &pattern(page, 4)).unwrap(); + files.write(&mut v, c.1, c.0, 0, &pattern(3 * page, 5)).unwrap(); + + files.fsync(&mut v, Some((a.1, a.0))).unwrap(); + assert!(!files.owes(&v), "the fsync left a change owed"); + let mut v = after_a_stop(v); + assert!(on_volume(&mut v, "home/a") == pattern(page, 4), "the fsynced file is not as written"); + let carried = on_volume(&mut v, "home/c"); + assert!(carried == pattern(3 * page, 5), "the carried file reads {} bytes, not its own", carried.len()); +} + +/// A file whose pages a sync could not write is owed after an fsync of +/// another, though that sync made its entry durable and the commit does not +/// carry it. +#[test] +fn a_file_a_sync_could_not_write_back_stays_owed_past_another_s_fsync() { + let page = PAGE as usize; + let mut v = volume(); + let fake = Fake::new(64, 16); + let mut files = Files::new(Some(fake.clone())); + let a = open(&mut files, &mut v, "home/a"); + let f = open(&mut files, &mut v, "home/f"); + files.write(&mut v, f.1, f.0, 0, &pattern(page, 6)).unwrap(); + fake.0.borrow_mut().unwritable = Some(files.files[&f.1].object); + assert_eq!(files.sync(&mut v).map(|u| u.len()), Ok(1), "the sync wrote the refused file's pages"); + assert!(!v.owes(), "the sync left f's entry owed"); + files.write(&mut v, a.1, a.0, 0, &pattern(page, 7)).unwrap(); + + files.fsync(&mut v, Some((a.1, a.0))).unwrap(); + assert!(files.owes(&v), "f's unwritten pages are owed to a later sync"); +} + +/// A directory's fsync commits the names every change already wrote. +#[test] +fn a_directorys_fsync_makes_a_new_name_durable() { + let mut v = stale_volume(); + let mut files = Files::new(Some(Fake::new(64, 16))); + v.mkdir("home/d").unwrap(); + files.fsync(&mut v, None).unwrap(); + let mut v = after_a_stop(v); + assert_eq!(v.lstat("home/d").map(|m| m.kind), Ok(crate::volume::Kind::Dir)); +} diff --git a/system/fileserver/src/root.rs b/system/fileserver/src/root.rs index 5160117f1f..1fba2ea160 100644 --- a/system/fileserver/src/root.rs +++ b/system/fileserver/src/root.rs @@ -250,6 +250,10 @@ impl Volume for RootVolume { Ok(Vec::new()) } + fn commit(&mut self, _node: Option) -> Result<(), Error> { + Ok(()) + } + /// None: ROOT is read-only. fn room(&mut self) -> Result { Ok(toyos::fs::Room { free: 0, unit: BLOCK as u64 }) diff --git a/system/fileserver/src/volume.rs b/system/fileserver/src/volume.rs index 25faa80d37..f6ada02f69 100644 --- a/system/fileserver/src/volume.rs +++ b/system/fileserver/src/volume.rs @@ -190,6 +190,28 @@ pub trait Volume { /// volume's own flush refused. fn sync(&mut self) -> Result, Error>; + /// Whether the volume's next commit carries `node`'s entry whatever it is + /// asked to: one written since the last, or one naming blocks taken since, + /// which the commit holds as taken. The file's pages reach the volume + /// before that commit, so no entry it makes durable names a block that + /// does not hold the file's bytes. + fn carried(&self, _node: Node) -> bool { + false + } + + /// Whether an open file's entry is changed past what a commit or sync + /// last made durable. Everything else written is durable after either. + fn owes(&self) -> bool { + false + } + + /// `node`'s entry durable, with every entry [`Self::carried`] names and + /// no other open file's: an fsync's commit. `None` commits only those, as + /// a directory's fsync does, whose names every change writes at once. + /// `Err` is `node`'s entry or the commit refused; another's refused entry + /// is said, and kept for the next commit. + fn commit(&mut self, node: Option) -> Result<(), Error>; + /// What more can be written to the volume: none, where nothing can be. fn room(&mut self) -> Result; diff --git a/tests/guest/osbench/README.md b/tests/guest/osbench/README.md index 6b488884ea..dbdd42f614 100644 --- a/tests/guest/osbench/README.md +++ b/tests/guest/osbench/README.md @@ -220,9 +220,11 @@ What makes the disk rows one comparison, and what stays each system's own: `nobarrier` would show in `fs.disk.mount`'s superblock options, and the device's write-cache mode is in `served_by` (`write cache write back`). On ToyOS, the NVMe driver issues Flush where the controller has a volatile - write cache, which its boot log says. ToyOS's fsync makes its whole volume - durable, not just the file - (`issues/an-fsync-of-a-served-file-syncs-its-whole-volume.md`). + write cache, which its boot log says. ToyOS's fsync makes its file + durable with every file the volume's commit carries beside it: one whose + entry was written or whose blocks were taken since the last commit + (`persist.write_4k_fsync_beside_1m` writes over another file's blocks, + which no commit carries). - **One procedure for the cache.** Every row reads only files it wrote in its own run, and neither system's cache is dropped before it. What then reaches the disk is each system's own write-back and cache policy, and diff --git a/tests/guest/osbench/src/storage.rs b/tests/guest/osbench/src/storage.rs index d5b7d371c2..4f02bc11f3 100644 --- a/tests/guest/osbench/src/storage.rs +++ b/tests/guest/osbench/src/storage.rs @@ -56,6 +56,28 @@ fn durable(disk: &files::Target) { Err(e) => refuse("persist.write_4k_fsync", e), } + // As `write_4k_fsync`, each after 1 MiB written over another 1 MiB file + // and not synced: 100 after 5. An fsync of one file that makes another's + // write durable too pays for it here. + let beside = scratch.at("beside"); + let made = (|| -> std::io::Result<(File, File)> { + fs::write(&beside, vec![2u8; MIB])?; + let other = OpenOptions::new().write(true).open(&beside)?; + other.sync_all()?; + Ok((OpenOptions::new().write(true).open(&file)?, other)) + })(); + let other_bytes = vec![0x3cu8; MIB]; + match made { + Ok((mut f, mut other)) => each("persist.write_4k_fsync_beside_1m", 5, 100, &served, |_| { + io("seek", other.seek(SeekFrom::Start(0)))?; + io("write", other.write_all(&other_bytes))?; + io("seek", f.seek(SeekFrom::Start(0)))?; + io("write", f.write_all(&block))?; + io("fsync", f.sync_all()) + }), + Err(e) => refuse("persist.write_4k_fsync_beside_1m", e), + } + // 4 KiB appended, then `sync_data`: 100 after 5, so the file grows by 420 KiB. match OpenOptions::new().create(true).append(true).open(scratch.at("log")) { Ok(mut f) => each("persist.append_4k_fsync", 5, 100, &served, |_| { diff --git a/tests/guest/rust-tests/src/bin/a_served_files_read_never_tears_against_its_write.rs b/tests/guest/rust-tests/src/bin/a_served_files_read_never_tears_against_its_write.rs new file mode 100644 index 0000000000..7658067e50 --- /dev/null +++ b/tests/guest/rust-tests/src/bin/a_served_files_read_never_tears_against_its_write.rs @@ -0,0 +1,56 @@ +//! XSH 2.9.7: a read and a write of one regular file are atomic with respect +//! to each other. A writer puts whole generations of a served file, each byte +//! the generation's number mod 256, while a reader on its own description +//! reads the whole file through its paged object; every read is one +//! generation. +//! +//! A guest test because the tear is an interleaving of the kernel's copy of a +//! holder's read with the file server's write into the same frames, on two +//! CPUs; the host suites hold each side's rule (`kernel::paged`, the file +//! server's `pages` tests). + +use std::fs::{self, OpenOptions}; +use std::os::toyos::fs::FileExt; +use std::sync::atomic::{AtomicBool, Ordering}; +use std::thread; + +const FILE: &str = "/home/a-served-files-read-never-tears"; +/// Sixty-four pages: four of the kernel's sixteen-page copies, inside one +/// request's window. +const BYTES: usize = 64 * 4096; +const GENERATIONS: u32 = 400; + +fn main() { + let _ = fs::remove_file(FILE); + let writer = OpenOptions::new().write(true).create(true).truncate(true).open(FILE).expect("open to write"); + assert_eq!(writer.write_at(&[0u8; BYTES], 0).expect("the first generation"), BYTES); + let reader = OpenOptions::new().read(true).open(FILE).expect("open to read"); + let done = AtomicBool::new(false); + + let (reads, seen) = thread::scope(|s| { + let reading = s.spawn(|| { + let mut buf = vec![0u8; BYTES]; + let (mut reads, mut seen) = (0u32, std::collections::BTreeSet::new()); + while !done.load(Ordering::Acquire) { + assert_eq!(reader.read_at(&mut buf, 0).expect("read"), BYTES, "a short read of a file no write shortens"); + if let Some(at) = buf.iter().position(|&b| b != buf[0]) { + panic!("read {reads} is torn: generation {} to byte {at}, then generation {}", buf[0], buf[at]); + } + seen.insert(buf[0]); + reads += 1; + } + (reads, seen) + }); + for generation in 1..=GENERATIONS { + let byte = generation as u8; + assert_eq!(writer.write_at(&[byte; BYTES], 0).expect("write"), BYTES, "generation {generation} written short"); + } + done.store(true, Ordering::Release); + reading.join().expect("the reader") + }); + // The premise: the reader ran beside the writes and met more than one. + assert!(seen.len() > 2, "{reads} reads met only generations {seen:?}: nothing ran beside the writes"); + drop((writer, reader)); + fs::remove_file(FILE).unwrap(); + println!("a_served_files_read_never_tears_against_its_write: {reads} reads, {} generations met, none torn", seen.len()); +}