Conversation
On an Apple NHI the interrupt throttle register (0xd004c) is not the one tb_ring_throttling() programs (0x38c00); the kernel's own ring activation writes it from ring->interval_nsec and skips the write when that field is zero. This module never set the field, so on Apple hosts the throttle was silently skipped and the register kept its firmware default -- measurable as a latency floor that does not move with message size (2 B through 4 KB all ~65.3 us on a USB4 link). Set ring->interval_nsec before activation for every ring on an Apple host (including native-backend rings toward a Linux peer: the NHI is Apple's regardless of the peer), log the before/after value, and guard the field access behind a Makefile compile probe so stock kernels still build. Co-Authored-By: Claude Code <noreply@anthropic.com>
…build
Hydra build 12180 failed with
path.c:1328:31: error: 'struct tb_ring' has no member named 'interval_nsec'
on linux 6.18.37. The throttle code is already guarded by
TBV_HAVE_RING_INTERVAL_NSEC, so the guard was not missing -- the probe that
sets it reached the wrong verdict, in the one direction a probe must never
reach it.
Two defects combined:
1. It looked only under $(KDIR)/include. The nixpkgs kernel -- and therefore
every Hydra builder -- is packaged as a SPLIT build/source tree: KDIR is
.../lib/modules/6.18.37/build, and the real headers live in the sibling
.../lib/modules/6.18.37/source/include/linux/. The log shows both paths
in the same compile line. So the header was simply never found.
2. It then failed OPEN. The old form was
ifneq ($(shell grep -c interval_nsec $(TBV_TB_HDR) 2>/dev/null),0)
and grep on a missing file prints nothing at all, so $(shell ...) expanded
to the empty string, "" != "0" was true, and the flag was defined. The
probe asserted the member was present precisely because it had been unable
to look -- a capability probe whose failure mode is to claim the
capability.
Either defect alone is survivable. Together they turn "I cannot find your
kernel headers" into "your kernel definitely has this member", which is how a
correctly guarded file still fails to compile.
The probe now searches $(KDIR)/include, $(KDIR)/source/include and
$(KDIR)/../source/include, and uses grep -l, which prints a filename on a
match and nothing otherwise -- so an absent or unreadable header can only ever
yield "absent". The pattern matches a member declaration rather than the bare
string, so prose in a comment cannot enable the code either.
Encoded as a check rather than a comment, because a comment would not have
caught this. tools/ci/kernel-compat-probe-test.sh drives the probe against
seven synthetic kernel trees through a new kernel/Makefile compat-report
target, and flake.nix runs it as checks.kernel-compat-probe. It is cheap: no
kernel, no compiler, ~1 second.
Test results, against the probe as it stands after this change:
ok split tree, member absent (the Hydra case) -> no
ok split tree, member present -> yes
ok nested source/ packaging, member present -> yes
ok flat tree, member absent (stock kernel) -> no
ok flat tree, member present (Asahi USB4) -> yes
ok header mentions it only in a comment -> no
ok no header at all (must fail CLOSED) -> no
The matrix earned its place immediately: the first version of this fix looked
for $(KDIR)/source/include and passed every case except "split tree, member
present", because build/ and source/ are siblings, not nested. A comment
would have shipped that.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016rZMfxUZ4TdHnxd4ZjS5At
…s/ for the diff and the lane spec for the measured evidence. Co-Authored-By: Claude Code <noreply@anthropic.com> (cherry picked from commit 18b1873)
Applied patch tbv-fedora-ring-diagnostics.patch from the fleet lane; see patches/ for the diff and the lane spec for the measured evidence. Co-Authored-By: Claude Code <noreply@anthropic.com> (cherry picked from commit a76bb0c)
Applied patch tbv-uc-local-completion.patch from the fleet lane; see patches/ for the diff and the lane spec for the measured evidence. Co-Authored-By: Claude Code <noreply@anthropic.com> (cherry picked from commit efad96c)
Applied patch tbv-rx-supp-poll.patch from the fleet lane; see patches/ for the diff and the lane spec for the measured evidence. Co-Authored-By: Claude Code <noreply@anthropic.com> (cherry picked from commit 1b2a165)
The fedora port carried two hand-edits beyond the lane patches: an extra TBV_TB_HDR interval_nsec compile probe in the Makefile, and the linux/io.h + version.h + ring_diag.h includes with the Linux 6.18 PCI NHI diagnostic block in debugfs.c. Captured verbatim so this repo is byte-identical to what actually builds and runs on both nodes. Co-Authored-By: Claude Code <noreply@anthropic.com> (cherry picked from commit 5d72ecc)
tbv-rc-reorder.patch: the receiver's reorder timeout equalled the sender's
retry budget exactly (zero margin, both ends quantizing on their own reap
tick), so the receiver's cleanup raced the sender's retransmit, dropped the
message, and killed both QPs with an ACK_ERROR. Head-only silent expiry +
two extra budget units of margin + a poison-guard history entry.
256 KiB: 107.7 -> 371.1 MiB/s (inflight=8), all drop counters zero.
tbv-hot-poll.patch: rx_supp_poll/tx_progress_poll mode 2, delay-0 re-arm on
the module's WQ_HIGHPRI workqueue, armed from TX and RX, bypassing the
kernel's normal-priority kworker delivery hop. 2 B latency typical
21-22 -> 13.7-14.3 us, min 7.5, max 23-25 (tail gone), stdev ~2.
1M UC pingpong 96.19 -> 32.46 us/iter.
kernel/{ibdev,path}.c synced to the exact deployed build on both ends.
Co-Authored-By: Claude Code <noreply@anthropic.com>
(cherry picked from commit 161c5f8)
Hydra: passedHead All 22 builds passed. |
The native-credit patch changed TBV_NATIVE_DATA_CREDIT_BATCH 32->256 in proto/native_data.h, but that header is wire contract: upstream's checks.proto-smoke pins the start-credit invariant against it, and a stock peer computing 32-frame batches against our 256 could starve start credits on large messages. The batch is policy, not frame layout, so it now lives in a module param (tbv_native_credit_batch, default 32 = stock-compatible); path.c computes the return threshold and start-credit requirement from the param with the same formulas, and the header stays upstream-identical. Fleet keeps the measured 256 (of +28-51% at 64-256 KiB) via modprobe.d. Smokes: proto/reliability/identity rc=0; .ko builds on Fedora 6.18 and Asahi 7.1.13-usb4gpu. Co-Authored-By: Claude Code <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.
UC reliability over TB4/USB4: hybrid completion, retransmit budget, and the measured loss classes
Stacked on #75 — commits 36a47f1/2b65af3 (the NHI throttle + its compat probe) are #75's; review those there. This PR's own content is the 6 commits after them. Builds clean on both ends of our leg: Fedora 6.18 (stock NHI tree) and Asahi 7.1.13-usb4gpu (Apple NHI tree).
The problem
Two hosts — Apple M2 Ultra, Asahi 7.1.13-usb4gpu (Apple NHI) and AMD Strix Halo, Fedora 6.18-rc7 (stock PCI NHI) — joined by one 20 Gb/s TB4 cable. After the throttle fix, every sustained UC run froze permanently: the sender's last send completed, the receiver had posted exactly one fewer reply, no RNR / duplicate / error anywhere, empty dmesg on both ends.
What the counters said (module debugfs, per-QP)
The design
tbv-uc-local-completion): the WC fires at local TX drain (UC semantics) but the send context stays pending until the peer ACK; a reap retransmits drained-but-unacked sends. UC sends are solicited unconditionally and the receiver ACKs at message completion, so an ACK is a real delivery receipt.tbv-rx-supp-poll: the driver's own supplemental RX poll (1 ms delay / 16 ms window) reaps frames whose completion interrupt was lost. It existed but was hard-disabled out of an Apple-backend reorder concern; now enabled native-only behind a module param (rx_supp_poll=-1). One 1M-iteration run logged 18,334 rescues — the stranded-completion class is two orders of magnitude more common than previously estimated.ibv_uc_pingpong's normal shape) dies atmax_send_wrwith zero counter evidence.rnr_waitingforever; they now retry underTBV_UC_RNR_MAX_RETRIES=7(module policy — UCretry_cnt=0must not mean "zero wire retries"). This intersects Default data QPs to infinite RNR retries #71 (infinite RNR retries as default): we'd rather keep the budget explicit and bounded; happy to converge either way.rc-reorder + hot-poll: per-frame completion was the bandwidth ceiling; reordered completion polling gives 3.4× at 256 KiB and takes the 2 B floor from 22 to 14 µs.Everything new sits behind module params and is gated to the native backend; the Apple compatibility backend is untouched.
Measured acceptance (both ends checksummed)
Limitations, stated
Bandwidth at large messages is per-frame-completion-CPU-bound (bigger native frames / striping next). A true wire loss still costs the full retransmit stall — a selective fragment NAK would shrink 5.09 s to ~200 µs. Wire budgets are module policy, not negotiated. Validated on exactly one leg topology (Apple NHI ↔ stock NHI, Asahi 7.1.13 ↔ Fedora 6.18).