From 4c9484250824d6ca4899f4b8a0574b89f227ea0e Mon Sep 17 00:00:00 2001 From: Chris Freehill Date: Thu, 18 Jun 2020 14:55:46 -0500 Subject: [PATCH] Ensure no device mutexes are left held on shut_down Also, fix TestMutualExclusion and TestEvtNofifReadWrite. Previously, some of the normal SetUp function was not being done for this test. In some cases, no DRM devices are being found on the test machine. Skip those. Change-Id: Iaa5a257841eb459aa57491ae9680c34a60d5ac2b [ROCm/rocm_smi_lib commit: 59394f3354096760481c487d47ace07b61f16f55] --- projects/rocm-smi-lib/src/rocm_smi.cc | 14 +++ .../src/shared_mutex/shared_mutex.cc | 1 + .../functional/evt_notif_read_write.cc | 4 + .../functional/mutual_exclusion.cc | 92 +++++++++++++------ .../tests/rocm_smi_test/test_base.h | 2 +- 5 files changed, 82 insertions(+), 31 deletions(-) diff --git a/projects/rocm-smi-lib/src/rocm_smi.cc b/projects/rocm-smi-lib/src/rocm_smi.cc index f632ddacd0..282a4ceaa3 100755 --- a/projects/rocm-smi-lib/src/rocm_smi.cc +++ b/projects/rocm-smi-lib/src/rocm_smi.cc @@ -588,6 +588,20 @@ rsmi_shut_down(void) { return RSMI_STATUS_INIT_ERROR; } + // Release any device mutexes that are being held +#if DEBUG + int ret = 0; +#endif + for (int i = 0; i < smi.monitor_devices().size(); ++i) { +#if DEBUG + ret = pthread_mutex_unlock(smi.monitor_devices()[i]->mutex()); + std::cout << "pthread_mutex_unlock() returned " << ret << + " for device " << i << " in rsmi_shut_down()" << std::endl; +#else + (void)pthread_mutex_unlock(smi.monitor_devices()[i]->mutex()); +#endif + } + (void)smi.ref_count_dec(); if (smi.ref_count() == 0) { diff --git a/projects/rocm-smi-lib/src/shared_mutex/shared_mutex.cc b/projects/rocm-smi-lib/src/shared_mutex/shared_mutex.cc index 6ccd546c72..ccb55f4a23 100755 --- a/projects/rocm-smi-lib/src/shared_mutex/shared_mutex.cc +++ b/projects/rocm-smi-lib/src/shared_mutex/shared_mutex.cc @@ -91,6 +91,7 @@ shared_mutex_t shared_mutex_init(const char *name, mode_t mode) { if (ret || (mutex.created == 0 && reinterpret_cast(addr)->ptr == NULL)) { // Something is out of sync. + fprintf(stderr, "pthread_mutex_timedlock() returned %d\n", ret); perror("Failed to initialize RSMI device mutex after 5 seconds. Previous " "execution may not have shutdown cleanly. To fix problem, stop all " "rocm_smi programs, and then delete the rocm_smi* shared memory files in" diff --git a/projects/rocm-smi-lib/tests/rocm_smi_test/functional/evt_notif_read_write.cc b/projects/rocm-smi-lib/tests/rocm_smi_test/functional/evt_notif_read_write.cc index 19f3c6c42b..520bd5ab05 100755 --- a/projects/rocm-smi-lib/tests/rocm_smi_test/functional/evt_notif_read_write.cc +++ b/projects/rocm-smi-lib/tests/rocm_smi_test/functional/evt_notif_read_write.cc @@ -88,6 +88,10 @@ void TestEvtNotifReadWrite::Run(void) { uint32_t dv_ind; TestBase::Run(); + if (num_monitor_devs() == 0) { + return; + } + if (setup_failed_) { IF_VERB(STANDARD) { std::cout << "** SetUp Failed for this test. Skipping.**" << diff --git a/projects/rocm-smi-lib/tests/rocm_smi_test/functional/mutual_exclusion.cc b/projects/rocm-smi-lib/tests/rocm_smi_test/functional/mutual_exclusion.cc index 237ec0f35a..8bb3c80dd7 100755 --- a/projects/rocm-smi-lib/tests/rocm_smi_test/functional/mutual_exclusion.cc +++ b/projects/rocm-smi-lib/tests/rocm_smi_test/functional/mutual_exclusion.cc @@ -91,6 +91,9 @@ void TestMutualExclusion::SetUp(void) { // RSMI_INIT_FLAG_RESRV_TEST1 tells rsmi to fail immediately // if it can't get the mutex instead of waiting. ret = rsmi_init(RSMI_INIT_FLAG_RESRV_TEST1); + if (ret != RSMI_STATUS_SUCCESS) { + setup_failed_ = true; + } ASSERT_EQ(ret, RSMI_STATUS_SUCCESS); sleep(2); // Let both processes get through rsmi_init @@ -98,10 +101,26 @@ void TestMutualExclusion::SetUp(void) { sleep(1); // Let the sleeper process get through rsmi_init() before // this one goes, so it doesn't fail. ret = rsmi_init(RSMI_INIT_FLAG_RESRV_TEST1); + if (ret != RSMI_STATUS_SUCCESS) { + setup_failed_ = true; + } ASSERT_EQ(ret, RSMI_STATUS_SUCCESS); sleep(2); // Let both processes get through rsmi_init; } + + ret = rsmi_num_monitor_devices(&num_monitor_devs_); + if (ret != RSMI_STATUS_SUCCESS) { + setup_failed_ = true; + } + ASSERT_EQ(ret, RSMI_STATUS_SUCCESS); + + if (num_monitor_devs_ == 0) { + std::cout << "No monitor devices found on this machine." << std::endl; + std::cout << "No ROCm SMI tests can be run." << std::endl; + setup_failed_ = true; + } + return; } @@ -130,6 +149,11 @@ rsmi_test_sleep(uint32_t dv_ind, uint32_t seconds); void TestMutualExclusion::Run(void) { rsmi_status_t ret; + if (setup_failed_) { + std::cout << "** SetUp Failed for this test. Skipping.**" << std::endl; + return; + } + if (sleeper_process_) { IF_VERB(STANDARD) { std::cout << "MUTEX_HOLDER process: started sleeping for 10 seconds..." << @@ -168,66 +192,74 @@ void TestMutualExclusion::Run(void) { rsmi_error_count_t dmy_err_cnt; rsmi_ras_err_state_t dmy_ras_err_st; + // This can be replaced with ASSERT_EQ() once env. stabilizes +#define CHECK_RET(A, B) { \ + if ((A) != (B)) { \ + std::cout << "Expected return value of " << B << \ + " but got " << A << std::endl; \ + std::cout << "at " << __FILE__ << ":" << __LINE__ << std::endl; \ + } \ +} ret = rsmi_dev_id_get(0, &dmy_ui16); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_vendor_id_get(0, &dmy_ui16); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_name_get(0, dmy_str, 10); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_brand_get(0, dmy_str, 10); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_vendor_name_get(0, dmy_str, 10); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_vram_vendor_get(0, dmy_str, 10); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_serial_number_get(0, dmy_str, 10); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_subsystem_id_get(0, &dmy_ui16); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_subsystem_vendor_id_get(0, &dmy_ui16); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_unique_id_get(0, &dmy_ui64); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_pci_id_get(0, &dmy_ui64); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_pci_throughput_get(0, &dmy_ui64, &dmy_ui64, &dmy_ui64); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_pci_replay_counter_get(0, &dmy_ui64); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_pci_bandwidth_set(0, 0); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_fan_rpms_get(0, dmy_ui32, &dmy_i64); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_fan_speed_get(0, 0, &dmy_i64); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_fan_speed_max_get(0, 0, &dmy_ui64); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_temp_metric_get(0, dmy_ui32, RSMI_TEMP_CURRENT, &dmy_i64); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_fan_reset(0, 0); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_fan_speed_set(0, dmy_ui32, 0); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_perf_level_get(0, &dmy_perf_lvl); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_overdrive_level_get(0, &dmy_ui32); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_gpu_clk_freq_get(0, RSMI_CLK_TYPE_SYS, &dmy_freqs); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_od_volt_info_get(0, &dmy_od_volt); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_od_volt_curve_regions_get(0, &dmy_ui32, &dmy_vlt_reg); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_overdrive_level_set(dmy_i32, 0); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_gpu_clk_freq_set(0, RSMI_CLK_TYPE_SYS, 0); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_ecc_count_get(0, RSMI_GPU_BLOCK_UMC, &dmy_err_cnt); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_ecc_enabled_get(0, &dmy_ui64); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); ret = rsmi_dev_ecc_status_get(0, RSMI_GPU_BLOCK_UMC, &dmy_ras_err_st); - ASSERT_EQ(ret, RSMI_STATUS_BUSY); + CHECK_RET(ret, RSMI_STATUS_BUSY); IF_VERB(STANDARD) { std::cout << "TESTER process: Finished verifying that all " diff --git a/projects/rocm-smi-lib/tests/rocm_smi_test/test_base.h b/projects/rocm-smi-lib/tests/rocm_smi_test/test_base.h index f343b17c00..9e07bf3873 100755 --- a/projects/rocm-smi-lib/tests/rocm_smi_test/test_base.h +++ b/projects/rocm-smi-lib/tests/rocm_smi_test/test_base.h @@ -118,9 +118,9 @@ class TestBase { void MakeHeaderStr(const char *inStr, std::string *outStr) const; void PrintDeviceHeader(uint32_t dv_ind); bool setup_failed_; ///< Record that setup failed to return ierr in Run + uint32_t num_monitor_devs_; ///< Number of monitor devices found private: - uint32_t num_monitor_devs_; ///< Number of monitor devices found std::string description_; std::string title_; ///< Displayed title of test uint32_t verbosity_; ///< How much additional output to produce