[Japabu]

Problem

Block I/O below the file protocol was synchronous at every layer: toyos_disk::Disk, diskserver's Served, and toyos-fat32's byte-addressed BlockAccess. The bcachefs engine and the node core need several requests in flight on one device, and FAT needs to be awaited rather than blocked on. This is stage B1 of the storage design: one crate for asynchronous block I/O, with every existing user moved onto it.

Resulting behaviour

• toyos-blockdev (lib/blockdev, no_std, no SDK):
  • BlockDev has blocks(), read(lba, &mut [Block]), write(lba, &[Block]) and flush(), all on &self. Because a transfer is [Block] ([u8; 4096]), a whole-block transfer is guaranteed by the type.
  • A single-threaded executor offers spawn, run_until_stalled and block_on. When no task can go on, block_on calls a wait the caller supplies; block_on_ready is for devices that answer as they are asked.
  • Bytes is a device over memory.
  • The crash model sits behind the model feature (details below).
• Crash model (model): a completed write stays in a volatile cache until a flush covers it. At a crash, the medium holds every flushed write plus any subset of the unflushed writes' atomic units. The unit comes from the device (a 512-byte LBA by default), never from the format. Small state sets are enumerated; large ones are sampled from a recorded seed, and a red run names that seed.
• toyos-fat32 runs over BlockDev, and every function that touches I/O is async. A private bridge (src/device.rs) turns a byte range into one whole-block request, reading any partial block before writing it. BlockAccess and IoError are gone.
• diskserver's Served is an async BlockDev over a blockring session:
  • An answer reaches the waiting task at progress(), or at wait() when it is block_on's wait. A session end is found there and reconnected.
  • A read on the wire at a session end is now asked again, as writes already were. Before, the read answered Gone at once.
  • A failed reconnect is now final. The old code tried again at the next call, which is unsafe once requests are queued for the next session: they would go out under requests asked since.
  • A request refused across more than MAX_RECONNECTS ends answers Gone, as before.
  • Session::run/read/write/flush and its answer stash are deleted.
• fileserver's Cache is an async BlockDev over its disk. Several tasks may use it at once, and it holds no borrow across a disk request. The disk never has two requests for one block in flight when either is a write, which the block service's client forbids:
  • One flush runs at a time. A second waits for it, then takes its own dirty set.
  • A flush writes no block that a fetch is reading. A fetch reads only blocks the cache does not hold, and a flush's blocks stay held while it writes them.
  • A block written while a flush carries an older copy of it stays dirty (each slot has a version).
  • The FAT and DATA volumes stay synchronous at the Volume boundary until C2. They drive each request with Cache::block_on, which waits on the disk; bcachefs's synchronous BlockIO does the same.
  • fileserver's Disk: BlockDev trait names that wait. Only Served waits; Ram and the test devices never do.
• update's install is async and generic over BlockDev. It runs under block_on over its three Served disks; it asks one request at a time, so the wait is the pending disk's.
  • Its held volume (Cached) is a holding BlockDev. A held block is read from the hold and never from the partition, which matters because fat32 reads a block before it writes part of one. Its flush now writes the held runs and then flushes the partition, as the trait requires, so the volume's sync makes it durable and the install's separate boot flush is gone.
• The supervisor reads the slot table through Served with block_on. lib/update's slots::read and NotRead took a synchronous fill and had only these two callers; both now read the two copies and call slots::current.
• The build (image.rs, fatformat.rs) and imgstat read and write FAT through Bytes and block_on_ready.
• toyos-disk (lib/disk) is deleted.

Decisions

The orchestrator accepted these four:

1. tests/metal and lib/fat32/check are not toyos-fat32 consumers: they depend only on the checker, a pure byte judge. They are unchanged, and so the checker is untouched. The supervisor's slot read does use Served, so it moves here.
2. Transfers are typed &[Block] / &mut [Block]. An arena-backed buffer is decided when W2 or C2 needs one.
3. fat32's test devices changed shape, because the device under fat32 now takes only whole blocks and the old kernel-adapter cache model (AdapterCache) no longer exists. Every test's claim is unchanged:
   • Refusal plans now see block writes and pick their targets by the bytes they change. active_fat covers only the blocks FAT 0 holds alone.
   • The rollback test counts per changed FAT word.
   • The free-refusal test refuses writes that change FAT bytes.
   • The "one sector short" boundary became "one block short".
