Fix the seqlock's writer and reader barriers to match tyndall - #2
Merged
Merged
Conversation
Two of tyndall's four seqlock barriers were missing here. Both are silent when they go wrong: a torn record reaching a consumer, not a crash. The writer stored the odd sequence number with a Release increment. Release orders what comes *before* the increment, which is the previous write, and does nothing about the payload stores that follow it. On a weakly ordered machine those stores may become visible first, and a reader that samples seq either side of them sees the same even number over a half-written entry and accepts it. tyndall issues smp_wmb() there; AcqRel is the equivalent, and the acquire half is what keeps the payload below the increment. The reader loaded the second sequence number with Acquire, which orders what comes *after* the load, not the entry read before it. Without a barrier the entry read may drift past the validation and pick up bytes from the next write. tyndall issues smp_rmb() there; fence(Acquire) is the equivalent. On aarch64 the difference is visible in one instruction each: the writer's increment becomes ldaddal instead of ldaddl, and the reader gains a dmb ishld, which is literally what tyndall's smp_rmb() expands to on that target. This matters now because be-node-drone is about to become the writer on 28 topics at 100 Hz on ARM, feeding C++ readers. Adds a stress test with one writer and three readers over a payload wide enough to tear. Its limits are documented at the site: with the writer's barrier reverted it still passes on an Apple M-series host, so the evidence for the fix is the code generation and the test is there for gross regressions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`cargo clippy --all-targets -- -D warnings` failed on main and would have kept failing after the barrier fix, which makes the lint useless as a gate. All of it is mechanical: * `uninlined_format_args` in the shm-name builder, the open log line, and the two examples: the arguments move into the format string; * `approx_constant` on the example writer's `3.14`, which is now `std::f32::consts::PI` — it was standing in for pi anyway; * the virtual workspace left `resolver` unset while every member is edition 2021, so cargo warned on every invocation. Stating `resolver = "2"` is what the members already imply. No behaviour changes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
sindrehan
approved these changes
Sep 4, 2026
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
2 participants
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.
Follow-up to #1, which was the layout. This is the ordering.
Two of tyndall's four seqlock barriers were missing:
seqincrement wasRelease, which orders what comes before it, not the payload stores after it. tyndall issuessmp_wmb()there. NowAcqRel.seqload, so the read can drift past the validation. tyndall issuessmp_rmb()there. Nowfence(Acquire).Either one lets a reader accept a torn record on ARM, silently.
be-node-droneis about to be the writer on 28 topics at 100 Hz.On aarch64 the fix is one instruction each:
ldaddlldaddaldmb ishlddmb ishldis what tyndall'ssmp_rmb()expands to on that target.The new stress test is not the proof of this; the code generation is. With the writer's barrier reverted the test still passes on an Apple M-series host, and it could never fail on x86. It is there to catch gross regressions, and its limits are written at the test.
Second commit makes
cargo clippy --all-targets -- -D warningspass, which it did not on main. Mechanical: format arguments inlined, the example's3.14becomesPI, and the virtual workspace statesresolver = "2", which its edition-2021 members already imply.Version to 0.3.1.
🤖 Generated with Claude Code