Repository navigation
Draft
Conversation
Collaborator
Author
Review r1 of #1141 at b4ededeI read the three commits, the full diff, Architecture
What I checked and found correct
RequiredR1.
R2.
R3. A false durability claim.
Suggestions
SEND BACK |
Japabu
added a commit
that referenced
this pull request
Nov 1, 2026
…aims narrowed - R1: the holder's answer WouldBlock is documented on paged_read, the boundary a holder implements against; PagedOp::Write keeps only the EndWrite rule. - R3: a carried file's test no longer claims no block is held that no entry names: DataVolume::commit commits past a refused carried entry, whose taken blocks then leak after a stop. Filed with S3, one carried file's refused write-back refusing every fsync on the volume (issues/a-refused-carried-file-costs-every-other-files-fsync-or-leaks-its-blocks.md). - S2: the tear member's generations are its bytes mod 256. R2 is in the fsync commit: its issue stays open until the A/B records the exit's measurement. S1 (the version move at a write's end is redundant) is declined for this round: it changes the kernel the staged A/B images were built from. Comments, ABI docs and issue records only: no code changes, so the staged images at b4edede still measure this head. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
As persist.write_4k_fsync, each iteration first writing 1 MiB over another 1 MiB file and leaving it unsynced: an fsync of one file that also makes another file's write durable pays for it here. It is the A/B base for a per-file fsync (issues/an-fsync-of-a-served-file-syncs-its-whole-volume.md), which asks for one file's fsync measured beside the volume's. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A holder's SYS_PAGED_READ copied a pinned chunk of 16 pages at a time while the file server's write landed page by page in the same frames, so a read beside a write of several pages could return pages from before and after it, or a page torn at the copy (memory stage 7a's recorded gap). Every write of an object's now runs as a write span on it (kernel::paged::Object): each write call and each dirty supply begins one before its first byte lands and ends it after its last, and a resize moves the object's version. A holder's read begins only while no span is open and counts its bytes only where the version has not moved since; a read a write met answers WouldBlock, and the SDK, which already asks the server whenever a holder read fails, reads through the server, whose loop orders the read against the write. A server write takes many calls when a page is absent, in part or at the dirty ceiling. A call that does not take all it was given leaves the span open (held) until the new PagedOp::EndWrite, and later calls nest in it, so the file server's whole write is one span. The server's first call of a write is always a write: one that takes everything is the whole write at no extra syscall; otherwise the server ends the held span on every path, the refused one included. Holder reads never wait on a write: the pager is never made to wait on its readers, and a read that met one costs one round trip. Tests: - kernel::paged: a read counts only where no write began or was under way; a short call holds the write until its end. - fileserver pages: a write of many calls (an absent page supplied, the dirty ceiling written back, a last page in part) is read whole or not at all at every call boundary, and ends whether it lands or is refused. - guest a_served_files_read_never_tears_against_its_write: a writer puts 400 whole generations of a 64-page served file while a reader on its own description reads it whole through its holder; red on a torn read. Closes issues/a-read-and-a-write-of-a-paged-file-can-tear-across-pages.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
FSYNC synced the whole volume: every dirty page of every file written back and every open file's entry written before the format's commit. An fsync now makes durable the file it names and only what the volume's commit cannot leave out: - Volume::carried names the open files the next commit carries whatever it is asked: on DATA one whose entry was written since the last commit (a rename, a shrink) or whose reserve took blocks since, which the commit's allocator holds as taken; on FAT every changed open file, since a cache flush writes every chain the cache holds. - Files::fsync writes back the named file's pages and those of every carried file, then Volume::commit writes the named entry and each carried one and commits. A carried file whose pages are refused refuses the commit: its entry would name blocks without its bytes. - So after a stop no entry names a block holding an earlier tenant's bytes. A write over blocks a commit already named is carried by nothing, and waits for its own fsync or the volume's sync. A directory's fsync commits the carried files only: DATA writes every name change into the tree at once. SYNC and the write-back timer still sync the whole volume, and a per-file fsync leaves the timer due. Found while doing this: a sync commits the entry of a file whose pages were refused (issues/a-sync-commits-the-entry-of-a-file-whose-pages-were-refused.md). Tests (fileserver pages): an fsync makes its file durable across a stop and leaves another file's overwrite unwritten; a carried file's pages and entry go with another's fsync, so after a stop on a volume of stale blocks it reads its own bytes; a directory's fsync makes a new name durable. issues/an-fsync-of-a-served-file-syncs-its-whole-volume.md stays open until its measurement, osbench's persist.write_4k_fsync_beside_1m base against head on metal, is recorded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…aims narrowed - R1: the holder's answer WouldBlock is documented on paged_read, the boundary a holder implements against; PagedOp::Write keeps only the EndWrite rule. - R3: a carried file's test no longer claims no block is held that no entry names: DataVolume::commit commits past a refused carried entry, whose taken blocks then leak after a stop. Filed with S3, one carried file's refused write-back refusing every fsync on the volume (issues/a-refused-carried-file-costs-every-other-files-fsync-or-leaks-its-blocks.md). - S2: the tear member's generations are its bytes mod 256. R2 is in the fsync commit: its issue stays open until the A/B records the exit's measurement. S1 (the version move at a write's end is redundant) is declined for this round: it changes the kernel the staged A/B images were built from. Comments, ABI docs and issue records only: no code changes, so the staged images at b4edede still measure this head. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The volume-wide sync an fsync used to run cleared the write-back the server's loop keeps due. The per-file fsync never did: after it, the server waited every turn with a deadline, which every park and idle entry programs into the CPU's timer, and 2 s later walked the volume in a sync with nothing left to write. The volume now says whether an open file's entry is changed past what its last commit made durable (Volume::owes), and Files adds its dirty pages (Files::owes). An fsync that leaves neither answers the write-back as a sync does. One that leaves another file's write keeps it due from that write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the two defects memory stage 7a (#1123) recorded, each at its owner.
A holder's read is atomic against its pager's write (XSH 2.9.7)
Problem. A holder's
SYS_PAGED_READcopied 16 pinned pages at a time while the file server's write landed page by page in the same frames, so a read beside a multi-page write could return pages from before and after it, or a page torn mid-copy (issues/a-read-and-a-write-of-a-paged-file-can-tear-across-pages.md).Now.
kernel::paged::Object). EachWritecall and each dirtySupplyopens one before its first byte lands and closes it after its last. AResizemoves the object's version.WouldBlock. The SDK already asks the server whenever a holder read fails, and the server's loop orders that read against the write. Holder reads never wait on a write, so a pager never waits on its readers.PagedOp::EndWritecloses it. The server's first call of a write is always aWrite. If it takes everything, that is the whole write, at no extra syscall. Otherwise the server ends the held span on every path, refusals included.An fsync commits its own file and what the commit carries
Problem.
FSYNCsynced the whole volume. It wrote back every dirty page and every open entry (issues/an-fsync-of-a-served-file-syncs-its-whole-volume.md).Now.
Volume::carried(node)names the open files the next commit carries whatever it is asked to.Files::fsyncwrites back the named file's pages and every carried file's. ThenVolume::commitwrites the named entry and each carried one, and commits.SYNCand the write-back timer still sync everything. An fsync that leaves nothing owed (no dirty page, no entry the volume owes:Files::owes,Volume::owes) answers the write-back as a sync does; one that leaves another file's write keeps it due from that write. Round 1 found the fsync left it due always, so the server kept a 2 s deadline on every wait and ran a sync with nothing left to write.Found along the way and filed, not fixed:
issues/a-sync-commits-the-entry-of-a-file-whose-pages-were-refused.md).issues/a-refused-carried-file-costs-every-other-files-fsync-or-leaks-its-blocks.md, from review r1). A persistently refused write-back refuses every fsync on the volume. A refused carried entry lets the commit land, and the blocks that file took leak after a stop. Neither discloses another file's bytes.issues/an-fsync-of-a-served-file-syncs-its-whole-volume.mdstays open until the metal A/B records its exit's measurement.Commits
Rebased onto main 058bea2;
PagedOp::EndWrite = 9is still free there.a4b6a1ee7osbench:persist.write_4k_fsync_beside_1malone, the A/B base.31206cbf4atomic holder reads (kernel, ABIPagedOp::EndWrite, SDK, file server, guest test). Closes the tear issue.b3897d50aper-file fsync (file server volumes). The fsync issue stays open for its measurement.6ae35a46ereview r1: docs and issue records only.029ca221can fsync that leaves nothing owed answers the write-back (new in r2).Round 1's A/B, investigated
Round 1 (evidence r1) missed its claim's bound (AMD −28.1% against −30%; T14 unmoved) and flagged regressions. Its details: https://github.com/ToyOSOrg/evidence/tree/4abdb91acc0e9949310bf79ea9b02b054cc37c1a/r2
FSYNCreset the write-back deadline and the new one never did, so every server wait after a write and fsync carried a deadline that each park programs into the CPU's timer. Fixed by029ca221c.write_begin/write_end): tens of ns on a 10 us call, estimated, not measured. Folding them into the pin and unpin loops of both write and supply would tangle the span bookkeeping for that.Checks
Evidence: https://github.com/ToyOSOrg/evidence/tree/4abdb91acc0e9949310bf79ea9b02b054cc37c1a/r2 (r0 and r1 on the same branch hold round 0's controls and round 1's A/B).
cargo run -- --ci hostat tree bfae305ac (029ca22): rc=0, 109 steps green.--debug) at 029ca22, clean tree. All five members exit 0:a_served_files_read_never_tears_against_its_write(1105 reads, 244 generations met, none torn),a_served_file_reads_through_its_paged_object,served_file_offset,a_pager_s_end_answers_its_waiting_holders_gone,std_fs.owesalways false or always true, or drop the dirty-page term. TheFSYNCarm's three-line use has no host test (Serverhas none); the metal rows are its evidence.metal-staged/rules.md): per bench, 3 base and 3 head benchcase boots interleaved, then one head testcases boot. Claim: every head d below every base d, with mean ≤ 0.5x. Regression: all three heads beyond all three bases, and the median worse by more than 10%.Risks
data=orderedwithout delalloc.🤖 Generated with Claude Code