[Japabu]

Problem

osbench persist on the T14 at aeba59f had ToyOS 67× slower than Linux on fsync_clean (2.0 ms p50 against 30 µs), the worst ratio in the group.

What the row measures. On a benchcase boot, ToyOS's persist files are on the boot stick's TOYOS-LOG partition: FAT32 with 512-byte clusters, served by the LOG file server through usbd. The stick is a SanDisk Ultra on a high-speed (USB 2.0) link. The run's persist.medium condition records this, and so does kernel.log: boot lines 270, 279 and 286 and line 14349. That boot has no DATA partition: DATA is bcachefs in memory (lines 298–301). Linux ran on ext4 on the internal NVMe. bcachefs and NVMe are not on the measured ToyOS path, so the whole group compares a USB 2.0 stick with NVMe.

Where a clean fsync went. I counted disk requests on a host disk at the base revision. An fsync of a file with no change since its last sync cost one block write and one device flush. Over usbd that is a WRITE(10) and a SYNCHRONIZE CACHE. It came from two redundancies:

1. FatVolume::level rewrote each open file's directory entry at every sync and last close, stamped with the time, even when nothing had written the file. It also changed the mtime of a file that was only read, or fsynced twice.
2. Cache::flush asked the disk to flush even when nothing had been written since a flush the disk answered.

Change

• FAT (system/fileserver/src/fat.rs): an open file is dirty once written, resized or opened with O_TRUNC. O_TRUNC counts even on an empty file, because POSIX's open stamps it. level writes the entry only when the file is dirty or its handle disagrees with its entry (needs_reconcile), and clears the flag only when the level succeeds. This matches the DATA volume's dirty/persist.
• Cache (system/fileserver/src/cache.rs): the cache owes the disk a flush from each write it issues, until a flush the disk answers. The debt starts set, because a previous server (restart = true) may have left writes in the device's volatile cache. A refused flush keeps it. Only the running flush writes the disk, so no write falls between the disk's flush and the debt being cleared.

Durability. An acknowledged fsync still means the file's data and entry reached the medium before a flush the disk answered. Skipping the flush is safe because every write the cache ever issued was already covered by an answered flush. This sits in the cache, not in usbd or nvme, because the cache is the one place that knows what this server wrote. A flush owed across a device loss still goes out, and blockhold judges it as before.

Disk requests per sync, host disk, base → this branch

sync after	writes	flushes
nothing changed (fsync_clean)	1 → 0	1 → 0
4 KiB overwrite (write_4k_fsync)	2 → 2	1 → 1
4 KiB append (append_4k_fsync)	5 → 5	2 → 2

Inference, not measured per command. The T14 times fit about 1 ms per USB mass-storage command: 2 commands ≈ 2.0 ms, 3 ≈ 2.16 ms, 7 ≈ 7.4 ms. So fsync_clean should fall to about the cost of the IPC round trip. The other rows send the requests FAT's ordering needs on a USB 2.0 stick: the chain and data are flushed before the entry that names them. seq_write_fsync's 14 MB/s is in the range of a USB 2.0 stick's writes. The T14 readback below is the timing evidence. It has not been run yet.

Checks

The head is 2118299, rebased onto main 8348c2a. All checks ran on a macOS aarch64 host. Index: evidence pr-1067.

• cargo run -- --ci host: exit 0, 103 steps green, including the bcachefs crash-model suites and the fileserver's kill and refusal tests (log).
• cargo test --test toyos-build -- the_log_lands_on_the_boot_stick, x86-64 QEMU: exit 0. The boot's log is written through usbd onto the stick and read back on the host after the stop (log).
• cargo run -- --build-only: exit 0 at 991f08d (log). Since then the image has also been built at the head by the guest test and the readback staging.
• Two new host tests: cache::a_flush_asks_the_disk_to_flush_only_what_it_wrote_since_its_last and fat::a_sync_of_files_nothing_changed_asks_the_disk_nothing.
• Negative controls, each applied as a patch, built, run and reversed. Every one turned the suite red (exit 101): always flushing (m1), clearing the debt on a refused flush (m2), no debt at mount (m3), and levelling unchanged files (m4 at 991f08d; m4r1 against the r1 condition at the head) (patches, logs). The cache code has not changed since m1–m3 ran.
• Disk-request probe, base against the change: the table above (base, change).
• T14 timing: none yet. An earlier run was invalid: the pre-toyos-metal: a bench's disks are known by what they hold, not by their node #1064 tool flashed it onto the wrong stick. The readback is re-staged at the head and touches no machine. The harness picks rows, not groups, so the request is the osbench row's single benchcase boot, with persist as its last group.

Not addressed

• FSYNC still syncs the whole volume (issues/an-fsync-of-a-served-file-syncs-its-whole-volume.md). Now that unchanged files cost nothing, this matters only when another file is dirty.
• The DATA volume's Mounted::sync (lib/bcachefs/src/fs.rs) still writes the superblock and flushes on a clean commit. With this change a clean DATA fsync costs one flush instead of two. The measured row does not reach this code, and the clean-room engine replaces it.
• seq_read_cached (955 MB/s against 8.7 GB/s) is a read-path copy cost, not a durable write. I did not investigate it.

🤖 Generated with Claude Code

[Japabu]

Review r1: PR #1067 at 991f08d

Architecture

The change sits in the right owners. Cache is the only component that knows what this server has written to its Served disk, so it is the right place to track whether a flush is owed. Usbd, nvme and blockhold should not learn this. The FAT volume already owns the open-file table, so the dirty flag belongs there, and it matches the existing DATA Open::dirty/persist (data.rs:331-341). There is no new mechanism, protocol or dependency. Production code grows by about 15 lines: one bool and two branches in cache.rs, one bool and three assignments in fat.rs. Tests grow by about 85 lines across two focused host tests. I agree with the PR's correction of the brief's premise, that the measured rows ran on FAT/USB 2.0 and not on bcachefs/NVMe. The evidence's persist.medium condition and the kernel.log lines support it.

Durability: is every acknowledged fsync covered by an answered flush?

I traced this myself, and the answer is yes in every path I could build.

• Only Cache::flush writes the disk. write only fills slots, visit only reads, and no non-test caller reaches cache.disk() (grep: there are none outside tests). unflushed = true is set before each disk.write (cache.rs:291) and cleared only after disk.flush() returns Ok (cache.rs:306-309). flushing excludes a second flush, so no write can fall between that answer and the clear.
• Errors and cancellation mid-sync. A refused disk.write or disk.flush returns through ? with the debt still set. A dropped flush future, at any await, leaves unflushed true. Dirty blocks of a refused run stay dirty and are written again. All of these fail toward sending another flush.
• Writes racing a flush. A block rewritten during flush F stays dirty (versions), and only the cache holds it. An fsync covering it calls its own flush after F ends, and that flush writes it and owes a flush again.
• Several files or writers. FAT and DATA sync cover the whole volume (main.rs:1135). Another partition's server has its own Cache and its own debt. A device-wide flush that another server sends never clears this server's debt; at worst a flush is redundant.
• Served-internal flushes. ask_room flushes on a full arena (diskserver/src/disk.rs:186). That flush leaves the cache's debt set, so the cache still sends its own. This is conservative and correct.
• Block-service restart, loss, give-up. blockring::client keeps an acknowledged write only until a flush answered Durable. A Lost answer or a session end is reissued and asked again inside Served. A given-up write poisons every later flush with Device. That flush is refused, so the debt stays, and every later fsync still sends a flush and reports the poison. A given-up write can only be one not yet covered, and such a write implies the debt is set. Blockhold's loss reporting is therefore unaffected, as the PR says.
• USB detach. A clean fsync after a detach now answers Ok instead of Gone. That is correct: everything it covers was answered Durable before the detach. Logkeeper treats sync_all only as a durability step (logkeeper/src/store.rs:227), not as a liveness probe. A dirty fsync still reaches the disk and fails as before.
• Fileserver restart over an un-flushed device. Cache::new starts with the debt set, so the first sync of the new server flushes whatever the previous one left in the device cache. The negative control m3 shows the test catches its removal.
• bcachefs DATA. Mounted::sync (lib/bcachefs/src/fs.rs:927-938) uses io.flush() as an ordering barrier: flush, superblock, flush. Skipping a barrier is sound when no write has been issued since an answered flush, because everything before it is already durable. A clean commit therefore costs one flush instead of two, matching the PR's claim. The crash-model suites in --ci host are green at this head.

