Skip to content

A descriptor call holds its object without a lock - #1234

Merged
Japabu merged 3 commits into
mainfrom
fd-hold-without-lock
Nov 11, 2026
Merged

Japabu merged 3 commits into
mainfrom
fd-hold-without-lock

Conversation

@Japabu

@Japabu Japabu commented Nov 11, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

A read or write through a descriptor took the process's one descriptor-table lock to take its hold and again to give it up, so two threads doing I/O on different descriptors met on it twice a call. In #1150's metal A/B the pipe benchmark's two threads slept on that lock 128,000 to 153,000 times a run once the kernel stopped serializing their calls.

Result

A descriptor call's hold takes no lock, except on the table's first use and when its word names a free slot, where it takes the writer lock once to settle. The table's representation carries the hold: a descriptor's word names an object's slot, and a slot counts its references (the descriptors naming it and the calls holding it).

  • A call counts itself onto its slot only while the count is not 0 (compare-exchange), then keeps the reference only if the descriptor still names that slot. Giving it up is one subtraction.
  • Only the writer takes a count off 0, after writing the object, so nothing is written under a reader and a slot is reused only after its last hold.
  • Whoever takes a count to 0 gets the object and lets it go: once, after the last call.
  • A descriptor changes in one store: a call beside dup2/replace holds the old object or the new; a closed number is free at once (lowest-free numbering unchanged).

Changing a descriptor (install, dup, dup2, close, replace, the inheritable flag) still takes one lock, as a Writer handed to one holder at a time; the mutators are its methods, so none is reachable without it. Endowed descriptors are still installed under that lock on first use; a call checks one Acquire flag to know that happened. A call whose descriptor names a free slot takes that lock once, and the writer decides: a descriptor closed meanwhile is refused, and a broken table panics, as it did on main.

Closes issues/every-descriptor-call-takes-one-process-wide-lock-twice.md. The manual describes no descriptor-table locking, so it is unchanged.

Invariants and where they are held

Invariant Held by
No use after close A call's reference keeps the slot's count above 0; only the writer takes a count off 0 (hold), and the reader's object read precedes its decrement (AcqRel), which the writer's Acquire load of 0 orders before its write. fd-reuse-relaxed shows the models red without that edge.
A closed number is free at once; a slot is reused only after its last hold close clears the word at once, as main does and POSIX requires (libc's poll relies on it); the slot stays counted until the last unuse. The stronger form, "a number is reused only after its holds are released", does not hold here and should not.
dup/dup2/close/replace/flag semantics Unchanged logic, now Writer methods; host unit tests in fd::table::tests.
Spawn's endowment snapshot Unchanged: libc's and std's spawns read each descriptor with a separate copy and inheritable call; it was never one atomic snapshot of the table.
No lock across a blocking call, but at first use A call's hold takes no lock (except on a free slot, where it settles once under the writer lock, which it holds across no blocking call), and a change lets released objects go after its lock. The exception is the table's first use, which installs the spawn-given descriptors under the writer lock: files::adopt_exported asks a file server (Dir::adopt, fstat) while it is held. Main did the same under its TABLE lock, and every other call has to wait for the endowment anyway.

Consequential decisions

  • Slot per cache line, paid in committed memory. Each slot is #[repr(align(64))], so calls on two objects write no line they share (two descriptors that dup one object share its slot). The table is .bss in a loaded segment, which is committed memory: 272 KiB per process (16 KiB of words, 4096 × 64 B of slots) against main's 136 KiB (u16 words and 32-B entries in .data). Of the +136 KiB, 128 KiB is the price of the padding: natural 32-B slots would cost 144 KiB but put neighbouring objects, such as a pipe's two ends, on one line. The padding is not measured separately; the A/B below cannot isolate it. Physical frames are similar on both sides, since only touched pages are populated.
  • A free slot under a call is settled by the writer. A descriptor that names a free slot is a close in flight or a broken table, and only the writer can tell them apart. Table::acquire answers None there, and the owner holds the descriptor under the writer lock in a cold, out-of-line function the loop tail-calls. Rereading the word lock-free, as d8f9a49 did, would spin forever on a broken table, which is an internal invariant violation and must fail loudly. The settling path costs the lock-free loop one more callee-saved register saved and restored, and a frame of 0x20 bytes instead of 0x18 (below).

Review r1

  • Blockers: the slot comment says two objects; the model's header no longer claims every memory order (loom does not explore load buffering) or a control map it does not keep (build/ci.rs's CONTROLS is that map); this description states committed memory, the first-use exception and the corrected invariant 2.
  • S1, taken: fd-reuse-relaxed makes the writer's free-slot load Relaxed; both reuse cases red with loom's "Causality violation: Concurrent read and write accesses to UnsafeCell".
  • S2, taken: the reuse case closes the second descriptor before the call ends, so it also covers a call left the object its own descriptor never named.
  • S3, taken (dafb2e8), in the shape that disturbs the hot loop least; four shapes were tried.
  • S4: Word is implemented on loom's atomic directly. One Word trait across fd::table, transport and log::ring is declined here: their operation sets differ (fetch_sub here, swap in the transport), and merging them changes two other modules' seams this PR does not need.
  • S5, taken: ObjectCell and TableWriter.
  • S6: recorded as issues/calls-on-two-served-files-meet-on-one-process-wide-lock.md.
  • S7: the 128 KiB is stated above as the padding's price.

