Skip to content

pipe: a write of at most MAX_WHOLE_PIPE_WRITE bytes lands whole - #977

Merged
Japabu merged 2 commits into
mainfrom
wt/toyos-pipebuf
Oct 21, 2026
Merged

Japabu merged 2 commits into
mainfrom
wt/toyos-pipebuf

Conversation

@Japabu

@Japabu Japabu commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Implements the owner's decision (Jan, 2026-10-21, claude-questions item 10): small pipe writes are indivisible as ordinary ToyOS pipe behaviour. Closes issues/a-pipe-write-of-a-few-bytes-can-tear-between-two-writers.md.

Problem

pipe::try_write wrote min(len, free space). Whenever the reader lagged, a 40-byte line could split, and another writer's bytes could land between its two parts. Pipe writers are separate processes that share no lock, so only the pipe can keep a write whole.

Resulting behaviour

  • The bound. A pipe write of at most MAX_WHOLE_PIPE_WRITE = 4096 bytes (toyos-abi) lands whole or waits until there is room for all of it. The non-blocking form lands whole or answers WouldBlock. Longer writes still land as far as they fit, as before. libc declares the bound as PIPE_BUF 4096 in limits.h, and a libc host test (pipe_buf_is_the_kernels_whole_pipe_write) holds the two equal. A compile-time assertion in kernel/src/pipe.rs keeps the bound within the ring (2 MiB less its header).
  • One write function. kernel::pipe::Landing (kernel/pure/pipe.rs) says how a write lands: Whole (a message), Pipe (whole up to the bound) or Partial (a connection's plain write). Its room(len) is the room the write waits for, and takes(space, len) is what it writes; both come from the same comparison. crate::pipe::write(id, buf, landing) replaces both try_write and write_whole. ipc: a frame and its handles travel as one message; the standalone handle send is retired #973's message send is now the same write with Landing::Whole, so the whole-write mechanism is reused rather than duplicated.
  • Size-aware waiting. sys_write waits on has_space(id, landing.room(len)) instead of one byte. A waiting writer wakes only when its write will land. Every read and the reader's departure still post the write watch, which wakes every waiter to check its own condition again. No new wake path exists, and none is missing.
  • Readiness. A pipe's write end polls writable only when MAX_WHOLE_PIPE_WRITE bytes are free. A non-blocking write that was refused and is retried on readiness therefore lands instead of spinning.
  • Empty writes wait for nothing. Holders::writable no longer raises the room it wants to one byte. A zero-byte write to a full pipe or connection answers 0 (or Gone with no reader) instead of parking.
  • Unchanged. Byte counts, Gone / ResourceExhausted errors, files, consoles and devices, and the connection's plain write (Landing::Partial) behave as before.

Decisions

  • Connections are excluded. The approval covers pipes, not sockets, so a connection's plain write stays partial and its writability stays one byte. Messages on a connection were already whole. Extending Landing::Pipe to connections is a one-line change if the owner wants it.
  • 4096 bytes. That is the common PIPE_BUF, above POSIX's 512 minimum. It is far below the 2 MiB ring, so a whole write always fits eventually, and readiness costs little.
  • No guest test (review r1 agrees). The behaviour lives in the pure Landing/Holders functions, which the host tests exercise. The kernel glue around them is three call sites, held by inspection and not by any test or control: sys_write passes room, ops::has_space passes the bound, and sys_write_nonblock already maps a refused write to WouldBlock. The QEMU smoke run shows the write path is not regressed. It does not establish that the size-aware wait or readiness behave as designed; those rest on inspection plus the host tests.
  • Declined review suggestions: S1 (Landing::ready_room()) would need a meaningless arm for Whole, which no object polls; S2 (renaming Landing::Pipe) is kept as is, since its doc states the behaviour.

Checks

Round 2 (head 1fcbfd9d2, rebased onto e74dbf183, after #975 and #976). Review r1's fixes: stale pipe::try_write names fixed in kernel/loom/tests/inbox_answer.rs and issues/the-global-pipe-lock-spans-a-user-copy.md; the PIPE_BUF check; and one clause in kernel/pure/pipe.rs's header.

check exit
cargo run -- --ci host 0 (88 steps)
cargo run -- --build-only (x86_64), --arch aarch64 0, 0
control c7: PIPE_BUF set to 512, cargo test -p toyos-libc-copies pipe_buf 101; restored 0

Round 1 at 584d4f246 (the pipe code is unchanged since, apart from a module comment, so its results carry over):

check exit
cargo run -- --ci host 0 (88 steps)
cargo run -- --build-only (x86_64), --arch aarch64 0, 0
cargo test -p kernel --lib --features sched-check pipe:: 0 (12 tests, 7 new)
QEMU x86_64 console_image_boots libc_children (stdio pipes, spawn_pipe_eof, spawn_pipe_reader_gone, popen) 0 (2/2)
negative controls c1–c6, each applied, built, run and reverted each builds (0) and reds (101); restored 0

New host tests in kernel/pure/pipe.rs model the pipe's write, wait condition and poll readiness over the real Landing and Holders:

  • whole-or-wait at every amount of room, with a long write still partial;
  • a refused non-blocking write is answered by readiness, and the retry lands;
  • empty writes;
  • the reader leaving answers a waiting writer, and the write is refused;
  • wake-iff-lands, checked exhaustively over room, length and landing;
  • three writers with a lagging reader: 1042 writes are refused for room, every record arrives intact and every writer finishes.

The same competing run under Partial tears, which is the test's own control.

Controls:

  • c1 is the old partial landing.
  • c2 is the old want.max(1).
  • c3 is a writable that ignores the reader.
  • c4 mutates Holders::writable to want at most one byte. Its effect equals the old one-byte wait (a busy wake), but it does not mutate the sys_write glue.
  • c5 wants one byte more than needed (a missed wake).
  • c6 mutates only the test model's poll, to one byte. It shows the test's sensitivity to a busy retry, not a product property.

No control can catch a revert of the io.rs room argument or the ops.rs bound; those are held by inspection. Red test names are in controls/summary.txt.

Evidence (masked): round 2 https://github.com/ToyOSOrg/evidence/tree/afbfa442ca959f2a73f339b8dfb6c62189410a3c/r2, round 1 https://github.com/ToyOSOrg/evidence/tree/afbfa442ca959f2a73f339b8dfb6c62189410a3c/r1

Risks

  • No order among waiting writers. A whole write of up to 4096 bytes can keep losing freed room to smaller writes that fit first. Starving it would require the ring to stay within 4 KiB of full under continuous competing writes. This is not measured, and POSIX promises no fairness here either.
  • Writability now needs 4 KiB free rather than one byte. A poller that would have written a few bytes into a nearly full pipe now waits until 4 KiB is free. No in-tree program relies on writing to a pipe that is almost full.
  • The glue is held by inspection only. The three kernel call sites have no test or control; the QEMU smoke run shows only that the write path is not regressed.

🤖 Generated with Claude Code

@Japabu

Japabu commented Oct 21, 2026

Copy link
Copy Markdown
Collaborator Author

Review r1: #977 at 584d4f2 (base b48aab8)

I read the commit, the full diff, kernel/pure/pipe.rs, kernel/src/pipe.rs, kernel/src/syscall/io.rs, kernel/src/syscall/ipc.rs (the message send's wait), kernel/src/object/{ops,service}.rs, kernel/src/watch.rs (wait_until and wait_inner), the inbox's readiness path, the in-tree nonblocking pipe writers, and the evidence at ToyOSOrg/evidence@3b90d11a/r1, including all six control patches. I ran nothing.

Architectural assessment

The design matches the owner's decision and the existing structure. The pipe that owns the ring is the only place that can keep a write whole, and the change is made there. Landing is a pure value whose room and takes come from one comparison. That one comparison drives the write, the blocking wait condition and, by inspection, the poll. The whole-write mechanism from #973 is reused rather than duplicated: write_whole and try_write become one pipe::write. No descriptor numbers, C policy or new wake path enter the kernel. MAX_WHOLE_PIPE_WRITE is a kernel/program contract, so toyos-abi is the right home for it. Connections keep Partial, which is consistent with "pipes, not ... sockets". By my count, production code grows by about 35 lines net, tests by about 260, and the closed issue file accounts for 46 deleted lines.

Message send is unchanged in behaviour. With Landing::Whole, room(len) = len and takes(space, len) = (space >= len).then(len). Together with the unchanged order (BrokenPipe, then NoMemory, then NoRoom) and end = pipe.written, this is exactly the old write_whole. On the IPC wait, has_space(tx, len) loses the old max(1). That never mattered for messages, because a zero-length whole write can never answer NoRoom, so the send could not park on it.

Connection plain writes change only as documented. A zero-byte write to a full ring now answers 0 instead of parking. The missing-pipe word changes from NotFound to Gone, but no caller can reach it while it holds an end.

Kernel glue, checked by inspection:

  • sys_write takes room from the same pipe_write(object) landing that try_write used, under the same process-lock hold.
  • wait_until registers before it re-reads has_space(id, room). A registration survives the loop, and a post made after registration refuses the park (wait_inner). So a woken writer that finds too little room re-parks without missing a later read.
  • sys_write_nonblock maps NoRoom (None) to WouldBlock. Nothing else changes there.
  • ops::has_space at 4096 makes every refused write of at most 4096 bytes, and every longer one (room 1), land when retried on readiness. Inbox polls look again at has_space after every post, so readiness is level-triggered and cannot hand out a writable answer that is already stale.

Wakeups. Every byte-consuming read (sys_read and sys_read_nonblock, including connection reads via try_read_within) posts the write watch unconditionally. The reader's last release posts it too. sys_handle_recv consumes no ring bytes. Waking every writer is required, because waiters want different amounts of room, and waking just one without an order could strand the waiter whose condition holds. Compared with the base, the herd is smaller. Before, every parked writer's 1-byte condition became true on any read, and each re-ran a full write attempt. Now a waiter whose room is still short re-checks in the kernel and parks again.

Behaviour changes.

  • An empty write no longer blocks. POSIX leaves a zero-byte write to a non-regular file unspecified, and common implementations return 0 at once. No Rust Write caller issues one, because write_all skips empty slices.
  • Writable now means 4096 bytes free. This matches POLLOUT's "a write [of at most PIPE_BUF] will not block", and it is what keeps the retry-on-readiness loop from spinning. The in-tree callers are fine. The 1-byte wake pipes (sdk/toyos/src/wake.rs, system/netstack/src/pipes.rs) still wait for 1 byte of room. logkeeper's PipeSink retries on WRITABLE, which now guarantees that the retry lands.

Fairness. The risk is real but bounded. It needs the reader to keep the 2 MiB ring within 4 KiB of full while small writers keep landing, and the small writers are making progress while it lasts. Common POSIX pipes share the property. The competing-writers model shows every writer finishing under seeded random scheduling, but that is not a measurement of fairness. I do not require an issue for it; see S3.

Findings

BLOCKER

  1. kernel/loom/tests/inbox_answer.rs:234: stale comment. It says "pipe::try_write answers Wrote(0)", but this change removes pipe::try_write, and PipeWrite::Wrote is now a struct variant. Comments that this change makes false must be fixed in the same change. Rewrite it as "pipe::write answers zero bytes", or drop the parenthetical. While there, issues/the-global-pipe-lock-spans-a-user-copy.md names the removed pipe::try_write in its title and mechanism (lines 7 and 16). The issue is otherwise unaffected, so only rename the identifier to pipe::write; this is not a cleanup of that file's older stale line numbers. issues/deferred-release-outlives-its-syscall.md:112 is a dated investigation narrative and can stay as written.

  2. sdk/libc/include/limits.h:30 and sdk/abi/src/syscall.rs:1069: nothing keeps PIPE_BUF equal to MAX_WHOLE_PIPE_WRITE. The owner's decision is that libc exposes the native bound. Two hand-copied 4096s can drift, and a C program trusting PIPE_BUF would then tear silently. The tree already guards the identical pairing for PATH_MAX: sdk/libc/arch/src/path_resolution.rs:198 has path_max_is_limits_h_s, which asserts MAX_PATH == header::int(LIMITS_H, "PATH_MAX"). Add the same one-line host test for PIPE_BUF. This is the cheapest sufficient check, and without it the limits.h claim is unheld.

SUGGESTION

S1. kernel/src/object/ops.rs:860-861: has_space hardcodes "PipeWrite means 4096, Connection means 1". That restates which landing each object has, which pipe_write already decides at :233-236. The test model's polls_writable restates it a third time. One option is a Landing::ready_room() (Pipe gives MAX_WHOLE_PIPE_WRITE, Partial gives 1), used by has_space through pipe_write(object) and by the model. Then the poll bound has one source, host-tested together with room. Today the check is inspection only.

S2. Landing::Pipe is named after its caller, while Whole and Partial are named after behaviour, and a connection is also a pipe internally. A behavioural name such as WholeUpToBound would read the same at service.rs and ops.rs. This is optional.

S3. Fairness: if the orchestrator wants the risk recorded durably rather than only in the PR body, file a short kind: finding with the condition stated above. I do not consider that required: the property is unmeasured, shared with POSIX pipes, and no in-tree workload is known to hit it.

S4. kernel/pure/pipe.rs:1 module header: it now also owns how a write lands, which "Who holds a pipe's two ends, and what a watch on one of them is told" does not mention. One clause would fix that. system/logkeeper/src/serve.rs:184, "one write takes what fits", is now loose for writes of at most 4096 bytes. It still satisfies Write, so changing it is optional.

Tests and evidence

  • The host tests exercise the real Landing and Holders through a model of the locked write. They cover whole-or-wait, a non-blocking retry answered by readiness, empty writes, the reader leaving, wake-iff-lands checked exhaustively over room, length and landing, and seeded competing writers under backpressure, with a Partial run as the test's own control. The tests are deterministic and focused, and together they establish the pure claims the owner listed.
  • Controls c1, c2, c3 and c5 mutate production Landing or Holders and go red as claimed. c4 mutates Holders::writable (want.min(1)), not the sys_write glue it is described as ("the old has_space(id, 1)"). Its effect is equivalent, but the label is loose. c6 mutates only the test model, so it shows the test's sensitivity, not any product property. No control or test can catch a revert of the io.rs room argument or the ops.rs bound. Those lines are held by inspection, which I accept for three one-line call sites over the generic, separately tested watch and inbox machinery. S1 would move the poll bound under a test.
  • Guest test: I do not require one. Beyond the glue there is no host-unestablishable claim that the watch and inbox tests do not already cover.
  • For the orchestrator, on the PR text: "The QEMU smoke run ... exercises that glue" overstates. libc_children reaches the size-aware wait only in spawn_pipe_reader_gone, and only when the child's 4096-byte writes beat the parent's close. That order is a race, so it is not established. The smoke run shows that the write path is not regressed, not that the wait or readiness behaves as designed.
  • The reused evidence is valid. ci-host and the two build-only runs used the 584d4f2 tree before the issue deletion, and the deletion is the only difference.
  • Closing the issue follows issues/README.md. The durable rule now lives in MAX_WHOLE_PIPE_WRITE's doc, and git grep finds no other citation of the slug.

SEND BACK

Japabu and others added 2 commits October 21, 2026 16:19
A pipe write took min(len, free space), so whenever the reader lagged a
40-byte line could split, and another writer's bytes could land between
its parts. Its writers are separate processes sharing no lock, so only
the pipe can keep a write whole.

Owner decision (Jan, 2026-10-21): up to a documented bound a pipe write
lands whole or waits for room for all of it; the non-blocking form lands
whole or answers WouldBlock; longer writes may stay partial; libc
exposes the bound as PIPE_BUF. Pipes only, not files, sockets or devices.

- toyos-abi: MAX_WHOLE_PIPE_WRITE = 4096, libc's PIPE_BUF. The kernel
  asserts at compile time that it fits the ring.
- kernel::pipe::Landing says how a write lands: Whole (a message), Pipe
  (whole up to the bound, as far as it fits past it) or Partial (a
  connection's plain write, unchanged). room(len) is what the write waits
  for and takes(space, len) what it writes, from one comparison.
- crate::pipe::write(id, buf, landing) replaces both try_write and
  write_whole: the message send is the same write with Landing::Whole.
- sys_write waits on has_space(id, landing.room(len)) instead of one
  byte, so a waiter wakes only when its write lands, and every read and
  the reader's leaving still post the write watch, which wakes every
  waiter to re-read its condition.
- A pipe end polls writable only with MAX_WHOLE_PIPE_WRITE bytes free,
  so a non-blocking write refused and retried on readiness lands rather
  than spinning.
- An empty write waits for nothing: Holders::writable no longer raises
  the wanted room to one byte, so a zero-byte write to a full pipe or
  connection answers 0 (or Gone without a reader) instead of parking.

Host tests in kernel/pure/pipe.rs model the pipe's write, wait condition
and poll readiness over the real Landing and Holders: whole-or-wait,
non-blocking refusal answered by readiness, empty writes, the reader
leaving, wake-iff-lands at every amount of room, and three writers with a
lagging reader whose records never tear (with the same run under Partial
as the test's own control).

Closes issues/a-pipe-write-of-a-few-bytes-can-tear-between-two-writers.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…names

- sdk/libc/arch: a host test asserts limits.h's PIPE_BUF equals
  MAX_WHOLE_PIPE_WRITE, as path_max_is_limits_h_s does for PATH_MAX.
- kernel/loom/tests/inbox_answer.rs and
  issues/the-global-pipe-lock-spans-a-user-copy.md name pipe::write, which
  replaced pipe::try_write.
- kernel/pure/pipe.rs's header says it also owns how a write lands.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu
Japabu marked this pull request as ready for review October 21, 2026 14:36
@Japabu
Japabu enabled auto-merge October 21, 2026 14:36
@Japabu
Japabu added this pull request to the merge queue Oct 21, 2026
Merged via the queue into main with commit 6f4d8db Oct 21, 2026
8 checks passed
@Japabu
Japabu deleted the wt/toyos-pipebuf branch October 21, 2026 15:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant