CPU Spin Lock and Log Flooding on NVML Event Error

[aniket866]

Closes #2321

Location:

• File: health.go in pkg/device-plugin/nvidiadevice/nvinternal/rm/
• Lines: 146-152 (checkHealth)

Code:

e, ret := eventSet.Wait(5000)
...
if ret != nvml.SUCCESS {
    klog.Infof("Error waiting for event: %v; Marking all devices as unhealthy", ret)
    for _, d := range devices {
        unhealthy <- d
    }
    continue
}

Explanation:

checkHealth uses NVML events to watch for hardware errors (XIDs). It listens for event notifications by calling eventSet.Wait(5000) inside a loop.

Reason for Crash:

If eventSet.Wait() fails with a persistent error (for instance, if the NVML context becomes corrupted or the GPU driver encounters a critical error), it returns a non-SUCCESS code immediately without blocking. The loop captures this, writes a log, sends all devices to the unhealthy channel, and executes a continue. Because the error is persistent, the loop immediately checks again, creating a tight CPU spin lock consuming 100% of a CPU core and flooding logs/channels, locking up or crashing the device plugin.

Spin Lock Loop:

graph TD
    A[eventSet.Wait fails immediately] --> B[Log error]
    B --> C[Send unhealthy notification]
    C --> D[continue loop]
    D --> A
    Note over A,D: Loops with zero sleep delay:<br/>100% CPU usage & log spam!

Loading

Summary by CodeRabbit

• Bug Fixes
  • NVML event-wait failures are now reported immediately when they are not timeout-related.
  • Devices are still marked unhealthy, while the underlying error is preserved for clearer diagnostics.

[hami-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aniket866
Once this PR has been reviewed and has the lgtm label, please assign archlitchi for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c4d5f92a-7b6f-47e6-b35b-f2c762ed4de4

📥 Commits

Reviewing files that changed from the base of the PR and between 0345bd8 and 95f5e50.

📒 Files selected for processing (1)

• pkg/device-plugin/nvidiadevice/nvinternal/rm/health.go

---

📝 Walkthrough

Walkthrough

The NVML health check now returns non-timeout event-wait errors after marking all devices unhealthy. It no longer retries the wait loop for these errors.

Changes

NVML health monitoring

Layer / File(s)	Summary
Return NVML wait errors pkg/device-plugin/nvidiadevice/nvinternal/rm/health.go	checkHealth returns the NVML error after marking all devices unhealthy instead of continuing the wait loop.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

• Project-HAMi/HAMi#2043: Both changes modify NVML health-check error handling.
• Project-HAMi/HAMi#2201: Both changes modify the checkHealth NVML error path.

Suggested labels: enhancement

Suggested reviewers: ouyangluwei163

Poem

A rabbit saw the NVML gate,
And stopped the loop before too late.
Devices were marked unhealthy there,
The error hopped into the air.
No more spinning, logs stay fair.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the CPU spin lock and log flooding caused by NVML event errors.
Linked Issues check	✅ Passed	The change returns non-timeout NVML wait errors instead of continuing the loop, preventing repeated logging and tight-loop activity described in issue #2321.
Out of Scope Changes check	✅ Passed	The single-line change directly addresses the NVML event error loop and introduces no unrelated changes.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[Shouren]

explained in #2321