Checks

At dafb2e8 (base 34cb4c9; main has changed none of these files since, and the merge is clean). Host checks: evidence r2. host-35d452b2a/ ran at the round's first commit. host-s3-patch/ ran the same checks on that commit with S3's patch applied, which is byte-for-byte dafb2e8's tree; S3 changes nothing the build-system test reads. Metal: evidence r3. r1, at d8f9a49, is beside both.

Chosen under the owner's targeted-testing rule: the change is the SDK's descriptor table, its owner module, four rows of build/ci.rs's CONTROLS and one issue record. The full cargo run -- --ci host was not run; the merge queue runs it.

Check Why Result
cargo test -p toyos --lib fd:: the table's host tests and its five loom models, the settling call among them exit 0, 16 passed (15 at 35d452b)
cargo test -p toyos --features fd-hold-revives --lib -- --exact <its 3 cases> negative control: a call counts itself onto a free slot exit 101; all 3 FAILED, each "object n was let go twice"
cargo test -p toyos --features fd-hold-unchecked --lib -- --exact <its case> negative control: a call keeps a reference without rereading the descriptor exit 101; FAILED, "a call on a closed descriptor held Some(1)"
cargo test -p toyos --features fd-publish-relaxed --lib -- --exact <its 2 cases> negative control: the writer publishes a count Relaxed exit 101; both FAILED, loom "Causality violation: Concurrent read and write accesses"
cargo test -p toyos --features fd-reuse-relaxed --lib -- --exact <its 2 cases> negative control: the writer finds a slot free by a Relaxed load exit 101; both FAILED, loom "Causality violation: Concurrent read and write accesses to UnsafeCell"
cargo test -p toyos-build --lib -- every_model_control_is_run ci:: the four CONTROLS rows match declared features, and the judge's own tests exit 0; every_model_control_is_run and the 20 ci:: tests passed (6 ignored are drivers that never run alone)
cargo clippy -p toyos --all-targets, -p toyos --lib --features fd-table, and -p toyos --all-targets --features <the four controls>, each -- <ADOPTED> -D warnings the toyos shapes of build/clippy.rs, the owner module only under fd-table, and the controls' arms exit 0, exit 0, exit 0; no warning

The control commands are the ones build/ci.rs's run_control issues; each named case's ... FAILED line is present and no run selected 0 tests, which is its judge.

The guest rows and the metal A/B measured d8f9a49, not this head. The landed hot loop differs from what they measured only by S3's register save. The ToyOS compiler from the sysroot built at d8f9a49 compiled the toyos crate with fd-table for x86_64-unknown-toyos at d8f9a49, at 35d452b and at dafb2e8's source (r2/disasm/). At 35d452b all eleven descriptor functions are byte-identical to d8f9a49 once crate hashes are masked: the nine toyos_fd_* entry points, toyos_fd_acquire's closure (the lock-free loop) and the owner's table. With S3 only that closure changes (before/after diff). It pushes and pops r12 as well, its frame is 0x20 bytes instead of 0x18, and the slot pointer moves from rsi to r8; the success path also gains one unconditional jump, the loop head loses its alignment padding, the result is stored differently and the path that loses a race saves one more register: about one instruction more than d8f9a49 on the success path, and the settling path is a tail call. The local build at d8f9a49 reproduces the production sysroot's toyos_fd_acquire, its closure, toyos_fd_unuse and toyos_fd_inheritable byte for byte. table and the writer-side entry points differ there only inside the writer lock's wait path, from std's own build of the crate.

Guest rows at d8f9a49: run 43036214335, green, 7 of 7:

  • libc_children (x86_64): libc_recv_beside_close (a recv blocked across its descriptor's close and the number's reuse), stdio_named, spawn_cloexec, poll_closed and the rest of libc's descriptor and spawn cases, in real processes.
  • stream_holders, netstack_holders_exported_to_children (x86_64): a stream's holders across processes, and descriptors exported to children.
  • a_launch_gives_each_unsent_stdio_slot_a_null (x86_64): the endowed descriptors installed on first use.
  • std_threads_count_the_cpus_and_give_back_ended_threads, osbench_capability_groups_exit_0 (x86_64): threaded std programs and osbench end to end.
  • virt_smp (aarch64, emulated): an aarch64 SMP userland boots through the new table. Emulation shows no weak ordering; the loom models check that.

Metal, at d8f9a49 (evidence r3), every boot's driver verdict passed:

  • osbench A/B: the osbench benchcase image, base 34cb4c9 against d8f9a49, 4 boots a side on the T14 and on the AMD bench, booted base-head-head-base twice. On pipe.*, ipc.* and fs.*read_4k*/write_4k* (25 rows a bench), the median p50 of each side's boots was compared. No head row is slower than base by more than base's own boot-to-boot spread, and none is over twice its machine's recorded value (the scoreboard's ToyOS median, where it lists the row). The small pipe writes the lock serialized gained the most:

    row T14 AMD
    pipe.throughput.64 114.5 -> 139 MB/s (+21%) 114 -> 144.5 MB/s (+27%)
    pipe.throughput.256 435.5 -> 528 MB/s (+21%) 461 -> 558.5 MB/s (+21%)
    pipe.throughput.1024 1595.5 -> 1841.5 MB/s (+15%) 1770.5 -> 2167.5 MB/s (+22%)
    pipe.throughput.4096 4873.5 -> 5362 MB/s (+10%) 3016 -> 3034 MB/s (within spread)
    fs.mem.read_4k_at 1039 -> 972.5 ns (-6%) 1477.5 -> 1383 ns (-6%)
    fs.disk.read_4k_at 1060 -> 966.5 ns (-9%) 1463 -> 1357.5 ns (-7%)
    ipc.reqrep.8 6186 -> 6113 ns (-1%) 6912.5 -> 6983 ns (within spread)

    The 128 KiB of slot padding is not isolated by this A/B.

  • T14 testcases shared boot: 256 jobs ended. 251 exit 0, and wake_onto_busy's 36 is its p99 in µs, which its row passes. The four reds are not this change's:

Unresolved

🤖 Generated with Claude Code

A `read` or `write` through a descriptor took the process's one
descriptor-table lock to take its hold and again to give it up, so two
threads doing I/O on different descriptors met on it twice a call. In
#1150's metal A/B the pipe benchmark's two threads slept on it 128,000 to
153,000 times a run once the kernel stopped serializing them itself.

The table's representation now carries the hold. A descriptor's word names
an object's slot, and a slot counts its references, the descriptors naming
it and the calls holding it:

- A call counts itself onto the slot its descriptor names only while the
  count is not 0, then keeps the reference only if the descriptor still
  names that slot. That is one compare-exchange and two loads; giving the
  hold up is one subtraction.
