From 49f9927e038f0337c18a2ed886c5ae76bae915da Mon Sep 17 00:00:00 2001 From: "Jonathan R. Madsen" Date: Mon, 11 Jul 2022 20:59:57 -0500 Subject: [PATCH] pthread_rwlock deadlock fix (#82) - found when using ROCm-enabled OpenMPI with rocHPL - when wrapping pthread_rwlock_rdlock, pthread_rwlock_wrlock, and pthread_rwlock_unlock, omnitrace has been found to deadlock for some unknown reason - New configuration variable: OMNITRACE_TRACE_THREAD_RW_LOCKS which defaults to false [ROCm/rocprofiler-systems commit: e099c846406cb79d62281527374b678cb06482cf] --- .../library/components/mpi_gotcha.cpp | 34 ++++++----- .../library/components/mpi_gotcha.hpp | 6 +- .../components/pthread_mutex_gotcha.cpp | 57 ++++++++++++------- .../source/lib/omnitrace/library/config.cpp | 14 ++++- .../source/lib/omnitrace/library/config.hpp | 3 + 5 files changed, 79 insertions(+), 35 deletions(-) diff --git a/projects/rocprofiler-systems/source/lib/omnitrace/library/components/mpi_gotcha.cpp b/projects/rocprofiler-systems/source/lib/omnitrace/library/components/mpi_gotcha.cpp index 0e94749b97..bd4eebc4ca 100644 --- a/projects/rocprofiler-systems/source/lib/omnitrace/library/components/mpi_gotcha.cpp +++ b/projects/rocprofiler-systems/source/lib/omnitrace/library/components/mpi_gotcha.cpp @@ -137,15 +137,7 @@ mpi_gotcha::configure() }; } -void -mpi_gotcha::stop() -{ - OMNITRACE_BASIC_VERBOSE(0, "[pid=%i] Stopping MPI gotcha...\n", process::get_id()); - if(mpip_index == std::numeric_limits::max()) return; - update(); -} - -void +bool mpi_gotcha::update() { auto_lock_t _lk{ type_mutex(), std::defer_lock }; @@ -174,13 +166,23 @@ mpi_gotcha::update() tim::settings::default_process_suffix() = _rank; get_perfetto_output_filename().clear(); - OMNITRACE_BASIC_VERBOSE(0, "[pid=%i] MPI rank: %i (%i)\n", process::get_id(), - tim::mpi::rank(), _rank); - OMNITRACE_BASIC_VERBOSE(0, "[pid=%i] MPI size: %i (%i)\n", process::get_id(), + OMNITRACE_BASIC_VERBOSE(0, "[pid=%i] MPI rank: %i (%i), MPI size: %i (%i)\n", + process::get_id(), tim::mpi::rank(), _rank, tim::mpi::size(), _size); last_comm_record = _rank_data; config::get_use_pid() = true; + return true; } + return false; +} + +void +mpi_gotcha::disable_comm_intercept() +{ +#if defined(OMNITRACE_USE_MPI_HEADERS) && OMNITRACE_USE_MPI_HEADERS > 0 + mpi_gotcha_t::revert<3>(); + mpi_gotcha_t::revert<4>(); +#endif } void @@ -325,7 +327,13 @@ mpi_gotcha::audit(const gotcha_data_t& _data, audit::outgoing, int _retval) _data.tool_id.c_str(), (int) _retval); } - if(_comm_entry.updated()) update(); + if(_comm_entry.updated()) + { + static thread_local int _num_updates = 0; + static int _disable_after = + tim::get_env("OMNITRACE_MPI_MAX_COMM_UPDATES", 4); + if(update() && ++_num_updates >= _disable_after) disable_comm_intercept(); + } } } omnitrace_pop_trace_hidden(_data.tool_id.c_str()); diff --git a/projects/rocprofiler-systems/source/lib/omnitrace/library/components/mpi_gotcha.hpp b/projects/rocprofiler-systems/source/lib/omnitrace/library/components/mpi_gotcha.hpp index 5fc01f3a4f..4b7cae7d2d 100644 --- a/projects/rocprofiler-systems/source/lib/omnitrace/library/components/mpi_gotcha.hpp +++ b/projects/rocprofiler-systems/source/lib/omnitrace/library/components/mpi_gotcha.hpp @@ -43,6 +43,7 @@ struct mpi_gotcha : comp::base // generate the gotcha wrappers static void configure(); + static void shutdown(); // called right before MPI_Init with that functions arguments static void audit(const gotcha_data_t& _data, audit::incoming, int*, char***); @@ -62,10 +63,11 @@ struct mpi_gotcha : comp::base // without these you will get a verbosity level 1 warning static void start() {} - static void stop(); + static void stop() {} - static void update(); + static bool update(); static uintptr_t null_comm() { return std::numeric_limits::max(); } + static void disable_comm_intercept(); private: int m_rank = 0; diff --git a/projects/rocprofiler-systems/source/lib/omnitrace/library/components/pthread_mutex_gotcha.cpp b/projects/rocprofiler-systems/source/lib/omnitrace/library/components/pthread_mutex_gotcha.cpp index b65e06fa5f..872eb67896 100644 --- a/projects/rocprofiler-systems/source/lib/omnitrace/library/components/pthread_mutex_gotcha.cpp +++ b/projects/rocprofiler-systems/source/lib/omnitrace/library/components/pthread_mutex_gotcha.cpp @@ -51,16 +51,27 @@ pthread_mutex_gotcha::get_hashes() // fully populated. If that fails to be the case for some reason, // we could see weird results. static auto _v = []() { - const auto& _data = pthread_mutex_gotcha_t::get_gotcha_data(); - hash_array_t _init = {}; - size_t i0 = (config::get_trace_thread_locks()) ? 0 : 3; - for(size_t i = i0; i < gotcha_capacity; ++i) + const auto& _data = pthread_mutex_gotcha_t::get_gotcha_data(); + auto _init = hash_array_t{}; + auto _skip = std::set{}; + if(!config::get_trace_thread_locks()) + { + for(size_t i = 0; i < 3; ++i) + _skip.emplace(i); + } + if(!config::get_trace_thread_rwlocks()) + { + for(size_t i = 3; i < 8; ++i) + _skip.emplace(i); + } + for(size_t i = 0; i < gotcha_capacity; ++i) { auto&& _id = _data.at(i).tool_id; if(!_id.empty()) _init.at(i) = critical_trace::add_hash_id(_id.c_str()); else { + if(_skip.count(i) > 0) continue; OMNITRACE_VERBOSE( 1, "WARNING!!! pthread_mutex_gotcha tool id at index %zu was empty!\n", @@ -93,23 +104,31 @@ pthread_mutex_gotcha::configure() comp::gotcha_config<2, int, pthread_mutex_t*>{ "pthread_mutex_trylock" }); } - pthread_mutex_gotcha_t::configure( - comp::gotcha_config<3, int, pthread_barrier_t*>{ "pthread_barrier_wait" }); + if(config::get_trace_thread_rwlocks()) + { + pthread_mutex_gotcha_t::configure( + comp::gotcha_config<3, int, pthread_rwlock_t*>{ + "pthread_rwlock_rdlock" }); + + pthread_mutex_gotcha_t::configure( + comp::gotcha_config<4, int, pthread_rwlock_t*>{ + "pthread_rwlock_wrlock" }); + + pthread_mutex_gotcha_t::configure( + comp::gotcha_config<5, int, pthread_rwlock_t*>{ + "pthread_rwlock_tryrdlock" }); + + pthread_mutex_gotcha_t::configure( + comp::gotcha_config<6, int, pthread_rwlock_t*>{ + "pthread_rwlock_trywrlock" }); + + pthread_mutex_gotcha_t::configure( + comp::gotcha_config<7, int, pthread_rwlock_t*>{ + "pthread_rwlock_unlock" }); + } pthread_mutex_gotcha_t::configure( - comp::gotcha_config<4, int, pthread_rwlock_t*>{ "pthread_rwlock_rdlock" }); - - pthread_mutex_gotcha_t::configure( - comp::gotcha_config<5, int, pthread_rwlock_t*>{ "pthread_rwlock_tryrdlock" }); - - pthread_mutex_gotcha_t::configure( - comp::gotcha_config<6, int, pthread_rwlock_t*>{ "pthread_rwlock_trywrlock" }); - - pthread_mutex_gotcha_t::configure( - comp::gotcha_config<7, int, pthread_rwlock_t*>{ "pthread_rwlock_unlock" }); - - pthread_mutex_gotcha_t::configure( - comp::gotcha_config<8, int, pthread_rwlock_t*>{ "pthread_rwlock_wrlock" }); + comp::gotcha_config<8, int, pthread_barrier_t*>{ "pthread_barrier_wait" }); pthread_mutex_gotcha_t::configure( comp::gotcha_config<9, int, pthread_spinlock_t*>{ "pthread_spin_lock" }); diff --git a/projects/rocprofiler-systems/source/lib/omnitrace/library/config.cpp b/projects/rocprofiler-systems/source/lib/omnitrace/library/config.cpp index d2d00f7db7..6f686e2222 100644 --- a/projects/rocprofiler-systems/source/lib/omnitrace/library/config.cpp +++ b/projects/rocprofiler-systems/source/lib/omnitrace/library/config.cpp @@ -303,10 +303,15 @@ configure_settings(bool _init) "critical_trace"); OMNITRACE_CONFIG_SETTING(bool, "OMNITRACE_TRACE_THREAD_LOCKS", - "Enable tracking calls to pthread_mutex_lock, " + "Enable tracing calls to pthread_mutex_lock, " "pthread_mutex_unlock, pthread_mutex_trylock", false, "backend", "parallelism", "gotcha"); + OMNITRACE_CONFIG_SETTING(bool, "OMNITRACE_TRACE_THREAD_RW_LOCKS", + "Enable tracing calls to pthread_rwlock_* functions. May " + "cause deadlocks with ROCm-enabled OpenMPI.", + false, "backend", "parallelism", "gotcha"); + OMNITRACE_CONFIG_SETTING(bool, "OMNITRACE_FLAT_SAMPLING", "Ignore hierarchy in all statistical sampling entries", _config->get_flat_profile(), "timemory", "sampling", @@ -1437,6 +1442,13 @@ get_trace_thread_locks() return static_cast&>(*_v->second).get(); } +bool +get_trace_thread_rwlocks() +{ + static auto _v = get_config()->find("OMNITRACE_TRACE_THREAD_RW_LOCKS"); + return static_cast&>(*_v->second).get(); +} + bool get_debug_tid() { diff --git a/projects/rocprofiler-systems/source/lib/omnitrace/library/config.hpp b/projects/rocprofiler-systems/source/lib/omnitrace/library/config.hpp index 8479cf264a..c33a331aca 100644 --- a/projects/rocprofiler-systems/source/lib/omnitrace/library/config.hpp +++ b/projects/rocprofiler-systems/source/lib/omnitrace/library/config.hpp @@ -273,6 +273,9 @@ get_critical_trace_per_row(); bool get_trace_thread_locks(); + +bool +get_trace_thread_rwlocks(); } // namespace config //