GPUOP-880: SR-IOV GIM host-side metric coverage (Phase A 8.7 + Phase …#76
GPUOP-880: SR-IOV GIM host-side metric coverage (Phase A 8.7 + Phase …#76bhatturu wants to merge 1 commit into
Conversation
eb9df44 to
5266a98
Compare
| std::unique_ptr<amdsmi_metric_t[]> *heap, | ||
| amdsmi_metric_t **out_buf, uint32_t *out_count) | ||
| { | ||
| amdsmi_status_t amdsmi_ret; |
There was a problem hiding this comment.
follow ascending len declaration uint32_t count = 0; to the top
| amdsmi_metric_t buf_stack[kStackMax]; | ||
| std::unique_ptr<amdsmi_metric_t[]> buf_heap; | ||
| amdsmi_metric_t *buf = nullptr; | ||
| uint32_t count = 0; |
5266a98 to
bf8969a
Compare
| amdsmi_gpu_metrics_t metrics_info = {}; | ||
| amdsmi_gpu_metrics_t cached_metrics = {}; | ||
| amdsmi_power_info_t power_info = {}; | ||
| amdsmi_node_handle node_handle = {}; |
There was a problem hiding this comment.
Variables should be in increasing order of line width
f1a906a to
e0e461f
Compare
| amdsmi_power_info_t power_info = {}; | ||
| amdsmi_engine_usage_t usage_info = {}; | ||
| amdsmi_node_handle node_handle = {}; | ||
| amdsmi_npm_info_t npm_info = {}; |
There was a problem hiding this comment.
variable ordering is incorrect
| gpu_handle, amdsmi_ret); | ||
| return amdsmi_ret_to_sdk_ret(amdsmi_ret); | ||
| } | ||
| if (count > AGA_GIM_MAX_METRICS) { |
There was a problem hiding this comment.
do we need this check again? Does the count get set when metrics list is not null?
| idx = m.res_instance; | ||
|
|
||
| if (unlikely(debug)) { | ||
| AGA_TRACE_ERR("gim_metrics GPU {} name {} val {} flags {} inst {}", |
There was a problem hiding this comment.
Why do we need an env variable? Can't we make it a VERBOSE log?
e0e461f to
2b64c9a
Compare
| /// \return SDK_RET_OK or error code | ||
| static sdk_ret_t | ||
| smi_walk_gpu_metrics (aga_gpu_handle_t gpu_handle, | ||
| aga_gpu_stats_t *stats) |
There was a problem hiding this comment.
can be moved to previous line
| is_inst = (m.flags & AMDSMI_METRIC_TYPE_INST); | ||
| idx = m.res_instance; | ||
|
|
||
| AGA_TRACE_DEBUG("gim_metrics GPU {} name {} val {} flags {} inst {}", |
There was a problem hiding this comment.
Make this VERBOSE. Also make the log more readable, we cannot have _ and is the last field in the log supposed to be instantaneous or index? Make the log something like "GPU {} metrics name {} value {} flags {} ..."
2b64c9a to
ed0b469
Compare
ed0b469 to
52132fc
Compare
…B 9.0.0.K)
Motivation
Technical Details
Test Plan
Test Result
Submission Checklist