- A slot is free while its count is 0, and only the writer takes a count
  off 0, after writing the object. Whoever holds a reference reads the
  object, so nothing is written under a reader and a slot is reused only
  after its last hold.
- Whoever takes a count to 0 is answered the object, read while it still
  held its reference, and lets it go: once, after the last call.
- A descriptor changes in one store, so a call beside a `dup2` or a
  `replace` holds the old object or the new, and a closed number is free at
  once, as lowest-free numbering needs and as before.

What changes a descriptor (`install`, `dup`, `dup2`, `close`, `replace`,
the flag) still takes one lock, as a `Writer` the table hands out to one
holder at a time; the mutators are its methods, so none is reachable
without it. The lock is held across no blocking call: a released object is
let go after it, as before. The first use still installs the endowed
descriptors under it, and a call looks at one flag to know that happened.

Each slot is a cache line of its own, so calls on two descriptors write no
line they share. The table is all zeros now and costs an image no
initialised data; the old one's free objects were not.

A call that finds its descriptor naming a free slot reads the descriptor
again: that state is a close in flight, and no longer told from a broken
table on this path. The writer, under whose lock it cannot be in flight,
still answers it as broken.

`fd::table::model` runs a call beside a close, a close and a reuse of the
slot, a reopen of the number, a second call and a `dup2` under loom, the
object in loom's cell. Three controls show it can red: a call that counts
itself onto a free slot, one that does not look at the descriptor again,
and a writer that publishes a count unordered after its object.

Closes issues/every-descriptor-call-takes-one-process-wide-lock-twice.md:
a hold takes no lock at all.

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
@Japabu

Japabu commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Review of #1234 at d8f9a49 (base 34cb4c9)

I read the commit, the full diff, fd.rs, fd/table.rs, fd/table/model.rs, the callers (std on_fd, libc read/poll/sockets/spawn, files::owner, log::stdio), the loader's commit accounting, build/ci.rs's controls and build/clippy.rs, PR #1226's branch at ab77bd2, and the evidence at d229a62. I ran nothing.

Architecture

The fit is right. The table stays in the SDK copy that owns it. Nothing changes in the kernel, toyos-abi or the toyos_fd_* C symbols, and the manual describes no table locking. The representation (type-stable static slots, a count taken only from a non-zero value, then a recheck of the descriptor word) is the simplest lock-free design that keeps every capability: it needs no epochs, no hazard pointers and no heap. Making the mutators Writer methods puts the one-writer rule in the type system.

Size: production code is +186 lines (table.rs 190→351, fd.rs +25) and control wiring is +25. Tests are +259 (unit tests +31, model.rs +228). The issue file (−27) is deleted, and no citation of it remains in the tree. The production growth is the protocol plus two small seams for loom (Word, Place).

Invariants, checked against the code

  1. No use after close: holds. A call reads an object only while it holds a counted reference. Only the writer takes a count off 0 (hold, table.rs:255-264), so no call can revive a freed slot. The read in unref comes before its AcqRel decrement, and the writer's Acquire load of 0 synchronizes with that decrement through the release sequence, so the read happens before the writer's overwrite. The Release store of 1 in hold publishes the object to a call whose CAS reads that count. The count can go 1→0→1 (ABA) under a stale word; that case is harmless because the recheck then requires the descriptor to name the slot now.
  2. Number reuse. As the brief words this invariant ("reused only after every hold is released"), the code does not hold it, and it should not. A closed number is free at once and a slot is reused only after its holds. That matches main, matches POSIX, and libc's poll relies on it (posix_io.rs:990). The new unit assertion at table.rs:429-432 checks it. The PR body states the corrected form.
  3. dup / dup2 / replace / flag semantics: preserved. The Writer methods are line-for-line ports, and the ported host tests pass.
  4. Spawn snapshot: unchanged. libc (spawn.rs:294-295) and std (process.rs:502) read each descriptor with separate copy and inheritable calls, and each of those used to take the lock on its own. Each read is still one atomic word.
  5. No lock across a blocking call: holds except at first-use endowment. change holds WRITING across endow (fd.rs:421-433), and files::adopt_exported makes round trips to a file server under it (Dir::adopt and fstat, files/owner.rs:377-380). This comes from main's TABLE lock and is largely inherent, because every other call must wait for endowment anyway. The PR's invariant table states the rule without this exception.

