Fix assertion failure when duplicate client_id encountered #434
Conversation
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
The assertion "We should not be processing a client id twice per update" can fail when a process has multiple file descriptors referencing the same DRM client (e.g., via dup(), fork(), or DRM master operations). The kcmp syscall filters duplicate file descriptions but not distinct file descriptions that report the same underlying DRM client_id. This change converts the debug assertion into a runtime check that gracefully skips duplicate entries and frees any newly allocated cache entries to prevent memory leaks. Fixes the crash: nvtop: ./src/extract_gpuinfo_amdgpu.c:964: parse_drm_fdinfo_amd: Assertion `!cache_entry_check && "We should not be processing a client id twice per update"' failed. Applied to all affected drivers: - AMDGPU - Intel i915 - Intel Xe - Qualcomm MSM (also fixed incorrect hash key usage) - ARM Mali
Contributor
Author
|
Fixes #435 |
Owner
|
Thanks a lot for looking into it |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Fixes the assertion failure crash:
Root Cause
The assertion fails when a process has multiple file descriptors referencing the same DRM client (e.g., via
dup(),fork(), or DRM master operations). Thekcmpsyscall filters duplicate file descriptions but not distinct file descriptions that report the same underlying DRMclient_id.Fix
Converts the debug assertion into a runtime check that:
client_identries incurrent_update_process_cacheFiles Changed
Applied fix to all affected drivers:
extract_gpuinfo_amdgpu.c- AMD GPUsextract_gpuinfo_intel_i915.c- Intel i915extract_gpuinfo_intel_xe.c- Intel Xeextract_gpuinfo_msm.c- Qualcomm MSM (also fixed incorrect hash key: was using&cidinstead of&cache_entry->client_id)extract_gpuinfo_mali_common.c- ARM MaliTesting
Tested on AMD GPU where the crash was occurring - nvtop now runs without crashing.