gpu: fire the sampled probe before the batched add - #68
Merged
Conversation
A sampled launch and its batched gpu_launch_v1 record are twins - same correlation, one carrying the launch, the other only the CPU stack the consumer staples onto it. The consumer joins them in either order, but the orders are not equally safe. Sampled first parks the stack in pendingStacks, where only the twin can claim it and any number of unrelated batches may pass. Batched first holds the launch in deferredLaunches, which the next batch of any other kind releases stackless - deliberately, since the timeline wants launches promptly - leaving the stack to park with nothing to join. Both producers added the launch to its batch and only then fired the sampled probe, so a launch that both filled the batch and was sampled put its own batched record on the wire inside that add(), with the exec batch of the same iteration landing between the twins. On the privileged gate: 58 sampled, 57 attached, 1 parked forever, and the accounting identity held exactly, so nothing was lost silently - only attributed. At the old fixed sampler stride the collision was arithmetically unreachable: batches hold 32 records, the sampler took one in 8, and a multiple of 8 is never 31 mod 32. #50's jittered stride draws gaps from [4,12], so a sampled ordinal eventually lands on a batch boundary. #50 did not cause this; it removed the arithmetic that was hiding it. Fire the probe before the batched add() in both shims. A record cannot be in a batch before add() puts it there, so no flush - on the launching thread, on the drain thread, or on a CUPTI worker - can carry the twin past the sampled probe, and sampled-first holds unconditionally. The records are unchanged and fully built before the probe fires: no ABI change, no sampler change, no consumer change. In the adapter this closes a thread race rather than a same-thread ordering bug: the exec batch is flushed by the CUPTI worker and the drain timer, never by on_launch, so the window between add() and the probe was small but real. The CUDA path measured PendingStacks:0 in every run because it kept missing that window, not because it could not hit it. Rejected: holding the deferred queue across a non-launch batch hands every exec to the sink ahead of its own launch for 32 launches per batch, and needs a number no arrival pattern justifies. Attaching late stacks via LaunchCache means re-emitting a launch the sink already has, breaking "one launch in, one launch out". deferredLaunches stays for foreign producers that may still emit batched-first; what it cannot be is lossless. Tests, all unprivileged: shim/stub/probe_order_test.cc drives the real producer and reads the wire ORDER of its probe fires by patching the sites with int3 and reading the trapped registers - the same bytes bpf_probe_read_user copies. At period 1 the batch-filling launch is sampled by construction, so it fails on the old producer on every machine: "2 of 64 sampled launches had their BATCHED record emitted before their sampled twin". At period 8 over 2048 launches it reported 7 of 253, the same order as the 24 of 250 the demo run recorded. TestBothShimsFireTheSampledProbeBeforeTheBatchedAdd pins the order against both shims' source, which is the only unprivileged guard the CUPTI adapter can have. TestStackSurvivesASplittingBatchWhenTheSampledRecordLeads is the positive counterpart of the parking test, in the consumer's own terms. The gate now asserts PendingStacks == 0 and StacksAttached == wantSampled at rest, and logs the whole identity line. The assertion that fails today is unchanged.
dpsoft
force-pushed
the
fix/sampled-stack-join-race
branch
from
August 25, 2026 00:42
9e3bcfd to
3063ff3
Compare
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.
Closes #67.
main's GPU gate is currently red because of this — it merged with #61 and CI never noticed, because the gate needscap_bpf/cap_perfmonand skips without them.The race
The producer queued the launch batch, then the exec batch, then the sampled probe. When one launch was both the record that filled the launch batch and the one the sampler picked, the exec batch landed between the twins, released the deferred queue, the launch went out stackless, and its stack parked in
pendingStackswith nothing to join.gpuprobe/sampledstacks.go:14-40documented this and quantified it — "24 of 250 captures at 2000 launches; none at 500"."None at 500" was arithmetic, not luck. Batches flush every 32 records; the old sampler took every 8th launch; a multiple of 8 is never ≡ 31 (mod 32). #50's jittered stride made sampled ordinals irregular, so one eventually lands on a batch boundary. The aliasing bug had been masking this race.
Measured on
main, two consecutive privileged runs:The fix
Both producers fire the unbatched
gpu_launch_sampled_v1probe before the batchedadd(). The record is still fully built first — the sampled record readsl.kernel_id/l.correlation— so only the two probe fires swapped. No ABI change, no sampler change, no consumer change.This makes batched-first unreachable rather than rare: a record cannot be in a batch before
add()puts it there, so no flush on any thread can carry the twin past the probe.The adapter case was worse than the issue described
On the stub, the interleave needs one launch to fill the batch. On the CUPTI adapter the exec batch is flushed by the CUPTI worker thread and the drain timer, never by
on_launch— so it was a genuine thread race with a small but real window.PendingStacks:0on the CUDA path across nine measured runs meant we kept missing that window, not that it could not be hit.Testing it without privilege
The bug is only observable under capabilities, which is how it reached
main. So the producer's mechanism is now testable without any:shim/stub/probe_order_test.cclinks the realstub.cc, patches the probe sites withint3(the trickprobe_args_test.ccalready uses) and reads the wire order back from the trapped registers.Pre-fix:
7-of-253 at 2048 launches reproduces the demo run's 24-of-250 rate with no privilege at all. Plus a source-order pin covering both shims — the only unprivileged guard the CUPTI adapter can have — and a positive consumer-side counterpart.
Deliberately not written: a consumer test driving the hostile order. Such a test can only pass if the consumer changes, and the argument here is that it should not — once the launch is at the sink there is nothing left to attach to. Rejected for the same reason: holding the deferred queue (hands every exec to the sink ahead of its own launch, and needs a magic number) and
LaunchCachereattachment (the Consumer talks to agpu.EventSinkand does not own the cache; reaching it breaks "one launch in, one launch out").Gate
assert.Zero(PendingStacks)andassert.Equal(wantSampled, StacksAttached)at rest, plus astack attach:identity line. The previously failing assertion is unchanged and nothing was weakened.Predicted, derived before any run:
sampled=58 resolved=58 attached=58 pending=0, timelinesampledLaunches=58. CUDA path expected unchanged at505 == 505,PendingStacks:0.