[Japabu]

Problem

On the T14, files the DATA file server serves (/home, in memory) are 20–430× slower than Linux's /dev/shm (osbench fs.served.*, PR #1053's tables): a stat 59.7 µs, an open and close 95 µs, a 1,000-entry listing 76 ms, a rename 292 µs.

Where the time went

Measured on the host, against DATA over memory, with each osbench files row's volume calls in the order src/main.rs makes them and no IPC (harness, before, sampler call graphs). Nearly all of it was the format's tree:

• Searches. The DATA volume searched twice per stat (file_extents, then file_mtime), once more for each path component the resolver looks up, and twice per file in a listing. A stat cost 8 node reads; a 1,000-file listing cost 4,000.
• The checksum. Every node read and every node write ran a CRC-32C one byte at a time from a table, about five cycles a byte. That loop is most of the samples in both reads and writes.

The IPC round trips explain what is left.

What changes

1. DATA's name index holds what each entry records, its kind, length and time (system/fileserver/src/data.rs).
   • It is updated wherever an entry changes: persist, truncate, create, rename, symlink and removal. A directory's rename moves each record as it stands after the entries of its open files are written.
   • It is read whole at mount from the format's one listing walk. The interim Mounted::list now also answers each entry's symlink flag and time (bcachefs::Recorded), which also drops the mount's search per name.
   • A stat, a resolver lookup and a listing are map queries; an open searches once, for the extents.
   • A listing skips each child directory's subtree instead of walking every name beneath it.
2. CLOSE is answered with nothing (sdk/toyos/src/fs.rs). A close costs its client a send, not a round trip; the client already ignored the answer.
   • A refused close keeps its node for the next sync, and the volume says so, as before.
   • Order on one connection is unchanged.
3. The serve loop watches again only what fired. A watch replaces its handle's poll by a pass over every poll the ring holds (Polls::admit), so re-watching every client per wait cost the kernel O(clients²) per request.
4. CRC-32C is the CPU's instruction (lib/bcachefs/src/crc32c/): SSE4.2's CRC32 on x86-64 and FEAT_CRC32's CRC32C on AArch64, through core::arch under target_feature, in one architecture file each.
   • There is no other implementation.
   • Every supported machine has the instruction: x86-64 parts of 2020 on, and AArch64 from Armv8.1, where FEAT_CRC32 is mandatory.
   • So do the test hosts and the QEMU models: build/arch.rs gives TCG's x86-64 guest +sse4.2, and the aarch64 models are max and host.
   • Any other architecture fails the build by name.
   • Who benefits: every checksum the crate takes, which includes the engine's node, journal, superblock and extent checksums. So ROOT's reads, the loader's and SYSTEM's alike, benefit, and so does spawn: a ROOT program is spawned from SYSTEM's object for it, not a copy #1058's verify pass at a program's first spawn, not only DATA.

Host, release, volume calls only (before; after changes 1–3 only, log):

row	before	after 1–3
stat	181,810 ns	673 ns
open_close	298,944 ns	27,390 ns
readdir.1000	128.8 ms	0.19 ms
create_write_unlink	701 µs	259 µs
rename	419 µs	308 µs
mkdir_rmdir	305 µs	316 µs

The CRC itself, timed directly on the aarch64 host, interleaved in one run, per 4,088-byte node (log, source): the old table 12.4 µs, the instruction 0.65 µs. The whole-profile rerun with the instruction (log) ran at host load 11–14, and its rows that take no checksum (stat, readdir) came out slower than at the earlier run. So it is not evidence of the mutation rows' gain; the T14 A/B is.

Checks and evidence