Concurrency models and controls

The five cases cover the real races between a call and the writer:

  • close beside a call;
  • two calls beside a close;
  • a slot reused by another descriptor's object;
  • a reopen at the same number (word ABA);
  • a dup2 onto the descriptor being called.

A dup cannot race a close, because both are writer operations serialized by WRITING. A dup's count increment beside a call's CAS or decrement is plain RMW on one atomic and needs no model. replace and set_inheritable change the word in one store and then unname, which is the dup2 shape. The recheck ignores the flag. loom::model runs with no preemption bound here.

Each control fails for its stated reason (r1/host/control-*.log):

  • fd-hold-revives: the revived count is dropped to 0 again, giving "object n was let go twice". This is the only failure the revival can produce, because the recheck always fails after a close.
  • fd-hold-unchecked: "held Some(1)".
  • fd-publish-relaxed: a loom causality violation on the object cell.

None of the three removes the reader→writer edge that invariant 1 depends on (unref's release paired with hold's acquire); see S1.

Cost

The PR says the table's .bss is "address space". It is committed memory. A PT_LOAD segment is Kind::Copy, which is committed (kernel/pure/vm.rs:113-115): every page is promised a frame when the program loads, and the promise is refused when memory is short. The table is 272 KiB per process (16 KiB of words plus 4096 × 64 B of slots), against 136 KiB on main (u16 words plus 32-B Held entries in .data). That is +136 KiB committed per process, and 128 KiB of it is the align(64). Physical frames are similar on both sides, because only touched pages are populated.

Alternatives:

  • Natural 32-B slots cost 144 KiB, about main's figure, but two neighbouring objects share a line. A pipe's two ends usually sit in adjacent slots.
  • A lazily allocated second level adds pointer publication on the lock-free path.
  • 128-B padding costs more.

My judgment is that the padding is defensible on the call path but has not been measured separately. The A/B cannot isolate it (S7).

Overlap with #1226

The conflict is textual only. #1226 calls changed(fd) after the table operation, outside the lock, and changed clears the cached sink after any ask already under way. So #1226 does not rely on the table lock serializing acquire.

One placement rule matters when merging. changed(fd) must stay after change(..) returns, never inside its closure. #1226's ask holds ASKING and calls fd::hold, and under #1234 that call takes WRITING on a first use. A changed inside the closure would take ASKING under WRITING, which is an ABBA deadlock. The handoff's wording, "moves its calls onto change(..)", should say "after".

Evidence

Verified:

  • 15/15 host tests at d8f9a49 (rev.txt).
  • The three controls exit 101, each with its named FAILED line.
  • The toyos-build controls tests pass.
  • Three clippy shapes pass, and $CONTROLS in build/clippy.rs covers the new features.
  • Guest run 43036214335 checked out d8f9a49 and passed 7/7.
  • check-head/check-base are at d8f9a49 and 34cb4c9.
  • Image sizes match the PR body. The exact 2 MiB difference is the image's rounding; the per-binary change is about 136 KiB.
  • Main has not touched these files since the base, and the merge is clean.

The fixes required below touch only comments and the description, so the guest and metal evidence at d8f9a49 stays valid. The only rerun needed is a compile.

Findings

BLOCKER

  • sdk/toyos/src/fd/table.rs:103-104: "calls on two descriptors write no line they share" is false. Two descriptors that dup one object share one slot and write the same count. It should say two objects.
  • sdk/toyos/src/fd/table/model.rs:1-3, 132-134, 168-173: the header says "Each case names the controls that red it", but three cases name none.
    • "the two cases below that free a slot under a call" fits four of the cases below it. build/ci.rs's CONTROLS is the authoritative mapping, so either name the controls at each case or delete these cross-references.
    • Also delete "and every memory order": loom's README says it does not explore load-buffering executions. That gap does not matter for this protocol, whose call-side loads are all Acquire, but the sentence is false as written.
  • PR description (for the orchestrator):
    • (a) Under Consequential decisions, "address space" should say "committed memory": 272 KiB per process against main's 136 KiB.
    • (b) The "No lock across a blocking call" row should name the first-use endowment exception.

