Follow up fixes for pinning hardware device by melody-ren · Pull Request #678 · NVIDIA/cudaqx

melody-ren · 2026-07-11T05:05:58Z

Summary:

Worker threads in the decoding server now pin themselves to the decoder's cuda_device_id before serving. Construction only pinned the registry thread, so every worker was decoding on the default device regardless of the pin prior to this patch.
The direct call path (no realtime session) had the same problem: configure_decoders leaves the thread on the last decoder's device, so decoder 0 would decode on decoder N's GPU. Each decode now selects its own decoder's device first.
If a pin can't be honored we fail instead of decoding on the wrong device: a worker that can't pin aborts server startup, and a cudaSetDevice failure during dispatch returns an error response instead of logging a warning and carrying on.
Hololink: HOLOLINK_GPU_ID and cuda_device_id both name the GPU the FPGA is attached to, so they must agree. Either one alone selects the device; setting both to different values throws at transport creation. Single decoder only, same as the rest of the gpu_roce path.
Nothing changes if cuda_device_id is not set.
Decoder construction is transactional: if a plugin constructor throws after its device was selected, the calling thread's CUDA device is restored to its previous value instead of being left on the failed decoder's device. Originally first introduced in Enforce decoder GPU ownership in the realtime server #687. Adopting the change here.

Outdated: This is a follow-up PR to #664, which should be merged before this PR

This PR depends on #690

Decoders can now be pinned to a specific CUDA device at construction via a cuda_device_id parameter, settable through C++/Python kwargs and the YAML realtime config (top-level decoder field, next to type/ transport). Model: one thread owns one decoder. decoder::get() validates the id (negative or >= device count throws), persistently pins the constructing thread with cudaSetDevice (no restore), strips the key before the plugin constructor, and stores it (get_cuda_device_id()). Plugin constructors therefore allocate on the right device with zero plugin changes -- this covers trt_decoder and the closed-source nv-qldpc-decoder transparently. decode_async() is the one exception: its fresh std::async worker pins itself for the call's duration via a lib-private RAII CudaDeviceGuard (libs/qec/lib/hardware_guards.h). The realtime host dispatcher (one thread serving all decoders) applies each decoder's device with set-if-different before enqueue and before DEVICE-mode graph capture; no-op for unpinned decoders. Tests: 7 C++ unit tests incl. 2-GPU placement and async-worker pinning, YAML round-trip + prepare_decoder_params coverage, Python kwargs tests. NUMA/mempolicy/cpu_affinity and thread-binding APIs are deferred to a follow-up PR per review feedback on NVIDIA#634. Signed-off-by: kvmto <kmato@nvidia.com>

cuda_device_id pinned only the constructing thread, so two dispatch paths decoded on whatever device was current: - decoding-server DecodingSession workers start unpinned; every session decoded on the default device regardless of its pin. The worker now pins itself once at worker_loop entry. - the direct (no realtime session) path decodes on the caller thread, which configure_decoders leaves on the LAST decoder's device. It now selects the decoder's device before each decode. Both paths share a new fail-fast helper (hardware_guards.h, set-if-different, throws). apply_decoder_cuda_device also uses it now: a cudaSetDevice failure previously warned and continued on the wrong device; it now surfaces as a dispatch error response, and graph capture aborts during initialization. Signed-off-by: Melody Ren <melodyr@nvidia.com>

Two paths could previously run with a decoder's cuda_device_id silently violated -- the worst failure mode, because decoding can still succeed on the wrong GPU with nothing but an unread log line as evidence: - DecodingSession workers logged a cudaSetDevice failure and kept serving. The pin now runs on the worker thread behind a promise/future handshake in start_worker(): the worker either starts pinned or the exception is rethrown on the registry thread and server startup aborts, the same channel as a decoder that fails to construct. - gpu_roce chose its GPU from HOLOLINK_GPU_ID (default 0) with no knowledge of the decoder's pin, splitting graph capture (pinned device) from ring buffers and device-side graph launch (env device). Both knobs name the same topology fact -- the FPGA-affine GPU -- so they are now reconciled at transport creation: agreement or a single set knob resolves the device, a conflict throws, and an unset environment defers to the decoder's pin. The reconciliation is a plain function compiled in every configuration and unit-tested; the gpu_roce call site remains behind CUDAQ_GPU_ROCE_AVAILABLE and needs hardware validation. Signed-off-by: Melody Ren <melodyr@nvidia.com>

