From d9381b6dae0090ae2724c9396ea68669385600aa Mon Sep 17 00:00:00 2001 From: "Galantsev, Dmitrii" Date: Wed, 13 Sep 2023 19:49:46 -0500 Subject: [PATCH 01/11] Fix misspelling averge -> average Change-Id: I3546348560acadb1e775e10ad24115de4ccfc800 Signed-off-by: Galantsev, Dmitrii --- rocm_smi/example/rocm_smi_example.cc | 2 +- tests/rocm_smi_test/functional/power_read.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rocm_smi/example/rocm_smi_example.cc b/rocm_smi/example/rocm_smi_example.cc index 0e78debb91..815b00382e 100755 --- a/rocm_smi/example/rocm_smi_example.cc +++ b/rocm_smi/example/rocm_smi_example.cc @@ -839,7 +839,7 @@ int main() { } CHK_RSMI_NOT_SUPPORTED_RET(ret) - std::cout << "\t**Averge Power Usage: "; + std::cout << "\t**Average Power Usage: "; ret = rsmi_dev_power_ave_get(i, 0, &val_ui64); if (ret == RSMI_STATUS_SUCCESS) { std::cout << static_cast(val_ui64)/1000 << " W" << std::endl; diff --git a/tests/rocm_smi_test/functional/power_read.cc b/tests/rocm_smi_test/functional/power_read.cc index 70f3b104d0..02ec355b46 100755 --- a/tests/rocm_smi_test/functional/power_read.cc +++ b/tests/rocm_smi_test/functional/power_read.cc @@ -118,7 +118,7 @@ void TestPowerRead::Run(void) { err = rsmi_dev_power_ave_get(i, 0, &val_ui64); IF_VERB(STANDARD) { - std::cout << "\t**Averge Power Usage: "; + std::cout << "\t**Average Power Usage: "; CHK_RSMI_PERM_ERR(err) if (err == RSMI_STATUS_SUCCESS) { std::cout << static_cast(val_ui64)/1000 << " mW" << std::endl; From 12f395e592b781feb0df21ed4130bc6bbb28708a Mon Sep 17 00:00:00 2001 From: "Oliveira, Daniel" Date: Tue, 12 Sep 2023 16:34:04 -0500 Subject: [PATCH 02/11] rocm_smi_lib: Fix rocm-smi --resetfans results in Permission Denied For operations related to: --resetfans --setfan We report 'Not supported' for these cases instead of 'Permission denied' Code changes related to the following: * rocm_smi_properties * rocm_smi related APIs Change-Id: I144646efc3804fabd45cc5a46351803950b4feb7 Signed-off-by: Oliveira, Daniel --- python_smi_tools/rocm_smi.py | 8 ++++++-- src/rocm_smi.cc | 9 +++++---- src/rocm_smi_properties.cc | 35 +++++++++++++++++++++++++---------- src/rocm_smi_utils.cc | 7 +++++++ 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/python_smi_tools/rocm_smi.py b/python_smi_tools/rocm_smi.py index 4a943a66a4..2d96cf3c73 100755 --- a/python_smi_tools/rocm_smi.py +++ b/python_smi_tools/rocm_smi.py @@ -813,8 +813,10 @@ def resetFans(deviceList): for device in deviceList: sensor_ind = c_uint32(0) ret = rocmsmi.rsmi_dev_fan_reset(device, sensor_ind) - if rsmi_ret_ok(ret, device, 'reset_fan'): + if rsmi_ret_ok(ret, device, silent=True): printLog(device, 'Successfully reset fan speed to driver control', None) + else: + printLog(device, 'Not supported on the given system', None) printLogSpacer() @@ -1335,8 +1337,10 @@ def setFanSpeed(deviceList, fan): else: fanLevel = int(str(fan)) ret = rocmsmi.rsmi_dev_fan_speed_set(device, 0, int(fanLevel)) - if rsmi_ret_ok(ret, device, 'set_fan_speed'): + if rsmi_ret_ok(ret, device, silent=True): printLog(device, 'Successfully set fan speed to level %s' % (str(int(fanLevel))), None) + else: + printLog(device, 'Not supported on the given system', None) printLogSpacer() diff --git a/src/rocm_smi.cc b/src/rocm_smi.cc index d8bd892ac8..892f6664c7 100755 --- a/src/rocm_smi.cc +++ b/src/rocm_smi.cc @@ -407,6 +407,10 @@ static rsmi_status_t set_dev_mon_value(amd::smi::MonitorTypes type, } int ret = dev->monitor()->writeMonitor(type, sensor_ind, std::to_string(val)); + /// If the sysfs file doesn't exist, it is not supported. + if (ret == ENOENT) { + return rsmi_status_t::RSMI_STATUS_NOT_SUPPORTED; + } return amd::smi::ErrnoToRsmiStatus(ret); } @@ -2631,9 +2635,8 @@ rsmi_dev_fan_reset(uint32_t dv_ind, uint32_t sensor_ind) { LOG_TRACE(ss); ++sensor_ind; // fan sysfs files have 1-based indices - + REQUIRE_ROOT_ACCESS DEVICE_MUTEX - ret = set_dev_mon_value(amd::smi::kMonFanCntrlEnable, dv_ind, sensor_ind, 2); return ret; @@ -2669,14 +2672,12 @@ rsmi_dev_fan_speed_set(uint32_t dv_ind, uint32_t sensor_ind, uint64_t speed) { // First need to set fan mode (pwm1_enable) to 1 (aka, "manual") ret = set_dev_mon_value(amd::smi::kMonFanCntrlEnable, dv_ind, sensor_ind, 1); - if (ret != RSMI_STATUS_SUCCESS) { return ret; } ret = set_dev_mon_value(amd::smi::kMonFanSpeed, dv_ind, sensor_ind, speed); - return ret; CATCH diff --git a/src/rocm_smi_properties.cc b/src/rocm_smi_properties.cc index b076991881..d73f974286 100644 --- a/src/rocm_smi_properties.cc +++ b/src/rocm_smi_properties.cc @@ -166,6 +166,7 @@ const AMDGpuVerbList_t amdgpu_verb_check_list { { AMDGpuVerbTypes_t::kGetGpuOdVoltCurveRegions, "amdsmi_get_gpu_od_volt_curve_regions" } }; +const uint16_t kDevIDAll(0xFFFF); const uint16_t kDevRevIDAll(0xFFFF); const AMDGpuPropertyList_t amdgpu_property_reinforcement_list { // @@ -176,6 +177,14 @@ const AMDGpuPropertyList_t amdgpu_property_reinforcement_list { // rsmi_dev_perf_level::RSMI_DEV_PERF_LEVEL_MANUAL = rsmi_dev_clk_range_set; // + // AMD All Families + {kDevIDAll, {kDevRevIDAll, + make_unique_property_id(AMDGpuPropertyTypesOffset_t::kMonitorTypes, + MonitorTypes::kMonFanCntrlEnable), + AMDGpuVerbTypes_t::kResetGpuFan, + AMDGpuPropertyOpModeTypes_t::kBoth, false } + }, + // AMD Instinct MI210 {0x740F, {0x02, make_unique_property_id(AMDGpuPropertyTypesOffset_t::kDevInfoTypes, @@ -239,12 +248,6 @@ const AMDGpuPropertyList_t amdgpu_property_reinforcement_list { AMDGpuVerbTypes_t::kGetGpuPowerProfilePresets, AMDGpuPropertyOpModeTypes_t::kBoth, false } }, - {0x74A1, {kDevRevIDAll, - make_unique_property_id(AMDGpuPropertyTypesOffset_t::kDevInfoTypes, - DevInfoTypes::kDevGpuReset), - AMDGpuVerbTypes_t::kResetGpu, - AMDGpuPropertyOpModeTypes_t::kSrIov, false } - }, {0x74A1, {kDevRevIDAll, make_unique_property_id(AMDGpuPropertyTypesOffset_t::kPerfTypes, rsmi_dev_perf_level::RSMI_DEV_PERF_LEVEL_DETERMINISM), @@ -350,7 +353,7 @@ rsmi_status_t validate_property_reinforcement_query(uint32_t dv_ind, AMDGpuVerbT // likely the reinforcement table does not contain any entries/rules for the // dev_id in question. // - auto amdgpu_property_query_result_hdlr = [](rsmi_status_t query_result) { + auto amdgpu_property_query_result_hdlr = [&](const rsmi_status_t query_result) { switch (query_result) { case (rsmi_status_t::RSMI_STATUS_UNKNOWN_ERROR): case (rsmi_status_t::RSMI_STATUS_NO_DATA): @@ -363,7 +366,7 @@ rsmi_status_t validate_property_reinforcement_query(uint32_t dv_ind, AMDGpuVerbT break; default: - return rsmi_status_t::RSMI_STATUS_NOT_FOUND; + return actual_error_code; break; } }; @@ -415,7 +418,7 @@ rsmi_status_t Device::check_amdgpu_property_reinforcement_query(uint32_t dev_idx std::ostringstream osstream; auto rsmi_status(rsmi_status_t::RSMI_STATUS_UNKNOWN_ERROR); - AMDGpuPropertyQuery_t amdgpu_property_query = [&]() { + auto amdgpu_property_query = [&]() { AMDGpuPropertyQuery_t amdgpu_property_query_init{}; amdgpu_property_query_init.m_asic_id = 0; amdgpu_property_query_init.m_pci_rev_id = 0; @@ -445,6 +448,18 @@ rsmi_status_t Device::check_amdgpu_property_reinforcement_query(uint32_t dev_idx LOG_TRACE(osstream); bool is_proper_query(false); + + // Generic filter for checking properties for all asics and revisions. + auto amdgpu_property_query_all_asics = amdgpu_property_query; + amdgpu_property_query_all_asics.m_asic_id = kDevIDAll; + amdgpu_property_query_all_asics.m_pci_rev_id = kDevRevIDAll; + auto amdgpu_property_query_result = run_amdgpu_property_reinforcement_query(amdgpu_property_query_all_asics); + // We found a generic entry for all asics and revisions + if (amdgpu_property_query_result != rsmi_status_t::RSMI_STATUS_UNKNOWN_ERROR) { + return amdgpu_property_query_result; + } + + // If no generic entry, then we query for specific asic and revision ids. amdgpu_property_query = build_asic_id_filters(amdgpu_property_query, is_proper_query); if (!is_proper_query) { rsmi_status = rsmi_status_t::RSMI_STATUS_NO_DATA; @@ -487,7 +502,7 @@ rsmi_status_t Device::run_amdgpu_property_reinforcement_query(const AMDGpuProper osstream << __PRETTY_FUNCTION__ << " asic id found: " << itr_begin->first << "\n"; // Pci_rev_id matches the filter or ALL Revisions if ((itr_begin->second.m_pci_rev_id == amdgpu_property_query.m_pci_rev_id) || - (itr_begin->second.m_pci_rev_id == kDevRevIDAll)) { + (itr_begin->second.m_pci_rev_id == kDevRevIDAll)) { osstream << __PRETTY_FUNCTION__ << " asic rev.id found: " << itr_begin->second.m_pci_rev_id << "\n"; // Do we have the property we are looking for? if (((amdgpu_property_query.m_property != 0) && diff --git a/src/rocm_smi_utils.cc b/src/rocm_smi_utils.cc index 1e9a444320..4b8f61a842 100755 --- a/src/rocm_smi_utils.cc +++ b/src/rocm_smi_utils.cc @@ -176,6 +176,13 @@ int isRegularFile(std::string fname, bool *is_reg) { } int WriteSysfsStr(std::string path, std::string val) { + // On success, zero is returned. On error, -1 is returned, and + // errno is set to indicate the error. + auto is_regular_file_result = isRegularFile(path, nullptr); + if (is_regular_file_result != 0) { + return ENOENT; + } + std::ofstream fs; int ret = 0; std::ostringstream ss; From a4b470fe71f723fe2c3b90480922820ae8102558 Mon Sep 17 00:00:00 2001 From: "Galantsev, Dmitrii" Date: Tue, 29 Aug 2023 19:33:10 -0500 Subject: [PATCH 03/11] Add errors for existing but empty dev files Change-Id: Iad9febc50f9b8e6085f8b605249ee884d2f134d6 Signed-off-by: Galantsev, Dmitrii --- python_smi_tools/rocm_smi.py | 22 ++++--- src/rocm_smi.cc | 4 ++ src/rocm_smi_device.cc | 5 +- .../functional/frequencies_read.cc | 26 +++++---- .../functional/frequencies_read_write.cc | 58 +++++-------------- 5 files changed, 49 insertions(+), 66 deletions(-) diff --git a/python_smi_tools/rocm_smi.py b/python_smi_tools/rocm_smi.py index 2d96cf3c73..8834e518b5 100755 --- a/python_smi_tools/rocm_smi.py +++ b/python_smi_tools/rocm_smi.py @@ -1791,15 +1791,19 @@ def showClocks(deviceList): for clk_type in sorted(rsmi_clk_names_dict): if rocmsmi.rsmi_dev_gpu_clk_freq_get(device, rsmi_clk_names_dict[clk_type], None) == 1: ret = rocmsmi.rsmi_dev_gpu_clk_freq_get(device, rsmi_clk_names_dict[clk_type], byref(freq)) - if rsmi_ret_ok(ret, device, 'get_clk_freq_' + clk_type, True): - printLog(device, 'Supported %s frequencies on GPU%s' % (clk_type, str(device)), None) - for x in range(freq.num_supported): - fr = '{:>.0f}Mhz'.format(freq.frequency[x] / 1000000) - if x == freq.current: - printLog(device, str(x), str(fr) + ' *') - else: - printLog(device, str(x), str(fr)) - printLog(device, '', None) + if ret == rsmi_status_t.RSMI_STATUS_UNEXPECTED_DATA: + printLog(device, 'Clock [%s] on device [%s] exists but EMPTY! Likely driver error!' % (clk_type, str(device))) + continue + if not rsmi_ret_ok(ret, device, 'get_clk_freq_' + clk_type, True): + continue + printLog(device, 'Supported %s frequencies on GPU%s' % (clk_type, str(device)), None) + for x in range(freq.num_supported): + fr = '{:>.0f}Mhz'.format(freq.frequency[x] / 1000000) + if x == freq.current: + printLog(device, str(x), str(fr) + ' *') + else: + printLog(device, str(x), str(fr)) + printLog(device, '', None) else: logging.debug('{} frequency is unsupported on device[{}]'.format(clk_type, device)) printLog(device, '', None) diff --git a/src/rocm_smi.cc b/src/rocm_smi.cc index 892f6664c7..ed96a7ee65 100755 --- a/src/rocm_smi.cc +++ b/src/rocm_smi.cc @@ -3740,6 +3740,10 @@ rsmi_dev_memory_reserved_pages_get(uint32_t dv_ind, uint32_t *num_pages, ret = GetDevValueVec(amd::smi::kDevMemPageBad, dv_ind, &val_vec); + // file is empty, which is valid for no errors + if (ret == RSMI_STATUS_UNEXPECTED_DATA) { + ret = RSMI_STATUS_SUCCESS; + } if (ret == RSMI_STATUS_FILE_ERROR) { return RSMI_STATUS_NOT_SUPPORTED; } diff --git a/src/rocm_smi_device.cc b/src/rocm_smi_device.cc index b01734f0b1..03a38dc749 100755 --- a/src/rocm_smi_device.cc +++ b/src/rocm_smi_device.cc @@ -858,8 +858,8 @@ int Device::readDevInfoMultiLineStr(DevInfoTypes type, ss << "Read devInfoMultiLineStr for DevInfoType (" << RocmSMI::devInfoTypesStrings.at(type) << ")" << ", but contained no string lines"; - LOG_INFO(ss); - return 0; + LOG_ERROR(ss); + return ENXIO; } // Remove any *trailing* empty (whitespace) lines while (!retVec->empty() && @@ -882,6 +882,7 @@ int Device::readDevInfoMultiLineStr(DevInfoTypes type, << RocmSMI::devInfoTypesStrings.at(type) << ")" << ", but lines were empty"; LOG_INFO(ss); + return ENXIO; } return 0; } diff --git a/tests/rocm_smi_test/functional/frequencies_read.cc b/tests/rocm_smi_test/functional/frequencies_read.cc index 2b5466ae93..37bb9ec0b2 100755 --- a/tests/rocm_smi_test/functional/frequencies_read.cc +++ b/tests/rocm_smi_test/functional/frequencies_read.cc @@ -123,16 +123,22 @@ void TestFrequenciesRead::Run(void) { // Verify api support checking functionality is working err = rsmi_dev_gpu_clk_freq_get(i, t, nullptr); ASSERT_EQ(err, RSMI_STATUS_NOT_SUPPORTED); - } else { - CHK_ERR_ASRT(err) - IF_VERB(STANDARD) { - std::cout << "\t**Supported " << name << " clock frequencies: "; - std::cout << f.num_supported << std::endl; - print_frequencies(&f); - // Verify api support checking functionality is working - err = rsmi_dev_gpu_clk_freq_get(i, t, nullptr); - ASSERT_EQ(err, RSMI_STATUS_INVALID_ARGS); - } + } + + // special driver issue, shouldn't normally occur + if (err == RSMI_STATUS_UNEXPECTED_DATA) { + std::cerr << "WARN: Clock file [" << FreqEnumToStr(t) << "] exists on device [" << i << "] but empty!" << std::endl; + std::cerr << " Likely a driver issue!" << std::endl; + } + + CHK_ERR_ASRT(err) + IF_VERB(STANDARD) { + std::cout << "\t**Supported " << name << " clock frequencies: "; + std::cout << f.num_supported << std::endl; + print_frequencies(&f); + // Verify api support checking functionality is working + err = rsmi_dev_gpu_clk_freq_get(i, t, nullptr); + ASSERT_EQ(err, RSMI_STATUS_INVALID_ARGS); } }; diff --git a/tests/rocm_smi_test/functional/frequencies_read_write.cc b/tests/rocm_smi_test/functional/frequencies_read_write.cc index 9fce5c429e..5ad627cb5f 100755 --- a/tests/rocm_smi_test/functional/frequencies_read_write.cc +++ b/tests/rocm_smi_test/functional/frequencies_read_write.cc @@ -114,14 +114,20 @@ void TestFrequenciesReadWrite::Run(void) { std::cout << "\t**Set " << FreqEnumToStr(rsmi_clk) << ": Not supported on this machine" << std::endl; return false; - } else { - // CHK_ERR_ASRT(ret) - IF_VERB(STANDARD) { - std::cout << "Initial frequency for clock " << - FreqEnumToStr(rsmi_clk) << " is " << f.current << std::endl; - } - return true; } + + // special driver issue, shouldn't normally occur + if (ret == RSMI_STATUS_UNEXPECTED_DATA) { + std::cerr << "WARN: Clock file [" << FreqEnumToStr(rsmi_clk) << "] exists on device [" << dv_ind << "] but empty!" << std::endl; + std::cerr << " Likely a driver issue!" << std::endl; + } + + // CHK_ERR_ASRT(ret) + IF_VERB(STANDARD) { + std::cout << "Initial frequency for clock " << + FreqEnumToStr(rsmi_clk) << " is " << f.current << std::endl; + } + return true; }; auto freq_write = [&]() { @@ -177,44 +183,6 @@ void TestFrequenciesReadWrite::Run(void) { } freq_write(); CHK_ERR_ASRT(ret) -#if 0 - ret = rsmi_dev_gpu_clk_freq_get(dv_ind, rsmi_clk, &f); - CHK_ERR_ASRT(ret) - - IF_VERB(STANDARD) { - std::cout << "Initial frequency for clock " << rsmi_clk << " is " << - f.current << std::endl; - } - // Set clocks to something other than the usual default of the lowest - // frequency. - freq_bitmask = 0b01100; // Try the 3rd and 4th clocks - - std::string freq_bm_str = - std::bitset(freq_bitmask).to_string(); - - freq_bm_str.erase(0, std::min(freq_bm_str.find_first_not_of('0'), - freq_bm_str.size()-1)); - - IF_VERB(STANDARD) { - std::cout << "Setting frequency mask for clock " << rsmi_clk << - " to 0b" << freq_bm_str << " ..." << std::endl; - } - ret = rsmi_dev_gpu_clk_freq_set(dv_ind, rsmi_clk, freq_bitmask); - CHK_ERR_ASRT(ret) - - ret = rsmi_dev_gpu_clk_freq_get(dv_ind, rsmi_clk, &f); - CHK_ERR_ASRT(ret) - - IF_VERB(STANDARD) { - std::cout << "Frequency is now index " << f.current << std::endl; - std::cout << "Resetting mask to all frequencies." << std::endl; - } - ret = rsmi_dev_gpu_clk_freq_set(dv_ind, rsmi_clk, 0xFFFFFFFF); - CHK_ERR_ASRT(ret) - - ret = rsmi_dev_perf_level_set(dv_ind, RSMI_DEV_PERF_LEVEL_AUTO); - CHK_ERR_ASRT(ret) -#endif } } } From 26c4578ee2d8caefb9209836320cdec25c9c9e9e Mon Sep 17 00:00:00 2001 From: "Galantsev, Dmitrii" Date: Wed, 13 Sep 2023 19:16:49 -0500 Subject: [PATCH 04/11] README - Add a documentation link Change-Id: Ia56994825e99e72829283f07bed7379d95d24498 Signed-off-by: Galantsev, Dmitrii --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 948ef14988..9d7b0a019d 100755 --- a/README.md +++ b/README.md @@ -4,6 +4,8 @@ The ROCm System Management Interface Library, or ROCm SMI library, is part of the Radeon Open Compute [ROCm](https://github.com/RadeonOpenCompute) software stack . It is a C library for Linux that provides a user space interface for applications to monitor and control GPU applications. +For additional information refer to [ROCm Documentation](https://rocm.docs.amd.com/projects/rocm_smi_lib/en/latest/) + ## DISCLAIMER The information contained herein is for informational purposes only, and is subject to change without notice. In addition, any stated support is planned and is also subject to change. While every precaution has been taken in the preparation of this document, it may contain technical inaccuracies, omissions and typographical errors, and AMD is under no obligation to update or otherwise correct this information. Advanced Micro Devices, Inc. makes no representations or warranties with respect to the accuracy or completeness of the contents of this document, and assumes no liability of any kind, including the implied warranties of noninfringement, merchantability or fitness for particular purposes, with respect to the operation or use of AMD hardware, software or other products described herein. @@ -56,7 +58,6 @@ python3 -m venv .venv In order to verify the build and capability of ROCm SMI on your system and to see an example of how ROCm SMI can be used, you may build and run the tests that are available in the repo. To build the tests, follow these steps: ```shell -# Set environment variables used in CMakeLists.txt file mkdir build cd build cmake -DBUILD_TESTS=ON .. From 238c7f6dcab244081ab5e47dea0fedef8da5e8af Mon Sep 17 00:00:00 2001 From: "Galantsev, Dmitrii" Date: Wed, 13 Sep 2023 19:33:21 -0500 Subject: [PATCH 05/11] README - shell -> bash Change-Id: I3a50c38ae280747b4874cff443091f332980fe50 Signed-off-by: Galantsev, Dmitrii --- README.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 9d7b0a019d..12a12bd63c 100755 --- a/README.md +++ b/README.md @@ -27,24 +27,24 @@ In order to build the latest documentation, the following are required: The source code for ROCm SMI is available on [Github](https://github.com/RadeonOpenCompute/rocm_smi_lib). After the ROCm SMI library git repository has been cloned to a local Linux machine, building the library is achieved by following the typical CMake build sequence. Specifically, -```shell +```bash mkdir -p build cd build cmake .. make -j $(nproc) # Install library file and header; default location is /opt/rocm -$ make install +make install ``` The built library will appear in the `build` folder. To build the rpm and deb packages follow the above steps with: -```shell +```bash make package ``` #### Documentation The following is an example of how to build the docs: -```shell +```bash sudo apt install -y npm sudo npm install -g sass @@ -57,7 +57,7 @@ python3 -m venv .venv #### Building the Tests In order to verify the build and capability of ROCm SMI on your system and to see an example of how ROCm SMI can be used, you may build and run the tests that are available in the repo. To build the tests, follow these steps: -```shell +```bash mkdir build cd build cmake -DBUILD_TESTS=ON .. From 5c574ac79cbf75d89ff6f304b2c0e640c15765c6 Mon Sep 17 00:00:00 2001 From: "Galantsev, Dmitrii" Date: Wed, 13 Sep 2023 20:48:49 -0500 Subject: [PATCH 06/11] TESTS - Check power and frequency support It is not guaranteed that power can be read or set for some GPUs (MI300). It is also not guaranteed that frequencies can be set. As this is not a tool issue - we simply skip the failing test. Change-Id: I134e96a476040cef513cd924f00e30cd6dea42a5 Signed-off-by: Galantsev, Dmitrii --- .../functional/frequencies_read_write.cc | 23 ++++++++++++++----- .../functional/power_cap_read_write.cc | 7 ++++++ tests/rocm_smi_test/functional/power_read.cc | 10 ++++++++ .../functional/power_read_write.cc | 9 ++++++++ tests/rocm_smi_test/rsmitst.exclude | 3 +-- 5 files changed, 44 insertions(+), 8 deletions(-) diff --git a/tests/rocm_smi_test/functional/frequencies_read_write.cc b/tests/rocm_smi_test/functional/frequencies_read_write.cc index 5ad627cb5f..5060b59208 100755 --- a/tests/rocm_smi_test/functional/frequencies_read_write.cc +++ b/tests/rocm_smi_test/functional/frequencies_read_write.cc @@ -104,8 +104,7 @@ void TestFrequenciesReadWrite::Run(void) { for (uint32_t dv_ind = 0; dv_ind < num_monitor_devs(); ++dv_ind) { PrintDeviceHeader(dv_ind); - for (uint32_t clk = (uint32_t)RSMI_CLK_TYPE_FIRST; - clk <= RSMI_CLK_TYPE_LAST; ++clk) { + for (uint32_t clk = RSMI_CLK_TYPE_FIRST; clk <= RSMI_CLK_TYPE_LAST; ++clk) { rsmi_clk = (rsmi_clk_type)clk; auto freq_read = [&]() -> bool { @@ -147,14 +146,18 @@ void TestFrequenciesReadWrite::Run(void) { std::endl; } ret = rsmi_dev_gpu_clk_freq_set(dv_ind, rsmi_clk, freq_bitmask); - //Certain ASICs does not allow to set particular clocks. If set function for a clock returns - //permission error despite root access, manually set ret value to success and return - if (ret == RSMI_STATUS_PERMISSION && geteuid() == 0) { + // Certain ASICs does not allow to set particular clocks. If set function for a clock returns + // permission error despite root access, manually set ret value to success and return + // + // Sometimes setting clock frequencies is completely not supported + if ((ret == RSMI_STATUS_PERMISSION && geteuid() == 0) || + (ret == RSMI_STATUS_NOT_SUPPORTED)) { std::cout << "\t**Set " << FreqEnumToStr(rsmi_clk) << ": Not supported on this machine. Skipping..." << std::endl; ret = RSMI_STATUS_SUCCESS; return; } + CHK_ERR_ASRT(ret) ret = rsmi_dev_gpu_clk_freq_get(dv_ind, rsmi_clk, &f); if (ret != RSMI_STATUS_SUCCESS) { @@ -166,12 +169,20 @@ void TestFrequenciesReadWrite::Run(void) { std::cout << "Resetting mask to all frequencies." << std::endl; } ret = rsmi_dev_gpu_clk_freq_set(dv_ind, rsmi_clk, 0xFFFFFFFF); + if (ret == RSMI_STATUS_NOT_SUPPORTED) { + std::cout << "\t**Set " << FreqEnumToStr(rsmi_clk) + << ": Not supported on this machine. Skipping..." << std::endl; + ret = RSMI_STATUS_SUCCESS; + return; + } if (ret != RSMI_STATUS_SUCCESS) { return; } ret = rsmi_dev_perf_level_set(dv_ind, RSMI_DEV_PERF_LEVEL_AUTO); - if (ret != RSMI_STATUS_SUCCESS) { + if (ret == RSMI_STATUS_NOT_SUPPORTED) { + std::cout << "\t**Setting performance level is not supported on this machine. Skipping..." << std::endl; + ret = RSMI_STATUS_SUCCESS; return; } }; diff --git a/tests/rocm_smi_test/functional/power_cap_read_write.cc b/tests/rocm_smi_test/functional/power_cap_read_write.cc index 5d57ec0db2..5481996627 100755 --- a/tests/rocm_smi_test/functional/power_cap_read_write.cc +++ b/tests/rocm_smi_test/functional/power_cap_read_write.cc @@ -114,6 +114,13 @@ void TestPowerCapReadWrite::Run(void) { ret = rsmi_dev_power_cap_get(dv_ind, 0, nullptr); ASSERT_EQ(ret, RSMI_STATUS_INVALID_ARGS); + // Check if power cap is within the range + // skip the test otherwise + if (orig < min || orig > max) { + std::cout << "Power cap is not within the range. Skipping test for " << dv_ind << std::endl; + continue; + } + new_cap = (max + min)/2; IF_VERB(STANDARD) { diff --git a/tests/rocm_smi_test/functional/power_read.cc b/tests/rocm_smi_test/functional/power_read.cc index 02ec355b46..a18cd70676 100755 --- a/tests/rocm_smi_test/functional/power_read.cc +++ b/tests/rocm_smi_test/functional/power_read.cc @@ -117,6 +117,16 @@ void TestPowerRead::Run(void) { } err = rsmi_dev_power_ave_get(i, 0, &val_ui64); + if (err == RSMI_STATUS_NOT_SUPPORTED) { + std::cout << + "\t**Power average information is not supported for this device" + << std::endl; + + // Verify api support checking functionality is working + err = rsmi_dev_power_ave_get(i, 0, nullptr); + ASSERT_EQ(err, RSMI_STATUS_NOT_SUPPORTED); + continue; + } IF_VERB(STANDARD) { std::cout << "\t**Average Power Usage: "; CHK_RSMI_PERM_ERR(err) diff --git a/tests/rocm_smi_test/functional/power_read_write.cc b/tests/rocm_smi_test/functional/power_read_write.cc index dc18cb6de0..1040716ad0 100755 --- a/tests/rocm_smi_test/functional/power_read_write.cc +++ b/tests/rocm_smi_test/functional/power_read_write.cc @@ -123,6 +123,15 @@ void TestPowerReadWrite::Run(void) { PrintDeviceHeader(dv_ind); ret = rsmi_dev_power_profile_presets_get(dv_ind, 0, &status); + if (ret == RSMI_STATUS_NOT_SUPPORTED) { + std::cout << + "\t**Power profile presets are not supported for this device" + << std::endl; + // Verify api support checking functionality is working + ret = rsmi_dev_power_profile_presets_get(dv_ind, 0, nullptr); + ASSERT_EQ(ret, RSMI_STATUS_NOT_SUPPORTED); + continue; + } CHK_ERR_ASRT(ret) // Verify api support checking functionality is working diff --git a/tests/rocm_smi_test/rsmitst.exclude b/tests/rocm_smi_test/rsmitst.exclude index 0632242f93..d87c409421 100644 --- a/tests/rocm_smi_test/rsmitst.exclude +++ b/tests/rocm_smi_test/rsmitst.exclude @@ -64,8 +64,7 @@ $BLACKLIST_ALL_ASICS\ # /sys/class/kfd/kfd/topology/nodes/*/properties FILTER[90400]=\ $BLACKLIST_ALL_ASICS\ -"rsmitstReadOnly.TestVoltCurvRead:"\ -"rsmitstReadWrite.TestPowerReadWrite" +"rsmitstReadOnly.TestVoltCurvRead" FILTER[90401]=${FILTER[90400]} FILTER[90402]=${FILTER[90400]} From 3b95214fffafe68978fb82dde033e9b764b5fb4b Mon Sep 17 00:00:00 2001 From: "Galantsev, Dmitrii" Date: Thu, 14 Sep 2023 11:30:47 -0500 Subject: [PATCH 07/11] rsmiBindings.py - Add initRsmiBindings() Library path was printed at all times even with --json flag. This commit adds a mandatory initRsmiBindings function which is a core component of the rsmiBindings.py library. It **MUST** be called on import. Change-Id: Ic6ae1ec5d1fabba288910e6aed6c4706e53e5cd7 Signed-off-by: Galantsev, Dmitrii --- .gitignore | 5 ++- python_smi_tools/rocm_smi.py | 11 ++++-- python_smi_tools/rsmiBindings.py.in | 59 ++++++++++++++++------------- 3 files changed, 44 insertions(+), 31 deletions(-) diff --git a/.gitignore b/.gitignore index 91cbeef563..1629ea81d3 100644 --- a/.gitignore +++ b/.gitignore @@ -17,4 +17,7 @@ build/ .cache/ # Simulated SYSFS - for early development or debug -device/ \ No newline at end of file +device/ + +# Misc +__pycache__ diff --git a/python_smi_tools/rocm_smi.py b/python_smi_tools/rocm_smi.py index 8834e518b5..a8b3e5a74a 100755 --- a/python_smi_tools/rocm_smi.py +++ b/python_smi_tools/rocm_smi.py @@ -3507,8 +3507,7 @@ def save(deviceList, savefilepath): # The code below is for when this script is run as an executable instead of when imported as a module if __name__ == '__main__': parser = argparse.ArgumentParser( - description='AMD ROCm System Management Interface | ROCM-SMI version: %s | Kernel version: %s' % ( - __version__, getVersion(None, rsmi_sw_component_t.RSMI_SW_COMP_DRIVER)), + description=f'AMD ROCm System Management Interface | ROCM-SMI version: {__version__}', formatter_class=lambda prog: argparse.HelpFormatter(prog, max_help_position=90, width=120)) groupDev = parser.add_argument_group() groupDisplayOpt = parser.add_argument_group('Display Options') @@ -3668,6 +3667,11 @@ if __name__ == '__main__': args = parser.parse_args() + # Must set PRINT_JSON early so the prints can be silenced + if args.json or args.csv: + PRINT_JSON = True + # Initialize rsmiBindings + rocmsmi = initRsmiBindings(silent=PRINT_JSON) # Initialize the rocm SMI library initializeRsmi() @@ -3703,8 +3707,7 @@ if __name__ == '__main__': sys.exit(1) # If we want JSON/CSV output, initialize the keys (devices) - if args.json or args.csv: - PRINT_JSON = True + if PRINT_JSON: for device in deviceList: JSON_DATA['card' + str(device)] = {} diff --git a/python_smi_tools/rsmiBindings.py.in b/python_smi_tools/rsmiBindings.py.in index 9ffcac138d..e6b141889f 100644 --- a/python_smi_tools/rsmiBindings.py.in +++ b/python_smi_tools/rsmiBindings.py.in @@ -1,5 +1,6 @@ #!/usr/bin/env python3 """ROCm_SMI_LIB CLI Tool Python Bindings""" +# NOTE: You MUST call rsmiBindings.initRsmiBindings() when using this library! # TODO: Get most (or all) of these from rocm_smi.h to avoid mismatches and redundancy from __future__ import print_function @@ -14,36 +15,42 @@ import os # relative path changed accordingly. # if ROCM_SMI_LIB_PATH is set, we can load 'librocm_smi64.so' from that location # +# Library load is wrapped in a function so prints can be hidden for PRINT_JSON mode. path_librocm = str() -rocm_smi_lib_path = os.getenv('ROCM_SMI_LIB_PATH') -if (rocm_smi_lib_path != None): - path_librocm = rocm_smi_lib_path -else: - path_librocm = os.path.dirname(os.path.realpath(__file__)) + '/../../@CMAKE_INSTALL_LIBDIR@/librocm_smi64.so.@VERSION_MAJOR@' +def initRsmiBindings(silent=False): + def print_silent(*args): + if not silent: + print(args) -if not os.path.isfile(path_librocm): - print('Unable to find %s . Trying /opt/rocm*' % path_librocm) - for root, dirs, files in os.walk('/opt', followlinks=True): - if 'librocm_smi64.so.@VERSION_MAJOR@' in files: - path_librocm = os.path.join(os.path.realpath(root), 'librocm_smi64.so.@VERSION_MAJOR@') - if os.path.isfile(path_librocm): - print('Using lib from %s' % path_librocm) + rocm_smi_lib_path = os.getenv('ROCM_SMI_LIB_PATH') + if (rocm_smi_lib_path != None): + path_librocm = rocm_smi_lib_path else: - print('Unable to find librocm_smi64.so.@VERSION_MAJOR@') -else: - print('Library loaded from: %s ' % path_librocm) + path_librocm = os.path.dirname(os.path.realpath(__file__)) + '/../../@CMAKE_INSTALL_LIBDIR@/librocm_smi64.so.@VERSION_MAJOR@' -# ----------> TODO: Support static libs as well as SO -try: - cdll.LoadLibrary(path_librocm) - rocmsmi = CDLL(path_librocm) -except OSError: - print('Unable to load the rocm_smi library.\n'\ - 'Set LD_LIBRARY_PATH to the folder containing librocm_smi64.so.@VERSION_MAJOR@\n'\ - '{0}Please refer to https://github.com/'\ - 'RadeonOpenCompute/rocm_smi_lib for the installation guide.{1}'\ - .format('\33[33m', '\033[0m')) - exit() + if not os.path.isfile(path_librocm): + print_silent('Unable to find %s . Trying /opt/rocm*' % path_librocm) + for root, dirs, files in os.walk('/opt', followlinks=True): + if 'librocm_smi64.so.@VERSION_MAJOR@' in files: + path_librocm = os.path.join(os.path.realpath(root), 'librocm_smi64.so.@VERSION_MAJOR@') + if os.path.isfile(path_librocm): + print_silent('Using lib from %s' % path_librocm) + else: + print('Unable to find librocm_smi64.so.@VERSION_MAJOR@') + else: + print_silent('Library loaded from: %s ' % path_librocm) + + # ----------> TODO: Support static libs as well as SO + try: + cdll.LoadLibrary(path_librocm) + return CDLL(path_librocm) + except OSError: + print('Unable to load the rocm_smi library.\n'\ + 'Set LD_LIBRARY_PATH to the folder containing librocm_smi64.so.@VERSION_MAJOR@\n'\ + '{0}Please refer to https://github.com/'\ + 'RadeonOpenCompute/rocm_smi_lib for the installation guide.{1}'\ + .format('\33[33m', '\033[0m')) + exit() # Device ID dv_id = c_uint64() From 094c98a74fa9d770dc5f714de512eedd4ab78971 Mon Sep 17 00:00:00 2001 From: "Galantsev, Dmitrii" Date: Thu, 14 Sep 2023 11:58:49 -0500 Subject: [PATCH 08/11] rocm_smi.py: Fix pipe into head error When piping rocm_smi into 'head' it failed with "Broken pipe" error. The error can be safely ignored. head closes the pipe early which causes calls a SIGPIPE signal to be raised. https://docs.python.org/3/library/signal.html#note-on-sigpipe Change-Id: I4a589c6ed9a8c5b50de84b33e28115c6b510045f Signed-off-by: Galantsev, Dmitrii --- python_smi_tools/rocm_smi.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/python_smi_tools/rocm_smi.py b/python_smi_tools/rocm_smi.py index a8b3e5a74a..4d1a5bad41 100755 --- a/python_smi_tools/rocm_smi.py +++ b/python_smi_tools/rocm_smi.py @@ -638,10 +638,21 @@ def printLog(device, metricName, value=None, extraSpace=False, useItalics=False) lock.acquire() if useItalics: logstr = italics + logstr + end - if extraSpace: - print('\n' + logstr + '\n', end='', flush=True) - else: - print(logstr + '\n', end='', flush=True) + try: + if extraSpace: + print('\n', end='') + print(logstr + '\n', end='') + sys.stdout.flush() + # when piped into programs like 'head' - print throws an error. + # silently ignore instead + except(BrokenPipeError, IOError): + # https://docs.python.org/3/library/signal.html#note-on-sigpipe + # Python flushes standard streams on exit; redirect remaining output + # to devnull to avoid another BrokenPipeError at shutdown + devnull = os.open(os.devnull, os.O_WRONLY) + os.dup2(devnull, sys.stdout.fileno()) + sys.exit(1) # Python exits with error code 1 on EPIPE + lock.release() From 7b32ea614b45d35a9b47f1db832edd8273f108b6 Mon Sep 17 00:00:00 2001 From: Sam Wu Date: Wed, 6 Sep 2023 10:55:33 -0600 Subject: [PATCH 09/11] fix toc to point to correct doxysphinx output path update doc requirements; rocm-docs-core to 0.24.1 Change-Id: I78257d476a8bc47fd1a4ee03aa3db1a430ed116f --- README.md | 31 ++++++++++++++++++------------- docs/.doxygen/.gitignore | 1 + docs/.gitignore | 2 ++ docs/.sphinx/_toc.yml | 5 +++-- docs/.sphinx/_toc.yml.in | 5 +++-- docs/.sphinx/requirements.in | 2 +- docs/.sphinx/requirements.txt | 26 ++++++++++++++++---------- docs/conf.py | 2 -- 8 files changed, 44 insertions(+), 30 deletions(-) create mode 100644 docs/.doxygen/.gitignore diff --git a/README.md b/README.md index 12a12bd63c..c987c36a82 100755 --- a/README.md +++ b/README.md @@ -1,5 +1,3 @@ - - # ROCm System Management Interface (ROCm SMI) Library The ROCm System Management Interface Library, or ROCm SMI library, is part of the Radeon Open Compute [ROCm](https://github.com/RadeonOpenCompute) software stack . It is a C library for Linux that provides a user space interface for applications to monitor and control GPU applications. @@ -12,22 +10,25 @@ The information contained herein is for informational purposes only, and is subj © 2022-2023 Advanced Micro Devices, Inc. All Rights Reserved. +## Building ROCm SMI -# Building ROCm SMI +### Additional Required software for building -#### Additional Required software for building In order to build the ROCm SMI library, the following components are required. Note that the software versions listed are what was used in development. Earlier versions are not guaranteed to work: + * CMake (v3.5.0) * g++ (5.4.0) In order to build the latest documentation, the following are required: + * Python 3.8+ * NPM (sass) The source code for ROCm SMI is available on [Github](https://github.com/RadeonOpenCompute/rocm_smi_lib). After the ROCm SMI library git repository has been cloned to a local Linux machine, building the library is achieved by following the typical CMake build sequence. Specifically, -```bash + +```shell mkdir -p build cd build cmake .. @@ -35,26 +36,27 @@ make -j $(nproc) # Install library file and header; default location is /opt/rocm make install ``` + The built library will appear in the `build` folder. To build the rpm and deb packages follow the above steps with: -```bash + +```shell make package ``` #### Documentation + The following is an example of how to build the docs: -```bash -sudo apt install -y npm -sudo npm install -g sass +```shell python3 -m venv .venv - .venv/bin/python3 -m pip install -r docs/.sphinx/requirements.txt .venv/bin/python3 -m sphinx -T -E -b html -d docs/_build/doctrees -D language=en docs docs/_build/html ``` #### Building the Tests + In order to verify the build and capability of ROCm SMI on your system and to see an example of how ROCm SMI can be used, you may build and run the tests that are available in the repo. To build the tests, follow these steps: ```bash @@ -66,11 +68,14 @@ make -j $(nproc) To run the test, execute the program `rsmitst` that is built from the steps above. -# Usage Basics -## Device Indices +## Usage Basics + +### Device Indices + Many of the functions in the library take a "device index". The device index is a number greater than or equal to 0, and less than the number of devices detected, as determined by `rsmi_num_monitor_devices()`. The index is used to distinguish the detected devices from one another. It is important to note that a device may end up with a different index after a reboot, so an index should not be relied upon to be constant over reboots. -# Hello ROCm SMI +## Hello ROCm SMI + The only required ROCm-SMI call for any program that wants to use ROCm-SMI is the `rsmi_init()` call. This call initializes some internal data structures that will be used by subsequent ROCm-SMI calls. When ROCm-SMI is no longer being used, `rsmi_shut_down()` should be called. This provides a way to do any releasing of resources that ROCm-SMI may have held. In many cases, this may have no effect, but may be necessary in future versions of the library. diff --git a/docs/.doxygen/.gitignore b/docs/.doxygen/.gitignore new file mode 100644 index 0000000000..5ebfac1dea --- /dev/null +++ b/docs/.doxygen/.gitignore @@ -0,0 +1 @@ +docBin/ diff --git a/docs/.gitignore b/docs/.gitignore index b8ea6fcbcd..b84233aed8 100644 --- a/docs/.gitignore +++ b/docs/.gitignore @@ -7,3 +7,5 @@ /_templates/ /html/ /latex/ +404.md +data/AMD-404.png diff --git a/docs/.sphinx/_toc.yml b/docs/.sphinx/_toc.yml index 84f11ded40..45624f4efe 100644 --- a/docs/.sphinx/_toc.yml +++ b/docs/.sphinx/_toc.yml @@ -2,5 +2,6 @@ defaults: numbered: False maxdepth: 6 root: index -entries: - - file: html/index \ No newline at end of file +subtrees: + - entries: + - file: .doxygen/docBin/html/index diff --git a/docs/.sphinx/_toc.yml.in b/docs/.sphinx/_toc.yml.in index ac4814ce11..a9a3dd8f89 100755 --- a/docs/.sphinx/_toc.yml.in +++ b/docs/.sphinx/_toc.yml.in @@ -4,5 +4,6 @@ defaults: numbered: False maxdepth: 6 root: index -entries: - - file: html/index \ No newline at end of file +subtrees: + - entries: + - file: .doxygen/docBin/html/index diff --git a/docs/.sphinx/requirements.in b/docs/.sphinx/requirements.in index b98c8ab534..0bcf7dd930 100755 --- a/docs/.sphinx/requirements.in +++ b/docs/.sphinx/requirements.in @@ -1 +1 @@ -rocm-docs-core[api_reference]==0.9.0 +rocm-docs-core[api_reference]==0.24.1 diff --git a/docs/.sphinx/requirements.txt b/docs/.sphinx/requirements.txt index 5257eac6b3..297193c462 100644 --- a/docs/.sphinx/requirements.txt +++ b/docs/.sphinx/requirements.txt @@ -2,7 +2,7 @@ # This file is autogenerated by pip-compile with Python 3.8 # by the following command: # -# pip-compile .sphinx/requirements.in +# pip-compile requirements.in # accessible-pygments==0.0.4 # via pydata-sphinx-theme @@ -41,7 +41,9 @@ docutils==0.16 # myst-parser # pydata-sphinx-theme # sphinx -doxysphinx==3.2.3 +doxysphinx==3.3.4 + # via rocm-docs-core +fastjsonschema==2.18.0 # via rocm-docs-core gitdb==4.0.10 # via gitpython @@ -59,10 +61,8 @@ jinja2==3.1.2 # via # myst-parser # sphinx -json5==0.9.11 +libsass==0.22.0 # via doxysphinx -linkify-it-py==1.0.3 - # via myst-parser lxml==4.9.2 # via doxysphinx markdown-it-py==2.2.0 @@ -75,7 +75,9 @@ mdit-py-plugins==0.3.5 # via myst-parser mdurl==0.1.2 # via markdown-it-py -myst-parser[linkify]==1.0.0 +mpire==2.8.0 + # via doxysphinx +myst-parser==1.0.0 # via rocm-docs-core packaging==23.1 # via @@ -92,8 +94,11 @@ pygithub==1.58.1 pygments==2.15.0 # via # accessible-pygments + # mpire # pydata-sphinx-theme # sphinx +pyjson5==1.6.4 + # via doxysphinx pyjwt[crypto]==2.6.0 # via pygithub pynacl==1.5.0 @@ -105,13 +110,14 @@ pytz==2023.3 pyyaml==6.0 # via # myst-parser + # rocm-docs-core # sphinx-external-toc requests==2.28.2 # via # pygithub # sphinx -rocm-docs-core[api_reference]==0.9.0 - # via -r .sphinx/requirements.in +rocm-docs-core[api_reference]==0.24.1 + # via -r requirements.in smmap==5.0.0 # via gitdb snowballstemmer==2.2.0 @@ -151,10 +157,10 @@ sphinxcontrib-qthelp==1.0.3 # via sphinx sphinxcontrib-serializinghtml==1.1.5 # via sphinx +tqdm==4.66.1 + # via mpire typing-extensions==4.5.0 # via pydata-sphinx-theme -uc-micro-py==1.0.1 - # via linkify-it-py urllib3==1.26.15 # via requests wrapt==1.15.0 diff --git a/docs/conf.py b/docs/conf.py index e2d485b9ce..733e8bcc24 100755 --- a/docs/conf.py +++ b/docs/conf.py @@ -13,5 +13,3 @@ docs_core.setup() for sphinx_var in ROCmDocs.SPHINX_VARS: globals()[sphinx_var] = getattr(docs_core, sphinx_var) - -html_theme_options["show_navbar_depth"] = 2 From 431a7071a044409947da2e5320ec9c8b6e1a7e89 Mon Sep 17 00:00:00 2001 From: "Galantsev, Dmitrii" Date: Fri, 15 Sep 2023 02:59:28 -0500 Subject: [PATCH 10/11] PACKAGE - Cleanup packaging - Clean-up packaging scripts. More consistent with RDC. - Remove all 'sudo' calls. all these scripts are to be ran by root. - Reduce scope of variables. - Remove unnecessary functions Change-Id: Ib90f8e66ef4eae24f73e940fff44f515e12233f5 Signed-off-by: Galantsev, Dmitrii --- DEBIAN/postinst.in | 79 ++++++++++++++++++---------------------- DEBIAN/prerm.in | 48 +++++++++++-------------- RPM/post.in | 90 +++++++++++++++++++--------------------------- RPM/postun.in | 2 +- RPM/preun.in | 28 +++++++-------- 5 files changed, 106 insertions(+), 141 deletions(-) diff --git a/DEBIAN/postinst.in b/DEBIAN/postinst.in index ab2f640553..9e0fe4457c 100755 --- a/DEBIAN/postinst.in +++ b/DEBIAN/postinst.in @@ -1,21 +1,18 @@ #!/bin/bash -#set -x - -packageName="rocm-smi-lib" -logPath=/var/log/rocm_smi_lib -logName=ROCm-SMI-lib.log -logFile="${logPath}/${logName}" -logrotateConfFile=/etc/logrotate.d/rocm_smi.conf - -do_addLogFolder() { - sudo mkdir -p "${logPath}" - sudo touch "${logFile}" - sudo chmod -R a+rw "${logPath}" - sudo chmod a+rw "${logFile}" -} do_configureLogrotate() { - logrotate --version &>/dev/null + local IS_SYSTEMD=0 + local packageName="rocm-smi-lib" + local logPath=/var/log/rocm_smi_lib + local logFile="${logPath}/ROCm-SMI-lib.log" + local logrotateConfFile=/etc/logrotate.d/rocm_smi.conf + + mkdir -p "${logPath}" + touch "${logFile}" + chmod -R a+rw "${logPath}" + chmod a+rw "${logFile}" + + command -v logrotate &>/dev/null if [ $? -ne 0 ]; then echo "[WARNING] Detected logrotate is not installed."\ "$packageName logs (when turned on) will not rotate properly." @@ -23,14 +20,14 @@ do_configureLogrotate() { fi if [ ! -f $logrotateConfFile ]; then - sudo touch "${logrotateConfFile}" - sudo chmod 644 "${logrotateConfFile}" # root r/w, all others read + touch "${logrotateConfFile}" + chmod 644 "${logrotateConfFile}" # root r/w, all others read # ROCm SMI logging rotation, rotates files using root user/group # Hourly logrotation check # Only rotates if size grew larger than 1MB # Max of 4 rotation files, oldest will be removed # Rotated files use date extention of ex. ROCm-SMI-lib.log.2023-05-09_16:51:42 - cat << EOF | sudo tee "${logrotateConfFile}" >/dev/null + cat << EOF > "${logrotateConfFile}" ${logFile} { su root root hourly @@ -47,43 +44,29 @@ EOF # issue was RPM build thought we were using macros # https://gitlab.kitware.com/cmake/cmake/-/issues/22965 # https://rpm-software-management.github.io/rpm/manual/spec.html - sudo sed -i s/%%/%/g "${logrotateConfFile}" + sed -i s/%%/%/g "${logrotateConfFile}" # workaround: remove extra 'OURCE' text # from rocm_smi.conf. Unsure if CMAKE, # bash, or here document # issue (only seen on RHEL 8.7) - sudo sed -i s/OURCE//g "${logrotateConfFile}" + sed -i s/OURCE//g "${logrotateConfFile}" fi # check if logrotate uses system timers, Ubuntu/modern OS's do # Several older OS's like RHEL 8.7, do not. Instead defaults # to use daily cron jobs - see https://stackoverflow.com/a/69465677 - sudo systemctl list-timers|grep -iq logrotate - if [ $? -ne 0 ]; then - # confirm logrotate file exists in daily - if [ -f /etc/cron.daily/logrotate ]; then - # move logrotate daily to hourly - if [ -d /etc/cron.hourly ]; then - sudo mv /etc/cron.daily/logrotate /etc/cron.hourly/logrotate - else - echo "[WARNING] Could find and configure hourly cron for $packageName's"\ - " logrotate. $packageName logs (when turned on) will not rotate properly." - fi - else - # confirm that it's already been moved to hourly - sudo find /etc/cron.* -iname logrotate -print -quit |grep -iq hourly - if [ $? -ne 0 ]; then - echo "[WARNING] Could not configure an hourly cron for $packageName's logrotate."\ - "$packageName logs (when turned on) may not rotate properly." - fi + if [ -d /run/systemd/system ]; then + systemctl list-timers | grep -iq logrotate + if [ $? -ne 0 ]; then + IS_SYSTEMD=1 fi - return #done configuring for non-systemd timers - else + fi + if [ "$IS_SYSTEMD" -eq 1 ]; then # Configure systemd timers - the typical setup for modern Linux logrotation setups if [ -f /lib/systemd/system/logrotate.timer ]; then if [ ! -f /lib/systemd/system/logrotate.timer.backup ]; then - sudo cp /lib/systemd/system/logrotate.timer /lib/systemd/system/logrotate.timer.backup + cp /lib/systemd/system/logrotate.timer /lib/systemd/system/logrotate.timer.backup fi - cat <<'EOF' | sudo tee /lib/systemd/system/logrotate.timer >/dev/null + cat << EOF > /lib/systemd/system/logrotate.timer [Unit] Description=Hourly rotation of log files Documentation=man:logrotate(8) man:logrotate.conf(5) @@ -97,12 +80,19 @@ Persistent=true [Install] WantedBy=timers.target EOF - sudo systemctl reenable --now logrotate.timer + systemctl reenable --now logrotate.timer else echo "[WARNING] Could not configure systemd timer for $packageName's logrotate."\ "$packageName logs (when turned on) will not rotate properly." fi - return #done configuring for systemd timers + else + # $IS_SYSTEMD -eq 0 + if [ -f /etc/cron.daily/logrotate ]; then + # move logrotate daily to hourly + if [ -d /etc/cron.hourly ]; then + mv /etc/cron.daily/logrotate /etc/cron.hourly/logrotate + fi + fi fi } @@ -117,7 +107,6 @@ do_ldconfig() { case "$1" in ( configure ) do_ldconfig - do_addLogFolder do_configureLogrotate ;; ( abort-upgrade | abort-remove | abort-deconfigure ) diff --git a/DEBIAN/prerm.in b/DEBIAN/prerm.in index 9b4efa9899..7bf61f02d1 100755 --- a/DEBIAN/prerm.in +++ b/DEBIAN/prerm.in @@ -1,29 +1,4 @@ #!/bin/bash -set -e - -packageName="rocm-smi-lib" -logPath=/var/log/rocm_smi_lib -logName=ROCm-SMI-lib.log -logFile="${logPath}/${logName}" -logrotateConfFile=/etc/logrotate.d/rocm_smi.conf - -rm_logFolder() { - sudo rm -rf "$logPath" -} - -return_logrotateToOrigConfig() { - if [ -f $logrotateConfFile ]; then - sudo rm -rf "${logrotateConfFile}" - fi - if [ -f /etc/cron.hourly/logrotate ]; then - sudo mv /etc/cron.hourly/logrotate /etc/cron.daily/logrotate - fi - if [ -f /lib/systemd/system/logrotate.timer.backup ]; then - sudo cp /lib/systemd/system/logrotate.timer.backup /lib/systemd/system/logrotate.timer - sudo rm -rf /lib/systemd/system/logrotate.timer.backup - sudo systemctl reenable --now logrotate.timer - fi -} rm_ldconfig() { # left-hand term originates from ENABLE_LDCONFIG = ON/OFF at package build @@ -34,8 +9,27 @@ rm_ldconfig() { } rm_pyc() { - # remove pyc file generated by python - rm -rf @CPACK_PACKAGING_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBEXECDIR@/rocm_smi/__pycache__ + # remove pyc file generated by python + rm -rf @CPACK_PACKAGING_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBEXECDIR@/rocm_smi/__pycache__ +} + +rm_logFolder() { + rm -rf /var/log/rocm_smi_lib +} + +return_logrotateToOrigConfig() { + local logrotateConfFile=/etc/logrotate.d/rocm_smi.conf + if [ -f $logrotateConfFile ]; then + rm -rf "$logrotateConfFile" + fi + if [ -f /etc/cron.hourly/logrotate ]; then + mv /etc/cron.hourly/logrotate /etc/cron.daily/logrotate + fi + if [ -f /lib/systemd/system/logrotate.timer.backup ]; then + cp /lib/systemd/system/logrotate.timer.backup /lib/systemd/system/logrotate.timer + rm -rf /lib/systemd/system/logrotate.timer.backup + systemctl reenable --now logrotate.timer + fi } diff --git a/RPM/post.in b/RPM/post.in index b04e31c9f9..ccf1f814a5 100755 --- a/RPM/post.in +++ b/RPM/post.in @@ -1,21 +1,18 @@ #!/bin/bash -#set -x - -packageName="rocm-smi-lib" -logPath=/var/log/rocm_smi_lib -logName=ROCm-SMI-lib.log -logFile="${logPath}/${logName}" -logrotateConfFile=/etc/logrotate.d/rocm_smi.conf - -do_addLogFolder() { - sudo mkdir -p "${logPath}" - sudo touch "${logFile}" - sudo chmod -R a+rw "${logPath}" - sudo chmod a+rw "${logFile}" -} do_configureLogrotate() { - logrotate --version &>/dev/null + local IS_SYSTEMD=0 + local packageName="rocm-smi-lib" + local logPath=/var/log/rocm_smi_lib + local logFile="${logPath}/ROCm-SMI-lib.log" + local logrotateConfFile=/etc/logrotate.d/rocm_smi.conf + + mkdir -p "${logPath}" + touch "${logFile}" + chmod -R a+rw "${logPath}" + chmod a+rw "${logFile}" + + command -v logrotate &>/dev/null if [ $? -ne 0 ]; then echo "[WARNING] Detected logrotate is not installed."\ "$packageName logs (when turned on) will not rotate properly." @@ -23,14 +20,14 @@ do_configureLogrotate() { fi if [ ! -f $logrotateConfFile ]; then - sudo touch "${logrotateConfFile}" - sudo chmod 644 "${logrotateConfFile}" # root r/w, all others read + touch "${logrotateConfFile}" + chmod 644 "${logrotateConfFile}" # root r/w, all others read # ROCm SMI logging rotation, rotates files using root user/group # Hourly logrotation check # Only rotates if size grew larger than 1MB # Max of 4 rotation files, oldest will be removed # Rotated files use date extention of ex. ROCm-SMI-lib.log.2023-05-09_16:51:42 - cat << EOF | sudo tee "${logrotateConfFile}" >/dev/null + cat << EOF > "${logrotateConfFile}" ${logFile} { su root root hourly @@ -47,43 +44,29 @@ EOF # issue was RPM build thought we were using macros # https://gitlab.kitware.com/cmake/cmake/-/issues/22965 # https://rpm-software-management.github.io/rpm/manual/spec.html - sudo sed -i s/%%/%/g "${logrotateConfFile}" + sed -i s/%%/%/g "${logrotateConfFile}" # workaround: remove extra 'OURCE' text # from rocm_smi.conf. Unsure if CMAKE, # bash, or here document # issue (only seen on RHEL 8.7) - sudo sed -i s/OURCE//g "${logrotateConfFile}" + sed -i s/OURCE//g "${logrotateConfFile}" fi # check if logrotate uses system timers, Ubuntu/modern OS's do # Several older OS's like RHEL 8.7, do not. Instead defaults # to use daily cron jobs - see https://stackoverflow.com/a/69465677 - sudo systemctl list-timers|grep -iq logrotate - if [ $? -ne 0 ]; then - # confirm logrotate file exists in daily - if [ -f /etc/cron.daily/logrotate ]; then - # move logrotate daily to hourly - if [ -d /etc/cron.hourly ]; then - sudo mv /etc/cron.daily/logrotate /etc/cron.hourly/logrotate - else - echo "[WARNING] Could find and configure hourly cron for $packageName's"\ - " logrotate. $packageName logs (when turned on) will not rotate properly." - fi - else - # confirm that it's already been moved to hourly - sudo find /etc/cron.* -iname logrotate -print -quit |grep -iq hourly - if [ $? -ne 0 ]; then - echo "[WARNING] Could not configure an hourly cron for $packageName's logrotate."\ - "$packageName logs (when turned on) may not rotate properly." - fi + if [ -d /run/systemd/system ]; then + systemctl list-timers | grep -iq logrotate + if [ $? -ne 0 ]; then + IS_SYSTEMD=1 fi - return #done configuring for non-systemd timers - else + fi + if [ "$IS_SYSTEMD" -eq 1 ]; then # Configure systemd timers - the typical setup for modern Linux logrotation setups if [ -f /lib/systemd/system/logrotate.timer ]; then if [ ! -f /lib/systemd/system/logrotate.timer.backup ]; then - sudo cp /lib/systemd/system/logrotate.timer /lib/systemd/system/logrotate.timer.backup + cp /lib/systemd/system/logrotate.timer /lib/systemd/system/logrotate.timer.backup fi - cat <<'EOF' | sudo tee /lib/systemd/system/logrotate.timer >/dev/null + cat << EOF > /lib/systemd/system/logrotate.timer [Unit] Description=Hourly rotation of log files Documentation=man:logrotate(8) man:logrotate.conf(5) @@ -97,12 +80,19 @@ Persistent=true [Install] WantedBy=timers.target EOF - sudo systemctl reenable --now logrotate.timer + systemctl reenable --now logrotate.timer else echo "[WARNING] Could not configure systemd timer for $packageName's logrotate."\ "$packageName logs (when turned on) will not rotate properly." fi - return #done configuring for systemd timers + else + # $IS_SYSTEMD -eq 0 + if [ -f /etc/cron.daily/logrotate ]; then + # move logrotate daily to hourly + if [ -d /etc/cron.hourly ]; then + mv /etc/cron.daily/logrotate /etc/cron.hourly/logrotate + fi + fi fi } @@ -114,14 +104,8 @@ do_ldconfig() { fi } -# left-hand term originates from ENABLE_LDCONFIG = ON/OFF at package build -if [ "@ENABLE_LDCONFIG@" == "ON" ]; then - echo -e "@CPACK_PACKAGING_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBDIR@" > /etc/ld.so.conf.d/x86_64-librocm_smi_lib.conf - ldconfig -fi - # post install or upgrade, $i is 1 or 2 -> do these actions -if [ $1 -ge 1 ]; then - do_addLogFolder +if [ "$1" -ge 1 ]; then + do_ldconfig do_configureLogrotate -fi \ No newline at end of file +fi diff --git a/RPM/postun.in b/RPM/postun.in index 0dd41d82d8..b42b5b90b9 100755 --- a/RPM/postun.in +++ b/RPM/postun.in @@ -1,7 +1,7 @@ #!/bin/bash # second term originates from ENABLE_LDCONFIG = ON/OFF at package build -if [ $1 -le 1 ] && [ "@ENABLE_LDCONFIG@" == "ON" ]; then +if [ "$1" -le 1 ] && [ "@ENABLE_LDCONFIG@" == "ON" ]; then # perform the below actions for rpm remove($1=0) or upgrade($1=1) operations rm -f /etc/ld.so.conf.d/x86_64-librocm_smi_lib.conf ldconfig diff --git a/RPM/preun.in b/RPM/preun.in index b9c2eda6b3..b72c39465c 100755 --- a/RPM/preun.in +++ b/RPM/preun.in @@ -1,34 +1,32 @@ #!/bin/bash -#set -x -packageName="rocm-smi-lib" -logPath=/var/log/rocm_smi_lib -logName=ROCm-SMI-lib.log -logFile="${logPath}/${logName}" -logrotateConfFile=/etc/logrotate.d/rocm_smi.conf +rm_pyc() { + # remove pyc file generated by python + rm -rf @CPACK_PACKAGING_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBEXECDIR@/rocm_smi/__pycache__ +} rm_logFolder() { - sudo rm -rf "$logPath" + rm -rf /var/log/rocm_smi_lib } return_logrotateToOrigConfig() { + local logrotateConfFile=/etc/logrotate.d/rocm_smi.conf if [ -f $logrotateConfFile ]; then - sudo rm -rf "${logrotateConfFile}" + rm -rf "$logrotateConfFile" fi if [ -f /etc/cron.hourly/logrotate ]; then - sudo mv /etc/cron.hourly/logrotate /etc/cron.daily/logrotate + mv /etc/cron.hourly/logrotate /etc/cron.daily/logrotate fi if [ -f /lib/systemd/system/logrotate.timer.backup ]; then - sudo cp /lib/systemd/system/logrotate.timer.backup /lib/systemd/system/logrotate.timer - sudo rm -rf /lib/systemd/system/logrotate.timer.backup - sudo systemctl reenable --now logrotate.timer + cp /lib/systemd/system/logrotate.timer.backup /lib/systemd/system/logrotate.timer + rm -rf /lib/systemd/system/logrotate.timer.backup + systemctl reenable --now logrotate.timer fi } -if [ $1 -le 1 ]; then +if [ "$1" -le 1 ]; then # perform the below actions for rpm remove($1=0) or upgrade($1=1) operations - # remove pyc file generated by python - rm -rf @CPACK_PACKAGING_INSTALL_PREFIX@/@CMAKE_INSTALL_LIBEXECDIR@/rocm_smi/__pycache__ + rm_pyc rm_logFolder return_logrotateToOrigConfig fi From b99867eb80cc7b6b47ae4b3e89a9e0ca9df0e4a7 Mon Sep 17 00:00:00 2001 From: "Galantsev, Dmitrii" Date: Wed, 20 Sep 2023 10:05:34 -0500 Subject: [PATCH 11/11] PACKAGE - Fix packaging Allow for configureLogrotate to fail without failing configure In previous commit I forgot to invert the check when switching "IS_SYSTEMD" and "!IS_SYSTEMD" if-else statements. Change-Id: I8eb8e7981c6353a2e60064eb3a6e35821ea2a0d0 Signed-off-by: Galantsev, Dmitrii --- DEBIAN/postinst.in | 4 ++-- RPM/post.in | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/DEBIAN/postinst.in b/DEBIAN/postinst.in index 9e0fe4457c..641949f95a 100755 --- a/DEBIAN/postinst.in +++ b/DEBIAN/postinst.in @@ -56,7 +56,7 @@ EOF # to use daily cron jobs - see https://stackoverflow.com/a/69465677 if [ -d /run/systemd/system ]; then systemctl list-timers | grep -iq logrotate - if [ $? -ne 0 ]; then + if [ $? -eq 0 ]; then IS_SYSTEMD=1 fi fi @@ -107,7 +107,7 @@ do_ldconfig() { case "$1" in ( configure ) do_ldconfig - do_configureLogrotate + do_configureLogrotate || return 0 ;; ( abort-upgrade | abort-remove | abort-deconfigure ) echo "$1" diff --git a/RPM/post.in b/RPM/post.in index ccf1f814a5..d2bd016589 100755 --- a/RPM/post.in +++ b/RPM/post.in @@ -56,7 +56,7 @@ EOF # to use daily cron jobs - see https://stackoverflow.com/a/69465677 if [ -d /run/systemd/system ]; then systemctl list-timers | grep -iq logrotate - if [ $? -ne 0 ]; then + if [ $? -eq 0 ]; then IS_SYSTEMD=1 fi fi @@ -107,5 +107,5 @@ do_ldconfig() { # post install or upgrade, $i is 1 or 2 -> do these actions if [ "$1" -ge 1 ]; then do_ldconfig - do_configureLogrotate + do_configureLogrotate || return 0 fi