Skip to content

ibgda: Trap on error CQEs in release builds - #127

Open
unionxic wants to merge 1 commit into
NVIDIA:develfrom
unionxic:ibgda-error-cqe-release
Open

unionxic wants to merge 1 commit into
NVIDIA:develfrom
unionxic:ibgda-error-cqe-release

Conversation

@unionxic

Copy link
Copy Markdown

Fixes #120.

Summary

In release builds the IBGDA device path ignores error CQEs: nvshmem_quiet() returns
normally after a peer dies, later quiets return as if complete, and fetching operations
return wrong values with nothing reported. This change makes release builds fail the
kernel at the error CQE, as debug builds already do through their assert.

Root cause

ibgda_poll_cq() detects MLX5_CQE_REQ_ERR and returns -1, but ibgda_quiet() and
ibgda_wait_for_slot_availability() check the result only with assert(), which
-DNDEBUG removes. The code comment in ibgda_poll_cq() says CQE errors are fatal.

Fix

Two lines at each of the two call sites, right after the existing assert:
if (unlikely(status != 0)) { __trap(); }. No structure, host-side or environment
variable change; the polling loop is untouched. Diagnostic detail stays where it is
today: NVSHMEM_IBGDA_DEBUG builds print the poll error before the assert.

Limitations

  • After the trap the CUDA context is unusable; the process has to be restarted.
  • Neither NIC handler reads the CQ on the host, so the failure surfaces as the CUDA error
    of the kernel, not as an NVSHMEM message.
  • The GDAKI device path has the same pattern and is not changed here.
  • In NVSHMEM_TIMEOUT_DEVICE_POLLING builds a polling timeout also traps at this point, as
    the debug assert already does.

Testing

Builds (CUDA 12.8, sm_75 and sm_86): release with our options and with LTO-IR, Debug
(sm_75), and the header-contract target; warning lists identical to the unpatched tree.

Cluster: 2 nodes, one PE each (sm_75 and sm_86), ConnectX-6 RoCE v2,
NVSHMEM_IB_ENABLE_IBGDA=1 NVSHMEM_IB_TIMEOUT=14 NVSHMEM_IB_RETRY_CNT=7 NVSHMEM_IBGDA_NUM_RC_PER_PE=1, the reproducer attached to #120 and a fetch-add variant of it (quiet loop, 40 iterations
250 ms apart; fetch-add loop, 160 iterations 15 ms apart), PE 1 SIGKILLed after iteration 3.
5 trials per case on devel (2dca770) before and after.

Case NIC handler Before After
quiet, PE 1 killed gpu all 40 return, exit 0; iteration 5 returns after 3.5–3.6 s (retries), later ones in 1.1 ms; nothing reported iteration 5 fails the kernel (unspecified launch failure), exit code 4, no later iterations
quiet, PE 1 killed cpu_host_memory same as gpu same as gpu
fetch-add, PE 1 killed gpu all 160 return, exit 0; 129–133 of 160 fetches return a wrong value as success the kernel fails at the first fetch after the retries (iteration 29–30); 0 wrong values returned
quiet, no kill gpu, cpu_host_memory 40/40, max 2.0 ms per iteration same
fetch-add, no kill gpu 160/160 exact same

ibgda_poll_cq() detects an error CQE and returns -1, but its two
callers, ibgda_quiet() and ibgda_wait_for_slot_availability(), only
check the result with assert(), which release builds compile out. After
a peer dies, nvshmem_quiet() returns once the IB retries run out, later
quiets return as if complete, fetching operations return wrong values,
and nothing is reported.

Trap right after the existing assert in both callers, so release builds
fail the kernel at the error CQE as debug builds already do. The host
sees the failure as a CUDA error. Debug builds are unchanged.

Limitations: after the trap the CUDA context is unusable; neither NIC
handler reads the CQ on the host; the GDAKI device path is not changed.

Fixes NVIDIA#120

Signed-off-by: unionxic <187358711+unionxic@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Issue]: IBGDA release builds ignore error CQEs: nvshmem_quiet() returns normally, fetches return stale values

1 participant