- set_cuda_device_for_decode: -1 is a no-op and an impossible device id throws -- both runnable on GPU-less CI (cudaSetDevice past the device count fails there too), covering the failure transport the worker handshake rides on, which cannot be induced end-to-end after construction-time validation. - DecodingSession handshake smoke: a decoder pinned to device 0 starts its worker through the promise/future handshake and serves a queued item (skips below 1 GPU). Signed-off-by: Melody Ren <melodyr@nvidia.com>

- Add the handshake failure-injection test: cuda_device_id_ is protected, so a test decoder can carry an impossible id past the construction-time range check, and start_worker() must throw and join the worker. This is the test that fails if the handshake ever reverts to log-and-continue. - Select the decoder's device on the get_corrections and reset paths (host dispatcher and direct call), not just enqueue: a plugin's clear_corrections or reset_decoder may touch device memory, and interleaved pinned decoders left the wrong device current. - On the direct path, pin before the syndrome-capture callback so a pin failure cannot record a round in a --save_syndrome trace that was never decoded. Session-forwarded requests still capture before forwarding and still skip the pin. - Reject a negative HOLOLINK_GPU_ID in the gpu_roce reconciler instead of resolving to a garbage device. - Join already-started workers before the transports are destroyed when a constructor that installs transports first fails mid load_from_config; drop a dead include. Signed-off-by: Melody Ren <melodyr@nvidia.com>

…fixes

copy-pr-bot · 2026-07-13T20:15:21Z

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Merge Melody Ren's decoder pinning follow-up so its production-tested fixes remain in history with their original authorship and this work becomes a true descendant of PR NVIDIA#678. Adopt the shared fail-fast CUDA device-selection helper, the decoding-session worker startup handshake, explicit HOLOLINK_GPU_ID tracking, and the tested GPU RoCE device-reconciliation contract. Retain the additional decoder-server ownership guarantees from this branch: select the owning device across enqueue, correction, and reset operations; restore configuration threads after decoder construction; and perform CUDA graph capture, scheduler initialization, graph release, decoder destruction, and transport teardown on the owning device. Resolve GPU RoCE placement before decoder construction so graph capture and transport allocation cannot land on different devices. Reject conflicting environment and decoder placement, explicitly negative device IDs, and ambiguous graph-dispatch configurations. Combine Melody's CI-runnable reconciliation and worker-handshake coverage with the stronger multi-GPU execution, realtime dispatch, and device-correct teardown tests from this branch. Signed-off-by: kvmto <kmato@nvidia.com>

melody-ren · 2026-07-13T20:48:43Z

/ok to test 3b156de

@bmhowe23