4. The two Served behaviour changes above.

Also in this PR:

• Two fileserver-local bridge tests (a refused read is never zeros; a partial write over an unreadable block writes nothing) moved into toyos-fat32's src/device.rs, where that bridge now lives. One test was added for writes across part blocks.
• toyos-blockdev's model feature joins NOT_A_CONTROL in build/build.rs, because it is a capability rather than a negative control. The --all-targets clippy shape compiles the model under cfg(test), so no separate clippy shape is needed.
• await_holding_refcell_ref needs no build/clippy.rs change, but clippy only enforces it on host crates.
  • It is on by default: a borrow held across an await in Cache::flush is refused by plain cargo clippy -- -D warnings (control below). The gate's clippy therefore covers toyos-blockdev and toyos-fat32.
  • The gate does not lint fileserver or diskserver, which already carry unrelated clippy findings on main. update's install is linted, through update-copies. The gap is recorded in issues/the-storage-programs-are-never-linted.md, with its evidence, an owner (the storage track, before C2) and an exit.

Checks

Review round 1 (head f863887, rebased onto 777c8c5)

Evidence: r2/.

• cargo run -- --ci host: exit 0, 101 of 101 steps green (log).
• cargo run -- --build-only: exit 0 (log).
• QEMU x86-64, the_log_lands_on_the_boot_stick and nvme_disk_boots: 2 of 2 pass (log). These were rerun because the cache changed.
• New tests:
  • two flushes with a write between them;
  • a flush while a fetch of its block is out;
  • a held block never read from the partition;
  • the install's flush order.
• Controls, each red on its own claim:
  • no flush serialisation;
  • no wait for fetches;
  • the version ignored;
  • round 0's held read, which made 5654 partition reads for a 256 KiB file, 5644 of them for held blocks (the fix makes 10);
  • Cached::flush without the partition's flush.

Round 0 (head fe3ecb5)

• cargo run -- --ci host at c8ef90a (log): 92 of 93 steps green. The one red was build::tests::every_model_control_is_run, which counted the new model feature as an undeclared control. After the NOT_A_CONTROL entry, cargo test -p toyos-build --lib passed (367 tests, log).
  • toyos-blockdev: 11 tests.
  • toyos-fat32: lib 21, host_read 10, host_write 21, hostile 33, refused_writes 20, replace_rename 2.
  • toyos-fat32-check: untouched.
  • toyos-update: 52. update-copies: 3.
  • diskserver: 6 Served tests. fileserver: 70.
  • The clippy step is clean.
• cargo run -- --ci host at the final commit fe3ecb5: exit 0, all 93 steps green (log).
• cargo run -- --build-only exit 0 at 8a8e15c (log). Its tree equals fe3ecb5's. The first attempt (log) caught a temporary stdin lock in update's main.rs.
• QEMU x86-64: cargo test --test toyos-build -- the_log_lands_on_the_boot_stick nvme_disk_boots, 2 of 2 pass at 8a8e15c (log).
  • nvme_disk_boots serves LOG, BOOT and DATA off the NVMe driver's sessions.
  • the_log_lands_on_the_boot_stick writes the boot's log through fileserver's FAT over Served and usbd. Its 1.47 MB is read back off the image on the host.
  • These are the guest paths through the new Served::wait over a real session, which no host test reaches.
• Negative controls (patches and logs, table), each applied as a checked patch, failing on its own claim, then restored:
  • Cache, a block's version ignored at flush: red on a_block_written_while_a_flush_carries_its_older_copy_stays_dirty only.
  • Cache, overtaken ignored: red on a_fetch_a_write_overtook_is_not_kept only.
  • Cache, a RefCell borrow held across disk.flush().await: clippy refuses it (await_holding_refcell_ref).
  • Served, a refused request not asked again: red on the three re-ask/gone tests.
  • Served, a failed reconnect not made final: red on a_refused_reconnect_makes_the_disk_gone_for_every_request only.
• The crash model's own control is red and green in its tests, as is the torn write at one LBA (any subset of 8 sectors with 512-byte units; only none or all with 4096-byte units).