• Host CI. cargo run -- --ci host at a0806ca07 (the head): exit 0, 103 steps (log); at a0aba9cc8 also exit 0 (log). Earlier, at 04ec3f6f4, also green (log). cargo run -- --build-only at b0c014009: exit 0 (log).
• CRC tests. Both instructions are held to a bit-at-a-time reference. The cases are RFC 3720's examples (Appendix B.4), every length to 24 bytes at every word alignment, and whole nodes, with the uninverted register included. AArch64 ran natively; x86-64 ran as x86_64-apple-darwin under Rosetta, which has SSE4.2 (log). Every target that links the crate compiles the intrinsics with no warning (probe). In the built x86-64 loader, every crc32 is in one function of 62 instructions with no %xmm/%ymm operand; the built aarch64 loader contains no CRC32C encoding, as no CRC is reachable there (command, output and scan).
• Data-index tests (data.rs). One is a differential: every closed name's index record against the format's own entry, under a clock that advances, across writes, a truncate, renames (one of a directory holding an open, written file), an unlink and a remount. The other is a listing whose children sort around /.
• Negative controls. Seven mutations, each red and each restored in its own run (patches and logs): the index not updated by persist or by truncate; a directory rename moving the records collected before its persists (m8); no dedup of a child that has its own entry; no subtree skip; the x86-64 byte tail dropped; the AArch64 words read big-endian.
• QEMU x86-64 and AArch64 at fcd809961 (log): virt_jobs_at_el2, virt_el1_smp, stream_holders, a_launch_gives_each_unsent_stdio_slot_a_null, a_row_reaches_its_own_state_and_no_other and session_lifecycle passed. netstack_holders_exported_to_children stalled; see below. Earlier, at 04ec3f6f4: libc_children too (logs).

The session_lifecycle stall

It stalled once, at 04ec3f6f4 with the host at 5.82×. Asked whether the new loop can lose a client's wakeup, I checked it against the kernel's poll semantics (kernel/src/inbox/polls.rs, mod.rs):

• A poll nothing replaces stays standing across submits, and only a fired poll is looked at.
• A watch looks at its object when it is made, and again after registering, so readiness already there is answered with no post.
• The server marks a handle disarmed only when its token is delivered, re-watches it before the next wait while the client lives, and reads one frame per answer. FrameRx reads no byte past a frame, so a second queued frame keeps the handle ready.
• Gone clients' ids are never reused, so a stale token is ignored. A reused handle number is replaced by the new client's first watch.
• CLOSE going one-way reorders nothing within a connection.

The loom case that was here tested its own model of arm rather than the kernel's and is removed (review r1); the argument above stands on reading polls.rs and mod.rs, which the reviewer repeated independently.

Since that stall, session_lifecycle has passed 9 times at the head: six alone, at 2.45× to 7.03× load (summary), and three beside other tests. The mechanism this change could have introduced is ruled out; the one stall itself is not explained.

The netstack_holders_exported_to_children stall (independent, recorded)

It stalls at the step where the parent and a second child race on exported sockets, after the racer's spawn has completed. That step touches no served file. It reproduces on main: 1 of 6 runs at 32046264b (summary), against 2 of 6 at the head (logs). It is recorded as issues/two-holders-racing-on-an-exported-socket-can-both-stall.md.

T14

Run by the coordinator at a0aba9cc8: all 4 head boots exit 0, the judge 14 passed and 0 failed. The base is #1061's two osbench runs at the merge base 32046264b. Served ratios, head against base:

row	head / base
readdir.1000	0.012 (76 ms → 0.90 ms)
open_close	0.25
stat	0.30 (62 → 18 µs)
rename	0.40
tree.100	0.44
mkdir_rmdir	0.45
create_write_unlink	0.46
read/write_4k_at	about 0.95

The 4 KiB sequential rows are within the base's spread. mem.anon.map read 1.48× in the one head run. Nothing here touches anonymous mapping: no kernel code changed, and the file server's resident memory grows by the index's 23 more bytes a name. #1061 reads that row flat at the same base.

Since a0aba9cc8, review r1's fixes change no measured path's cost. The rename fix changes behaviour the testcases members cover, so only that image was booted again at a0806ca07: T14, 1 boot, rc=0, the orchestrator's judge 11 passed, 0 failed (request, words).

Unresolved

• The wire carries no version, so a file server swapped by swap across this change leaves clients built with the old CLOSE waiting for a reply. Every earlier wire change carried the same exposure.
• What is left of the served cost is round trips. read_4k_at and write_4k_at are a seek and a transfer each, and seq 4 KiB is one round trip a call. Mapping file pages from the server's cache is the owner's decision, taken there.
• Mounted::is_symlink and file_mtime in the interim format now have only its own tests as callers. They stay until C3 removes the format.
• For the IPC-latency work, passed on separately: Polls::admit scans every poll for each watch, and diskserver re-watches every connection on each wait.