SUGGESTION

  • S1: Add a fourth control, e.g. fd-reuse-relaxed, which makes hold's free-slot load (table.rs:257) Relaxed. It should red a_call_on_a_closed_descriptor_never_holds_the_object_that_took_its_slot with a causality violation, showing that the models have teeth on the edge "no use after close" rests on.
  • S2: model.rs:184-188: close other before caller.join(). The case would then also cover a call that lets go of an object its descriptor never named, which this design newly allows.
  • S3: A word that names a free slot now spins forever in acquire (table.rs:179-183). Main panicked there, and the unit test at table.rs:500-514 avoids calling acquire in that state for this reason. On that rare path, the owner could take WRITING once and let entry decide: under the lock, a word naming a free slot always means Broken. That restores the loud failure without touching the fast path.
  • S4: model.rs:15-30: Word belongs to this crate, so implement it on loom's AtomicU32 directly. transport_loom.rs needs its wrapper only because it is a separate crate. Also consider whether fd::table::Word and transport::Word should be one trait.
  • S5: Names. table::Cell reads as core::cell::Cell at fd.rs:400, and type Names (fd.rs:401) for the writer reads like a collection.
  • S6: Record the sighting that served-file calls still meet on one process-wide lock. files::owner's OPEN lock (held(), owner.rs:193-196) is taken inside every hold of a served file (owner.rs:332-338). The closed issue's exit is met (the hold takes no lock), but two threads reading two files still contend there.
  • S7: If per-process commit matters to the owner, compare 32-B slots on the pipe rows. Otherwise state the 128 KiB as the price of the padding.

What metal must show for LAND

  • The T14 testcases shared boot at d8f9a49: toyos-metal exits 0 with every job passing. Any red must be shown independent of this change before landing.
  • The osbench A/B, 4 boots against 4 on both benches. On pipe.*, ipc.* and fs.*.read_4k*/write_4k*, no head row may be slower than base by more than the spread across base's own boots, and no row may exceed twice its machine's recorded value.

SEND BACK. The blockers above are two comment fixes and two description corrections. I found no defect in the concurrency code. Once they are fixed: LAND, pending the metal results above.

Japabu and others added 2 commits November 11, 2026 18:22
…ments that say what is true

`fd-reuse-relaxed` makes the writer's load that finds a slot free `Relaxed`.
Both reuse cases red with loom's causality violation, so the models have
teeth on the edge "no use after close" rests on: the last call's read of an
object before the writer's next write to its slot.

The reuse case closes the second descriptor before the call ends, so it also
covers a call left the object its own descriptor never named. The model
implements `Word` on loom's atomic directly.

`Cell` is `ObjectCell` and the owner's writer type `TableWriter`. The slot
comment says two objects, not two descriptors, since a dup shares a slot.
The model's header no longer claims every memory order (loom does not
explore load buffering) or a map of controls it does not keep: build/ci.rs's
`CONTROLS` is that map.

Calls on two served files still meet on `files::owner`'s `OPEN` lock:
recorded in issues/calls-on-two-served-files-meet-on-one-process-wide-lock.md.

Nothing the descriptor calls execute changes: a rename and a constant
ordering compile to the same code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A call whose descriptor names a free slot reread the word until it changed.
That state is a close in flight or a broken table, and only the writer can
tell the two apart, so a broken table spun the call forever where main
panicked. The table's `acquire` now answers `None` there, and the owner
holds the descriptor under the writer's lock once: a closed descriptor is
refused, and a broken table panics once the lock is let go.

The settling call is cold, out of line and tail-called, so the lock-free
loop keeps its shape: one more callee-saved register saved and a frame of
0x20 bytes instead of 0x18, against d8f9a49, with the instructions
otherwise the same. The A/B on metal measured d8f9a49.

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

Japabu commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Review r2 of #1234 at dafb2e8 (base 34cb4c9)

