Repository navigation
[Bugfix] Join side-stream work before resuming breakable CUDA graph c… - #57785
quanliu1991 wants to merge 1 commit into
Conversation
|
@peakcrosser7 @ZJY0516 @Isotr0py This fixes a capture race we hit while running #54371 in production. Since you reviewed the PLE UVA prefetch path, could you please take a look when you have time? Thanks! |
…apture Fixes vllm-project#57759 An eager break that queues asynchronous work on a side stream (e.g. the pinned-host PLE UVA prefetch launched by start_prefetch) and returns without joining leaves that work in flight when a later capture segment calls capture_end. Depending on timing this either invalidates the capture (cudaErrorNotPermitted at capture_end, load-dependent and thus intermittent) or lets the side-stream kernels leak past the capture and corrupt post-capture state. Join all outstanding device work in the legal window - after the previous segment has ended and before the next capture_begin - by calling torch.cuda.synchronize() at the top of _begin_segment(). cudaDeviceSynchronize is only illegal inside an active capture; between segments it is legal and costs nothing at steady state (capture happens once at startup). Note: the sync must live in _begin_segment, not _end_segment. Placing it in _end_segment runs it while the segment's capture is still active and fails deterministically. A regression test mimics the PLE prefetch pattern (fork a side stream, queue 256+ async kernels, return without joining). On unfixed code it fails deterministically on the first run (side-stream adds leak past the capture and corrupt post-capture zeroing: 518 instead of 256); with the fix it passes and replays correctly. Co-authored-by: Cursor <cursoragent@cursor.com> Signed-off-by: quanliu <18646313696@163.com>
518ff86 to
2f625a4
Compare
|
Thanks for the PR! The change looks reasonable to me, though I have limited experience with CUDA graph capture and am not sure whether unfinished side-stream work alone explains the One concern about the regression test: I think the main stream should wait for the side stream before |
Could you share your config? We didn't see related issue recently And I don't want it to be specialized for prefetch |
|
f1d6448 to
2f625a4
Compare
|
I don't see any issue using 1b9fa3e |
|
I will further confirm whether there is a stable reproduction method for #57759 . |
|
Closing without merge. Re-reading the code and re-running the experiments, this fix does not target the failure it claims to fix (see my closing comment on #57759):
I agree with @ZJY0516 that the breakable capture code should not be specialized for the prefetch pattern. If I get a reliable reproduction I will open a new PR with a fix that actually addresses the failing path. Thanks for the reviews. |
Purpose
Fixes #57759
An eager break that queues asynchronous work on a side CUDA stream and returns without joining (e.g. the pinned-host PLE UVA prefetch launched by
start_prefetchinvllm/models/qwen4_exp/nvidia/ngram_embedding.py) leaves that work in flight when a later capture segment callscapture_end(). Depending on timing, one of two things happens:torch.AcceleratorError: CUDA error: operation not permittedatcapture_end()(thecudaErrorStreamCaptureUnjoined/Invalidatedfamily). This is load-dependent — it succeeds on an idle host and fails under host load, because host load slows the PCIe UVA reads and widens the race window. Observed on 4x L20 TP4 with--engram-config '{"cpu_offload": true}'and--compilation-config '{"cudagraph_mode": "FULL_AND_PIECEWISE"}'.Fix: join all outstanding device work in the legal window — after the previous segment has ended and before the next
capture_begin()— by callingtorch.cuda.synchronize()at the top ofBreakableCUDAGraphCapture._begin_segment().Two properties worth highlighting:
_begin_segment, not_end_segment._end_segmentruns while the segment's capture is still active (the sync would then itself be illegal and fails deterministically — verified on a rig while developing this fix). Only the between-segments window is valid.profile_cudagraph_memory/ capture phase); serving-path replay is untouched.Test Plan
New regression test
test_eager_break_side_stream_work_is_joined_before_next_captureintests/v1/cudagraph/test_breakable_cudagraph.pymimics the PLE prefetch pattern: the eager-break fn forks a side stream, queues 256–2048 async kernels, and returns without joining back.Test Result
Verified on L20 (driver 535.183.01, torch 2.13.0+cu130):
518instead of256, i.e. leaked kernels re-adding after the zeroing), no synthetic host load needed.cpu_offload+FULL_AND_PIECEWISE): patched build survives restarts under synthetic host load (CPU spinners + concurrent direct reads of the checkpoint) that reproduces the original production failure (cudaErrorNotPermittedat FULL capture 1/2 on all 4 workers); unpatched nightly failed under the same conditions.Note on authorship: this fix was developed with AI assistance (GLM5.3, via Cursor — see the
Co-authored-bytrailer). I reviewed every changed line of code and personally ran all the tests described above on our 4x L20 rig, including the production-reproduction restarts under synthetic load.