AI/rocm-systems
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

rdc_field_t replaces uint32_t; centralize field data

Browse Source 
Make the RDC use the new rdc_field_t enum instead of uint32_t.
This will help prevent invalid field types from being passed in.

Also, centralize where data related to fields is kept. This will
reduce the number of places where changes are required each
time a new field is added.

Finally, cleaned up several cpplint issues.

Change-Id: I48e4512e18c164411d8b09ae3d4bed99fba359ec
This commit is contained in:
Chris Freehill
2020-07-24 19:40:48 -05:00
parent e6d910f67a
commit 5950ebadc4
43 changed files with 459 additions and 388 deletions
Download Patch File Download Diff File Expand all files Collapse all files
rdc_libs/CMakeLists.txt
+6 -1
View File
@@ -63,6 +63,7 @@ endif()
set(SRC_DIR "${PROJECT_SOURCE_DIR}/rdc_libs")
set(RDC_LIB_INC_DIR "${PROJECT_SOURCE_DIR}/include")
set(COMMON_DIR "${PROJECT_SOURCE_DIR}/common")
################# Determine the library version #########################
@@ -112,12 +113,13 @@ set(BOOTSTRAP_LIB_COMPONENT "lib${BOOTSTRAP_LIB}")
set(BOOTSTRAP_LIB_SRC_LIST "${SRC_DIR}/bootstrap/src/RdcBootStrap.cc")
set(BOOTSTRAP_LIB_SRC_LIST ${BOOTSTRAP_LIB_SRC_LIST} "${SRC_DIR}/bootstrap/src/RdcLogger.cc")
set(BOOTSTRAP_LIB_SRC_LIST ${BOOTSTRAP_LIB_SRC_LIST} "${SRC_DIR}/bootstrap/src/RdcLibraryLoader.cc")
set(BOOTSTRAP_LIB_SRC_LIST ${BOOTSTRAP_LIB_SRC_LIST} "${COMMON_DIR}/rdc_fields_supported.cc")
set(BOOTSTRAP_LIB_INC_LIST "${RDC_LIB_INC_DIR}/rdc/rdc.h")
set(BOOTSTRAP_LIB_INC_LIST ${BOOTSTRAP_LIB_INC_LIST} "${RDC_LIB_INC_DIR}/rdc_lib/rdc_common.h")
set(BOOTSTRAP_LIB_INC_LIST ${BOOTSTRAP_LIB_INC_LIST} "${RDC_LIB_INC_DIR}/rdc_lib/RdcLogger.h")
set(BOOTSTRAP_LIB_INC_LIST ${BOOTSTRAP_LIB_INC_LIST} "${RDC_LIB_INC_DIR}/rdc_lib/RdcHandler.h")
set(BOOTSTRAP_LIB_INC_LIST ${BOOTSTRAP_LIB_INC_LIST} "${RDC_LIB_INC_DIR}/rdc_lib/RdcLibraryLoader.h")
set(BOOTSTRAP_LIB_INC_LIST ${BOOTSTRAP_LIB_INC_LIST} "${COMMON_DIR}/rdc_fields_supported.h")
message("BOOTSTRAP_LIB_INC_LIST=${BOOTSTRAP_LIB_INC_LIST}")
add_library(${BOOTSTRAP_LIB} SHARED ${BOOTSTRAP_LIB_SRC_LIST} ${BOOTSTRAP_LIB_INC_LIST})
@@ -125,6 +127,7 @@ target_link_libraries(${BOOTSTRAP_LIB} pthread dl)
target_include_directories(${BOOTSTRAP_LIB} PRIVATE
"${PROJECT_SOURCE_DIR}"
"${PROJECT_SOURCE_DIR}/include"
"${COMMON_DIR}"
"${CMAKE_CURRENT_SOURCE_DIR}/include")
# TODO: set the properties for the library once we have one
@@ -143,6 +146,7 @@ set(RDC_LIB_SRC_LIST ${RDC_LIB_SRC_LIST} "${SRC_DIR}/rdc/src/RdcGroupSettingsImp
set(RDC_LIB_SRC_LIST ${RDC_LIB_SRC_LIST} "${SRC_DIR}/rdc/src/RdcCacheManagerImpl.cc")
set(RDC_LIB_SRC_LIST ${RDC_LIB_SRC_LIST} "${SRC_DIR}/rdc/src/RdcMetricsUpdaterImpl.cc")
set(RDC_LIB_SRC_LIST ${RDC_LIB_SRC_LIST} "${SRC_DIR}/rdc/src/RdcWatchTableImpl.cc")
set(RDC_LIB_SRC_LIST ${RDC_LIB_SRC_LIST} "${COMMON_DIR}/rdc_fields_supported.cc")
set(RDC_LIB_INC_LIST ${RDC_LIB_INC_LIST} "${RDC_LIB_INC_DIR}/rdc_lib/impl/RdcEmbeddedHandler.h")
set(RDC_LIB_INC_LIST ${RDC_LIB_INC_LIST} "${RDC_LIB_INC_DIR}/rdc_lib/RdcMetricFetcher.h")
@@ -155,6 +159,7 @@ set(RDC_LIB_INC_LIST ${RDC_LIB_INC_LIST} "${RDC_LIB_INC_DIR}/rdc_lib/RdcMetricsU
set(RDC_LIB_INC_LIST ${RDC_LIB_INC_LIST} "${RDC_LIB_INC_DIR}/rdc_lib/impl/RdcMetricsUpdaterImpl.h")
set(RDC_LIB_INC_LIST ${RDC_LIB_INC_LIST} "${RDC_LIB_INC_DIR}/rdc_lib/RdcWatchTable.h")
set(RDC_LIB_INC_LIST ${RDC_LIB_INC_LIST} "${RDC_LIB_INC_DIR}/rdc_lib/impl/RdcWatchTableImpl.h")
set(RDC_LIB_INC_LIST ${RDC_LIB_INC_LIST} "${COMMON_DIR}/rdc_fields_supported.h")
message("RDC_LIB_INC_LIST=${RDC_LIB_INC_LIST}")
rdc_libs/bootstrap/src/RdcBootStrap.cc
+8 -27
View File
@@ -22,6 +22,7 @@ THE SOFTWARE.
#include <dlfcn.h>
#include <string.h>
#include <map>
#include "common/rdc_fields_supported.h"
#include "rdc/rdc.h"
#include "rdc_lib/RdcHandler.h"
#include "rdc_lib/RdcLogger.h"
@@ -204,7 +205,7 @@ rdc_status_t rdc_device_get_attributes(rdc_handle_t p_rdc_handle,
}
rdc_status_t rdc_group_field_create(rdc_handle_t p_rdc_handle,
uint32_t num_field_ids, uint32_t* field_ids,
uint32_t num_field_ids, rdc_field_t* field_ids,
const char* field_group_name, rdc_field_grp_t* rdc_field_group_id) {
if (!p_rdc_handle || !field_ids ||
!field_group_name || !rdc_field_group_id) {
@@ -270,7 +271,7 @@ rdc_status_t rdc_field_watch(rdc_handle_t p_rdc_handle,
}
rdc_status_t rdc_field_get_latest_value(rdc_handle_t p_rdc_handle,
uint32_t gpu_index, uint32_t field, rdc_field_value* value) {
uint32_t gpu_index, rdc_field_t field, rdc_field_value* value) {
if (!p_rdc_handle || !value) {
return RDC_ST_INVALID_HANDLER;
}
@@ -280,7 +281,7 @@ rdc_status_t rdc_field_get_latest_value(rdc_handle_t p_rdc_handle,
}
rdc_status_t rdc_field_get_value_since(rdc_handle_t p_rdc_handle,
uint32_t gpu_index, uint32_t field, uint64_t since_time_stamp,
uint32_t gpu_index, rdc_field_t field, uint64_t since_time_stamp,
uint64_t *next_since_time_stamp, rdc_field_value* value) {
if (!p_rdc_handle || !next_since_time_stamp || !value) {
return RDC_ST_INVALID_HANDLER;
@@ -350,30 +351,10 @@ const char* rdc_status_string(rdc_status_t result) {
}
}
const char* field_id_string(uint32_t field_id) {
const std::map<uint16_t, const char*> id_name = {
{RDC_FI_GPU_MEMORY_USAGE, "GPU_MEMORY_USAGE"},
{RDC_FI_GPU_MEMORY_TOTAL, "GPU_MEMORY_TOTAL"},
{RDC_FI_POWER_USAGE, "POWER_USAGE"},
{RDC_FI_GPU_CLOCK, "GPU_CLOCK"},
{RDC_FI_GPU_UTIL, "GPU_UTIL"},
{RDC_FI_GPU_TEMP, "GPU_TEMP"},
{RDC_FI_GPU_COUNT, "GPU_COUNT"},
{RDC_FI_MEM_CLOCK, "MEM_CLOCK"},
{RDC_FI_PCIE_TX, "PCIE_TX"},
{RDC_FI_PCIE_RX, "PCIE_RX"},
{RDC_FI_ECC_CORRECT_TOTAL, "ECC_CORRECT"},
{RDC_FI_ECC_UNCORRECT_TOTAL, "ECC_UNCORRECT"},
{RDC_FI_MEMORY_TEMP, "MEMORY_TEMP"},
{RDC_FI_DEV_NAME, "DEV_NAME"}
};
auto search = id_name.find(field_id);
if (search == id_name.end()) {
return "UNKNOWN_FIELD";
}
return search->second;
const char* field_id_string(rdc_field_t field_id) {
amd::rdc::fld_id2name_map_t &field_id_to_descript =
amd::rdc::get_field_id_description_from_id();
return field_id_to_descript.find(field_id)->second.label.c_str();
}
char *strncpy_with_null(char *dest, const char *src, size_t n) {
rdc_libs/rdc/src/RdcCacheManagerImpl.cc
+3 -3
View File
@@ -32,7 +32,7 @@ namespace amd {
namespace rdc {
rdc_status_t RdcCacheManagerImpl::rdc_field_get_value_since(
uint32_t gpu_index, uint32_t field_id, uint64_t since_time_stamp,
uint32_t gpu_index, rdc_field_t field_id, uint64_t since_time_stamp,
uint64_t *next_since_time_stamp, rdc_field_value* value) {
if (!next_since_time_stamp || !value) {
return RDC_ST_BAD_PARAMETER;
@@ -72,7 +72,7 @@ rdc_status_t RdcCacheManagerImpl::rdc_field_get_value_since(
rdc_status_t RdcCacheManagerImpl::evict_cache(uint32_t gpu_index,
uint32_t field_id, uint64_t max_keep_samples, double max_keep_age) {
rdc_field_t field_id, uint64_t max_keep_samples, double max_keep_age) {
std::lock_guard<std::mutex> guard(cache_mutex_);
RdcFieldKey field{gpu_index, field_id};
@@ -108,7 +108,7 @@ rdc_status_t RdcCacheManagerImpl::evict_cache(uint32_t gpu_index,
}
rdc_status_t RdcCacheManagerImpl::rdc_field_get_latest_value(
uint32_t gpu_index, uint32_t field_id, rdc_field_value* value) {
uint32_t gpu_index, rdc_field_t field_id, rdc_field_value* value) {
if (!value) {
return RDC_ST_BAD_PARAMETER;
}
rdc_libs/rdc/src/RdcEmbeddedHandler.cc
+7 -6
View File
@@ -29,6 +29,7 @@ THE SOFTWARE.
#include "rdc_lib/rdc_common.h"
#include "rdc_lib/RdcLogger.h"
#include "rdc_lib/RdcException.h"
#include "common/rdc_fields_supported.h"
#include "rocm_smi/rocm_smi.h"
namespace {
@@ -259,7 +260,7 @@ rdc_status_t RdcEmbeddedHandler::rdc_group_gpu_add(rdc_gpu_group_t group_id,
}
rdc_status_t RdcEmbeddedHandler::rdc_group_field_create(uint32_t num_field_ids,
uint32_t* field_ids, const char* field_group_name,
rdc_field_t* field_ids, const char* field_group_name,
rdc_field_grp_t* rdc_field_group_id) {
if (!field_group_name || !rdc_field_group_id || !field_ids) {
return RDC_ST_BAD_PARAMETER;
@@ -268,7 +269,7 @@ rdc_status_t RdcEmbeddedHandler::rdc_group_field_create(uint32_t num_field_ids,
// Check the field is valid or not
if (num_field_ids <= RDC_MAX_FIELD_IDS_PER_FIELD_GROUP) {
for (uint32_t i = 0; i < num_field_ids; i++) {
if (!metric_fetcher_->is_field_valid(field_ids[i])) {
if (!is_field_valid(field_ids[i])) {
RDC_LOG(RDC_INFO,
"Fail to create field group with unknown field id "
<< field_ids[i]);
@@ -341,11 +342,11 @@ rdc_status_t RdcEmbeddedHandler::rdc_field_watch(rdc_gpu_group_t group_id,
}
rdc_status_t RdcEmbeddedHandler::rdc_field_get_latest_value(
uint32_t gpu_index, uint32_t field, rdc_field_value* value) {
uint32_t gpu_index, rdc_field_t field, rdc_field_value* value) {
if (!value) {
return RDC_ST_BAD_PARAMETER;
}
if (!metric_fetcher_->is_field_valid(field)) {
if (!is_field_valid(field)) {
RDC_LOG(RDC_INFO,
"Fail to get latest value with unknown field id "
<< field);
@@ -355,12 +356,12 @@ rdc_status_t RdcEmbeddedHandler::rdc_field_get_latest_value(
}
rdc_status_t RdcEmbeddedHandler::rdc_field_get_value_since(uint32_t gpu_index,
uint32_t field, uint64_t since_time_stamp,
rdc_field_t field, uint64_t since_time_stamp,
uint64_t *next_since_time_stamp, rdc_field_value* value) {
if (!next_since_time_stamp || !value) {
return RDC_ST_BAD_PARAMETER;
}
if (!metric_fetcher_->is_field_valid(field)) {
if (!is_field_valid(field)) {
RDC_LOG(RDC_INFO,
"Fail to get value since with unknown field id "
<< field);
rdc_libs/rdc/src/RdcGroupSettingsImpl.cc
+3 -3
View File
@@ -29,7 +29,7 @@ namespace rdc {
RdcGroupSettingsImpl::RdcGroupSettingsImpl() {
// Add the default job stats fields
uint32_t job_fields[] = {RDC_FI_GPU_MEMORY_USAGE,
rdc_field_t job_fields[] = {RDC_FI_GPU_MEMORY_USAGE,
RDC_FI_POWER_USAGE, RDC_FI_GPU_CLOCK, RDC_FI_GPU_UTIL,
RDC_FI_PCIE_TX, RDC_FI_PCIE_RX, RDC_FI_MEM_CLOCK,
RDC_FI_GPU_TEMP};
@@ -37,7 +37,7 @@ RdcGroupSettingsImpl::RdcGroupSettingsImpl() {
rdc_field_grp_t fgid = JOB_FIELD_ID;
rdc_group_field_create(sizeof(job_fields)/sizeof(uint32_t),
job_fields, job_field_group, &fgid);
job_fields, job_field_group, &fgid);
}
rdc_status_t RdcGroupSettingsImpl::rdc_group_gpu_create(
@@ -133,7 +133,7 @@ rdc_status_t RdcGroupSettingsImpl::rdc_group_get_all_ids(
}
rdc_status_t RdcGroupSettingsImpl::rdc_group_field_create(
uint32_t num_field_ids, uint32_t* field_ids,
uint32_t num_field_ids, rdc_field_t* field_ids,
const char* field_group_name, rdc_field_grp_t* rdc_field_group_id) {
RDC_LOG(RDC_DEBUG, "Create field group " << field_group_name);
rdc_libs/rdc/src/RdcMetricFetcherImpl.cc
+4 -14
View File
@@ -26,23 +26,13 @@ THE SOFTWARE.
#include <algorithm>
#include <vector>
#include "rdc_lib/rdc_common.h"
#include "common/rdc_fields_supported.h"
#include "rdc_lib/RdcLogger.h"
#include "rocm_smi/rocm_smi.h"
namespace amd {
namespace rdc {
bool RdcMetricFetcherImpl::is_field_valid(uint32_t field_id) const {
const std::vector<uint32_t> all_fields = {RDC_FI_GPU_MEMORY_USAGE,
RDC_FI_GPU_MEMORY_TOTAL, RDC_FI_GPU_COUNT, RDC_FI_POWER_USAGE,
RDC_FI_GPU_CLOCK, RDC_FI_GPU_UTIL, RDC_FI_DEV_NAME, RDC_FI_GPU_TEMP,
RDC_FI_MEM_CLOCK, RDC_FI_PCIE_TX, RDC_FI_PCIE_RX,
RDC_FI_ECC_CORRECT_TOTAL, RDC_FI_ECC_UNCORRECT_TOTAL, RDC_FI_MEMORY_TEMP};
return std::find(all_fields.begin(), all_fields.end(), field_id)
!= all_fields.end();
}
RdcMetricFetcherImpl::RdcMetricFetcherImpl() {
task_started_ = true;
@@ -81,7 +71,7 @@ uint64_t RdcMetricFetcherImpl::now() {
}
void RdcMetricFetcherImpl::get_ecc_error(uint32_t gpu_index,
uint32_t field_id, rdc_field_value* value) {
rdc_field_t field_id, rdc_field_value* value) {
rsmi_status_t err = RSMI_STATUS_SUCCESS;
uint64_t correctable_err = 0;
uint64_t uncorrectable_err = 0;
@@ -121,7 +111,7 @@ void RdcMetricFetcherImpl::get_ecc_error(uint32_t gpu_index,
}
bool RdcMetricFetcherImpl::async_get_pcie_throughput(uint32_t gpu_index,
uint32_t field_id, rdc_field_value* value) {
rdc_field_t field_id, rdc_field_value* value) {
if (!value) {
return false;
}
@@ -216,7 +206,7 @@ void RdcMetricFetcherImpl::get_pcie_throughput(const RdcFieldKey& key) {
}
rdc_status_t RdcMetricFetcherImpl::fetch_smi_field(uint32_t gpu_index,
uint32_t field_id, rdc_field_value* value) {
rdc_field_t field_id, rdc_field_value* value) {
if (!value) {
return RDC_ST_BAD_PARAMETER;
}
rdc_libs/rdc/src/RdcMetricsUpdaterImpl.cc
+1 -1
View File
@@ -22,7 +22,7 @@ THE SOFTWARE.
#include "rdc_lib/impl/RdcMetricsUpdaterImpl.h"
#include <sys/time.h>
#include <ctime>
#include <chrono>
#include <chrono> // NOLINT(build/c++11)
#include "rdc_lib/rdc_common.h"
namespace amd {
rdc_libs/rdc/src/RdcWatchTableImpl.cc
+4 -4
View File
@@ -176,7 +176,7 @@ rdc_status_t RdcWatchTableImpl::rdc_field_watch(rdc_gpu_group_t group_id,
rdc_field_grp_t field_group_id, uint64_t update_freq,
double max_keep_age, uint32_t max_keep_samples) {
std::lock_guard<std::mutex> guard(watch_mutex_);
RdcFieldKey gkey({group_id, field_group_id});
RdcFieldGroupKey gkey({group_id, field_group_id});
auto table_iter = watch_table_.find(gkey);
// Already in the watch table
@@ -234,7 +234,7 @@ rdc_status_t RdcWatchTableImpl::rdc_field_watch(rdc_gpu_group_t group_id,
}
rdc_status_t RdcWatchTableImpl::update_field_in_table_when_unwatch(
const RdcFieldKey& entry) {
const RdcFieldGroupKey& entry) {
// Get individual fields for this unwatch
std::vector<RdcFieldKey> fields;
rdc_status_t result = get_fields_from_group(
@@ -306,7 +306,7 @@ rdc_status_t RdcWatchTableImpl::rdc_field_unwatch(
std::lock_guard<std::mutex> guard(watch_mutex_);
// Set is_watching = false
auto ite = watch_table_.find(RdcFieldKey({group_id, field_group_id}));
auto ite = watch_table_.find(RdcFieldGroupKey({group_id, field_group_id}));
if (ite == watch_table_.end()) {
return RDC_ST_NOT_FOUND;
}
@@ -318,7 +318,7 @@ rdc_status_t RdcWatchTableImpl::rdc_field_unwatch(
}
bool RdcWatchTableImpl::is_job_watch_field(uint32_t gpu_index,
uint32_t field_id, std::string& job_id) const {
rdc_field_t field_id, std::string& job_id) const {
RdcFieldKey key{gpu_index, field_id};
for (auto ite = job_watch_table_.begin();
rdc_libs/rdc_client/src/RdcStandaloneHandler.cc
+7 -6
View File
@@ -290,7 +290,7 @@ rdc_status_t RdcStandaloneHandler::rdc_group_gpu_add(rdc_gpu_group_t group_id,
}
rdc_status_t RdcStandaloneHandler::rdc_group_field_create(
uint32_t num_field_ids, uint32_t* field_ids,
uint32_t num_field_ids, rdc_field_t* field_ids,
const char* field_group_name, rdc_field_grp_t* rdc_field_group_id) {
if (!field_ids || !field_group_name || !rdc_field_group_id) {
return RDC_ST_BAD_PARAMETER;
@@ -339,7 +339,8 @@ rdc_status_t RdcStandaloneHandler::rdc_group_field_get_info(
strncpy_with_null(field_group_info->group_name,
reply.filed_group_name().c_str(), RDC_MAX_STR_LENGTH);
for (int i = 0; i < reply.field_ids_size(); i++) {
field_group_info->field_ids[i] = reply.field_ids(i);
field_group_info->field_ids[i] =
static_cast<rdc_field_t>(reply.field_ids(i));
}
return RDC_ST_OK;
@@ -471,7 +472,7 @@ rdc_status_t RdcStandaloneHandler::rdc_field_watch(rdc_gpu_group_t group_id,
}
rdc_status_t RdcStandaloneHandler::rdc_field_get_latest_value(
uint32_t gpu_index, uint32_t field, rdc_field_value* value) {
uint32_t gpu_index, rdc_field_t field, rdc_field_value* value) {
if (!value) {
return RDC_ST_BAD_PARAMETER;
}
@@ -487,7 +488,7 @@ rdc_status_t RdcStandaloneHandler::rdc_field_get_latest_value(
rdc_status_t err_status = error_handle(status, reply.status());
if (err_status != RDC_ST_OK) return err_status;
value->field_id = reply.field_id();
value->field_id = static_cast<rdc_field_t>(reply.field_id());
value->status = reply.rdc_status();
value->ts = reply.ts();
value->type = static_cast<rdc_field_type_t>(reply.type());
@@ -504,7 +505,7 @@ rdc_status_t RdcStandaloneHandler::rdc_field_get_latest_value(
}
rdc_status_t RdcStandaloneHandler::rdc_field_get_value_since(uint32_t gpu_index,
uint32_t field, uint64_t since_time_stamp,
rdc_field_t field, uint64_t since_time_stamp,
uint64_t *next_since_time_stamp, rdc_field_value* value) {
if (!next_since_time_stamp || !value) {
return RDC_ST_BAD_PARAMETER;
@@ -522,7 +523,7 @@ rdc_status_t RdcStandaloneHandler::rdc_field_get_value_since(uint32_t gpu_index,
rdc_status_t err_status = error_handle(status, reply.status());
if (err_status != RDC_ST_OK) return err_status;
value->field_id = reply.field_id();
value->field_id = static_cast<rdc_field_t>(reply.field_id());
value->status = reply.rdc_status();
value->ts = reply.ts();
value->type = static_cast<rdc_field_type_t>(reply.type());