Skip to content

A block session never waits on its server's word alone - #1235

Merged
Japabu merged 7 commits into
mainfrom
block-client-never-waits-forever
Nov 11, 2026
Merged

Japabu merged 7 commits into
mainfrom
block-client-never-waits-forever

Conversation

@Japabu

@Japabu Japabu commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

A client of the block service (diskserver's Session) could be held for good by a server that keeps its connection:

  • A request-ring cursor past what the client published (HeadPastTail) stopped the push and was left to "the next drain", which reads only the completion ring; the wait then blocked for u64::MAX.
  • Client::complete took a completion's tag off the wire before judging its status. A read or write answered Lost (or a read, a reissue or a flush answered ReadOnly) was refused with its request already gone, so the session's end answered nobody: the caller's ticket stayed pending across every reconnect, a refused reissue dropped the only copy of an acknowledged write, and a refused pushback flush left writes waiting on it.
  • A server that took a request and never answered was waited on without bound.
  • A session's end requeued the flushes and rewritten writes it carried without counting it, so a server that ended every session over the same one held it, and every request behind it, through endless reconnects.

The contract

A block server is never silent past toyos_blockring::SILENCE_BOUND_NS (365 s) while it owes a session an answer: from a request's publication, or from its last completion, it posts a completion or ends the session within it. A client that has waited longer ends the session. The bound is the protocol's, stated in lib/blockring and in the manual's storage page; each driver is to show that its worst case from a request's publication, the wait to be taken from the session included, fits. The bound is set from the NVMe driver's figure, which counts from a command's submission to the device; what that leaves out is recorded (below).

The NVMe driver shows it at compile time. A command unanswered for COMMAND_SILENCE (10 s) is taken back by a controller reset, during which the driver answers nothing: two CSTS.RDY transitions, each bounded by CAP.TO (at most 255 × 500 ms, NVMe Base 2.0 §3.1.4.1), and the admin commands of STEPS, a const list the reset steps through at most one command a step (Set Features for the queue count, a completion and a submission queue for each of four, and since #1131 the APST table), each now bounded by the driver's own command policy, ADMIN_BOUND = 10 s, rather than CAP.TO. The count is the list's length, so the figure follows the code: ANSWERED_WITHIN = 10 + 2 × 127.5 + 10 × 10 = 365 s, and a compile-time assertion holds it under SILENCE_BOUND_NS, set to exactly that: no rounding up. Bring-up runs the same list (Controller::configure, which replaces make_io_queues and set_power). admin_command used to check its bound and then wait a whole bound more for an interrupt, so one step could run to twice its bound; every wait of the reset now waits only for what is left of its bound (within).

usbd cannot show the same yet: its device serves one command at a time from a queue every session on it shares, so one session can wait minutes behind others' commands with nothing failing. issues/a-usb-disk-session-can-wait-past-the-block-silence-bound-behind-other-sessions.md records it with its exit.

Resulting behaviour

toyos_blockring::ends::ClientEnds is the client's two ring ends over the session page and the decision of what the client does next, pure and host-tested. exchange sends what can go, takes every completion posted, sends what those let go, and only then answers:

  • End(Violation): a cursor no server writes, words that are no completion, or a completion Client::complete refuses;
  • End(Silence): owed a completion for SILENCE_BOUND_NS, longer than any server keeping the contract is silent. Looking without taking a completion renews nothing, so a server ringing the bell buys no time;
  • Answered, if any answer is decided, including one the last send decided;
  • Wait(ns) for the server, or Idle.

Session::wait collects every decided answer after the exchange and acts on that answer; ending is exactly the hang-up path. A blocking wait with nothing owed and nothing decided is an invariant violation (its one caller, Served::wait, asserts a request pending) and panics.

Client::complete judges the status against the request before the tag leaves the wire (Inflight::answer_admitted in sdk/toyos's transport), by one rule stated in code and the module contract: a server says Lost only of a flush, and ReadOnly only of a caller's write. A refused completion leaves its request for the session's end.

A session's end is a failed attempt, as a device's refusal is, of each flush and each rewritten write it carried: past MAX_ATTEMPTS the flush is answered Device and the write is given up. A session that ended with nothing on the wire (a server past the request ring before anything went) ended over the first request waiting, which is treated as if it had been on the wire: a caller's request is answered Refused, which Served counts against MAX_RECONNECTS. So no server can hold a caller across reconnects for good.

What a session's end leads to: Served reconnects through the same name and reaches the same running server. A server that still holds a request of the ended session (a silence end, typically) refuses the open Held until that request is answered, so the reconnect fails, the disk is gone for the client's life, and nothing is issued again. A server that holds nothing (it hung up and was restarted, or broke the protocol over a request it had already answered) takes the session back, and the writes the client keeps go out again first.

Every session is opened over a region no server has held, so nothing a server writes to an earlier session's page reaches the new one, whether or not that server has let the page go. That also removes the case issues/nothing-reopens-a-diskserver-session-over-a-used-page.md owed a client test for. reconnect no longer pumps; held writes go out at the next wait, where a violation has somewhere to go.

The server needed nothing: toyos-blockserve pulls, never waits on a client, and closes a session on any cursor or entry violation.

Closes issues/a-block-session-waits-forever-on-a-server-past-its-submission-ring.md, issues/a-read-or-write-answered-lost-is-never-answered.md and issues/nothing-reopens-a-diskserver-session-over-a-used-page.md. The kernel record's references to the last are updated. Session::peak_on_the_wire, which nothing read, is gone with the pump it was measured in.

Checks

The behaviour is a client's reaction to a misbehaving server, which module tests drive over a real session page with the server's ends held by the test (lib/blockring/src/ends.rs), and the client's bookkeeping alone (client.rs). No real server produces these inputs, so no guest or metal row can show them.

Host, macOS arm64, at 31fda55 (rebased onto main 6d4b7e4 with #1131; the final head):

  • cargo test -p toyos-blockring -p diskserver -p toyos -p nvme: exit 0. New in the driver: a_command_left_silent_is_answered_within_365_s (the arithmetic), a_reset_steps_through_the_queues_then_the_power_table (the list's order), a_wake_before_the_bound_buys_no_second_bound (a wake 1 ms before the bound, then nothing: the whole takes the bound), an_answer_ends_the_wait.
  • cargo clippy -p toyos-blockring -p toyos --all-targets (gate lints, -D warnings): exit 0; the same over nvme and diskserver, which the gate does not lint: exit 0.
  • Controls, each applied, built, run and reverted: the old wait (a whole bound after every wake) reds a_wake_before_the_bound_buys_no_second_bound; the reset's admin commands bounded by CAP.TO's largest value, and one more step in STEPS, each fail the build at the bound's compile-time assertion.
  • The blockring side since r1 moves the bound and changes its value; the blockring tests above cover it, and r1's controls below are not rerun. B2's first round, at 941ac6f before the rebase, is in the evidence under r3/.

Host, macOS arm64, at 4eb8b1f (review r1):

  • cargo test -p toyos-blockring -p diskserver -p toyos: exit 0. New here: an_answer_the_last_send_decides_is_never_waited_past (the review's B1 trace over a real page: the client's own flush failing past its attempts while a caller's write waits behind the limit), a_flush_every_session_ends_over_is_answered_device, a_reissue_every_session_ends_over_is_given_up, a_server_that_ends_every_session_over_one_write_holds_nothing_for_good (a rewritten write answered Lost in every session), a_server_past_the_request_ring_before_anything_went_ends_over_what_waited, and a_reissue_answered_a_word_no_server_says_of_it_is_kept (ReadOnly and Lost).
  • The blockring model's durability law weighed every failure as one attempt. With a server's death now an attempt, its bound of two resets and two deaths gives a flush up, because a reset costs a flush two attempts (the request it fails, and the loss the next flush is told of). The law now weighs failures by the attempts they can cost, and the give-up script is also held at the boundary, a reset and two deaths, which must never give a flush up.
  • cargo clippy -p toyos-blockring -p toyos --all-targets, the gate's adopted lints, -D warnings: exit 0; the same over diskserver, which the gate does not lint: exit 0.
  • Negative controls, each a patch applied, built, run and reverted, all red: m1 send ignoring the request ring's violation; m2 the tag taken before the status is judged; m3 every look renewing patience; m4 the decision taken before the last send (B1's order), red at an_answer_the_last_send_decides_is_never_waited_past; m5 a session's end counting nothing, red at the three B3 tests; m6 an end with nothing on the wire taking nothing; m7 Lost admitted of a rewritten write. The CI's three blockring controls still redden the model tests build/ci.rs names. A first m4 run stayed green because the B1 test let its loop run out; the test now requires Answered on the last exchange, and that run is kept in the evidence.
  • The full --ci host ran at 500507174 before round one (1 of 110 steps red: the build system's macOS llvm environment test, issues/the-llvm-environment-test-breaks-git-on-macos.md). It is not rerun: this round changes lib/blockring and system/diskserver only, whose tests and lint are above.

Guest: nvme_disk_boots (QEMU x86-64) boots off one NVMe disk with every role served through the block service's sessions, so every request runs through Session::wait and the driver's bring-up runs its admin commands under the new bound. It passed at 7c6717a (run 43049658371). The landing run, https://github.com/ToyOSOrg/ToyOS/actions/runs/43069269031 at 31fda55, passed: PASS nvme_disk_boots, 1 of 1, all five steps green (2026-11-11 20:01Z); run 43062272826 at 4eb8b1f was cancelled unrun, superseded. The head, 19c3b5f, differs from 31fda55 in text alone: the manual, three records, and comments in lib.rs, model.rs and nvme.rs (git diff -U0 31fda554e 19c3b5f3a -- '*.rs' changes no line that is not a comment), so the run stands for it.

Metal: both benches are down, and metal is no longer a landing condition. The T14 booted metalcase (rows microcode and machine_reboot) at 97c93a6, code-identical to the head, and passed: the nvme driver brought its controller up under the 10 s admin bound (up: 2 ms after it started, APST on, no admin command unanswered), DATA was served off the NVMe, and no session ended. Owed on metal (recorded in issues/the-nvme-driver-resets-a-drive-that-is-slow-and-healthy.md): the AMD bench, the same two rows in one boot of main after this lands, which must pass with the nvme driver's up: line, no NOT SERVING or was not answered in, DATA served off the NVMe, and no session ended by a violation, silence or a refused reconnect. This branch also carries #1236's owed list, which reached no branch before #1236 merged: both benches, osbench_disk's fs.disk.* rows in one A/B of main before and after #1236; on the AMD bench main after must show readdir.1000 below main before on every boot, and on both no other row may resolve against main after, which is where open_close and stat (1.8 and 8 % against #1236's head on the T14) recur or not. It is recorded in issues/smaller-data-rows-trail-main-by-up-to-half.md, the DATA rows' record both this branch and main carry. No guest row restarts the block service, so Session::reconnect runs on no guest; diskserver's host tests cover Served's reconnect decisions over a scripted link.

Evidence: https://github.com/ToyOSOrg/evidence/tree/55f29b4c0aa4fe30901a691e5d89d6831d4f4de2 (round one at the top, review r1 under r2/, B2 under r3/, the rebase onto #1131 under r4/).

Risks

  • usbd does not keep the contract (record above, severity: [data-loss]): a stick serving several busy sessions can have one ended by its client, whose disk is then gone for its life, and acknowledged writes no flush covered are reported lost. The NVMe driver's figure equals the bound with no slack and counts from a command's submission; the same record names what it leaves out: the wait to be pulled (behind other sessions' requests while the device is full, or behind a reset), the serve loop's lag before a reset, the reset's own work, posting its answers, and the client's clock starting before it publishes.
  • An admin command at bring-up is now given 10 s, the command policy, rather than CAP.TO; a controller slower than that to answer Identify is refused rather than served.
  • Session::reconnect over a fresh region runs on no guest (no row restarts the block service); diskserver's host tests cover Served's reconnect decisions over a scripted link.

🤖 Generated with Claude Code

@Japabu

Japabu commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Review of #1235 at 7c6717a

Architectural assessment

  • Ownership is right. The client's ring half now lives in toyos_blockring::ends::ClientEnds: pure code, host-tested, sitting beside Client. That shrinks the part of the session that only a guest can reach. Inflight::answer_admitted judges a completion before taking its tag, which is the correct fix for the Lost/ReadOnly record. A fresh region per session removes the used-page state, at the cost of a 2 MiB device allocation on each reconnect. The cost is acceptable.
  • Wait rule. The 60 s bound satisfies root's rule. Every wait in Session::wait now has a failure timeout, and on expiry the session ends with a printed report.
  • Swapped memory. A block wait under a pager is not the swapped-memory case. No swap exists yet. The exception covers the faulting thread's wait for an anonymous page. In the swap track that is the kernel's fault wait, which is reported overdue and never failed. The block client's wait is a service waiting on its peer, and its expiry ends a session, not a fault. When the memory service carries swap I/O over these sessions, it must keep faults overdue across a session end or a gone disk. That design belongs to the swap track.
  • Panic with nothing outstanding. A panic is right only for an internal invariant. The stated invariant ("a pending request is in the client's outbox or on the wire") does not hold, and an external condition reaches the panic (B1).
  • Reconnect coverage. diskserver's host tests cover Served's reconnect decisions over a scripted Link, and this PR does not change them. Nothing covers Session::reconnect's new body or what a live server answers it. By inspection the body is small and correct on its own. B2 covers what it actually meets after a silence end.
  • Deleted records. nothing-reopens-a-diskserver-session-over-a-used-page.md's exit is met by elimination. Every session is opened over Region::create(), so no client sends a page a server has held, and the client half that record owed a test for no longer exists. The other two records' exits are met:
    • a_server_past_the_request_ring_ends_the_session, red under control m1;
    • a_write_answered_lost_is_answered_by_the_sessions_end, red under control m2.
  • Evidence.
    • Targeted tests and clippy at 7c6717a exit 0.
    • The three controls are red at the named tests.
    • nvme_disk_boots passed at 7c6717a (run 43049658371, 1 passed, all steps green).
    • The full host run was at the pre-rebase 500507174. The rebase touched none of these files, so reusing that run is sound.
  • Size. About 90 production lines in ends.rs replace session.rs's pump/drain loop, a small net addition. Tests grow by about 240 lines.

Required

B1. system/diskserver/src/session.rs:269-274, 302: Session::wait panics when an answer is waiting.

  • Problem. exchange collects answers before its second pump. Client::next_request itself answers a caller's write Device when the client's own flush has failed past MAX_ATTEMPTS (lib/blockring/src/client.rs:294-297). That answer is not collected. If it empties the outbox, owe sets owed_since = None, waited.answers is empty, and patience.expect(...) panics while the answer sits in client.answers.
  • Trace.
    1. Kept writes reach KEPT_BLOCKS, so the client's own flush is Asked(4) on the wire, and write W waits behind it in the outbox.
    2. The device answers that flush Device (the fifth failure). lost() requeues the kept writes as reissues, and the client's flush becomes Failed. The second pump sends the reissues, and the wait blocks.
    3. The reissues are answered Ok. No caller is answered. The second pump then finds W at the limit with Failed, pops it and answers it Device. The outbox and the wire are empty, so patience is None and the expect panics.
  • Consequence. A device whose flushes keep failing crashes the file server. An external condition reaches this, and the PR's Risks bullet calling it unreachable is false. Before this PR the same path blocked forever.
  • Fix. Collect after the final pump. end() already collects on every error path, so pump?; receive?; pump?; collect is enough. The expect then guards a real invariant. The Risks bullet needs correcting.

B2. lib/blockring/src/ends.rs:18-21 and system/diskserver/src/session.rs:21-26: the silence bound's stated basis and its consequence are not what the code meets.

  • (a) The basis. The comment says a server silent for several times NVMe's 10 s "is not waiting on a device".
    • The NVMe driver takes a command back by a synchronous controller reset: reset → Controller::reset → enable/settle.
    • Each CSTS.RDY transition is bounded by CAP.TO, which NVMe Base 2.0 §3.1.4.1 allows up to 255 × 500 ms. The driver answers nothing during the reset.
    • usbd's recovery ladder adds several 10 s rungs to COMMAND_NS.
    • So 60 s is a policy below the drivers' worst case, not above it.
  • (b) The consequence. After a silence end, the reconnect goes to the same live driver.
    • Sessions::place refuses the open Held while the old session still has the unanswered request in flight (lib/blockserve/src/sessions.rs:166-170). retire (line 356) releases the partition only once nothing is in flight.
    • Served::progress then marks the disk gone for the client's life (system/diskserver/src/disk.rs:114-119).
    • Where the server is still responsive and holds the request (usbd mid-ladder, or a server that kept it), a silence end makes the disk gone. It does not lead to "requests refused, reissued and asked again" as the PR says.
    • The stated reason for the fresh region ("its device may still be writing there") cannot arise with a ToyOS server on the same partition. Its open is refused until the old session has retired and its region has left the domain.
  • Required. State the policy and its consequence truthfully in both comments and the PR description: what the bound is set against, and that a silence end with the request still held makes the disk gone. Whether 60 s should exceed the drivers' worst-case answer time is a policy decision for its owner. If 60 s stays, say so.

B3. lib/blockring/src/client.rs:456-457 with system/diskserver/src/disk.rs:150-156: a caller's flush and a reissued write are never bounded across repeated session ends against a live server.

  • Problem. Three counters exist and none applies here:
    • session_ended requeues flushes and reissues without counting the end.
    • Served::ask counts only Refused answers.
    • MAX_ATTEMPTS counts only device answers.
  • Scenario. A server answers a flush or a reissue with a violation in every session. Its own session then has nothing in flight, so it retires and the reconnect is accepted.
  • Consequence. The caller is held forever in a reconnect loop. Each iteration allocates a 2 MiB region and prints two lines. With a reissue outstanding, every user request behind it is never sent and never refused either. This contradicts the title and the closed record's exit ("the caller's call is refused"). The tests show that exit only for a read.
  • Required. Choose one:
    • In session_ended, count each end a flush or reissue was on the wire for against MAX_ATTEMPTS (a flush is answered Device; a reissue is given up), with a host test.
    • Or narrow the PR's claim and file the remaining case in issues/ with its exit.

B4. lib/blockring/src/client.rs:180-189 against the module contract at lines 5-10: admits and its comment disagree with the contract.

  • Problem. admits accepts Lost for a reissued write. Servers answer Lost only for a flush (lib/blockring/src/server.rs:119-122). The module now promises that a word no server says of a request is Violation::Entry.
  • The comment. "Lost is a flush's word, and one a reissue is sent after" seems to explain the exception but does not. That a reissue is sent after Lost does not mean Lost is said of a reissue.
  • Required. One rule in code and comment. Either judge Lost by the op (Op::Flush only) and extend the ReadOnly test to cover it, or state why a reissue answered Lost counts as a device failure. If B3 is filed rather than fixed, making it a violation feeds that loop.

Suggestions

  • S1. B1 lives in the one loop no host test reaches, because Session holds a Region and a Bell. If the decision step moved into ClientEnds as a tested "what next" result, or the loop were made generic over its bell, a host test could hold "a blocking wait never returns with an answer uncollected".
  • S2. lib/blockring/src/ends.rs, a_server_that_frees_no_request_entry_is_owed_for_what_cannot_go: the for round in 0..2 { … if round == 1 { break } } shape hides a two-step scenario. Writing the two steps out would read more plainly.

Landing

nvme_disk_boots passed at 7c6717a. B1's fix changes Session::wait, which that row runs for every request, so the row must run again on the fixed head.

SEND BACK

Japabu added a commit that referenced this pull request Nov 11, 2026
…ion's end is an attempt

B1. `Session::wait` collected answers between its two pumps and decided
after the second. The second pump can answer a caller itself: a write
that meets the kept limit once the client's own flush has failed past
`MAX_ATTEMPTS` is answered `Device` unsent. With that the outbox and the
wire were empty, nothing was owed, no answer had been collected, and
the blocking wait's `expect` panicked over an answer it never took.

The decision moves into `ClientEnds::exchange`, pure and host-tested:
send, receive, send, and only then answer `End(Violation | Silence)`,
`Answered` (any answer decided, the last send's included), `Wait(ns)`
or `Idle`. `Session::wait` collects after every exchange and acts on
its answer. `an_answer_the_last_send_decides_is_never_waited_past`
drives the review's trace over a real page; deciding before the last
send turns it red.

B3. A session's end requeued the flushes and rewritten writes it took
uncounted, so a server that ended every session over one of them held
it, and every request behind a rewritten write, through endless
reconnects. A session's end is now a failed attempt of each flush and
rewritten write it took, as a device's refusal is: past `MAX_ATTEMPTS`
the flush is answered `Device` and the write is given up. A session
that ended with nothing on the wire, a server past the request ring
before anything went, ended over the first request waiting, which is
treated as if it had been on the wire; without that, such a server held
every caller through reconnects too.

The blockring model's durability law counted failures one each against
`MAX_ATTEMPTS`; with a death now an attempt, its bound of two resets
and two deaths gives a flush up, since a reset costs a flush two
attempts (the request it fails, and the loss the next flush is told
of). The law now weighs failures by the attempts they can cost, and the
give-up script is also held at the boundary, a reset and two deaths,
which must never give a flush up.

B4. One rule in `admits` and the module contract: a server says `Lost`
only of a flush and `ReadOnly` only of a caller's write. A rewritten
write answered `Lost` is a violation, which B3 bounds.

B2. The silence bound's comment states what it is set against: it sits
below the NVMe driver's worst-case silence, a controller reset whose
steps are each bounded by `CAP.TO`. Its value is left at 60 s for the
coordinator's decision. The session module says what an end leads to:
the reconnect reaches the same running server, which refuses the open
`Held` while it holds a request of the ended session, so the disk is
gone and nothing is issued again. The fresh region's reason is the true
one: no server's write to an earlier session's page reaches the next,
whether or not that server has let the page go.

`Session::peak_on_the_wire`, which nothing read, goes with the pump it
was measured in.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Japabu and others added 4 commits November 11, 2026 19:30
A client of the block service could be held for good three ways, each by
a server that keeps its connection:

- `Session::pump` met the transport's `HeadPastTail` from the request
  ring's `space`, stopped pushing and left it to "the next drain", which
  reads only the completion ring. The wait then blocked for `u64::MAX`.
- `Client::complete` took a completion's tag off the wire before it
  judged the status. A read or write answered `Lost` (and a read, a
  reissue or a flush answered `ReadOnly`) was refused as
  `Violation::Entry` with its request already gone from the wire, so the
  session's end answered nobody: the caller's ticket stayed pending
  across every reconnect, a refused reissue dropped the only copy of an
  acknowledged write, and a refused pushback flush left writes waiting
  on it for good.
- A server that took a request and answered nothing was waited on with
  no bound.

One mechanism now ends all three. `toyos_blockring::ends::ClientEnds`
is the client's two ring ends over the session page, pure and
host-tested: `send` and `receive` return the transport's or the
client's `Violation` at the call that met it, and `patience` counts
`SILENCE_BOUND_NS` (60 s, a policy: several times the 10 s after which
the NVMe driver takes a silent command back and answers it itself) from
when the client came to be owed a completion or last took one. A look
that takes no completion renews nothing, so a server ringing the bell
buys no time. `Session::wait` ends the session on a violation or on
patience run out exactly as on a hang-up, and `Client::session_ended`
answers every request on the wire `Refused`, reissues what was
acknowledged and asks waiting flushes again.

`Client::complete` judges the status against the request before the tag
leaves the wire (`Inflight::answer_admitted`), so a refused completion
leaves its request for the session's end. The arms that returned
`Violation::Entry` after the take are unreachable and say so.

A server ended for breaking the protocol or for silence is still alive:
it may hold the region, and its device may still be moving data into
the arena. `Session::reconnect` therefore opens over a fresh region, not
the one the last server held. That also removes the case
`issues/nothing-reopens-a-diskserver-session-over-a-used-page.md` owed a
client test for: no client sends a used page any more. `reconnect` no
longer pumps; what waits goes out at the next `wait`, where a violation
has somewhere to go.

The server's side needed nothing: `toyos-blockserve` pulls, never waits
on a client, and closes a session on any cursor or entry violation.

Tests, all on the host over a real session page with the server's ends
driven by the test (`lib/blockring/src/ends.rs`): a server cursor past
the request ring, a completion tail past the ring's depth, a second
answer for one tag, an answer for a tag never asked, words that are no
completion, a write answered `Lost`, an answer never given, a
completion renewing patience, and a server that answers but never
frees a request entry. Negative controls, each a patch applied, built,
run red and reverted: `send` ignoring the request ring's violation reds
`a_server_past_the_request_ring_ends_the_session`; the tag taken before
the status is judged reds
`a_write_answered_lost_is_answered_by_the_sessions_end`,
`read_only_answers_a_write_alone` and
`a_reissue_refused_read_only_is_kept`; every look renewing patience
reds `a_dropped_answer_runs_out_the_servers_patience`.

Closes issues/a-block-session-waits-forever-on-a-server-past-its-submission-ring.md,
issues/a-read-or-write-answered-lost-is-never-answered.md and
issues/nothing-reopens-a-diskserver-session-over-a-used-page.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ion's end is an attempt

B1. `Session::wait` collected answers between its two pumps and decided
after the second. The second pump can answer a caller itself: a write
that meets the kept limit once the client's own flush has failed past
`MAX_ATTEMPTS` is answered `Device` unsent. With that the outbox and the
wire were empty, nothing was owed, no answer had been collected, and
the blocking wait's `expect` panicked over an answer it never took.

The decision moves into `ClientEnds::exchange`, pure and host-tested:
send, receive, send, and only then answer `End(Violation | Silence)`,
`Answered` (any answer decided, the last send's included), `Wait(ns)`
or `Idle`. `Session::wait` collects after every exchange and acts on
its answer. `an_answer_the_last_send_decides_is_never_waited_past`
drives the review's trace over a real page; deciding before the last
send turns it red.

B3. A session's end requeued the flushes and rewritten writes it took
uncounted, so a server that ended every session over one of them held
it, and every request behind a rewritten write, through endless
reconnects. A session's end is now a failed attempt of each flush and
rewritten write it took, as a device's refusal is: past `MAX_ATTEMPTS`
the flush is answered `Device` and the write is given up. A session
that ended with nothing on the wire, a server past the request ring
before anything went, ended over the first request waiting, which is
treated as if it had been on the wire; without that, such a server held
every caller through reconnects too.

The blockring model's durability law counted failures one each against
`MAX_ATTEMPTS`; with a death now an attempt, its bound of two resets
and two deaths gives a flush up, since a reset costs a flush two
attempts (the request it fails, and the loss the next flush is told
of). The law now weighs failures by the attempts they can cost, and the
give-up script is also held at the boundary, a reset and two deaths,
which must never give a flush up.

B4. One rule in `admits` and the module contract: a server says `Lost`
only of a flush and `ReadOnly` only of a caller's write. A rewritten
write answered `Lost` is a violation, which B3 bounds.

B2. The silence bound's comment states what it is set against: it sits
below the NVMe driver's worst-case silence, a controller reset whose
steps are each bounded by `CAP.TO`. Its value is left at 60 s for the
coordinator's decision. The session module says what an end leads to:
the reconnect reaches the same running server, which refuses the open
`Held` while it holds a request of the ended session, so the disk is
gone and nothing is issued again. The fresh region's reason is the true
one: no server's write to an earlier session's page reaches the next,
whether or not that server has let the page go.

`Session::peak_on_the_wire`, which nothing read, goes with the pump it
was measured in.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ws it keeps the bound

Review r1's B2, decided by the coordinator: the client's silence bound is
a failure timeout for a hung server, so it sits above the longest a
server may legitimately stay silent, and that is the protocol's to state
and each driver's to show.

`toyos_blockring::SILENCE_BOUND_NS` is now the protocol's contract:
while a server owes a session an answer it posts a completion, or ends
the session, within it. `ClientEnds` ends a session silent longer, as
before; the bound moves from the client's module to the protocol's.

The NVMe driver keeps it. A command unanswered for `COMMAND_SILENCE`
(10 s) is taken back by a controller reset, which answers nothing until
it is done. Its two `CSTS.RDY` transitions stay bounded by `CAP.TO`, at
most 255 × 500 ms (NVMe Base 2.0 §3.1.4.1); its nine admin commands
(Set Features for the queue count, and a completion and a submission
queue for each of four) were bounded by `CAP.TO` too, which the
specification gives them no reason to be, and now by the driver's own
command policy, `ADMIN_BOUND` = `COMMAND_SILENCE`. `ANSWERED_WITHIN`
adds them up, 10 + 2 × 127.5 + 9 × 10 = 355 s, and a compile-time
assertion holds it under `SILENCE_BOUND_NS`, which is 355 s: no
rounding up.

`admin_command` checked the bound and then waited a whole bound for an
interrupt, so a wake just before the bound bought a second one and a
step could run to twice its bound. `within` waits only for what is left
of the bound; `admin_command` and `settle` both wait through it.

Tests, on the host: `a_command_left_silent_is_answered_within_355_s`
holds the arithmetic; `a_wake_before_the_bound_buys_no_second_bound`
wakes one look 1 ms early and holds the whole to the bound, red with
the old wait; `an_answer_ends_the_wait`.

usbd cannot show the same: its device serves one command at a time from
a queue every session on it shares, so a session's request can wait
behind others' for minutes with nothing failing.
`issues/a-usb-disk-session-can-wait-past-the-block-silence-bound-behind-other-sessions.md`
records it, with the NVMe driver's own gap at the edge (its figure
counts from a command's submission, not from the request's
publication) and its exit. The manual's storage page states the
requirement.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…bound counts APST's

On #1131: the reset also sets the APST table, a tenth admin command. The
reset's count was written by hand beside the code that issued them, so
nothing would have noticed it go stale.

`make_io_queues` and `set_power` become `Controller::configure`, which
bring-up and the reset both call. It steps through `STEPS`, a const list
of the queue count, a completion and a submission queue for each of
`MAX_IO_QUEUES`, and the APST table, and issues at most one admin
command per step and none besides. `RESET_ADMIN_COMMANDS` is the list's
length, so `ANSWERED_WITHIN` follows the code: 10 + 2 × 127.5 + 10 × 10
= 365 s. `toyos_blockring::SILENCE_BOUND_NS` is 365 s, no rounding up.

Tests, on the host: `a_command_left_silent_is_answered_within_365_s`
and `a_reset_steps_through_the_queues_then_the_power_table`, which holds
the list's order. A step added to the list fails the build at the bound's
assertion.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Japabu
Japabu force-pushed the block-client-never-waits-forever branch from 941ac6f to 31fda55 Compare November 11, 2026 17:33
@Japabu

Japabu commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Follow-up review of #1235 at 31fda55

Scope. This review covers B1–B4 from the first review and the code changed for them. I read:

  • the three commits since 7c6717a;
  • ends.rs, client.rs, model.rs, lib.rs, session.rs, nvme.rs and main.rs in full at the head;
  • blockserve's Sessions::pull and designate, Bell::wait_for and block_on;
  • the evidence at 32c72ecb: the r2 controls and first tries, the r2 CI-feature logs, and the r4 host checks and controls.

Architectural assessment

  • The decision is in the right place. What the client does next is decided in ClientEnds::exchange, which is pure and host-tested. Session::wait only acts on that decision. This was S1 of the first review, and it removes the class of bug B1 was.
  • B3 adds no state. It reuses the existing per-request counters (MAX_ATTEMPTS, MAX_RECONNECTS).
  • The silence bound is now the protocol's, and each driver must show it fits.
    • The NVMe driver's proof is a const sum over its own bounds, which the build checks.
    • STEPS ties the admin-command count to the code that issues the commands. It costs about 40 production lines (the Step enum, the const list, named). In return the figure cannot go stale, which a hand count did once nvme: the controller's own idle power states, by APST #1131 added a tenth command. Worth it.
  • Size since 7c6717a.
    • blockring and diskserver: +385/−155, mostly tests.
    • nvme.rs production: 762 → 861 lines; tests +57.

First review's blockers

B1: resolved.

  • exchange sends, receives and sends again, and only then decides. Answered wins over Wait whenever an answer is decided.
  • Session::exchange collects after every exchange, and end collects after session_ended. No path returns while an answer sits in the client.
  • I traced the first review's sequence through an_answer_the_last_send_decides_is_never_waited_past. On the fifth attempt the second send pops write 2 at the limit with Pushback::Failed and answers it Device. The outbox and the wire are then empty and patience is None. The test requires Answered on that exchange.
  • Control m4 (decide before the last send) reddens it (r2, 4eb8b1f). Since that revision, ends.rs and session.rs changed only by the bound's move, so the control still applies.
  • With this ordering the Idle if block panic guards a real invariant.
  • On expiry, Bell::wait_for returns Ok(false), so the loop goes back through exchange and ends as Silence with its message.

B3: resolved. I checked the rule and the law.

The rule.

  • session_ended charges what the end took through reissue_failed, flush_failed and pushback_failed. These are the same counters a device's refusal uses.
  • The three B3 tests are red under m5.

The empty-wire extension.

  • After an exchange, requests stay in the outbox with nothing on the wire only when the server has returned no request-ring room, or holds a cursor past the ring.
    • With nothing on the wire, all arena room is free.
    • The limit wait and the reissue wait always put their flush or reissue at the front, where it goes.
  • So the popped front is the request that server held:
    • a caller's request is answered Refused and counted by Served::ask;
    • a flush, reissue or pushback is counted as above.
  • m6 reddens the test.
  • lost() now runs before the taken requests are handled. requeue_reissue still inserts ahead of any flush, so "reissues first, then flushes" holds whatever order taken is sorted in.

The law. attempts = 2·resets + crashes + errors. Each weight is an upper bound on the attempts one flush or one write can lose:

  • Reset. abort_all answers what the device held Device: one attempt. The loss count it bumps makes Holds::flushed answer the next flush Lost, once: a second attempt, for a flush only. lost() requeues acknowledged writes without touching attempts. A completion the device posted before the reset finds no tag.
  • Death. session_ended charges each request it took once. The popped front is taken only when the wire is empty. The model's next server starts with fresh Holds, so a death brings no later Lost.
  • Device error. It charges one request once.

may_give_up only permits a give-up. The law's strength lies in the bounds that forbid one:

  • SCRIPT's four (at most 2 attempts);
  • ONE_BLOCK (1,1,0), at 3;
  • the new boundary (1,2,0), at exactly MAX_ATTEMPTS.

If any weight were too low, the exhaustive search would find a give-up at one of these bounds.

The first-try log shows the law had to move. Under the old count, (2,2,0) reached a legitimate give-up in five attempts: two deaths over the flush, then a reset's Device, the Lost after it, and a second reset's Device.

CI's three blockring mutations still redden the model tests build/ci.rs names (r2/ci-features).

B4: resolved.

  • admits accepts Lost only for Op::Flush, and ReadOnly only for a caller's write.
  • This matches the module contract (client.rs:9-10) and the server. server.rs posts Lost only for a flush and ReadOnly only at take. A reissue meets ReadOnly only if a later session's grant shrank, and B3 bounds that loop.
  • m7 reddens both tests.

B2: resolved in code. One statement of the contract is not (B5).

  1. configure preserves nvme: the controller's own idle power states, by APST #1131. I compared it with main's make_io_queues and set_power:
    • The commands and their order are the same: the queue count, a CQ and an SQ for each granted queue, then APST.
    • The table is still sent only under Power::Autonomous, with APSTE set only when the plan holds a transition.
    • Only APST's Refusal::Admin becomes Some(status). Every other refusal still fails bring-up or panics the reset, APST's Silent included.
    • open and reset hand the status unchanged to say_power and say_refused; main.rs changed only its import.
    • The remaining differences are harmless. io.clear() now comes before the count, which is moot because an error fails the whole operation. Each queue page is zeroed just before its own create command, and the controller reads neither page before that.
  2. The 10 s admin bound at bring-up is safe.
    • NVMe Base 2.0 gives no completion time for any command, admin or I/O. issues/the-nvme-driver-resets-a-drive-that-is-slow-and-healthy.md says the same.
    • CAP.TO (§3.1.4.1) is defined only as the worst case for CSTS.RDY to follow CC.EN. The old admin bound therefore borrowed a figure the spec defines for a different event. Because the old loop waited a whole further bound after each check, it allowed up to twice that figure.
    • The driver enables with CC.CRIME clear: CC_ENABLED sets only EN, IOSQES and IOCQES, leaving bit 24 clear. So CSTS.RDY = 1 means the controller is ready with its media. Identify, Set Features and Create I/O queue have nothing slower behind them.
    • So 10 s is policy, as the code says. It is generous: it exceeds the old bound wherever CAP.TO is under 10 s.
    • A slower controller is refused NOT SERVING at bring-up. In a reset, Silent panics the driver and ends its sessions. Both failures are clear, and both are inside the contract. The Risks bullet records this.
    • within reaps once more after its last wait, so an answer that arrives at the bound is taken. n1 reddens the wake test; n2 and n3 fail the build at the assertion.
    • Caveat: the spec statements are from my reading of Base 2.0's CAP and CC definitions. I did not re-read the spec in this session.
  3. Where the contract is stated.
    • It is stated correctly in lib.rs's module doc ("from a request it has not answered, or from its last completion"), the PR body, and the ends and session docs.
    • The manual's storage page states it more narrowly. See B5.

Required

B5. docs/src/architecture/storage.md:26-30, lib/blockring/src/lib.rs:64-67 and the new usbd record each say less than the contract does, or less than the record shows.

  • The manual.
    • What it says. A driver answers "every request it holds". The worst case it must show is "a device's silence before the driver takes a command back, and taking it back".
    • What the protocol says. lib.rs:32-36 counts from "a request it has not answered", and the client enforces that: owed_since starts when the client is owed, at publication or earlier.
    • The gap. A request a session has published but the driver has not yet pulled is not "held". Its wait behind other sessions' requests, or behind a reset during which pull does not run, is exactly the gap the new usbd record files for both drivers. That record leaves open whether to show the whole path or to redefine the bound so other sessions' work does not count.
    • Consequences. The manual already reads as the second choice and treats the NVMe figure as all there is to show. A driver that meets the manual can still have its sessions ended by a client that keeps the protocol.
    • Fix. State the requirement from a request's publication, with the wait to be pulled inside the worst case. The record keeps the fact that neither driver shows that yet.
  • SILENCE_BOUND_NS's doc.
    • "set by the slowest server: the NVMe driver … which it shows fit" is false by the PR's own record:
      • usbd is slower, at about 16 minutes;
      • the NVMe figure covers a command only from its submission to the device.
    • Say what the bound is actually set from: the NVMe driver's figure from submission, which is its silence before the reset plus the reset.
  • The record.
    • issues/a-usb-disk-session-can-wait-past-the-block-silence-bound-behind-other-sessions.md has no severity line, which issues/README.md reads as "none applies".
    • The consequence it describes is a disk gone for the client's life while the client keeps acknowledged writes no flush has covered. The next flush reports them not durable. That is README's data-loss ("accepted data later reported lost").

All three are text edits; nothing needs to run again.

Suggestions

  • S1. system/nvme/src/nvme.rs:172 with lib.rs:68: "no rounding up" leaves the figure exactly equal to the bound, but ANSWERED_WITHIN sums only the driver's waits. These fall outside it:

    • the serve loop's lag between COMMAND_SILENCE and reset;
    • the reset's own register and page work;
    • posting and ringing after disk.reset();
    • the client's clock, which starts just before its publish.

    Each is small, but with zero slack the assertion is not quite the proof the PR body calls it. Name these in the record beside the pull-wait gap, or keep a stated margin. This is optional because the record's exit already asks for the figure from publication to completion.

  • S2. lib/blockring/src/model.rs:727-728: "a reset and two deaths, the most failures of those kinds that never give a flush up" is not accurate, because four deaths also never give one up. Say what the bound is: failures that cost exactly MAX_ATTEMPTS attempts, the boundary where the law forbids a give-up.

Evidence and landing

  • Host evidence. The host tests and clippy in r4 are at 31fda55: blockring, diskserver, toyos and nvme, exit 0. The r2 controls ran at 4eb8b1f. Since then, the files they exercise changed only by the bound's move, so reusing them is sound.
  • Guest row. nvme_disk_boots, run 43069269031 at 31fda55, is still queued (read with gh run view).
    • It is the landing precondition the first review named.
    • It now also runs bring-up through configure and within.
    • B5's text edits do not invalidate it.
  • Metal. Every bench boots through this driver's bring-up, which now runs under the 10 s admin bound and the new wait.
    • QEMU establishes the order, not real controllers' timing.
    • Whether the next bench boot comes before landing is the orchestrator's call. I see no reason to expect a difference.

SEND BACK. The only blocker is B5, three text edits. Once B5 is fixed and the guest row is green, this is LAND.

…lication

B5. The manual's storage page said a driver answers every request it
holds, and that its worst case is a device's silence and the driver
taking a command back. The protocol counts from a request's
publication, and the client enforces that: the wait to be taken from
the session is inside the bound. The page now says so.

`SILENCE_BOUND_NS`'s doc said the bound is set by the slowest server,
which the PR's own record refutes: usbd is slower, and the NVMe figure
counts from a command's submission. It now says it is set from the
NVMe figure, counted from submission, and points at the record for
what that leaves out.

The usbd record's consequence, a disk gone while the client keeps
acknowledged writes no flush covered, is accepted data later reported
lost: `severity: [data-loss]`. It also names what the NVMe figure
leaves out besides the wait to be pulled: the serve loop's lag before
the reset, the reset's own work, posting its answers, and the client's
clock starting before it publishes (S1).

S2: the model's (1,2,0) bound is failures costing exactly
`MAX_ATTEMPTS` attempts, the boundary where the law forbids a give-up,
not the most failures of those kinds that never give one up.

Text only.

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

Japabu commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Orchestrator: verified 97c93a6 against follow-up r2's B5. git diff 31fda554e 97c93a6d2 changes only comments in the .rs files (blockring lib and model docs, nvme.rs), the manual's storage page (the bound counts from publication), and the usbd record (with a severity line and the delays outside the figure named). No code changed. Lands when guest 43069269031 is green and one metalcase boot per bench at the final head passes, which brings up NVMe under the 10 s admin bound.

Japabu and others added 2 commits November 11, 2026 21:58
Both benches are down. The T14's metalcase boot at 97c93a6 passed,
its controller up under the 10 s admin bound; the AMD bench's boot is
owed, recorded with what main must show.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#1236 merged before its owed list reached its branch's record. The
osbench_disk A/B of main before and after it, on both benches, goes
into the DATA rows' record, which this branch and main both carry,
with the readdir.1000 and open_close/stat conditions.

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

Japabu commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Orchestrator verification, landing under the benches-down rule (metal unavailable; review + host + guest, metal owed):

  • Review r2: B1–B4 fixed, B5 text verified (earlier). Head 19c3b5f vs guest-tested 31fda55: comments, the manual's storage page and issue records only; no non-comment Rust line changed (verified with git diff).
  • Guest run 43069269031 at 31fda55: success (nvme_disk_boots, all five steps).
  • T14 metalcase at code-identical 97c93a6 passed before the benches went down; the AMD boot of main is owed, as recorded. The record also carries A DATA listing reads its inodes in a pass, not one from the root each #1236's owed osbench_disk A/B (its branch merged before the ruling).

@Japabu
Japabu marked this pull request as ready for review November 11, 2026 20:03
@Japabu
Japabu added this pull request to the merge queue Nov 11, 2026
@Japabu
Japabu removed this pull request from the merge queue due to a manual request Nov 11, 2026
@Japabu
Japabu added this pull request to the merge queue Nov 11, 2026
Merged via the queue into main with commit 1063c5b Nov 11, 2026
10 checks passed
@Japabu
Japabu deleted the block-client-never-waits-forever branch November 11, 2026 21:48
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