Add verify_gpu_reset test case for GPU reset validation - #4716
Open
SrikanthMyakam (SRIKKANTH) wants to merge 2 commits into
Open
SrikanthMyakam (SRIKKANTH) wants to merge 2 commits into
SrikanthMyakam (SRIKKANTH) wants to merge 2 commits into
Conversation
Add a new GPU test case that resets all GPUs with 'nvidia-smi -r' several times and checks the guest kernel stays healthy. A CPU soft lockup during GPU reset is timing dependent, so the reset is repeated and dmesg is diffed against a baseline after every iteration to attribute only new kernel errors to the reset. The GPU count from lspci and the driver health are re-checked at the end so a GPU that never comes back on the PCI bus is caught. nvidia-persistenced, nvidia-fabricmanager and nvidia-dcgm hold /dev/nvidia* open and make the reset fail with "In use by another client", and idle nvidia_uvm/nvidia_drm still count as clients. Stop those services and unload the modules before resetting, then restore the services afterwards without masking the original failure. An unsupported reset path is reported as a skip instead of a failure. Add NvidiaSmi.reset() which returns the raw result so callers can tell an unsupported reset apart from a real failure. Key Test Cases: verify_gpu_reset|verify_load_gpu_driver Impacted LISA Features: Gpu, GpuEnabled Tested Azure Marketplace Images: - Canonical ubuntu-24_04-lts server latest
SrikanthMyakam (SRIKKANTH)
requested review from
LiliDeng,
Johnson (johnsongeorge-w),
paxue and
Umang Francis (umfranci)
as code owners
September 11, 2026 13:36
Copilot started reviewing on behalf of
SrikanthMyakam (SRIKKANTH)
September 11, 2026 13:36
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved cleanup, timeout, baseline, service-restoration, and post-reset validation findings remain.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds NVIDIA GPU reset validation and exposes raw reset results through NvidiaSmi.reset().
Changes:
- Repeats GPU resets with dmesg and GPU/driver health checks.
- Releases GPU-holding services/modules before reset and restores services afterward.
- Adds reset timeout and unsupported-path handling.
File summaries
| File | Summary and final findings |
|---|---|
lisa/tools/gpu_smi.py |
Adds NVIDIA reset support. Moderate (1 vote): the 2,000-second case timeout does not cover three 900-second resets plus setup and cleanup. |
lisa/microsoft/testsuites/gpu/gpusuite.py |
Adds verify_gpu_reset. Moderate (3 votes): protect holder release with cleanup and preserve partially stopped services. Moderate (3 votes): catch AssertionError during service restoration. Moderate (1 vote): increase the case timeout. Critical (2 votes): force-refresh the post-reset GPU query. Moderate (1 vote): capture the dmesg baseline after holder release. |
Review details
Suppressed comments (3)
lisa/microsoft/testsuites/gpu/gpusuite.py:291
- The case timeout is 2,000 seconds, but each reset is allowed 900 seconds and this loop performs three resets (2,700 seconds) before setup, dmesg checks, and cleanup are included. On a slow multi-GPU SKU, a valid reset can therefore be terminated by the case timeout during the third iteration; increase this case timeout to cover the reset budget plus setup/cleanup.
timeout=TIMEOUT,
lisa/microsoft/testsuites/gpu/gpusuite.py:315
- Major: This baseline is captured before
_release_gpu_holders()stops services and unloads the NVIDIA modules. If that setup emits a matching kernel error, the first iteration reports it as a reset regression even though the reset did not cause it. Capture the baseline after holder release and immediately before the reset loop, while keeping it inside the cleanup scope.
baseline_errors = set(
dmesg.check_kernel_errors(force_run=True, throw_error=False).splitlines()
lisa/tools/gpu_smi.py:36
- The test method is limited to
GpuTestSuite.TIMEOUT = 2000seconds, but it can run three resets and eachNvidiaSmi.resetmay wait up toRESET_TIMEOUT = 900seconds, before accounting for driver setup. Three slow-but-successful resets can therefore exceed the outer case timeout and interrupt the method during reset/cleanup. Increase the case timeout to cover the loop and setup, or derive each reset timeout from the remaining case budget.
RESET_TIMEOUT = 900
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "back on the PCI bus" | ||
| ).is_equal_to(expected_count) | ||
|
|
||
| _check_driver_installed(node, log) |
Collaborator
AI Test Case SelectionSelected 6 test case(s): verify_gpu_reset,verify_load_gpu_driver,verify_gpu_extension_installation,verify_gpu_adapter_count,verify_gpu_rescind_validation,verify_gpu_cuda_with_pytorch Marketplace image: Result: Succeeded |
Copilot started reviewing on behalf of
SrikanthMyakam (SRIKKANTH)
September 11, 2026 16:12
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Fix the dmesg duplicate-counting gap and ensure service restoration works across backends and surfaces failures.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
Comment on lines
+328
to
+331
| baseline_errors = set( | ||
| dmesg.check_kernel_errors( | ||
| force_run=True, throw_error=False | ||
| ).splitlines() |
| # start_service asserts on the exit code, so a failed restart | ||
| # surfaces as an AssertionError. | ||
| try: | ||
| service.start_service(name) |
Comment on lines
+376
to
+377
| except (LisaException, AssertionError) as identifier: | ||
| log.info(f"could not restart '{name}' after reset: {identifier}") |
Collaborator
AI Test Case SelectionSelected 6 test case(s): verify_gpu_reset,verify_load_gpu_driver,verify_gpu_extension_installation,verify_gpu_adapter_count,verify_gpu_rescind_validation,verify_gpu_cuda_with_pytorch Marketplace image: Result: Succeeded |
LiliDeng
reviewed
Sep 16, 2026
| 5. Validate the gpu count is unchanged and the driver is still healthy. | ||
| """, | ||
| timeout=GPU_RESET_TIMEOUT, | ||
| priority=3, |
LiliDeng
reviewed
Sep 16, 2026
| output = f"{result.stdout}\n{result.stderr}" | ||
| if self._gpu_reset_unsupported_pattern.search(output): | ||
| raise SkippedException( | ||
| f"GPU reset is not available to the guest on this VM " |
Collaborator
There was a problem hiding this comment.
f" is not needed in this line.
LiliDeng
reviewed
Sep 16, 2026
| raise LisaException( | ||
| f"'nvidia-smi -r' failed with exit code {result.exit_code} " | ||
| f"on gpu reset iteration {iteration}: {output}. Verify no " | ||
| f"process is holding a GPU and that the VM size allows a " |
Collaborator
There was a problem hiding this comment.
f" is not needed in this line.
LiliDeng
reviewed
Sep 16, 2026
| f"'nvidia-smi -r' failed with exit code {result.exit_code} " | ||
| f"on gpu reset iteration {iteration}: {output}. Verify no " | ||
| f"process is holding a GPU and that the VM size allows a " | ||
| f"guest initiated reset." |
Collaborator
There was a problem hiding this comment.
f" is not needed in this line.
LiliDeng
reviewed
Sep 16, 2026
| - baseline_errors | ||
| ) | ||
| assert_that(sorted(new_errors)).described_as( | ||
| f"new kernel errors appeared in dmesg after gpu reset iteration " |
Collaborator
There was a problem hiding this comment.
f" is not needed in this line.
LiliDeng
reviewed
Sep 16, 2026
| assert_that(sorted(new_errors)).described_as( | ||
| f"new kernel errors appeared in dmesg after gpu reset iteration " | ||
| f"{iteration}; resetting a GPU must not hang or destabilize the " | ||
| f"guest kernel" |
Collaborator
There was a problem hiding this comment.
f" is not needed in this line.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Add a new GPU test case that resets all GPUs with 'nvidia-smi -r' several times and checks the guest kernel stays healthy. A CPU soft lockup during GPU reset is timing dependent, so the reset is repeated and dmesg is diffed against a baseline after every iteration to attribute only new kernel errors to the reset. The GPU count from lspci and the driver health are re-checked at the end so a GPU that never comes back on the PCI bus is caught.
nvidia-persistenced, nvidia-fabricmanager and nvidia-dcgm hold /dev/nvidia* open and make the reset fail with "In use by another client", and idle nvidia_uvm/nvidia_drm still count as clients. Stop those services and unload the modules before resetting, then restore the services afterwards without masking the original failure. An unsupported reset path is reported as a skip instead of a failure.
Add NvidiaSmi.reset() which returns the raw result so callers can tell an unsupported reset apart from a real failure.
Related Issue
Type of Change
Checklist
Test Validation
Key Test Cases:
Impacted LISA Features:
Tested Azure Marketplace Images:
Test Results