Skip to content

TOPO: resolve NVML fabric symbols lazily#1305

Merged
janjust merged 4 commits into
openucx:masterfrom
janjust:master-fix-cuda12-topo-dependency
Jul 9, 2026
Merged

TOPO: resolve NVML fabric symbols lazily#1305
janjust merged 4 commits into
openucx:masterfrom
janjust:master-fix-cuda12-topo-dependency

Conversation

@janjust

@janjust janjust commented Apr 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

What

This PR avoids a hard runtime dependency on newer NVML fabric-info symbols in the CUDA sysinfo component.

Specifically, nvmlDeviceGetGpuFabricInfoV and the legacy nvmlDeviceGetGpuFabricInfo path are now resolved lazily with dlsym() before use.

Why ?

HPC SDK users can build UCC/HPC-X with newer CUDA/NVML headers but run on systems with older NVIDIA drivers. In that case, libucc_sysinfo_cuda.so could fail to load with:

undefined symbol: nvmlDeviceGetGpuFabricInfoV

That symbol is only available in newer libnvidia-ml.so.1 versions, so directly referencing it raises the effective runtime driver requirement even when fabric metadata is optional.

How ?

The CUDA sysinfo fabric query now uses a small runtime resolver for optional NVML fabric APIs. If the versioned fabric-info symbol is available, UCC uses it and preserves partition ID support. If it is unavailable, UCC falls back to the legacy fabric-info API when present. If neither symbol exists, fabric metadata remains unset and UCC continues without failing library load.

Testing

Tested before/after with CUDA 12.9.1 and NVML 555 at build time, then switched runtime NVML to R525.60.13.

Before this PR:

  • config.h defined HAVE_NVML_GPU_FABRIC_INFO_V.
  • readelf -Ws libucc_sysinfo_cuda.so | grep nvmlDeviceGetGpuFabricInfo showed an undefined relocation for nvmlDeviceGetGpuFabricInfoV.
  • With R525 NVML loaded, ldd -r libucc_sysinfo_cuda.so failed with:
    undefined symbol: nvmlDeviceGetGpuFabricInfoV

After this PR:

  • readelf and nm -D show no dynamic dependency on nvmlDeviceGetGpuFabricInfo*.
  • With R525 NVML loaded, ldd -r libucc_sysinfo_cuda.so reports no undefined fabric-info symbols.

@janjust

janjust commented Apr 30, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

@Sergei-Lebedev @ikryukov codex solution - I check it with the above tests, before/after PR, if you get a chance and have pointers how to guide it for a better fix if needed - let me know

@janjust janjust requested review from Juee14Desai and nsarka April 30, 2026 21:17
@janjust

janjust commented Apr 30, 2026

Copy link
Copy Markdown
Collaborator Author

/build

Juee14Desai
Juee14Desai reviewed Apr 30, 2026
View reviewed changes
Comment thread src/components/topo/cuda/ucc_sysinfo_cuda.c Outdated
@janjust

janjust commented May 6, 2026

Copy link
Copy Markdown
Collaborator Author

/build

@janjust janjust requested a review from Juee14Desai May 6, 2026 17:07
@janjust janjust force-pushed the master-fix-cuda12-topo-dependency branch from 642553f to 2189389 Compare May 6, 2026 17:15
@janjust

janjust commented May 6, 2026

Copy link
Copy Markdown
Collaborator Author

/build

@janjust janjust force-pushed the master-fix-cuda12-topo-dependency branch from 2189389 to 83a5f6a Compare June 3, 2026 17:29
@janjust

janjust commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator Author

/build

ikryukov
ikryukov approved these changes Jun 25, 2026
View reviewed changes
Comment thread src/components/topo/cuda/ucc_sysinfo_cuda.c Outdated
Comment thread src/components/topo/cuda/ucc_sysinfo_cuda.c Outdated
Comment thread src/components/topo/cuda/ucc_sysinfo_cuda.c Outdated
janjust added 2 commits July 7, 2026 13:55
@janjust
TOPO: resolve NVML fabric symbols lazily
0d058cb 
Signed-off-by: Tomislav Janjusic <tomislavj@nvidia.com>
@janjust
TOPO: log NVML handle fallback
8f7629a 
Signed-off-by: Tomislav Janjusic <tomislavj@nvidia.com>
@janjust janjust force-pushed the master-fix-cuda12-topo-dependency branch from 3bff122 to 11a6cc0 Compare July 7, 2026 11:57
@janjust
TOPO: format decls, hoist locals to top
ef1dd2d 
Align the static resolver declarations per .clang-format and move all
locals in ucc_sysinfo_cuda_update_fabric_info to the top of the function
(UCC C89 style). The versioned fabric struct is renamed to fabric_info_v
to avoid a same-name/different-type collision now that both branch locals
share function scope. No functional change.