Metal: usbd_serves_the_boot_stick on the T14 is prepared with --metal-readback, and the run is requested from the orchestrator. It rides the shared testcases boot, and its judge reads the log fileserver wrote over Served off the stick. QEMU already establishes order and contents on this path; the metal row confirms the same path over the T14's own xHC and stick.

Risks

• No guest test runs update's install (nor the supervisor's slot grant). It is host-tested over Bytes. The only guest-only part is main.rs's choice of which disk to wait on.
• fileserver still blocks per request at the Volume boundary until C2. Throughput and latency are as before: one request at a time.

Evidence: ToyOSOrg/evidence@0e64fb2, b1/.

🤖 Generated with Claude Code

https://claude.ai/code/session_01PXEk8BBMDT8tXu4AnENB9Q

[Japabu]

Review of #1025, round 1 (head fe3ecb5, base 18a3873)

Architecture

The layering matches design §2.4 and B1's row. toyos-blockdev is a pure no_std leaf, and it owns the one shared subject: the device trait, the executor, the memory device and the crash model. Served stays in diskserver's client library, where the SDK is. fat32's byte-to-block bridge is private to fat32 (src/device.rs). The superseded paths are gone: lib/disk; BlockAccess/IoError; fileserver's FAT Bytes adapter; the build's VolumeIo; imgstat's Volume; Session::run/read/write/flush with its stash; and slots::read/NotRead. No live reference to any of them remains outside history. The kernel does not appear in this change. Typing transfers as &[Block] makes whole-block transfers a type fact (accepted note 2).

The crash model meets §3.4 for B1:

• the unit comes from the device and never from the format;
• small state sets are enumerated, and large ones are sampled from a recorded seed;
• the control is red and green, and the torn write at one LBA tears at 512-byte units and only there.

Accepted note 4 (finality) is consistent with the client's contract. Once gone wakes every waiter with Gone, including waiters whose requests the client still holds unsent for the next session, those requests must never go out. A final gone is what guarantees that.

Size, approximately, from numstat:

• Production: about +400 lines net. About 220 of them are the crash model, which is test support behind model. The rest are the executor and trait (about 280), Served's async state (about +130), and the cache's version and fetch bookkeeping (about +170). Deletions offset them: lib/disk, the three byte adapters, the session's synchronous calls and slots::read, about −400 together.
• Tests: about +350 net. That is 11 blockdev tests, 6 new Served tests (it had none) and 2 cache tests. fat32's common shrank by about 200 lines when AdapterCache went.

These amounts are proportionate to the stage.

Findings

BLOCKER 1: the cache's concurrency claim is false under two flushes

system/fileserver/src/cache.rs:14-17, 222-263

The header says "Several tasks may use it at once", and the PR presents the version and overtake checks as mechanism for C2. Two concurrent flushes break it. That happens with a sync beside a write over DIRTY_LIMIT (write calls flush, line 195), or with any two C2 tasks syncing.

1. Two overlapping writes go on the wire. Flush A snapshots block X dirty and sends X at v1. A task writes X at v2. Flush B snapshots X, which is still dirty because A has not completed, and sends X at v2 while A's write is in flight. That violates toyos_blockring::client's rule that "A caller must not have two in flight whose ranges overlap when either is a write". The rule matters because requests in flight together are unordered (lib/blockdev/src/lib.rs:11-13). The device may land v2 and then v1.
   • A completes and leaves X dirty, since v1 ≠ v2. B completes and cleans X, since v2 matches.
   • The cache now holds X clean at v2, and the disk may hold v1. B's disk.flush() then reports it durable. After an eviction, a read returns v1. That is a lost acknowledged write that a flush called durable.
2. The cache can panic. At line 238, &inner.slots[&(first + k)] indexes a slot from B's snapshot. While B awaits an earlier run, A can clean X and an evict() can drop it: A's own one at line 261, or any visit's at line 170. B's later run then panics on a missing key.
3. Reads overlap writes too. A fetch in flight for an absent block can overlap a flush's write of the same block once a write has made it dirty. That is also an overlapping pair the client forbids. overtaken keeps the result out of the cache, but it does not make the pair legal.

B1's callers are one task at a time, so none of this happens today. The module still claims it, though, and C2 is told it can rely on it.

Fix, one of:

• (a) Make it true. Allow one flush at a time, so a second flush awaits the first and then takes its own snapshot. Issue no disk write for a block that has a fetch in flight, or the reverse. Skip a snapshotted block that is no longer present and dirty. The cheapest evidence is a Held test: two flushes spawned with a write between them end with the newest copy on the disk and no panic, red without the serialisation.
• (b) Narrow the claim to one task at a time, with an assertion. Then delete the version and overtake mechanism until C2 designs its concurrency.

BLOCKER 2: update's held volume now reads the partition for every block it already holds

system/update/src/install/volume.rs:36-42 (and the header at :1-8)

Cached::read always asks the disk for the whole range and only then overlays held blocks. The old Cached::block served a held block locally and read the disk only for blocks it did not hold.

fat32 now does every partial write as read-modify-write through this device (lib/fat32/src/device.rs:44-61). So every 4-byte FAT update costs two disk reads per FAT copy:

• the write_fat_entry read (fat.rs:55);
• the bridge's read under the partial write.

Each alloc_cluster adds its FAT-sector read on top. A slot volume has 512-byte clusters (build/fatformat.rs:82, volumes ≤ 260 MiB). Writing about 16 MiB of slot files therefore costs on the order of 10 Served reads per 512-byte cluster, roughly 300 000 round trips through diskserver to NVMe. That is an estimate from reading the code; nothing measured it. The old code served nearly all of these from held blocks. No guest test runs the install (PR "Risks"), so nothing would see the regression.

The header's premise is also false now. It says holding avoids "a read and a write of one block per 512 bytes"; under the new read path it avoids only the write.

Fix: read from the disk only the blocks that are not held, or skip the disk entirely when the whole range is held. One host test covers it: a counting disk under Cached shows that writing a multi-cluster file issues no disk read for an already-held block.

BLOCKER 3: the design's await_holding_refcell_ref requirement is unmet for three programs, and nothing records it

Design §2.4 (S3) requires this lint for the crates that hold RefCells across the new awaits. The PR shows correctly that the lint is on by default, so toyos-blockdev and toyos-fat32 are covered. But the gate does not lint fileserver, diskserver or update, and those hold most of the RefCell-plus-await code (Cache, Served, Cached).

The handoff says this "may deserve an issue" and leaves it unrecorded. issues/userland-programs-are-never-linted.md does not cover these programs: its exit names the crates build/userlandhost.rs's survey gates, and all three carry exempt manifests.

That makes this a discovered compromise of an approved requirement, so it belongs in issues/ (root: "No silent debt"). The fix is cheap: extend that issue, or open a short one. Give the evidence (the mut-refcell control, and the existing findings that keep these programs out of the gate), an owner and an exit (a clippy shape covering these three host-built programs).

SUGGESTION 1: a host test for the install's flush order

system/update/copies/src/installing.rs

The install stays crash-safe by inspection:

1. ROOT is written, then the boot volume.
2. fs.sync makes the boot volume durable: Cached::flush writes the held runs and then calls disk.flush.
3. root.disk.flush() runs.
4. The table copy is written, then flushed.

The boot partition's flush now lives inside Cached::flush (install/mod.rs:115, volume.rs:71), not in a visible line of install. Every test device's flush is a no-op. A regression there would therefore land unseen and break crash safety only on metal. A device in installing.rs that logs (partition, write or flush) could assert three things: each partition's flush comes after its last write, both come before the table write, and the table's flush comes last. That test is cheap.

SUGGESTION 2: stale and minor text

• system/update/Cargo.toml:17: "written with the driver the kernel mounts FAT with" is stale. The kernel mounts no FAT, and this PR removed the same claim from lib/fat32/Cargo.toml. The PR edits the adjacent line.
• lib/fat32/src/error.rs:1 starts with an empty line left by the removed use.
• build/bin/imgstat.rs:52: bytes.clone() can move once the FAT attempt owns its own copy. This one is cosmetic.

Evidence

• --ci host at fe3ecb5 exits 0 with 93 of 93 green (log inspected).
• --build-only at 8a8e15c exits 0. Its tree equals fe3ecb5 per the evidence README.
• QEMU x86-64 passed the_log_lands_on_the_boot_stick and nvme_disk_boots at 8a8e15c.
• Each of the five controls was red on its own claim. The Served controls bite on re-ask and finality. The cache controls bite on the single-flush version and overtake paths.

