From 69b6321b4f602808353d6c7f14afb87d0f717241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20K=2E=20Guti=C3=A9rrez?= Date: Tue, 19 Mar 2024 22:16:40 -0600 Subject: [PATCH] Cleanup more code. (#96) Signed-off-by: Samuel K. Gutierrez --- src/qvi-bind.cc | 5 +---- src/qvi-devinfo.h | 14 +++++++++----- src/qvi-hwloc.cc | 36 ++++++++++++++++++++---------------- src/qvi-hwloc.h | 7 ------- src/qvi-hwpool.cc | 4 +++- src/qvi-rmi.cc | 12 ++++++------ src/qvi-scope.cc | 2 +- src/qvi-utils.cc | 10 +++++----- 8 files changed, 45 insertions(+), 45 deletions(-) diff --git a/src/qvi-bind.cc b/src/qvi-bind.cc index 96e19c30..bd28961d 100644 --- a/src/qvi-bind.cc +++ b/src/qvi-bind.cc @@ -88,10 +88,7 @@ qvi_bind_push( ) { // Copy input bitmap because we don't want to directly modify it. hwloc_cpuset_t bitmap_copy = nullptr; - int rc = qvi_hwloc_bitmap_calloc(&bitmap_copy); - if (rc != QV_SUCCESS) goto out; - - rc = qvi_hwloc_bitmap_copy(cpuset, bitmap_copy); + int rc = qvi_hwloc_bitmap_dup(cpuset, &bitmap_copy); if (rc != QV_SUCCESS) goto out; // Change policy rc = qvi_rmi_task_set_cpubind_from_cpuset( diff --git a/src/qvi-devinfo.h b/src/qvi-devinfo.h index 171d6a09..d0d3e2d9 100644 --- a/src/qvi-devinfo.h +++ b/src/qvi-devinfo.h @@ -1,6 +1,6 @@ /* -*- Mode: C++; c-basic-offset:4; indent-tabs-mode:nil -*- */ /* - * Copyright (c) 2022 Triad National Security, LLC + * Copyright (c) 2022-2024 Triad National Security, LLC * All rights reserved. * * This file is part of the quo-vadis project. See the LICENSE file at the @@ -22,6 +22,7 @@ /** Device information. */ struct qvi_devinfo_t { + int qvim_rc = QV_ERR_INTERNAL; /** Device type. */ qv_hw_obj_type_t type = QV_HW_OBJ_LAST; /** Device ID. */ @@ -44,14 +45,17 @@ struct qvi_devinfo_t { { int nw = asprintf(&this->pci_bus_id, "%s", pci_bus_id); if (nw == -1) { - this->pci_bus_id = nullptr; + qvim_rc = QV_ERR_OOR; + return; } + nw = asprintf(&this->uuid, "%s", uuid); if (nw == -1) { - this->uuid = nullptr; + qvim_rc = QV_ERR_OOR; + return; } - (void)qvi_hwloc_bitmap_calloc(&affinity); - (void)qvi_hwloc_bitmap_copy(c, affinity); + + qvim_rc = qvi_hwloc_bitmap_dup(c, &affinity); } /** Destructor */ ~qvi_devinfo_t(void) diff --git a/src/qvi-hwloc.cc b/src/qvi-hwloc.cc index 5a56c0da..6aef394c 100644 --- a/src/qvi-hwloc.cc +++ b/src/qvi-hwloc.cc @@ -21,6 +21,10 @@ #include "qvi-nvml.h" #include "qvi-rsmi.h" +constexpr int pci_bus_id_buff_size = 16; +constexpr int dev_name_buff_size = 32; +constexpr int uuid_buff_size = 64; + /** Device list type. */ using qvi_hwloc_dev_list_t = std::vector< std::shared_ptr @@ -66,11 +70,11 @@ typedef struct qvi_hwloc_device_s { /** CUDA/ROCm visible devices ID */ int visdev_id = QVI_HWLOC_DEVICE_INVISIBLE_ID; /** Device name */ - char name[QVI_HWLOC_DEV_NAME_BUFF_SIZE] = {'\0'}; + char name[dev_name_buff_size] = {'\0'}; /** PCI bus ID */ - char pci_bus_id[QVI_HWLOC_PCI_BUS_ID_BUFF_SIZE] = {'\0'}; + char pci_bus_id[pci_bus_id_buff_size] = {'\0'}; /** UUID */ - char uuid[QVI_HWLOC_UUID_BUFF_SIZE] = {'\0'}; + char uuid[uuid_buff_size] = {'\0'}; /** Constructor */ qvi_hwloc_device_s(void) { @@ -373,19 +377,19 @@ set_general_device_info( // Save device name. int nw = snprintf( device->name, - QVI_HWLOC_DEV_NAME_BUFF_SIZE, + dev_name_buff_size, "%s", obj->name ); - if (nw >= QVI_HWLOC_DEV_NAME_BUFF_SIZE) { + if (nw >= dev_name_buff_size) { return QV_ERR_INTERNAL; } // Set the PCI bus ID. nw = snprintf( device->pci_bus_id, - QVI_HWLOC_PCI_BUS_ID_BUFF_SIZE, + pci_bus_id_buff_size, "%s", pci_bus_id ); - if (nw >= QVI_HWLOC_PCI_BUS_ID_BUFF_SIZE) { + if (nw >= pci_bus_id_buff_size) { return QV_ERR_INTERNAL; } // Set visible device ID, if applicable. @@ -403,10 +407,10 @@ set_gpu_device_info( if (sscanf(obj->name, "rsmi%d", &id) == 1) { device->smi = id; int nw = snprintf( - device->uuid, QVI_HWLOC_UUID_BUFF_SIZE, "%s", + device->uuid, uuid_buff_size, "%s", hwloc_obj_get_info_by_name(obj, "AMDUUID") ); - if (nw >= QVI_HWLOC_UUID_BUFF_SIZE) { + if (nw >= uuid_buff_size) { return QV_ERR_INTERNAL; } return qvi_hwloc_rsmi_get_device_cpuset_by_device_id( @@ -419,10 +423,10 @@ set_gpu_device_info( if (sscanf(obj->name, "nvml%d", &id) == 1) { device->smi = id; int nw = snprintf( - device->uuid, QVI_HWLOC_UUID_BUFF_SIZE, "%s", + device->uuid, uuid_buff_size, "%s", hwloc_obj_get_info_by_name(obj, "NVIDIAUUID") ); - if (nw >= QVI_HWLOC_UUID_BUFF_SIZE) { + if (nw >= uuid_buff_size) { return QV_ERR_INTERNAL; } return qvi_hwloc_nvml_get_device_cpuset_by_pci_bus_id( @@ -442,11 +446,11 @@ set_of_device_info( ) { // TODO(skg) Get cpuset, if available. int nw = snprintf( - device->uuid, QVI_HWLOC_UUID_BUFF_SIZE, "%s", + device->uuid, uuid_buff_size, "%s", hwloc_obj_get_info_by_name(obj, "NodeGUID") ); // Internal error because our buffer is too small. - if (nw >= QVI_HWLOC_UUID_BUFF_SIZE) return QV_ERR_INTERNAL; + if (nw >= uuid_buff_size) return QV_ERR_INTERNAL; return QV_SUCCESS; } @@ -467,7 +471,7 @@ discover_all_devices( continue; } // Try to get the PCI object. - char busid[QVI_HWLOC_PCI_BUS_ID_BUFF_SIZE] = {'\0'}; + char busid[pci_bus_id_buff_size] = {'\0'}; hwloc_obj_t pci_obj = get_pci_busid(obj, busid, sizeof(busid)); if (!pci_obj) continue; // Have we seen this device already? For example, opencl0d0 and cuda0 @@ -505,7 +509,7 @@ discover_gpu_devices( continue; } // Try to get the PCI object. - char busid[QVI_HWLOC_PCI_BUS_ID_BUFF_SIZE] = {'\0'}; + char busid[pci_bus_id_buff_size] = {'\0'}; hwloc_obj_t pci_obj = get_pci_busid(obj, busid, sizeof(busid)); if (!pci_obj) continue; @@ -569,7 +573,7 @@ discover_nic_devices( while ((obj = hwloc_get_next_osdev(hwl->topo, obj)) != nullptr) { if (obj->attr->osdev.type != HWLOC_OBJ_OSDEV_OPENFABRICS) continue; // Try to get the PCI object. - char busid[QVI_HWLOC_PCI_BUS_ID_BUFF_SIZE] = {'\0'}; + char busid[pci_bus_id_buff_size] = {'\0'}; hwloc_obj_t pci_obj = get_pci_busid(obj, busid, sizeof(busid)); if (!pci_obj) continue; diff --git a/src/qvi-hwloc.h b/src/qvi-hwloc.h index ca8b613a..ff50aa84 100644 --- a/src/qvi-hwloc.h +++ b/src/qvi-hwloc.h @@ -26,10 +26,6 @@ extern "C" { #endif -#define QVI_HWLOC_PCI_BUS_ID_BUFF_SIZE 16 -#define QVI_HWLOC_DEV_NAME_BUFF_SIZE 32 -#define QVI_HWLOC_UUID_BUFF_SIZE 64 - struct qvi_hwloc_s; typedef struct qvi_hwloc_s qvi_hwloc_t; @@ -418,9 +414,6 @@ qvi_hwloc_get_device_affinity( #ifdef __cplusplus } -#endif - -#ifdef __cplusplus /** * C++ style hwloc bitmap. diff --git a/src/qvi-hwpool.cc b/src/qvi-hwpool.cc index 484f2e26..c42bc668 100644 --- a/src/qvi-hwpool.cc +++ b/src/qvi-hwpool.cc @@ -251,8 +251,10 @@ qvi_hwpool_add_device( auto dinfo = std::make_shared( type, id, pcibid, uuid, affinity ); + const int rc = qvi_construct_rc(dinfo); + if (rc != QV_SUCCESS) return rc; rpool->devinfos.insert({type, dinfo}); - return QV_SUCCESS; + return rc; } int diff --git a/src/qvi-rmi.cc b/src/qvi-rmi.cc index 0dcf2d5b..bd3b9b3c 100644 --- a/src/qvi-rmi.cc +++ b/src/qvi-rmi.cc @@ -1,6 +1,6 @@ /* -*- Mode: C++; c-basic-offset:4; indent-tabs-mode:nil -*- */ /* - * Copyright (c) 2020-2023 Triad National Security, LLC + * Copyright (c) 2020-2024 Triad National Security, LLC * All rights reserved. * * Copyright (c) 2020-2021 Lawrence Livermore National Security, LLC @@ -33,7 +33,7 @@ #include "zmq.h" -#define ZINPROC_ADDR "inproc://qvi-rmi-workers" +static const cstr_t zinproc_addr = "inproc://qvi-rmi-workers"; struct qvi_rmi_server_s { /** Server configuration */ @@ -98,7 +98,7 @@ qvi_zerr_msg( cstr_t ers, int err_no ) { - int erno = (err_no); + const int erno = err_no; qvi_log_error("{} with errno={} ({})", ers, erno, qvi_strerr(erno)); } @@ -107,7 +107,7 @@ qvi_zwrn_msg( cstr_t ers, int err_no ) { - int erno = (err_no); + const int erno = err_no; qvi_log_warn("{} with errno={} ({})", (ers), erno, qvi_strerr(erno)); } @@ -731,7 +731,7 @@ server_go( qvi_rmi_server_t *server = (qvi_rmi_server_t *)data; void *zworksock = zsocket_create_and_connect( - server->zctx, ZMQ_REP, ZINPROC_ADDR + server->zctx, ZMQ_REP, zinproc_addr ); if (!zworksock) return nullptr; @@ -878,7 +878,7 @@ server_start_threads( } void *workers = zsocket_create_and_bind( - server->zctx, ZMQ_DEALER, ZINPROC_ADDR + server->zctx, ZMQ_DEALER, zinproc_addr ); if (!workers) { cstr_t ers = "zsocket_create_and_bind() failed"; diff --git a/src/qvi-scope.cc b/src/qvi-scope.cc index 7a25cb67..3db5b74e 100644 --- a/src/qvi-scope.cc +++ b/src/qvi-scope.cc @@ -45,7 +45,7 @@ struct qvi_global_split_t { * The root task ID used for collective operations. * Note: We use 0 as the root because 0 will always exist. */ - static const int rootid = 0; + static constexpr int rootid = 0; /** * Points to the parent scope that we are splitting. */ diff --git a/src/qvi-utils.cc b/src/qvi-utils.cc index c6233e46..8eeb6216 100644 --- a/src/qvi-utils.cc +++ b/src/qvi-utils.cc @@ -1,6 +1,6 @@ /* -*- Mode: C++; c-basic-offset:4; indent-tabs-mode:nil -*- */ /* - * Copyright (c) 2020-2022 Triad National Security, LLC + * Copyright (c) 2020-2024 Triad National Security, LLC * All rights reserved. * * Copyright (c) 2020-2021 Lawrence Livermore National Security, LLC @@ -147,10 +147,10 @@ qvi_url( const char * qvi_conn_ers(void) { - static cstr_t msg = "Cannot determine connection information. " - "Please make sure that the following environment " - "variable is set to an unused port number: " - QVI_ENV_PORT; + static const cstr_t msg = "Cannot determine connection information. " + "Please make sure that the following environment " + "variable is set to an unused port number: " + QVI_ENV_PORT; return msg; }