Carrying the content of #664 , and its commit 7fe9529 in which the `cuda_device_id` knob was introduced. This PR is meant to revert 664's scope change introduced by 19b95bf. This PR should be followed by #678 to enable support for the decoding server. PR description copied verbatim from #664 : ## Summary First PR of the #634 split (per @bmhowe23's review): `cuda_device_id` only — GPU decoder pinning, settable at construction via kwargs and the YAML realtime config. NUMA/mempolicy/cpu_affinity and thread-binding APIs are deferred to a follow-up draft PR (next release), Python interfaces for those to a third. ## What it does - **`decoder::get()`** reads and validates `cuda_device_id` (negative or >= device count → `std::runtime_error`), **persistently pins the constructing thread** (`cudaSetDevice`, no restore), strips the key before the plugin constructor, and stores it (`get_cuda_device_id()`, -1 = unpinned). - **Model: one thread owns one decoder.** The thread that creates a decoder drives its decode calls; construction-time and lazy decode-time allocations all land on the pinned device with **zero plugin changes** — covers `trt_decoder` and the closed-source `nv-qldpc-decoder` transparently. This addresses the lazily-allocating-decoder concern from the #634 review without per-call guards or new decode entry points. - **`decode_async()`** is the sole exception: its fresh `std::async` worker pins itself for the call's duration via a lib-private RAII `CudaDeviceGuard` (`libs/qec/lib/hardware_guards.h`, header-only; PR2 extends it). - **YAML realtime config:** `cuda_device_id` is a top-level `decoder_config` field (next to `type`/`transport` — placement knob common to any GPU decoder, not a per-decoder algorithm arg). Surfaced by `prepare_decoder_params()` for every decoder type; also exposed as `decoder_config.cuda_device_id` in Python. - **Realtime session:** the host dispatcher (one thread, all decoders) applies each decoder's device set-if-different (no restore) before enqueue and before DEVICE-mode graph capture; no-op for unpinned decoders. - **Python kwargs** work with no binding changes: `qec.get_decoder("trt_decoder", H, cuda_device_id=1)`. To place N decoders on N GPUs, create each decoder on its own thread (`std::async` / `threading.Thread`) — documented in `realtime_decoding.rst`. ## Testing - 7 new C++ unit tests (`DecoderCudaDeviceId.*`): kwarg contract (absent/negative/out-of-range), key stripped before strict-validating plugin ctors, persistent pin observable after construction, **two decoders on two GPUs from two threads**, async worker pinning itself (verified RED→GREEN on a 2-GPU machine). GPU tests skip below the needed device count. - YAML: round-trip with the new field; `prepare_decoder_params` surfaces the knob for trt and non-trt types (ordering vs the trt-only early return is locked by test). - Python: kwargs error path (`cuda_device_id` named in the error) and valid-id construct+decode. Full `test_decoder.py` 61/61. - Full suites: `test_decoders` 52/52, `test_decoders_yaml` 20 pass / 2 skips (nv-qldpc-gated). ## Notes for reviewers - Known pre-existing quirk (not introduced here): negative int kwargs from Python surface as an opaque `bad_cast` because `hetMapFromKwargs` stores Python ints as `size_t` (`libs/core` `kwargs_utils.h`) — the friendly negative-id message is only reachable from C++/YAML. Follow-up candidate in core. - `nv-qldpc-decoder` internal changes (mirroring the trt pattern) are a separate follow-up. Supersedes #663 (same content, reopened from the fork). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Signed-off-by: kvmto <kmato@nvidia.com> Signed-off-by: Melody Ren <melodyr@nvidia.com> Co-authored-by: kvmto <kmato@nvidia.com>

…fixes Signed-off-by: Melody Ren <melodyr@nvidia.com>

Signed-off-by: Melody Ren <melodyr@nvidia.com>

melody-ren · 2026-07-13T22:24:44Z

/ok to test 8c266d3

bmhowe23

Approved given that #692 will follow soon thereafter

Brings in upstream NVIDIA#678 ("Follow up fixes for pinning hardware device"), which adds gpu_roce GPU-device reconciliation: the transport must run on the same GPU the FPGA/NIC is affine to (HOLOLINK_GPU_ID) as the one the decoder is pinned to (cuda_device_id), since a captured CUDA graph can't be launched from another device. Conflict resolution (libs/qec/lib/realtime/decoding-server-cqr/DecodingServer.cpp): upstream added that reconciliation inline in make_transport, directly constructing GpuRoceTransceiver behind #ifdef CUDAQ_GPU_ROCE_AVAILABLE -- the old in-core layout. This branch had already moved gpu_roce out of the core into the cudaq-qec-decoding-server-gpuroce component, reached through a weak factory, so the core carries no DOCA/hololink/CUDA-driver deps (driverless CI tests + the CQR plugin must stay loadable). Reconciled to keep BOTH: - make_transport keeps the weak-factory dispatch but now threads the decoder's pinned device: cudaqx_qec_make_gpu_roce_transceiver(pinned_cuda_device). - The env read + reconcile + GpuRoceTransceiver construction move into GpuRoceFactory.cpp (the component), where GpuRoceConfig is visible; the weak factory gains an `int pinned_cuda_device` parameter. - reconcile_gpu_roce_device stays in the core (pure int logic, unit-tested by test_decoding_server_core, must not pull DOCA); the factory calls it via the component's PUBLIC link on the core (added #include "DecodingServer.h"). - The config ctor keeps the branch's transport_type local (used by the scheduler-wiring block) AND takes upstream's pinned-device computation. ABI note: upstream's decoder.h change makes five realtime-API methods virtual, adding a vtable pointer to the decoder base class -- an ABI break. Consumers built against the old header must be rebuilt; in particular the external nv-qldpc plugin + proprietary cudevice archive (private tree) were rebuilt against the synced decoder.h and the daemon relinked. nvq++-device-compiled targets (surface_code-4-yaml and the -cqr producers) don't track header deps, so they need a forced/clean rebuild after this merge -- CI builds fresh, so it is not masked there. Signed-off-by: Chuck Ketcham <cketcham@nvidia.com>

