[AMD-SMI] Clean up SMI

projects/amdsmi/amdsmi_cli/amdsmi_commands.py

@@ -9939,10 +9939,7 @@ def reset(
                 self.logger.clear_multiple_devices_output()
                 return
             if args.power_cap:
-                final_output = {
-                    "ppt0": "N/A",
-                    "ppt1": "N/A",
-                }
+                final_output = {"ppt0": "N/A", "ppt1": "N/A"}
                 power_limit_types = {}
                 for power_type in amdsmi_interface.AmdSmiPowerCapType:
                     # Strip 'AMDSMI_POWER_CAP_TYPE_' prefix and convert to lowercase

projects/amdsmi/example/amd_smi_drm_example.cc

@@ -302,7 +302,7 @@ int main() {
   // Allocate the memory for the sockets
   std::vector<amdsmi_socket_handle> sockets(socket_count);
   // Get the sockets of the system
-  ret = amdsmi_get_socket_handles(&socket_count, &sockets[0]);
+  ret = amdsmi_get_socket_handles(&socket_count, sockets.data());
   CHK_AMDSMI_RET(ret)
 
   std::cout << "Total Socket: " << socket_count << std::endl;
@@ -338,7 +338,7 @@ int main() {
       // Allocate the memory for the device handlers on the socket
       std::vector<amdsmi_processor_handle> processor_handles(device_count);
       // Get all devices of the socket
-      ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]);
+      ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data());
       PRINT_AMDSMI_RET(ret)
 
       std::cout << "\t**Processor Count: " << device_count << std::endl;
@@ -434,7 +434,7 @@ int main() {
       // Allocate the memory for the device handlers on the socket
       std::vector<amdsmi_processor_handle> processor_handles(device_count);
       // Get all devices of the socket
-      ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]);
+      ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data());
       PRINT_AMDSMI_RET(ret)
 
       std::cout << "\t**Processor Count: " << device_count << std::endl;
@@ -525,7 +525,7 @@ int main() {
       // Allocate the memory for the device handlers on the socket
       std::vector<amdsmi_processor_handle> processor_handles(device_count);
       // Get all devices of the socket
-      ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]);
+      ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data());
       PRINT_AMDSMI_RET(ret)
 
       std::cout << "\t**Processor Count: " << device_count << std::endl;
@@ -649,7 +649,7 @@ int main() {
       // Allocate the memory for the device handlers on the socket
       std::vector<amdsmi_processor_handle> processor_handles(device_count);
       // Get all devices of the socket
-      ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]);
+      ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data());
       PRINT_AMDSMI_RET(ret)
 
       std::cout << "\t**Processor Count: " << device_count << std::endl;
@@ -744,7 +744,7 @@ int main() {
       // Allocate the memory for the device handlers on the socket
       std::vector<amdsmi_processor_handle> processor_handles(device_count);
       // Get all devices of the socket
-      ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]);
+      ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data());
       PRINT_AMDSMI_RET(ret)
 
       std::cout << "\t**Processor Count: " << device_count << std::endl;
@@ -819,7 +819,7 @@ int main() {
     // Allocate the memory for the device handlers on the socket
     std::vector<amdsmi_processor_handle> processor_handles(device_count);
     // Get all devices of the socket
-    ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]);
+    ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data());
     CHK_AMDSMI_RET(ret)
 
     std::cout << "Processor Count: " << device_count << std::endl;

projects/amdsmi/example/amd_smi_nic.cc

@@ -281,7 +281,7 @@ std::optional<std::vector<amd::smi::AMDSmiAINICDevice::AINICInfo>> get_nics() {
   uint32_t soc_count = 10;
   std::vector<amdsmi_socket_handle> sockets(soc_count);
   // Get the sockets of the system
-  amdsmi_status_t status = amdsmi_get_socket_handles(&soc_count, &sockets[0]);
+  amdsmi_status_t status = amdsmi_get_socket_handles(&soc_count, sockets.data());
   if (status != AMDSMI_STATUS_SUCCESS) {
     return std::nullopt;
   }

projects/amdsmi/example/amd_smi_nodrm_example.cc

@@ -64,7 +64,7 @@ int main() {
   // Allocate the memory for the sockets
   std::vector<amdsmi_socket_handle> sockets(socket_count);
   // Get the sockets of the system
-  ret = amdsmi_get_socket_handles(&socket_count, &sockets[0]);
+  ret = amdsmi_get_socket_handles(&socket_count, sockets.data());
   CHK_AMDSMI_RET(ret)
 
   std::cout << "Total Socket: " << socket_count << std::endl;
@@ -85,7 +85,7 @@ int main() {
     // Allocate the memory for the device handlers on the socket
     std::vector<amdsmi_processor_handle> processor_handles(device_count);
     // Get all devices of the socket
-    ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]);
+    ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data());
     CHK_AMDSMI_RET(ret)
 
     // For each device of the socket, get name and temperature.

