From 03b519114e22cd91d2641cadfddf2024d1c2710d Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Fri, 14 Sep 2018 11:04:00 -0500 Subject: [PATCH] common/Mutex: kill mutex_perf_counter This has a measurable overhead even when turned off, and we do not use it. Signed-off-by: Sage Weil --- src/common/Mutex.cc | 40 ++---------------------- src/common/Mutex.h | 14 +-------- src/common/legacy_config_opts.h | 1 - src/common/mutex_debug.cc | 24 ++------------ src/common/mutex_debug.h | 16 ++++------ src/common/options.cc | 4 --- src/common/shared_mutex_debug.cc | 3 +- src/os/filestore/FileJournal.h | 8 ++--- src/os/filestore/JournalingObjectStore.h | 6 ++-- src/os/filestore/WBThrottle.cc | 2 +- src/osd/OSD.h | 4 +-- 11 files changed, 23 insertions(+), 99 deletions(-) diff --git a/src/common/Mutex.cc b/src/common/Mutex.cc index 4c449a06c4d61..a4ddd1e4e8032 100644 --- a/src/common/Mutex.cc +++ b/src/common/Mutex.cc @@ -19,22 +19,13 @@ #include "common/valgrind.h" Mutex::Mutex(const std::string &n, bool r, bool ld, - bool bt, - CephContext *cct) : + bool bt) : name(n), id(-1), recursive(r), lockdep(ld), backtrace(bt), nlock(0), - locked_by(0), cct(cct), logger(0) + locked_by(0) { ANNOTATE_BENIGN_RACE_SIZED(&id, sizeof(id), "Mutex lockdep id"); ANNOTATE_BENIGN_RACE_SIZED(&nlock, sizeof(nlock), "Mutex nlock"); ANNOTATE_BENIGN_RACE_SIZED(&locked_by, sizeof(locked_by), "Mutex locked_by"); - if (cct) { - PerfCountersBuilder b(cct, string("mutex-") + name, - l_mutex_first, l_mutex_last); - b.add_time_avg(l_mutex_wait, "wait", "Average time of mutex in locked state"); - logger = b.create_perf_counters(); - cct->get_perfcounters_collection()->add(logger); - logger->set(l_mutex_wait, 0); - } if (recursive) { // Mutexes of type PTHREAD_MUTEX_RECURSIVE do all the same checks as // mutexes of type PTHREAD_MUTEX_ERRORCHECK. @@ -77,42 +68,17 @@ Mutex::~Mutex() { ANNOTATE_BENIGN_RACE_SIZED(&_m, sizeof(_m), "Mutex primitive"); pthread_mutex_destroy(&_m); - if (cct && logger) { - cct->get_perfcounters_collection()->remove(logger); - delete logger; - } if (lockdep && g_lockdep) { lockdep_unregister(id); } } void Mutex::Lock(bool no_lockdep) { - int r; - if (lockdep && g_lockdep && !no_lockdep && !recursive) _will_lock(); - - if (logger && cct && cct->_conf->mutex_perf_counter) { - utime_t start; - // instrumented mutex enabled - start = ceph_clock_now(); - if (TryLock()) { - goto out; - } - - r = pthread_mutex_lock(&_m); - - logger->tinc(l_mutex_wait, - ceph_clock_now() - start); - } else { - r = pthread_mutex_lock(&_m); - } - + int r = pthread_mutex_lock(&_m); ceph_assert(r == 0); if (lockdep && g_lockdep) _locked(); _post_lock(); - -out: - ; } void Mutex::Unlock() { diff --git a/src/common/Mutex.h b/src/common/Mutex.h index 2784319295f56..10130d9a4693a 100644 --- a/src/common/Mutex.h +++ b/src/common/Mutex.h @@ -23,15 +23,6 @@ using namespace ceph; -class CephContext; -class PerfCounters; - -enum { - l_mutex_first = 999082, - l_mutex_wait, - l_mutex_last -}; - class Mutex { private: std::string name; @@ -43,8 +34,6 @@ private: pthread_mutex_t _m; int nlock; pthread_t locked_by; - CephContext *cct; - PerfCounters *logger; // don't allow copying. void operator=(const Mutex &M); @@ -64,8 +53,7 @@ private: } public: - Mutex(const std::string &n, bool r = false, bool ld=true, bool bt=false, - CephContext *cct = 0); + Mutex(const std::string &n, bool r = false, bool ld=true, bool bt=false); ~Mutex(); bool is_locked() const { return (nlock > 0); diff --git a/src/common/legacy_config_opts.h b/src/common/legacy_config_opts.h index b8fb6cb001bde..dd9cf3e2fdcf3 100644 --- a/src/common/legacy_config_opts.h +++ b/src/common/legacy_config_opts.h @@ -1514,7 +1514,6 @@ OPTION(rgw_list_bucket_min_readahead, OPT_INT) // minimum number of entries to r OPTION(rgw_rest_getusage_op_compat, OPT_BOOL) // dump description of total stats for s3 GetUsage API -OPTION(mutex_perf_counter, OPT_BOOL) // enable/disable mutex perf counter OPTION(throttler_perf_counter, OPT_BOOL) // enable/disable throttler perf counter /* The following are tunables for torrent data */ diff --git a/src/common/mutex_debug.cc b/src/common/mutex_debug.cc index 0024a25ae3492..a7b2bb697a0fd 100644 --- a/src/common/mutex_debug.cc +++ b/src/common/mutex_debug.cc @@ -25,10 +25,8 @@ enum { l_mutex_last }; -mutex_debugging_base::mutex_debugging_base(const std::string &n, bool bt, - CephContext *cct) : - id(-1), backtrace(bt), nlock(0), locked_by(thread::id()), - cct(cct), logger(0) { +mutex_debugging_base::mutex_debugging_base(const std::string &n, bool bt) : + id(-1), backtrace(bt), nlock(0), locked_by(thread::id()) { if (n.empty()) { uuid_d uu; uu.generate_random(); @@ -36,25 +34,12 @@ mutex_debugging_base::mutex_debugging_base(const std::string &n, bool bt, } else { name = n; } - if (cct) { - PerfCountersBuilder b(cct, string("mutex-") + name, - l_mutex_first, l_mutex_last); - b.add_time_avg(l_mutex_wait, "wait", - "Average time of mutex in locked state"); - logger = b.create_perf_counters(); - cct->get_perfcounters_collection()->add(logger); - logger->set(l_mutex_wait, 0); - } if (g_lockdep) _register(); } mutex_debugging_base::~mutex_debugging_base() { ceph_assert(nlock == 0); - if (cct && logger) { - cct->get_perfcounters_collection()->remove(logger); - delete logger; - } if (g_lockdep) { lockdep_unregister(id); } @@ -74,16 +59,11 @@ void mutex_debugging_base::_will_unlock() { // about to unlock } ceph::mono_time mutex_debugging_base::before_lock_blocks() { - if (logger && cct && cct->_conf->mutex_perf_counter) - return ceph::mono_clock::now(); return ceph::mono_time::min(); } void mutex_debugging_base::after_lock_blocks(ceph::mono_time start, bool no_lockdep) { - if (logger && cct && cct->_conf->mutex_perf_counter) - logger->tinc(l_mutex_wait, - ceph::mono_clock::now() - start); if (!no_lockdep && g_lockdep) _locked(); } diff --git a/src/common/mutex_debug.h b/src/common/mutex_debug.h index d4a60f7181bf5..9ec6aae2c2ba8 100644 --- a/src/common/mutex_debug.h +++ b/src/common/mutex_debug.h @@ -39,16 +39,14 @@ protected: int nlock; std::thread::id locked_by; - CephContext *cct; - PerfCounters *logger; + void _register(); void _will_lock(); // about to lock void _locked(); // just locked void _will_unlock(); // about to unlock - mutex_debugging_base(const std::string &n = std::string(), bool bt = false, - CephContext *cct = nullptr); + mutex_debugging_base(const std::string &n = std::string(), bool bt = false); ~mutex_debugging_base(); ceph::mono_time before_lock_blocks(); @@ -72,9 +70,8 @@ class mutex_debugging : public mutex_debugging_base { Mutex* impl; public: - mutex_debugging(const std::string &n = std::string(), bool bt = false, - CephContext *cct = nullptr) : - mutex_debugging_base(n, bt, cct), impl(static_cast(this)) {} + mutex_debugging(const std::string &n = std::string(), bool bt = false) : + mutex_debugging_base(n, bt), impl(static_cast(this)) {} ~mutex_debugging() = default; @@ -136,9 +133,8 @@ public: static constexpr bool recursive = Recursive; // Mutex concept is DefaultConstructible - mutex_debug_impl(const std::string &n = std::string(), bool bt = false, - CephContext *cct = nullptr) : - mutex_debugging >(n, bt, cct) { + mutex_debug_impl(const std::string &n = std::string(), bool bt = false) : + mutex_debugging >(n, bt) { pthread_mutexattr_t a; pthread_mutexattr_init(&a); int r; diff --git a/src/common/options.cc b/src/common/options.cc index f5a05999bef06..3224438205578 100644 --- a/src/common/options.cc +++ b/src/common/options.cc @@ -4954,10 +4954,6 @@ std::vector