None of the tests or controls exercises two concurrent flushes, which is BLOCKER 1. None exercises the held-block read path, which is BLOCKER 2. The T14 row usbd_serves_the_boot_stick is pending, as briefed. Fixing BLOCKER 2 changes a path no guest test reaches, so the host checks suffice for it. Fixing BLOCKER 1 changes fileserver's cache, so the two QEMU rows should be rerun or their reuse justified.

SEND BACK

[Japabu]

Round 1 response. The head is f863887, rebased onto 777c8c5; the only conflict was stream_root against #1019's toyos_crypto hasher. The fixes are in their own commit. Evidence: https://github.com/ToyOSOrg/evidence/blob/92188548fa16a5e3102103116ba97c768a2accc3/r2/README.md

BLOCKER 1, fixed with option (a).

• One flush at a time. A second waits for the first, then takes its own snapshot. The release is a drop guard, so an error or an abandoned flush also releases.
• Before each run, a flush waits until no fetch overlaps it.
• A fetch reads only blocks the cache does not hold. A flush's blocks stay present and dirty until that same flush cleans them, since only a flush cleans and flushes are serial. So no fetch starts under a flush's write, and the line-238 indexing can't miss: it is now an expect naming that invariant.
• This also made overtaken dead. A block that a write dirtied can leave the cache only through a flush, and that flush waits for the fetch. I deleted it.
• Tests use Held, which now counts every request sent while one for an overlapping block is in flight with a write:
  • a_flush_asked_during_another_waits_and_then_writes_the_newer_copy
  • a_flush_waits_for_a_fetch_of_its_block_and_the_write_is_kept
• Controls: removing the flush wait reds the first test only; removing the fetch wait reds the second only. The version control still bites.

BLOCKER 2, fixed.

• Cached::read serves held blocks from the hold and reads only the runs it doesn't hold.
• a_held_block_is_never_read_from_the_partition puts a recorder above Cached and a checker below it, writes a 256 KiB file, and fails on any partition read of a held block.
• Round 0's read path is the control: 5644 of 5654 partition reads were of held blocks. The fixed path makes 10.

BLOCKER 3, recorded in issues/the-storage-programs-are-never-linted.md.

• Evidence: the mut-refcell control, plus the 14 fileserver and 1 diskserver findings. All are on lines older than this PR, which I checked with blame.
• Owner: the storage track, before C2.
• Exit: a build/clippy.rs shape that lints these programs, with a planted borrow-across-await going red.
• One correction to the finding: update's install, Cached included, is linted already, because update-copies compiles it under --all-targets. Only its main.rs isn't.

SUGGESTION 1, taken. the_mark_moves_only_once_the_slot_is_durable checks that each partition's flush follows its last write, that both come before the table's single write, and that the table's flush comes last. Removing the partition flush from Cached::flush reds it.

SUGGESTION 2, taken. I fixed update's Cargo comment, removed the blank first line of error.rs, and replaced imgstat's clone with a borrow.

Reruns at f863887: --ci host 101 of 101 green, --build-only exit 0, and both QEMU rows pass.

On the queued T14 row (fe3ecb5): in B1, every Cache call is driven one at a time by block_on. The new waits are therefore always ready at once, with no flush running and no fetch out, and the LOG path makes the same disk requests. That is my argument for reusing the fe3ecb5 result for this head; you decide whether it holds.

[Japabu]

Review of #1025, round 2 (head f863887, base 777c8c5)

Scope: round 1's three blockers, plus the new code in f863887 (+364/−61: about +190 production across cache.rs and volume.rs, +150 tests, and a 48-line issue). I also compared 40526d4 with the reviewed fe3ecb5 using git range-diff. The rebase changed only the stream_root hasher (#1019's toyos_crypto) and the Cargo.lock lines; nothing else moved. The branch merges cleanly into current origin/main.

Architecture. Option (a) is the right fix. It uses one flushing flag and one waker list, and the overtaken mark is gone. The fix replaces the old mechanism rather than adding to it, and the cache's concurrency claim is now true.

Round 1 blockers

B1 (two concurrent flushes): resolved. I traced the invariants in system/fileserver/src/cache.rs:

• until(|i| !i.flushing) and flushing = true have no await between them. The executor is single-threaded, so only one flush can take the flag. Every woken waiter checks the condition again.
• visit creates a fetch only for blocks the cache does not hold (:138-151).
  • A block that a flush snapshotted is dirty, and only a flush cleans a block, so it stays in the cache. That makes both expects (:278, :289) true invariants.
  • A fetch that was already in flight before a write dirtied its block is waited out at :270-272.
  • So no read and write of the same block are in flight together, and no two writes are.
• wake() runs before read? (:157-158). A failed fetch therefore still releases a waiting flush. The Flushing guard releases the flag on every exit path.
• Removing overtaken is sound. A fetched block is installed only when no slot exists. Any write while the fetch was out creates a slot, and that slot then wins.

The two Held tests check the property that matters: the number of overlapping requests in flight with a write. Each of the controls (mut-flush-concurrent, mut-no-fetch-wait) is red on its own test only (logs inspected).

B2 (held volume reads held blocks): resolved. system/update/src/install/volume.rs:38-60 reads only the runs it does not hold, and the run logic is correct. a_held_block_is_never_read_from_the_partition puts a recorder above the volume and a checker below it, and fails on any partition read of a held block. The mut-read-held control (round 0's read) is red with "5644 of 5654 partition reads were of held blocks". The new header states the premise correctly.

B3 (lint gap unrecorded): resolved.

• issues/the-storage-programs-are-never-linted.md has the evidence, owner and exit, and its frontmatter is valid.
• I checked its claims at the head: build/clippy.rs:29,276 excludes hostws::guest_packages, and system/update/copies is a workspace member (Cargo.toml:136). The correction about update-copies therefore holds.

S1 and S2 were taken. The flush-order test asserts the right order, and its control reds it.

T14 reuse

The argument for reusing fe3ecb5's usbd_serves_the_boot_stick pass (agents/storage/b1/metal/judge-b.log: 1 passed, 1 boot) holds.

• On that path, fileserver drives each Cache call to its end through Cache::block_on. The flush inside write is nested in the same task, not concurrent with it.
• So when either until runs, no other flush holds the flag and no fetch is out. Both return Ready on their first poll and register no waker.
• overtaken could only be set by a write during an outstanding fetch. That cannot happen sequentially, so removing it changes no installed block.
• The device sees exactly the same sequence of requests.
• The other changes since fe3ecb5 are not on the LOG path. They are update's Cached, the install's hasher, imgstat and a blank line in fat32/error.rs.

Inspection settles this, and the QEMU rows at f863887 (the_log_lands_on_the_boot_stick, nvme_disk_boots, exit 0) confirm the guest path still works.

Findings

SUGGESTION 1 system/fileserver/src/cache.rs:200-202, 156: dropping a future in the middle of a request breaks the new invariants.

• A visit dropped while its read is out leaves its Fetch record forever. Every later flush that overlaps it then waits in until with nothing to wake it, and nothing reports the stall.
• A flush dropped mid-write releases flushing through the guard, even though Served keeps the request on the wire. A second flush can then send an overlapping write.
• B1 never drops these futures, so this is not a defect today. But the guard's "or is abandoned" suggests abandonment is handled.
• Before C2, either state the contract in the header ("a request's future is driven to its end"), or make abandonment safe: a drop guard on the fetch record, and keep flushing until the write lands.

SUGGESTION 2 system/update/src/install/volume.rs:3,10,45: two smaller points.

• The header states twice that held blocks are not read from the partition: once in its first sentence and again as the last clause of its second paragraph. One copy is enough.
• for (k, out) in out.iter_mut() shadows the slice it iterates. A name like slot reads more easily.

Evidence

• --ci host at f863887: exit 0, 101 of 101 steps green.
• --build-only at f863887: exit 0.
• QEMU x86-64, both rows at f863887: exit 0, 2 of 2 pass.
• Five controls, each red only on its claim (logs and patches inspected).

These cover every new claim.

For the orchestrator: evidence/r2/ci-host-f863887.log and the b1 logs contain macOS per-user temp paths (/private/var/folders/<hash>/T/...). That directory name is specific to the user and the machine, so mask it the way home paths already are. It does not block the code.

LAND