projects/amdsmi/example/amdsmi_esmi_intg_example.cc

@@ -72,7 +72,7 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv) {
   vector<amdsmi_socket_handle> sockets(socket_count);
 
   // Get the sockets of the system
-  ret = amdsmi_get_socket_handles(&socket_count, &sockets[0]);
+  ret = amdsmi_get_socket_handles(&socket_count, sockets.data());
   CHK_AMDSMI_RET(ret)
 
   cout << "Total Socket: " << socket_count << endl;
@@ -92,7 +92,8 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv) {
     vector<amdsmi_processor_handle> plist(cpu_count);
 
     // Get the cpus for each socket
-    ret = amdsmi_get_processor_handles_by_type(sockets[i], processor_type, &plist[0], &cpu_count);
+    ret =
+        amdsmi_get_processor_handles_by_type(sockets[i], processor_type, plist.data(), &cpu_count);
     CHK_AMDSMI_RET(ret)
 
     // Set processor type as AMDSMI_PROCESSOR_TYPE_AMD_CPU_CORE
@@ -104,7 +105,7 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv) {
     vector<amdsmi_processor_handle> core_list(core_count);
 
     // Get the cpu cores for each socket
-    ret = amdsmi_get_processor_handles_by_type(sockets[i], processor_type, &core_list[0],
+    ret = amdsmi_get_processor_handles_by_type(sockets[i], processor_type, core_list.data(),
                                                &core_count);
     CHK_AMDSMI_RET(ret)
 

projects/amdsmi/py-interface/amdsmi_interface.py

@@ -41,8 +41,8 @@ class MaxUIntegerTypes(IntEnum):
     UINT64_T = 0xFFFFFFFFFFFFFFFF
 
 
-NO_OF_32BITS = sys.getsizeof(ctypes.c_uint32) * 8
-NO_OF_64BITS = sys.getsizeof(ctypes.c_uint64) * 8
+NO_OF_32BITS = ctypes.sizeof(ctypes.c_uint32) * 8
+NO_OF_64BITS = ctypes.sizeof(ctypes.c_uint64) * 8
 KILO = math.pow(10, 3)
 AMDSMI_MAX_UTIL = 0xFFFFFFFF
 AMDSMI_MAX_PPT_LIMIT = 0xFFFFFFFF
@@ -474,7 +474,7 @@ class AmdSmiEvtNotificationType(IntEnum):
     GPU_POST_RESET = amdsmi_wrapper.AMDSMI_EVT_NOTIF_GPU_POST_RESET
     MIGRATE_START = amdsmi_wrapper.AMDSMI_EVT_NOTIF_MIGRATE_START
     MIGRATE_END = amdsmi_wrapper.AMDSMI_EVT_NOTIF_MIGRATE_END
-    PAGE_FAULT_START = amdsmi_wrapper.AMDSMI_EVT_NOTIF_PAGE_FAULT_END
+    PAGE_FAULT_START = amdsmi_wrapper.AMDSMI_EVT_NOTIF_PAGE_FAULT_START
     PAGE_FAULT_END = amdsmi_wrapper.AMDSMI_EVT_NOTIF_PAGE_FAULT_END
     QUEUE_EVICTION = amdsmi_wrapper.AMDSMI_EVT_NOTIF_QUEUE_EVICTION
     QUEUE_RESTORE = amdsmi_wrapper.AMDSMI_EVT_NOTIF_QUEUE_RESTORE
@@ -5148,7 +5148,7 @@ def amdsmi_set_gpu_clk_limit(
         clk_type_conversion = amdsmi_wrapper.AMDSMI_CLK_TYPE_DF
     else:
         raise AmdSmiParameterException(f"Unsupported clock type: {clk_type}", str)
-    
+
     if limit_type.lower() == "min":
         limit_type_conversion = amdsmi_wrapper.CLK_LIMIT_MIN
     elif limit_type.lower() == "max":

projects/amdsmi/rocm_smi/src/rocm_smi.cc

@@ -7052,17 +7052,32 @@ rsmi_status_t rsmi_event_notification_init(uint32_t dv_ind) {
 
   int ret = ioctl(smi.kfd_notif_evt_fh(), AMDKFD_IOC_SMI_EVENTS, &args);
   if (ret < 0) {
-    return amd::smi::ErrnoToRsmiStatus(errno);
+    rsmi_status_t err = amd::smi::ErrnoToRsmiStatus(errno);
+    if (smi.kfd_notif_evt_fh_refcnt_dec() == 0) {
+      close(smi.kfd_notif_evt_fh());
+      smi.set_kfd_notif_evt_fh(-1);
+    }
+    return err;
   }
   if (args.anon_fd < 1) {
+    if (smi.kfd_notif_evt_fh_refcnt_dec() == 0) {
+      close(smi.kfd_notif_evt_fh());
+      smi.set_kfd_notif_evt_fh(-1);
+    }
     return RSMI_STATUS_NO_DATA;
   }
 
   dev->set_evt_notif_anon_fd(args.anon_fd);
   FILE* anon_file_ptr = fdopen(static_cast<int>(args.anon_fd), "r");
   if (anon_file_ptr == nullptr) {
+    rsmi_status_t err = amd::smi::ErrnoToRsmiStatus(errno);
     close(dev->evt_notif_anon_fd());
-    return amd::smi::ErrnoToRsmiStatus(errno);
+    dev->set_evt_notif_anon_fd(-1);
+    if (smi.kfd_notif_evt_fh_refcnt_dec() == 0) {
+      close(smi.kfd_notif_evt_fh());
+      smi.set_kfd_notif_evt_fh(-1);
+    }
+    return err;
   }
   dev->set_evt_notif_anon_file_ptr(anon_file_ptr);
 
@@ -7177,7 +7192,7 @@ rsmi_status_t rsmi_event_notification_get(int timeout_ms, uint32_t* num_elem,
             char task_name[MAX_EVENT_NOTIFICATION_MSG_SIZE];
             memset(task_name, '\0', MAX_EVENT_NOTIFICATION_MSG_SIZE);
 
-            sscanf(message, "%x:%s\n", &pid, task_name);
+            sscanf(message, "%x:%255s\n", &pid, task_name);
             std::stringstream final_message;
             final_message << "PID: " << std::to_string(pid).c_str() << "  task name: " << task_name;
 
@@ -7201,7 +7216,7 @@ rsmi_status_t rsmi_event_notification_get(int timeout_ms, uint32_t* num_elem,
             char reset_cause[MAX_EVENT_NOTIFICATION_MSG_SIZE];
             memset(reset_cause, '\0', MAX_EVENT_NOTIFICATION_MSG_SIZE);
 
-            sscanf(message, "%x %[^\n]\n", &reset_seq_num, reset_cause);
+            sscanf(message, "%x %255[^\n]\n", &reset_seq_num, reset_cause);
             std::stringstream final_message;
             final_message << "reset sequence number: " << std::to_string(reset_seq_num).c_str()
                           << "  reset cause: " << reset_cause;
@@ -7213,7 +7228,7 @@ rsmi_status_t rsmi_event_notification_get(int timeout_ms, uint32_t* num_elem,
             uint32_t reset_seq_num;
 
             char tmp[MAX_EVENT_NOTIFICATION_MSG_SIZE];
-            sscanf(message, "%x %[^\n]\n", &reset_seq_num, tmp);
+            sscanf(message, "%x %255[^\n]\n", &reset_seq_num, tmp);
             std::stringstream final_message;
             final_message << "reset sequence number: " << std::to_string(reset_seq_num).c_str();
 
@@ -7356,7 +7371,7 @@ rsmi_status_t rsmi_event_notification_get(int timeout_ms, uint32_t* num_elem,
           case RSMI_EVT_NOTIF_EVENT_PROCESS_END: {
             uint32_t pid;
             char task[MAX_EVENT_NOTIFICATION_MSG_SIZE];
-            int rc = sscanf(message, "%x %s", &pid, task);
+            int rc = sscanf(message, "%x %255s", &pid, task);
             std::stringstream msg;
             if (rc == 2) {
               msg << "PID: " << pid << "  task: " << task;

projects/amdsmi/rocm_smi/src/rocm_smi_binary_parser.cc

@@ -113,6 +113,7 @@ int present_pmmetrics(const char* fname, rsmi_name_value_t** kv, uint32_t* kvnum
 
   buf1 = reinterpret_cast<uint8_t*>(calloc(1, 65536));
   if (!buf1) {
+    fclose(infile);
     return -1;
   }
 
@@ -127,6 +128,8 @@ int present_pmmetrics(const char* fname, rsmi_name_value_t** kv, uint32_t* kvnum
       break;
     default:
       fprintf(stderr, "Metrics version %d not supported\n", pmmetrics_version);
+      fclose(infile);
+      free(buf1);
       return -1;
   }
   r = parse_pmmetric_table(buf1, table, len, kv, kvnum);

projects/amdsmi/rocm_smi/src/rocm_smi_device.cc

@@ -1778,7 +1778,7 @@ rsmi_status_t Device::isRestartInProgress(bool* isRestartInProgress, bool* isAMD
   if ((success == true) && (!out.empty())) {
     isSystemAMDGPUModuleLive = containsString(out, "live");
   }
-  if (*isAMDGPUModuleLive) {
+  if (isSystemAMDGPUModuleLive) {
     deviceRestartInProgress = false;
   }
   *isRestartInProgress = deviceRestartInProgress;

projects/amdsmi/rocm_smi/src/rocm_smi_kfd.cc

@@ -432,7 +432,7 @@ static int ReadKFDGpuId(uint32_t kfd_node_id, uint64_t* gpu_id) {
     return ENXIO;
   }
 
-  *gpu_id = static_cast<uint64_t>(std::stoi(gpu_id_str));
+  *gpu_id = std::stoull(gpu_id_str);
   return 0;
 }
 
@@ -668,7 +668,7 @@ int GetProcessGPUs(uint32_t pid, std::unordered_set<uint64_t>* gpu_set) {
 
       uint64_t val;
       try {
-        val = static_cast<uint64_t>(std::stoi(tmp));
+        val = std::stoull(tmp);
       } catch (...) {
         std::cerr << "Error; read invalid data: " << tmp << " from " << q_gpu_id_str << std::endl;
         closedir(queues_dir_hd);

projects/amdsmi/rocm_smi/src/rocm_smi_monitor.cc

@@ -458,6 +458,7 @@ static int get_supported_sensors(std::string dir_path, std::string fn_reg_ex,
     std::cout << "Regular expression error:" << std::endl;
     std::cout << e.what() << std::endl;
     std::cout << "Regex error code: " << e.code() << std::endl;
+    closedir(hwmon_dir);
     return -3;
   }
   return 0;

projects/amdsmi/rocm_smi/src/rocm_smi_utils.cc

@@ -623,7 +623,7 @@ std::pair<bool, std::string> executeCommand(std::string command, bool stdOut) {
   }
 
   // any return code other than 0, is a failed execution
-  if (pclose(pipe) != 0) {
+  if (pipe && pclose(pipe) != 0) {
     successfulRun = false;
   }
 
@@ -681,7 +681,7 @@ rsmi_status_t storeTmpFile(uint32_t dv_ind, std::string parameterName, std::stri
   }
   // template for our file
   std::string fullTempFilePath = "/tmp/" + fullFileName + ".XXXXXX";
-  char* fileName = &fullTempFilePath[0];
+  char* fileName = fullTempFilePath.data();
   int fd = mkstemp(fileName);
   if (fd == -1) {
     return RSMI_STATUS_FILE_ERROR;
@@ -1101,8 +1101,7 @@ const char* my_fname(void) {
   dladdr(reinterpret_cast<void*>(my_fname), &dl_info);
   return (dl_info.dli_fname);
 #else
-  std::string emptyRet = "";
-  return emptyRet.c_str();
+  return "";
 #endif
 }
 

projects/amdsmi/src/amd_smi/amd_smi.cc

@@ -2761,7 +2761,7 @@ amdsmi_status_t amdsmi_get_gpu_event_notification(int timeout_ms, uint32_t* num_
 
   // Get the rsmi data
   std::vector<rsmi_evt_notification_data_t> r_data(*num_elem);
-  rsmi_status_t r = rsmi_event_notification_get(timeout_ms, num_elem, &r_data[0]);
+  rsmi_status_t r = rsmi_event_notification_get(timeout_ms, num_elem, r_data.data());
   if (r != RSMI_STATUS_SUCCESS) {
     return amd::smi::rsmi_to_amdsmi_status(r);
   }
@@ -5611,7 +5611,7 @@ amdsmi_status_t amdsmi_get_processor_handle_from_bdf(amdsmi_bdf_t bdf,
 
   std::vector<amdsmi_socket_handle> sockets(socket_count);
 
-  status = amdsmi_get_socket_handles(&socket_count, &sockets[0]);
+  status = amdsmi_get_socket_handles(&socket_count, sockets.data());
   if (status != AMDSMI_STATUS_SUCCESS) {
     return status;
   }
@@ -5634,7 +5634,7 @@ amdsmi_status_t amdsmi_get_processor_handle_from_bdf(amdsmi_bdf_t bdf,
     // Allocate the memory for the device handlers on the socket
     std::vector<amdsmi_processor_handle> processor_handles(processor_count);
     // Get all processors of the socket
-    status = amdsmi_get_processor_handles(sockets[i], &processor_count, &processor_handles[0]);
+    status = amdsmi_get_processor_handles(sockets[i], &processor_count, processor_handles.data());
     if (status != AMDSMI_STATUS_SUCCESS) {
       return status;
     }
@@ -5716,8 +5716,8 @@ amdsmi_status_t amdsmi_get_link_topology_nearest(amdsmi_processor_handle process
     return api_status;
   }
 
-  amdsmi_socket_handle socket_list[socket_counter];
-  if (auto api_status = amdsmi_get_socket_handles(&socket_counter, &socket_list[0]);
+  std::vector<amdsmi_socket_handle> socket_list(socket_counter);
+  if (auto api_status = amdsmi_get_socket_handles(&socket_counter, socket_list.data());
       (api_status != amdsmi_status_t::AMDSMI_STATUS_SUCCESS)) {
     return api_status;
   }
@@ -7290,7 +7290,7 @@ amdsmi_status_t amdsmi_get_cpu_handles(uint32_t* cpu_count,
   // Allocate the memory for the sockets
   std::vector<amdsmi_socket_handle> sockets(soc_count);
   // Get the sockets of the system
-  status = amdsmi_get_socket_handles(&soc_count, &sockets[0]);
+  status = amdsmi_get_socket_handles(&soc_count, sockets.data());
   if (status != AMDSMI_STATUS_SUCCESS) return status;
 
   for (index = 0; index < soc_count; index++) {
@@ -7303,7 +7303,7 @@ amdsmi_status_t amdsmi_get_cpu_handles(uint32_t* cpu_count,
     // Allocate the memory for the cpus
     std::vector<amdsmi_processor_handle> plist(cpu_per_soc);
     // Get the cpus for each socket
-    status = amdsmi_get_processor_handles_by_type(sockets[index], processor_type, &plist[0],
+    status = amdsmi_get_processor_handles_by_type(sockets[index], processor_type, plist.data(),
                                                   &cpu_per_soc);
     if (status != AMDSMI_STATUS_SUCCESS) return status;
     cpu_handles.insert(cpu_handles.end(), plist.begin(), plist.end());
@@ -7342,7 +7342,7 @@ amdsmi_status_t amdsmi_get_cpucore_handles(uint32_t* cores_count,
   // Allocate the memory for the sockets
   std::vector<amdsmi_socket_handle> sockets(soc_count);
   // Get the sockets of the system
-  status = amdsmi_get_socket_handles(&soc_count, &sockets[0]);
+  status = amdsmi_get_socket_handles(&soc_count, sockets.data());
   if (status != AMDSMI_STATUS_SUCCESS) return status;
 
   for (index = 0; index < soc_count; index++) {
@@ -7354,7 +7354,7 @@ amdsmi_status_t amdsmi_get_cpucore_handles(uint32_t* cores_count,
     // Allocate the memory for the cores
     std::vector<amdsmi_processor_handle> plist(cores_per_soc);
     // Get the coress for each socket
-    status = amdsmi_get_processor_handles_by_type(sockets[index], processor_type, &plist[0],
+    status = amdsmi_get_processor_handles_by_type(sockets[index], processor_type, plist.data(),
                                                   &cores_per_soc);
     if (status != AMDSMI_STATUS_SUCCESS) {
       return status;