diff --git a/issues/two-holders-racing-on-an-exported-socket-can-both-stall.md b/issues/two-holders-racing-on-an-exported-socket-can-both-stall.md new file mode 100644 index 00000000000..1dc15e0ff62 --- /dev/null +++ b/issues/two-holders-racing-on-an-exported-socket-can-both-stall.md @@ -0,0 +1,27 @@ +--- +status: open +kind: defect +opened: 2026-10-24 +--- + +# Two holders racing on an exported socket can both stall + +`netstack_holders_exported_to_children`'s last step (`tests/guest/rust-tests/src/bin/socket_export.rs`, +`race`) has the parent and a second child each block on a listener and a +datagram socket they both hold, while the harness connects twice and sends two +datagrams. Now and then neither process goes on: the racer is spawned +(`spawn: /system/bin/test_rs_socket_export`), the first part's line is printed, +and the guest says nothing more until the harness's ceiling (952–970 s guards +expired). + +QEMU x86-64 under TCG on an aarch64 Mac, the test alone, `--nocapture`: one +stall in six runs at `32046264b` (main), two in six at PR #1060's head, whose +change touches no socket or netstack code. Each stalled run's last guest lines +are the racer's spawn and netstack's mDNS line. Logs: PR #1060's evidence, +`qemu/netstack-*`. + +What blocks is not established: no guest line says whether either racer sent +its `racing` datagram, which the harness waits for before it connects. + +**Exit**: the race finishes in every run of a repeated QEMU loop, with the +blocked step named by a line the guest prints, or the cause found and fixed. diff --git a/lib/bcachefs/src/crc32c.rs b/lib/bcachefs/src/crc32c.rs deleted file mode 100644 index c80e4dea65a..00000000000 --- a/lib/bcachefs/src/crc32c.rs +++ /dev/null @@ -1,72 +0,0 @@ -//! Software CRC-32c (Castagnoli) implementation. -//! -//! Uses the polynomial 0x1EDC6F41. This is the same CRC used by iSCSI, ext4, -//! btrfs, and bcachefs. On x86 with SSE4.2, the hardware `crc32` instruction -//! computes this same polynomial. - -const CRC32C_TABLE: [u32; 256] = { - let mut table = [0u32; 256]; - let mut i = 0u32; - while i < 256 { - let mut crc = i; - let mut j = 0; - while j < 8 { - if crc & 1 != 0 { - crc = (crc >> 1) ^ 0x82F63B78; - } else { - crc >>= 1; - } - j += 1; - } - table[i as usize] = crc; - i += 1; - } - table -}; - -/// Compute CRC-32c over a byte slice. -pub fn crc32c(data: &[u8]) -> u32 { - update(0xFFFF_FFFF, data) ^ 0xFFFF_FFFF -} - -/// The same register with no initial or final inversion: bcachefs's data -/// checksum type 5, whose value over zeros is 0. -pub fn crc32c_zero(data: &[u8]) -> u32 { - update(0, data) -} - -fn update(mut crc: u32, data: &[u8]) -> u32 { - for &byte in data { - let idx = ((crc ^ byte as u32) & 0xFF) as usize; - crc = (crc >> 8) ^ CRC32C_TABLE[idx]; - } - crc -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn known_vectors() { - // CRC-32c of empty input - assert_eq!(crc32c(b""), 0x00000000); - // CRC-32c of "123456789" - assert_eq!(crc32c(b"123456789"), 0xE3069283); - } - - #[test] - fn the_uninverted_register_has_its_own_check_value() { - assert_eq!(crc32c_zero(b"123456789"), 0x58E3_FA20); - assert_eq!(crc32c_zero(&[0; 4096]), 0); - } - - #[test] - fn zeroes() { - // CRC of all zeroes should be deterministic - let zeros = [0u8; 4096]; - let c = crc32c(&zeros); - assert_eq!(c, crc32c(&zeros)); - assert_ne!(c, 0); - } -} diff --git a/lib/bcachefs/src/crc32c/aarch64.rs b/lib/bcachefs/src/crc32c/aarch64.rs new file mode 100644 index 00000000000..9ed69df359c --- /dev/null +++ b/lib/bcachefs/src/crc32c/aarch64.rs @@ -0,0 +1,26 @@ +//! FEAT_CRC32's `CRC32CX` and `CRC32CB`, which accumulate CRC-32C (Arm ARM, +//! DDI 0487, "CRC32C"; mandatory from Armv8.1, ID_AA64ISAR0_EL1.CRC32). +//! Every AArch64 machine ToyOS supports has it, and so do the hosts that +//! build and test it. + +use core::arch::aarch64::{__crc32cb, __crc32cd}; + +pub fn update(crc: u32, data: &[u8]) -> u32 { + // SAFETY: the CPU has FEAT_CRC32 (the module header), which is all + // `accumulate` asks of its caller. + unsafe { accumulate(crc, data) } +} + +/// # Safety +/// The CPU has FEAT_CRC32. +#[target_feature(enable = "crc")] +unsafe fn accumulate(mut crc: u32, data: &[u8]) -> u32 { + let (words, tail) = data.as_chunks::<8>(); + for word in words { + crc = __crc32cd(crc, u64::from_le_bytes(*word)); + } + for &byte in tail { + crc = __crc32cb(crc, byte); + } + crc +} diff --git a/lib/bcachefs/src/crc32c/mod.rs b/lib/bcachefs/src/crc32c/mod.rs new file mode 100644 index 00000000000..5f504ec8715 --- /dev/null +++ b/lib/bcachefs/src/crc32c/mod.rs @@ -0,0 +1,95 @@ +//! CRC-32C (Castagnoli, polynomial 0x1EDC6F41), by the CPU's own instruction: +//! SSE4.2's `CRC32` on x86-64, FEAT_CRC32's `CRC32C` on AArch64. Every +//! machine ToyOS runs on and every host that builds or tests it has one, so +//! nothing computes it any other way. + +#[cfg_attr(target_arch = "x86_64", path = "x86_64.rs")] +#[cfg_attr(target_arch = "aarch64", path = "aarch64.rs")] +mod arch; + +#[cfg(not(any(target_arch = "x86_64", target_arch = "aarch64")))] +compile_error!("bcachefs computes CRC-32C with the CPU's instruction, and names one for x86-64 and AArch64 alone"); + +/// Compute CRC-32c over a byte slice. +pub fn crc32c(data: &[u8]) -> u32 { + arch::update(0xFFFF_FFFF, data) ^ 0xFFFF_FFFF +} + +/// The same register with no initial or final inversion: bcachefs's data +/// checksum type 5, whose value over zeros is 0. +pub fn crc32c_zero(data: &[u8]) -> u32 { + arch::update(0, data) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The oracle: the reflected polynomial a bit at a time, sharing nothing + /// with the instruction but the register's definition. + fn reference(mut crc: u32, data: &[u8]) -> u32 { + for &byte in data { + crc ^= u32::from(byte); + for _ in 0..8 { + crc = if crc & 1 != 0 { (crc >> 1) ^ 0x82F6_3B78 } else { crc >> 1 }; + } + } + crc + } + + /// RFC 3720's own examples (Appendix B.4), and the catalogue's check + /// value over "123456789". + #[test] + fn rfc_3720_vectors() { + let ascending: Vec = (0..32).collect(); + let descending: Vec = (0..32).rev().collect(); + #[rustfmt::skip] + let read_pdu: [u8; 48] = [ + 0x01, 0xc0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x14, 0x00, 0x00, 0x00, 0x00, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x14, 0x00, 0x00, 0x00, 0x18, + 0x28, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + ]; + let vectors: [(&[u8], u32); 6] = [ + (&[0; 32], 0x8A91_36AA), + (&[0xFF; 32], 0x62A8_AB43), + (&ascending, 0x46DD_794E), + (&descending, 0x113F_DB5C), + (&read_pdu, 0xD996_3A56), + (b"123456789", 0xE306_9283), + ]; + for (data, want) in vectors { + assert_eq!(crc32c(data), want, "{data:02x?}"); + assert_eq!(reference(!0, data) ^ !0, want, "the oracle on {data:02x?}"); + } + assert_eq!(crc32c(b""), 0); + } + + #[test] + fn the_uninverted_register_has_its_own_check_value() { + assert_eq!(crc32c_zero(b"123456789"), 0x58E3_FA20); + assert_eq!(crc32c_zero(&[0; 4096]), 0); + } + + /// Every length to past two words, at every alignment a word can have, + /// and whole nodes, against the oracle: the instruction's word steps and + /// its byte tail agree with a bit at a time. + #[test] + fn the_instruction_is_the_oracle_at_every_length_and_alignment() { + let mut state = 0x9E37_79B9_7F4A_7C15u64; + let bytes: Vec = (0..16_384 + 8) + .map(|_| { + state ^= state << 13; + state ^= state >> 7; + state ^= state << 17; + state as u8 + }) + .collect(); + for at in 0..8 { + for len in (0..=24).chain([4088, 4096, 16_384]) { + let data = &bytes[at..at + len]; + assert_eq!(crc32c(data), reference(!0, data) ^ !0, "{len} bytes at {at}"); + assert_eq!(crc32c_zero(data), reference(0, data), "{len} bytes at {at}, uninverted"); + } + } + } +} diff --git a/lib/bcachefs/src/crc32c/x86_64.rs b/lib/bcachefs/src/crc32c/x86_64.rs new file mode 100644 index 00000000000..34dda6c7f42 --- /dev/null +++ b/lib/bcachefs/src/crc32c/x86_64.rs @@ -0,0 +1,29 @@ +//! SSE4.2's `CRC32`, which accumulates CRC-32C (Intel SDM Vol. 2A, "CRC32 — +//! Accumulate CRC32 Value"; CPUID.01H:ECX.SSE4_2[bit 20]). Every x86-64 part +//! ToyOS supports, 2020 and later, has it, and so do the hosts that build and +//! test it. + +use core::arch::x86_64::{_mm_crc32_u64, _mm_crc32_u8}; + +pub fn update(crc: u32, data: &[u8]) -> u32 { + // SAFETY: the CPU has SSE4.2 (the module header), which is all + // `accumulate` asks of its caller. + unsafe { accumulate(crc, data) } +} + +/// # Safety +/// The CPU has SSE4.2. +#[target_feature(enable = "sse4.2")] +unsafe fn accumulate(crc: u32, data: &[u8]) -> u32 { + let (words, tail) = data.as_chunks::<8>(); + let mut wide = u64::from(crc); + for word in words { + wide = _mm_crc32_u64(wide, u64::from_le_bytes(*word)); + } + // The instruction's 64-bit form leaves the register in the low half. + let mut crc = wide as u32; + for &byte in tail { + crc = _mm_crc32_u8(crc, byte); + } + crc +} diff --git a/lib/bcachefs/src/fs.rs b/lib/bcachefs/src/fs.rs index 662615095b7..ddf3da0179a 100644 --- a/lib/bcachefs/src/fs.rs +++ b/lib/bcachefs/src/fs.rs @@ -54,6 +54,14 @@ fn push_extent(extents: &mut Vec, start: u64, count: u32) { extents.push(Extent { start_block: start, block_count: count, _reserved: 0 }); } +/// What an entry records besides its name, as [`Mounted::list`] answers it. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub struct Recorded { + pub symlink: bool, + pub size: u64, + pub mtime: u64, +} + /// Filesystem error type with rich context. #[derive(Debug)] pub enum FsError { @@ -706,11 +714,11 @@ impl Mounted { } } - /// Up to `limit` of the files `keep` accepts, as (name, size); more is - /// refused before any materialise. `limit` counts the kept names, so a - /// caller listing one directory pays that directory's bound and not the - /// volume's. - pub fn list(&self, limit: usize, keep: &dyn Fn(&str) -> bool) -> Result, FsError> { + /// Up to `limit` of the files `keep` accepts, with what each one's entry + /// records; more is refused before any materialise. `limit` counts the + /// kept names, so a caller listing one directory pays that directory's + /// bound and not the volume's. + pub fn list(&self, limit: usize, keep: &dyn Fn(&str) -> bool) -> Result, FsError> { let mut result = Vec::new(); let ControlFlow::Continue(()) = btree::for_each_live(&self.io, self.sb.root_node, &mut |entry| { let Ok(leaf) = self.decode(&entry.value) else { return Ok(ControlFlow::Continue(())) }; @@ -720,7 +728,8 @@ impl Mounted { if result.len() >= limit { return Err(FsError::ListTooLong { limit }); } - result.push((String::from(leaf.name()), leaf.size())); + let symlink = matches!(leaf, LeafValue::Symlink { .. }); + result.push((String::from(leaf.name()), Recorded { symlink, size: leaf.size(), mtime: leaf.mtime() })); Ok(ControlFlow::Continue(())) })? else { unreachable!("`list`'s visitor never breaks") diff --git a/lib/bcachefs/src/lib.rs b/lib/bcachefs/src/lib.rs index acd9044b517..2d01bf93eb1 100644 --- a/lib/bcachefs/src/lib.rs +++ b/lib/bcachefs/src/lib.rs @@ -26,7 +26,7 @@ pub mod engine; pub use block_io::{BlockIO, BlockBuf, BlockNum, DeviceError, TransferError}; #[cfg(feature = "std")] pub use block_io::VecBlockIO; -pub use fs::{file_entry_fits, Formatted, Mounted, ReadOnly, ReadWrite, FsError, Extent}; +pub use fs::{file_entry_fits, Formatted, Mounted, ReadOnly, ReadWrite, Recorded, FsError, Extent}; pub use superblock::{DESIGNATION_BLOCKS_OFFSET, DESIGNATION_MAGIC, FsUuid, Superblock}; /// Records the largest single allocation each test thread makes, so a test can diff --git a/lib/bcachefs/tests/integration.rs b/lib/bcachefs/tests/integration.rs index 5a3e5709c50..f7cab590bab 100644 --- a/lib/bcachefs/tests/integration.rs +++ b/lib/bcachefs/tests/integration.rs @@ -21,7 +21,7 @@ fn create_single_small_file() { let files = mounted.list(usize::MAX, &|_| true).expect("list failed"); assert_eq!(files.len(), 1); assert_eq!(files[0].0, "hello.txt"); - assert_eq!(files[0].1, 13); + assert_eq!(files[0].1, bcachefs::Recorded { symlink: false, size: 13, mtime: 42 }); let data = mounted.read_file("hello.txt").expect("read failed"); assert_eq!(data, b"Hello, world!"); @@ -134,7 +134,7 @@ fn empty_file() { let files = mounted.list(usize::MAX, &|_| true).expect("list"); assert_eq!(files.len(), 1); assert_eq!(files[0].0, "empty"); - assert_eq!(files[0].1, 0); + assert_eq!(files[0].1.size, 0); let data = mounted.read_file("empty").expect("read empty"); assert!(data.is_empty()); diff --git a/sdk/toyos/src/fs.rs b/sdk/toyos/src/fs.rs index d98ac1be3fe..f2ddf50e074 100644 --- a/sdk/toyos/src/fs.rs +++ b/sdk/toyos/src/fs.rs @@ -31,9 +31,11 @@ //! **A request carries a tag and its span of the window.** The client lends //! the server a [`WINDOW_BYTES`] region at connect ([`HELLO`]); a request's //! paths, data and answer travel in the span it names, and the frames stay -//! small and fixed. The reply carries the request's tag. A client keeps at -//! most [`MAX_OUTSTANDING`] requests unanswered on one connection and lets no -//! two of their spans overlap; a server reads a request's input out of its +//! small and fixed. The reply carries the request's tag; a [`CLOSE`] has +//! none, so a close costs its client a send and no wait and is never +//! outstanding. A client keeps at most [`MAX_OUTSTANDING`] requests +//! unanswered on one connection and lets no two of their spans overlap; a +//! server reads a request's input out of its //! span before it writes its answer there. The window is shared memory both //! sides write, so each side copies what it reads out of it once and acts on //! the copy. @@ -93,6 +95,8 @@ pub const OPEN: u32 = 2; /// A file with no name, in the directory the path names, opened by `flags` as /// [`OPEN`] opens: a later [`LINK`] gives it one, and nothing else ever does. pub const CREATE_UNNAMED: u32 = 3; +/// Let the id `fid` go. Answered with no frame: a close the volume refuses +/// keeps its node for the next sync, which the server says in its log. pub const CLOSE: u32 = 4; /// Read the span's length at the description's offset into the span, and move /// the offset past it: `value` answers how much. @@ -927,8 +931,14 @@ impl Dir { Ok(Opened { fid, stat: stat_of(&reply) }) } + /// Let `fid` go, without waiting for the server: [`CLOSE`] is answered + /// with nothing. pub fn close(&mut self, fid: Fid) { - let _ = self.fid_call(CLOSE, fid, Request::new(), 0); + let Ok(id) = self.held(fid) else { return }; + let conn = self.conn.as_ref().expect("an id is held on a connection"); + if conn.send(CLOSE, &Request { fid: id, ..Request::new() }).is_err() { + self.conn = None; + } } /// Read at most `out.len()` bytes, and at most the window, at the diff --git a/system/fileserver/src/data.rs b/system/fileserver/src/data.rs index 5a08a64728a..15739097bba 100644 --- a/system/fileserver/src/data.rs +++ b/system/fileserver/src/data.rs @@ -8,9 +8,9 @@ //! boot — and a directory with anything under it is one whether or not that //! entry exists, since volumes written before these entries carry none. //! -//! **Every name is indexed in memory at mount** ([`DataVolume::names`]), so a -//! lookup, a listing and "is this a directory" are a map query and never the -//! walk of the whole tree the format's own `is_dir` is. +//! **Every name is indexed in memory at mount** ([`DataVolume::names`]), with +//! what its entry records, so a lookup, a stat, a listing and "is this a +//! directory" are map queries and never a search of the tree. //! //! **A file's data lives in the blocks its extents name, in the cache.** A //! write resolves (allocating where it must) the block of each page it @@ -32,6 +32,7 @@ //! before it reaches the tree. use std::collections::BTreeMap; +use std::ops::Bound; use std::rc::Rc; use bcachefs::{Extent, Formatted, FsError, Mounted, ReadWrite}; @@ -61,8 +62,10 @@ struct Open { pub struct DataVolume { fs: Mounted, ReadWrite>, cache: Rc>, - /// Every name on the volume, a directory entry's without its `/`. - names: BTreeMap, + /// Every name on the volume, a directory entry's without its `/`, and + /// what its entry records: for a file the length and time its last + /// written entry holds, which an open node's own supersede. + names: BTreeMap, /// Directories that exist whatever the volume holds: each connection's root. roots: Vec, open: BTreeMap, @@ -148,6 +151,9 @@ fn disk_word(e: toyos_blockdev::Error) -> Error { } } +/// What a directory is: the format keeps no length or time for one. +const DIR: Meta = Meta { kind: Kind::Dir, size: 0, mtime: 0 }; + /// The block holding page `page`, if the extents reach it. fn block_for(extents: &[Extent], page: u64) -> Option { let mut at = 0u64; @@ -246,16 +252,16 @@ impl DataVolume { ) -> Result { let listed = fs.list(usize::MAX, &|_| true).map_err(|e| format!("the volume would not list: {e:?}"))?; let mut names = BTreeMap::new(); - for (name, _) in listed { - let kind = match name.strip_suffix('/') { - Some(dir) => { - names.insert(dir.to_string(), Kind::Dir); + for (name, recorded) in listed { + let meta = match (name.strip_suffix('/'), recorded.symlink) { + (Some(dir), _) => { + names.insert(dir.to_string(), DIR); continue; } - None if fs.is_symlink(&name).map_err(|e| format!("{e:?}"))? => Kind::Symlink, - None => Kind::File, + (None, true) => Meta { kind: Kind::Symlink, size: recorded.size, mtime: recorded.mtime }, + (None, false) => Meta { kind: Kind::File, size: recorded.size, mtime: recorded.mtime }, }; - names.insert(name, kind); + names.insert(name, meta); } Ok(Self { fs, @@ -293,13 +299,26 @@ impl DataVolume { self.names.range(prefix.clone()..).next().is_some_and(|(name, _)| name.starts_with(&prefix)) } + fn kind(&self, path: &str) -> Option { + self.names.get(path).map(|meta| meta.kind) + } + fn is_dir(&self, path: &str) -> bool { path.is_empty() || self.roots.iter().any(|r| r == path) - || self.names.get(path) == Some(&Kind::Dir) + || self.kind(path) == Some(Kind::Dir) || (!self.names.contains_key(path) && self.has_children(path)) } + /// What `path`'s file or symlink is: its open node's length and time + /// where it has one, and its entry's otherwise. + fn current(&self, path: &str, recorded: Meta) -> Meta { + match self.by_path.get(path).and_then(|n| self.open.get(n)) { + Some(open) => Meta { kind: recorded.kind, size: open.size, mtime: open.mtime }, + None => recorded, + } + } + fn entry(&mut self, node: Node) -> Result<&mut Open, Error> { let open = self.open.get_mut(&node).ok_or(Error::Stale)?; if open.gone { @@ -316,6 +335,9 @@ impl DataVolume { } mapped("the entry", &open.path, self.fs.update_metadata(&open.path, &open.extents, open.size, open.mtime))?; open.dirty = false; + if let Some(meta) = self.names.get_mut(&open.path) { + (meta.size, meta.mtime) = (open.size, open.mtime); + } Ok(()) } @@ -350,7 +372,7 @@ impl DataVolume { fn require_parent_dir(&self, path: &str) -> Result<(), Error> { let mut dir = parent(path); while !dir.is_empty() { - if matches!(self.names.get(dir), Some(Kind::File | Kind::Symlink)) { + if matches!(self.kind(dir), Some(Kind::File | Kind::Symlink)) { return Err(Error::NotADirectory); } dir = parent(dir); @@ -365,23 +387,15 @@ impl Volume for DataVolume { } fn lstat(&mut self, path: &str) -> Result { - match self.names.get(path).copied() { - Some(Kind::Dir) => Ok(Meta { kind: Kind::Dir, size: 0, mtime: 0 }), - Some(kind) => { - if let Some(open) = self.by_path.get(path).and_then(|n| self.open.get(n)) { - return Ok(Meta { kind, size: open.size, mtime: open.mtime }); - } - let (_, size) = mapped("a lookup", path, self.fs.file_extents(path))?.ok_or(Error::NotFound)?; - let mtime = mapped("a lookup", path, self.fs.file_mtime(path))?.ok_or(Error::NotFound)?; - Ok(Meta { kind, size, mtime }) - } - None if self.is_dir(path) => Ok(Meta { kind: Kind::Dir, size: 0, mtime: 0 }), + match self.names.get(path) { + Some(&recorded) => Ok(self.current(path, recorded)), + None if self.is_dir(path) => Ok(DIR), None => Err(Error::NotFound), } } fn read_link(&mut self, path: &str) -> Result { - if self.names.get(path) != Some(&Kind::Symlink) { + if self.kind(path) != Some(Kind::Symlink) { return Err(if self.names.contains_key(path) || self.is_dir(path) { Error::NotALink } else { Error::NotFound }); } mapped("readlink", path, self.fs.read_link(path, MAX_LINK))?.ok_or(Error::Io) @@ -391,29 +405,35 @@ impl Volume for DataVolume { if !self.is_dir(dir) { return Err(if self.names.contains_key(dir) { Error::NotADirectory } else { Error::NotFound }); } + // The names beneath a child directory are passed over whole: a + // listing costs the directory's children, not everything below it. let prefix = if dir.is_empty() { String::new() } else { format!("{dir}/") }; - let mut children: BTreeMap = BTreeMap::new(); - for (name, kind) in self.names.range(prefix.clone()..) { - let Some(rest) = name.strip_prefix(&prefix) else { break }; - match rest.split_once('/') { - Some((child, _)) => { - children.insert(child.to_string(), Kind::Dir); - } - None => { - children.entry(rest.to_string()).or_insert(*kind); + let mut children = Vec::new(); + let mut from = prefix.clone(); + loop { + let mut past = None; + for (name, &recorded) in self.names.range::((Bound::Included(from.as_str()), Bound::Unbounded)) { + let Some(rest) = name.strip_prefix(&prefix) else { break }; + let Some((child, _)) = rest.split_once('/') else { + children.push((rest.to_string(), self.current(name, recorded))); + continue; + }; + // A child with an entry of its own was listed at it. + if !self.names.contains_key(&format!("{prefix}{child}")) { + children.push((child.to_string(), DIR)); } + // Every name beneath `child/`, and nothing else, sorts before + // `child0`: `0` follows `/`. + past = Some(format!("{prefix}{child}0")); + break; + } + match past { + Some(next) => from = next, + None => break, } } - let mut out = Vec::with_capacity(children.len()); - for (name, kind) in children { - let path = join(dir, &name); - let meta = match kind { - Kind::Dir => Meta { kind, size: 0, mtime: 0 }, - _ => self.lstat(&path)?, - }; - out.push(Listed { name, meta, identity: self.ids.of(path) }); - } - Ok(out) + children.sort_unstable_by(|(a, _), (b, _)| a.cmp(b)); + Ok(children.into_iter().map(|(name, meta)| Listed { identity: self.ids.of(join(dir, &name)), name, meta }).collect()) } fn open(&mut self, path: &str, how: OpenHow) -> Result { @@ -425,10 +445,9 @@ impl Volume for DataVolume { Some(node) => node, None => match self.names.get(path).copied() { Some(_) if how.create_new => return Err(Error::AlreadyExists), - Some(Kind::File) => { + Some(Meta { kind: Kind::File, mtime, .. }) => { let (extents, size) = mapped("open", path, self.fs.file_extents(path))?.ok_or(Error::NotFound)?; - let mtime = mapped("open", path, self.fs.file_mtime(path))?.ok_or(Error::NotFound)?; self.new_node(path, extents, size, mtime) } // A link the resolver did not follow is one with nothing behind it. @@ -437,7 +456,7 @@ impl Volume for DataVolume { self.require_parent_dir(path)?; let now = (self.clock)(); mapped("create", path, self.fs.create(path, &[], now))?; - self.names.insert(path.to_string(), Kind::File); + self.names.insert(path.to_string(), Meta { kind: Kind::File, size: 0, mtime: now }); self.new_node(path, Vec::new(), 0, now) } None => return Err(Error::NotFound), @@ -616,6 +635,9 @@ 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; + if let Some(meta) = self.names.get_mut(&open.path) { + (meta.size, meta.mtime) = (size, now); + } let path = open.path.clone(); mapped("a truncate's free", &path, self.fs.free_extents(&dropped)) } @@ -634,7 +656,7 @@ impl Volume for DataVolume { self.require_parent_dir(path)?; let now = (self.clock)(); mapped("mkdir", path, self.fs.create(&format!("{path}/"), &[], now))?; - self.names.insert(path.to_string(), Kind::Dir); + self.names.insert(path.to_string(), DIR); Ok(()) } @@ -658,7 +680,7 @@ impl Volume for DataVolume { } fn unlink(&mut self, path: &str) -> Result<(), Error> { - match self.names.get(path) { + match self.kind(path) { Some(Kind::File) | Some(Kind::Symlink) => {} Some(Kind::Dir) => return Err(Error::IsADirectory), None if self.is_dir(path) => return Err(Error::IsADirectory), @@ -698,8 +720,8 @@ impl Volume for DataVolume { // one leaves `to` naming its file, holders and unsynced length whole. mapped("rename", from, self.fs.rename(from, to))?; self.orphan(to); - self.names.remove(from); - self.names.insert(to.to_string(), kind); + let moved = self.names.remove(from).expect("a file the volume names"); + self.names.insert(to.to_string(), moved); if let Some(node) = self.by_path.remove(from) { self.open.get_mut(&node).expect("indexed").path = to.to_string(); self.by_path.insert(to.to_string(), node); @@ -718,7 +740,7 @@ impl Volume for DataVolume { // Every entry beneath, and the directory's own, in one // operation of the format's: the directory moves whole or not at all. let prefix = format!("{from}/"); - let moving: Vec<(String, Kind)> = self + let moving: Vec<(String, Meta)> = self .names .get_key_value(from) .into_iter() @@ -732,9 +754,9 @@ impl Volume for DataVolume { } let renames: Vec<(String, String)> = moving .iter() - .map(|(name, kind)| { + .map(|(name, meta)| { let moved = format!("{to}{}", &name[from.len()..]); - match kind { + match meta.kind { Kind::Dir => (format!("{name}/"), format!("{moved}/")), _ => (name.clone(), moved), } @@ -742,10 +764,11 @@ impl Volume for DataVolume { .collect(); let pairs: Vec<(&str, &str)> = renames.iter().map(|(a, b)| (a.as_str(), b.as_str())).collect(); mapped("rename", from, self.fs.rename_all(&pairs))?; - for (name, kind) in moving { + for (name, _) in moving { let moved = format!("{to}{}", &name[from.len()..]); - self.names.remove(&name); - self.names.insert(moved.clone(), kind); + // The record as it is now, which the persists above updated. + let meta = self.names.remove(&name).expect("a name the volume holds"); + self.names.insert(moved.clone(), meta); if let Some(node) = self.by_path.remove(&name) { self.open.get_mut(&node).expect("indexed").path = moved.clone(); self.by_path.insert(moved, node); @@ -764,7 +787,8 @@ impl Volume for DataVolume { } self.require_parent_dir(path)?; mapped("symlink", path, self.fs.create_symlink(path, target))?; - self.names.insert(path.to_string(), Kind::Symlink); + // What the format's symlink entry records: its target's length, undated. + self.names.insert(path.to_string(), Meta { kind: Kind::Symlink, size: target.len() as u64, mtime: 0 }); Ok(()) } @@ -982,6 +1006,92 @@ mod tests { assert_eq!(v.lstat("home/b/f").unwrap().size, 0, "replaced"); } + /// Every name the index records is what the format's own entry holds, + /// for each closed file and symlink: the stat a client is answered from + /// memory is the one a search of the tree would answer. + fn assert_index_is_the_format_s(v: &DataVolume) { + for (name, meta) in &v.names { + if meta.kind == Kind::Dir || v.by_path.contains_key(name) { + continue; + } + let (_, size) = v.fs.file_extents(name).unwrap().unwrap_or_else(|| panic!("{name} has no entry")); + let mtime = v.fs.file_mtime(name).unwrap().unwrap(); + assert_eq!((meta.size, meta.mtime), (size, mtime), "{name}"); + assert_eq!(meta.kind == Kind::Symlink, v.fs.is_symlink(name).unwrap(), "{name}"); + } + } + + /// A clock that reads a later nanosecond at every call, so a time left + /// stale anywhere is a time that differs. + fn advancing_clock() -> u64 { + static NOW: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(1_000_000_000); + NOW.fetch_add(1, std::sync::atomic::Ordering::Relaxed) + } + + #[test] + fn a_stat_from_the_index_is_what_the_entry_records() { + let mut v = DataVolume::format(Ram::new(4096), &["home"], advancing_clock).unwrap(); + let a = v.open("home/a", CREATE).unwrap(); + v.write(a, 0, &[1; 5000]).unwrap(); + assert_eq!(v.lstat("home/a").unwrap().size, 5000, "an open file's own length, before its entry has it"); + v.close(a).unwrap(); + assert_index_is_the_format_s(&v); + + let b = v.open("home/b", CREATE).unwrap(); + v.write(b, 0, &[2; 9000]).unwrap(); + v.truncate(b, 100).unwrap(); + v.close(b).unwrap(); + v.symlink("home/l", "a").unwrap(); + v.rename("home/a", "home/sub/c").unwrap(); + v.rename("home/b", "home/sub/c").unwrap(); + // Written and still open while its directory moves. + let open = v.open("home/sub/open", CREATE).unwrap(); + v.write(open, 0, &[3; 7000]).unwrap(); + let written = v.lstat("home/sub/open").unwrap(); + v.mkdir("home/d").unwrap(); + v.rename("home/sub", "home/d/moved").unwrap(); + v.close(open).unwrap(); + assert_index_is_the_format_s(&v); + assert_eq!(v.lstat("home/d/moved/open").unwrap(), written, "the open file's length and time moved with it"); + assert_eq!(v.lstat("home/d/moved/c").unwrap().size, 100); + assert_eq!(v.lstat("home/l").unwrap(), Meta { kind: Kind::Symlink, size: 1, mtime: 0 }); + v.unlink("home/l").unwrap(); + assert_eq!(v.lstat("home/l"), Err(Error::NotFound)); + + let before: Vec<(String, Meta)> = v.names.iter().map(|(n, m)| (n.clone(), *m)).collect(); + v.sync().unwrap(); + let DataVolume { fs, cache, .. } = v; + drop(fs); + let disk = Rc::try_unwrap(cache).ok().expect("one owner").into_disk(); + let Probed::Mounted(again) = DataVolume::probe(disk, &["home"], advancing_clock) else { panic!("remount") }; + let after: Vec<(String, Meta)> = again.names.iter().map(|(n, m)| (n.clone(), *m)).collect(); + assert_eq!(after, before, "the index a mount reads is the one the server kept"); + assert_index_is_the_format_s(&again); + } + + /// A listing passes over what is beneath a child directory and names + /// each child once: `x` with an entry of its own and names beneath, `y` + /// only the prefix of two, and the names beside `x` that sort among its + /// own (`-` and `.` before `/`, `0` after). + #[test] + fn a_listing_names_each_child_once_and_nothing_beneath_one() { + let mut v = vol(); + v.mkdir("home/x").unwrap(); + for path in ["home/x-a", "home/x.b", "home/x/deep/er/f", "home/x/g", "home/x0", "home/y/z", "home/y/z2"] { + let n = v.open(path, CREATE).unwrap(); + v.close(n).unwrap(); + } + v.mkdir("home/x/kept").unwrap(); + v.mkdir("home/e").unwrap(); + let listed: Vec<(String, Kind)> = v.list("home").unwrap().into_iter().map(|l| (l.name, l.meta.kind)).collect(); + let want = [("e", Kind::Dir), ("x", Kind::Dir), ("x-a", Kind::File), ("x.b", Kind::File), ("x0", Kind::File), ("y", Kind::Dir)]; + assert_eq!(listed, want.map(|(n, k)| (n.to_string(), k))); + let inner: Vec = v.list("home/x").unwrap().into_iter().map(|l| l.name).collect(); + assert_eq!(inner, ["deep", "g", "kept"]); + let root: Vec = v.list("").unwrap().into_iter().map(|l| l.name).collect(); + assert_eq!(root, ["home"]); + } + /// A clock that reads no whole second. fn nanosecond_clock() -> u64 { 1_717_245_296_123_456_789 diff --git a/system/fileserver/src/main.rs b/system/fileserver/src/main.rs index b4005e1c248..0550e233b0c 100644 --- a/system/fileserver/src/main.rs +++ b/system/fileserver/src/main.rs @@ -154,6 +154,8 @@ struct Client { window: Option, fids: Ids, since: Instant, + /// A watch on its connection waits in the poller. + armed: bool, } impl Client { @@ -181,6 +183,8 @@ struct Stream { file: Shared, /// The share of the client that asked for it, which it spends. share: u64, + /// A watch on its pipe waits in the poller. + armed: bool, } /// The private port exports are made on: its acceptor, and the namespace @@ -248,6 +252,7 @@ fn main() { writeback: WriteBack::default(), scratch: Vec::new(), volume_id: u64::from(toyos_abi::syscall::getpid().0), + acceptor_armed: false, } .serve() } @@ -369,6 +374,8 @@ struct Server { /// The volume every node is on: this start's pid, which the kernel never /// issues again, since a restart numbers its files afresh. volume_id: u64, + /// A watch on the acceptor waits in the poller. + acceptor_armed: bool, } /// What one request is answered. @@ -378,6 +385,8 @@ enum Answer { Follow(usize), /// A reply that carries a handle. WithHandle(Reply, toyos::OwnedHandle), + /// No frame: what a `CLOSE` is answered. + Nothing, /// The client broke the protocol and is let go. Drop(&'static str), /// The client is answered this refusal and let go, for this reason. @@ -416,18 +425,29 @@ impl Out for WindowOut { } impl Server { + /// Whether another accepted connection may wait on its hello. + fn takes_handshakes(&self) -> bool { + self.clients.values().filter(|c| c.waiting()).count() < MAX_HANDSHAKES + } + + /// A watch stays in the poller until it fires, so only what fired since + /// the last wait, and what is new, is watched again: each watch costs the + /// kernel a pass over every poll the ring holds. fn serve(mut self) -> ! { let poller = Poller::new(Poller::MAX_HANDLES); let mut ready = Vec::new(); loop { - if self.clients.values().filter(|c| c.waiting()).count() < MAX_HANDSHAKES { + if !self.acceptor_armed && self.takes_handshakes() { poller.watch(&self.acceptor, READABLE, TOKEN_ACCEPTOR); + self.acceptor_armed = true; } - for (id, c) in &self.clients { + for (id, c) in self.clients.iter_mut().filter(|(_, c)| !c.armed) { poller.watch(&c.conn, READABLE, TOKEN_CLIENT + id); + c.armed = true; } - for (id, s) in &self.streams { + for (id, s) in self.streams.iter_mut().filter(|(_, s)| !s.armed) { poller.watch(&s.pipe, READABLE, TOKEN_STREAM + id); + s.armed = true; } let now = Instant::now(); let timeout = self @@ -443,11 +463,20 @@ impl Server { for &token in &ready { if token == TOKEN_ACCEPTOR { + self.acceptor_armed = false; self.accept(); } else if token < TOKEN_STREAM { - self.pump(token - TOKEN_CLIENT); + let id = token - TOKEN_CLIENT; + if let Some(c) = self.clients.get_mut(&id) { + c.armed = false; + } + self.pump(id); } else { - self.drain(token - TOKEN_STREAM); + let sid = token - TOKEN_STREAM; + if let Some(s) = self.streams.get_mut(&sid) { + s.armed = false; + } + self.drain(sid); } } let now = Instant::now(); @@ -502,6 +531,7 @@ impl Server { window: None, fids: Ids::default(), since: Instant::now(), + armed: false, }; self.clients.insert(id, client); } @@ -548,6 +578,7 @@ impl Server { Answer::Reply(reply) => client.conn.try_send(REPLY, &Reply { tag, ..reply }), Answer::Follow(len) => client.conn.try_send(FOLLOW, &Reply { tag, value: len as u64, ..Reply::ok() }), Answer::WithHandle(reply, handle) => client.conn.try_send_with(REPLY, &Reply { tag, ..reply }, [handle]), + Answer::Nothing => return, Answer::Drop(why) => return self.drop_client(id, why), Answer::Refuse(e, why) => { // Let go whether or not the refusal went: either way it is said. @@ -753,11 +784,13 @@ impl Server { // These volumes keep no file without a name. CREATE_UNNAMED => Err(Error::Unsupported), CLOSE => { + // Nobody waits on the answer: an id the client does not hold + // closes nothing, and a refused close is its volume's to say. let client = self.clients.get_mut(&id).expect("pumped"); - if let Some(node) = client.fids.close(r.fid)? { - self.volume.close(node)?; + if let Ok(Some(node)) = client.fids.close(r.fid) { + let _ = self.volume.close(node); } - Ok(Answer::Reply(Reply::ok())) + Ok(Answer::Nothing) } DUP => { let client = self.clients.get_mut(&id).expect("pumped"); @@ -844,7 +877,7 @@ impl Server { let (read, write) = toyos::pipe_pair().map_err(|e| Error::kernel("a stream's pipe", e))?; let sid = self.next_stream; self.next_stream += 1; - self.streams.insert(sid, Stream { pipe: read, file, share }); + self.streams.insert(sid, Stream { pipe: read, file, share, armed: false }); Ok(Answer::WithHandle(Reply::ok(), write.into())) } READDIR => { @@ -1151,6 +1184,7 @@ impl Server { window: None, fids, since: Instant::now(), + armed: false, }; let child = self.next_client; self.next_client += 1;