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: e099c84640]
2022-07-11 20:59:57 -05:00
@@ -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<uint64_t>::max()) return;
-    update();
-}
-
-void
+bool
 mpi_gotcha::update()
 {
    auto_lock_t _lk{ type_mutex<mpi_gotcha>(), 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<int>("OMNITRACE_MPI_MAX_COMM_UPDATES", 4);
+                if(update() && ++_num_updates >= _disable_after) disable_comm_intercept();
+            }
        }
    }
    omnitrace_pop_trace_hidden(_data.tool_id.c_str());
@@ -43,6 +43,7 @@ struct mpi_gotcha : comp::base<mpi_gotcha, void>

    // 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<mpi_gotcha, void>

    // 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<uintptr_t>::max(); }
+    static void      disable_comm_intercept();

 private:
    int       m_rank     = 0;
@@ -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<size_t>{};
+        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" });
@@ -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<tim::tsettings<bool>&>(*_v->second).get();
 }

+bool
+get_trace_thread_rwlocks()
+{
+    static auto _v = get_config()->find("OMNITRACE_TRACE_THREAD_RW_LOCKS");
+    return static_cast<tim::tsettings<bool>&>(*_v->second).get();
+}
+
 bool
 get_debug_tid()
 {
@@ -273,6 +273,9 @@ get_critical_trace_per_row();

 bool
 get_trace_thread_locks();
+
+bool
+get_trace_thread_rwlocks();
 }  // namespace config

 //