Signed-off-by: Tomislav Janjusic <tomislavj@nvidia.com>
@janjust janjust force-pushed the master-fix-cuda12-topo-dependency branch from 11a6cc0 to ef1dd2d Compare July 7, 2026 11:59
@janjust

janjust commented Jul 7, 2026

Copy link
Copy Markdown
Collaborator Author

/build

Sergei-Lebedev
Sergei-Lebedev reviewed Jul 8, 2026
View reviewed changes
Comment thread src/components/topo/cuda/ucc_sysinfo_cuda.c Outdated
Comment thread src/components/topo/cuda/ucc_sysinfo_cuda.c Outdated
Comment thread src/components/topo/cuda/ucc_sysinfo_cuda.c Outdated
@janjust
TOPO: simplify NVML symbol resolution
6fcbe12 
Address review from Sergei-Lebedev:
- Resolve fabric symbols via dlsym(RTLD_DEFAULT): libnvidia-ml is already
  mapped (component links it and calls nvmlInit_v2), so the dedicated
  dlopen handle is unnecessary. Drops ucc_sysinfo_cuda_nvml_get_handle.
- Replace the two identical per-symbol loader wrappers with a single
  UCC_NVML_SYMBOL_LOADER macro.
- Restrict the legacy fabric-info API to when the versioned symbol is
  absent: if the versioned API is present it is authoritative, so a
  runtime error from it no longer falls back to the legacy call.

No functional change on drivers that expose the versioned API.

Signed-off-by: Tomislav Janjusic <tomislavj@nvidia.com>
@janjust

janjust commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

/build

@janjust

janjust commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

@Sergei-Lebedev done - and I verified that this fix actually works.

addressed all three in the latest commit:

RTLD_DEFAULT: dropped the dlopen/handle helper; libnvidia-ml is already loaded (we link it + call nvmlInit_v2), so we just dlsym(RTLD_DEFAULT, ...).
dedup: the two identical loaders are now generated by a single UCC_NVML_SYMBOL_LOADER macro.
fallback: if the versioned symbol resolves we treat it as authoritative and no longer fall through to the legacy API on a runtime error — legacy is only used when the versioned symbol is absent.
Also rebased on latest master and folded in the clusterUuid handling from #1308.

How I tested
Built against CUDA 13 (so the versioned symbol is compiled in), then two checks:

No hard dependency — confirms the symbol is resolved by name, not linked:

$ nm -D --undefined-only libucc_sysinfo_cuda.so | grep -i GpuFabricInfo
(nothing) # pre-fix this showed: U nvmlDeviceGetGpuFabricInfoV
Graceful fallback at runtime — ran a small osu_allreduce -d cuda (UCC) with an LD_PRELOAD shim that hides nvmlDeviceGetGpuFabricInfoV from dlsym, simulating an older driver:

NVML symbol nvmlDeviceGetGpuFabricInfoV is unavailable ...
GPU 0: caps=0x1 ... cluster_uuid=0000...
1024 197.23 # collective still completes, no load failure
Interestingly our R580 driver no longer exports the legacy symbol either, so this actually hit the "neither API available" path — loads and runs fine, fabric info just left unset.

@janjust janjust merged commit 4faf5b2 into openucx:master Jul 9, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Sergei-Lebedev Sergei-Lebedev Sergei-Lebedev approved these changes
@nsarka nsarka nsarka approved these changes
@Juee14Desai Juee14Desai Awaiting requested review from Juee14Desai
+1 more reviewer
@ikryukov ikryukov ikryukov approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

Target v1.9.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@janjust @ikryukov @Sergei-Lebedev @nsarka @Juee14Desai @wfaderhold21