🤖 Generated with Claude Code

[Japabu]

Review r1 of #1060 at a0aba9cc8 (base 32046264b)

Architecture

The ownership is right. The DATA volume already owned the in-memory name index; making it carry the entry's kind, length and time puts the cache with the one component that writes every entry. The format crate only gains Recorded on its existing listing walk. A one-way CLOSE is a wire simplification, and the SDK ignored the reply anyway. Re-watching only what fired matches polls.rs, where a poll stays kept until it fires. I checked admit, renew and settle, and arm's readiness check in inbox/mod.rs. Gone clients' tokens are never reused, and a reused handle number is replaced by the new client's first watch. FrameRx leaves a second frame in the kernel, so the handle stays ready. I found no lost wake. CRC-32C on the CPU instruction, with no fallback, is in scope for the supported machines. The QEMU models carry the instruction too (build/arch.rs: +sse4.2, max and host). The kernel does not link bcachefs. The loader, the fileserver and the build host do, and clippy covers both loader targets. Net: +450/−151, about 170 of the added lines tests. The listing skip is correct: every name under p/c/ sorts before p/c0, and nothing else lies between them.

Findings

BLOCKER: system/fileserver/src/data.rs:744-771. A directory rename leaves stale index entries for open, dirty files beneath it. moving copies each Meta out of names before the loop at 751-755 persists the open nodes. persist updates names[old path], and then 768-771 insert the stale copy under the new path. The node is no longer dirty, so its later close or sync never updates the index again. The divergence lasts until a remount. Reproduction by reading the code: open home/sub/f, write 5000 bytes, rename home/sub → home/t, close. lstat("home/t/f").size then answers 0 while the entry holds 5000. A reopen also pairs the entry's size with the index's stale mtime (open, 449-452). Fix: persist before collecting moving, or reread the metas after persisting. The PR's claim that "it is updated wherever an entry changes" is false until then.

