Repository navigation
shim: take the cubin CRC off the application's thread, which deadlocked it (#211) - #212
Merged
Merged
Conversation
…ed it (#211) The MODULE_LOADED callback computed the cubin CRC with cuptiGetCubinCrc, which takes the adapter's one CUPTI guard. Under lazy module loading the callback fires from inside cuLaunchKernel, on an application thread that holds a driver lock. The timer thread held the guard across cuptiActivityFlushAll, waiting for a CUPTI worker that was waiting for that driver lock. Three threads, a permanent deadlock of the profiled process: 1 run in 8 on a PyTorch workload with only the shim injected, and 4 captures in 7 empty with the agent attached. CubinQueue now has three steps instead of two. capture() copies and parks the copy as pending. It takes no CRC function, so an application thread inside a vendor callback waits on nothing but the queue's leaf mutex. settle() runs on the timer thread every tick, attached or not: it computes the CRC over the copy, de-duplicates, fires gpu_module_load_v1, and hands the entry to drain(). It holds the mutex across neither the CRC nor the record. The record carries the callback's module_id and load_ns, so its timestamp is still the load time, not the time of the tick. The pending list is bounded separately from the offer queue, so a full, retained queue still lets a new module reach settle() and its record. Past the pending bounds a module is lost whole and counted as cubin_pending_full, which the report line now prints alongside cubin_pending_depth. check-cubin-defer gains a mode 6: passing a CRC function to capture() must not compile. That makes the property structural, the way the copy deadline already is. Verified on an RTX 3090: 30 of 30 shim-only runs complete (the old shim hung 1 in 8), and 10 of 10 captures with the agent attached are full, about 79k records each. Module counters are identical to the old shim's on the same workload. Holding the mutex across the CRC in settle() fails the new queue test.
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.
Fixes #211.
The bug
The NVIDIA adapter could permanently deadlock the profiled process. The MODULE_LOADED callback computed the cubin CRC with
cuptiGetCubinCrc, which takes the adapter's one CUPTI guard. Under lazy module loading the callback fires from insidecuLaunchKernel, on an application thread that holds a driver lock. Meanwhile the timer thread held the guard acrosscuptiActivityFlushAll, waiting for a CUPTI worker that was itself waiting for that driver lock.With only the shim injected, a PyTorch workload hung in 1 of 8 runs. With the agent attached, 4 of 7 captures came back empty (
batches=0), because the workload froze before its first kernel.The change
CubinQueuenow works in three steps instead of two:capture(), on the application thread inside the callback, only copies the bytes and parks the copy as pending. It takes no CRC function, so an application thread inside a vendor callback waits on nothing but the queue's leaf mutex.settle(), on the timer thread every tick whether or not a consumer is attached, computes the CRC over the copy, de-duplicates, firesgpu_module_load_v1, and hands the entry todrain(). It holds the mutex across neither the CRC nor the record. The record carries the callback'smodule_idandload_ns, so its timestamp is still the load time, not the time of the tick.drain()is unchanged.The pending list is bounded separately from the offer queue, so a full, retained offer queue still lets a new module reach
settle()and its record. Past the pending bounds a module is lost whole (no record, because its CRC is never computed). That has its own counter,cubin_pending_full, which the report line prints next tocubin_pending_depth.check-cubin-defergains a sixth mode: passing a CRC function tocapture()must fail to compile. That makes the property structural, the same way the copy deadline already is.The stub is wired the same way: copy on the caller's thread, then settle and offer on its drain tick.
Verification
make -C shim testpasses, including 7 new queue tests: no CRC and no record incapture(); the record keeps the callback's ids; a capture completes whilesettle()is inside a blocked CRC; two pending loads of one CRC settle as a single module; and the separate pending bound.Mutation check: holding the queue mutex across the CRC in
settle()failstest_a_capture_completes_while_settle_is_inside_the_crc.gpuprobewith capabilities: all tests pass, includingTestStubDrivesThePipelineToPprofWithoutAGPUand the stub cubin-delivery tests. The only skip is the unprivileged-only test.RTX 3090, driver 610.57.04, CUDA 13.3 CUPTI:
modules_captured=12,cubin_too_large=1), and the new pending counters read zero.ThreadSanitizer:
cubinqueue_testruns clean under clang's-fsanitize=thread, with no warnings, including the new concurrent capture-during-settle test. No CI job runsmake -C shim test-tsan, so this local run is the only TSan coverage the change has.CI: all jobs pass, including the portable ELF gate on amd64 and arm64.
Not in this PR
According to the comment above
g_pc_ctxsinnvidia/cupti_adapter.cc, the PC-sampling module-unload path blocks on the guard from the application's thread, which is the same pattern. It only applies with PC sampling on (off by default), and it has not been reproduced. It is noted on #211 rather than changed here.