This is a focused follow-up covering two things: whether r1's required items are resolved, and whether the new code is correct. I read 35d452b and dafb2e8 in full against d8f9a49, along with the current fd.rs owner, fd/table.rs, fd/table/model.rs, build/ci.rs's CONTROLS, build/build.rs's every_model_control_is_run and files::owner's OPEN. I also read #1226's head at 7561f88 (changed, ask). The evidence I read is at 31543e103: r2/ (host logs, the S3 patch and disasm/) and r3/ (the A/B tables and the testcases jobs.txt). I ran nothing.

Architecture

S3 puts the decision with the authority that can make it. The lock-free path cannot tell a finished close from a broken table, and only the writer can. So Table::acquire answers None, and Writer::acquire rereads the word under WRITING. This adds no new state and no new mechanism, and the owner keeps the one lock it already had. Production code grows by +22 lines this round (table.rs 351→361, fd.rs +12), plus +7 lines of control wiring. Tests grow by +21: one unit test, the case widened by S2 and the settling call in the model. The issue record adds +25.

r1's required items: resolved

  • table.rs:103-104 now says "calls on two objects".
  • model.rs:1-4:
    • "every memory order" is gone, and the header states loom's load-buffering gap.
    • The per-case control cross-references are deleted, and the header points to CONTROLS, which is the authoritative map.
  • PR body:
    • The cost is now stated as committed memory (272 KiB against 136 KiB), with the 128 KiB padding price (S7).
    • The blocking-call row names the first-use endow exception.

New code

S1 fd-reuse-relaxed (table.rs:256, Cargo.toml:110-112, ci.rs:856-859): correct.

  • With hold's free-slot load Relaxed, the writer's cell write is no longer ordered after the last call's read, which comes before that call's AcqRel decrement.
  • Both reuse cases fail with "Concurrent read and write accesses to UnsafeCell". This differs from fd-publish-relaxed's read-side message, as expected for the other direction of the race. See r2/host-s3-patch/control-fd-reuse-relaxed.log, exit 101, both named FAILED lines present.
  • The control is registered with both cases. build/clippy.rs's $CONTROLS derives from CONTROLS, so the clippy shape covers it.
  • every_model_control_is_run reads only the manifests, which S3 does not touch.

S3 (fd.rs:517-531, table.rs:172-193, 278-286): correct.

  • No panic on a benign race.
    • Writer::acquire calls entry(fd), which rereads the descriptor's word under the lock. The slot read lock-free is discarded.
    • A close meanwhile gives BadFd. A reopen, including onto the same slot after a free-and-reuse between the read and the lock, holds the new object.
    • Under WRITING, every named slot carries its name's reference. Only writer operations remove a name, and each pairs the word store with its unname. The previous writer's last count write happens-before this Relaxed load through the lock, and every later value in the count's modification order is at least 1 while the name exists. So entry's check of 0 → Broken fires only on a real inconsistency.
    • The model exercises this. World::hold sends Ok(None) to w.acquire(fd) and panics on any fault but BadFd. Loom reaches that path in the close, reuse and reopen cases (call loads the word, close drops the count to 0, call sees 0), and all pass (r2/host-s3-patch/sdk-fd.log, 16 passed).
  • No deadlock.
    • Lock is a non-reentrant futex lock, so a thread already holding WRITING that reached settled would hang. No such path exists.
    • Under WRITING run only one Writer method per change closure, none of which calls out, and the first-use endow. endow runs before ENDOWED is set, so a descriptor call made there would already block in table(), exactly as on main's TABLE lock. S3 adds nothing to that.
    • Nothing is let go under the lock: Writer::acquire takes no let_go.
    • OPEN is never held across a descriptor call. on_fd_held takes it inside fd::with's closure, after settled has returned.
    • The one cross-lock order is A process holds its stdio stream state once, with its descriptor table #1226's ASKING → WRITING, through ask → fd::hold. It stays acyclic because A process holds its stdio stream state once, with its descriptor table #1226 at 7561f88 calls changed(..) after the table operation at all five mutators, and documents the placement at changed ("outside its own lock").
  • A broken table now panics in refusal, after change has dropped the guard. The loop is bounded: Ok(None) leaves it.

