Repository navigation
feat(amdsmi): add metric --usage-fields and keep partial usage - #12600
koushikbillakanti-amd wants to merge 4 commits into
Conversation
github-actions
Bot
added
documentation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
ba17b79 to
caa56ee
Compare
Signed-off-by: Koushik Billakanti <kbillaka@amd.com>
e61a161 to
50a9c34
Compare
koushikbillakanti-amd
changed the title
# Conflicts: # projects/amdsmi/CHANGELOG.md
|
🤖 claude: Merged develop at |
Signed-off-by: Koushik Billakanti <kbillaka@amd.com>
Signed-off-by: Koushik Billakanti <kbillaka@amd.com>
| ) | ||
| engine_usage["gfx_busy_inst"] = new_xcp_dict | ||
| engine_usage.update(amdsmi_interface.amdsmi_get_gpu_activity(args.gpu)) | ||
| except amdsmi_exception.AmdSmiLibraryException as error: |
There was a problem hiding this comment.
Since this now catches only AmdSmiLibraryException, an AmdSmiParameterException from a bad handle (a sibling class, not a subclass) would escape and abort the whole metric run — losing power/clock/temp readings too, not just usage. Same pattern repeats at the amdsmi_get_vcn_busy_percent call below (~line 563). Worth widening the catch here, or fixing it once in amdsmi_cli.py's top-level handler so every call site in the file benefits.
| values_dict["usage"] = { | ||
| key: "N/A" | ||
| if usage_fields and value in ({}, []) | ||
| else self._format_usage_value(value) |
There was a problem hiding this comment.
The usage_fields and guard means an empty vcn_activity renders as [] under plain --usage but as N/A under --usage-fields vcn_activity — same underlying data, different output depending which flag was used. Might be worth dropping the usage_fields and so both paths treat an empty container as N/A consistently.
| writer.writerows(rows) | ||
|
|
||
| def _print_dual_csv_output(self, multiple_device_enabled=False, watching_output=False): | ||
| if multiple_device_enabled: |
There was a problem hiding this comment.
_print_dual_csv_output (used by amd-smi monitor --process --csv, not the standalone process subcommand) still builds its header from a set, so column order here is hash-seed dependent — the same instability _print_csv_output just got fixed for above. The PR description says "CSV columns are written in a stable order," but that's only true for the metric path.
This branch has not been deployed
3 participants
Motivation
amd-smi metric --usagealways prints every usage field, and if one data source fails, the whole section turns intoN/A, even when other sources have good readings. This PR lets users pick the fields they want and keeps the readings that work.Technical Details
amd-smi metric --usage-fields FIELD[,...]prints only the named usage fields, in order, for human, JSON, and CSV output. A named field always shows up, asN/Aif it isn't available.usageis always a set of fields, even when every source fails.amd-smi metric --watch --filenow keeps the last sample for GPU, NIC, and switch, including on Ctrl-C. A watch stopped before its first sample leaves an existing file alone.About the ticket's
N/Afields: on the R9700 (metrics table v1.3), UMC, MM, and VCN activity read fine. JPEG activity, GFX busy, and JPEG busy aren't in that table, so they stayN/Auntil the driver exposes a newer one.Left for a follow-up:
amd-smi monitorwatch files still drop the last sample.JIRA ID
JIRA ID: ROCM-31813
Test Plan
developand this branch side by side on real GPUs with the same library, covering all output formats,--file, watch runs, and bad input.Test Result
developkeeps 8). R9700 and an RDNA4 GPU with no usage data passed on an earlier revision.ASAN unit tests(and the bot check that reports it), which fails ondevelopwith the same 16 C++ tests. This PR has no C++ changes.Submission Checklist