Dirty tracking (truncate, rename, mtime-only, size-only)

• Write. dirty is set before the attempt, so a refused write also re-levels. That is harmless and correct, since the in-place bytes may have landed.
• Size-only change through truncate/set_len. It sets dirty, which inspection establishes. Every handle change that needs_reconcile() can see comes from write or set_len, and both mark the file dirty.
• Rename. rename levels the source only when it is dirty. A clean file's entry is already in the cache, and move_entry copies the raw entry it reads from the cache. Nothing is lost.
• mtime-only. set_mtime is Unsupported on FAT, so there is no path to miss.
• Level failure. A failure at any step (pre-flush, flush_meta, reconcile) leaves dirty set. After a successful flush_meta, the entry block is a dirty cache block, and the cache keeps it until a flush succeeds.

Evidence

I checked the evidence branch at 558a5282. Host CI exited 0 at the head. The disk-request probe shows base 1w+1f → change 0/0 for a clean sync, with the overwrite and append counts unchanged. Each of m1–m4 is a single-line mutation of the claim it targets, and each turns the named test red (exit 101; the panics are at the asserted lines). The FAT test exercises write → dirty through its final mtime assertion. No test exercises truncate → dirty. Inspection establishes it, so I am not requiring one, but see S1. The T14 timing is the orchestrator's and is not requested here.

Findings

BLOCKER

1. system/fileserver/src/fat.rs:6-8: the module header still says "every open file is brought level — its entry written, its chain reconciled — at its last close, at an fsync and at a sync". That is now false, and it is false in exactly the behaviour this PR changes: an unchanged file's entry is no longer written. The header is the contract a reader uses to learn when entries hit the disk. Fix it with something like "every open file written or resized since it was last level is brought level …". The history tail "as the kernel's adapter did" can go in the same edit.

SUGGESTION

S1. system/fileserver/src/fat.rs:137: you can make a missed size or chain change unrepresentable instead of relying on every mutator remembering the flag. Use if open.gone || !(open.dirty || open.file.needs_reconcile()). needs_reconcile() already captures every entry/handle size or cluster mismatch, so dirty would then only add "data overwritten, restamp". This also covers a future mutator that forgets the flag, without a test.

S2. system/fileserver/src/fat.rs:252 with fat.rs:318: O_TRUNC on an existing file that is already empty no longer restamps it. POSIX open() says that with O_TRUNC on an existing file, open "shall mark for update the last data modification and last file status change timestamps". ftruncate says the same only "if the file size is changed". The base met the open case by accident, through universal restamping, and this branch drops it. The impact is low, since only the LOG and BOOT roles use FAT and set_mtime is already unsupported there. If you want it, set dirty in open's truncate branch rather than in truncate.

S3. system/fileserver/src/fat.rs:135: self.time() reads the clock before the clean early return, once per open file per sync. Move it below the check.

The durability argument holds in every path above. The only blocker is the stale header in finding 1, which is a one-line fix.

SEND BACK

[Japabu]

r1 addressed in 2118299, rebased onto main 8348c2a.