S2, S4, S5: taken as described.

  • S2: model.rs:173-192 closes other before the join. all_gone([0, 1]) now catches an object left to the call twice or not at all.
  • S4: Word is implemented on loom's AtomicU32.
  • S5: the renames are complete. No Cell<, Names or Shared remains.

S6: issues/calls-on-two-served-files-meet-on-one-process-wide-lock.md is well formed (finding, open, owner, exit), and its facts match owner.rs:183-196.

Evidence

  • r2/s3-settle-tailcall.patch is identical to git diff 35d452b2a dafb2e860 apart from its index lines, so the host-s3-patch/ logs are dafb2e8's tree.
  • 16 tests pass, all four controls go red, and the three clippy shapes exit 0.
  • Main has changed none of the touched files since the base, and the merge with b053390 is clean.

The A/B at d8f9a49: honest and sufficient.

  • Honest. The PR body, r3/README.md and Unresolved all say plainly that the boots measured d8f9a49.
  • Sufficient. The transfer rests on a sound method. The local build reproduces production's toyos_fd_acquire closure byte for byte at d8f9a49, and in the S3 build that closure is the only extracted function that changes. The gains (10–27% on the pipe rows, from lock sleeps removed) are orders of magnitude above the delta on a path that already executes lock cmpxchg. I checked r3/:
    • ab-*.txt: 25 rows judged per bench, 0 slower beyond base's spread, 0 over twice the record.
    • jobs.txt: 256 ended, 251 exit 0. The five others are as the body states.

The prose summary of the delta is a little too strong. See suggestion 1.

Findings

BLOCKER

None.

SUGGESTION

  1. dafb2e8's message ("with the instructions otherwise the same") and the PR body ("differs from what they measured only by S3's register save"; "the instructions are otherwise the same"). r2/disasm/local-d8f9a490b-vs-s3.diff shows more than that:

    • The success path's recheck gains an unconditional jmp (cb: jmp 0xd0).
    • The loop head loses its alignment nopl (it moves from 0x20 to 0x1d).
    • The result tag is stored as an immediate (movb $0x0,(%rdi)) instead of xorl/movb %al.
    • The lost-race path also saves and restores esi in r15d.

    On the success path, this nets about one more instruction than d8f9a49, so the accepted cost ("about one cycle") stands. Still, the delta is now stated more narrowly than the evidence shows. The body sentence is the orchestrator's to correct. A suggested wording: "pushes and pops r12, frames 0x20 instead of 0x18, and rearranges its blocks (one more jump on the success path); the loads, CAS and recheck are the same." Rewording the commit message is optional.

  2. PR body, invariant row "No lock across a blocking call" ("A call's hold takes no lock") and the opening line of Result. With S3, a call whose descriptor names a free slot takes WRITING once. The Result section says so two paragraphs later, but the row reads as unconditional. It would be clearer as "takes no lock but on first use and on a free slot, and holds none across a blocking call".

  3. fd.rs:416-417 (change's doc). The lock-order rule is now load-bearing on this side as well: a call takes WRITING under its callers' locks on first use and on a free slot. A clause such as "f takes no other lock and no descriptor's hold" would keep it with the code once A process holds its stdio stream state once, with its descriptor table #1226's handoff note is gone. A process holds its stdio stream state once, with its descriptor table #1226's changed doc already carries the other half.

LAND. r1's blockers are resolved, and S1–S6 are correct as implemented. S3 panics only on a real inconsistency and introduces no lock cycle under this PR or #1226's current placement. The metal evidence at d8f9a49 carries to this head. Landing remains the orchestrator's decision.

@Japabu

Japabu commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator Author

Orchestrator: focused re-review r2 LAND at dafb2e8. I corrected the PR body per its suggestions 1 and 2: the exact disassembly delta, and that a call's hold also takes the writer lock on a free slot. The A/B at d8f9a49, with pipe throughput +10–27% and no slower row on either bench, stands with the stated delta. Queueing.

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