fix: handle GetMemoryInfo NOT_SUPPORTED for unified memory GPUs (cherry-pick #1637 to release-v2.8) - #1841
Conversation
…ect-HAMi#1637) On NVIDIA GB10 (DGX Spark) and other unified memory architecture GPUs, nvmlDeviceGetMemoryInfo() returns ERROR_NOT_SUPPORTED, causing the device plugin to panic. Changes: - register.go: Handle ERROR_NOT_SUPPORTED by using PreConfiguredDeviceMemory config as fallback. Skip device gracefully (continue) instead of panic when config is not set. - metrics.go: Skip memory metrics collection for unsupported devices. - device.go: Add PreConfiguredDeviceMemory field to NodeDefaultConfig, supporting per-node configuration via nodeconfig. - charts: Plumb preConfiguredDeviceMemory through Helm values and ConfigMap. Fixes: Project-HAMi#1511 Signed-off-by: jsl9208 <shilong@heywhale.com> (cherry picked from commit 13009aa) Signed-off-by: Saiyam Pathak <saiyam911@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: saiyam1814 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Welcome @saiyam1814! It looks like this is your first PR to Project-HAMi/HAMi 🎉 |
There was a problem hiding this comment.
Code Review
This pull request introduces support for GPUs with unified memory architecture, such as NVIDIA GB10 and DGX Spark, which do not support traditional NVML memory queries. It adds a "preConfiguredDeviceMemory" configuration option to the Helm charts and internal configuration structures, allowing users to manually specify GPU memory. The vGPUmonitor and NVIDIA device plugin are updated to handle "nvml.ERROR_NOT_SUPPORTED" errors by either utilizing the pre-configured memory value or skipping the device with informative logging. I have no feedback to provide as there were no review comments to evaluate.
|
@saiyam1814 thx, looks good. and looked at the code before weighing in. but nr 3 s the right fix. the timestamp is already therea nd it just needs to be used. mirror the requesting logic: treat deleted_ older than 60s as recoverable, stamp requesting_, and return (true, true). i think, single-file change in pkg/device/devices.go, no device-plugin changes, covers all vendors (hygon, ascend, vastAI, etc. have no gPU-count fallback so they're stuck unconditionally today). and one note: bare "deleted" without a timestamp appears in some tests, make sure the parse-error path stays at return true, false rather than accidentally recovering. |
Codecov Report❌ Patch coverage is
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
What type of PR is this?
/kind bug
What this PR does / why we need it:
Cherry-picks commit
13009aa(PR #1637) onto therelease-v2.8branch so that an eventualv2.8.3ships with the unified-memory fix for NVIDIA GB10 (DGX Spark) and other architectures whose GPUs returnERROR_NOT_SUPPORTEDfromnvmlDeviceGetMemoryInfo.Without this backport, users on
v2.8.1andv2.8.2continue to hit the device-plugin panic atregister.go:115on GB10 hardware. See #1838 for the full reproduction and #1511 for the original bug report.The cherry-pick applies cleanly to
release-v2.8(no merge conflicts):The diff is byte-identical to the upstream merge in master; commit author is preserved as @jsl9208.
Which issue(s) this PR fixes:
Fixes #1838
(Also indirectly resolves the v2.8.x portion of #1511; master is already covered by #1637.)
Special notes for your reviewer:
GetMemoryInfo not supported for device <UUID>, using configured PreConfiguredDeviceMemory: 131072 MBinstead of panicking.nvidia.com/gpu: 2with the configureddeviceSplitCount.libvgpu) intentionally left untouched.Does this PR introduce a user-facing change?:
Yes — a new chart value,
devicePlugin.preConfiguredDeviceMemory(in MiB), which the device-plugin uses as the device's memory budget when NVML reportsERROR_NOT_SUPPORTED. Default0preserves the previous behaviour for non-unified-memory GPUs.AI assistance disclosure: this PR's body and the cherry-pick mechanics were drafted with Claude Code; the underlying code change is the unmodified content of #1637, authored by @jsl9208. End-to-end behaviour of the cherry-picked code was verified by me on a DGX Spark before submission.