• BLOCKER (fat.rs header): fixed. The header now says an open file changed since it was last level is brought level at its last close, fsync and sync. The "as the kernel's adapter did" tail is gone.
• S1: taken. level now skips only a file that is neither dirty nor needs_reconcile().
• S2: taken. An open with O_TRUNC marks the file dirty, even when it is already empty.
• S3: taken. The clock is read after the skip check.

Rerun at the head: fileserver tests, host CI (exit 0, 103 green) and the_log_lands_on_the_boot_stick (exit 0). The m4 control was redone against the new condition and is still red, exit 101. The T14 readback is re-staged. Evidence: https://github.com/ToyOSOrg/evidence/blob/ccda61f85a31d2ce10f6c5cdf7e4395a9197bb6c/INDEX.md

Please review the r1 commit.

[Japabu]

Review r2: PR #1067 at 2118299

Scope: the r1 commit 2118299 and the rebase onto main 8348c2a. I did not reopen the durability argument settled in r1.

Rebase

git range-diff aeba59fb0..991f08d9a 8348c2a2c..cee5365d6 reports 1: 991f08d9a = 1: cee5365d6. The rebased commit is byte-identical in content to the one reviewed in r1. Against main, the branch still touches only system/fileserver/src/cache.rs and system/fileserver/src/fat.rs.

r1 commit (fat.rs, +15/-11)

• Blocker (header): resolved. fat.rs:6-11 now says that an open file changed since it was last level is brought level, its entry written and stamped, at its last close, at an fsync and at a sync. That matches level. The "as the kernel's adapter did" tail is gone. The rest of the paragraph is unchanged and still true.
• S1: taken correctly. fat.rs:136-139 skips only a file that is gone, or that is neither dirty nor needs_reconcile(). A size or chain mismatch a future mutator forgets to flag is now still levelled. The second lookup's expect("just found") is a sound invariant: nothing between the two lookups touches self.open, since self.time() takes &self. dirty is still cleared only after every step succeeds (fat.rs:151).
• S2: taken correctly. fat.rs:255-256 marks the node dirty before truncate(node, 0), so truncate's equal-size early return (fat.rs:322) no longer loses the stamp. If the truncate fails, the node stays dirty and the existing close/next-sync path levels it. That is the same fail-toward-levelling behaviour r1 accepted for write. A read-only volume is unaffected, because close and sync level only when writable. The one-line POSIX comment explains a choice the code cannot show on its own, so it stays.
• S3: taken. self.time() now runs after the skip check (fat.rs:140).
• The dirty field doc (fat.rs:41-42) and the level doc (fat.rs:133-134) both stay accurate.

Evidence

The evidence branch at ccda61f8, section "Review r1: head 2118299 on main 8348c2a", records reruns at the head:

• fileserver lib tests: exit 0.
• --ci host: exit 0, 103 steps.
• the_log_lands_on_the_boot_stick on x86-64 QEMU: exit 0.
• The m4r1 control, which removes the new combined condition, turns the FAT clean-sync test red (exit 101).
• The readback is staged (exit 2), and no machine was touched.

No test covers S2's empty-file O_TRUNC stamp. It is a one-line flag set on a path inspection covers, and its impact is limited to FAT mtimes, so I do not require one. T14 timing is out of scope here and runs separately.

Findings

No blockers. No new suggestions.

LAND

[Japabu]

T14 benchcase at 2118299 (build line confirms the head; flashed with main's 8348c2a tool onto the ToyOS stick): persist.fsync_clean p50 2.00 ms -> 39 µs (Linux ext4/NVMe: 67 µs); write_4k_fsync 2.16 -> 1.01 ms; append/create/seq_write rows remain stick-bound. Readback: orchestrator scratch persist-fsync/t14/benchcase. Earlier m1067r1 run was invalid (pre-#1064 tool flashed the spare stick) and is not evidence. Review r2 LAND. Landing.