Bring in merged main, including the cuda_device_id GPU-pinning knob (NVIDIA#690) and its follow-up fixes (NVIDIA#678). NVIDIA#690's cuda_device_id additions (decoder_config field, YAML mapOptional, Python def_rw, the two YAML tests, and the realtime_decoding.rst example/prose) merge cleanly into the schema-registry config on this branch and are absorbed here. The docs conflict (this branch's decoder_custom_args schema section vs main's cuda_device_id section) is resolved by keeping both. Two branch-side adaptations NVIDIA#690 could not anticipate are layered on in the following commit: exporting cuda_device_id in this branch's decoder_config_json_schema(), and adapting NVIDIA#690's trt YAML test to the removed trt_decoder_config struct. Signed-off-by: Ben Howe <bhowe@nvidia.com>

Remove `hololink_gpu_id` env var Follow up to #678 --------- Signed-off-by: kvmto <kmato@nvidia.com> Signed-off-by: Melody Ren <melodyr@nvidia.com> Co-authored-by: kvmto <kmato@nvidia.com>

…ridge branch Main landed the squashed PR 670 plus NVIDIA#656/NVIDIA#673/NVIDIA#674/NVIDIA#678/NVIDIA#680/NVIDIA#683/NVIDIA#688/ NVIDIA#690/NVIDIA#691/NVIDIA#692. Resolution keeps this branch's design and ports main's content into it: - Naming: main's GpuRoce{Transceiver,Factory,LinkCheck} map 1:1 onto this branch's DeviceGraph* files (the de-transport-ing follow-up requested in the PR 670 review). Main's post-review fixes are ported into the renamed files: ring-size overflow + host-page-size alignment validation, gpu_id resolved from the decoder's cuda_device_id instead of an env var, the factory taking pinned_cuda_device, and the linkcheck exercising the new factory signature. - Schema: per-decoder 'dispatch: host|device_graph' plus the top-level transport section stay; main's per-decoder 'transport:' key does not return. cuda_device_id is adopted as a per-decoder placement knob (config struct + YAML trait), and the hsb script now injects 'dispatch: device_graph' + cuda_device_id and drives the server with QEC_DEVICE_GRAPH_* env and the device_graph READY sentinel. - Features adopted from main: decoder CUDA-device pinning (worker-thread pin via promise, graph-capture pin, hardware_guards.h), per-session decode counters + print_session_stats + QEC_DECODING_SERVER_STATS server-side stats printing, DecodingServer ctor exception safety, virtualized realtime decoder API (NVIDIA#674), CMP0126 fix, CUDA::cudart on the core lib, and the cudevice-archive dedup in the server link. - DecodingSession combines main's set_graph_capture_device with this branch's reserved-SMs decode-graph capture (QEC_DEVICE_GRAPH_RESERVED_SMS). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Ben Howe <bhowe@nvidia.com>

This reverts commit 9c5c230. Signed-off-by: Angela Burton <angelab@nvidia.com>

…ntegration Signed-off-by: Angela Burton <angelab@nvidia.com>

…VIDIA#609)". Resolve NVIDIA#609 revert conflicts by restoring the legacy host-call and direct-decoding paths while retaining the GPU device-placement fixes from NVIDIA#690 and NVIDIA#678. This reverts commit 927ec7f. Signed-off-by: Angela Burton <angelab@nvidia.com>

…698) ## Background - #690 introduced the cuda_device_id placement knob for GPU decoders. - #678 made worker threads and the inproc graph capture honor the pinned device, and pinned decoder construction. - #683 closed the remaining gap: the gpu_roce decoding-server capture path now pins too. Each fix added a device pin to one more path via its own ad-hoc helper. The "which device" decision ended up duplicated across dispatch, capture, the worker, construction, and the gpu_roce transport. And that is the motivation for this PR. ## What this PR does Basically, every path should be following a single source of truth. Consolidate all of it to: the decoder's `cuda_device_id`, resolved one way (`decode_device_for`) and applied through two sanctioned wrappers — `pin_decode_device` (dispatch/decode) and `capture_graph_pinned` (graph capture) — in `hardware_guards.h.` Every path now derives its device from that one field; none selects a device by any other means. Unpinned (< 0) keeps existing semantics: dispatch no-ops, capture defaults to device 0. Signed-off-by: Melody Ren <melodyr@nvidia.com>

