From a4b6a1ee7da971cab2ab9d9005092285ab2cf67a Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 1 Nov 2026 18:06:57 +0100 Subject: [PATCH 1/5] osbench: persist.write_4k_fsync_beside_1m As persist.write_4k_fsync, each iteration first writing 1 MiB over another 1 MiB file and leaving it unsynced: an fsync of one file that also makes another file's write durable pays for it here. It is the A/B base for a per-file fsync (issues/an-fsync-of-a-served-file-syncs-its-whole-volume.md), which asks for one file's fsync measured beside the volume's. Co-Authored-By: Claude Opus 5.5 --- tests/guest/osbench/src/storage.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) 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, |_| { From 31206cbf4231d9e0473d5938839237e6d9548141 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 1 Nov 2026 18:06:14 +0100 Subject: [PATCH 2/5] paged: a holder's read is atomic against its pager's write (XSH 2.9.7) A holder's SYS_PAGED_READ copied a pinned chunk of 16 pages at a time while the file server's write landed page by page in the same frames, so a read beside a write of several pages could return pages from before and after it, or a page torn at the copy (memory stage 7a's recorded gap). Every write of an object's now runs as a write span on it (kernel::paged::Object): each write call and each dirty supply begins one before its first byte lands and ends it after its last, and a resize moves the object's version. A holder's read begins only while no span is open and counts its bytes only where the version has not moved since; a read a write met answers WouldBlock, and the SDK, which already asks the server whenever a holder read fails, reads through the server, whose loop orders the read against the write. A server write takes many calls when a page is absent, in part or at the dirty ceiling. A call that does not take all it was given leaves the span open (held) until the new PagedOp::EndWrite, and later calls nest in it, so the file server's whole write is one span. The server's first call of a write is always a write: one that takes everything is the whole write at no extra syscall; otherwise the server ends the held span on every path, the refused one included. Holder reads never wait on a write: the pager is never made to wait on its readers, and a read that met one costs one round trip. Tests: - kernel::paged: a read counts only where no write began or was under way; a short call holds the write until its end. - fileserver pages: a write of many calls (an absent page supplied, the dirty ceiling written back, a last page in part) is read whole or not at all at every call boundary, and ends whether it lands or is refused. - guest a_served_files_read_never_tears_against_its_write: a writer puts 400 whole generations of a 64-page served file while a reader on its own description reads it whole through its holder; red on a torn read. Closes issues/a-read-and-a-write-of-a-paged-file-can-tear-across-pages.md. Co-Authored-By: Claude Opus 5.5 --- ...e-of-a-paged-file-can-tear-across-pages.md | 23 --- kernel/pure/paged.rs | 99 ++++++++++- kernel/src/object/paged.rs | 107 ++++++++++-- kernel/src/syscall/paged.rs | 1 + sdk/abi/src/syscall.rs | 16 +- sdk/toyos/src/paged.rs | 10 +- system/fileserver/src/pages.rs | 46 +++-- system/fileserver/src/pages/tests.rs | 160 +++++++++++++----- ...iles_read_never_tears_against_its_write.rs | 55 ++++++ 9 files changed, 423 insertions(+), 94 deletions(-) delete mode 100644 issues/a-read-and-a-write-of-a-paged-file-can-tear-across-pages.md create mode 100644 tests/guest/rust-tests/src/bin/a_served_files_read_never_tears_against_its_write.rs 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/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..cf3436efc8 100644 --- a/sdk/abi/src/syscall.rs +++ b/sdk/abi/src/syscall.rs @@ -3311,15 +3311,18 @@ 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 holder's read + /// that meets a write is answered `WouldBlock`; 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 +3348,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. diff --git a/sdk/toyos/src/paged.rs b/sdk/toyos/src/paged.rs index dfca567c87..1f5bef1e30 100644 --- a/sdk/toyos/src/paged.rs +++ b/sdk/toyos/src/paged.rs @@ -123,6 +123,13 @@ impl Pager { self.op(PagedOp::Write, args, Some(SyscallError::NotFound)).map(|n| n as usize) } + /// End the write of object `id` that a [`Self::write`] or dirty + /// [`Self::supply`] which did not take all it was given left under way: + /// until this, every holder's read of it is asked of this pager instead. + pub fn end_write(&self, id: u64) -> 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/pages.rs b/system/fileserver/src/pages.rs index b93579abb6..9e0c6e3b7e 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; @@ -558,6 +577,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..f6b26ad159 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,11 @@ struct Model { supplies: u32, /// Every supply from here is refused. refusing: bool, + /// 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 +40,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 +65,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 +103,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 +124,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,6 +203,7 @@ impl Pager for Fake { page.bytes[(size % PAGE) as usize..].fill(0); } m.sizes.insert(id, size); + m.changed(id, true); Ok(()) } @@ -475,3 +514,44 @@ 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"); +} 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..1e532242e2 --- /dev/null +++ b/tests/guest/rust-tests/src/bin/a_served_files_read_never_tears_against_its_write.rs @@ -0,0 +1,55 @@ +//! 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, 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()); +} From b3897d50a3a5282fbc47254f2917d48ea4a19882 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 1 Nov 2026 18:09:40 +0100 Subject: [PATCH 3/5] fileserver: an fsync commits its own file, and what the commit carries FSYNC synced the whole volume: every dirty page of every file written back and every open file's entry written before the format's commit. An fsync now makes durable the file it names and only what the volume's commit cannot leave out: - Volume::carried names the open files the next commit carries whatever it is asked: on DATA one whose entry was written since the last commit (a rename, a shrink) or whose reserve took blocks since, which the commit's allocator holds as taken; on FAT every changed open file, since a cache flush writes every chain the cache holds. - Files::fsync writes back the named file's pages and those of every carried file, then Volume::commit writes the named entry and each carried one and commits. A carried file whose pages are refused refuses the commit: its entry would name blocks without its bytes. - So after a stop no entry names a block holding an earlier tenant's bytes. A write over blocks a commit already named is carried by nothing, and waits for its own fsync or the volume's sync. A directory's fsync commits the carried files only: DATA writes every name change into the tree at once. SYNC and the write-back timer still sync the whole volume, and a per-file fsync leaves the timer due. Found while doing this: a sync commits the entry of a file whose pages were refused (issues/a-sync-commits-the-entry-of-a-file-whose-pages-were-refused.md). Tests (fileserver pages): an fsync makes its file durable across a stop and leaves another file's overwrite unwritten; a carried file's pages and entry go with another's fsync, so after a stop on a volume of stale blocks it reads its own bytes; a directory's fsync makes a new name durable. issues/an-fsync-of-a-served-file-syncs-its-whole-volume.md stays open until its measurement, osbench's persist.write_4k_fsync_beside_1m base against head on metal, is recorded. Co-Authored-By: Claude Opus 5.5 --- ...ntry-of-a-file-whose-pages-were-refused.md | 29 ++++++++ ...of-a-served-file-syncs-its-whole-volume.md | 12 +++- system/fileserver/src/absent.rs | 4 ++ system/fileserver/src/data.rs | 52 ++++++++++++-- system/fileserver/src/fat.rs | 24 +++++++ system/fileserver/src/main.rs | 30 ++++---- system/fileserver/src/pages.rs | 22 ++++++ system/fileserver/src/pages/tests.rs | 70 +++++++++++++++++++ system/fileserver/src/root.rs | 4 ++ system/fileserver/src/volume.rs | 16 +++++ tests/guest/osbench/README.md | 8 ++- 11 files changed, 245 insertions(+), 26 deletions(-) create mode 100644 issues/a-sync-commits-the-entry-of-a-file-whose-pages-were-refused.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/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..2bbe6a7af6 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,31 @@ 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 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 }) } @@ -1160,9 +1204,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..a903c586e8 100644 --- a/system/fileserver/src/fat.rs +++ b/system/fileserver/src/fat.rs @@ -433,6 +433,30 @@ 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 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 { diff --git a/system/fileserver/src/main.rs b/system/fileserver/src/main.rs index 92133ef966..732e2877bb 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,14 @@ 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)?; + 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 +1258,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 9e0c6e3b7e..35a9df05ca 100644 --- a/system/fileserver/src/pages.rs +++ b/system/fileserver/src/pages.rs @@ -549,6 +549,28 @@ 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)) + } + /// What the pager's objects asked. pub fn notice(&mut self, volume: &mut dyn Volume, notice: Notice) { match notice { diff --git a/system/fileserver/src/pages/tests.rs b/system/fileserver/src/pages/tests.rs index f6b26ad159..5da1f04ac5 100644 --- a/system/fileserver/src/pages/tests.rs +++ b/system/fileserver/src/pages/tests.rs @@ -555,3 +555,73 @@ fn a_write_of_many_calls_is_read_whole_or_not_at_all() { 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"); + 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 names no block that holds an earlier tenant's bytes, +/// and no block is held that nothing names. +#[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(); + 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 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..713ef86253 100644 --- a/system/fileserver/src/volume.rs +++ b/system/fileserver/src/volume.rs @@ -190,6 +190,22 @@ 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 + } + + /// `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 From 6ae35a46e9fcd7b7bb86fdec650dc6c12e3f7750 Mon Sep 17 00:00:00 2001 From: japabu Date: Sun, 1 Nov 2026 19:52:59 +0100 Subject: [PATCH 4/5] review r1 of #1141: a holder's WouldBlock is paged_read's, and the claims narrowed - R1: the holder's answer WouldBlock is documented on paged_read, the boundary a holder implements against; PagedOp::Write keeps only the EndWrite rule. - R3: a carried file's test no longer claims no block is held that no entry names: DataVolume::commit commits past a refused carried entry, whose taken blocks then leak after a stop. Filed with S3, one carried file's refused write-back refusing every fsync on the volume (issues/a-refused-carried-file-costs-every-other-files-fsync-or-leaks-its-blocks.md). - S2: the tear member's generations are its bytes mod 256. R2 is in the fsync commit: its issue stays open until the A/B records the exit's measurement. S1 (the version move at a write's end is redundant) is declined for this round: it changes the kernel the staged A/B images were built from. Comments, ABI docs and issue records only: no code changes, so the staged images at b4edede36 still measure this head. Co-Authored-By: Claude Opus 5.5 --- ...y-other-files-fsync-or-leaks-its-blocks.md | 37 +++++++++++++++++++ sdk/abi/src/syscall.rs | 9 +++-- system/fileserver/src/pages/tests.rs | 3 +- ...iles_read_never_tears_against_its_write.rs | 5 ++- 4 files changed, 46 insertions(+), 8 deletions(-) create mode 100644 issues/a-refused-carried-file-costs-every-other-files-fsync-or-leaks-its-blocks.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/sdk/abi/src/syscall.rs b/sdk/abi/src/syscall.rs index cf3436efc8..dd0911e4bb 100644 --- a/sdk/abi/src/syscall.rs +++ b/sdk/abi/src/syscall.rs @@ -3320,9 +3320,8 @@ pub enum PagedOp { /// 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. A holder's read - /// that meets a write is answered `WouldBlock`; a write that does not take - /// all it was given stays under way until [`PagedOp::EndWrite`]. + /// 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. @@ -3422,7 +3421,9 @@ pub fn paged_holder(pager: RawHandle, object: u64) -> Result Date: Mon, 2 Nov 2026 00:06:06 +0100 Subject: [PATCH 5/5] fileserver: an fsync that leaves nothing owed answers the write-back The volume-wide sync an fsync used to run cleared the write-back the server's loop keeps due. The per-file fsync never did: after it, the server waited every turn with a deadline, which every park and idle entry programs into the CPU's timer, and 2 s later walked the volume in a sync with nothing left to write. The volume now says whether an open file's entry is changed past what its last commit made durable (Volume::owes), and Files adds its dirty pages (Files::owes). An fsync that leaves neither answers the write-back as a sync does. One that leaves another file's write keeps it due from that write. Co-Authored-By: Claude Opus 5.5 --- system/fileserver/src/data.rs | 22 ++++++++++++++++++++++ system/fileserver/src/fat.rs | 18 ++++++++++++++++++ system/fileserver/src/main.rs | 4 ++++ system/fileserver/src/pages.rs | 6 ++++++ system/fileserver/src/pages/tests.rs | 28 ++++++++++++++++++++++++++++ system/fileserver/src/volume.rs | 6 ++++++ 6 files changed, 84 insertions(+) diff --git a/system/fileserver/src/data.rs b/system/fileserver/src/data.rs index 2bbe6a7af6..75389f9902 100644 --- a/system/fileserver/src/data.rs +++ b/system/fileserver/src/data.rs @@ -858,6 +858,10 @@ impl Volume for DataVolume { 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)?; @@ -1186,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. diff --git a/system/fileserver/src/fat.rs b/system/fileserver/src/fat.rs index a903c586e8..ec75dbf9f1 100644 --- a/system/fileserver/src/fat.rs +++ b/system/fileserver/src/fat.rs @@ -439,6 +439,10 @@ impl Volume for FatVolume { 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(()); @@ -573,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 732e2877bb..e4d46290ec 100644 --- a/system/fileserver/src/main.rs +++ b/system/fileserver/src/main.rs @@ -934,6 +934,10 @@ impl Server { Held::Dir { .. } => None, }; 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(), diff --git a/system/fileserver/src/pages.rs b/system/fileserver/src/pages.rs index 35a9df05ca..490439887f 100644 --- a/system/fileserver/src/pages.rs +++ b/system/fileserver/src/pages.rs @@ -571,6 +571,12 @@ impl Files

{ 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 { diff --git a/system/fileserver/src/pages/tests.rs b/system/fileserver/src/pages/tests.rs index 741e092b72..a35bdceb39 100644 --- a/system/fileserver/src/pages/tests.rs +++ b/system/fileserver/src/pages/tests.rs @@ -23,6 +23,8 @@ 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 @@ -208,6 +210,9 @@ impl Pager for Fake { } 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 { @@ -589,6 +594,7 @@ fn an_fsync_makes_its_file_durable_and_no_other_files_overwrite() { 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"); @@ -608,12 +614,34 @@ fn an_fsync_writes_first_the_pages_of_every_file_its_commit_carries() { 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() { diff --git a/system/fileserver/src/volume.rs b/system/fileserver/src/volume.rs index 713ef86253..f6ada02f69 100644 --- a/system/fileserver/src/volume.rs +++ b/system/fileserver/src/volume.rs @@ -199,6 +199,12 @@ pub trait Volume { 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.