From 85cd3b614fbac3d2ed9e7a8f9adc3d9f6fea3109 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 04:14:13 +0200 Subject: [PATCH 01/14] The terminal and toyos-window never block on a readiness answer already spent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `toolkit_winit_loop` went red in #638's whole run 638r4 because the terminal stopped reading its shell's output after stage 1 and never started again: the app went on through all 23 windows, its lines piled up unread in the shell's stdout pipe, and the harness waited for a `WINIT-LOOP CLOSE-ME` that was sitting in that pipe. ## The cause A poller's READABLE is a cue to look, not a promise that bytes are there. `process_watch` (`kernel/src/inbox/mod.rs`) completes a handle that is already ready at once and leaves the poll an earlier round registered on it armed; `toyos::poller`'s capacity counts that leftover answer by design. So a round that registers between a writer's data and that writer's post reads the data on its own answer, and the leftover poll then answers again for bytes that are gone. The writer's window is wide: `sys_write_nonblock` releases its locks after the copy, which is a preemption point, and only then posts. The terminal read every answer with a call that waits: `recv_event` on its window, std's `read` on the shell's stdout and stderr. On the window that wait is for good. The compositor writes to a window only after it presents or on input, so after stage 1's present and its frame event, a spent answer parked the terminal in `recv_event` with nothing coming; stage 2's line and every one after it stayed in the pipe. Every CPU reading `ready=0` from 11.4 s to 42.8 s is that: the terminal was parked in a connection read, and nothing else had work. Ruled out from the code: logd says every loss by name (a full ring, an allowance, the console's unshown count) and said none; klogd's queue counts what it refuses and counted none; the harness reads one serial stream, which carried the compositor's lines the whole time; and the terminal's log ring has one writer, its one thread, so no slot it reserved can stay unpublished. ## Decisions - **toyos-window reads every event through one `FrameRx`, without waiting.** `try_event` is the read for a loop that waits on the window's handle itself; `recv_event` and `poll_event` wait on readiness and look again, so a spent answer is waited past. `poll_event(0)` now looks without registering a poll. winit's ToyOS backend drains with `poll_event(0)`, so the same hang is closed for every winit app. - **The terminal reads its shell's pipes with `read_nonblock`.** std gives a child's pipe no `IntoRawFd` on ToyOS, and its end is the handle and nothing more, so the terminal forgets std's owner and takes the handle as a `toyos::Pipe`. It depends on `toyos-abi` for `SyscallError`, as the console does. - **The kernel is unchanged.** The leftover answer is the poller's documented contract, and closing it would not close staleness: a post that claims a poll before the replacing registration withdraws it writes its completion after. - **The console and `surface::Host::accept` wait the same way** and are off this path; `issues/design-debt/console-and-surface-host-wait-on-a-spent-readiness.md` holds them. `Host::accept` is under `toyos/src`. ## Tests `toolkit_window_spent` (`tests/toyos-rust-tests/src/bin/window_spent.rs`) leaves a spent answer in a window's own ring — a timed `poll_event` arms the poll, the frame event answers it, `recv_event` reads the frame — and requires `poll_event(0)` and a timed `poll_event` to answer `None` and the next frame to be read. Before this change `poll_event(0)` parks there. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...-surface-host-wait-on-a-spent-readiness.md | 29 ++++++ .../toyos-rust-tests/src/bin/window_spent.rs | 76 +++++++++++++++ tests/toyos.rs | 35 ++++++- userland/Cargo.lock | 1 + userland/terminal/Cargo.toml | 1 + userland/terminal/src/main.rs | 69 +++++++++----- userland/toyos-window/src/lib.rs | 92 +++++++++++++------ 7 files changed, 254 insertions(+), 49 deletions(-) create mode 100644 issues/design-debt/console-and-surface-host-wait-on-a-spent-readiness.md create mode 100644 tests/toyos-rust-tests/src/bin/window_spent.rs diff --git a/issues/design-debt/console-and-surface-host-wait-on-a-spent-readiness.md b/issues/design-debt/console-and-surface-host-wait-on-a-spent-readiness.md new file mode 100644 index 00000000000..990fd31f100 --- /dev/null +++ b/issues/design-debt/console-and-surface-host-wait-on-a-spent-readiness.md @@ -0,0 +1,29 @@ +--- +status: open +kind: defect +opened: 2026-10-01 +--- + +# The console and `surface::Host::accept` wait on a readiness answer that may be spent + +A poller's `READABLE` is a cue to look, not a promise that bytes are there: a +poll an earlier round left armed answers for a write a later round already read +(`process_watch` in `kernel/src/inbox/mod.rs` completes an already-ready handle +at once and leaves that poll registered, and `toyos::poller`'s capacity counts +both answers). The terminal and `toyos-window` read past such an answer without +waiting. Two readers still block on one: + +- `userland/console/src/main.rs` reads the shell's stdout and stderr with std's + blocking `read` on `TOKEN_STDOUT` and `TOKEN_STDERR`. A spent answer parks the + console until the shell writes again, and a shell at its prompt is waiting for + the keys the parked console no longer forwards. +- `toyos::surface::Host::accept` (`toyos/src/surface.rs`) calls the blocking + `Acceptor::accept` on the acceptor reading ready, so a spent answer parks the + terminal and the console until the next client connects. It is under + `toyos/src`, so it is an ABI brief's. + +Owner: `userland/console` and `toyos::surface`. + +**Exit**: neither waits on a readiness answer — the console reads its shell's +pipes as the terminal does, and `Host::accept` takes a connection only when one +is queued. diff --git a/tests/toyos-rust-tests/src/bin/window_spent.rs b/tests/toyos-rust-tests/src/bin/window_spent.rs new file mode 100644 index 00000000000..348173d113a --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/window_spent.rs @@ -0,0 +1,76 @@ +//! A window's event reads never wait on a readiness answer whose frame was +//! already read. +//! +//! A timed `poll_event` that finds nothing leaves the window's own poll armed. +//! The compositor's frame event answers it, and `recv_event` reads that frame +//! without looking at the answer, so the answer is still waiting in the +//! window's ring with nothing behind it. `poll_event(0)` has to say `None` at +//! once, a timed `poll_event` has to wait past the answer to its timeout, and +//! the next frame event has to be read. + +use std::process::exit; + +use toyos::poller::{Poller, READABLE}; +use window::{Event, Window}; + +/// Long enough for the wait to register its poll, which is all it is for: +/// nothing is sent to a window that has not presented, whatever this is. +const ARM_NANOS: u64 = 1_000_000; + +fn fail(what: &str) -> ! { + println!("WINDOW-SPENT-FAIL {what}"); + exit(1); +} + +fn named(event: &Option) -> &'static str { + match event { + None => "nothing", + Some(Event::KeyInput(_)) => "KeyInput", + Some(Event::MouseInput(_)) => "MouseInput", + Some(Event::ClipboardPaste(_)) => "ClipboardPaste", + Some(Event::Resized) => "Resized", + Some(Event::Close) => "Close", + Some(Event::LayoutChanged) => "LayoutChanged", + Some(Event::Frame) => "Frame", + } +} + +fn main() { + let mut window = Window::create_with_title(160, 100, "spent").unwrap_or_else(|e| { + println!("WINDOW-SPENT-REFUSED {e}"); + exit(1); + }); + + let armed = window.poll_event(ARM_NANOS); + if armed.is_some() { + fail(&format!("{} arrived before anything was presented", named(&armed))); + } + + window.present(); + // The frame event's post answers the window's armed poll before this one, + // which registered after it. + let arrival = Poller::new(1); + arrival.watch_raw(window.handle(), READABLE, 0); + arrival.wait(1, u64::MAX, |_| {}); + drop(arrival); + let frame = Some(window.recv_event()); + if !matches!(frame, Some(Event::Frame)) { + fail(&format!("the first present was answered with {}", named(&frame))); + } + + let at_once = window.poll_event(0); + if at_once.is_some() { + fail(&format!("poll_event(0) after the spent answer read {}", named(&at_once))); + } + let timed = window.poll_event(ARM_NANOS); + if timed.is_some() { + fail(&format!("a timed poll_event after the spent answer read {}", named(&timed))); + } + + window.present(); + let next = Some(window.recv_event()); + if !matches!(next, Some(Event::Frame)) { + fail(&format!("the second present was answered with {}", named(&next))); + } + println!("WINDOW-SPENT-OK"); +} diff --git a/tests/toyos.rs b/tests/toyos.rs index 52ce975d3e7..337fadf028f 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -302,6 +302,7 @@ const RUST_SKIP: &[&str] = &[ // Same again: the `toolkit_` tests of their names launch them from the // toolkit desktop. "window_wake", + "window_spent", "winit_loop", "winit_pace", // Its two spawning arms only mean anything when the two processes share a @@ -1166,9 +1167,11 @@ const MACHINE_TESTS: &[(&str, Sched)] = &[ // window, its text in the system font, no redraw it did not ask for, and // a clean exit when the compositor closes it. ("toolkit_iced", Sched::Parallel), - // The wait every winit loop blocks in, the loop itself through winit's - // API, and an animation held to the compositor's frame events. + // The wait every winit loop blocks in, a window's reads past a readiness + // already spent, the loop itself through winit's API, and an animation + // held to the compositor's frame events. ("toolkit_window_wake", Sched::Parallel), + ("toolkit_window_spent", Sched::Parallel), ("toolkit_winit_loop", Sched::Parallel), ("toolkit_winit_pace", Sched::Parallel), // Ctrl+Alt+D on the same machine. Parallel: it waits for a marker and its @@ -1417,6 +1420,7 @@ const CARRIES: &[(&str, &[&str])] = &[ ("metal_sim_hostile_clipboard", &["test_rs_compositor_hostile_clipboard"]), ("desktop_window_child", &["test_rs_window_child"]), ("toolkit_window_wake", &["test_rs_window_wake"]), + ("toolkit_window_spent", &["test_rs_window_spent"]), ("toolkit_winit_loop", &["test_rs_winit_loop"]), ("toolkit_winit_pace", &["test_rs_winit_pace"]), ("doom_frames", &["test_rs_doom_frames"]), @@ -7963,6 +7967,32 @@ fn toolkit_window_wake(rust_bins: &[(String, Vec)]) -> Result<(), String> { Err(format!("test_rs_window_wake never finished:\n{}", &log[launched..])) } +/// A window's event reads never wait on a readiness answer whose frame another +/// read already took. +/// +/// `tests/toyos-rust-tests/src/bin/window_spent.rs` leaves such an answer in +/// its window's ring and reads past it; a read that waits on it hangs, and the +/// ceiling reds that. +fn toolkit_window_spent(rust_bins: &[(String, Vec)]) -> Result<(), String> { + let (mut qemu, mut log, launched) = toolkit_launch(rust_bins, "window_spent", "test_rs_window_spent")?; + let log = &mut log; + let mut live = qemu::Liveness::new(Duration::from_secs(30), Duration::from_secs(120)); + while live.working(log) { + let said = &log[launched..]; + if said.contains("WINDOW-SPENT-OK") { + return Ok(()); + } + if said.contains("WINDOW-SPENT-FAIL") + || said.contains("WINDOW-SPENT-REFUSED") + || said.contains("panicked") + { + return Err(format!("a window read past a spent readiness went wrong:\n{said}")); + } + log.push_str(&qemu.drain_serial(Duration::from_millis(200))); + } + Err(format!("test_rs_window_spent never finished:\n{}", &log[launched..])) +} + /// The ToyOS winit backend's loop through winit's own API: user events sent /// from `AboutToWait` and from another thread, windows redrawn and dropped on /// another thread, a window dropped in the handler that made it, one dropped in @@ -10469,6 +10499,7 @@ fn run_machine_test( "desktop_window_child" => desktop_window_child(rust_bins), "toolkit_iced" => toolkit_iced(), "toolkit_window_wake" => toolkit_window_wake(rust_bins), + "toolkit_window_spent" => toolkit_window_spent(rust_bins), "toolkit_winit_loop" => toolkit_winit_loop(rust_bins), "toolkit_winit_pace" => toolkit_winit_pace(rust_bins), "blocked_dump" => blocked_dump(), diff --git a/userland/Cargo.lock b/userland/Cargo.lock index 5489d455e17..52f98400ae7 100644 --- a/userland/Cargo.lock +++ b/userland/Cargo.lock @@ -3912,6 +3912,7 @@ name = "terminal" version = "0.1.0" dependencies = [ "toyos", + "toyos-abi", "toyos-font", "toyos-window", ] diff --git a/userland/terminal/Cargo.toml b/userland/terminal/Cargo.toml index 13c03bd5461..e6645c56f03 100644 --- a/userland/terminal/Cargo.toml +++ b/userland/terminal/Cargo.toml @@ -6,6 +6,7 @@ license = "MIT OR Apache-2.0" [dependencies] toyos = { path = "../../toyos" } +toyos-abi = { path = "../../toyos-abi" } toyos-font = { path = "../toyos-font" } toyos-window = { path = "../toyos-window" } diff --git a/userland/terminal/src/main.rs b/userland/terminal/src/main.rs index 49cdc2f5f0e..124a57d72f7 100644 --- a/userland/terminal/src/main.rs +++ b/userland/terminal/src/main.rs @@ -6,7 +6,7 @@ //! instead of the bytes, which is what `locale detect` needs and what a //! terminal writing only translated bytes into a pipe could never give it. -use std::io::{Read, Write}; +use std::io::Write; use std::os::fd::AsRawFd; use std::os::toyos::process; use std::os::toyos::process::CommandExt; @@ -16,7 +16,8 @@ use terminal::Console; use toyos::poller::{Poller, READABLE}; use toyos::port; use toyos::surface::{self, Delivery, Host, Notice}; -use toyos::RawHandle; +use toyos::{Pipe, RawHandle}; +use toyos_abi::syscall::SyscallError; use window::Window; const TOKEN_STDOUT: u64 = 0; @@ -44,6 +45,29 @@ fn copy(text: &str) { } } +/// One of the shell's output pipes, as std spawned it, made a [`Pipe`] this +/// process can read without waiting. +fn pipe_end(end: impl AsRawFd) -> Pipe { + let raw = RawHandle(end.as_raw_fd() as u32); + // std's end is that handle and nothing more, and ToyOS's std gives a + // child's pipe no `IntoRawFd`: forgetting it hands the handle on unclosed. + std::mem::forget(end); + // SAFETY: a live pipe end this process holds, whose one other owner was + // just forgotten. + unsafe { Pipe::from_raw(raw) } +} + +/// What a readiness answer on one of the shell's pipes leads to: `None` where +/// the bytes it announced were already read, and an error read as the end, as +/// a blocking read's was. +fn read_ready(pipe: &Pipe, buf: &mut [u8]) -> Option { + match pipe.read_nonblock(buf) { + Ok(n) => Some(n), + Err(SyscallError::WouldBlock) => None, + Err(_) => Some(0), + } +} + fn main() { // **This terminal's surface is a port it makes, not a name it registers.** // One per instance: the connector goes into the namespace of the shell it @@ -67,8 +91,8 @@ fn main() { let mut console = Console::new(window.screen(), font); let mut shell_stdin = child.stdin.take().unwrap(); - let mut shell_stdout = child.stdout.take().unwrap(); - let mut shell_stderr = child.stderr.take().unwrap(); + let shell_stdout = pipe_end(child.stdout.take().unwrap()); + let shell_stderr = pipe_end(child.stderr.take().unwrap()); let poller = Poller::new(3 + Host::POLL_HANDLES); // The window exists and the shell's stdin is a pipe this process owns, so @@ -81,8 +105,8 @@ fn main() { eprintln!("terminal: ready"); loop { - poller.watch_raw(RawHandle(shell_stdout.as_raw_fd() as u32), READABLE, TOKEN_STDOUT); - poller.watch_raw(RawHandle(shell_stderr.as_raw_fd() as u32), READABLE, TOKEN_STDERR); + poller.watch(&shell_stdout, READABLE, TOKEN_STDOUT); + poller.watch(&shell_stderr, READABLE, TOKEN_STDERR); poller.watch_raw(window.handle(), READABLE, TOKEN_WINDOW); poller.watch_raw(host.acceptor_handle(), READABLE, TOKEN_LISTEN); for client in host.client_handles() { @@ -96,28 +120,30 @@ fn main() { if ready[TOKEN_STDOUT as usize] { let mut buf = [0u8; 4096]; - let n = shell_stdout.read(&mut buf).unwrap_or(0); - if n == 0 { - // The child closes every fd together, so a last line it wrote - // to stderr right before exiting is already sitting in that - // pipe — drained here or it is lost with the loop. - let n = shell_stderr.read(&mut buf).unwrap_or(0); - if n > 0 { + match read_ready(&shell_stdout, &mut buf) { + None => {} + Some(0) => { + // The child closes every fd together, so a last line it wrote + // to stderr right before exiting is already sitting in that + // pipe — drained here or it is lost with the loop. + if let Some(n @ 1..) = read_ready(&shell_stderr, &mut buf) { + console.write_bytes(&buf[..n]); + std::io::stdout().lock().write_all(&buf[..n]).ok(); + present(&console, &window); + } + break; + } + Some(n) => { console.write_bytes(&buf[..n]); std::io::stdout().lock().write_all(&buf[..n]).ok(); present(&console, &window); } - break; } - console.write_bytes(&buf[..n]); - std::io::stdout().lock().write_all(&buf[..n]).ok(); - present(&console, &window); } if ready[TOKEN_STDERR as usize] { let mut buf = [0u8; 4096]; - let n = shell_stderr.read(&mut buf).unwrap_or(0); - if n > 0 { + if let Some(n @ 1..) = read_ready(&shell_stderr, &mut buf) { console.write_bytes(&buf[..n]); std::io::stdout().lock().write_all(&buf[..n]).ok(); present(&console, &window); @@ -146,8 +172,9 @@ fn main() { } } - if ready[TOKEN_WINDOW as usize] { - match window.recv_event() { + let event = if ready[TOKEN_WINDOW as usize] { window.try_event() } else { None }; + if let Some(event) = event { + match event { // A client holding the grab takes the transition whole, and // the translator is not advanced — see `Window::text`. window::Event::KeyInput(key) if host.deliver(key.into()) == Delivery::Sent => {} diff --git a/userland/toyos-window/src/lib.rs b/userland/toyos-window/src/lib.rs index 393343c75d3..a5f7bc1f365 100644 --- a/userland/toyos-window/src/lib.rs +++ b/userland/toyos-window/src/lib.rs @@ -8,7 +8,9 @@ pub use wait::{Waiter, Waker, Woke}; pub use toyos_abi::RawHandle; use toyos_abi::syscall::SyscallError; -use toyos::ipc; +use std::time::{Duration, Instant}; + +use toyos::ipc::{self, FrameRx, RxStep}; use toyos::AsHandle; use toyos::poller::{Poller, READABLE}; use toyos::endow::{self, EndowError}; @@ -468,6 +470,9 @@ fn copy(bytes: &[u8]) -> Result<(), CreateError> { pub struct Window { conn: Connection, + /// Every event is read through this, so none is read with a call that + /// waits: see [`Window::try_event`]. + rx: FrameRx, poller: Poller, shm: SharedMemory, width: u32, @@ -544,6 +549,7 @@ impl Window { let poller = Poller::new(1); Ok(Self { conn, + rx: FrameRx::new(), poller, shm, width: info.width, @@ -579,11 +585,37 @@ impl Window { let _ = self.conn.signal(MSG_LAYOUT_CHANGED); } + /// The next event, waiting for as long as that takes. pub fn recv_event(&mut self) -> Event { - let Ok(header) = self.conn.recv_header() else { - return Event::Close; - }; - self.decode_event(&header) + loop { + if let Some(event) = self.try_event() { + return event; + } + self.wait_readable(u64::MAX); + } + } + + /// The next event if a whole one has arrived, and `None` at once if not. + /// + /// **The read for a loop that waits on [`Window::handle`] itself, because + /// a readiness answer is a cue to look and not a frame.** A poll left armed + /// by an earlier wait can answer for a frame a later read already took, and + /// a read that waits on that answer waits for the compositor's next + /// message — which a window that has stopped presenting is never sent. + pub fn try_event(&mut self) -> Option { + match self.rx.pump(&self.conn) { + RxStep::Idle => None, + RxStep::Eof | RxStep::Malformed => Some(Event::Close), + RxStep::Frame { msg_type, payload_len } => Some(self.decode_event(msg_type, payload_len)), + } + } + + /// Whether the connection read ready before `timeout_nanos` passed. + fn wait_readable(&self, timeout_nanos: u64) -> bool { + self.poller.watch(&self.conn, READABLE, 0); + let mut ready = false; + self.poller.wait(1, timeout_nanos, |_| ready = true); + ready } /// The next event, or `None` when `timeout_nanos` passes — and `None` @@ -603,23 +635,37 @@ impl Window { if self.closed { return None; } - self.poller.watch(&self.conn, READABLE, 0); - let mut ready = false; - self.poller.wait(1, timeout_nanos, |_| ready = true); - if !ready { - return None; + // `u64::MAX` is the poller's "forever", and so is a timeout no + // `Instant` reaches. + let deadline = (timeout_nanos != u64::MAX) + .then(|| Instant::now().checked_add(Duration::from_nanos(timeout_nanos))) + .flatten(); + loop { + if let Some(event) = self.try_event() { + self.closed = matches!(&event, Event::Close); + return Some(event); + } + let wait = match deadline { + None => u64::MAX, + Some(at) => match at.checked_duration_since(Instant::now()) { + Some(left) if !left.is_zero() => { + u64::try_from(left.as_nanos()).map_or(u64::MAX - 1, |n| n.min(u64::MAX - 1)) + } + _ => return None, + }, + }; + if !self.wait_readable(wait) { + return None; + } } - let event = self.recv_event(); - self.closed = matches!(&event, Event::Close); - Some(event) } /// A message the compositor cannot have meant closes the window rather /// than killing the client: this side is a library inside somebody else's /// program, and it has a way to say "the session is over". - fn decode_event(&mut self, header: &ipc::IpcHeader) -> Event { - match header.msg_type { - MSG_KEY_INPUT => match self.conn.recv_payload::(header) { + fn decode_event(&mut self, msg_type: u32, len: usize) -> Event { + match msg_type { + MSG_KEY_INPUT => match ipc::decode_payload::(self.rx.payload(len)) { Ok(key) => Event::KeyInput(key), Err(_) => Event::Close, }, @@ -627,12 +673,12 @@ impl Window { load_layout(&mut self.translator); Event::LayoutChanged } - MSG_MOUSE_INPUT => match self.conn.recv_payload(header) { + MSG_MOUSE_INPUT => match ipc::decode_payload(self.rx.payload(len)) { Ok(ev) => Event::MouseInput(ev), Err(_) => Event::Close, }, MSG_WINDOW_RESIZED => { - let Ok(info) = self.conn.recv_payload::(header) else { + let Ok(info) = ipc::decode_payload::(self.rx.payload(len)) else { return Event::Close; }; let buf_size = info.stride as usize * info.height as usize * 4; @@ -651,15 +697,9 @@ impl Window { self.pixel_format = info.pixel_format; Event::Resized } - MSG_CLIPBOARD_PASTE => { - let mut buf = [0u8; MAX_INLINE_PAYLOAD]; - let Ok(n) = self.conn.recv_bytes(header, &mut buf) else { - return Event::Close; - }; - Event::ClipboardPaste(buf[..n].to_vec()) - } + MSG_CLIPBOARD_PASTE => Event::ClipboardPaste(self.rx.payload(len).to_vec()), MSG_CLIPBOARD_PASTE_SHM => { - let Ok(info) = self.conn.recv_payload::(header) else { + let Ok(info) = ipc::decode_payload::(self.rx.payload(len)) else { return Event::Close; }; let Some([buffer]) = self.conn.recv_handles_exact::<1>() else { From aac1a752f4c77671a2e335bd41a5baf7677ccd17 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 08:45:52 +0200 Subject: [PATCH 02/14] toyos::poller hands out only the answer of a handle's latest registration A watch is one-shot, and watching a handle again replaces its registration: `process_watch` withdraws the armed poll whose `Poll::handle` matches. Two answers escape that withdrawal and stay in the ring under the caller's token: one the replaced registration posted before the replacement was processed, and one it posts after a replacement that found the handle ready and was answered at once (that path returns before the withdrawal). Either reads as news about the handle. A reader that answers it with a blocking call waits for bytes it already read, or for a connection it already took: the terminal parked in `Window::recv_event` in #638's `toolkit_winit_loop` run, and the same shape sits in init's acceptors, netd, blockd, filepicker, logd, soundd, the console and `toyos::surface`. The poller now submits every registration under a number of its own and keeps a registry of the live ones, at most one per handle. An answer whose number is not its handle's latest is dropped in `drain`; a live one hands the caller its token and ends the registration. No ABI change: the kernel echoes whatever token a submission carries. `wait` went on believing the kernel. The kernel counts a dropped answer towards `min_complete` and returns for it, so `wait` now loops, for what is left of its timeout on the clock page, until the answers it handed out make up the count. A caller's `wait(1, u64::MAX)` returns only with an answer. The registry is a fixed array, because the SDK is a dependency of std and links no allocator: 2 * MAX_HANDLES entries, with a per-poller limit of twice the declared capacity (the handles watched, and as many closed since the last wait whose end is still in the ring), past which a registration panics by name. `Poller` stops being `Sync`: the registry is a `RefCell`, and a watch already moved the submission tail in two steps. fsd's probe poller existed only to second-guess a doubled answer before its blocking `accept`; it goes. Host tests drive the poller over a fake page and a fake kernel that answers as `process_watch` and a post do, and reports readiness the reader has already spent. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- toyos/src/poller.rs | 412 +++++++++++++++++++++++++++++++++++++-- userland/fsd/src/main.rs | 23 +-- 2 files changed, 398 insertions(+), 37 deletions(-) diff --git a/toyos/src/poller.rs b/toyos/src/poller.rs index bbf48209d4f..553ef7bc76a 100644 --- a/toyos/src/poller.rs +++ b/toyos/src/poller.rs @@ -1,8 +1,9 @@ //! Event-driven I/O polling on an [inbox](toyos_abi::inbox). +use core::cell::RefCell; use core::sync::atomic::{AtomicU32, Ordering}; use toyos_abi::RawHandle; -use toyos_abi::syscall; +use toyos_abi::{clock, syscall}; use toyos_abi::inbox::{ Submission, Completion, RingHeader, RingLayout, OP_WATCH, SUBMISSION_RING_OFF, COMPLETION_RING_OFF, SUBMISSIONS_OFF, @@ -166,6 +167,75 @@ impl Rings { } } +/// The registrations that can still answer, at most one per handle. +/// +/// **What the kernel hands back is the registration's number, never the +/// caller's token.** Watching a handle again replaces its registration — the +/// key `process_watch` withdraws an armed poll by — but an answer the replaced +/// one has posted stays in the ring, and so does one it posts after a +/// replacement that answered at once. Under the caller's token either reads as +/// news about the handle, of bytes already read or already announced. Under a +/// number, an answer that is not its handle's latest registration's names +/// nothing here and is dropped. +struct Registry { + live: [Registration; MAX_LIVE], + len: usize, + /// Twice the poller's capacity: the handles it watches, and as many again + /// closed since the last wait whose end the ring has not yet handed out. + limit: usize, + next: u64, +} + +#[derive(Clone, Copy)] +struct Registration { + handle: RawHandle, + token: u64, + number: u64, +} + +const VACANT: Registration = Registration { handle: RawHandle(0), token: 0, number: 0 }; + +/// The widest registry, for a poller of [`Poller::MAX_HANDLES`]. +const MAX_LIVE: usize = 2 * Poller::MAX_HANDLES as usize; + +impl Registry { + fn new(limit: usize) -> Self { + Self { live: [VACANT; MAX_LIVE], len: 0, limit, next: 0 } + } + + /// The number a registration of `handle` is submitted under; whatever the + /// handle's earlier registration posts answers nothing from here on. + fn register(&mut self, handle: RawHandle, token: u64) -> u64 { + let registration = Registration { handle, token, number: self.next }; + self.next += 1; + match self.live[..self.len].iter_mut().find(|r| r.handle == handle) { + Some(replaced) => *replaced = registration, + None => { + assert!( + self.len < self.limit, + "Poller: {} handles hold a registration that has not answered, the most \ + a poller of capacity {} keeps: it watches past its declared set", + self.len, + self.limit / 2, + ); + self.live[self.len] = registration; + self.len += 1; + } + } + registration.number + } + + /// The caller's token for the answer posted under `number`, which ends its + /// registration, or `None` for one a later registration replaced. + fn answer(&mut self, number: u64) -> Option { + let at = self.live[..self.len].iter().position(|r| r.number == number)?; + let token = self.live[at].token; + self.len -= 1; + self.live[at] = self.live[self.len]; + Some(token) + } +} + /// An inbox, for watching handles for readiness. /// /// Owns the inbox handle and shared memory mapping. Submissions are batched @@ -193,18 +263,29 @@ impl Rings { /// [`wait`](Self::wait) reads the kernel's drop counter on every call — an /// assert that should be unreachable, kept because that is the shape a /// fail-fast check is supposed to have. +/// +/// **A token [`wait`](Self::wait) hands out is the answer of its handle's +/// latest registration, and a registration answers once.** Watching a handle +/// replaces its earlier registration, and whatever that one posts, before the +/// replacement or after it, is dropped (`Registry`). So a caller that watches +/// a handle before every wait is never told twice of one arrival, nor of bytes +/// it read before that watch: what it is told of is there to read. A +/// registration left standing across waits answers whenever its handle turns +/// ready, so a caller that reads such a handle untold watches it again before +/// it waits. pub struct Poller { inbox: RawHandle, rings: Rings, capacity: u32, + registry: RefCell, } // Safety: the base pointer is process-local shared memory mapped from the // kernel. It is only ever reached through `Rings`, which takes no reference // over it: atomics for the shared words, whole-value volatile copies for -// everything else. +// everything else. Not `Sync`: a watch moves the submission tail in two steps, +// and the registry is a `RefCell`. unsafe impl Send for Poller {} -unsafe impl Sync for Poller {} impl Poller { /// Widest handle set one poller can carry — the kernel's deepest @@ -245,7 +326,12 @@ impl Poller { rings.submission_ring_size, rings.completion_ring_size, ); - Self { inbox, rings, capacity } + Self::over(inbox, rings, capacity) + } + + fn over(inbox: RawHandle, rings: Rings, capacity: u32) -> Self { + let registry = RefCell::new(Registry::new(2 * capacity as usize)); + Self { inbox, rings, capacity, registry } } /// Watch the given handle for readiness. @@ -272,6 +358,7 @@ impl Poller { self.pending(), self.capacity, ); + let number = self.registry.borrow_mut().register(handle, token); let tail = self.rings.submission_tail().load(Ordering::Acquire); let idx = tail & (self.rings.submission_ring_size - 1); self.rings.write_submission( @@ -280,7 +367,7 @@ impl Poller { op: OP_WATCH, handle, op_flags: flags, - token, + token: number, ..Submission::default() }, ); @@ -304,20 +391,59 @@ impl Poller { .expect("Poller::submit: inbox_submit rejected the batch"); } - /// Submit pending entries and wait for completions. - /// - /// Blocks until at least `min_complete` completions are ready or `timeout_nanos` - /// elapses. Calls `f` for each completed token. + /// Submit pending entries and hand `f` the token of every answer, until at + /// least `min_complete` have been handed out or `timeout_nanos` has passed + /// — `0` looks once, `u64::MAX` never passes. pub fn wait(&self, min_complete: u32, timeout_nanos: u64, mut f: impl FnMut(u64)) { - self.submit(min_complete, timeout_nanos); - self.drain(&mut f); + self.wait_on( + min_complete, + timeout_nanos, + &mut f, + |min, nanos| self.submit(min, nanos), + clock::nanos_since_boot, + ); } - /// Read every completion the kernel has published, oldest first. + /// [`wait`](Self::wait) over the kernel's half and a clock it is handed, + /// so a host test can hand it fakes. + /// + /// The kernel counts a dropped answer towards `min_complete` and returns + /// for it, so the wait goes on, for what is left of its time, until the + /// answers handed out make up the count. + fn wait_on( + &self, + min_complete: u32, + timeout_nanos: u64, + f: &mut impl FnMut(u64), + mut submit: impl FnMut(u32, u64), + now: impl Fn() -> u64, + ) { + let deadline = now().saturating_add(timeout_nanos); + let mut nanos = timeout_nanos; + let mut handed = 0; + loop { + submit(min_complete - handed, nanos); + handed += self.drain(f); + if handed >= min_complete { + return; + } + nanos = match timeout_nanos { + 0 => return, + u64::MAX => u64::MAX, + _ => match deadline.checked_sub(now()) { + Some(left) if left > 0 => left, + _ => return, + }, + }; + } + } + + /// Read every completion the kernel has published, oldest first, and hand + /// `f` the token of each that answers a live registration; how many did. /// /// Split from [`wait`](Self::wait) because it is the half that is a pure /// function of the page: a host test can hand it a fake one. - fn drain(&self, f: &mut impl FnMut(u64)) { + fn drain(&self, f: &mut impl FnMut(u64)) -> u32 { // Unreachable, and kept for that reason: `capacity` bounds the // registrations and the rings are sized from `capacity`, so nothing a // conforming caller does can make the kernel drop a completion here. @@ -332,14 +458,16 @@ impl Poller { self.capacity, self.rings.submission_ring_size, self.rings.completion_ring_size, ); + let mut handed = 0; loop { let head = self.rings.completion_head().load(Ordering::Acquire); let tail = self.rings.completion_tail().load(Ordering::Acquire); if head == tail { - break; + return handed; } let idx = head & (self.rings.completion_ring_size - 1); let completion = self.rings.completion_at(idx); + self.rings.completion_head().store(head.wrapping_add(1), Ordering::Release); // Do not filter on `completion.result`. A negative result is the // kernel saying the registration is over and will never fire // (a watched handle's close answers every poll on a watch it ends @@ -347,8 +475,11 @@ impl Poller { // to that exactly as to readiness — by looking at the handle again. // A zero result is meaningful too: `OP_ACCEPT` reports handle 0 // that way. - f(completion.token); - self.rings.completion_head().store(head.wrapping_add(1), Ordering::Release); + let Some(token) = self.registry.borrow_mut().answer(completion.token) else { + continue; + }; + f(token); + handed += 1; } } } @@ -362,6 +493,8 @@ impl Drop for Poller { #[cfg(test)] mod tests { use super::*; + use core::cell::Cell; + use core::mem::ManuallyDrop; use toyos_abi::inbox::{ RING_DROPPED_OFF, RING_HEAD_OFF, RING_SIZE_OFF, RING_TAIL_OFF, }; @@ -434,6 +567,109 @@ mod tests { .write(entry); } } + + /// The submission in slot `index`, copied out as `submission_at` does. + fn submission(&mut self, index: u32) -> Submission { + let base = self.base(); + // SAFETY: `index` is under the submission ring size, so the slot + // is inside `PAGE_BYTES`; `SUBMISSIONS_OFF` is page-aligned and + // `Submission` is 40 bytes, 8-aligned. + unsafe { + (base.add(SUBMISSIONS_OFF as usize + index as usize * core::mem::size_of::()) + as *const Submission) + .read_volatile() + } + } + } + + /// The kernel's half of a watch, as `process_watch` and an object's post + /// do it, over a [`FakePage`]: a submission on a handle already ready is + /// answered at once and leaves any earlier poll on it armed; one on a + /// handle not ready withdraws the earlier poll and arms; a post answers + /// every poll armed on its handle. Every answer carries its submission's + /// token, as the kernel's do. + struct FakeKernel { + page: FakePage, + ready: Vec, + armed: Vec<(RawHandle, u64)>, + } + + /// A poller of `capacity` and the kernel behind its page. The poller is + /// never dropped: its `Drop` is a syscall. + fn pair(capacity: u32) -> (FakeKernel, ManuallyDrop) { + let entries = capacity.next_power_of_two(); + let mut page = FakePage::new(entries, 2 * entries); + let poller = ManuallyDrop::new(Poller::over(RawHandle(0), rings(&mut page), capacity)); + (FakeKernel { page, ready: Vec::new(), armed: Vec::new() }, poller) + } + + impl FakeKernel { + /// `inbox_submit`'s first half: every queued submission, registered. + fn submit(&mut self) { + let head_at = SUBMISSION_RING_OFF as usize + RING_HEAD_OFF; + let size = self.page.get(SUBMISSION_RING_OFF as usize + RING_SIZE_OFF); + loop { + let head = self.page.get(head_at); + if head == self.page.get(SUBMISSION_RING_OFF as usize + RING_TAIL_OFF) { + return; + } + let s = self.page.submission(head & (size - 1)); + self.page.put(head_at, head.wrapping_add(1)); + if self.ready.contains(&s.handle) { + self.answer(s.token); + } else { + self.armed.retain(|&(h, _)| h != s.handle); + self.armed.push((s.handle, s.token)); + } + } + } + + /// Bytes reach `handle`, and its post has not run yet. + fn fill(&mut self, handle: RawHandle) { + self.ready.push(handle); + } + + /// `handle`'s post: every poll armed on it answers. + fn post(&mut self, handle: RawHandle) { + let (fired, armed): (Vec<_>, Vec<_>) = + core::mem::take(&mut self.armed).into_iter().partition(|&(h, _)| h == handle); + self.armed = armed; + for (_, token) in fired { + self.answer(token); + } + } + + /// Bytes reach `handle` and its post runs. + fn arrive(&mut self, handle: RawHandle) { + self.fill(handle); + self.post(handle); + } + + /// `handle`'s bytes are read by a call that did not ask the poller. + fn take(&mut self, handle: RawHandle) { + self.ready.retain(|&h| h != handle); + } + + /// Answers posted and not yet drained. + fn posted(&mut self) -> u32 { + let head = self.page.get(COMPLETION_RING_OFF as usize + RING_HEAD_OFF); + self.page.get(COMPLETION_RING_OFF as usize + RING_TAIL_OFF).wrapping_sub(head) + } + + fn answer(&mut self, token: u64) { + let tail_at = COMPLETION_RING_OFF as usize + RING_TAIL_OFF; + let tail = self.page.get(tail_at); + let size = self.page.get(COMPLETION_RING_OFF as usize + RING_SIZE_OFF); + self.page.post(tail & (size - 1), Completion { token, result: READABLE as i32, flags: 0 }); + self.page.put(tail_at, tail.wrapping_add(1)); + } + } + + /// Every token one drain hands out. + fn drained(poller: &Poller) -> Vec { + let mut seen = Vec::new(); + poller.drain(&mut |token| seen.push(token)); + seen } fn rings(page: &mut FakePage) -> Rings { @@ -559,4 +795,148 @@ mod tests { let r = rings(&mut page); assert_eq!(r.pending(), 3); } + + const H: RawHandle = RawHandle(5); + const G: RawHandle = RawHandle(6); + + /// A registration a wait did not see answer, answered by bytes a call that + /// did not ask the poller read; the next watch finds the handle empty. The + /// answer still in the ring announces bytes that are gone, and a reader + /// that took it for news would block in its read. + #[test] + fn an_answer_for_bytes_already_read_is_not_handed_out() { + let (mut kernel, poller) = pair(1); + poller.watch_raw(H, READABLE, 7); + kernel.submit(); + assert!(drained(&poller).is_empty()); + kernel.arrive(H); + kernel.take(H); + poller.watch_raw(H, READABLE, 7); + kernel.submit(); + assert!(drained(&poller).is_empty()); + kernel.arrive(H); + assert_eq!(drained(&poller), [7]); + } + + /// A watch that finds its handle ready is answered at once and leaves the + /// poll it replaced armed, which answers again when the post that made the + /// handle ready runs. + #[test] + fn an_answer_the_replaced_registration_posts_late_is_not_handed_out() { + let (mut kernel, poller) = pair(1); + poller.watch_raw(H, READABLE, 1); + kernel.submit(); + kernel.fill(H); + poller.watch_raw(H, READABLE, 2); + kernel.submit(); + kernel.post(H); + assert_eq!(drained(&poller), [2]); + } + + /// epoll(7): "Does an operation on a file descriptor affect the already + /// collected but not yet reported events? … Modify will reread available + /// I/O." An answer collected before its handle is watched again is + /// reported once, under the new watch's token. + #[test] + fn a_handle_watched_again_is_answered_once_under_its_new_token() { + let (mut kernel, poller) = pair(1); + poller.watch_raw(H, READABLE, 1); + kernel.submit(); + kernel.arrive(H); + poller.watch_raw(H, READABLE, 2); + kernel.submit(); + assert_eq!(drained(&poller), [2]); + } + + /// A registration the caller does not renew is not replaced: it answers in + /// whichever later wait its handle turns ready. + #[test] + fn a_registration_left_standing_still_answers() { + let (mut kernel, poller) = pair(2); + poller.watch_raw(H, READABLE, 3); + poller.watch_raw(G, READABLE, 4); + kernel.submit(); + assert!(drained(&poller).is_empty()); + poller.watch_raw(G, READABLE, 4); + kernel.submit(); + kernel.arrive(H); + assert_eq!(drained(&poller), [3]); + } + + /// The kernel leaves a wait for an answer the poller then drops, as + /// `an_answer_for_bytes_already_read_is_not_handed_out` sets up, and the + /// wait goes on to the live one rather than returning with none. + #[test] + fn a_wait_sleeps_past_a_dropped_answer_to_the_live_one() { + let (mut kernel, poller) = pair(1); + poller.watch_raw(H, READABLE, 7); + kernel.submit(); + kernel.arrive(H); + kernel.take(H); + poller.watch_raw(H, READABLE, 7); + let mut submits = 0; + let mut seen = Vec::new(); + let submit = |_, _| { + kernel.submit(); + submits += 1; + if submits == 2 { + kernel.arrive(H); + } + }; + poller.wait_on(1, u64::MAX, &mut |token| seen.push(token), submit, || 0); + assert_eq!((seen, submits), (vec![7], 2)); + } + + /// A wait woken only by answers it drops ends at its deadline, and sleeps + /// only what is left of it. + #[test] + fn a_wait_woken_only_by_dropped_answers_ends_at_its_deadline() { + let (mut kernel, poller) = pair(1); + poller.watch_raw(H, READABLE, 7); + kernel.submit(); + kernel.arrive(H); + kernel.take(H); + poller.watch_raw(H, READABLE, 7); + let clock = Cell::new(100); + let mut slept = Vec::new(); + let mut seen = Vec::new(); + let submit = |min, nanos| { + kernel.submit(); + slept.push(nanos); + // Back 300 ns later for what is posted, and at its deadline for nothing. + clock.set(clock.get() + if kernel.posted() >= min { 300 } else { nanos }); + }; + poller.wait_on(1, 1_000, &mut |token| seen.push(token), submit, || clock.get()); + assert_eq!((seen, slept), (vec![], vec![1_000, 700])); + } + + /// A wait of zero looks once, whatever it drops. + #[test] + fn a_wait_of_zero_looks_once() { + let (mut kernel, poller) = pair(1); + poller.watch_raw(H, READABLE, 7); + kernel.submit(); + kernel.arrive(H); + kernel.take(H); + poller.watch_raw(H, READABLE, 7); + let mut submits = 0; + let submit = |_, _| { + kernel.submit(); + submits += 1; + }; + poller.wait_on(1, 0, &mut |token| panic!("handed out {token}"), submit, || 0); + assert_eq!(submits, 1); + } + + /// Past the handles it declared and as many again closed and unreported, + /// a registration is refused by name. + #[test] + #[should_panic(expected = "watches past its declared set")] + fn a_registration_past_twice_the_capacity_panics() { + let (mut kernel, poller) = pair(1); + for handle in 1..=3 { + poller.watch_raw(RawHandle(handle), READABLE, 0); + kernel.submit(); + } + } } diff --git a/userland/fsd/src/main.rs b/userland/fsd/src/main.rs index 44a3b3e910c..14c93f03dba 100644 --- a/userland/fsd/src/main.rs +++ b/userland/fsd/src/main.rs @@ -207,7 +207,6 @@ fn main() { if end_at_mount == Some(role) && first_this_boot(&format!("end-at-mount-{role:?}")) { end_under_a_waiting_connection(&caps); } - let caps_len = caps.len() as u32; Server { volume, caps, @@ -220,7 +219,6 @@ fn main() { end_at_read, let_go_at_read, let_go_client: None, - probe: Poller::new(caps_len), scratch: Vec::new(), } .serve() @@ -383,9 +381,6 @@ struct Server { let_go_at_read: Option, /// The client whose next request ends this server, once [`LET_GO_AT_READ`] fired. let_go_client: Option, - /// Asks an acceptor whether a connection waits, before [`Server::accept`] - /// takes it. - probe: Poller, /// A write's bytes, copied out of the client's window or a stream's pipe /// into this process's own memory before the volume sees them. Kept, so a /// write allocates nothing. @@ -520,23 +515,9 @@ impl Server { } } - /// Take a connection that waits on `cap`'s port, if one does. - /// - /// **Asked first, because `accept` parks and a completion is a hint**: a - /// watch replaced while a connection arrives can answer beside the watch - /// that replaced it, so two completions name one connection, and the - /// second `accept` would park this server for good. This process is the - /// port's one acceptor, so a connection the probe sees is still there to - /// take. The probe's ring is drained whole each time and a completion is - /// read by its port's token, so what counts is an arrival on this port - /// since its last probe, none of them taken. + /// Take the connection `cap`'s port answered ready for: this process is + /// the port's one acceptor, so it is still queued. fn accept(&mut self, cap: usize) { - self.probe.watch(&self.caps[cap].acceptor, READABLE, cap as u64); - let mut waiting = false; - self.probe.wait(0, 0, |token| waiting |= token == cap as u64); - if !waiting { - return; - } let conn = match self.caps[cap].acceptor.accept() { Ok(conn) => conn, Err(why) => panic!("fsd: its own acceptor refused an accept: {why:?}"), From 9ba71ec0955b7ce418a0bd93418e34a4ba07eacd Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 08:46:02 +0200 Subject: [PATCH 03/14] The terminal and toyos-window read on the poller's answer again With toyos::poller handing out only a handle's latest registration's answer, and `wait` returning only with one or at its timeout, a blocking read on an answer finds what it was told of. The per-reader workarounds of 85cd3b614 go back to main's code: toyos-window's `FrameRx` read path, `try_event` and its deadline loop, the terminal's `pipe_end` and `read_ready`, and its `toyos-abi` dependency. The console and `surface::Host::accept`, which the issue this branch filed named, read on answers of registrations they renew before every wait, so the poller's rule covers them and the issue is deleted. `toolkit_window_spent` goes with the per-reader loop it pinned: the decision it reached is the poller's, and `toyos/src/poller.rs`'s host tests reach it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...-surface-host-wait-on-a-spent-readiness.md | 29 ------ .../toyos-rust-tests/src/bin/window_spent.rs | 76 --------------- tests/toyos.rs | 35 +------ userland/Cargo.lock | 1 - userland/terminal/Cargo.toml | 1 - userland/terminal/src/main.rs | 69 +++++--------- userland/toyos-window/src/lib.rs | 92 ++++++------------- 7 files changed, 49 insertions(+), 254 deletions(-) delete mode 100644 issues/design-debt/console-and-surface-host-wait-on-a-spent-readiness.md delete mode 100644 tests/toyos-rust-tests/src/bin/window_spent.rs diff --git a/issues/design-debt/console-and-surface-host-wait-on-a-spent-readiness.md b/issues/design-debt/console-and-surface-host-wait-on-a-spent-readiness.md deleted file mode 100644 index 990fd31f100..00000000000 --- a/issues/design-debt/console-and-surface-host-wait-on-a-spent-readiness.md +++ /dev/null @@ -1,29 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-01 ---- - -# The console and `surface::Host::accept` wait on a readiness answer that may be spent - -A poller's `READABLE` is a cue to look, not a promise that bytes are there: a -poll an earlier round left armed answers for a write a later round already read -(`process_watch` in `kernel/src/inbox/mod.rs` completes an already-ready handle -at once and leaves that poll registered, and `toyos::poller`'s capacity counts -both answers). The terminal and `toyos-window` read past such an answer without -waiting. Two readers still block on one: - -- `userland/console/src/main.rs` reads the shell's stdout and stderr with std's - blocking `read` on `TOKEN_STDOUT` and `TOKEN_STDERR`. A spent answer parks the - console until the shell writes again, and a shell at its prompt is waiting for - the keys the parked console no longer forwards. -- `toyos::surface::Host::accept` (`toyos/src/surface.rs`) calls the blocking - `Acceptor::accept` on the acceptor reading ready, so a spent answer parks the - terminal and the console until the next client connects. It is under - `toyos/src`, so it is an ABI brief's. - -Owner: `userland/console` and `toyos::surface`. - -**Exit**: neither waits on a readiness answer — the console reads its shell's -pipes as the terminal does, and `Host::accept` takes a connection only when one -is queued. diff --git a/tests/toyos-rust-tests/src/bin/window_spent.rs b/tests/toyos-rust-tests/src/bin/window_spent.rs deleted file mode 100644 index 348173d113a..00000000000 --- a/tests/toyos-rust-tests/src/bin/window_spent.rs +++ /dev/null @@ -1,76 +0,0 @@ -//! A window's event reads never wait on a readiness answer whose frame was -//! already read. -//! -//! A timed `poll_event` that finds nothing leaves the window's own poll armed. -//! The compositor's frame event answers it, and `recv_event` reads that frame -//! without looking at the answer, so the answer is still waiting in the -//! window's ring with nothing behind it. `poll_event(0)` has to say `None` at -//! once, a timed `poll_event` has to wait past the answer to its timeout, and -//! the next frame event has to be read. - -use std::process::exit; - -use toyos::poller::{Poller, READABLE}; -use window::{Event, Window}; - -/// Long enough for the wait to register its poll, which is all it is for: -/// nothing is sent to a window that has not presented, whatever this is. -const ARM_NANOS: u64 = 1_000_000; - -fn fail(what: &str) -> ! { - println!("WINDOW-SPENT-FAIL {what}"); - exit(1); -} - -fn named(event: &Option) -> &'static str { - match event { - None => "nothing", - Some(Event::KeyInput(_)) => "KeyInput", - Some(Event::MouseInput(_)) => "MouseInput", - Some(Event::ClipboardPaste(_)) => "ClipboardPaste", - Some(Event::Resized) => "Resized", - Some(Event::Close) => "Close", - Some(Event::LayoutChanged) => "LayoutChanged", - Some(Event::Frame) => "Frame", - } -} - -fn main() { - let mut window = Window::create_with_title(160, 100, "spent").unwrap_or_else(|e| { - println!("WINDOW-SPENT-REFUSED {e}"); - exit(1); - }); - - let armed = window.poll_event(ARM_NANOS); - if armed.is_some() { - fail(&format!("{} arrived before anything was presented", named(&armed))); - } - - window.present(); - // The frame event's post answers the window's armed poll before this one, - // which registered after it. - let arrival = Poller::new(1); - arrival.watch_raw(window.handle(), READABLE, 0); - arrival.wait(1, u64::MAX, |_| {}); - drop(arrival); - let frame = Some(window.recv_event()); - if !matches!(frame, Some(Event::Frame)) { - fail(&format!("the first present was answered with {}", named(&frame))); - } - - let at_once = window.poll_event(0); - if at_once.is_some() { - fail(&format!("poll_event(0) after the spent answer read {}", named(&at_once))); - } - let timed = window.poll_event(ARM_NANOS); - if timed.is_some() { - fail(&format!("a timed poll_event after the spent answer read {}", named(&timed))); - } - - window.present(); - let next = Some(window.recv_event()); - if !matches!(next, Some(Event::Frame)) { - fail(&format!("the second present was answered with {}", named(&next))); - } - println!("WINDOW-SPENT-OK"); -} diff --git a/tests/toyos.rs b/tests/toyos.rs index 337fadf028f..52ce975d3e7 100644 --- a/tests/toyos.rs +++ b/tests/toyos.rs @@ -302,7 +302,6 @@ const RUST_SKIP: &[&str] = &[ // Same again: the `toolkit_` tests of their names launch them from the // toolkit desktop. "window_wake", - "window_spent", "winit_loop", "winit_pace", // Its two spawning arms only mean anything when the two processes share a @@ -1167,11 +1166,9 @@ const MACHINE_TESTS: &[(&str, Sched)] = &[ // window, its text in the system font, no redraw it did not ask for, and // a clean exit when the compositor closes it. ("toolkit_iced", Sched::Parallel), - // The wait every winit loop blocks in, a window's reads past a readiness - // already spent, the loop itself through winit's API, and an animation - // held to the compositor's frame events. + // The wait every winit loop blocks in, the loop itself through winit's + // API, and an animation held to the compositor's frame events. ("toolkit_window_wake", Sched::Parallel), - ("toolkit_window_spent", Sched::Parallel), ("toolkit_winit_loop", Sched::Parallel), ("toolkit_winit_pace", Sched::Parallel), // Ctrl+Alt+D on the same machine. Parallel: it waits for a marker and its @@ -1420,7 +1417,6 @@ const CARRIES: &[(&str, &[&str])] = &[ ("metal_sim_hostile_clipboard", &["test_rs_compositor_hostile_clipboard"]), ("desktop_window_child", &["test_rs_window_child"]), ("toolkit_window_wake", &["test_rs_window_wake"]), - ("toolkit_window_spent", &["test_rs_window_spent"]), ("toolkit_winit_loop", &["test_rs_winit_loop"]), ("toolkit_winit_pace", &["test_rs_winit_pace"]), ("doom_frames", &["test_rs_doom_frames"]), @@ -7967,32 +7963,6 @@ fn toolkit_window_wake(rust_bins: &[(String, Vec)]) -> Result<(), String> { Err(format!("test_rs_window_wake never finished:\n{}", &log[launched..])) } -/// A window's event reads never wait on a readiness answer whose frame another -/// read already took. -/// -/// `tests/toyos-rust-tests/src/bin/window_spent.rs` leaves such an answer in -/// its window's ring and reads past it; a read that waits on it hangs, and the -/// ceiling reds that. -fn toolkit_window_spent(rust_bins: &[(String, Vec)]) -> Result<(), String> { - let (mut qemu, mut log, launched) = toolkit_launch(rust_bins, "window_spent", "test_rs_window_spent")?; - let log = &mut log; - let mut live = qemu::Liveness::new(Duration::from_secs(30), Duration::from_secs(120)); - while live.working(log) { - let said = &log[launched..]; - if said.contains("WINDOW-SPENT-OK") { - return Ok(()); - } - if said.contains("WINDOW-SPENT-FAIL") - || said.contains("WINDOW-SPENT-REFUSED") - || said.contains("panicked") - { - return Err(format!("a window read past a spent readiness went wrong:\n{said}")); - } - log.push_str(&qemu.drain_serial(Duration::from_millis(200))); - } - Err(format!("test_rs_window_spent never finished:\n{}", &log[launched..])) -} - /// The ToyOS winit backend's loop through winit's own API: user events sent /// from `AboutToWait` and from another thread, windows redrawn and dropped on /// another thread, a window dropped in the handler that made it, one dropped in @@ -10499,7 +10469,6 @@ fn run_machine_test( "desktop_window_child" => desktop_window_child(rust_bins), "toolkit_iced" => toolkit_iced(), "toolkit_window_wake" => toolkit_window_wake(rust_bins), - "toolkit_window_spent" => toolkit_window_spent(rust_bins), "toolkit_winit_loop" => toolkit_winit_loop(rust_bins), "toolkit_winit_pace" => toolkit_winit_pace(rust_bins), "blocked_dump" => blocked_dump(), diff --git a/userland/Cargo.lock b/userland/Cargo.lock index 52f98400ae7..5489d455e17 100644 --- a/userland/Cargo.lock +++ b/userland/Cargo.lock @@ -3912,7 +3912,6 @@ name = "terminal" version = "0.1.0" dependencies = [ "toyos", - "toyos-abi", "toyos-font", "toyos-window", ] diff --git a/userland/terminal/Cargo.toml b/userland/terminal/Cargo.toml index e6645c56f03..13c03bd5461 100644 --- a/userland/terminal/Cargo.toml +++ b/userland/terminal/Cargo.toml @@ -6,7 +6,6 @@ license = "MIT OR Apache-2.0" [dependencies] toyos = { path = "../../toyos" } -toyos-abi = { path = "../../toyos-abi" } toyos-font = { path = "../toyos-font" } toyos-window = { path = "../toyos-window" } diff --git a/userland/terminal/src/main.rs b/userland/terminal/src/main.rs index 124a57d72f7..49cdc2f5f0e 100644 --- a/userland/terminal/src/main.rs +++ b/userland/terminal/src/main.rs @@ -6,7 +6,7 @@ //! instead of the bytes, which is what `locale detect` needs and what a //! terminal writing only translated bytes into a pipe could never give it. -use std::io::Write; +use std::io::{Read, Write}; use std::os::fd::AsRawFd; use std::os::toyos::process; use std::os::toyos::process::CommandExt; @@ -16,8 +16,7 @@ use terminal::Console; use toyos::poller::{Poller, READABLE}; use toyos::port; use toyos::surface::{self, Delivery, Host, Notice}; -use toyos::{Pipe, RawHandle}; -use toyos_abi::syscall::SyscallError; +use toyos::RawHandle; use window::Window; const TOKEN_STDOUT: u64 = 0; @@ -45,29 +44,6 @@ fn copy(text: &str) { } } -/// One of the shell's output pipes, as std spawned it, made a [`Pipe`] this -/// process can read without waiting. -fn pipe_end(end: impl AsRawFd) -> Pipe { - let raw = RawHandle(end.as_raw_fd() as u32); - // std's end is that handle and nothing more, and ToyOS's std gives a - // child's pipe no `IntoRawFd`: forgetting it hands the handle on unclosed. - std::mem::forget(end); - // SAFETY: a live pipe end this process holds, whose one other owner was - // just forgotten. - unsafe { Pipe::from_raw(raw) } -} - -/// What a readiness answer on one of the shell's pipes leads to: `None` where -/// the bytes it announced were already read, and an error read as the end, as -/// a blocking read's was. -fn read_ready(pipe: &Pipe, buf: &mut [u8]) -> Option { - match pipe.read_nonblock(buf) { - Ok(n) => Some(n), - Err(SyscallError::WouldBlock) => None, - Err(_) => Some(0), - } -} - fn main() { // **This terminal's surface is a port it makes, not a name it registers.** // One per instance: the connector goes into the namespace of the shell it @@ -91,8 +67,8 @@ fn main() { let mut console = Console::new(window.screen(), font); let mut shell_stdin = child.stdin.take().unwrap(); - let shell_stdout = pipe_end(child.stdout.take().unwrap()); - let shell_stderr = pipe_end(child.stderr.take().unwrap()); + let mut shell_stdout = child.stdout.take().unwrap(); + let mut shell_stderr = child.stderr.take().unwrap(); let poller = Poller::new(3 + Host::POLL_HANDLES); // The window exists and the shell's stdin is a pipe this process owns, so @@ -105,8 +81,8 @@ fn main() { eprintln!("terminal: ready"); loop { - poller.watch(&shell_stdout, READABLE, TOKEN_STDOUT); - poller.watch(&shell_stderr, READABLE, TOKEN_STDERR); + poller.watch_raw(RawHandle(shell_stdout.as_raw_fd() as u32), READABLE, TOKEN_STDOUT); + poller.watch_raw(RawHandle(shell_stderr.as_raw_fd() as u32), READABLE, TOKEN_STDERR); poller.watch_raw(window.handle(), READABLE, TOKEN_WINDOW); poller.watch_raw(host.acceptor_handle(), READABLE, TOKEN_LISTEN); for client in host.client_handles() { @@ -120,30 +96,28 @@ fn main() { if ready[TOKEN_STDOUT as usize] { let mut buf = [0u8; 4096]; - match read_ready(&shell_stdout, &mut buf) { - None => {} - Some(0) => { - // The child closes every fd together, so a last line it wrote - // to stderr right before exiting is already sitting in that - // pipe — drained here or it is lost with the loop. - if let Some(n @ 1..) = read_ready(&shell_stderr, &mut buf) { - console.write_bytes(&buf[..n]); - std::io::stdout().lock().write_all(&buf[..n]).ok(); - present(&console, &window); - } - break; - } - Some(n) => { + let n = shell_stdout.read(&mut buf).unwrap_or(0); + if n == 0 { + // The child closes every fd together, so a last line it wrote + // to stderr right before exiting is already sitting in that + // pipe — drained here or it is lost with the loop. + let n = shell_stderr.read(&mut buf).unwrap_or(0); + if n > 0 { console.write_bytes(&buf[..n]); std::io::stdout().lock().write_all(&buf[..n]).ok(); present(&console, &window); } + break; } + console.write_bytes(&buf[..n]); + std::io::stdout().lock().write_all(&buf[..n]).ok(); + present(&console, &window); } if ready[TOKEN_STDERR as usize] { let mut buf = [0u8; 4096]; - if let Some(n @ 1..) = read_ready(&shell_stderr, &mut buf) { + let n = shell_stderr.read(&mut buf).unwrap_or(0); + if n > 0 { console.write_bytes(&buf[..n]); std::io::stdout().lock().write_all(&buf[..n]).ok(); present(&console, &window); @@ -172,9 +146,8 @@ fn main() { } } - let event = if ready[TOKEN_WINDOW as usize] { window.try_event() } else { None }; - if let Some(event) = event { - match event { + if ready[TOKEN_WINDOW as usize] { + match window.recv_event() { // A client holding the grab takes the transition whole, and // the translator is not advanced — see `Window::text`. window::Event::KeyInput(key) if host.deliver(key.into()) == Delivery::Sent => {} diff --git a/userland/toyos-window/src/lib.rs b/userland/toyos-window/src/lib.rs index a5f7bc1f365..393343c75d3 100644 --- a/userland/toyos-window/src/lib.rs +++ b/userland/toyos-window/src/lib.rs @@ -8,9 +8,7 @@ pub use wait::{Waiter, Waker, Woke}; pub use toyos_abi::RawHandle; use toyos_abi::syscall::SyscallError; -use std::time::{Duration, Instant}; - -use toyos::ipc::{self, FrameRx, RxStep}; +use toyos::ipc; use toyos::AsHandle; use toyos::poller::{Poller, READABLE}; use toyos::endow::{self, EndowError}; @@ -470,9 +468,6 @@ fn copy(bytes: &[u8]) -> Result<(), CreateError> { pub struct Window { conn: Connection, - /// Every event is read through this, so none is read with a call that - /// waits: see [`Window::try_event`]. - rx: FrameRx, poller: Poller, shm: SharedMemory, width: u32, @@ -549,7 +544,6 @@ impl Window { let poller = Poller::new(1); Ok(Self { conn, - rx: FrameRx::new(), poller, shm, width: info.width, @@ -585,37 +579,11 @@ impl Window { let _ = self.conn.signal(MSG_LAYOUT_CHANGED); } - /// The next event, waiting for as long as that takes. pub fn recv_event(&mut self) -> Event { - loop { - if let Some(event) = self.try_event() { - return event; - } - self.wait_readable(u64::MAX); - } - } - - /// The next event if a whole one has arrived, and `None` at once if not. - /// - /// **The read for a loop that waits on [`Window::handle`] itself, because - /// a readiness answer is a cue to look and not a frame.** A poll left armed - /// by an earlier wait can answer for a frame a later read already took, and - /// a read that waits on that answer waits for the compositor's next - /// message — which a window that has stopped presenting is never sent. - pub fn try_event(&mut self) -> Option { - match self.rx.pump(&self.conn) { - RxStep::Idle => None, - RxStep::Eof | RxStep::Malformed => Some(Event::Close), - RxStep::Frame { msg_type, payload_len } => Some(self.decode_event(msg_type, payload_len)), - } - } - - /// Whether the connection read ready before `timeout_nanos` passed. - fn wait_readable(&self, timeout_nanos: u64) -> bool { - self.poller.watch(&self.conn, READABLE, 0); - let mut ready = false; - self.poller.wait(1, timeout_nanos, |_| ready = true); - ready + let Ok(header) = self.conn.recv_header() else { + return Event::Close; + }; + self.decode_event(&header) } /// The next event, or `None` when `timeout_nanos` passes — and `None` @@ -635,37 +603,23 @@ impl Window { if self.closed { return None; } - // `u64::MAX` is the poller's "forever", and so is a timeout no - // `Instant` reaches. - let deadline = (timeout_nanos != u64::MAX) - .then(|| Instant::now().checked_add(Duration::from_nanos(timeout_nanos))) - .flatten(); - loop { - if let Some(event) = self.try_event() { - self.closed = matches!(&event, Event::Close); - return Some(event); - } - let wait = match deadline { - None => u64::MAX, - Some(at) => match at.checked_duration_since(Instant::now()) { - Some(left) if !left.is_zero() => { - u64::try_from(left.as_nanos()).map_or(u64::MAX - 1, |n| n.min(u64::MAX - 1)) - } - _ => return None, - }, - }; - if !self.wait_readable(wait) { - return None; - } + self.poller.watch(&self.conn, READABLE, 0); + let mut ready = false; + self.poller.wait(1, timeout_nanos, |_| ready = true); + if !ready { + return None; } + let event = self.recv_event(); + self.closed = matches!(&event, Event::Close); + Some(event) } /// A message the compositor cannot have meant closes the window rather /// than killing the client: this side is a library inside somebody else's /// program, and it has a way to say "the session is over". - fn decode_event(&mut self, msg_type: u32, len: usize) -> Event { - match msg_type { - MSG_KEY_INPUT => match ipc::decode_payload::(self.rx.payload(len)) { + fn decode_event(&mut self, header: &ipc::IpcHeader) -> Event { + match header.msg_type { + MSG_KEY_INPUT => match self.conn.recv_payload::(header) { Ok(key) => Event::KeyInput(key), Err(_) => Event::Close, }, @@ -673,12 +627,12 @@ impl Window { load_layout(&mut self.translator); Event::LayoutChanged } - MSG_MOUSE_INPUT => match ipc::decode_payload(self.rx.payload(len)) { + MSG_MOUSE_INPUT => match self.conn.recv_payload(header) { Ok(ev) => Event::MouseInput(ev), Err(_) => Event::Close, }, MSG_WINDOW_RESIZED => { - let Ok(info) = ipc::decode_payload::(self.rx.payload(len)) else { + let Ok(info) = self.conn.recv_payload::(header) else { return Event::Close; }; let buf_size = info.stride as usize * info.height as usize * 4; @@ -697,9 +651,15 @@ impl Window { self.pixel_format = info.pixel_format; Event::Resized } - MSG_CLIPBOARD_PASTE => Event::ClipboardPaste(self.rx.payload(len).to_vec()), + MSG_CLIPBOARD_PASTE => { + let mut buf = [0u8; MAX_INLINE_PAYLOAD]; + let Ok(n) = self.conn.recv_bytes(header, &mut buf) else { + return Event::Close; + }; + Event::ClipboardPaste(buf[..n].to_vec()) + } MSG_CLIPBOARD_PASTE_SHM => { - let Ok(info) = ipc::decode_payload::(self.rx.payload(len)) else { + let Ok(info) = self.conn.recv_payload::(header) else { return Event::Close; }; let Some([buffer]) = self.conn.recv_handles_exact::<1>() else { From bd06aa1c0939ea079d26d086e270c904cf8f37da Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 08:46:02 +0200 Subject: [PATCH 04/14] issues: a Finder file in the C++ runtime scratch panics its removal Met by this branch's `cargo run -- --build-only`; the rerun passed. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...n-the-c-runtime-scratch-panics-its-removal.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 issues/build/a-finder-file-in-the-c-runtime-scratch-panics-its-removal.md diff --git a/issues/build/a-finder-file-in-the-c-runtime-scratch-panics-its-removal.md b/issues/build/a-finder-file-in-the-c-runtime-scratch-panics-its-removal.md new file mode 100644 index 00000000000..5bf50f98289 --- /dev/null +++ b/issues/build/a-finder-file-in-the-c-runtime-scratch-panics-its-removal.md @@ -0,0 +1,16 @@ +--- +status: open +kind: tooling +opened: 2026-10-01 +--- + +# A Finder file in the C++ runtime's scratch panics its removal + +`libcxx::build` ends with `fs::remove_dir_all(scratch)`, and the macOS Finder +wrote a `.DS_Store` into that scratch while the removal ran: `remove +/rust/build/sysroots/acd58e51940b13be.libcxx-x86_64: Directory not +empty (os error 66)` at `src/libcxx.rs:95`, after the runtime had installed. The +`cargo run -- --build-only` that hit it exited 101. + +**Exit**: a host writer in the scratch does not fail a build whose runtime +installed, with a test. From 4812962fe343c5973a83870bf2d4f30f524a4928 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 08:46:59 +0200 Subject: [PATCH 05/14] poller: an empty drain asserts against what it handed out So a red names the token that escaped instead of `is_empty()` failing bare. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- toyos/src/poller.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/toyos/src/poller.rs b/toyos/src/poller.rs index 553ef7bc76a..be7d982fdfe 100644 --- a/toyos/src/poller.rs +++ b/toyos/src/poller.rs @@ -808,12 +808,12 @@ mod tests { let (mut kernel, poller) = pair(1); poller.watch_raw(H, READABLE, 7); kernel.submit(); - assert!(drained(&poller).is_empty()); + assert_eq!(drained(&poller), [0u64; 0]); kernel.arrive(H); kernel.take(H); poller.watch_raw(H, READABLE, 7); kernel.submit(); - assert!(drained(&poller).is_empty()); + assert_eq!(drained(&poller), [0u64; 0]); kernel.arrive(H); assert_eq!(drained(&poller), [7]); } @@ -856,7 +856,7 @@ mod tests { poller.watch_raw(H, READABLE, 3); poller.watch_raw(G, READABLE, 4); kernel.submit(); - assert!(drained(&poller).is_empty()); + assert_eq!(drained(&poller), [0u64; 0]); poller.watch_raw(G, READABLE, 4); kernel.submit(); kernel.arrive(H); From 4a26c19ccc7bd1980674d95fae91e2467a1a5930 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 08:47:23 +0200 Subject: [PATCH 06/14] poller: a registration that answered holds no place in the registry Pins the removal in `Registry::answer`: without it a poller that watches one handle after another reaches its bound and panics a long-running server. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- toyos/src/poller.rs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/toyos/src/poller.rs b/toyos/src/poller.rs index be7d982fdfe..ef51ca874d6 100644 --- a/toyos/src/poller.rs +++ b/toyos/src/poller.rs @@ -928,6 +928,19 @@ mod tests { assert_eq!(submits, 1); } + /// A registration that answered holds no place, so a poller that watches + /// one handle after another for its whole life never reaches its bound. + #[test] + fn a_registration_that_answered_holds_no_place() { + let (mut kernel, poller) = pair(1); + for handle in 1..=3 { + poller.watch_raw(RawHandle(handle), READABLE, u64::from(handle)); + kernel.submit(); + kernel.arrive(RawHandle(handle)); + assert_eq!(drained(&poller), [u64::from(handle)]); + } + } + /// Past the handles it declared and as many again closed and unreported, /// a registration is refused by name. #[test] From 4aac6572372c2458f72726e39983d63e02cf8b8d Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 08:49:23 +0200 Subject: [PATCH 07/14] poller: the registry's bound says what it rests on After a wait every live registration is on a handle still open, because a close answers the poll on it and that wait handed the answer out, except a close `ops::close_ends_polls` says ends nothing; so at most the declared set stands, and one round adds at most the declared set again. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- toyos/src/poller.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/toyos/src/poller.rs b/toyos/src/poller.rs index ef51ca874d6..d3a44238462 100644 --- a/toyos/src/poller.rs +++ b/toyos/src/poller.rs @@ -180,8 +180,10 @@ impl Rings { struct Registry { live: [Registration; MAX_LIVE], len: usize, - /// Twice the poller's capacity: the handles it watches, and as many again - /// closed since the last wait whose end the ring has not yet handed out. + /// Twice the poller's capacity. After a wait a live registration is on a + /// handle still open — a close answers the poll on it, and the wait handed + /// that out, unless `ops::close_ends_polls` says the close ends nothing — + /// so at most the declared set; a round adds at most the declared set again. limit: usize, next: u64, } @@ -941,8 +943,7 @@ mod tests { } } - /// Past the handles it declared and as many again closed and unreported, - /// a registration is refused by name. + /// Past twice the declared set, a registration is refused by name. #[test] #[should_panic(expected = "watches past its declared set")] fn a_registration_past_twice_the_capacity_panics() { From 7d31acac4da8ddaa5964d773c0597477c6d736e1 Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 09:59:06 +0200 Subject: [PATCH 08/14] inbox: a watch is answered after a look at its object, never by a post A reader stalled when a ring answered READABLE for bytes it had already read, and its blocking read then waited for good. The previous round's registry in toyos::poller dropped the answers of replaced registrations, but a post fires every armed poll without reading the object again, so a peer's zero-length write, or a write's post that lands after the reader took its bytes and watched again, still wrote an answer for nothing. The ring carries no readiness, so nothing in userland could filter that out; a token that only a non-blocking read accepts would have moved the burden onto every reader, and left libc's poll(), std's Read and every reader outside the SDK exposed. So the kernel no longer writes an answer when a post fires. A post fires the poll, which owes the ring a look (polls::Wake::owe), and the ring's own inbox_submit looks at the object again before it writes anything (polls::deliver): an object ready at that look is answered with the directions it is ready in, and one that is not is armed again. That is epoll's model: ep_send_events polls every ready item again (ep_item_poll) before it reports it. - A handle has one poll that may answer. A watch now replaces its handle's earlier poll on both paths, armed or fired, and a watch whose object is ready at once is a fired poll like any other, so one look answers a handle once, under its newest token. - The log and a console keep their post as the answer (ops::read_posts_are_readiness): the log's unread records are its reader's cursor's, and a console's watch is the keyboard's while its data is the serial line's, an open issue. Poll::posted records which watch posted, so a registrant's own look is never taken for a post. - A handle the process closed since it watched it answers -NotFound at the look; it is not the handle fault a submission naming one is. - A full completion ring stops the look and leaves the rest owed for the next wait, so a readiness answer is never dropped. - inbox_submit parks until a poll is owed a look as well as on its count. The debt is an AtomicBool, stored before the ring's watch is posted and swapped clear before each look. Deleted with it: toyos::poller's registry, its wait loop and their tests. The poller is main's again but not Sync, since a watch moves the submission tail in two steps. fsd's per-reader probe stays deleted, and the comment that claimed its acceptor was still queued goes. libc's poll() keeps one watch per fd, under the first entry naming it with every entry's interest, because a second watch on one fd replaces the first and its entry would go unanswered even when the fd was ready. kernel-loom/tests/inbox_answer.rs drives polls.rs against fake objects. post-is-an-answer is its negative control (a fired poll is answered with no look) and is red on the zero-length write, the late post and a look that re-arms; --ci host runs it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...e-in-a-build-directory-fails-the-build.md} | 0 ...he-c-runtime-scratch-panics-its-removal.md | 16 - kernel-loom/Cargo.toml | 10 + kernel-loom/src/lib.rs | 12 + kernel-loom/tests/inbox_answer.rs | 320 +++++++++++++ kernel/Cargo.toml | 5 + kernel/src/inbox/mod.rs | 260 ++++++----- kernel/src/inbox/once.rs | 17 +- kernel/src/inbox/polls.rs | 188 ++++++++ kernel/src/object/ops.rs | 15 + kernel/src/watch.rs | 18 +- src/ci.rs | 5 + toyos-abi/src/inbox.rs | 17 +- toyos-sched/src/watch.rs | 8 +- toyos/src/poller.rs | 430 +----------------- userland/fsd/src/main.rs | 2 - userland/libc/src/posix_io.rs | 21 +- 17 files changed, 760 insertions(+), 584 deletions(-) rename issues/build/{a-finder-file-in-a-store-directory-panics-its-sweep.md => a-finder-file-in-a-build-directory-fails-the-build.md} (100%) delete mode 100644 issues/build/a-finder-file-in-the-c-runtime-scratch-panics-its-removal.md create mode 100644 kernel-loom/tests/inbox_answer.rs create mode 100644 kernel/src/inbox/polls.rs diff --git a/issues/build/a-finder-file-in-a-store-directory-panics-its-sweep.md b/issues/build/a-finder-file-in-a-build-directory-fails-the-build.md similarity index 100% rename from issues/build/a-finder-file-in-a-store-directory-panics-its-sweep.md rename to issues/build/a-finder-file-in-a-build-directory-fails-the-build.md diff --git a/issues/build/a-finder-file-in-the-c-runtime-scratch-panics-its-removal.md b/issues/build/a-finder-file-in-the-c-runtime-scratch-panics-its-removal.md deleted file mode 100644 index 5bf50f98289..00000000000 --- a/issues/build/a-finder-file-in-the-c-runtime-scratch-panics-its-removal.md +++ /dev/null @@ -1,16 +0,0 @@ ---- -status: open -kind: tooling -opened: 2026-10-01 ---- - -# A Finder file in the C++ runtime's scratch panics its removal - -`libcxx::build` ends with `fs::remove_dir_all(scratch)`, and the macOS Finder -wrote a `.DS_Store` into that scratch while the removal ran: `remove -/rust/build/sysroots/acd58e51940b13be.libcxx-x86_64: Directory not -empty (os error 66)` at `src/libcxx.rs:95`, after the runtime had installed. The -`cargo run -- --build-only` that hit it exited 101. - -**Exit**: a host writer in the scratch does not fail a build whose runtime -installed, with a test. diff --git a/kernel-loom/Cargo.toml b/kernel-loom/Cargo.toml index 53a3aef831e..c0915af4bc4 100644 --- a/kernel-loom/Cargo.toml +++ b/kernel-loom/Cargo.toml @@ -49,6 +49,16 @@ wake-fence-off = [] # # Never on by default, and no kernel build can reach it. poll-fire-load-store = [] +# The negative control for a poll ring's answer. It makes `inbox/polls.rs` +# answer a fired poll with its interest and look at nothing, which is what a +# ring did before that file, so a post with nothing behind it is an answer and +# `inbox_answer.rs` must red: +# +# cargo test --manifest-path kernel-loom/Cargo.toml --features post-is-an-answer \ +# --test inbox_answer +# +# Never on by default, and no kernel build can reach it. +post-is-an-answer = [] # The negative control for the ticket lock's acquire edge. It makes `sync.rs`'s # two loads of `now` — the ones that decide ownership — `Relaxed`, so the # previous owner's writes are unordered against the next owner's reads, and diff --git a/kernel-loom/src/lib.rs b/kernel-loom/src/lib.rs index adfd920c320..4fb3120f294 100644 --- a/kernel-loom/src/lib.rs +++ b/kernel-loom/src/lib.rs @@ -183,6 +183,18 @@ pub mod device_irq; #[path = "../../kernel/src/inbox/once.rs"] pub mod poll_once; +/// `polls.rs` names its one-shot as `super::once`, which in the kernel is +/// `crate::inbox::once`; this is what makes that path resolve here. +pub use poll_once as once; + +extern crate alloc; + +/// A ring's polls and when one is answered, driven against a fake object by +/// `tests/inbox_answer.rs`. It names the one-shot above, `toyos-abi` and +/// `alloc`, and nothing of the kernel's. +#[path = "../../kernel/src/inbox/polls.rs"] +pub mod inbox_polls; + /// What `sleeplock.rs` names of the kernel's watch, and nothing more. /// /// **The park is shimmed, and that is the scope statement for diff --git a/kernel-loom/tests/inbox_answer.rs b/kernel-loom/tests/inbox_answer.rs new file mode 100644 index 00000000000..f28378fe959 --- /dev/null +++ b/kernel-loom/tests/inbox_answer.rs @@ -0,0 +1,320 @@ +//! **A post is not an answer**: a poll ring's answer is what the object held +//! when the ring's submitter looked, never what a post once announced. +//! +//! `kernel/src/inbox/polls.rs` against fake objects, standing in for +//! `inbox::submit`'s: a watch is the handle's one poll, a post fires every poll +//! armed on its object, and a submit looks at every fired poll's object again. +//! A reader that takes an answer for bytes and then reads blocking parks for +//! good on one written for nothing, which is why each case here is a reader's +//! sequence and its assertion the answers it is handed. +//! +//! In `loom::model`, because the one-shot under each poll is built from +//! loom's atomics in this crate; every model here is one thread. +//! +//! The negative case is a cargo feature: +//! +//! ```text +//! cargo test --manifest-path kernel-loom/Cargo.toml --features post-is-an-answer \ +//! --test inbox_answer +//! ``` +//! +//! answers a fired poll without looking, as a ring did before `polls.rs`, and +//! this file must red. + +#![cfg(feature = "loom")] + +use std::cell::{Cell, RefCell}; +use std::rc::Rc; +use std::sync::Arc; + +use kernel_loom::inbox_polls::{deliver, Look, Poll, Polls, Submitter, Wake}; +use toyos_abi::handle::RawHandle; +use toyos_abi::inbox::READABLE; +use toyos_abi::syscall::SyscallError; + +const H: RawHandle = RawHandle(5); +const G: RawHandle = RawHandle(6); +/// The log: what it holds for a reader is the reader's cursor's to say. +const L: RawHandle = RawHandle(7); + +/// The answer for bytes. +const BYTES: i32 = READABLE as i32; + +const NOTHING: [(u64, i32); 0] = []; + +/// What a fire tells: a count, standing in for the ring's waiter. +#[derive(Clone, Default)] +struct Owed(Rc>); + +impl Wake for Owed { + fn owe(&self) { + self.0.set(self.0.get() + 1); + } +} + +/// One object: whether it holds bytes, and the polls armed on its watch. +#[derive(Default)] +struct Object { + bytes: Cell, + /// Whether a post on it is its readiness, the kernel holding none to look at. + posts_are_readiness: bool, + armed: RefCell>>>, +} + +/// One ring's kernel over the objects `H`, `G` and `L`. +struct Kernel { + owed: Owed, + polls: RefCell>, + h: Object, + g: Object, + l: Object, + /// The completion ring, and how many answers it holds. + ring: RefCell>, + size: usize, +} + +impl Kernel { + fn new(size: usize) -> Self { + Self { + owed: Owed::default(), + polls: RefCell::new(Polls::new()), + h: Object::default(), + g: Object::default(), + l: Object { posts_are_readiness: true, ..Object::default() }, + ring: RefCell::new(Vec::new()), + size, + } + } + + fn object(&self, handle: RawHandle) -> &Object { + match handle { + H => &self.h, + G => &self.g, + L => &self.l, + other => panic!("no object behind {other:?}"), + } + } + + /// `OP_WATCH` for bytes on `handle`, answered under `token`. + fn watch(&self, handle: RawHandle, token: u64) { + self.arm(Poll::new(self.owed.clone(), token, handle, READABLE)); + } + + /// The handle's one poll: fired now if its object holds bytes, armed on + /// its watch if not. + fn arm(&self, poll: Poll) { + let poll = Arc::new(poll); + assert!(self.polls.borrow_mut().admit(poll.clone(), 16)); + let object = self.object(poll.handle); + if object.bytes.get() { + poll.fire(0); + } else { + object.armed.borrow_mut().push(poll); + } + } + + /// Bytes reach the object; its post has not run. + fn fill(&self, handle: RawHandle) { + self.object(handle).bytes.set(true); + } + + /// The object's post: every poll armed on it fires, whatever it holds. + fn post(&self, handle: RawHandle) { + for poll in self.object(handle).armed.take() { + poll.fire(READABLE); + } + } + + /// The reader takes every byte the object holds. + fn take(&self, handle: RawHandle) { + self.object(handle).bytes.set(false); + } + + /// The object's source ends. + fn end(&self, handle: RawHandle) { + for poll in self.object(handle).armed.take() { + poll.end(); + } + } + + /// `inbox_submit`'s look at what the ring is owed. + fn submit(&self) { + deliver(|| self.polls.borrow_mut().take_owed(), self); + } + + /// Every answer the reader's drain hands it. + fn drain(&self) -> Vec<(u64, i32)> { + self.ring.take() + } +} + +impl Submitter for Kernel { + fn room(&self) -> bool { + self.ring.borrow().len() < self.size + } + + fn answer(&self, user_data: u64, result: i32) { + self.ring.borrow_mut().push((user_data, result)); + } + + fn look(&self, poll: &Poll) -> Look { + let object = self.object(poll.handle); + if object.bytes.get() || (object.posts_are_readiness && poll.posted() & READABLE != 0) { + return Look::Ready(READABLE); + } + self.arm(Poll::new(self.owed.clone(), poll.user_data, poll.handle, poll.flags)); + Look::Armed + } +} + +/// A peer's write of no bytes posts the pipe all the same +/// (`pipe::try_write` answers `Wrote(0)` and `sys_write` wakes the readers). +/// Nothing is there to read, so nothing is answered, and the poll armed again +/// answers the bytes that do come. +#[test] +fn a_post_with_nothing_to_read_answers_nothing() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(H, 7); + kernel.submit(); + assert_eq!(kernel.drain(), NOTHING); + + kernel.post(H); + kernel.submit(); + assert_eq!(kernel.drain(), NOTHING, "a post with nothing behind it was answered"); + + kernel.fill(H); + kernel.post(H); + kernel.submit(); + assert_eq!(kernel.drain(), [(7, BYTES)]); + }); +} + +/// A frame is two writes, and a write posts after it has published its +/// bytes. The header's post wakes the reader, which reads header and payload +/// and watches again before the payload's post has run; that post lands on +/// the new watch with nothing left to read. +#[test] +fn a_post_that_lands_after_its_bytes_were_read_answers_nothing() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(H, 7); + kernel.submit(); + + kernel.fill(H); + kernel.post(H); + kernel.submit(); + assert_eq!(kernel.drain(), [(7, BYTES)]); + + kernel.take(H); + kernel.watch(H, 7); + kernel.post(H); + kernel.submit(); + assert_eq!(kernel.drain(), NOTHING, "the payload's late post was answered"); + }); +} + +/// A post fires a poll its reader then replaces before its next wait: one +/// arrival, answered once, under the newest watch's token. +#[test] +fn a_watch_replaces_a_poll_a_post_already_fired() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(H, 1); + kernel.submit(); + + kernel.fill(H); + kernel.post(H); + kernel.watch(H, 2); + kernel.submit(); + assert_eq!(kernel.drain(), [(2, BYTES)]); + }); +} + +/// A look that finds nothing arms the poll again and goes on to the next one. +#[test] +fn a_poll_armed_again_does_not_end_the_look() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(H, 3); + kernel.watch(G, 4); + kernel.submit(); + + kernel.post(H); + kernel.fill(G); + kernel.post(G); + kernel.submit(); + assert_eq!(kernel.drain(), [(4, BYTES)]); + }); +} + +/// A watch replaces only its own handle's poll: one the reader did not renew +/// still answers when its object fills. +#[test] +fn a_poll_left_standing_still_answers() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(H, 3); + kernel.watch(G, 4); + kernel.submit(); + kernel.watch(G, 4); + kernel.submit(); + + kernel.fill(H); + kernel.post(H); + kernel.submit(); + assert_eq!(kernel.drain(), [(3, BYTES)]); + }); +} + +/// A source that ended is answered as gone: there is nothing to look at. +#[test] +fn an_ended_source_is_answered_gone_without_a_look() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(H, 7); + kernel.submit(); + + kernel.end(H); + kernel.submit(); + assert_eq!(kernel.drain(), [(7, -(SyscallError::NotFound as i32))]); + }); +} + +/// A full ring stops the look, and the poll it did not reach is answered by +/// the next wait's rather than dropped. +#[test] +fn a_full_ring_leaves_a_fired_poll_for_the_next_look() { + loom::model(|| { + let kernel = Kernel::new(1); + kernel.watch(H, 3); + kernel.watch(G, 4); + kernel.submit(); + + for handle in [H, G] { + kernel.fill(handle); + kernel.post(handle); + } + kernel.submit(); + assert_eq!(kernel.drain(), [(3, BYTES)]); + kernel.submit(); + assert_eq!(kernel.drain(), [(4, BYTES)]); + }); +} + +/// The log's unread records are a property of the reader's cursor, which the +/// kernel does not hold: there is nothing to look at, and its post is the +/// answer. +#[test] +fn a_post_answers_for_an_object_with_nothing_to_look_at() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(L, 7); + kernel.submit(); + assert_eq!(kernel.drain(), NOTHING); + + kernel.post(L); + kernel.submit(); + assert_eq!(kernel.drain(), [(7, BYTES)]); + }); +} diff --git a/kernel/Cargo.toml b/kernel/Cargo.toml index 5beca3f086b..cf3ff8fdc55 100644 --- a/kernel/Cargo.toml +++ b/kernel/Cargo.toml @@ -34,6 +34,11 @@ serial-try-lock-then-some = [] # `kernel-loom/tests/poll_once.rs` must red when it does — a poll two posts # both answer. poll-fire-load-store = [] +# The same for a poll ring's answer: `kernel-loom` turns it on so +# `inbox/polls.rs` answers a fired poll without looking at its object again, +# and `kernel-loom/tests/inbox_answer.rs` must red — a post with nothing to +# read, answered. +post-is-an-answer = [] # The same arrangement, five more times, one per `kernel-loom` model that had # no control of its own until 2026-08-17. Each is declared here and never # enabled here; `kernel-loom` turns exactly one on at a time and the named diff --git a/kernel/src/inbox/mod.rs b/kernel/src/inbox/mod.rs index 8e6ed014fd1..24d0ad2baf7 100644 --- a/kernel/src/inbox/mod.rs +++ b/kernel/src/inbox/mod.rs @@ -5,37 +5,40 @@ //! //! **A watch is a poll registered on the watched object's own //! [`Watch`](crate::watch::Watch)**, one entry per direction it asked for, and -//! the object's post completes it. There is no table of sources here: what a +//! the object's post fires it. There is no table of sources here: what a //! handle watches is `ops::read_watch`/`ops::write_watch`'s answer, and the //! poll holds no reference to the object at all. //! +//! **Only the ring's submitter writes a watch's answer, after a look** +//! ([`polls`]): a post owes the poll a look, and `submit` looks at the object +//! again before it writes anything, so an answer is never older than the wait +//! that returned it. +//! //! **A completion is a trust boundary, and the kernel is the only writer of //! its position.** `completion_tail` lives here, never in the page; the head -//! is the process's and is read once per post, so a head the process lies +//! is the process's and is read once per write, so a head the process lies //! about makes the kernel drop the completion and count it, never write //! outside the ring. What a completion says comes from the kernel — the -//! caller's own `token`, and a result that is either the direction the object -//! posted or the refusal — so a process cannot make one appear in another -//! process's ring or say something no object said. A post writes at most one -//! entry, and a ring holds at most [`MAX_PENDING_WATCHES`] polls. +//! caller's own `token`, and a result that is either the directions the object +//! was ready in or the refusal — so a process cannot make one appear in another +//! process's ring or say something no object said. A ring holds at most +//! [`MAX_PENDING_WATCHES`] polls. //! //! **Locks.** What a completion writes, and the page it is written into, sit -//! behind an [`IrqLock`] of their own, because a device's interrupt handler -//! posts in place, firing its polls under its list lock; nothing is taken -//! under it. The rest of a ring, its submissions and its polls, is its -//! `Lock`'s, which no post reaches. A ring's own watch is an [`IrqWatch`] and -//! holds only threads, because no handle names a ring as a thing to watch. +//! behind an [`IrqLock`] of their own; nothing is taken under it. The rest of a +//! ring, its submissions and its polls, is its `Lock`'s, which no post +//! reaches. A ring's own watch is an [`IrqWatch`] and holds only threads, +//! because no handle names a ring as a thing to watch. use alloc::sync::Arc; -use alloc::vec::Vec; -use core::sync::atomic::Ordering; +use core::sync::atomic::{AtomicBool, Ordering}; use toyos_sched::sync::CellLock; use toyos_sched::task::WaitClass; use toyos_sched::watch::{Fire, Ring}; use crate::object::shm::SharedMemObject; -use crate::object::{ops, KObjectRef}; +use crate::object::{ops, HandleError, KObjectRef}; use crate::process::{self, Pid}; use crate::scheduler; use crate::sync::Lock; @@ -51,8 +54,9 @@ use toyos_abi::handle::{RawHandle, Rights}; use toyos_abi::syscall::SyscallError; mod once; +mod polls; -use once::Once; +use polls::{Look, Polls, Submitter}; /// The one owned reference to a ring, held by its handle's object; dropping it /// tears the ring down. @@ -66,7 +70,7 @@ impl InboxRef { impl Drop for InboxRef { fn drop(&mut self) { - // The page is the completions', so it goes only once no post can reach + // The page is the completions', so it goes only once nothing can reach // them. Both halves are taken out under their locks and let go of // outside them: the unmap flushes. let Some(completions) = self.0.completions.with(Option::take) else { @@ -75,9 +79,7 @@ impl Drop for InboxRef { let Some(mut state) = self.0.state.lock().take() else { unreachable!("an inbox is torn down by its one reference, once"); }; - for poll in state.pending.drain(..) { - poll.withdraw(); - } + state.polls.withdraw_all(); // `Unmapped`'s drop flushes; the `Arc` drop after it frees the pages. drop(completions.shm.unmap_from(state.owner_pid)); } @@ -142,35 +144,20 @@ impl Readiness { if self.writable { flags |= WatchFlags::WRITABLE.raw(); } flags } -} -/// One `OP_WATCH` a ring is waiting on: one-shot across every watch it is -/// registered on and against its own registrant's recheck. -pub struct Poll { - inbox: Arc, - user_data: u64, - /// The handle the poll was submitted against; the dedup key. - handle: RawHandle, - /// Taken by exactly one of a fire and a withdrawal. - state: Once, -} - -impl Poll { - /// Post this poll's completion if nothing has answered it yet. - fn complete(&self, result: i32) { - if self.state.fire() { - self.inbox.complete(self.user_data, result); - } + fn any(self) -> bool { + self.readable || self.writable } +} - /// Answer nothing: a newer poll on the same handle replaced it, or its - /// ring went away. - fn withdraw(&self) { - let _ = self.state.withdraw(); - } +type Poll = polls::Poll>; - fn armed(&self) -> bool { - self.state.armed() +impl polls::Wake for Arc { + fn owe(&self) { + // Before the post, so the waiter it wakes finds the debt; in place, + // because a device's interrupt handler fires polls. + self.owed.store(true, Ordering::Release); + self.watch.post_in_place(); } } @@ -178,16 +165,15 @@ impl Poll { /// watch is. pub struct PollEntry { poll: Arc, - direction: Readiness, + direction: WatchFlags, } impl Ring for PollEntry { fn fire(&self, how: Fire) { - self.poll.complete(match how { - // The direction this watch is: its object posted it. - Fire::Ready => self.direction.result_flags() as i32, - Fire::Gone => -(SyscallError::NotFound as i32), - }); + match how { + Fire::Ready => self.poll.fire(self.direction.raw()), + Fire::Gone => self.poll.end(), + } } fn live(&self) -> bool { @@ -206,6 +192,8 @@ pub struct Inbox { completions: IrqLock>, /// Threads parked in `submit`; never a poll — see the module header. watch: IrqWatch, + /// A poll has been taken since `submit` last looked. + owed: AtomicBool, } struct RingState { @@ -213,8 +201,7 @@ struct RingState { /// lets go of only after it has taken this. shm_phys: DirectMap, submission_size: u32, - /// Polls still armed as of the last registration, which sweeps the rest. - pending: Vec>, + polls: Polls>, owner_pid: Pid, } @@ -249,8 +236,8 @@ impl RingState { } } -/// What a poll's completion writes, which a post from an interrupt handler -/// reaches, and the ring's page, which goes only with these. +/// What a poll's completion writes, and the ring's page, which goes only +/// with these. struct Completions { /// A ring's page has no lifetime of its own; it goes with the last handle to the ring. shm: Arc, @@ -281,8 +268,6 @@ impl Completions { } /// Posts a completion, or records a drop if the ring reports itself full. - /// A full ring is not fatal here: a poll completes on the poster's thread, - /// which belongs to a different process. fn post_completion(&mut self, user_data: u64, result: i32, flags: u32) { let tail = self.completion_tail; if tail.wrapping_sub(self.completion_head().load(Ordering::Acquire)) >= self.completion_size { @@ -303,6 +288,10 @@ impl Completions { self.completion_tail.wrapping_sub(head) } + fn room(&self) -> bool { + self.completion_count() < self.completion_size + } + /// Cumulative, never cleared. fn dropped(&self) -> u32 { self.completion_dropped().load(Ordering::Relaxed) @@ -310,7 +299,7 @@ impl Completions { } impl Inbox { - /// Post one completion and wake whoever waits in `submit`. A ring already + /// Write one completion and wake whoever waits in `submit`. A ring already /// torn down takes nothing and wakes nobody. fn complete(&self, user_data: u64, result: i32) { let posted = self.completions.with(|c| { @@ -391,7 +380,7 @@ pub fn create(depth: u32) -> Result<(InboxRef, u64), SyscallError> { state: Lock::new(Some(RingState { shm_phys, submission_size, - pending: Vec::new(), + polls: Polls::new(), owner_pid: pid, })), completions: IrqLock::new(Some(Completions { @@ -401,6 +390,7 @@ pub fn create(depth: u32) -> Result<(InboxRef, u64), SyscallError> { completion_tail: 0, })), watch: IrqWatch::new(), + owed: AtomicBool::new(false), }); Ok((InboxRef(inbox), shm_vaddr)) } @@ -427,18 +417,14 @@ impl Staged { completion_tail: 0, })), watch: IrqWatch::new(), + owed: AtomicBool::new(false), })) } - /// A poll of this ring on `watch`, which that watch's next post completes. + /// A poll of this ring on `watch`, which that watch's next post fires. pub(crate) fn poll(&self, watch: &IrqWatch) { - let poll = Arc::new(Poll { - inbox: self.0.clone(), - user_data: 0, - handle: RawHandle(0), - state: Once::new(), - }); - watch.add_poll(PollEntry { poll, direction: Readiness { readable: true, writable: false } }); + let poll = Arc::new(Poll::new(self.0.clone(), 0, RawHandle(0), WatchFlags::READABLE.raw())); + watch.add_poll(PollEntry { poll, direction: WatchFlags::READABLE }); } pub(crate) fn complete(&self) { @@ -474,6 +460,10 @@ pub fn submit( } loop { + // Before the debt is read again: a fire after this swap sets it anew. + // `AcqRel`, so the polls the swap clears the debt of are seen taken. + inbox.owed.swap(false, Ordering::AcqRel); + polls::deliver(|| inbox.with_state(|s| s.polls.take_owed()).ok().flatten(), inbox); let (count, dropped) = inbox.with_completions(|c| (c.completion_count(), c.dropped()))?; if count >= min_complete || min_complete == 0 { @@ -501,7 +491,10 @@ pub fn submit( 0, WaitClass::Io, deadline, - || inbox.with_completions(|c| c.completion_count()).map_or(true, |n| n >= min_complete), + || { + inbox.owed.load(Ordering::Acquire) + || inbox.with_completions(|c| c.completion_count()).map_or(true, |n| n >= min_complete) + }, ) .is_err() { @@ -564,9 +557,8 @@ fn process_submission(inbox: &Arc, submission: &Submission) { } } -/// Registers an `OP_WATCH`, or answers it immediately; every refusal posts a completion rather than going silent. +/// Takes an `OP_WATCH` as its handle's one poll; every refusal writes a completion rather than going silent. fn process_watch(inbox: &Arc, submission: &Submission) { - let handle = submission.handle; let user_data = submission.token; let flags = match WatchFlags::from_raw(submission.op_flags) { Ok(flags) => flags, @@ -575,80 +567,61 @@ fn process_watch(inbox: &Arc, submission: &Submission) { return; } }; - - // Readiness is checked on the process's table, not the thread's: a ring is process-wide. - // The object is cloned out so the registration below holds no process lock. - let resolved = process::with_process_data(|data| { - data.handles.get_ref(handle, Rights::WAIT).cloned() + // Nothing is held here: `resolve` has given the guard up. + let refused = resolve(submission.handle).map_err(HandleError::refuse_as_error).and_then(|object| { + arm(inbox, Poll::new(inbox.clone(), user_data, submission.handle, flags.raw()), &object) }); - let object = match resolved { - Ok(object) => object, - // Nothing is held here: `with_process_data` has given the guard up. - Err(e) => { - let refusal = e.refuse_as_error(); - inbox.complete(user_data, -(refusal as i32)); - return; - } - }; - - let readiness = readiness_of(&object, flags); - if readiness.readable || readiness.writable { - // Ready already: complete now, one-shot, with the directions that fired. - inbox.complete(user_data, readiness.result_flags() as i32); - return; + if let Err(refusal) = refused { + inbox.complete(user_data, -(refusal as i32)); } +} - let read = if flags.readable() { ops::read_watch(&object) } else { None }; - let write = if flags.writable() { ops::write_watch(&object) } else { None }; - // No readiness in either direction: nothing could ever complete this poll, so it is refused, not registered. - if read.is_none() && write.is_none() { - inbox.complete(user_data, -(SyscallError::NotSupported as i32)); - return; +/// The object `handle` names in this process's table, not the thread's: a ring is process-wide. +/// Cloned out, so what follows holds no process lock. +fn resolve(handle: RawHandle) -> Result { + process::with_process_data(|data| data.handles.get_ref(handle, Rights::WAIT).cloned()) +} + +/// Keep `poll` as its handle's one poll, and fire it if `object` is ready or +/// arm it on the object's watches if not; the refusal, when nothing could ever +/// answer it. +fn arm(inbox: &Arc, poll: Poll, object: &KObjectRef) -> Result<(), SyscallError> { + let flags = WatchFlags(poll.flags); + let ready = readiness_of(object, flags).any(); + let read = if flags.readable() { ops::read_watch(object) } else { None }; + let write = if flags.writable() { ops::write_watch(object) } else { None }; + // No readiness in either direction: nothing could ever answer this poll, so it is refused, not registered. + if !ready && read.is_none() && write.is_none() { + return Err(SyscallError::NotSupported); } - let poll = Arc::new(Poll { - inbox: inbox.clone(), - user_data, - handle, - state: Once::new(), - }); + let poll = Arc::new(poll); // The cap is checked before registering: registering first would leave a // watch holding a poll the ring never counted. - let admitted = inbox.with_state(|state| { - // The old poll on this handle answers nothing once this one replaces it. - if let Some(at) = state.pending.iter().position(|p| p.handle == handle && p.armed()) { - state.pending.swap_remove(at).withdraw(); - } - state.pending.retain(|p| p.armed()); - if state.pending.len() >= MAX_PENDING_WATCHES { - return false; - } - state.pending.push(poll.clone()); - true - }); - match admitted { + match inbox.with_state(|state| state.polls.admit(poll.clone(), MAX_PENDING_WATCHES)) { Ok(true) => {} - Ok(false) => { - inbox.complete(user_data, -(SyscallError::ResourceExhausted as i32)); - return; - } + Ok(false) => return Err(SyscallError::ResourceExhausted), // The ring's last handle closed under this submit. - Err(_) => return, + Err(_) => return Ok(()), + } + if ready { + poll.fire(0); + return Ok(()); } // Registered with no ring lock held, then rechecked: a post either ran // before the registration — and the recheck sees what it changed — or - // finds the entry. Whichever answers first answers alone. + // finds the entry. Whichever fires first fires alone. if let Some(watch) = &read { - watch.add_poll(PollEntry { poll: poll.clone(), direction: Readiness { readable: true, writable: false } }); + watch.add_poll(PollEntry { poll: poll.clone(), direction: WatchFlags::READABLE }); } if let Some(watch) = &write { - watch.add_poll(PollEntry { poll: poll.clone(), direction: Readiness { readable: false, writable: true } }); + watch.add_poll(PollEntry { poll: poll.clone(), direction: WatchFlags::WRITABLE }); } - let now = readiness_of(&object, flags); - if now.readable || now.writable { - poll.complete(now.result_flags() as i32); + if readiness_of(object, flags).any() { + poll.fire(0); } + Ok(()) } /// Per-direction readiness of the object, restricted to what was asked for. @@ -659,6 +632,43 @@ fn readiness_of(object: &KObjectRef, flags: WatchFlags) -> Readiness { } } +impl Submitter> for Arc { + fn room(&self) -> bool { + self.with_completions(Completions::room).unwrap_or(false) + } + + fn answer(&self, user_data: u64, result: i32) { + self.complete(user_data, result); + } + + fn look(&self, poll: &Poll) -> Look { + let object = match resolve(poll.handle) { + Ok(object) => object, + // Closed since it was watched, which is no bug of the process's: + // the poll is over, as a close that ends it says. + Err(HandleError::BadHandle | HandleError::Stale | HandleError::WrongType { .. }) => { + return Look::Refused(SyscallError::NotFound); + } + Err(HandleError::Rights { .. }) => return Look::Refused(SyscallError::PermissionDenied), + Err(HandleError::TableFull) => return Look::Refused(SyscallError::ResourceExhausted), + }; + let flags = WatchFlags(poll.flags); + let mut now = readiness_of(&object, flags); + // A post on its read watch is the object's readability, and nothing + // in the kernel could be looked at instead. + now.readable |= flags.readable() + && poll.posted() & WatchFlags::READABLE.raw() != 0 + && ops::read_posts_are_readiness(&object); + if now.any() { + return Look::Ready(now.result_flags()); + } + match arm(self, Poll::new(self.clone(), poll.user_data, poll.handle, poll.flags), &object) { + Ok(()) => Look::Armed, + Err(refusal) => Look::Refused(refusal), + } + } +} + /// The submission form of `SYS_ACCEPT`; refusals fold into one `-InvalidArgument` completion instead of ending the process. fn process_accept(inbox: &Inbox, submission: &Submission) { let user_data = submission.token; diff --git a/kernel/src/inbox/once.rs b/kernel/src/inbox/once.rs index 46097fb1ec6..7376f2bcbfa 100644 --- a/kernel/src/inbox/once.rs +++ b/kernel/src/inbox/once.rs @@ -1,6 +1,7 @@ //! The one decision a poll makes that two CPUs race for: which of a post, the -//! registrant's own recheck and a withdrawal answers it. Exactly one wins, so -//! a poll completes at most once and a withdrawn poll never does. +//! registrant's own recheck, the end of its source and a withdrawal takes it. +//! Exactly one wins, so a poll is owed at most one look and a withdrawn poll +//! none. //! //! Compiled a second time by `kernel-loom`, so it names only atomics. @@ -12,6 +13,7 @@ use loom::sync::atomic::{AtomicU8, Ordering}; const ARMED: u8 = 0; const FIRED: u8 = 1; const WITHDRAWN: u8 = 2; +const ENDED: u8 = 3; /// A poll's answer, taken once. pub struct Once(AtomicU8); @@ -34,6 +36,12 @@ impl Once { self.take(FIRED) } + /// `true` for the one caller that takes the poll because its source + /// ended, so the answer is that and not a look at the object. + pub fn end(&self) -> bool { + self.take(ENDED) + } + /// Take the poll back unanswered; `true` if it had not been answered. pub fn withdraw(&self) -> bool { self.take(WITHDRAWN) @@ -44,6 +52,11 @@ impl Once { self.0.load(Ordering::Acquire) == ARMED } + /// Whether the end of its source took the poll. Permanent once `true`. + pub fn ended(&self) -> bool { + self.0.load(Ordering::Acquire) == ENDED + } + // `poll-fire-load-store` is the negative control: the exchange split into // a load and a store lets two callers both answer, and `poll_once` reds. #[cfg(not(feature = "poll-fire-load-store"))] diff --git a/kernel/src/inbox/polls.rs b/kernel/src/inbox/polls.rs new file mode 100644 index 00000000000..da3686a9aa3 --- /dev/null +++ b/kernel/src/inbox/polls.rs @@ -0,0 +1,188 @@ +//! A ring's polls, and when one of them is answered. +//! +//! **A post is not an answer.** An object's post fires the polls on its watch, +//! and a fire only owes the poll a look ([`Wake::owe`]). The answer is written +//! by the ring's own submitter, in its `inbox_submit`, after it has looked at +//! the object again ([`deliver`]): an object ready at that look is answered with +//! what it holds then, and one that is not is armed again. So an answer says +//! what the object held when the wait that returned it looked — a peer's empty +//! write, and a post that lands after its bytes were read, answer nothing. +//! +//! **A handle has one poll that may answer.** A watch replaces the handle's +//! earlier poll whether a post has fired it or not ([`Polls::admit`]), so one +//! look answers a handle once, under the token of its newest watch. +//! +//! Compiled a second time by `kernel-loom`, so it names nothing of the kernel's. + +use alloc::sync::Arc; +use alloc::vec::Vec; + +#[cfg(not(feature = "loom"))] +use core::sync::atomic::{AtomicU32, Ordering}; +#[cfg(feature = "loom")] +use loom::sync::atomic::{AtomicU32, Ordering}; + +use toyos_abi::handle::RawHandle; +use toyos_abi::syscall::SyscallError; + +use super::once::Once; + +/// Who a fire tells. +pub trait Wake { + /// A poll is owed a look: wake whoever waits in this ring's `inbox_submit`. + fn owe(&self); +} + +/// One `OP_WATCH` a ring is waiting on: one-shot across every watch it is +/// registered on and against its own registrant's recheck. +pub struct Poll { + wake: W, + pub user_data: u64, + /// The handle the poll was submitted against; the key a watch replaces by. + pub handle: RawHandle, + /// The submission's interest, which the look asks about again. + pub flags: u32, + /// The directions whose watch posted it, for an object whose post is its + /// readiness. + posted: AtomicU32, + state: Once, +} + +impl Poll { + pub fn new(wake: W, user_data: u64, handle: RawHandle, flags: u32) -> Self { + Self { wake, user_data, handle, flags, posted: AtomicU32::new(0), state: Once::new() } + } + + /// The object may be ready: the watch of the directions in `posted` was + /// posted, or with `0` its registrant saw it ready. Owes a look, once. + pub fn fire(&self, posted: u32) { + // Before the exchange, so the look that the winning fire owes sees it. + self.posted.fetch_or(posted, Ordering::Release); + if self.state.fire() { + self.wake.owe(); + } + } + + /// The object's source ended: the poll is answered as gone, with no look. + pub fn end(&self) { + if self.state.end() { + self.wake.owe(); + } + } + + /// Answer nothing: a newer poll on the same handle replaced it, or its + /// ring went away. + pub fn withdraw(&self) { + let _ = self.state.withdraw(); + } + + pub fn armed(&self) -> bool { + self.state.armed() + } + + /// The directions whose watch posted this poll. + pub fn posted(&self) -> u32 { + self.posted.load(Ordering::Acquire) + } +} + +/// A ring's polls that may still answer: armed, or taken and not yet looked at. +pub struct Polls { + polls: Vec>>, +} + +impl Polls { + pub const fn new() -> Self { + Self { polls: Vec::new() } + } + + /// Keep `poll` as its handle's one poll, unless `cap` are kept already. + /// Every earlier poll on the handle answers nothing from here on, whether + /// a post has fired it or not. + pub fn admit(&mut self, poll: Arc>, cap: usize) -> bool { + self.polls.retain(|p| { + let other = p.handle != poll.handle; + if !other { + p.withdraw(); + } + other + }); + if self.polls.len() >= cap { + return false; + } + self.polls.push(poll); + true + } + + /// The oldest poll something has taken, given up for its look. + pub fn take_owed(&mut self) -> Option>> { + let at = self.polls.iter().position(|p| !p.armed())?; + Some(self.polls.remove(at)) + } + + /// The ring is going: no poll answers. + pub fn withdraw_all(&mut self) { + for poll in self.polls.drain(..) { + poll.withdraw(); + } + } +} + +impl Default for Polls { + fn default() -> Self { + Self::new() + } +} + +/// What a look at a poll's object found. +pub enum Look { + /// Ready in the directions this word names, which is the answer. + Ready(u32), + /// The handle names nothing that could answer any more; the refusal is + /// the answer. + Refused(SyscallError), + /// Not ready: a new poll on the handle is armed, and this one answers + /// nothing. + Armed, +} + +/// The submitter's side of [`deliver`]. +pub trait Submitter { + /// Whether the completion ring takes one more answer. + fn room(&self) -> bool; + fn answer(&self, user_data: u64, result: i32); + /// Look at the object `poll` watches, and arm a new poll on it if it is + /// not ready. + fn look(&self, poll: &Poll) -> Look; +} + +/// Answer every poll `take` gives up, oldest first, while the ring has room; a +/// poll left over is answered by a later look, never dropped. +pub fn deliver(mut take: impl FnMut() -> Option>>, ring: &impl Submitter) { + while ring.room() { + let Some(poll) = take() else { return }; + let result = if poll.state.ended() { + -(SyscallError::NotFound as i32) + } else { + match look(ring, &poll) { + Look::Ready(flags) => flags as i32, + Look::Refused(e) => -(e as i32), + Look::Armed => continue, + } + }; + ring.answer(poll.user_data, result); + } +} + +/// `post-is-an-answer` is the negative control: a fired poll is answered with +/// its interest and nobody looks, which is what a ring did before this file, +/// and `kernel-loom`'s `inbox_answer` reds. +#[cfg(not(feature = "post-is-an-answer"))] +fn look(ring: &impl Submitter, poll: &Poll) -> Look { + ring.look(poll) +} + +#[cfg(feature = "post-is-an-answer")] +fn look(_ring: &impl Submitter, poll: &Poll) -> Look { + Look::Ready(poll.flags) +} diff --git a/kernel/src/object/ops.rs b/kernel/src/object/ops.rs index d92b4f8abda..b85091afdbb 100644 --- a/kernel/src/object/ops.rs +++ b/kernel/src/object/ops.rs @@ -780,6 +780,21 @@ pub fn has_data(object: &KObjectRef) -> bool { } } +/// Whether a post on this object's read watch is its readability, which +/// [`has_data`] cannot be asked for: the log's, whose unread records are a +/// property of the reader's cursor, which the kernel does not hold; and a +/// console's, whose watch is the keyboard's while its data is the serial +/// line's (`issues/kernel/a-console-watch-waits-on-the-keyboard-not-the-serial-line.md`). +pub fn read_posts_are_readiness(object: &KObjectRef) -> bool { + match object { + KObjectRef::SysCap(_) | KObjectRef::Console(_) => true, + KObjectRef::PipeRead(_) | KObjectRef::PipeWrite(_) | KObjectRef::Connection(_) + | KObjectRef::Acceptor(_) | KObjectRef::File(_) | KObjectRef::Device(_) + | KObjectRef::Inbox(_) | KObjectRef::Connector(_) | KObjectRef::Namespace(_) + | KObjectRef::SharedMem(_) | KObjectRef::Process(_) => false, + } +} + pub fn has_space(object: &KObjectRef) -> bool { match object { KObjectRef::PipeWrite(w) => pipe::has_space(w.id()), diff --git a/kernel/src/watch.rs b/kernel/src/watch.rs index 268d4e115f8..b68dde850da 100644 --- a/kernel/src/watch.rs +++ b/kernel/src/watch.rs @@ -1,6 +1,6 @@ //! The one way to wait: every waitable object holds exactly one [`Watch`], and //! a waiter on it is either a thread, which a post *wakes*, or a user poll -//! ring's entry, which a post *completes*. +//! ring's entry, which a post *fires*. //! //! The protocol is `toyos_sched::watch`'s and `toyos_sched::park`'s, and its //! lost-wake argument is theirs: a thread registers before it reads its @@ -42,14 +42,14 @@ pub struct Waitable>(toyos_sched::watch::Watch>; -/// A watch an interrupt handler posts, and the watch of a ring such a post -/// completes into. It has no `post`, which frees: [`IrqWatch::post_in_place`] +/// A watch an interrupt handler posts, and the watch of a ring whose poll such +/// a post fires. It has no `post`, which frees: [`IrqWatch::post_in_place`] /// frees nothing. pub type IrqWatch = Waitable>; -/// What an interrupt handler's post takes: an [`IrqWatch`]'s list and a poll -/// ring's completions. Held with interrupts off, so a handler never finds one -/// held by the context it interrupted; nothing allocates or frees under it. +/// What an interrupt handler's post takes: an [`IrqWatch`]'s list. Held with +/// interrupts off, so a handler never finds one held by the context it +/// interrupted; nothing allocates or frees under it. pub struct IrqLock(masked::Masked); impl IrqLock { @@ -97,7 +97,7 @@ impl Watch { } /// Something about the object changed: wake every thread waiting on it and - /// complete every poll. + /// fire every poll. pub fn post(&self) { self.post_as(WakeCause::new(WakeReason::Woken)); } @@ -144,7 +144,7 @@ impl IrqWatch { } /// Something about the object changed: wake every thread waiting on it and - /// complete every poll where it stands, freeing nothing. A handler makes it + /// fire every poll where it stands, freeing nothing. A handler makes it /// with its CPU's preempt count raised, as `device_irq_entry` holds it, so /// this post's own never reaches zero, and a pass, inside the interrupt. pub fn post_in_place(&self) { @@ -291,7 +291,7 @@ pub fn wait_until>( /// handler before any pass can run. Raised inside a post of the watch itself, /// the handler's post follows once the outer one lets go; raised inside a /// completion written into a ring that polls the watch, or inside that ring's -/// own watch's list lock, the handler's post completes that poll once the +/// own watch's list lock, the handler's post fires that poll once the /// section lets go. A hold counts the posts of the watch made on its CPU, and /// no other watch's, and lapses at its budget. One run of [`HOLDS`] holds per /// arm, on whichever idle loop reaches it first with interrupts open; its diff --git a/src/ci.rs b/src/ci.rs index 5323f09f130..b01d675d408 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -258,6 +258,11 @@ pub(crate) const CONTROLS: &[Control] = &[ red(KERNEL_LOOM, "log-ring-loads-swapped", Some("log_ring"), &[ "a_published_record_is_whole_and_read_once ... FAILED", ]), + red(KERNEL_LOOM, "post-is-an-answer", Some("inbox_answer"), &[ + "a_post_with_nothing_to_read_answers_nothing ... FAILED", + "a_post_that_lands_after_its_bytes_were_read_answers_nothing ... FAILED", + "a_poll_armed_again_does_not_end_the_look ... FAILED", + ]), red(KERNEL_LOOM, "poll-fire-load-store", Some("poll_once"), &[ "a_post_and_a_recheck_answer_a_poll_once ... FAILED", "a_withdrawal_and_a_post_never_both_take_a_poll ... FAILED", diff --git a/toyos-abi/src/inbox.rs b/toyos-abi/src/inbox.rs index c9f85adf054..c454cef8a50 100644 --- a/toyos-abi/src/inbox.rs +++ b/toyos-abi/src/inbox.rs @@ -10,6 +10,15 @@ use crate::RawHandle; pub const OP_NOP: u8 = 0; +/// Answered once, when the handle is ready. +/// +/// **An answer is what the object held when the waiting `inbox_submit` looked +/// at it.** A post only owes the watch a look, so neither a post with nothing +/// behind it nor one for bytes already read is answered, and a handle no other +/// reader drains holds what its answer says. The exceptions are the objects +/// whose readiness the kernel does not hold — the log, read on the reader's own +/// cursor, and a console — whose read post is the answer. A watch replaces its +/// handle's earlier one, fired or not, so one look answers a handle once. pub const OP_WATCH: u8 = 1; // Op code 2 unused (formerly IORING_OP_POLL_REMOVE): a watch this kernel takes // is one-shot, consumed by the completion it posts, so the interest a remove @@ -78,12 +87,8 @@ pub struct RingHeader { /// Completions the kernel could not post because the completion ring /// reported itself full. Cumulative, and never cleared. /// - /// The 2x sizing makes this unreachable only for a process that keeps its - /// registrations within the depth it asked for: over-registering flushes a - /// full submission ring mid-registration, and the kernel then posts - /// completions for the handles already ready while the caller is still - /// registering the rest. `toyos`'s `Poller` sizes its rings so that cannot - /// happen and reads this on every wait. + /// `toyos`'s `Poller` sizes its rings so that cannot happen and reads this + /// on every wait. pub dropped: core::sync::atomic::AtomicU32, } diff --git a/toyos-sched/src/watch.rs b/toyos-sched/src/watch.rs index 598c14b1d07..5ec8d01f420 100644 --- a/toyos-sched/src/watch.rs +++ b/toyos-sched/src/watch.rs @@ -6,7 +6,7 @@ //! claims its word if it is parked or committing and flags it otherwise, so its //! own next commit rechecks instead of parking. A **ring** entry is one poll a //! process submitted and is *posted*: a post hands it to [`Ring::fire`], once, -//! and the environment writes that poll's completion into the ring it names. +//! and the environment owes the ring it names a look at that poll's object. //! //! **A lost wake has no expression here.** A thread registers before it reads //! its condition, under the list lock a post also takes, so a post either @@ -54,14 +54,14 @@ pub enum Fire { /// One poll a ring is waiting on, as the watch holds it. pub trait Ring { - /// Post this poll's completion. One-shot across every watch the poll is + /// Fire this poll for its ring. One-shot across every watch the poll is /// registered on: an entry that already fired, or whose poll was withdrawn, - /// posts nothing. Called with at most the posting watch's list lock held, + /// does nothing. Called with at most the posting watch's list lock held, /// and from an interrupt handler by a post in place: may take only its /// ring's own lock and post only the watch its ring's submitters park on, /// and allocates and frees nothing. fn fire(&self, how: Fire); - /// Whether a fire would still post anything. `false` is permanent. + /// Whether a fire would still do anything. `false` is permanent. fn live(&self) -> bool; } diff --git a/toyos/src/poller.rs b/toyos/src/poller.rs index d3a44238462..cc8ac259b2f 100644 --- a/toyos/src/poller.rs +++ b/toyos/src/poller.rs @@ -1,9 +1,8 @@ //! Event-driven I/O polling on an [inbox](toyos_abi::inbox). -use core::cell::RefCell; use core::sync::atomic::{AtomicU32, Ordering}; use toyos_abi::RawHandle; -use toyos_abi::{clock, syscall}; +use toyos_abi::syscall; use toyos_abi::inbox::{ Submission, Completion, RingHeader, RingLayout, OP_WATCH, SUBMISSION_RING_OFF, COMPLETION_RING_OFF, SUBMISSIONS_OFF, @@ -167,77 +166,6 @@ impl Rings { } } -/// The registrations that can still answer, at most one per handle. -/// -/// **What the kernel hands back is the registration's number, never the -/// caller's token.** Watching a handle again replaces its registration — the -/// key `process_watch` withdraws an armed poll by — but an answer the replaced -/// one has posted stays in the ring, and so does one it posts after a -/// replacement that answered at once. Under the caller's token either reads as -/// news about the handle, of bytes already read or already announced. Under a -/// number, an answer that is not its handle's latest registration's names -/// nothing here and is dropped. -struct Registry { - live: [Registration; MAX_LIVE], - len: usize, - /// Twice the poller's capacity. After a wait a live registration is on a - /// handle still open — a close answers the poll on it, and the wait handed - /// that out, unless `ops::close_ends_polls` says the close ends nothing — - /// so at most the declared set; a round adds at most the declared set again. - limit: usize, - next: u64, -} - -#[derive(Clone, Copy)] -struct Registration { - handle: RawHandle, - token: u64, - number: u64, -} - -const VACANT: Registration = Registration { handle: RawHandle(0), token: 0, number: 0 }; - -/// The widest registry, for a poller of [`Poller::MAX_HANDLES`]. -const MAX_LIVE: usize = 2 * Poller::MAX_HANDLES as usize; - -impl Registry { - fn new(limit: usize) -> Self { - Self { live: [VACANT; MAX_LIVE], len: 0, limit, next: 0 } - } - - /// The number a registration of `handle` is submitted under; whatever the - /// handle's earlier registration posts answers nothing from here on. - fn register(&mut self, handle: RawHandle, token: u64) -> u64 { - let registration = Registration { handle, token, number: self.next }; - self.next += 1; - match self.live[..self.len].iter_mut().find(|r| r.handle == handle) { - Some(replaced) => *replaced = registration, - None => { - assert!( - self.len < self.limit, - "Poller: {} handles hold a registration that has not answered, the most \ - a poller of capacity {} keeps: it watches past its declared set", - self.len, - self.limit / 2, - ); - self.live[self.len] = registration; - self.len += 1; - } - } - registration.number - } - - /// The caller's token for the answer posted under `number`, which ends its - /// registration, or `None` for one a later registration replaced. - fn answer(&mut self, number: u64) -> Option { - let at = self.live[..self.len].iter().position(|r| r.number == number)?; - let token = self.live[at].token; - self.len -= 1; - self.live[at] = self.live[self.len]; - Some(token) - } -} - /// An inbox, for watching handles for readiness. /// /// Owns the inbox handle and shared memory mapping. Submissions are batched @@ -251,9 +179,7 @@ impl Registry { /// and sizes both rings from it: the submission ring holds them all, so no /// batch is ever flushed mid-registration, and the kernel's completion ring — /// always twice the submission ring — holds the most completions that can exist -/// between two [`wait`](Self::wait) calls, which is two per watched handle (a -/// registration left over from the previous round firing, and this round's -/// registration finding the handle ready). +/// between two [`wait`](Self::wait) calls. /// /// Going past the capacity is a contract violation and panics, because it is /// the caller's own bug and the alternative is the failure this replaced: the @@ -265,28 +191,16 @@ impl Registry { /// [`wait`](Self::wait) reads the kernel's drop counter on every call — an /// assert that should be unreachable, kept because that is the shape a /// fail-fast check is supposed to have. -/// -/// **A token [`wait`](Self::wait) hands out is the answer of its handle's -/// latest registration, and a registration answers once.** Watching a handle -/// replaces its earlier registration, and whatever that one posts, before the -/// replacement or after it, is dropped (`Registry`). So a caller that watches -/// a handle before every wait is never told twice of one arrival, nor of bytes -/// it read before that watch: what it is told of is there to read. A -/// registration left standing across waits answers whenever its handle turns -/// ready, so a caller that reads such a handle untold watches it again before -/// it waits. pub struct Poller { inbox: RawHandle, rings: Rings, capacity: u32, - registry: RefCell, } // Safety: the base pointer is process-local shared memory mapped from the // kernel. It is only ever reached through `Rings`, which takes no reference // over it: atomics for the shared words, whole-value volatile copies for -// everything else. Not `Sync`: a watch moves the submission tail in two steps, -// and the registry is a `RefCell`. +// everything else. Not `Sync`: a watch moves the submission tail in two steps. unsafe impl Send for Poller {} impl Poller { @@ -328,12 +242,7 @@ impl Poller { rings.submission_ring_size, rings.completion_ring_size, ); - Self::over(inbox, rings, capacity) - } - - fn over(inbox: RawHandle, rings: Rings, capacity: u32) -> Self { - let registry = RefCell::new(Registry::new(2 * capacity as usize)); - Self { inbox, rings, capacity, registry } + Self { inbox, rings, capacity } } /// Watch the given handle for readiness. @@ -360,7 +269,6 @@ impl Poller { self.pending(), self.capacity, ); - let number = self.registry.borrow_mut().register(handle, token); let tail = self.rings.submission_tail().load(Ordering::Acquire); let idx = tail & (self.rings.submission_ring_size - 1); self.rings.write_submission( @@ -369,7 +277,7 @@ impl Poller { op: OP_WATCH, handle, op_flags: flags, - token: number, + token, ..Submission::default() }, ); @@ -393,59 +301,21 @@ impl Poller { .expect("Poller::submit: inbox_submit rejected the batch"); } - /// Submit pending entries and hand `f` the token of every answer, until at - /// least `min_complete` have been handed out or `timeout_nanos` has passed - /// — `0` looks once, `u64::MAX` never passes. - pub fn wait(&self, min_complete: u32, timeout_nanos: u64, mut f: impl FnMut(u64)) { - self.wait_on( - min_complete, - timeout_nanos, - &mut f, - |min, nanos| self.submit(min, nanos), - clock::nanos_since_boot, - ); - } - - /// [`wait`](Self::wait) over the kernel's half and a clock it is handed, - /// so a host test can hand it fakes. + /// Submit pending entries and wait for completions. /// - /// The kernel counts a dropped answer towards `min_complete` and returns - /// for it, so the wait goes on, for what is left of its time, until the - /// answers handed out make up the count. - fn wait_on( - &self, - min_complete: u32, - timeout_nanos: u64, - f: &mut impl FnMut(u64), - mut submit: impl FnMut(u32, u64), - now: impl Fn() -> u64, - ) { - let deadline = now().saturating_add(timeout_nanos); - let mut nanos = timeout_nanos; - let mut handed = 0; - loop { - submit(min_complete - handed, nanos); - handed += self.drain(f); - if handed >= min_complete { - return; - } - nanos = match timeout_nanos { - 0 => return, - u64::MAX => u64::MAX, - _ => match deadline.checked_sub(now()) { - Some(left) if left > 0 => left, - _ => return, - }, - }; - } + /// Blocks until at least `min_complete` completions are ready or `timeout_nanos` + /// elapses. Calls `f` for each completed token: a handle that was ready when + /// the kernel looked, inside this wait ([`OP_WATCH`]). + pub fn wait(&self, min_complete: u32, timeout_nanos: u64, mut f: impl FnMut(u64)) { + self.submit(min_complete, timeout_nanos); + self.drain(&mut f); } - /// Read every completion the kernel has published, oldest first, and hand - /// `f` the token of each that answers a live registration; how many did. + /// Read every completion the kernel has published, oldest first. /// /// Split from [`wait`](Self::wait) because it is the half that is a pure /// function of the page: a host test can hand it a fake one. - fn drain(&self, f: &mut impl FnMut(u64)) -> u32 { + fn drain(&self, f: &mut impl FnMut(u64)) { // Unreachable, and kept for that reason: `capacity` bounds the // registrations and the rings are sized from `capacity`, so nothing a // conforming caller does can make the kernel drop a completion here. @@ -460,16 +330,14 @@ impl Poller { self.capacity, self.rings.submission_ring_size, self.rings.completion_ring_size, ); - let mut handed = 0; loop { let head = self.rings.completion_head().load(Ordering::Acquire); let tail = self.rings.completion_tail().load(Ordering::Acquire); if head == tail { - return handed; + break; } let idx = head & (self.rings.completion_ring_size - 1); let completion = self.rings.completion_at(idx); - self.rings.completion_head().store(head.wrapping_add(1), Ordering::Release); // Do not filter on `completion.result`. A negative result is the // kernel saying the registration is over and will never fire // (a watched handle's close answers every poll on a watch it ends @@ -477,11 +345,8 @@ impl Poller { // to that exactly as to readiness — by looking at the handle again. // A zero result is meaningful too: `OP_ACCEPT` reports handle 0 // that way. - let Some(token) = self.registry.borrow_mut().answer(completion.token) else { - continue; - }; - f(token); - handed += 1; + f(completion.token); + self.rings.completion_head().store(head.wrapping_add(1), Ordering::Release); } } } @@ -495,8 +360,6 @@ impl Drop for Poller { #[cfg(test)] mod tests { use super::*; - use core::cell::Cell; - use core::mem::ManuallyDrop; use toyos_abi::inbox::{ RING_DROPPED_OFF, RING_HEAD_OFF, RING_SIZE_OFF, RING_TAIL_OFF, }; @@ -569,109 +432,6 @@ mod tests { .write(entry); } } - - /// The submission in slot `index`, copied out as `submission_at` does. - fn submission(&mut self, index: u32) -> Submission { - let base = self.base(); - // SAFETY: `index` is under the submission ring size, so the slot - // is inside `PAGE_BYTES`; `SUBMISSIONS_OFF` is page-aligned and - // `Submission` is 40 bytes, 8-aligned. - unsafe { - (base.add(SUBMISSIONS_OFF as usize + index as usize * core::mem::size_of::()) - as *const Submission) - .read_volatile() - } - } - } - - /// The kernel's half of a watch, as `process_watch` and an object's post - /// do it, over a [`FakePage`]: a submission on a handle already ready is - /// answered at once and leaves any earlier poll on it armed; one on a - /// handle not ready withdraws the earlier poll and arms; a post answers - /// every poll armed on its handle. Every answer carries its submission's - /// token, as the kernel's do. - struct FakeKernel { - page: FakePage, - ready: Vec, - armed: Vec<(RawHandle, u64)>, - } - - /// A poller of `capacity` and the kernel behind its page. The poller is - /// never dropped: its `Drop` is a syscall. - fn pair(capacity: u32) -> (FakeKernel, ManuallyDrop) { - let entries = capacity.next_power_of_two(); - let mut page = FakePage::new(entries, 2 * entries); - let poller = ManuallyDrop::new(Poller::over(RawHandle(0), rings(&mut page), capacity)); - (FakeKernel { page, ready: Vec::new(), armed: Vec::new() }, poller) - } - - impl FakeKernel { - /// `inbox_submit`'s first half: every queued submission, registered. - fn submit(&mut self) { - let head_at = SUBMISSION_RING_OFF as usize + RING_HEAD_OFF; - let size = self.page.get(SUBMISSION_RING_OFF as usize + RING_SIZE_OFF); - loop { - let head = self.page.get(head_at); - if head == self.page.get(SUBMISSION_RING_OFF as usize + RING_TAIL_OFF) { - return; - } - let s = self.page.submission(head & (size - 1)); - self.page.put(head_at, head.wrapping_add(1)); - if self.ready.contains(&s.handle) { - self.answer(s.token); - } else { - self.armed.retain(|&(h, _)| h != s.handle); - self.armed.push((s.handle, s.token)); - } - } - } - - /// Bytes reach `handle`, and its post has not run yet. - fn fill(&mut self, handle: RawHandle) { - self.ready.push(handle); - } - - /// `handle`'s post: every poll armed on it answers. - fn post(&mut self, handle: RawHandle) { - let (fired, armed): (Vec<_>, Vec<_>) = - core::mem::take(&mut self.armed).into_iter().partition(|&(h, _)| h == handle); - self.armed = armed; - for (_, token) in fired { - self.answer(token); - } - } - - /// Bytes reach `handle` and its post runs. - fn arrive(&mut self, handle: RawHandle) { - self.fill(handle); - self.post(handle); - } - - /// `handle`'s bytes are read by a call that did not ask the poller. - fn take(&mut self, handle: RawHandle) { - self.ready.retain(|&h| h != handle); - } - - /// Answers posted and not yet drained. - fn posted(&mut self) -> u32 { - let head = self.page.get(COMPLETION_RING_OFF as usize + RING_HEAD_OFF); - self.page.get(COMPLETION_RING_OFF as usize + RING_TAIL_OFF).wrapping_sub(head) - } - - fn answer(&mut self, token: u64) { - let tail_at = COMPLETION_RING_OFF as usize + RING_TAIL_OFF; - let tail = self.page.get(tail_at); - let size = self.page.get(COMPLETION_RING_OFF as usize + RING_SIZE_OFF); - self.page.post(tail & (size - 1), Completion { token, result: READABLE as i32, flags: 0 }); - self.page.put(tail_at, tail.wrapping_add(1)); - } - } - - /// Every token one drain hands out. - fn drained(poller: &Poller) -> Vec { - let mut seen = Vec::new(); - poller.drain(&mut |token| seen.push(token)); - seen } fn rings(page: &mut FakePage) -> Rings { @@ -797,160 +557,4 @@ mod tests { let r = rings(&mut page); assert_eq!(r.pending(), 3); } - - const H: RawHandle = RawHandle(5); - const G: RawHandle = RawHandle(6); - - /// A registration a wait did not see answer, answered by bytes a call that - /// did not ask the poller read; the next watch finds the handle empty. The - /// answer still in the ring announces bytes that are gone, and a reader - /// that took it for news would block in its read. - #[test] - fn an_answer_for_bytes_already_read_is_not_handed_out() { - let (mut kernel, poller) = pair(1); - poller.watch_raw(H, READABLE, 7); - kernel.submit(); - assert_eq!(drained(&poller), [0u64; 0]); - kernel.arrive(H); - kernel.take(H); - poller.watch_raw(H, READABLE, 7); - kernel.submit(); - assert_eq!(drained(&poller), [0u64; 0]); - kernel.arrive(H); - assert_eq!(drained(&poller), [7]); - } - - /// A watch that finds its handle ready is answered at once and leaves the - /// poll it replaced armed, which answers again when the post that made the - /// handle ready runs. - #[test] - fn an_answer_the_replaced_registration_posts_late_is_not_handed_out() { - let (mut kernel, poller) = pair(1); - poller.watch_raw(H, READABLE, 1); - kernel.submit(); - kernel.fill(H); - poller.watch_raw(H, READABLE, 2); - kernel.submit(); - kernel.post(H); - assert_eq!(drained(&poller), [2]); - } - - /// epoll(7): "Does an operation on a file descriptor affect the already - /// collected but not yet reported events? … Modify will reread available - /// I/O." An answer collected before its handle is watched again is - /// reported once, under the new watch's token. - #[test] - fn a_handle_watched_again_is_answered_once_under_its_new_token() { - let (mut kernel, poller) = pair(1); - poller.watch_raw(H, READABLE, 1); - kernel.submit(); - kernel.arrive(H); - poller.watch_raw(H, READABLE, 2); - kernel.submit(); - assert_eq!(drained(&poller), [2]); - } - - /// A registration the caller does not renew is not replaced: it answers in - /// whichever later wait its handle turns ready. - #[test] - fn a_registration_left_standing_still_answers() { - let (mut kernel, poller) = pair(2); - poller.watch_raw(H, READABLE, 3); - poller.watch_raw(G, READABLE, 4); - kernel.submit(); - assert_eq!(drained(&poller), [0u64; 0]); - poller.watch_raw(G, READABLE, 4); - kernel.submit(); - kernel.arrive(H); - assert_eq!(drained(&poller), [3]); - } - - /// The kernel leaves a wait for an answer the poller then drops, as - /// `an_answer_for_bytes_already_read_is_not_handed_out` sets up, and the - /// wait goes on to the live one rather than returning with none. - #[test] - fn a_wait_sleeps_past_a_dropped_answer_to_the_live_one() { - let (mut kernel, poller) = pair(1); - poller.watch_raw(H, READABLE, 7); - kernel.submit(); - kernel.arrive(H); - kernel.take(H); - poller.watch_raw(H, READABLE, 7); - let mut submits = 0; - let mut seen = Vec::new(); - let submit = |_, _| { - kernel.submit(); - submits += 1; - if submits == 2 { - kernel.arrive(H); - } - }; - poller.wait_on(1, u64::MAX, &mut |token| seen.push(token), submit, || 0); - assert_eq!((seen, submits), (vec![7], 2)); - } - - /// A wait woken only by answers it drops ends at its deadline, and sleeps - /// only what is left of it. - #[test] - fn a_wait_woken_only_by_dropped_answers_ends_at_its_deadline() { - let (mut kernel, poller) = pair(1); - poller.watch_raw(H, READABLE, 7); - kernel.submit(); - kernel.arrive(H); - kernel.take(H); - poller.watch_raw(H, READABLE, 7); - let clock = Cell::new(100); - let mut slept = Vec::new(); - let mut seen = Vec::new(); - let submit = |min, nanos| { - kernel.submit(); - slept.push(nanos); - // Back 300 ns later for what is posted, and at its deadline for nothing. - clock.set(clock.get() + if kernel.posted() >= min { 300 } else { nanos }); - }; - poller.wait_on(1, 1_000, &mut |token| seen.push(token), submit, || clock.get()); - assert_eq!((seen, slept), (vec![], vec![1_000, 700])); - } - - /// A wait of zero looks once, whatever it drops. - #[test] - fn a_wait_of_zero_looks_once() { - let (mut kernel, poller) = pair(1); - poller.watch_raw(H, READABLE, 7); - kernel.submit(); - kernel.arrive(H); - kernel.take(H); - poller.watch_raw(H, READABLE, 7); - let mut submits = 0; - let submit = |_, _| { - kernel.submit(); - submits += 1; - }; - poller.wait_on(1, 0, &mut |token| panic!("handed out {token}"), submit, || 0); - assert_eq!(submits, 1); - } - - /// A registration that answered holds no place, so a poller that watches - /// one handle after another for its whole life never reaches its bound. - #[test] - fn a_registration_that_answered_holds_no_place() { - let (mut kernel, poller) = pair(1); - for handle in 1..=3 { - poller.watch_raw(RawHandle(handle), READABLE, u64::from(handle)); - kernel.submit(); - kernel.arrive(RawHandle(handle)); - assert_eq!(drained(&poller), [u64::from(handle)]); - } - } - - /// Past twice the declared set, a registration is refused by name. - #[test] - #[should_panic(expected = "watches past its declared set")] - fn a_registration_past_twice_the_capacity_panics() { - let (mut kernel, poller) = pair(1); - for handle in 1..=3 { - poller.watch_raw(RawHandle(handle), READABLE, 0); - kernel.submit(); - } - } } diff --git a/userland/fsd/src/main.rs b/userland/fsd/src/main.rs index 14c93f03dba..6b488848672 100644 --- a/userland/fsd/src/main.rs +++ b/userland/fsd/src/main.rs @@ -515,8 +515,6 @@ impl Server { } } - /// Take the connection `cap`'s port answered ready for: this process is - /// the port's one acceptor, so it is still queued. fn accept(&mut self, cap: usize) { let conn = match self.caps[cap].acceptor.accept() { Ok(conn) => conn, diff --git a/userland/libc/src/posix_io.rs b/userland/libc/src/posix_io.rs index 3d1bfb27748..aaffc17f21b 100644 --- a/userland/libc/src/posix_io.rs +++ b/userland/libc/src/posix_io.rs @@ -556,13 +556,19 @@ pub unsafe extern "C" fn poll(fds: *mut pollfd, nfds: u32, timeout: i32) -> i32 let timeout_ns = if timeout < 0 { None } else { Some(timeout as u64 * 1_000_000) }; let n = nfds as usize; - let poller = toyos::poller::Poller::new(n as u32); + // One watch per fd, under the first entry naming it and for every entry's + // interest: a watch replaces its handle's earlier one, so a second watch + // on the fd would leave the first entry unanswered. + let first = |i: usize| (0..i).find(|&j| (*fds.add(j)).fd == (*fds.add(i)).fd).unwrap_or(i); + let mut interest = alloc::vec![0u32; n]; for i in 0..n { - let pfd = &*fds.add(i); - let mut flags = 0u32; - if pfd.events & POLLIN != 0 { flags |= toyos::poller::READABLE; } - if pfd.events & POLLOUT != 0 { flags |= toyos::poller::WRITABLE; } - poller.watch_raw(toyos_abi::RawHandle(pfd.fd as u32), flags, i as u64); + let events = (*fds.add(i)).events; + if events & POLLIN != 0 { interest[first(i)] |= toyos::poller::READABLE; } + if events & POLLOUT != 0 { interest[first(i)] |= toyos::poller::WRITABLE; } + } + let poller = toyos::poller::Poller::new(n as u32); + for i in (0..n).filter(|&i| first(i) == i) { + poller.watch_raw(toyos_abi::RawHandle((*fds.add(i)).fd as u32), interest[i], i as u64); } let mut ready_set = alloc::vec![false; n]; @@ -571,9 +577,10 @@ pub unsafe extern "C" fn poll(fds: *mut pollfd, nfds: u32, timeout: i32) -> i32 }); let mut ready = 0i32; for i in 0..n { + let answered = ready_set[first(i)]; let pfd = &mut *fds.add(i); pfd.revents = 0; - if ready_set[i] { + if answered { pfd.revents = pfd.events; ready += 1; } From c3e104a9586e6bbb8eb8d8445a97238d60615fbf Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 09:59:07 +0200 Subject: [PATCH 09/14] issues: one Finder issue for both build steps; a ring's IRQ lock no handler takes The C++ runtime's scratch removal and the store sweep fail on the same host writer, a Finder .DS_Store in a directory the build takes as wholly its own, so they are one issue under a slug that names the class. A ring's completions keep their IrqLock although no interrupt handler writes a completion now that a post only owes a look; recorded with its exit. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...le-in-a-build-directory-fails-the-build.md | 27 ++++++++++++------- ...are-behind-an-irq-lock-no-handler-takes.md | 20 ++++++++++++++ 2 files changed, 38 insertions(+), 9 deletions(-) create mode 100644 issues/design-debt/a-rings-completions-are-behind-an-irq-lock-no-handler-takes.md diff --git a/issues/build/a-finder-file-in-a-build-directory-fails-the-build.md b/issues/build/a-finder-file-in-a-build-directory-fails-the-build.md index 3ee324dca57..e311978077d 100644 --- a/issues/build/a-finder-file-in-a-build-directory-fails-the-build.md +++ b/issues/build/a-finder-file-in-a-build-directory-fails-the-build.md @@ -1,16 +1,25 @@ --- status: open -kind: defect +kind: tooling opened: 2026-09-30 --- -# A Finder file in a store directory panics its sweep +# A Finder file in a build directory fails the build -`keystore::sweep_by` takes every entry of a store directory as `` or -`.`, so a `.DS_Store` the macOS Finder writes there names the key -`""`, and `buildlock::keyed_idle` then opens the lock directory itself: -`build lock: open /.git/toyos-build-locks/llvm/: Is a directory (os -error 21)`. It happened after an LLVM placement, whose product was whole, so -the next build went on; the sweep had removed nothing. +The macOS Finder writes a `.DS_Store` into a directory the build takes as +wholly its own, and the step that reads or removes it refuses: -**Exit**: a sweep passes over a store entry that names no key, with a test. +- `keystore::sweep_by` takes every entry of a store directory as `` or + `.`, so the Finder's file names the key `""`, and + `buildlock::keyed_idle` then opens the lock directory itself: `build lock: + open /.git/toyos-build-locks/llvm/: Is a directory (os error 21)`. + It happened after an LLVM placement, whose product was whole, so the next + build went on; the sweep had removed nothing. +- `libcxx::build` ends with `fs::remove_dir_all(scratch)`, and the Finder + wrote into that scratch while the removal ran: `remove + /rust/build/sysroots/acd58e51940b13be.libcxx-x86_64: Directory not + empty (os error 66)` at `src/libcxx.rs:95`, after the runtime had installed. + The `cargo run -- --build-only` that hit it exited 101. + +**Exit**: a host writer's file in a build directory fails neither a sweep nor +a scratch removal whose product is whole, each with a test. diff --git a/issues/design-debt/a-rings-completions-are-behind-an-irq-lock-no-handler-takes.md b/issues/design-debt/a-rings-completions-are-behind-an-irq-lock-no-handler-takes.md new file mode 100644 index 00000000000..35166bbb4d5 --- /dev/null +++ b/issues/design-debt/a-rings-completions-are-behind-an-irq-lock-no-handler-takes.md @@ -0,0 +1,20 @@ +--- +status: open +kind: defect +opened: 2026-10-01 +--- + +# A ring's completions are behind an IRQ lock no handler takes + +`kernel/src/inbox/mod.rs` keeps what a completion writes, and the ring's page, +behind an `IrqLock`, which masks interrupts while it is held. That was for a +device's interrupt handler, whose post wrote a poll's completion in place. +Since a post only owes the poll a look (`kernel/src/inbox/polls.rs`), every +completion is written by the ring's own submitter or by the +`handler-post` actuator's `Staged` ring, both in thread context, and no +handler reaches the lock. Each completion written still masks interrupts for +the write, and `handler-post`'s middle arm (`in_a_ring`) stages a handler +inside a section no handler's post enters any more. + +**Exit**: the completions sit behind a lock that leaves interrupts open, and +`handler-post` stages only sections a handler's post can reach. From 67881a65479af4b90a3deeee0a7c64802421d02c Mon Sep 17 00:00:00 2001 From: japabu Date: Thu, 1 Oct 2026 10:09:18 +0200 Subject: [PATCH 10/14] inbox: the host gates know a poll can end, and the control is declared --ci host was red on two steps at 7d31acac4: - the build system's the_kernel_declares_only_the_builds_that_earned_one did not list post-is-an-answer, which the kernel declares only so cfg checking knows the name; - clippy: toyos-sched/loom's loom_watch compiles inbox/once.rs and took neither end nor ended, and inbox_answer's fake ring held an Rc, so an Arc of its poll was neither Send nor Sync. loom_watch's two ring entries now do what the kernel's PollEntry does, firing the poll on readiness and ending it on Gone, and the end-racing models assert that the one-shot names which of the two took it. The model of a ring whose fire takes the ring's lock no longer claims to be the kernel's, and the issue that records that lock names it. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_016t9wjdQkB8SH7bmfUoiy6L --- ...are-behind-an-irq-lock-no-handler-takes.md | 8 +++-- kernel-loom/tests/inbox_answer.rs | 6 ++-- src/build.rs | 4 +++ toyos-sched/loom/tests/loom_watch.rs | 36 ++++++++++++++----- 4 files changed, 39 insertions(+), 15 deletions(-) diff --git a/issues/design-debt/a-rings-completions-are-behind-an-irq-lock-no-handler-takes.md b/issues/design-debt/a-rings-completions-are-behind-an-irq-lock-no-handler-takes.md index 35166bbb4d5..5d29cfdc2ac 100644 --- a/issues/design-debt/a-rings-completions-are-behind-an-irq-lock-no-handler-takes.md +++ b/issues/design-debt/a-rings-completions-are-behind-an-irq-lock-no-handler-takes.md @@ -13,8 +13,10 @@ Since a post only owes the poll a look (`kernel/src/inbox/polls.rs`), every completion is written by the ring's own submitter or by the `handler-post` actuator's `Staged` ring, both in thread context, and no handler reaches the lock. Each completion written still masks interrupts for -the write, and `handler-post`'s middle arm (`in_a_ring`) stages a handler -inside a section no handler's post enters any more. +the write; `handler-post`'s middle arm (`in_a_ring`) stages a handler inside +a section no handler's post enters any more; and `toyos-sched/loom`'s +`two_posts_through_one_rings_lock_lose_no_wake` models a fire that takes a +ring's lock, which no fire does. **Exit**: the completions sit behind a lock that leaves interrupts open, and -`handler-post` stages only sections a handler's post can reach. +`handler-post` and the loom models stage only nestings a post can reach. diff --git a/kernel-loom/tests/inbox_answer.rs b/kernel-loom/tests/inbox_answer.rs index f28378fe959..893ef73d50e 100644 --- a/kernel-loom/tests/inbox_answer.rs +++ b/kernel-loom/tests/inbox_answer.rs @@ -24,7 +24,7 @@ #![cfg(feature = "loom")] use std::cell::{Cell, RefCell}; -use std::rc::Rc; +use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::Arc; use kernel_loom::inbox_polls::{deliver, Look, Poll, Polls, Submitter, Wake}; @@ -44,11 +44,11 @@ const NOTHING: [(u64, i32); 0] = []; /// What a fire tells: a count, standing in for the ring's waiter. #[derive(Clone, Default)] -struct Owed(Rc>); +struct Owed(Arc); impl Wake for Owed { fn owe(&self) { - self.0.set(self.0.get() + 1); + self.0.fetch_add(1, Ordering::Relaxed); } } diff --git a/src/build.rs b/src/build.rs index f8932e7a13d..f6219544d44 100644 --- a/src/build.rs +++ b/src/build.rs @@ -2677,6 +2677,10 @@ mod tests { // turned on only by `kernel-loom`, to split `inbox/once.rs`'s // exchange and prove `poll_once` reds without it. "poll-fire-load-store", + // Costs no kernel build: turned on only by `kernel-loom`, so + // `inbox/polls.rs` answers a fired poll without a look and + // `inbox_answer` reds. + "post-is-an-answer", "reap-raise-relaxed", // `smp_roster.rs`'s count relaxed; `smp_bringup.rs` reds. "roster-commit-relaxed", diff --git a/toyos-sched/loom/tests/loom_watch.rs b/toyos-sched/loom/tests/loom_watch.rs index 6c57e61b7f0..874df303b47 100644 --- a/toyos-sched/loom/tests/loom_watch.rs +++ b/toyos-sched/loom/tests/loom_watch.rs @@ -31,9 +31,9 @@ //! //! **The ring entry is the kernel's [`Once`], compiled from //! `kernel/src/inbox/once.rs`**, the decision a `PollEntry` makes; what else a -//! `PollEntry` is — the ring's page, its lock, the completion it writes — names -//! half the kernel and cannot be compiled here, so the model's entry counts its -//! answers instead of writing them. +//! `PollEntry` is — the ring's page, its lock, the look it owes — names half the +//! kernel and cannot be compiled here, so the model's entry counts its answers +//! instead. //! //! [`TaskShared::notify`]: toyos_sched_loom::task::TaskShared::notify //! [`Gate`]: toyos_sched_loom::watch::Gate @@ -64,6 +64,8 @@ mod once; struct Poll { state: once::Once, posts: AtomicU32, + /// Whether the end of its source is what took it. + ended: AtomicBool, } #[derive(Clone)] @@ -74,6 +76,7 @@ impl Entry { Self(Arc::new(Poll { state: once::Once::new(), posts: AtomicU32::new(0), + ended: AtomicBool::new(false), })) } @@ -83,8 +86,14 @@ impl Entry { } impl Ring for Entry { - fn fire(&self, _how: Fire) { - if self.0.state.fire() { + // As the kernel's `PollEntry`: readiness fires the poll, an end ends it. + fn fire(&self, how: Fire) { + let took = match how { + Fire::Ready => self.0.state.fire(), + Fire::Gone => self.0.state.end(), + }; + if took { + self.0.ended.store(how == Fire::Gone, Ordering::Release); self.0.posts.fetch_add(1, Ordering::AcqRel); } } @@ -369,6 +378,11 @@ fn end_racing(end: fn(&World), post: fn(&World)) { assert_eq!(poll.posts(), 1); assert!(!poll.live()); + assert_eq!( + poll.0.state.ended(), + poll.0.ended.load(Ordering::Acquire), + "the one-shot names the wrong taker" + ); drop(world); } @@ -644,8 +658,8 @@ fn a_poll_on_two_watches_racing_both_posts_completes_exactly_once() { }); } -/// A poll ring as the kernel's is: its completions behind a lock of its own, -/// and a watch its submitter parks on, which holds threads and no ring. +/// A poll ring: its completions behind a lock of its own, and a watch its +/// submitter parks on, which holds threads and no ring. struct PollRing { cpus: CpuHandles, kicks: Kicks, @@ -664,8 +678,12 @@ struct RingEntry(Arc); type RingWatch = Watch>>; impl Ring for RingEntry { - fn fire(&self, _how: Fire) { - if self.0.state.fire() { + fn fire(&self, how: Fire) { + let took = match how { + Fire::Ready => self.0.state.fire(), + Fire::Gone => self.0.state.end(), + }; + if took { let ring = &self.0.ring; ring.written.with(|n| *n += 1); let env = Poster { cpus: &ring.cpus, kicker: &ring.kicks, preempt: &RemoteGuard }; From 648ced1fa82a6ac880800750c0299b112b3a7c55 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 2 Oct 2026 18:43:34 +0200 Subject: [PATCH 11/14] inbox: a submitter parks on its polls, a watch during a look holds its handle's place, and the answer has a guest test Answers the review of 67881a654. A. `tests/toyos-rust-tests/src/bin/inbox_empty_write.rs`: one poller watches two pipes, a write of no bytes goes into the first and one byte into the second, and the wait answers the second's token alone. It is the one test that runs `sys_write`'s post, the look's `resolve` and `pipe::has_data`. B. The `owed` mark and its swap are gone. A fire takes its poll and posts the ring's watch; the submitter, registered on that watch, reads its polls again before it parks (`polls::awake`). That is the watch's own lost-wake argument, with no word beside the poll to keep in step with it. The mark was first moved into `polls.rs` so `toyos-sched-loom` could run it: loom then parked the submitter over two fires, because it lets a plain store land before another thread's earlier swap in modification order, which no swap allows. Reading the polls needs no such argument. `toyos-sched/loom/tests/loom_watch.rs`'s ring model is rewritten over the kernel's own `polls.rs`: `Poll::fire`, `Polls`, `deliver` and `awake`, against the real watch and park. `two_posts_through_one_rings_lock_lose_no_wake` becomes `a_fire_racing_a_submitters_park_is_never_lost`, and `commit-ignores-notify` names it. A full completion ring keeps `awake` false: a submitter with a fired poll and no room to answer it would otherwise go round for good. C. `inbox_answer` gains a replaced poll's withdrawal, a torn-down ring's, and a closed handle's refusal as the answer. Notes: - A poll taken for its look stays among the ring's polls, marked, so a watch another thread submits during the look replaces it (`Polls::settle`, `Polls::renew`): the look then answers nothing and arms nothing beside the newer watch. Two cases stage the watch inside the look. - The cap is tested at the cap. - libc's `poll` plan is `pollreq.rs`, pure, and `toyos-libc-copies` tests a descriptor named twice. - A ring's completions sit behind a `Lock`: no interrupt handler reaches them, and the issue that said so closes. - `toyos::Poller` is `Sync` again, as on main; filed. - Filed: a close of one handle ends every ring's poll on its object; a ring handed to another process resolves its watches in the receiver's table. Removed: the comment in `Inbox::complete` about a handler's post, "what a ring did before this file" and its two copies, and `Poller::wait`'s clause about a handle that was ready. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm --- Cargo.lock | 1 + ...are-behind-an-irq-lock-no-handler-takes.md | 22 -- ...and-a-watch-moves-the-tail-in-two-steps.md | 16 ++ ...lves-its-watches-in-the-receivers-table.md | 20 ++ ...dle-ends-every-rings-poll-on-its-object.md | 20 ++ kernel-loom/Cargo.toml | 5 +- kernel-loom/tests/inbox_answer.rs | 232 +++++++++++++++--- kernel/src/inbox/mod.rs | 90 +++---- kernel/src/inbox/polls.rs | 147 +++++++---- src/ci.rs | 5 +- .../src/bin/inbox_empty_write.rs | 39 +++ toyos-libc-copies/src/lib.rs | 5 + toyos-libc-copies/src/poll_requests.rs | 27 ++ toyos-sched/loom/Cargo.toml | 11 + toyos-sched/loom/tests/loom_watch.rs | 166 ++++++++----- toyos/src/poller.rs | 6 +- userland/libc/src/lib.rs | 1 + userland/libc/src/pollreq.rs | 38 +++ userland/libc/src/posix_io.rs | 20 +- 19 files changed, 649 insertions(+), 222 deletions(-) delete mode 100644 issues/design-debt/a-rings-completions-are-behind-an-irq-lock-no-handler-takes.md create mode 100644 issues/design-debt/toyos-poller-is-sync-and-a-watch-moves-the-tail-in-two-steps.md create mode 100644 issues/isolation/a-ring-handed-to-another-process-resolves-its-watches-in-the-receivers-table.md create mode 100644 issues/kernel/a-close-of-one-handle-ends-every-rings-poll-on-its-object.md create mode 100644 tests/toyos-rust-tests/src/bin/inbox_empty_write.rs create mode 100644 toyos-libc-copies/src/poll_requests.rs create mode 100644 userland/libc/src/pollreq.rs diff --git a/Cargo.lock b/Cargo.lock index f2824e05b5a..b014afb198d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1376,6 +1376,7 @@ name = "toyos-sched-loom" version = "0.1.0" dependencies = [ "loom", + "toyos-abi", ] [[package]] diff --git a/issues/design-debt/a-rings-completions-are-behind-an-irq-lock-no-handler-takes.md b/issues/design-debt/a-rings-completions-are-behind-an-irq-lock-no-handler-takes.md deleted file mode 100644 index 5d29cfdc2ac..00000000000 --- a/issues/design-debt/a-rings-completions-are-behind-an-irq-lock-no-handler-takes.md +++ /dev/null @@ -1,22 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-10-01 ---- - -# A ring's completions are behind an IRQ lock no handler takes - -`kernel/src/inbox/mod.rs` keeps what a completion writes, and the ring's page, -behind an `IrqLock`, which masks interrupts while it is held. That was for a -device's interrupt handler, whose post wrote a poll's completion in place. -Since a post only owes the poll a look (`kernel/src/inbox/polls.rs`), every -completion is written by the ring's own submitter or by the -`handler-post` actuator's `Staged` ring, both in thread context, and no -handler reaches the lock. Each completion written still masks interrupts for -the write; `handler-post`'s middle arm (`in_a_ring`) stages a handler inside -a section no handler's post enters any more; and `toyos-sched/loom`'s -`two_posts_through_one_rings_lock_lose_no_wake` models a fire that takes a -ring's lock, which no fire does. - -**Exit**: the completions sit behind a lock that leaves interrupts open, and -`handler-post` and the loom models stage only nestings a post can reach. diff --git a/issues/design-debt/toyos-poller-is-sync-and-a-watch-moves-the-tail-in-two-steps.md b/issues/design-debt/toyos-poller-is-sync-and-a-watch-moves-the-tail-in-two-steps.md new file mode 100644 index 00000000000..9295124fe32 --- /dev/null +++ b/issues/design-debt/toyos-poller-is-sync-and-a-watch-moves-the-tail-in-two-steps.md @@ -0,0 +1,16 @@ +--- +status: open +kind: defect +opened: 2026-10-02 +--- + +# `toyos::Poller` is `Sync`, and a watch moves the submission tail in two steps + +`Poller::watch_raw` loads the submission tail, writes the entry at it and +stores the tail plus one, through `&self`, and `Poller` is `unsafe impl Sync` +(`toyos/src/poller.rs`). Two threads watching through one `&Poller` write one +slot at once and advance the tail once, so one watch is lost. `rg 'Arc'` +and `rg 'static.*Poller'` over `toyos`, `userland` and `tests` find no poller +shared between threads. + +**Exit**: `Poller` is not `Sync`, or a watch claims its slot in one step. diff --git a/issues/isolation/a-ring-handed-to-another-process-resolves-its-watches-in-the-receivers-table.md b/issues/isolation/a-ring-handed-to-another-process-resolves-its-watches-in-the-receivers-table.md new file mode 100644 index 00000000000..a03992186a7 --- /dev/null +++ b/issues/isolation/a-ring-handed-to-another-process-resolves-its-watches-in-the-receivers-table.md @@ -0,0 +1,20 @@ +--- +status: open +kind: defect +opened: 2026-10-02 +--- + +# A ring handed to another process resolves its watches in the receiver's table + +An `Inbox` handle carries `DUP` and `TRANSFER` (`ops::initial_rights`), and +`inbox_submit` resolves every handle a watch names in the calling process's +table (`inbox::resolve`): an `OP_WATCH` when it is submitted, and a fired +poll's when its submitter looks at the object again. The ring's page is mapped +into its creator alone, so a process handed the ring runs the submissions the +creator wrote, and looks at the creator's fired polls, against whatever its +own table holds under the creator's handle numbers. It reaches no object it +does not hold. + +**Exit**: a ring cannot leave the process that made it, or a watch names its +object by something its registrant's table decided; a test hands a ring to a +child. diff --git a/issues/kernel/a-close-of-one-handle-ends-every-rings-poll-on-its-object.md b/issues/kernel/a-close-of-one-handle-ends-every-rings-poll-on-its-object.md new file mode 100644 index 00000000000..4d0aec608cb --- /dev/null +++ b/issues/kernel/a-close-of-one-handle-ends-every-rings-poll-on-its-object.md @@ -0,0 +1,20 @@ +--- +status: open +kind: defect +opened: 2026-10-02 +--- + +# A close of one handle ends every ring's poll on its object + +`ops::close` answers every poll on a watch its object ends with `-NotFound` +(`Watch::cancel_polls`), in every ring, when any one handle to a pipe's read +end or an acceptor closes. A sibling handle from `dup` keeps the object open, +and its polls end all the same. `toyos::poller`'s `drain` hands the token of a +negative completion to its caller as it does a ready one's, so a reader that +takes that token for bytes and reads blocking parks on an object that is open +and empty. No caller in the tree reaches it: fsd's acceptors are endowed and +never duplicated. `inbox_cancel_wakes` stages the close. + +**Exit**: a poll ends only when the last handle to its source closes, or a +completion's result reaches `Poller`'s caller; a test closes a duplicate under +a watch and reads what the wait hands back. diff --git a/kernel-loom/Cargo.toml b/kernel-loom/Cargo.toml index c0915af4bc4..9111bcfc29f 100644 --- a/kernel-loom/Cargo.toml +++ b/kernel-loom/Cargo.toml @@ -50,9 +50,8 @@ wake-fence-off = [] # Never on by default, and no kernel build can reach it. poll-fire-load-store = [] # The negative control for a poll ring's answer. It makes `inbox/polls.rs` -# answer a fired poll with its interest and look at nothing, which is what a -# ring did before that file, so a post with nothing behind it is an answer and -# `inbox_answer.rs` must red: +# answer a fired poll with its interest and look at nothing, so a post with +# nothing behind it is an answer and `inbox_answer.rs` must red: # # cargo test --manifest-path kernel-loom/Cargo.toml --features post-is-an-answer \ # --test inbox_answer diff --git a/kernel-loom/tests/inbox_answer.rs b/kernel-loom/tests/inbox_answer.rs index 893ef73d50e..9ac6aa1f9d4 100644 --- a/kernel-loom/tests/inbox_answer.rs +++ b/kernel-loom/tests/inbox_answer.rs @@ -9,7 +9,10 @@ //! sequence and its assertion the answers it is handed. //! //! In `loom::model`, because the one-shot under each poll is built from -//! loom's atomics in this crate; every model here is one thread. +//! loom's atomics in this crate; every model here is one thread, and a watch +//! another thread submits during a look is staged inside the look. A fire +//! racing the submitter's park is `toyos-sched-loom`'s +//! `a_fire_racing_a_submitters_park_is_never_lost`. //! //! The negative case is a cargo feature: //! @@ -18,16 +21,14 @@ //! --test inbox_answer //! ``` //! -//! answers a fired poll without looking, as a ring did before `polls.rs`, and -//! this file must red. +//! answers a fired poll without looking, and this file must red. #![cfg(feature = "loom")] use std::cell::{Cell, RefCell}; -use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::Arc; -use kernel_loom::inbox_polls::{deliver, Look, Poll, Polls, Submitter, Wake}; +use kernel_loom::inbox_polls::{awake, deliver, Look, Poll, Polls, Submitter, Wake}; use toyos_abi::handle::RawHandle; use toyos_abi::inbox::READABLE; use toyos_abi::syscall::SyscallError; @@ -42,14 +43,12 @@ const BYTES: i32 = READABLE as i32; const NOTHING: [(u64, i32); 0] = []; -/// What a fire tells: a count, standing in for the ring's waiter. -#[derive(Clone, Default)] -struct Owed(Arc); +/// The ring a fire tells, which has no waiter: every model is one thread. +#[derive(Clone)] +struct Ring; -impl Wake for Owed { - fn owe(&self) { - self.0.fetch_add(1, Ordering::Relaxed); - } +impl Wake for Ring { + fn wake(&self) {} } /// One object: whether it holds bytes, and the polls armed on its watch. @@ -58,30 +57,39 @@ struct Object { bytes: Cell, /// Whether a post on it is its readiness, the kernel holding none to look at. posts_are_readiness: bool, - armed: RefCell>>>, + armed: RefCell>>>, } /// One ring's kernel over the objects `H`, `G` and `L`. struct Kernel { - owed: Owed, - polls: RefCell>, + ring: Ring, + polls: RefCell>, + /// How many polls the ring keeps. + cap: usize, h: Object, g: Object, l: Object, + /// The handle the process has closed since it watched it. + closed: Cell>, + /// A watch another thread submits while the next look runs. + lands_during_look: Cell>, /// The completion ring, and how many answers it holds. - ring: RefCell>, + answers: RefCell>, size: usize, } impl Kernel { fn new(size: usize) -> Self { Self { - owed: Owed::default(), + ring: Ring, polls: RefCell::new(Polls::new()), + cap: 16, h: Object::default(), g: Object::default(), l: Object { posts_are_readiness: true, ..Object::default() }, - ring: RefCell::new(Vec::new()), + closed: Cell::new(None), + lands_during_look: Cell::new(None), + answers: RefCell::new(Vec::new()), size, } } @@ -95,16 +103,24 @@ impl Kernel { } } - /// `OP_WATCH` for bytes on `handle`, answered under `token`. + /// `OP_WATCH` for bytes on `handle`, answered under `token`: the handle's + /// one poll. fn watch(&self, handle: RawHandle, token: u64) { - self.arm(Poll::new(self.owed.clone(), token, handle, READABLE)); + assert!(self.admit(handle, token), "the ring keeps no more polls"); + } + + /// The same, answering whether the ring kept the poll. + fn admit(&self, handle: RawHandle, token: u64) -> bool { + let poll = Arc::new(Poll::new(self.ring.clone(), token, handle, READABLE)); + let kept = self.polls.borrow_mut().admit(poll.clone(), self.cap); + if kept { + self.arm(poll); + } + kept } - /// The handle's one poll: fired now if its object holds bytes, armed on - /// its watch if not. - fn arm(&self, poll: Poll) { - let poll = Arc::new(poll); - assert!(self.polls.borrow_mut().admit(poll.clone(), 16)); + /// Fired now if its object holds bytes, armed on its watch if not. + fn arm(&self, poll: Arc>) { let object = self.object(poll.handle); if object.bytes.get() { poll.fire(0); @@ -137,33 +153,63 @@ impl Kernel { } } + /// The process closes `handle`, whose watch other handles share: no close + /// ends its polls. + fn close(&self, handle: RawHandle) { + self.closed.set(Some(handle)); + } + + /// How many polls on `handle`'s watch a post would still fire. + fn live(&self, handle: RawHandle) -> usize { + self.object(handle).armed.borrow().iter().filter(|poll| poll.armed()).count() + } + /// `inbox_submit`'s look at what the ring is owed. fn submit(&self) { - deliver(|| self.polls.borrow_mut().take_owed(), self); + deliver(self); + } + + /// Whether a submitter waiting for one more answer than the ring holds + /// would go round again instead of parking. + fn awake(&self) -> bool { + awake(self, || false) } /// Every answer the reader's drain hands it. fn drain(&self) -> Vec<(u64, i32)> { - self.ring.take() + self.answers.take() } } -impl Submitter for Kernel { +impl Submitter for Kernel { fn room(&self) -> bool { - self.ring.borrow().len() < self.size + self.answers.borrow().len() < self.size } fn answer(&self, user_data: u64, result: i32) { - self.ring.borrow_mut().push((user_data, result)); + self.answers.borrow_mut().push((user_data, result)); } - fn look(&self, poll: &Poll) -> Look { + fn polls(&self, f: impl FnOnce(&mut Polls) -> R) -> Option { + Some(f(&mut self.polls.borrow_mut())) + } + + fn look(&self, poll: &Arc>) -> Look { + if let Some((handle, token)) = self.lands_during_look.take() { + self.watch(handle, token); + } + if self.closed.get() == Some(poll.handle) { + return Look::Refused(SyscallError::NotFound); + } let object = self.object(poll.handle); if object.bytes.get() || (object.posts_are_readiness && poll.posted() & READABLE != 0) { return Look::Ready(READABLE); } - self.arm(Poll::new(self.owed.clone(), poll.user_data, poll.handle, poll.flags)); - Look::Armed + let again = Arc::new(Poll::new(self.ring.clone(), poll.user_data, poll.handle, poll.flags)); + if self.polls.borrow_mut().renew(poll, again.clone()) { + self.arm(again); + } + Look::Waits } } @@ -318,3 +364,123 @@ fn a_post_answers_for_an_object_with_nothing_to_look_at() { assert_eq!(kernel.drain(), [(7, BYTES)]); }); } + +/// A watch that replaces an armed poll takes it off its object's watch: a +/// poll left live there is one more entry for every re-watch of an idle handle. +#[test] +fn a_replaced_poll_is_withdrawn_from_its_watch() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(H, 1); + kernel.submit(); + kernel.watch(H, 2); + kernel.submit(); + assert_eq!(kernel.live(H), 1, "the replaced poll is still live on its object's watch"); + }); +} + +/// A ring torn down leaves no poll live on any watch. +#[test] +fn a_ring_torn_down_withdraws_every_poll() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(H, 1); + kernel.watch(G, 2); + kernel.submit(); + + kernel.polls.borrow_mut().withdraw_all(); + assert_eq!((kernel.live(H), kernel.live(G)), (0, 0)); + }); +} + +/// A handle closed since it was watched names nothing to look at: the look's +/// refusal is the answer. +#[test] +fn a_handle_closed_since_its_watch_is_answered_with_the_refusal() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(H, 7); + kernel.submit(); + + kernel.close(H); + kernel.post(H); + kernel.submit(); + assert_eq!(kernel.drain(), [(7, -(SyscallError::NotFound as i32))]); + }); +} + +/// A ring keeps its cap of polls and no more, and a watch that replaces one +/// of them is inside it. +#[test] +fn a_ring_keeps_its_cap_of_polls_and_no_more() { + loom::model(|| { + let kernel = Kernel { cap: 2, ..Kernel::new(4) }; + assert!(kernel.admit(H, 1)); + assert!(kernel.admit(G, 2), "a ring under its cap refused a poll"); + assert!(!kernel.admit(L, 3), "a ring at its cap kept one more poll"); + assert!(kernel.admit(H, 4), "a ring at its cap refused a watch that replaces a poll"); + }); +} + +/// A second thread's watch lands while the look at the handle's fired poll +/// finds nothing: the newer watch holds the handle's place, and the look arms +/// nothing beside it. +#[test] +fn a_watch_during_a_look_that_finds_nothing_is_the_handles_one_poll() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(H, 1); + kernel.submit(); + + kernel.post(H); + kernel.lands_during_look.set(Some((H, 2))); + kernel.submit(); + assert_eq!(kernel.drain(), NOTHING); + assert_eq!(kernel.live(H), 1, "the look armed the older watch again beside the newer"); + + kernel.fill(H); + kernel.post(H); + kernel.submit(); + assert_eq!(kernel.drain(), [(2, BYTES)]); + }); +} + +/// The same while the look finds bytes: one arrival, answered once, under the +/// newer watch's token. +#[test] +fn a_watch_during_a_look_that_finds_bytes_answers_alone() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(H, 1); + kernel.submit(); + + kernel.fill(H); + kernel.post(H); + kernel.lands_during_look.set(Some((H, 2))); + kernel.submit(); + assert_eq!(kernel.drain(), [(2, BYTES)]); + }); +} + +/// A submitter does not park over a poll it could answer, and does over a +/// poll a full ring leaves it no room to answer: it would go round for good. +#[test] +fn a_submitter_parks_only_with_nothing_it_could_answer() { + loom::model(|| { + let kernel = Kernel::new(1); + kernel.watch(H, 3); + kernel.watch(G, 4); + kernel.submit(); + assert!(!kernel.awake(), "nothing is owed a look"); + + for handle in [H, G] { + kernel.fill(handle); + kernel.post(handle); + } + assert!(kernel.awake(), "a fired poll is owed a look and the ring has room"); + kernel.submit(); + assert!(!kernel.awake(), "the ring is full: the poll left over waits for room"); + assert_eq!(kernel.drain(), [(3, BYTES)]); + assert!(kernel.awake(), "room came back with a poll still owed its look"); + }); +} diff --git a/kernel/src/inbox/mod.rs b/kernel/src/inbox/mod.rs index b6f061299f4..428f54bd08d 100644 --- a/kernel/src/inbox/mod.rs +++ b/kernel/src/inbox/mod.rs @@ -25,15 +25,15 @@ //! [`MAX_PENDING_WATCHES`] polls. //! //! **Locks.** What a completion writes, and the page it is written into, sit -//! behind an [`IrqLock`] of their own; nothing is taken under it. The rest of a -//! ring, its submissions and its polls, is its `Lock`'s, which no post -//! reaches. A ring's own watch is an [`IrqWatch`] and holds only threads, -//! because no handle names a ring as a thing to watch. +//! behind a `Lock` of their own; nothing is taken under it. The rest of a +//! ring, its submissions and its polls, is another's. No post reaches either: +//! a fire takes its poll and posts the ring's watch. That watch is an +//! [`IrqWatch`], because a device's interrupt handler fires polls, and holds +//! only threads, because no handle names a ring as a thing to watch. use alloc::sync::Arc; -use core::sync::atomic::{AtomicBool, Ordering}; +use core::sync::atomic::Ordering; -use toyos_sched::sync::CellLock; use toyos_sched::task::WaitClass; use toyos_sched::watch::{Fire, Ring}; @@ -43,7 +43,7 @@ use crate::process::{self, Pid}; use crate::scheduler; use crate::sync::Lock; use crate::time::{Deadline, Duration}; -use crate::watch::{IrqLock, IrqWatch}; +use crate::watch::IrqWatch; use crate::DirectMap; use toyos_abi::inbox::{ @@ -73,7 +73,7 @@ impl Drop for InboxRef { // The page is the completions', so it goes only once nothing can reach // them. Both halves are taken out under their locks and let go of // outside them: the unmap flushes. - let Some(completions) = self.0.completions.with(Option::take) else { + let Some(completions) = self.0.completions.lock().take() else { unreachable!("an inbox is torn down by its one reference, once"); }; let Some(mut state) = self.0.state.lock().take() else { @@ -153,10 +153,7 @@ impl Readiness { type Poll = polls::Poll>; impl polls::Wake for Arc { - fn owe(&self) { - // Before the post, so the waiter it wakes finds the debt; in place, - // because a device's interrupt handler fires polls. - self.owed.store(true, Ordering::Release); + fn wake(&self) { self.watch.post_in_place(); } } @@ -184,16 +181,13 @@ impl Ring for PollEntry { /// Hard cap on pending polls per ring. const MAX_PENDING_WATCHES: usize = 1024; -/// A ring: what a poll posts into, and what `submit` parks on. pub struct Inbox { /// `None` once the ring's one reference let go of it. state: Lock>, /// `None` from the moment that reference starts letting go of it. - completions: IrqLock>, + completions: Lock>, /// Threads parked in `submit`; never a poll — see the module header. watch: IrqWatch, - /// A poll has been taken since `submit` last looked. - owed: AtomicBool, } struct RingState { @@ -302,12 +296,8 @@ impl Inbox { /// Write one completion and wake whoever waits in `submit`. A ring already /// torn down takes nothing and wakes nobody. fn complete(&self, user_data: u64, result: i32) { - let posted = self.completions.with(|c| { - c.as_mut().map(|c| c.post_completion(user_data, result, 0)) - }); + let posted = self.completions.lock().as_mut().map(|c| c.post_completion(user_data, result, 0)); if posted.is_some() { - // In place: an interrupt handler's post reaches here through the - // poll it fires. self.watch.post_in_place(); } } @@ -317,7 +307,7 @@ impl Inbox { } fn with_completions(&self, f: impl FnOnce(&Completions) -> R) -> Result { - self.completions.with(|c| c.as_ref().map(f)).ok_or(SyscallError::NotFound) + self.completions.lock().as_ref().map(f).ok_or(SyscallError::NotFound) } } @@ -379,14 +369,13 @@ pub fn create(depth: u32) -> Result<(InboxRef, u64), SyscallError> { polls: Polls::new(), owner_pid: pid, })), - completions: IrqLock::new(Some(Completions { + completions: Lock::new(Some(Completions { shm, page: shm_phys, completion_size, completion_tail: 0, })), watch: IrqWatch::new(), - owed: AtomicBool::new(false), }); Ok((InboxRef(inbox), shm_vaddr)) } @@ -413,10 +402,7 @@ pub fn submit( } loop { - // Before the debt is read again: a fire after this swap sets it anew. - // `AcqRel`, so the polls the swap clears the debt of are seen taken. - inbox.owed.swap(false, Ordering::AcqRel); - polls::deliver(|| inbox.with_state(|s| s.polls.take_owed()).ok().flatten(), inbox); + polls::deliver(inbox); let (count, dropped) = inbox.with_completions(|c| (c.completion_count(), c.dropped()))?; if count >= min_complete || min_complete == 0 { @@ -445,8 +431,9 @@ pub fn submit( WaitClass::Io, deadline, || { - inbox.owed.load(Ordering::Acquire) - || inbox.with_completions(|c| c.completion_count()).map_or(true, |n| n >= min_complete) + polls::awake(inbox, || { + inbox.with_completions(|c| c.completion_count()).map_or(true, |n| n >= min_complete) + }) }, ) .is_err() @@ -522,7 +509,7 @@ fn process_watch(inbox: &Arc, submission: &Submission) { }; // Nothing is held here: `resolve` has given the guard up. let refused = resolve(submission.handle).map_err(HandleError::refuse_as_error).and_then(|object| { - arm(inbox, Poll::new(inbox.clone(), user_data, submission.handle, flags.raw()), &object) + arm(inbox, Poll::new(inbox.clone(), user_data, submission.handle, flags.raw()), None, &object) }); if let Err(refusal) = refused { inbox.complete(user_data, -(refusal as i32)); @@ -535,10 +522,16 @@ fn resolve(handle: RawHandle) -> Result { process::with_process_data(|data| data.handles.get_ref(handle, Rights::WAIT).cloned()) } -/// Keep `poll` as its handle's one poll, and fire it if `object` is ready or -/// arm it on the object's watches if not; the refusal, when nothing could ever -/// answer it. -fn arm(inbox: &Arc, poll: Poll, object: &KObjectRef) -> Result<(), SyscallError> { +/// Keep `poll` as its handle's one poll, or in the place of `looked_at`, the +/// poll a look found nothing for; then fire it if `object` is ready or arm it +/// on the object's watches if not. The refusal, when nothing could ever answer +/// it. +fn arm( + inbox: &Arc, + poll: Poll, + looked_at: Option<&Arc>, + object: &KObjectRef, +) -> Result<(), SyscallError> { let flags = WatchFlags(poll.flags); let ready = readiness_of(object, flags).any(); let read = if flags.readable() { ops::read_watch(object) } else { None }; @@ -551,11 +544,17 @@ fn arm(inbox: &Arc, poll: Poll, object: &KObjectRef) -> Result<(), Syscal let poll = Arc::new(poll); // The cap is checked before registering: registering first would leave a // watch holding a poll the ring never counted. - match inbox.with_state(|state| state.polls.admit(poll.clone(), MAX_PENDING_WATCHES)) { - Ok(true) => {} - Ok(false) => return Err(SyscallError::ResourceExhausted), - // The ring's last handle closed under this submit. - Err(_) => return Ok(()), + let kept = inbox.with_state(|state| match looked_at { + None if state.polls.admit(poll.clone(), MAX_PENDING_WATCHES) => Ok(true), + None => Err(SyscallError::ResourceExhausted), + Some(old) => Ok(state.polls.renew(old, poll.clone())), + }); + match kept { + Ok(Ok(true)) => {} + Ok(Err(full)) => return Err(full), + // A newer watch took the handle's place during the look, or the + // ring's last handle closed under this submit. + Ok(Ok(false)) | Err(_) => return Ok(()), } if ready { poll.fire(0); @@ -594,7 +593,11 @@ impl Submitter> for Arc { self.complete(user_data, result); } - fn look(&self, poll: &Poll) -> Look { + fn polls(&self, f: impl FnOnce(&mut Polls>) -> R) -> Option { + self.with_state(|state| f(&mut state.polls)).ok() + } + + fn look(&self, poll: &Arc) -> Look { let object = match resolve(poll.handle) { Ok(object) => object, // Closed since it was watched, which is no bug of the process's: @@ -615,8 +618,9 @@ impl Submitter> for Arc { if now.any() { return Look::Ready(now.result_flags()); } - match arm(self, Poll::new(self.clone(), poll.user_data, poll.handle, poll.flags), &object) { - Ok(()) => Look::Armed, + let again = Poll::new(self.clone(), poll.user_data, poll.handle, poll.flags); + match arm(self, again, Some(poll), &object) { + Ok(()) => Look::Waits, Err(refusal) => Look::Refused(refusal), } } diff --git a/kernel/src/inbox/polls.rs b/kernel/src/inbox/polls.rs index da3686a9aa3..2ef866b1677 100644 --- a/kernel/src/inbox/polls.rs +++ b/kernel/src/inbox/polls.rs @@ -1,18 +1,25 @@ //! A ring's polls, and when one of them is answered. //! //! **A post is not an answer.** An object's post fires the polls on its watch, -//! and a fire only owes the poll a look ([`Wake::owe`]). The answer is written -//! by the ring's own submitter, in its `inbox_submit`, after it has looked at -//! the object again ([`deliver`]): an object ready at that look is answered with -//! what it holds then, and one that is not is armed again. So an answer says -//! what the object held when the wait that returned it looked — a peer's empty -//! write, and a post that lands after its bytes were read, answer nothing. +//! and a fire only owes the poll a look. The answer is written by the ring's +//! own submitter, in its `inbox_submit`, after it has looked at the object +//! again ([`deliver`]): an object ready at that look is answered with what it +//! holds then, and one that is not is armed again. So an answer says what the +//! object held when the wait that returned it looked — a peer's empty write, +//! and a post that lands after its bytes were read, answer nothing. //! //! **A handle has one poll that may answer.** A watch replaces the handle's -//! earlier poll whether a post has fired it or not ([`Polls::admit`]), so one -//! look answers a handle once, under the token of its newest watch. +//! earlier poll whether a post has fired it or not, and whether or not a +//! submitter is looking at it ([`Polls::admit`]), so one look answers a handle +//! once, under the token of its newest watch. //! -//! Compiled a second time by `kernel-loom`, so it names nothing of the kernel's. +//! **A submitter parks on the polls themselves** ([`awake`]): a fire takes its +//! poll and then posts the watch the submitter parks on, and the submitter, +//! registered there, reads its polls again before it parks. The lost-wake +//! argument is that watch's, and nothing here records a fire beside the poll. +//! +//! Compiled a second time by `kernel-loom` and by `toyos-sched-loom`, so it +//! names nothing of the kernel's. use alloc::sync::Arc; use alloc::vec::Vec; @@ -27,16 +34,17 @@ use toyos_abi::syscall::SyscallError; use super::once::Once; -/// Who a fire tells. +/// A ring, as a fire sees it. pub trait Wake { - /// A poll is owed a look: wake whoever waits in this ring's `inbox_submit`. - fn owe(&self); + /// Wake whoever waits in this ring's `inbox_submit`. Called from an + /// interrupt handler too. + fn wake(&self); } /// One `OP_WATCH` a ring is waiting on: one-shot across every watch it is /// registered on and against its own registrant's recheck. pub struct Poll { - wake: W, + ring: W, pub user_data: u64, /// The handle the poll was submitted against; the key a watch replaces by. pub handle: RawHandle, @@ -49,8 +57,8 @@ pub struct Poll { } impl Poll { - pub fn new(wake: W, user_data: u64, handle: RawHandle, flags: u32) -> Self { - Self { wake, user_data, handle, flags, posted: AtomicU32::new(0), state: Once::new() } + pub fn new(ring: W, user_data: u64, handle: RawHandle, flags: u32) -> Self { + Self { ring, user_data, handle, flags, posted: AtomicU32::new(0), state: Once::new() } } /// The object may be ready: the watch of the directions in `posted` was @@ -59,14 +67,14 @@ impl Poll { // Before the exchange, so the look that the winning fire owes sees it. self.posted.fetch_or(posted, Ordering::Release); if self.state.fire() { - self.wake.owe(); + self.ring.wake(); } } /// The object's source ended: the poll is answered as gone, with no look. pub fn end(&self) { if self.state.end() { - self.wake.owe(); + self.ring.wake(); } } @@ -86,9 +94,22 @@ impl Poll { } } -/// A ring's polls that may still answer: armed, or taken and not yet looked at. +/// A ring's polls that may still answer: armed, or taken and not yet answered. pub struct Polls { - polls: Vec>>, + polls: Vec>, +} + +struct Kept { + poll: Arc>, + /// A submitter took it for its look, and no other takes it. + looking: bool, +} + +impl Kept { + /// Something has taken the poll, and no submitter is looking at it. + fn owed(&self) -> bool { + !self.looking && !self.poll.armed() + } } impl Polls { @@ -100,30 +121,57 @@ impl Polls { /// Every earlier poll on the handle answers nothing from here on, whether /// a post has fired it or not. pub fn admit(&mut self, poll: Arc>, cap: usize) -> bool { - self.polls.retain(|p| { - let other = p.handle != poll.handle; + self.polls.retain(|kept| { + let other = kept.poll.handle != poll.handle; if !other { - p.withdraw(); + kept.poll.withdraw(); } other }); if self.polls.len() >= cap { return false; } - self.polls.push(poll); + self.polls.push(Kept { poll, looking: false }); true } - /// The oldest poll something has taken, given up for its look. + /// The oldest poll owed a look, kept while it is looked at so a watch on + /// its handle still replaces it. pub fn take_owed(&mut self) -> Option>> { - let at = self.polls.iter().position(|p| !p.armed())?; - Some(self.polls.remove(at)) + let kept = self.polls.iter_mut().find(|kept| kept.owed())?; + kept.looking = true; + Some(kept.poll.clone()) + } + + /// Let go of `poll` once its look has an answer. `false` if a newer watch + /// replaced it during the look or the ring went: it answers nothing. + pub fn settle(&mut self, poll: &Arc>) -> bool { + let Some(at) = self.place(poll) else { return false }; + self.polls.remove(at); + true + } + + /// Keep `again` in `poll`'s place once its look found nothing; `false` as + /// [`Self::settle`] says it, and `again` is not kept. + pub fn renew(&mut self, poll: &Arc>, again: Arc>) -> bool { + let Some(at) = self.place(poll) else { return false }; + self.polls[at] = Kept { poll: again, looking: false }; + true + } + + /// Whether a poll is owed a look. + pub fn owed(&self) -> bool { + self.polls.iter().any(Kept::owed) + } + + fn place(&self, poll: &Arc>) -> Option { + self.polls.iter().position(|kept| Arc::ptr_eq(&kept.poll, poll)) } /// The ring is going: no poll answers. pub fn withdraw_all(&mut self) { - for poll in self.polls.drain(..) { - poll.withdraw(); + for kept in self.polls.drain(..) { + kept.poll.withdraw(); } } } @@ -141,9 +189,9 @@ pub enum Look { /// The handle names nothing that could answer any more; the refusal is /// the answer. Refused(SyscallError), - /// Not ready: a new poll on the handle is armed, and this one answers - /// nothing. - Armed, + /// Not ready: the poll that holds the handle's place now answers later, + /// and this one answers nothing. + Waits, } /// The submitter's side of [`deliver`]. @@ -151,38 +199,49 @@ pub trait Submitter { /// Whether the completion ring takes one more answer. fn room(&self) -> bool; fn answer(&self, user_data: u64, result: i32); - /// Look at the object `poll` watches, and arm a new poll on it if it is - /// not ready. - fn look(&self, poll: &Poll) -> Look; + /// Look at the object `poll` watches, and [`Polls::renew`] the poll if it + /// is not ready. + fn look(&self, poll: &Arc>) -> Look; + /// Run `f` on the ring's polls under their lock; `None` once the ring is + /// gone. + fn polls(&self, f: impl FnOnce(&mut Polls) -> R) -> Option; } -/// Answer every poll `take` gives up, oldest first, while the ring has room; a -/// poll left over is answered by a later look, never dropped. -pub fn deliver(mut take: impl FnMut() -> Option>>, ring: &impl Submitter) { +/// Answer every poll owed a look, oldest first, while the ring has room; a +/// poll left over is answered by a later call, never dropped. +pub fn deliver(ring: &impl Submitter) { while ring.room() { - let Some(poll) = take() else { return }; + let Some(poll) = ring.polls(Polls::take_owed).flatten() else { return }; let result = if poll.state.ended() { -(SyscallError::NotFound as i32) } else { match look(ring, &poll) { Look::Ready(flags) => flags as i32, Look::Refused(e) => -(e as i32), - Look::Armed => continue, + Look::Waits => continue, } }; - ring.answer(poll.user_data, result); + if ring.polls(|polls| polls.settle(&poll)) == Some(true) { + ring.answer(poll.user_data, result); + } } } +/// The park predicate of a submitter waiting until `enough()`, read once it is +/// registered on the watch a fire posts: it does not park over a poll +/// [`deliver`] would answer. +pub fn awake(ring: &impl Submitter, enough: impl FnOnce() -> bool) -> bool { + ring.room() && ring.polls(|polls| polls.owed()) == Some(true) || enough() +} + /// `post-is-an-answer` is the negative control: a fired poll is answered with -/// its interest and nobody looks, which is what a ring did before this file, -/// and `kernel-loom`'s `inbox_answer` reds. +/// its interest and nobody looks, and `kernel-loom`'s `inbox_answer` reds. #[cfg(not(feature = "post-is-an-answer"))] -fn look(ring: &impl Submitter, poll: &Poll) -> Look { +fn look(ring: &impl Submitter, poll: &Arc>) -> Look { ring.look(poll) } #[cfg(feature = "post-is-an-answer")] -fn look(_ring: &impl Submitter, poll: &Poll) -> Look { +fn look(_ring: &impl Submitter, poll: &Arc>) -> Look { Look::Ready(poll.flags) } diff --git a/src/ci.rs b/src/ci.rs index d286ec1d785..d9e4158b6e5 100644 --- a/src/ci.rs +++ b/src/ci.rs @@ -327,9 +327,8 @@ pub(crate) const CONTROLS: &[Control] = &[ message: "parked with the condition true and no wake owed: the post was lost", }, Says { - test: "two_posts_through_one_rings_lock_lose_no_wake", - message: "parked with both completions written and no wake owed: a ring's post was \ - lost", + test: "a_fire_racing_a_submitters_park_is_never_lost", + message: "parked over a fired poll and no wake owed: a fire was lost", }, ]), // The notify's flagged arm answering off a load: a second post reads the diff --git a/tests/toyos-rust-tests/src/bin/inbox_empty_write.rs b/tests/toyos-rust-tests/src/bin/inbox_empty_write.rs new file mode 100644 index 00000000000..29f433662a2 --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/inbox_empty_write.rs @@ -0,0 +1,39 @@ +//! A watch is answered for what its object holds, never for a post. +//! +//! One poller watches two pipes, both empty and both armed. A write of no +//! bytes into the first posts its readers and leaves nothing to read; one byte +//! then goes into the second. The wait that follows answers the second pipe's +//! token alone: a reader handed the first pipe's would read it blocking and +//! park for good. + +use toyos::poller::{Poller, READABLE}; +use toyos_abi::syscall; + +const EMPTY: u64 = 1; +const FILLED: u64 = 2; + +fn main() { + let empty = syscall::pipe().expect("the pipe that stays empty"); + let filled = syscall::pipe().expect("the pipe that takes a byte"); + + let poller = Poller::new(2); + poller.watch_raw(empty.read, READABLE, EMPTY); + poller.watch_raw(filled.read, READABLE, FILLED); + // A non-blocking enter, so both polls are armed before either write: a + // write that found none would post nothing. + poller.wait(0, 0, |token| panic!("nothing is ready yet, got token {token}")); + + let byte = [0x5A]; + // A slice of a real buffer: the kernel is handed an address it can read. + assert_eq!(syscall::write(empty.write, &byte[..0]), Ok(0), "a write of no bytes"); + assert_eq!(syscall::write(filled.write, &byte), Ok(1), "a write of one byte"); + + let mut tokens = Vec::new(); + poller.wait(1, u64::MAX, |token| tokens.push(token)); + assert_eq!(tokens, [FILLED], "the wait answered a pipe with nothing to read"); + println!("inbox_empty_write: a write of no bytes answered no watch"); + + for end in [empty.read, empty.write, filled.read, filled.write] { + syscall::close(end); + } +} diff --git a/toyos-libc-copies/src/lib.rs b/toyos-libc-copies/src/lib.rs index fd0393b3999..0de1d30f573 100644 --- a/toyos-libc-copies/src/lib.rs +++ b/toyos-libc-copies/src/lib.rs @@ -34,6 +34,9 @@ mod listing; #[path = "../../userland/libc/src/memreq.rs"] mod memreq; #[cfg(test)] +#[path = "../../userland/libc/src/pollreq.rs"] +mod pollreq; +#[cfg(test)] #[path = "../../userland/libc/src/sigmask.rs"] mod sigmask; #[cfg(test)] @@ -67,6 +70,8 @@ mod long_double; #[cfg(test)] mod memory_refusals; #[cfg(test)] +mod poll_requests; +#[cfg(test)] mod prototypes; #[cfg(test)] mod signal_masks; diff --git a/toyos-libc-copies/src/poll_requests.rs b/toyos-libc-copies/src/poll_requests.rs new file mode 100644 index 00000000000..ec1881ae069 --- /dev/null +++ b/toyos-libc-copies/src/poll_requests.rs @@ -0,0 +1,27 @@ +//! What `poll` asks a ring to watch: one watch per descriptor, which answers +//! every entry that names it. + +use toyos_abi::inbox::{READABLE, WRITABLE}; + +use crate::header; +use crate::pollreq::{watch_of, watches}; + +const POLL_H: &str = include_str!("../../userland/libc/include/poll.h"); + +fn events(names: &[&str]) -> i16 { + names.iter().fold(0, |events, name| events | header::int(POLL_H, name) as i16) +} + +#[test] +fn a_descriptor_named_twice_is_watched_once_for_both_interests() { + let entries = [(5, events(&["POLLIN"])), (7, events(&["POLLOUT"])), (5, events(&["POLLOUT"]))]; + assert_eq!(watches(&entries), [(0, 5, READABLE | WRITABLE), (1, 7, WRITABLE)]); + assert_eq!([0, 1, 2].map(|entry| watch_of(&entries, entry)), [0, 1, 0]); +} + +#[test] +fn each_descriptor_named_once_has_its_own_watch() { + let entries = [(3, events(&["POLLIN", "POLLOUT"])), (4, events(&["POLLIN"])), (9, events(&["POLLHUP"]))]; + assert_eq!(watches(&entries), [(0, 3, READABLE | WRITABLE), (1, 4, READABLE), (2, 9, 0)]); + assert_eq!([0, 1, 2].map(|entry| watch_of(&entries, entry)), [0, 1, 2]); +} diff --git a/toyos-sched/loom/Cargo.toml b/toyos-sched/loom/Cargo.toml index 1c9dcfe234c..5bec98dce47 100644 --- a/toyos-sched/loom/Cargo.toml +++ b/toyos-sched/loom/Cargo.toml @@ -114,6 +114,11 @@ fault-posted-before-it-is-set = [] [dependencies] loom = "0.7" +# `loom_watch.rs` compiles `kernel/src/inbox/polls.rs`, which names a handle and +# a refusal. +[dev-dependencies] +toyos-abi = { path = "../../toyos-abi" } + [lib] # The primitives are compiled into the lib; the models are the test targets. path = "src/lib.rs" @@ -121,3 +126,9 @@ path = "src/lib.rs" # and run there, against real atomics. Loom atomics outside a `loom::model` # panic, so this crate exposes no unit tests of its own. test = false + +# `kernel-loom`'s control for a ring's answer, which `polls.rs` carries and no +# model here turns on. A `cfg` name and not a feature, since a feature here is +# a model control. +[lints.rust] +unexpected_cfgs = { level = "warn", check-cfg = ['cfg(feature, values("post-is-an-answer"))'] } diff --git a/toyos-sched/loom/tests/loom_watch.rs b/toyos-sched/loom/tests/loom_watch.rs index 874df303b47..11e72a463a6 100644 --- a/toyos-sched/loom/tests/loom_watch.rs +++ b/toyos-sched/loom/tests/loom_watch.rs @@ -30,10 +30,11 @@ //! models must red with a poll completed by neither. //! //! **The ring entry is the kernel's [`Once`], compiled from -//! `kernel/src/inbox/once.rs`**, the decision a `PollEntry` makes; what else a -//! `PollEntry` is — the ring's page, its lock, the look it owes — names half the -//! kernel and cannot be compiled here, so the model's entry counts its answers -//! instead. +//! `kernel/src/inbox/once.rs`**, the decision a `PollEntry` makes, and the last +//! model's ring is the kernel's `kernel/src/inbox/polls.rs` whole: its polls, +//! the submitter's `deliver` and its park predicate. What else a ring is — its +//! page, the objects its looks read — names half the kernel and cannot be +//! compiled here, so the models count answers instead of writing them. //! //! [`TaskShared::notify`]: toyos_sched_loom::task::TaskShared::notify //! [`Gate`]: toyos_sched_loom::watch::Gate @@ -41,6 +42,8 @@ //! [`prepare`]: toyos_sched_loom::park::prepare //! [`Ring::fire`]: toyos_sched_loom::watch::Ring::fire +extern crate alloc; + use loom::sync::atomic::{AtomicBool, AtomicU32, Ordering}; use loom::sync::Arc; use toyos_sched_loom::cpu::{CpuHandle, CpuHandles}; @@ -58,6 +61,11 @@ use toyos_sched_loom::watch::{Fire, Gate, Poster, Ring, Waiters, Watch}; #[path = "../../../kernel/src/inbox/once.rs"] mod once; +/// Names the one-shot above as `super::once`. +#[expect(dead_code, reason = "every look here finds its object ready: a renewal and a refusal are `kernel-loom`'s `inbox_answer`'s")] +#[path = "../../../kernel/src/inbox/polls.rs"] +mod polls; + /// One poll, as the kernel's is: its answer taken once, by the kernel's own /// [`once::Once`], across everything that may fire it; it counts what it /// posted. @@ -658,92 +666,137 @@ fn a_poll_on_two_watches_racing_both_posts_completes_exactly_once() { }); } -/// A poll ring: its completions behind a lock of its own, and a watch its -/// submitter parks on, which holds threads and no ring. +/// A poll ring as its fires and its submitter see it: the kernel's polls, the +/// answers its submitter wrote, and the watch that submitter parks on, which +/// holds threads and no ring. struct PollRing { cpus: CpuHandles, kicks: Kicks, - written: LoomLock, + polls: LoomLock>, + answers: LoomLock, parked: RingWatch, } -/// One of that ring's polls, registered on one device's watch. -struct RingPoll { - ring: Arc, - state: once::Once, +#[derive(Clone)] +struct RingRef(Arc); + +type RingPoll = alloc::sync::Arc>; + +impl polls::Wake for RingRef { + fn wake(&self) { + let ring = &self.0; + let env = Poster { cpus: &ring.cpus, kicker: &ring.kicks, preempt: &RemoteGuard }; + ring.parked.post_in_place(WakeCause::new(WakeReason::Woken), &env); + } +} + +/// Every look finds its object ready: each device posts once, for good. +impl polls::Submitter for PollRing { + fn room(&self) -> bool { + true + } + + fn answer(&self, _user_data: u64, _result: i32) { + self.answers.with(|n| *n += 1); + } + + fn look(&self, poll: &RingPoll) -> polls::Look { + polls::Look::Ready(poll.flags) + } + + fn polls(&self, f: impl FnOnce(&mut polls::Polls) -> R) -> Option { + Some(self.polls.with(f)) + } } -struct RingEntry(Arc); +/// One of that ring's polls, as one device's watch holds it. +struct RingEntry(RingPoll); type RingWatch = Watch>>; impl Ring for RingEntry { + // As the kernel's `PollEntry`. fn fire(&self, how: Fire) { - let took = match how { - Fire::Ready => self.0.state.fire(), - Fire::Gone => self.0.state.end(), - }; - if took { - let ring = &self.0.ring; - ring.written.with(|n| *n += 1); - let env = Poster { cpus: &ring.cpus, kicker: &ring.kicks, preempt: &RemoteGuard }; - ring.parked.post_in_place(WakeCause::new(WakeReason::Woken), &env); + match how { + Fire::Ready => self.0.fire(self.0.flags), + Fire::Gone => self.0.end(), } } fn live(&self) -> bool { - self.0.state.armed() + self.0.armed() } } -/// **A post in place fires its rings under its own list lock**, so beneath it -/// are the ring's lock and the ring's watch. Two devices' watches each hold a -/// poll of one ring and are posted at once, while the ring's submitter waits -/// for both completions: a submitter parked with both completions written was -/// owed the wake the second one posted. +/// `inbox::submit`'s wait for `want` answers, as far as its first park: +/// whether it parked. A park leaves the submitter registered, as a thread +/// blocked in `watch::wait_until` is. +fn submit_parks(ring: &PollRing, submitter: &Arc>, want: u32) -> bool { + let enough = || ring.answers.with(|n| *n) >= want; + // A fire ends at most one iteration short of the last. + for _ in 0..=want { + polls::deliver(ring); + if enough() { + return false; + } + // `watch::wait_until` over `submit`'s predicate. + if polls::awake(ring, enough) { + continue; + } + ring.parked.register(submitter, 0); + while !polls::awake(ring, enough) { + let Ok(ticket) = + prepare(&CurrentTask::new(submitter, CPU0), Cancel::Answers, WaitClass::Io) + else { + continue; + }; + match ticket.commit() { + Commit::Parked(_) => return true, + Commit::AlreadyWoken => continue, + Commit::Killed => unreachable!("nothing retires in this model"), + } + } + ring.parked.unregister(submitter); + } + unreachable!("{want} fires ended more than {want} iterations") +} + +/// **A submitter parks on its polls, and a fire posts the watch it parks on.** +/// Two devices' watches each hold a poll of one ring and are posted at once, +/// from an interrupt handler's post in place, while the ring's submitter waits +/// for both answers. A fire takes its poll and posts the ring's watch; the +/// submitter answers what was fired and, registered, reads its polls again +/// before it parks. A submitter parked over a fired poll was owed the wake +/// that fire posted. #[test] -fn two_posts_through_one_rings_lock_lose_no_wake() { +fn a_fire_racing_a_submitters_park_is_never_lost() { model(|| { let (tx, mut rx) = mailbox::(); let ring = Arc::new(PollRing { cpus: CpuHandles::new(vec![CpuHandle::new(CPU0, tx)]), kicks: Kicks::new(), - written: LoomLock::new(0), + polls: LoomLock::new(polls::Polls::new()), + answers: LoomLock::new(0), parked: Watch::new(watch_list()), }); let devices: Vec> = (0..2).map(|_| Arc::new(Watch::new(watch_list()))).collect(); - for device in &devices { - device.add_ring(RingEntry(Arc::new(RingPoll { - ring: ring.clone(), - state: once::Once::new(), - }))); + for (handle, device) in devices.iter().enumerate() { + let poll = alloc::sync::Arc::new(polls::Poll::new( + RingRef(ring.clone()), + handle as u64, + toyos_abi::handle::RawHandle(handle as u32), + toyos_abi::inbox::READABLE, + )); + assert!(ring.polls.with(|polls| polls.admit(poll.clone(), devices.len()))); + device.add_ring(RingEntry(poll)); } let submitter = task(1); let waiting = { let ring = ring.clone(); let submitter = submitter.clone(); - loom::thread::spawn(move || { - ring.parked.register(&submitter, 0); - // Each post ends at most one iteration. - for _ in 0..4 { - if ring.written.with(|n| *n) == 2 { - return false; - } - let Ok(ticket) = - prepare(&CurrentTask::new(&submitter, CPU0), Cancel::Answers, WaitClass::Io) - else { - continue; - }; - match ticket.commit() { - Commit::Parked(_) => return true, - Commit::AlreadyWoken => continue, - Commit::Killed => unreachable!("nothing retires in this model"), - } - } - unreachable!("two posts ended more than three iterations") - }) + loom::thread::spawn(move || submit_parks(&ring, &submitter, 2)) }; let posters: Vec<_> = devices .iter() @@ -768,11 +821,14 @@ fn two_posts_through_one_rings_lock_lose_no_wake() { assert_eq!( msgs, [Msg::Wake(TaskKey(1), WakeReason::Woken)], - "parked with both completions written and no wake owed: a ring's post was lost", + "parked over a fired poll and no wake owed: a fire was lost", ); } else { + assert_eq!(ring.answers.with(|n| *n), 2, "the wait ended short of its answers"); assert!(msgs.is_empty(), "a submitter that never parked is owed nothing: {msgs:?}"); } ring.parked.unregister(&submitter); + // The ring's teardown: its polls hold it. + ring.polls.with(polls::Polls::withdraw_all); }); } diff --git a/toyos/src/poller.rs b/toyos/src/poller.rs index cc8ac259b2f..f7df1ec4e35 100644 --- a/toyos/src/poller.rs +++ b/toyos/src/poller.rs @@ -200,8 +200,9 @@ pub struct Poller { // Safety: the base pointer is process-local shared memory mapped from the // kernel. It is only ever reached through `Rings`, which takes no reference // over it: atomics for the shared words, whole-value volatile copies for -// everything else. Not `Sync`: a watch moves the submission tail in two steps. +// everything else. unsafe impl Send for Poller {} +unsafe impl Sync for Poller {} impl Poller { /// Widest handle set one poller can carry — the kernel's deepest @@ -304,8 +305,7 @@ impl Poller { /// Submit pending entries and wait for completions. /// /// Blocks until at least `min_complete` completions are ready or `timeout_nanos` - /// elapses. Calls `f` for each completed token: a handle that was ready when - /// the kernel looked, inside this wait ([`OP_WATCH`]). + /// elapses. Calls `f` for each completed token. pub fn wait(&self, min_complete: u32, timeout_nanos: u64, mut f: impl FnMut(u64)) { self.submit(min_complete, timeout_nanos); self.drain(&mut f); diff --git a/userland/libc/src/lib.rs b/userland/libc/src/lib.rs index 8b184115473..77da705e7b9 100644 --- a/userland/libc/src/lib.rs +++ b/userland/libc/src/lib.rs @@ -18,6 +18,7 @@ mod math; mod memory; mod memreq; mod misc; +mod pollreq; mod posix_io; mod printf; mod pthread; diff --git a/userland/libc/src/pollreq.rs b/userland/libc/src/pollreq.rs new file mode 100644 index 00000000000..dc2c7dc5b56 --- /dev/null +++ b/userland/libc/src/pollreq.rs @@ -0,0 +1,38 @@ +//! What `poll` asks a ring to watch and which watch answers each entry. It +//! reads nothing but what it is handed, so the host tests it +//! (`toyos-libc-copies`). + +use alloc::vec::Vec; + +use toyos_abi::inbox::{READABLE, WRITABLE}; + +const POLLIN: i16 = 1; +const POLLOUT: i16 = 4; + +/// The entry whose watch answers for `entries[entry]`, each a descriptor and +/// its `events`: the first that names its descriptor. +pub(crate) fn watch_of(entries: &[(i32, i16)], entry: usize) -> usize { + let (fd, _) = entries[entry]; + entries[..entry].iter().position(|&(other, _)| other == fd).unwrap_or(entry) +} + +/// The watches a `poll` of `entries` submits, each the entry it is answered +/// under, its descriptor and its interest. One per descriptor, for every +/// entry's interest: a watch replaces its handle's earlier one, so a second on +/// the descriptor would leave the first entry unanswered. +pub(crate) fn watches(entries: &[(i32, i16)]) -> Vec<(usize, i32, u32)> { + let mut interest = alloc::vec![0u32; entries.len()]; + for (entry, &(_, events)) in entries.iter().enumerate() { + let watch = watch_of(entries, entry); + if events & POLLIN != 0 { + interest[watch] |= READABLE; + } + if events & POLLOUT != 0 { + interest[watch] |= WRITABLE; + } + } + (0..entries.len()) + .filter(|&entry| watch_of(entries, entry) == entry) + .map(|entry| (entry, entries[entry].0, interest[entry])) + .collect() +} diff --git a/userland/libc/src/posix_io.rs b/userland/libc/src/posix_io.rs index 40cdec48de2..f88fdaeb7cb 100644 --- a/userland/libc/src/posix_io.rs +++ b/userland/libc/src/posix_io.rs @@ -573,9 +573,6 @@ pub struct pollfd { pub revents: i16, } -const POLLIN: i16 = 1; -const POLLOUT: i16 = 4; - #[no_mangle] pub unsafe extern "C" fn poll(fds: *mut pollfd, nfds: u32, timeout: i32) -> i32 { if nfds == 0 { @@ -597,19 +594,10 @@ pub unsafe extern "C" fn poll(fds: *mut pollfd, nfds: u32, timeout: i32) -> i32 let timeout_ns = if timeout < 0 { None } else { Some(timeout as u64 * 1_000_000) }; let n = nfds as usize; - // One watch per fd, under the first entry naming it and for every entry's - // interest: a watch replaces its handle's earlier one, so a second watch - // on the fd would leave the first entry unanswered. - let first = |i: usize| (0..i).find(|&j| (*fds.add(j)).fd == (*fds.add(i)).fd).unwrap_or(i); - let mut interest = alloc::vec![0u32; n]; - for i in 0..n { - let events = (*fds.add(i)).events; - if events & POLLIN != 0 { interest[first(i)] |= toyos::poller::READABLE; } - if events & POLLOUT != 0 { interest[first(i)] |= toyos::poller::WRITABLE; } - } + let entries: alloc::vec::Vec<(i32, i16)> = (0..n).map(|i| ((*fds.add(i)).fd, (*fds.add(i)).events)).collect(); let poller = toyos::poller::Poller::new(n as u32); - for i in (0..n).filter(|&i| first(i) == i) { - poller.watch_raw(toyos_abi::RawHandle((*fds.add(i)).fd as u32), interest[i], i as u64); + for (entry, fd, interest) in crate::pollreq::watches(&entries) { + poller.watch_raw(toyos_abi::RawHandle(fd as u32), interest, entry as u64); } let mut ready_set = alloc::vec![false; n]; @@ -618,7 +606,7 @@ pub unsafe extern "C" fn poll(fds: *mut pollfd, nfds: u32, timeout: i32) -> i32 }); let mut ready = 0i32; for i in 0..n { - let answered = ready_set[first(i)]; + let answered = ready_set[crate::pollreq::watch_of(&entries, i)]; let pfd = &mut *fds.add(i); pfd.revents = 0; if answered { From b6b61967db1a427de1f76272544cb199bd724eb6 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 2 Oct 2026 19:07:44 +0200 Subject: [PATCH 12/14] inbox: one look is one pass, so a peer posting an empty pipe holds no submitter `polls::deliver` took the oldest poll owed a look until none was. A poll it renewed could be fired again before the next take, by a peer writing no bytes as fast as the submitter looked, and the pass then never reached the handles behind it nor returned. Each kept poll now carries its order among every poll its ring kept, and a pass takes only those kept before it began: a renewed poll, and a watch that lands during the pass, wait for the next, which `submit`'s loop makes at once because `awake` sees them owed. `a_poll_fired_as_it_is_renewed_waits_for_the_next_look` stages the peer inside the look. `a_watch_during_a_look_that_finds_bytes_answers_alone` now reads the newer watch's answer from the pass after the one it landed in. Filed, neither removed here: - two submitters of one ring race its last completion slot, and the loser's answer is dropped and counted; - a peer that posts as fast as a submitter looks keeps it from parking, so a kill waits for the first park. Not measured. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm --- ...a-submitter-looks-keeps-it-from-parking.md | 21 ++++++++++ ...-one-ring-race-its-last-completion-slot.md | 17 +++++++++ kernel-loom/tests/inbox_answer.rs | 38 ++++++++++++++++++- kernel/src/inbox/polls.rs | 37 +++++++++++++----- 4 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 issues/kernel/a-peer-posting-as-fast-as-a-submitter-looks-keeps-it-from-parking.md create mode 100644 issues/kernel/two-submitters-of-one-ring-race-its-last-completion-slot.md diff --git a/issues/kernel/a-peer-posting-as-fast-as-a-submitter-looks-keeps-it-from-parking.md b/issues/kernel/a-peer-posting-as-fast-as-a-submitter-looks-keeps-it-from-parking.md new file mode 100644 index 00000000000..aa372bb7b83 --- /dev/null +++ b/issues/kernel/a-peer-posting-as-fast-as-a-submitter-looks-keeps-it-from-parking.md @@ -0,0 +1,21 @@ +--- +status: open +kind: finding +opened: 2026-10-02 +--- + +# A peer that posts as fast as a submitter looks keeps it from parking + +A write of no bytes posts a pipe's readers. A submitter waiting on that pipe +looks, finds nothing, arms the poll again and reads its polls before it parks +(`polls::awake`); a peer whose next post has landed by then sends it round +again with no park. Each round is one bounded pass and answers whatever else +is ready, so the wait returns as soon as anything is. With nothing else ready +the thread stays in the kernel for as long as the peer wins that race, and +`watch::wait_until` tells a thread it was killed only where it would park. +Not measured: no test sustains the race. On main the same posts each return +the waiter to Ring 3 with an answer for nothing. + +**Exit**: a round that answered nothing reads the caller's kill before it goes +round again, as `ops::until_answered` does, with a test; or a measurement +that no peer sustains the race. diff --git a/issues/kernel/two-submitters-of-one-ring-race-its-last-completion-slot.md b/issues/kernel/two-submitters-of-one-ring-race-its-last-completion-slot.md new file mode 100644 index 00000000000..9ffa89392b6 --- /dev/null +++ b/issues/kernel/two-submitters-of-one-ring-race-its-last-completion-slot.md @@ -0,0 +1,17 @@ +--- +status: open +kind: defect +opened: 2026-10-02 +--- + +# Two submitters of one ring race its last completion slot + +`polls::deliver` asks the ring for room and then answers, in two holds of the +completions' lock. Two threads in `inbox_submit` on one ring can both find the +last slot free; the second answer finds the ring full, and `post_completion` +drops it and counts it in `dropped`, as it does every completion written to a +full ring. A ring one thread submits to drops no watch's answer, and +`toyos::Poller` sizes its rings past what its watches can answer. + +**Exit**: the room an answer was looked up for is the room it is written +into; a test runs two submitters against a ring with one slot. diff --git a/kernel-loom/tests/inbox_answer.rs b/kernel-loom/tests/inbox_answer.rs index 9ac6aa1f9d4..09deb83ccf8 100644 --- a/kernel-loom/tests/inbox_answer.rs +++ b/kernel-loom/tests/inbox_answer.rs @@ -73,6 +73,10 @@ struct Kernel { closed: Cell>, /// A watch another thread submits while the next look runs. lands_during_look: Cell>, + /// How many more renewals a peer's post follows at once. + posts_after_renewal: Cell, + /// How many looks the submitter has made. + looks: Cell, /// The completion ring, and how many answers it holds. answers: RefCell>, size: usize, @@ -89,6 +93,8 @@ impl Kernel { l: Object { posts_are_readiness: true, ..Object::default() }, closed: Cell::new(None), lands_during_look: Cell::new(None), + posts_after_renewal: Cell::new(0), + looks: Cell::new(0), answers: RefCell::new(Vec::new()), size, } @@ -195,6 +201,7 @@ impl Submitter for Kernel { } fn look(&self, poll: &Arc>) -> Look { + self.looks.set(self.looks.get() + 1); if let Some((handle, token)) = self.lands_during_look.take() { self.watch(handle, token); } @@ -209,6 +216,10 @@ impl Submitter for Kernel { if self.polls.borrow_mut().renew(poll, again.clone()) { self.arm(again); } + if let Some(left) = self.posts_after_renewal.get().checked_sub(1) { + self.posts_after_renewal.set(left); + self.post(poll.handle); + } Look::Waits } } @@ -446,7 +457,7 @@ fn a_watch_during_a_look_that_finds_nothing_is_the_handles_one_poll() { } /// The same while the look finds bytes: one arrival, answered once, under the -/// newer watch's token. +/// newer watch's token, by the pass after the one it landed in. #[test] fn a_watch_during_a_look_that_finds_bytes_answers_alone() { loom::model(|| { @@ -458,6 +469,9 @@ fn a_watch_during_a_look_that_finds_bytes_answers_alone() { kernel.post(H); kernel.lands_during_look.set(Some((H, 2))); kernel.submit(); + assert_eq!(kernel.drain(), NOTHING, "the look answered under the watch it was replaced by"); + assert!(kernel.awake(), "the newer watch's fire is owed the next look"); + kernel.submit(); assert_eq!(kernel.drain(), [(2, BYTES)]); }); } @@ -484,3 +498,25 @@ fn a_submitter_parks_only_with_nothing_it_could_answer() { assert!(kernel.awake(), "room came back with a poll still owed its look"); }); } + +/// A peer that posts an empty object as fast as the submitter looks at it does +/// not hold the look: the poll renewed in this pass waits for the next, and +/// the handle behind it is answered. +#[test] +fn a_poll_fired_as_it_is_renewed_waits_for_the_next_look() { + loom::model(|| { + let kernel = Kernel::new(4); + kernel.watch(H, 3); + kernel.watch(G, 4); + kernel.submit(); + + kernel.post(H); + kernel.fill(G); + kernel.post(G); + kernel.posts_after_renewal.set(3); + kernel.submit(); + assert_eq!(kernel.looks.get(), 2, "the look went round on a poll it had renewed"); + assert_eq!(kernel.drain(), [(4, BYTES)]); + assert!(kernel.awake(), "the renewed poll's fire is owed the next look"); + }); +} diff --git a/kernel/src/inbox/polls.rs b/kernel/src/inbox/polls.rs index 2ef866b1677..722f58e32de 100644 --- a/kernel/src/inbox/polls.rs +++ b/kernel/src/inbox/polls.rs @@ -97,12 +97,16 @@ impl Poll { /// A ring's polls that may still answer: armed, or taken and not yet answered. pub struct Polls { polls: Vec>, + /// How many polls this ring ever kept: the next one's `order`. + kept: u64, } struct Kept { poll: Arc>, /// A submitter took it for its look, and no other takes it. looking: bool, + /// Its place among every poll its ring ever kept. + order: u64, } impl Kept { @@ -114,7 +118,12 @@ impl Kept { impl Polls { pub const fn new() -> Self { - Self { polls: Vec::new() } + Self { polls: Vec::new(), kept: 0 } + } + + fn keep(&mut self, poll: Arc>) -> Kept { + self.kept += 1; + Kept { poll, looking: false, order: self.kept - 1 } } /// Keep `poll` as its handle's one poll, unless `cap` are kept already. @@ -131,14 +140,21 @@ impl Polls { if self.polls.len() >= cap { return false; } - self.polls.push(Kept { poll, looking: false }); + let kept = self.keep(poll); + self.polls.push(kept); true } - /// The oldest poll owed a look, kept while it is looked at so a watch on - /// its handle still replaces it. - pub fn take_owed(&mut self) -> Option>> { - let kept = self.polls.iter_mut().find(|kept| kept.owed())?; + /// The bound of one pass of looks: a poll kept from here on waits for the + /// next. + pub fn pass(&self) -> u64 { + self.kept + } + + /// The oldest poll owed a look among those kept before `pass`, kept while + /// it is looked at so a watch on its handle still replaces it. + pub fn take_owed(&mut self, pass: u64) -> Option>> { + let kept = self.polls.iter_mut().find(|kept| kept.order < pass && kept.owed())?; kept.looking = true; Some(kept.poll.clone()) } @@ -155,7 +171,7 @@ impl Polls { /// [`Self::settle`] says it, and `again` is not kept. pub fn renew(&mut self, poll: &Arc>, again: Arc>) -> bool { let Some(at) = self.place(poll) else { return false }; - self.polls[at] = Kept { poll: again, looking: false }; + self.polls[at] = self.keep(again); true } @@ -208,10 +224,13 @@ pub trait Submitter { } /// Answer every poll owed a look, oldest first, while the ring has room; a -/// poll left over is answered by a later call, never dropped. +/// poll left over is answered by a later call, never dropped. One pass: a +/// poll a look renews waits for the next call, so a peer that posts an empty +/// object as fast as it is looked at holds nobody. pub fn deliver(ring: &impl Submitter) { + let Some(pass) = ring.polls(|polls| polls.pass()) else { return }; while ring.room() { - let Some(poll) = ring.polls(Polls::take_owed).flatten() else { return }; + let Some(poll) = ring.polls(|polls| polls.take_owed(pass)).flatten() else { return }; let result = if poll.state.ended() { -(SyscallError::NotFound as i32) } else { From 5f62645aeb32614b51080d81788ff360919141d3 Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 2 Oct 2026 20:32:20 +0200 Subject: [PATCH 13/14] inbox: a wait that goes round reads its kill, an answer's wake is the model's, and the log's arm has a guest test The fourth review of #655 (issuecomment-5958166619). A wait that goes round reads no kill. `watch::wait_until` returns on a true predicate before it reads the kill, and `submit`'s predicate is true whenever a poll is owed a look, so a peer whose posts keep landing kept a killed thread in the kernel. `submit` now reads the caller's kill beside the deadline, as `ops::until_answered` does. Measured before the read went in, in one QEMU guest of 8 vCPUs on a 14-core host at load 10: a child watching one empty pipe through 256 handles, four and six threads of its parent writing no bytes into it, was seen running in every roster sample for 70 s and for 4.2 s after its kill, and ended only when the posts left a gap; with the read it was gone in 13 ms. `kill_ends_every_wait` gains that child as `posted-poll`. Its posts stop one second after the kill, since a held kill is held for as long as its peers win a race and no longer, which the harness's deadline cannot see. `issues/kernel/a-peer-posting-as-fast-as-a-submitter-looks-keeps-it-from-parking.md` is closed by this. An answer's wake had no test. `Kept::owed` hides a poll a submitter is looking at, so a second submitter of the ring parks over it, the fire's wake spent before it registered; the looker's answer is what wakes it. That wake was `Inbox::complete`'s, in code no model compiles. It is now `polls::complete`, the one writer of a completion for every op, and `Submitter::answer` writes and wakes nobody. `toyos-sched-loom`'s `an_answer_wakes_the_submitter_its_look_hid_the_poll_from` runs two submitters on two CPUs against one fire of one poll. The log's arm of the look (`ops::read_posts_are_readiness`) had no test that could fail. `inbox_log_post` is a shared-boot member that arms a watch on the endowed `logread` capability, ends a process, is handed its token by `wait(1, u64::MAX)` and reads the kernel's `exit:` record of that pid. Two issues this branch already met are closed: `a-zero-byte-pipe-write-wakes-the-readers-watch.md`, by the look and `inbox_empty_write`, and `two-completions-can-name-one-arrival-and-accept-parks.md`, by `Polls::admit` and `a_watch_replaces_a_poll_a_post_already_fired`. The doc comment in `netd_stream.rs` that said the first goes with it. The interrupts-off-walk issue no longer says a ring's completions sit behind an `IrqLock` or that a fire takes their lock. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm --- ...a-submitter-looks-keeps-it-from-parking.md | 21 -- ...alk-by-the-threads-it-parks-on-one-ring.md | 10 +- ...byte-pipe-write-wakes-the-readers-watch.md | 27 --- ...s-can-name-one-arrival-and-accept-parks.md | 33 ---- kernel-loom/tests/inbox_answer.rs | 8 +- kernel/src/inbox/mod.rs | 40 ++-- kernel/src/inbox/polls.rs | 19 +- .../src/bin/inbox_log_post.rs | 77 ++++++++ .../src/bin/kill_ends_every_wait.rs | 134 ++++++++++++- tests/toyos-rust-tests/src/netd_stream.rs | 4 - toyos-sched/loom/tests/loom_watch.rs | 185 +++++++++++++----- 11 files changed, 383 insertions(+), 175 deletions(-) delete mode 100644 issues/kernel/a-peer-posting-as-fast-as-a-submitter-looks-keeps-it-from-parking.md delete mode 100644 issues/kernel/a-zero-byte-pipe-write-wakes-the-readers-watch.md delete mode 100644 issues/kernel/two-completions-can-name-one-arrival-and-accept-parks.md create mode 100644 tests/toyos-rust-tests/src/bin/inbox_log_post.rs diff --git a/issues/kernel/a-peer-posting-as-fast-as-a-submitter-looks-keeps-it-from-parking.md b/issues/kernel/a-peer-posting-as-fast-as-a-submitter-looks-keeps-it-from-parking.md deleted file mode 100644 index aa372bb7b83..00000000000 --- a/issues/kernel/a-peer-posting-as-fast-as-a-submitter-looks-keeps-it-from-parking.md +++ /dev/null @@ -1,21 +0,0 @@ ---- -status: open -kind: finding -opened: 2026-10-02 ---- - -# A peer that posts as fast as a submitter looks keeps it from parking - -A write of no bytes posts a pipe's readers. A submitter waiting on that pipe -looks, finds nothing, arms the poll again and reads its polls before it parks -(`polls::awake`); a peer whose next post has landed by then sends it round -again with no park. Each round is one bounded pass and answers whatever else -is ready, so the wait returns as soon as anything is. With nothing else ready -the thread stays in the kernel for as long as the peer wins that race, and -`watch::wait_until` tells a thread it was killed only where it would park. -Not measured: no test sustains the race. On main the same posts each return -the waiter to Ring 3 with an answer for nothing. - -**Exit**: a round that answered nothing reads the caller's kill before it goes -round again, as `ops::until_answered` does, with a test; or a measurement -that no peer sustains the race. diff --git a/issues/kernel/a-process-lengthens-an-interrupts-off-walk-by-the-threads-it-parks-on-one-ring.md b/issues/kernel/a-process-lengthens-an-interrupts-off-walk-by-the-threads-it-parks-on-one-ring.md index b38d2a9496f..a5d16c497e7 100644 --- a/issues/kernel/a-process-lengthens-an-interrupts-off-walk-by-the-threads-it-parks-on-one-ring.md +++ b/issues/kernel/a-process-lengthens-an-interrupts-off-walk-by-the-threads-it-parks-on-one-ring.md @@ -10,9 +10,9 @@ Held by the small-kernel track's stage 6 step 2 (`issues/kernel/the-kernel-is-small-interrupts-post-and-threads-wait.md`), whose instrument is the only thing that can read it. -Every poll ring's own watch and its completions sit behind an `IrqLock` +Every poll ring's own watch sits behind an `IrqLock` (`kernel/src/inbox/mod.rs`), because a device handler's post -reaches them through the polls it fires. So any process, not only a device's +reaches it through the polls it fires. So any process, not only a device's holder, decides how long a CPU runs with interrupts masked: - **N threads parked in `submit` on one ring** @@ -25,9 +25,9 @@ holder, decides how long a CPU runs with interrupts masked: that finds the list full copies it, all with interrupts masked. - **A claim's holder polling its claim from R rings, P polls each** (up to `MAX_PENDING_WATCHES`, 1024) makes its device's - handler fire R × P entries under the claim's list lock, each taking that - ring's completions lock and posting that ring's watch, whose own N threads - it notifies. Entries a post in place fired stay in the list until + handler fire R × P entries under the claim's list lock, each posting its + ring's watch, whose own N threads it notifies. Entries a post in place + fired stay in the list until registrations sweep them four at a time. Nothing caps N: a thread costs its process a 128 KiB kernel stack diff --git a/issues/kernel/a-zero-byte-pipe-write-wakes-the-readers-watch.md b/issues/kernel/a-zero-byte-pipe-write-wakes-the-readers-watch.md deleted file mode 100644 index 5bae5e2a1de..00000000000 --- a/issues/kernel/a-zero-byte-pipe-write-wakes-the-readers-watch.md +++ /dev/null @@ -1,27 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-09-26 ---- - -# A zero-byte pipe write wakes the reader's watch - -`sys_write_nonblock` (`kernel/src/syscall/io.rs`) wakes the pipe's -readers after every write that returns `Ok(n)`, `n == 0` included, and -`complete_pending_for_event` (`kernel/src/inbox/mod.rs`) completes a pending -`READABLE` watch on that wake as ready without looking at the ring. So a -zero-byte write, which moves nothing, completes the reader's watch as -readable while the reader's `read` still answers `WouldBlock`. - -netd's liveness probes are exactly such writes, once a pass, into every -piped connection's receive pipe and every piped listener's notify pipe, and -netd passes every millisecond while a piped connection lives. Every client -watching one of those pipes is woken about a thousand times a second for -nothing. Seen, not guessed: a guest waiting on its receive pipe with a -`READABLE` watch was completed as ready within `netd_refused_pipes`' first -two seconds and then read `WouldBlock` (the run with netd's send-side -refusal mutated away, before that test learned to re-check after a wake). - -Exit condition: a write that moves no bytes wakes nobody, or a wake -completes a watch only if its direction is ready, with a test whose reader -watch stays pending across a zero-byte write. diff --git a/issues/kernel/two-completions-can-name-one-arrival-and-accept-parks.md b/issues/kernel/two-completions-can-name-one-arrival-and-accept-parks.md deleted file mode 100644 index 0313fffbad5..00000000000 --- a/issues/kernel/two-completions-can-name-one-arrival-and-accept-parks.md +++ /dev/null @@ -1,33 +0,0 @@ ---- -status: open -kind: defect -opened: 2026-09-27 ---- - -# Two completions can name one arrival, and an `accept` after the second parks - -`process_watch` (`kernel/src/inbox/mod.rs`) answers a watch on a handle that -is already ready at once, and only a watch it has to register replaces the -armed one on the same handle. So a watch submitted while a connection arrives -can complete immediately beside the older armed watch that the same arrival -fires: two completions for one connection, in one drain or in two. A read -after a spurious completion is harmless where the read does not block, but -`SYS_ACCEPT` (`kernel/src/syscall/ipc.rs`, `sys_accept`) parks until a -connection is queued, so a server that accepts once per completion takes the -connection on the first and parks for good on the second — the port it serves -then answers nobody. - -Measured on `/system/bin/fsd`'s log server under `tests/quiescecase`, whose six -writers connect beside logd at boot: a blocked-task dump showed the server -`ipc parked` (the class only `sys_accept` parks in) while logd and every writer -waited on it, in one boot of four and in one of three in a second series. fsd -now asks the acceptor with a zero-timeout watch on a probe ring before it -accepts (`userland/fsd/src/main.rs`, `Server::accept`), and after the change -the same boot ran six times and never parked. Every other server that accepts after -a completion — blockd, logd's inspect and serve threads, init, -soundd, the compositor, netd, filepicker — does not. - -**Exit**: a spurious completion cannot park a server — an accept that does not -park when nothing is queued, or a watch that withdraws the armed one on its -handle whether or not it answers at once, with a test that submits a watch -while a connection arrives and counts the completions. diff --git a/kernel-loom/tests/inbox_answer.rs b/kernel-loom/tests/inbox_answer.rs index 09deb83ccf8..d51108fb5d4 100644 --- a/kernel-loom/tests/inbox_answer.rs +++ b/kernel-loom/tests/inbox_answer.rs @@ -12,7 +12,8 @@ //! loom's atomics in this crate; every model here is one thread, and a watch //! another thread submits during a look is staged inside the look. A fire //! racing the submitter's park is `toyos-sched-loom`'s -//! `a_fire_racing_a_submitters_park_is_never_lost`. +//! `a_fire_racing_a_submitters_park_is_never_lost`, and two submitters of one +//! ring its `an_answer_wakes_the_submitter_its_look_hid_the_poll_from`. //! //! The negative case is a cargo feature: //! @@ -187,6 +188,11 @@ impl Kernel { } } +/// An answer's wake, which has no waiter either. +impl Wake for Kernel { + fn wake(&self) {} +} + impl Submitter for Kernel { fn room(&self) -> bool { self.answers.borrow().len() < self.size diff --git a/kernel/src/inbox/mod.rs b/kernel/src/inbox/mod.rs index 428f54bd08d..1d343a30029 100644 --- a/kernel/src/inbox/mod.rs +++ b/kernel/src/inbox/mod.rs @@ -293,15 +293,6 @@ impl Completions { } impl Inbox { - /// Write one completion and wake whoever waits in `submit`. A ring already - /// torn down takes nothing and wakes nobody. - fn complete(&self, user_data: u64, result: i32) { - let posted = self.completions.lock().as_mut().map(|c| c.post_completion(user_data, result, 0)); - if posted.is_some() { - self.watch.post_in_place(); - } - } - fn with_state(&self, f: impl FnOnce(&mut RingState) -> R) -> Result { self.state.lock().as_mut().map(f).ok_or(SyscallError::NotFound) } @@ -422,6 +413,12 @@ pub fn submit( return Ok(count); } + // Read here and not only at the park: a peer whose posts keep this + // thread looking keeps it from the park, and may not keep its kill. + if crate::sched::driver::current_kill_pending() { + return Err(SyscallError::Gone); + } + // The recheck closure is this ring's own condition, not mere readiness — else a waiter for `min_complete` spins. let parkable = scheduler::Parkable::at_entry(); if crate::watch::wait_until( @@ -479,19 +476,19 @@ fn process_submission(inbox: &Arc, submission: &Submission) { // `Submission::flags` is declared and read by nothing, so a caller setting // it is asking for a behaviour that does not exist. if submission.flags != 0 { - inbox.complete(submission.token, -(SyscallError::InvalidArgument as i32)); + polls::complete(inbox, submission.token, -(SyscallError::InvalidArgument as i32)); return; } let op = match Op::from_raw(submission.op) { Ok(op) => op, Err(_) => { - inbox.complete(submission.token, -(SyscallError::InvalidArgument as i32)); + polls::complete(inbox, submission.token, -(SyscallError::InvalidArgument as i32)); return; } }; match op { - Op::Nop => inbox.complete(submission.token, 0), + Op::Nop => polls::complete(inbox, submission.token, 0), Op::Watch => process_watch(inbox, submission), Op::Accept => process_accept(inbox, submission), } @@ -503,7 +500,7 @@ fn process_watch(inbox: &Arc, submission: &Submission) { let flags = match WatchFlags::from_raw(submission.op_flags) { Ok(flags) => flags, Err(e) => { - inbox.complete(user_data, -(e as i32)); + polls::complete(inbox, user_data, -(e as i32)); return; } }; @@ -512,7 +509,7 @@ fn process_watch(inbox: &Arc, submission: &Submission) { arm(inbox, Poll::new(inbox.clone(), user_data, submission.handle, flags.raw()), None, &object) }); if let Err(refusal) = refused { - inbox.complete(user_data, -(refusal as i32)); + polls::complete(inbox, user_data, -(refusal as i32)); } } @@ -590,7 +587,10 @@ impl Submitter> for Arc { } fn answer(&self, user_data: u64, result: i32) { - self.complete(user_data, result); + // A ring already torn down takes nothing. + if let Some(completions) = self.completions.lock().as_mut() { + completions.post_completion(user_data, result, 0); + } } fn polls(&self, f: impl FnOnce(&mut Polls>) -> R) -> Option { @@ -627,7 +627,7 @@ impl Submitter> for Arc { } /// The submission form of `SYS_ACCEPT`; refusals fold into one `-InvalidArgument` completion instead of ending the process. -fn process_accept(inbox: &Inbox, submission: &Submission) { +fn process_accept(inbox: &Arc, submission: &Submission) { let user_data = submission.token; let acceptor = process::with_process_data(|data| { @@ -639,7 +639,7 @@ fn process_accept(inbox: &Inbox, submission: &Submission) { // Nothing held: `with_process_data` has given the guard up. Err(e) => { let refusal = e.refuse_as_error(); - inbox.complete(user_data, -(refusal as i32)); + polls::complete(inbox, user_data, -(refusal as i32)); return; } }; @@ -658,10 +658,10 @@ fn process_accept(inbox: &Inbox, submission: &Submission) { ) }); match installed { - Ok(h) => inbox.complete(user_data, h.0 as i32), - Err(e) => inbox.complete(user_data, -(e as i32)), + Ok(h) => polls::complete(inbox, user_data, h.0 as i32), + Err(e) => polls::complete(inbox, user_data, -(e as i32)), } } - None => inbox.complete(user_data, -(SyscallError::WouldBlock as i32)), + None => polls::complete(inbox, user_data, -(SyscallError::WouldBlock as i32)), } } diff --git a/kernel/src/inbox/polls.rs b/kernel/src/inbox/polls.rs index 722f58e32de..974a66829bd 100644 --- a/kernel/src/inbox/polls.rs +++ b/kernel/src/inbox/polls.rs @@ -17,6 +17,9 @@ //! poll and then posts the watch the submitter parks on, and the submitter, //! registered there, reads its polls again before it parks. The lost-wake //! argument is that watch's, and nothing here records a fire beside the poll. +//! A poll one submitter is looking at is hidden from every other, which may +//! park over it: what wakes that one is the answer ([`complete`]), which +//! posts the same watch once the completion is written. //! //! Compiled a second time by `kernel-loom` and by `toyos-sched-loom`, so it //! names nothing of the kernel's. @@ -210,10 +213,12 @@ pub enum Look { Waits, } -/// The submitter's side of [`deliver`]. -pub trait Submitter { +/// The submitter's side of [`deliver`]; its [`Wake`] is the ring's own. +pub trait Submitter: Wake { /// Whether the completion ring takes one more answer. fn room(&self) -> bool; + /// Write one completion and wake nobody: [`complete`] is the one caller, + /// and owes the wake. fn answer(&self, user_data: u64, result: i32); /// Look at the object `poll` watches, and [`Polls::renew`] the poll if it /// is not ready. @@ -223,6 +228,14 @@ pub trait Submitter { fn polls(&self, f: impl FnOnce(&mut Polls) -> R) -> Option; } +/// Write one completion, then wake every submitter parked on the ring: one +/// may wait for this count, and one may have parked over the poll it answers +/// while a look hid it. +pub fn complete(ring: &impl Submitter, user_data: u64, result: i32) { + ring.answer(user_data, result); + ring.wake(); +} + /// Answer every poll owed a look, oldest first, while the ring has room; a /// poll left over is answered by a later call, never dropped. One pass: a /// poll a look renews waits for the next call, so a peer that posts an empty @@ -241,7 +254,7 @@ pub fn deliver(ring: &impl Submitter) { } }; if ring.polls(|polls| polls.settle(&poll)) == Some(true) { - ring.answer(poll.user_data, result); + complete(ring, poll.user_data, result); } } } diff --git a/tests/toyos-rust-tests/src/bin/inbox_log_post.rs b/tests/toyos-rust-tests/src/bin/inbox_log_post.rs new file mode 100644 index 00000000000..529d02aab80 --- /dev/null +++ b/tests/toyos-rust-tests/src/bin/inbox_log_post.rs @@ -0,0 +1,77 @@ +//! A watch on the kernel's log is answered by the post a record makes. +//! +//! What the log holds for a reader is the reader's cursor's to say, which the +//! kernel does not hold, so no look can ask it: the post is the answer. A +//! reader arms its watch, a process ends and the kernel records it, and the +//! wait hands the reader its token; the record is then there to read. A watch +//! the post does not answer leaves the wait parked for good, and the harness's +//! deadline is what says so. + +use std::process::Command; + +use toyos::endow::{Endowments, SYSCAP_LABEL}; +use toyos::log::{LogTail, Record}; +use toyos::poller::{Poller, READABLE}; +use toyos::syscap::SysCap; + +const SELF_PATH: &str = "/system/bin/test_rs_inbox_log_post"; + +const LOG: u64 = 7; + +fn main() { + if std::env::args().nth(1).is_some() { + return; + } + let cap: SysCap = Endowments::get() + .take(SYSCAP_LABEL) + .expect("test-runner endows every binary it spawns a system capability"); + let poller = Poller::new(1); + let mut tail = LogTail::new(); + let mut ended: Option = None; + loop { + // Armed before the log is read, as every reader of an edge arms. An + // answer here is some other record's post: the watch is spent, and + // the round begins again. + poller.watch(&cap, READABLE, LOG); + let mut spent = false; + poller.wait(0, 0, |_| spent = true); + if recorded(&mut tail, &cap, ended) { + break; + } + if spent { + continue; + } + // Only with a watch armed and unanswered: the record is the post it + // waits for. + let pid = *ended.get_or_insert_with(end_a_process); + let mut tokens = Vec::new(); + poller.wait(1, u64::MAX, |token| tokens.push(token)); + assert_eq!(tokens, [LOG], "the wait after pid {pid}'s end was recorded"); + } + println!("inbox_log_post: a record's post answered the log's watch"); +} + +/// Run a process to its end, which the kernel records, and answer its pid. +fn end_a_process() -> u32 { + let mut child = Command::new(SELF_PATH).arg("end").spawn().expect("spawn a process to end"); + let pid = child.id(); + assert!(child.wait().expect("wait for it").success(), "the process that only ends"); + pid +} + +/// Read every record the tail has not seen; whether one of them is the end of +/// `pid`. +fn recorded(tail: &mut LogTail, cap: &SysCap, pid: Option) -> bool { + let end = pid.map(|pid| format!(" pid={pid} code=0 ")); + let mut records = vec![Record::EMPTY; 64]; + let mut found = false; + loop { + let read = tail.read(cap, &mut records).expect("read the kernel's records"); + if read.is_empty() { + return found; + } + if let Some(end) = &end { + found |= read.iter().any(|r| r.message().starts_with("exit: ") && r.message().contains(end)); + } + } +} diff --git a/tests/toyos-rust-tests/src/bin/kill_ends_every_wait.rs b/tests/toyos-rust-tests/src/bin/kill_ends_every_wait.rs index 3417cb5ac7b..e6863549674 100644 --- a/tests/toyos-rust-tests/src/bin/kill_ends_every_wait.rs +++ b/tests/toyos-rust-tests/src/bin/kill_ends_every_wait.rs @@ -7,20 +7,29 @@ //! cannot end keeps the child's last thread in its process for ever, and //! `wait` below never returns; the harness's deadline is what says so. //! `mutual_kill` holds a kill inside a kill. +//! +//! `posted-poll` is the poll ring's wait a peer keeps from parking: this +//! process's threads write no bytes into the pipe the child watches, so every +//! post sends the child's wait round to look again. A kill those posts hold is +//! held for as long as they win a race and no longer, which the harness's +//! deadline cannot see: the posts stop at a ceiling of their own, [`HELD`], +//! and the child has to have ended before it. use std::io::{Read, Write}; use std::os::toyos::process::{ChildExt, CommandExt}; use std::process::{Child, Command, Stdio}; -use std::sync::atomic::AtomicU32; -use std::sync::OnceLock; +use std::sync::atomic::{AtomicU32, AtomicU64, Ordering}; +use std::sync::{Arc, Barrier, OnceLock}; +use std::thread::JoinHandle; use std::time::Duration; -use toyos::endow::{Endowments, SYSCAP_LABEL}; -use toyos::poller::Poller; +use toyos::endow::{Endowments, FromHandle, SYSCAP_LABEL}; +use toyos::poller::{Poller, READABLE}; use toyos::process::Process; use toyos::syscap::SysCap; use toyos::AsHandle; -use toyos_abi::syscall; +use toyos_abi::clock; +use toyos_abi::syscall::{self, SyscallError}; use toyos_abi::RawHandle; const SELF_PATH: &str = "/system/bin/test_rs_kill_ends_every_wait"; @@ -28,6 +37,25 @@ const SELF_PATH: &str = "/system/bin/test_rs_kill_ends_every_wait"; /// The label the process-wait child finds the process it waits on under. const WAITED: &str = "waited"; +/// The label the posted-poll child finds the pipe it watches under. +const POSTED: &str = "posted"; + +/// That pipe's read end, which the child holds until it ends. +struct Posted(RawHandle); + +impl FromHandle for Posted { + unsafe fn from_handle(raw: RawHandle) -> Self { + Self(raw) + } +} + +/// The threads that post the pipe the posted-poll child watches. +const POSTERS: usize = 4; + +/// How long those threads go on posting after the kill before they say the +/// child outlived it: a hang ceiling, for a hang that ends when its peers do. +const HELD: Duration = Duration::from_secs(1); + /// `process::KILLED_EXIT_CODE`. const KILLED: i32 = 137; @@ -41,7 +69,7 @@ const ZOMBIE: u8 = 3; // sleep underneath (the waited process's `nanosleep`, the joined thread's // `std::thread::sleep`), so a mutation that breaks sleep would otherwise surface // under one of their names instead of its own. -const WAITS: [&str; 5] = ["futex", "poll", "sleep", "process-wait", "thread-join"]; +const WAITS: [&str; 6] = ["futex", "poll", "posted-poll", "sleep", "process-wait", "thread-join"]; fn main() { match std::env::args().nth(1).as_deref() { @@ -54,13 +82,22 @@ fn test() { for role in WAITS { // The process the process-wait child waits on, which nothing ends but this. let mut waited = (role == "process-wait").then(|| spawn("sleep", None)); - let endow = waited.as_ref().map(|w| { - let dup = syscall::dup(RawHandle(w.as_raw_handle())).expect("a handle to endow"); - (WAITED.to_string(), dup.0) - }); + // The pipe the posted-poll child watches, which no byte ever enters. + let posted = (role == "posted-poll").then(|| syscall::pipe().expect("a pipe to post")); + let endow = waited + .as_ref() + .map(|w| { + let dup = syscall::dup(RawHandle(w.as_raw_handle())).expect("a handle to endow"); + (WAITED.to_string(), dup.0) + }) + .or(posted.as_ref().map(|pipe| (POSTED.to_string(), pipe.read.0))); let mut child = spawn(role, endow); + let posts = posted.map(|pipe| Posts::hold(child.id(), pipe.write)); println!(" {role}: killing"); child.kill().expect("kill the parked child"); + if let Some(posts) = posts { + posts.until_the_child_ends(); + } let code = child.wait().expect("wait for the killed child").code(); assert_eq!(code, Some(KILLED), "a child killed in its {role} wait ended with {code:?}"); if let Some(mut waited) = waited.take() { @@ -105,6 +142,69 @@ fn spawn(role: &str, endow: Option<(String, u32)>) -> Child { child } +/// The threads posting the pipe the posted-poll child watches. +struct Posts { + /// Each answers whether the pipe lost its reader before the ceiling. + threads: Vec>, + /// When the threads stop, in nanoseconds since boot: never, until the kill. + stop_at: Arc, + write: RawHandle, +} + +impl Posts { + /// Start [`POSTERS`] threads, each writing no bytes into `write` until the + /// pipe has no reader, and return once the roster shows `pid`'s main + /// thread out of the park the posts woke it from. + fn hold(pid: u32, write: RawHandle) -> Self { + let stop_at = Arc::new(AtomicU64::new(u64::MAX)); + let posting = Arc::new(Barrier::new(POSTERS + 1)); + let threads = (0..POSTERS) + .map(|_| { + let (stop_at, posting) = (stop_at.clone(), posting.clone()); + std::thread::spawn(move || { + let byte = [0u8]; + posting.wait(); + while clock::nanos_since_boot() < stop_at.load(Ordering::Relaxed) { + // A slice of a real buffer: the kernel is handed an address it can read. + match syscall::write(write, &byte[..0]) { + Ok(0) => {} + // The child ended, and the pipe's one read end with it. + Err(SyscallError::Gone) => return true, + other => panic!("a write of no bytes answered {other:?}"), + } + } + false + }) + }) + .collect(); + // Every thread is posting before the kill: one alone loses the race + // that holds the child. + posting.wait(); + println!(" posted-poll: waiting for the roster to show it looking"); + loop { + match main_thread_status(pid) { + RosterStatus::NotParked => break, + RosterStatus::Parked => std::thread::sleep(Duration::from_millis(10)), + RosterStatus::Gone => panic!("posted-poll: a write of no bytes ended the child's wait"), + } + } + Self { threads, stop_at, write } + } + + /// After the kill: the posts go on until the child ends, and it has to + /// end within [`HELD`] of them. + fn until_the_child_ends(self) { + self.stop_at.store(clock::nanos_since_boot() + HELD.as_nanos() as u64, Ordering::Relaxed); + for thread in self.threads { + assert!( + thread.join().expect("a posting thread"), + "posted-poll: the child still watched its pipe {HELD:?} after its kill: the posts held it in its wait" + ); + } + syscall::close(self.write); + } +} + /// What the roster says about `pid`'s main thread. enum RosterStatus { /// In the roster, blocked: parked in the wait under test. @@ -157,6 +257,20 @@ fn child(role: &str) -> ! { say(role); poller.wait(1, u64::MAX, |_| {}); } + "posted-poll" => { + let Posted(read) = Endowments::get().take(POSTED).expect("the parent endowed a pipe"); + let poller = Poller::new(Poller::MAX_HANDLES); + // As many polls as a poller holds, one handle each: a post fires + // them all, and the wait parks only if no post lands while it + // looks at every one of them. + poller.watch_raw(read, READABLE, 0); + for token in 1..u64::from(Poller::MAX_HANDLES) { + let dup = syscall::dup(read).expect("another handle to the pipe"); + poller.watch_raw(dup, READABLE, token); + } + say(role); + poller.wait(1, u64::MAX, |_| {}); + } "process-wait" => { let waited: Process = Endowments::get().take(WAITED).expect("the parent endowed a process"); say(role); diff --git a/tests/toyos-rust-tests/src/netd_stream.rs b/tests/toyos-rust-tests/src/netd_stream.rs index 996fdd6c319..748c290d656 100644 --- a/tests/toyos-rust-tests/src/netd_stream.rs +++ b/tests/toyos-rust-tests/src/netd_stream.rs @@ -50,10 +50,6 @@ pub fn ring_capacity() -> u64 { /// Wait, with no deadline, until `check` answers, re-asking it each time /// `handle` reports ready for `flags`: an answer that never comes is a hang the /// harness ceiling reds. -/// -/// **A readiness completion is a reason to look again, not an answer**: a -/// zero-byte write still wakes the other end's watch, and netd's liveness -/// probes are zero-byte writes. pub fn await_until( handle: &impl AsHandle, flags: u32, diff --git a/toyos-sched/loom/tests/loom_watch.rs b/toyos-sched/loom/tests/loom_watch.rs index 11e72a463a6..042924459b2 100644 --- a/toyos-sched/loom/tests/loom_watch.rs +++ b/toyos-sched/loom/tests/loom_watch.rs @@ -31,8 +31,9 @@ //! //! **The ring entry is the kernel's [`Once`], compiled from //! `kernel/src/inbox/once.rs`**, the decision a `PollEntry` makes, and the last -//! model's ring is the kernel's `kernel/src/inbox/polls.rs` whole: its polls, -//! the submitter's `deliver` and its park predicate. What else a ring is — its +//! two models' ring is the kernel's `kernel/src/inbox/polls.rs` whole: its +//! polls, the submitter's `deliver`, its park predicate and the wake an answer +//! owes. What else a ring is — its //! page, the objects its looks read — names half the kernel and cannot be //! compiled here, so the models count answers instead of writing them. //! @@ -47,6 +48,7 @@ extern crate alloc; use loom::sync::atomic::{AtomicBool, AtomicU32, Ordering}; use loom::sync::Arc; use toyos_sched_loom::cpu::{CpuHandle, CpuHandles}; +use toyos_sched_loom::hw::CpuId; use toyos_sched_loom::mailbox::{mailbox, MailboxConsumer}; use toyos_sched_loom::model::{ model, watch_list, Kicks, LoomLock, Msg, PreemptModel, RemoteGuard, CPU0, CPU1, @@ -691,13 +693,13 @@ impl polls::Wake for RingRef { } /// Every look finds its object ready: each device posts once, for good. -impl polls::Submitter for PollRing { +impl polls::Submitter for RingRef { fn room(&self) -> bool { true } fn answer(&self, _user_data: u64, _result: i32) { - self.answers.with(|n| *n += 1); + self.0.answers.with(|n| *n += 1); } fn look(&self, poll: &RingPoll) -> polls::Look { @@ -705,7 +707,7 @@ impl polls::Submitter for PollRing { } fn polls(&self, f: impl FnOnce(&mut polls::Polls) -> R) -> Option { - Some(self.polls.with(f)) + Some(self.0.polls.with(f)) } } @@ -728,13 +730,22 @@ impl Ring for RingEntry { } } -/// `inbox::submit`'s wait for `want` answers, as far as its first park: -/// whether it parked. A park leaves the submitter registered, as a thread -/// blocked in `watch::wait_until` is. -fn submit_parks(ring: &PollRing, submitter: &Arc>, want: u32) -> bool { - let enough = || ring.answers.with(|n| *n) >= want; - // A fire ends at most one iteration short of the last. - for _ in 0..=want { +/// `inbox::submit`'s wait for `want` answers on `cpu`, as far as its first +/// park: whether it parked. A park leaves the submitter registered, as a +/// thread blocked in `watch::wait_until` is. `siblings` is how many other +/// submitters answer into the ring. +fn submit_parks( + ring: &RingRef, + submitter: &Arc>, + cpu: CpuId, + want: u32, + siblings: u32, +) -> bool { + let enough = || ring.0.answers.with(|n| *n) >= want; + // A fire ends at most one iteration short of the last, and so does a + // sibling's look at a poll this one saw fired. + let rounds = want + siblings; + for _ in 0..=rounds { polls::deliver(ring); if enough() { return false; @@ -743,10 +754,10 @@ fn submit_parks(ring: &PollRing, submitter: &Arc>, want: u32) -> if polls::awake(ring, enough) { continue; } - ring.parked.register(submitter, 0); + ring.0.parked.register(submitter, 0); while !polls::awake(ring, enough) { let Ok(ticket) = - prepare(&CurrentTask::new(submitter, CPU0), Cancel::Answers, WaitClass::Io) + prepare(&CurrentTask::new(submitter, cpu), Cancel::Answers, WaitClass::Io) else { continue; }; @@ -756,9 +767,59 @@ fn submit_parks(ring: &PollRing, submitter: &Arc>, want: u32) -> Commit::Killed => unreachable!("nothing retires in this model"), } } - ring.parked.unregister(submitter); + ring.0.parked.unregister(submitter); } - unreachable!("{want} fires ended more than {want} iterations") + unreachable!("{rounds} fires and looks ended more than {rounds} iterations") +} + +/// A ring nobody waits on yet, and the mailbox of each CPU a submitter of it +/// runs on. +fn poll_ring(cpus: [CpuId; CPUS]) -> (RingRef, [MailboxConsumer; CPUS]) { + let (handles, mailboxes): (Vec<_>, Vec<_>) = cpus + .into_iter() + .map(|cpu| { + let (tx, rx) = mailbox::(); + (CpuHandle::new(cpu, tx), rx) + }) + .unzip(); + let ring = RingRef(Arc::new(PollRing { + cpus: CpuHandles::new(handles), + kicks: Kicks::new(), + polls: LoomLock::new(polls::Polls::new()), + answers: LoomLock::new(0), + parked: Watch::new(watch_list()), + })); + let Ok(mailboxes) = mailboxes.try_into() else { + unreachable!("one mailbox per CPU"); + }; + (ring, mailboxes) +} + +/// `devices` watches, each holding one armed poll of `ring`. +fn devices_polled(ring: &RingRef, devices: usize) -> Vec> { + (0..devices) + .map(|handle| { + let poll = alloc::sync::Arc::new(polls::Poll::new( + ring.clone(), + handle as u64, + toyos_abi::handle::RawHandle(handle as u32), + toyos_abi::inbox::READABLE, + )); + assert!(ring.0.polls.with(|polls| polls.admit(poll.clone(), devices))); + let device = Arc::new(Watch::new(watch_list())); + device.add_ring(RingEntry(poll)); + device + }) + .collect() +} + +/// An interrupt handler's post of `device`, on its own thread. +fn posted_in_place(ring: &RingRef, device: &Arc) -> loom::thread::JoinHandle<()> { + let (device, ring) = (device.clone(), ring.clone()); + loom::thread::spawn(move || { + let env = Poster { cpus: &ring.0.cpus, kicker: &ring.0.kicks, preempt: &RemoteGuard }; + device.post_in_place(WakeCause::new(WakeReason::Woken), &env); + }) } /// **A submitter parks on its polls, and a fire posts the watch it parks on.** @@ -771,44 +832,16 @@ fn submit_parks(ring: &PollRing, submitter: &Arc>, want: u32) -> #[test] fn a_fire_racing_a_submitters_park_is_never_lost() { model(|| { - let (tx, mut rx) = mailbox::(); - let ring = Arc::new(PollRing { - cpus: CpuHandles::new(vec![CpuHandle::new(CPU0, tx)]), - kicks: Kicks::new(), - polls: LoomLock::new(polls::Polls::new()), - answers: LoomLock::new(0), - parked: Watch::new(watch_list()), - }); - let devices: Vec> = - (0..2).map(|_| Arc::new(Watch::new(watch_list()))).collect(); - for (handle, device) in devices.iter().enumerate() { - let poll = alloc::sync::Arc::new(polls::Poll::new( - RingRef(ring.clone()), - handle as u64, - toyos_abi::handle::RawHandle(handle as u32), - toyos_abi::inbox::READABLE, - )); - assert!(ring.polls.with(|polls| polls.admit(poll.clone(), devices.len()))); - device.add_ring(RingEntry(poll)); - } + let (ring, [mut rx]) = poll_ring([CPU0]); + let devices = devices_polled(&ring, 2); let submitter = task(1); let waiting = { let ring = ring.clone(); let submitter = submitter.clone(); - loom::thread::spawn(move || submit_parks(&ring, &submitter, 2)) + loom::thread::spawn(move || submit_parks(&ring, &submitter, CPU0, 2, 0)) }; - let posters: Vec<_> = devices - .iter() - .map(|device| { - let (device, ring) = (device.clone(), ring.clone()); - loom::thread::spawn(move || { - let env = - Poster { cpus: &ring.cpus, kicker: &ring.kicks, preempt: &RemoteGuard }; - device.post_in_place(WakeCause::new(WakeReason::Woken), &env); - }) - }) - .collect(); + let posters: Vec<_> = devices.iter().map(|device| posted_in_place(&ring, device)).collect(); let parked = waiting.join().unwrap(); for poster in posters { @@ -824,11 +857,61 @@ fn a_fire_racing_a_submitters_park_is_never_lost() { "parked over a fired poll and no wake owed: a fire was lost", ); } else { - assert_eq!(ring.answers.with(|n| *n), 2, "the wait ended short of its answers"); + assert_eq!(ring.0.answers.with(|n| *n), 2, "the wait ended short of its answers"); assert!(msgs.is_empty(), "a submitter that never parked is owed nothing: {msgs:?}"); } - ring.parked.unregister(&submitter); + ring.0.parked.unregister(&submitter); + // The ring's teardown: its polls hold it. + ring.0.polls.with(polls::Polls::withdraw_all); + }); +} + +/// **A submitter parked over a poll its sibling is looking at is owed the +/// answer's wake.** One device's post fires the one poll of a ring two +/// submitters wait on, each for one answer. A look hides its poll from the +/// other submitter, which reads nothing owed and parks, the fire's wake spent +/// before it registered; the looker's answer then posts the watch both park +/// on. A submitter parked with the answer written and no wake owed was lost +/// to that look. +#[test] +fn an_answer_wakes_the_submitter_its_look_hid_the_poll_from() { + model(|| { + let (ring, mut mailboxes) = poll_ring([CPU0, CPU1]); + let devices = devices_polled(&ring, 1); + let submitters = [ + (task(1), CPU0), + (Arc::new(TaskShared::new(TaskKey(2), TaskState::Running(CPU1))), CPU1), + ]; + + let waiting: Vec<_> = submitters + .iter() + .map(|(submitter, cpu)| { + let (ring, submitter, cpu) = (ring.clone(), submitter.clone(), *cpu); + loom::thread::spawn(move || submit_parks(&ring, &submitter, cpu, 1, 1)) + }) + .collect(); + let poster = posted_in_place(&ring, &devices[0]); + + let parked: Vec = waiting.into_iter().map(|wait| wait.join().unwrap()).collect(); + poster.join().unwrap(); + let guard = PreemptModel::new(); + + let answers = ring.0.answers.with(|n| *n); + for (((submitter, _), rx), parked) in submitters.iter().zip(&mut mailboxes).zip(parked) { + let msgs = drain(rx, &guard); + if parked { + assert_eq!( + msgs, + [Msg::Wake(submitter.key(), WakeReason::Woken)], + "parked with {answers} answer(s) written and no wake owed: the answer woke nobody", + ); + } else { + assert_eq!(answers, 1, "the wait ended short of its answer"); + assert!(msgs.is_empty(), "a submitter that never parked is owed nothing: {msgs:?}"); + } + ring.0.parked.unregister(submitter); + } // The ring's teardown: its polls hold it. - ring.polls.with(polls::Polls::withdraw_all); + ring.0.polls.with(polls::Polls::withdraw_all); }); } From fac2352d84bf28b1f524f1c50dc20bf77a62b4dd Mon Sep 17 00:00:00 2001 From: japabu Date: Fri, 2 Oct 2026 21:15:41 +0200 Subject: [PATCH 14/14] Review round on #655: `HELD`'s doc drops the clause the header already carries The fifth review of #655 (issuecomment-5959593490) names the clause for removal: ": a hang ceiling, for a hang that ends when its peers do". With the kill's read reverted the hold ended with its peers still posting, and the file's header already says what the ceiling is for. A comment alone; no behaviour changes. The same review's BLOCKER is answered by a boot and by the pull request's body, not by the tree: the T14 booted `5f62645ae` with `k1-the-wait-loop-reads-no-kill.patch` applied as the `kill_ends_every_wait` row, and `posted-poll` went red there (issuecomment-5959632256). That commit's message says the four-thread child was seen running in every roster sample for 70 s. The samples cover the first 354 ms after its kill, in a guest whose run took 80 s; the pull request's hold table is the figure that stands. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_013UDZQ6fSKw14e4w2TKTRfm --- tests/toyos-rust-tests/src/bin/kill_ends_every_wait.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/toyos-rust-tests/src/bin/kill_ends_every_wait.rs b/tests/toyos-rust-tests/src/bin/kill_ends_every_wait.rs index e6863549674..13cd5d9c2d9 100644 --- a/tests/toyos-rust-tests/src/bin/kill_ends_every_wait.rs +++ b/tests/toyos-rust-tests/src/bin/kill_ends_every_wait.rs @@ -53,7 +53,7 @@ impl FromHandle for Posted { const POSTERS: usize = 4; /// How long those threads go on posting after the kill before they say the -/// child outlived it: a hang ceiling, for a hang that ends when its peers do. +/// child outlived it. const HELD: Duration = Duration::from_secs(1); /// `process::KILLED_EXIT_CODE`.