BLOCKER: system/fileserver/src/data.rs:1025-1053, 1080. The index differential test misses this and cannot see mtime. Every renamed file in the scenario is closed before its rename. nanosecond_clock is a constant, so a path that leaves a stale mtime (set_mtime→persist, a write's time) passes as long as the size matches. Add an open, written file under a directory being renamed, checked after its close and against the remount comparison. Use a clock that advances, for example an atomic counter behind the fn() -> u64. The existing remount check after == before already catches the bug once the scenario reaches it.

BLOCKER: system/fileserver/src/main.rs:466-471. The acceptor's second bound check is dead, and its comment is false. A waiting client is created only by accept(), and accept() runs only when the acceptor fires. The acceptor is armed only below MAX_HANDSHAKES. Exported clients are not waiting(), and drops only lower the count. So takes_handshakes() is always true when the token is delivered, and "which may be met since" never happens. Delete the check and the comment. The PR description also presents the check as a change ("now also checked when the acceptor fires").

BLOCKER: kernel/loom/tests/inbox_answer.rs:333-363. The new loom case tests the test's own fake, not the kernel, and its negative control mutates that fake. Its distinguishing assertion depends on Kernel::arm in the test file, which fires on bytes at watch time. That is the property the m7 control removes. The kernel's real check (inbox/mod.rs arm, readiness_of) never runs. The PR says so: "read rather than ran". The polls.rs behaviour it does exercise, a poll left standing answering later, is a_poll_left_standing_still_answers. Root policy: no test for what inspection establishes. The lost-wake argument holds by reading polls.rs and mod.rs, which I did independently. Delete the case, and stop citing it and m7 as evidence that the mechanism is ruled out. Alternatively, show which polls.rs defect it catches that no existing case does.

BLOCKER: evidence for "emits the instruction with no XMM register" on every target, the UEFI loaders' included. crc/target-probe-lib.rs is only the probe's source. Nothing records the targets it was built for, the command or the disassembly. The question is material. The loader runs accumulate under UEFI on the x86-softfloat ABI with sse4.2 enabled for that function, and firmware's MS-x64 ABI preserves XMM6–15. The cheapest sufficient check: disassemble accumulate in the built x86-64 loader (or the probe built for x86_64-unknown-uefi and aarch64-unknown-uefi) and commit the command and its output to the evidence.

SUGGESTION: sdk/toyos/src/fs.rs:36. "A client keeps at most MAX_OUTSTANDING requests unanswered" now reads as counting every CLOSE against the limit forever. Say that a CLOSE is not outstanding.

SUGGESTION: system/fileserver/src/data.rs:13-14. "Only an open reads an entry" is false for read_link. Drop the sentence, or name both.

SUGGESTION: lib/bcachefs/src/fs.rs:57. "What one entry [Mounted::list] answers records besides its name" does not parse. Perhaps: "What an entry records besides its name, as Mounted::list answers it."

SUGGESTION: system/fileserver/src/main.rs armed ×3. Three flags record "a watch is pending". Watching a client again right after pump while it still exists, and once at insertion, removes that state. Submissions are flushed only at the next wait anyway. The flags are correct as written.

Evidence

• Host CI at the head: exit 0. Build-only at b0c014009: exit 0. The CRC changes precede b0c014009, and later commits change no production CRC code.
• The table's revision 04ec3f6f4 has a tree identical to b0c89ed30.
• The CRC oracle is independent: bitwise, reflected 0x82F63B78. The RFC 3720 B.4 vectors and bytes match the RFC, as does the check value 0xE3069283.
• The netstack stall's independence is accepted: 1 in 6 on main, and the stalled step does no served-file work.
• session_lifecycle's original stall is still unexplained. It is not reproduced in 9 runs at the head.
• The file-semantics guest tests (std_fs, file_mtime, fs_truncate_persist, readdir_bound, app_view) and the T14 A/B are not in this evidence. Landing depends on their result.
• No identifier leaks. Paths are redacted. The only GUIDs are QEMU per-image partition GUIDs, freshly generated per test image, and no metal logs are included. No vendor, product, BIOS, serial or MAC strings appear.

SEND BACK

[Japabu]

Review r1, answered at a0806ca07:

1. Directory rename (stale record). Fixed. The move takes each record from names after the open files' entries are written (data.rs, the Kind::Dir arm).
2. Test. The differential now runs under an advancing clock and renames a directory holding an open, written file, checked after its close and across the remount. With the old move, m8 reds (log).
3. Acceptor check. Removed, with its comment and the PR sentence.
4. Loom case. Removed; the PR no longer cites it or m7.
5. CRC and XMM. The x86-64 loader's disassembly: every crc32 is in one 62-instruction function with no %xmm/%ymm operand. The aarch64 loader holds no CRC32C encoding at all. Command, output and scan: crc/disasm.

Suggestions taken: the MAX_OUTSTANDING wording, the "only an open reads an entry" sentence (dropped), and the Recorded doc. The armed flags stay as they are.

Host CI at a0806ca07: exit 0. The testcases image is re-requested at a0806ca07.

[Japabu]

Review r2 of #1060 at a0806ca07 (r1 reviewed a0aba9cc8)

Architecture

Unchanged since r1. The r2 commit changes +29/−48 lines. It removes the loom case and the dead acceptor branch, fixes the rename in place and strengthens the one differential test. It adds no new mechanism.

r1 blockers

1. Directory rename: fixed. system/fileserver/src/data.rs:751-771. The open nodes beneath from are persisted (751-755) before the move loop. persist writes names[open.path] (338-340), and the loop now takes each record with self.names.remove(&name) (770) after those writes. The Meta collected in moving is used only for the kind that builds the format's pairs, and a persist leaves the kind unchanged. A persist or rename_all refused partway leaves every index record keyed at its old path and matching its persisted entry. I checked the other mutation sites (names.insert/remove/get_mut: 459, 638, 659, 677, 695, 723, 770, 791) for the same copy-then-persist shape:
   • File rename, within or across directories (Kind::File | Kind::Symlink arm, 716-724). It persists from first and takes names[from] afterwards, so it holds the current record.
   • Rename over an existing target (722-724). orphan(to) marks the target's open node gone, and persist skips gone nodes (334). The gone node therefore never writes names[to] after the moved record replaces it. Its release leaves by_path[to] to the moved node (347). No stale record results.
   • No other path copies a Meta and writes it back later.
2. Test: has the power asked for. data.rs:1024-1028 adds advancing_clock, and 1047-1055 rename a directory that holds an open file with 7,000 bytes written. The test checks the file after its close, through assert_index_is_the_format_s and against the stat taken while it was open. The remount comparison at 1066 still runs. The m8 patch restores exactly the pre-fix loop. Its log fails in this test at the format comparison, with home/d/moved/open (0, 1000000005) against (7000, 1000000006), so the size and the advancing time both expose the defect. The full Meta comparison of home/d/moved/c became a size check. That is right with an advancing clock, and assert_index_is_the_format_s still checks its mtime against the entry.
3. Dead acceptor check: gone. main.rs:466-467. The acceptor is armed only under takes_handshakes() (440), and the PR sentence is gone.
4. Loom case and m7: gone. kernel/loom/tests/inbox_answer.rs drops the case. The PR now counts seven controls without m7 and says the lost-wake argument rests on reading polls.rs/mod.rs. No source or document in the tree cites the case or m7. The m7 files remain in the evidence tree but nothing cites them.
5. CRC/XMM evidence: established for x86-64. crc/disasm/ records the command (llvm-objdump -d --no-show-raw-insn …/x86_64-unknown-uefi/toyos/bootloader.efi) and the function's full listing. I disassembled the built loader in the worktree with the toolchain's llvm-objdump and got the same result: 16 crc32, all within 0x140050f90..0x14005103c, one function, no vector register. Only general registers are touched, so the MS-x64 XMM6-15 question does not arise. The CRC source is unchanged since b0c014009, so the loader built at a0aba9cc8 stands for the head.

aarch64 loader claim

It is true and harmless, which I verified from source. The evidence README says "presumably None (its body not read)"; the source settles it:

• loader/src/arch/aarch64.rs:19-21: microcode_cpu() is the constant None.
• loader/src/microcode.rs:24: find returns at crate::arch::microcode_cpu()? before engine::read_file.
• microcode.rs:12 is the loader's only bcachefs use, so the aarch64 loader reaches no CRC.
• My own llvm-objdump -d --mattr=+crc of the built aarch64 loader finds no crc32 mnemonic, which agrees.

If a later aarch64 loader path did read ROOT, it would run CRC32CX/CRC32CB on general registers. That instruction is FEAT_CRC32, mandatory from Armv8.1, and the kernel's machines are the loader's machines. #[target_feature(enable = "crc")] on accumulate is the only enablement it needs, and the clippy loader target already compiles it. Nothing in that path touches FP/SIMD state.

Accepted suggestions

• sdk/toyos/src/fs.rs:35-36: "is never outstanding" is true. The SDK sends a CLOSE without a tag and does not count it.
• data.rs:11-13: the sentence is dropped, which leaves nothing false.
• lib/bcachefs/src/fs.rs:57: the doc now parses and is accurate.

Findings

SUGGESTION: system/fileserver/src/data.rs:1061-1064. The test inlines a fourth copy of the remount body because remount() (1117) fixes its clock to clock. fn remount(v, clock: fn() -> u64) would serve this test, a_write_s_stamp_keeps_its_nanosecond_across_a_remount and the existing callers.

SUGGESTION: sdk/toyos/src/fs.rs:38-39. The reflow leaves a short line ("server reads a request's input out of its"). This is cosmetic.

The handoff still says the stall's mechanism is "ruled out by reasoning and the loom model". The PR body is correct; only the orchestrator's handoff note is stale.

Evidence

• Host CI at a0806ca07: exit 0, per the PR's log. Reused evidence from a0aba9cc8 stays valid, because r2 changes no measured path except the rename's index bookkeeping, which is a map remove and insert.
• The T14 A/B at a0aba9cc8 is settled. The testcases image at a0806ca07 is the orchestrator's to judge.

LAND