From fcf494bbc562059ba293f75f5479d28d29ccbf00 Mon Sep 17 00:00:00 2001
From: Maisam Arif <Maisam.Arif@amd.com>
Date: Mon, 14 Jul 2025 07:41:00 -0500
Subject: [PATCH] gpu_metrics caching fix

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Change-Id: I6dacb0b81d6677c354ef3c86af4d7d5156a76d8b
---
 rocm_smi/src/rocm_smi_device.cc | 40 ++++++++++++++++++++-------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/rocm_smi/src/rocm_smi_device.cc b/rocm_smi/src/rocm_smi_device.cc
index 1681b5d1be..80457273e1 100755
--- a/rocm_smi/src/rocm_smi_device.cc
+++ b/rocm_smi/src/rocm_smi_device.cc
@@ -1083,7 +1083,6 @@ const char* Device::get_type_string(DevInfoTypes type) {
 }
 
 namespace {
-
   static int read_env_ms(const char* name, int def) {
     if (const char* s = std::getenv(name)) {
       try {
@@ -1109,25 +1108,28 @@ namespace {
   );
 }
 
-
 int Device::readDevInfoBinary(DevInfoTypes type, std::size_t b_size,
                                 void *p_binary_data) {
   auto sysfs_path = path_;
   std::ostringstream ss;
 
-  // Size will either be 4, or 3872. When 4, it's only reading from the header.
+  // Size will either be 4, or 3872+. When 4, it's only reading from the header.
   // If this header read is inconsequential, we could only cache full read.
-  // However, it seems reading the sysfs in any capacity is the issue, so should remain.
-  const std::string key = path_ + "/device/" + kDevAttribNameMap.at(type) + "#" + std::to_string(b_size);
+  // However, it seems reading the gpu_metrics sysfs in any capacity
+  // is the issue, so should remain.
+  const std::string key = path_ + "/device/" + kDevAttribNameMap.at(type)
+                                + "#" + std::to_string(b_size);
   auto& cache = g_gpu_metrics_cache_map[key];
 
   // Only cache for kDevGpuMetrics
   if (type == DevInfoTypes::kDevGpuMetrics) {
     std::lock_guard<std::mutex> lock(cache.mtx);
     auto now = std::chrono::steady_clock::now();
+    auto last_read_delta = std::chrono::duration_cast<std::chrono::milliseconds>(now - cache.last_read);
 
     if (!cache.data.empty() &&
-        std::chrono::duration_cast<std::chrono::milliseconds>(now - cache.last_read) < kGpuMetricsCacheDuration &&
+        kGpuMetricsCacheDuration > std::chrono::milliseconds::zero() &&
+        last_read_delta < kGpuMetricsCacheDuration &&
         cache.data.size() == b_size) {
 
       std::memcpy(p_binary_data, cache.data.data(), b_size);
@@ -1174,6 +1176,7 @@ int Device::readDevInfoBinary(DevInfoTypes type, std::size_t b_size,
     LOG_ERROR(ss);
     return ENOENT;
   }
+
   if (ROCmLogging::Logger::getInstance()->isLoggerEnabled()) {
     ss << "Successfully read DevInfoBinary for DevInfoType ("
        << get_type_string(type) << ") - SYSFS ("
@@ -1187,15 +1190,22 @@ int Device::readDevInfoBinary(DevInfoTypes type, std::size_t b_size,
   }
 
   // Cache metric data
-   if (type == DevInfoTypes::kDevGpuMetrics) {
-     auto now = std::chrono::steady_clock::now();
-     auto& cache = g_gpu_metrics_cache_map[key];
-     std::lock_guard<std::mutex> lock(cache.mtx);
-     cache.data.assign(
-       reinterpret_cast<uint8_t*>(p_binary_data),
-       reinterpret_cast<uint8_t*>(p_binary_data) + b_size);
-     cache.last_read = now;
-   }
+  if (type == DevInfoTypes::kDevGpuMetrics &&
+      kGpuMetricsCacheDuration > std::chrono::milliseconds::zero()) {
+    auto now = std::chrono::steady_clock::now();
+    auto& cache = g_gpu_metrics_cache_map[key];
+    std::lock_guard<std::mutex> lock(cache.mtx);
+    cache.data.assign(
+      reinterpret_cast<uint8_t*>(p_binary_data),
+      reinterpret_cast<uint8_t*>(p_binary_data) + b_size);
+    cache.last_read = now;
+
+    if (ROCmLogging::Logger::getInstance()->isLoggerEnabled()) {
+      ss << "Successfully Cached GPU Metrics binaryData = " << p_binary_data
+        << "; byte_size = " << std::dec << static_cast<int>(b_size);
+      LOG_INFO(ss);
+    }
+  }
 
   return 0;
 }