kvmto and others added 5 commits July 9, 2026 17:31

Merge branch 'main' into decoder-gpu-pinning

710cd0b

melody-ren force-pushed the melodyr/pin-device-fixes branch from e0fda65 to 310fdff Compare July 11, 2026 05:13

melody-ren mentioned this pull request Jul 13, 2026

feat(qec): add cuda_device_id placement knob for GPU decoders #690

Merged

Merge remote-tracking branch 'upstream/main' into melodyr/pin-device-…

3b156de

…fixes

melody-ren added 3 commits July 13, 2026 14:22

Merge remote-tracking branch 'upstream/main' into melodyr/pin-device-…

9f2b130

…fixes Signed-off-by: Melody Ren <melodyr@nvidia.com>

Fix clang-format: remove extra blank line in hardware_guards.h

b9c3ded

Signed-off-by: Melody Ren <melodyr@nvidia.com>

Restore the calling thread's CUDA device when decoder construction fails

8c266d3

Signed-off-by: Melody Ren <melodyr@nvidia.com>

melody-ren marked this pull request as ready for review July 13, 2026 23:11

melody-ren mentioned this pull request Jul 13, 2026

Remove hololink_gpu_id env var #692

Merged

bmhowe23 approved these changes Jul 13, 2026

View reviewed changes

melody-ren merged commit 9c5c230 into NVIDIA:main Jul 13, 2026
24 checks passed

melody-ren deleted the melodyr/pin-device-fixes branch July 13, 2026 23:34

This was referenced Jul 15, 2026

DRAFT: QEC decoding server: transport-agnostic bridge providers, one ring per decoder, mixed host + device_graph dispatch #682

Draft

Enforce decoder GPU ownership in the realtime server #687

Closed

melody-ren mentioned this pull request Jul 15, 2026

Route every decoder device pin through one resolver and two wrappers #698

Merged

anjbur added a commit to anjbur/cudaqx that referenced this pull request Jul 16, 2026

Revert "Follow up fixes for pinning hardware device (NVIDIA#678)"

80c1b34

This reverts commit 9c5c230. Signed-off-by: Angela Burton <angelab@nvidia.com>

anjbur added a commit to anjbur/cudaqx that referenced this pull request Jul 16, 2026

Cherry pick PR NVIDIA#678 (9c5c230) back, excluding decoding-server i…

736ec49

…ntegration Signed-off-by: Angela Burton <angelab@nvidia.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Follow up fixes for pinning hardware device#678

Follow up fixes for pinning hardware device#678
melody-ren merged 10 commits into
NVIDIA:mainfrom
melody-ren:melodyr/pin-device-fixes

melody-ren commented Jul 11, 2026 •

edited

Loading

Uh oh!

copy-pr-bot Bot commented Jul 13, 2026

Uh oh!

melody-ren commented Jul 13, 2026

Uh oh!

melody-ren commented Jul 13, 2026

Uh oh!

bmhowe23 left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

melody-ren commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

copy-pr-bot Bot commented Jul 13, 2026

Uh oh!

melody-ren commented Jul 13, 2026

Uh oh!

melody-ren commented Jul 13, 2026

Uh oh!

bmhowe23 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

melody-ren commented Jul 11, 2026 •

edited

Loading