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 <sage@redhat.com>

diff --git a/src/common/Mutex.cc b/src/common/Mutex.cc
index 4c449a06c4d61b20270c14ced70b6bc07b23a9bd..a4ddd1e4e8032eaf851b6c2883e08961a95e42da 100644 (file)
--- 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 2784319295f56dbba0a06b6ed128c7bc97ebeae2..10130d9a4693a5e8ef448918ca229ba63282be6d 100644 (file)
--- 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 b8fb6cb001bdec2f9ceff8f859a50b1363e8951d..dd9cf3e2fdcf3bfd885fde6259858f267492c669 100644 (file)
--- 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 0024a25ae3492a3ae78d38cf8a0962a1ecf31a17..a7b2bb697a0fd39f8602e3ee8b5b5f38f1a10b74 100644 (file)
--- 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 d4a60f7181bf58f433ab362072465f5506b44c45..9ec6aae2c2ba824bc26cdbaab903fd812a81ed79 100644 (file)
--- 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<Mutex*>(this)) {}
+  mutex_debugging(const std::string &n = std::string(), bool bt = false) :
+    mutex_debugging_base(n, bt), impl(static_cast<Mutex*>(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<mutex_debug_impl<Recursive> >(n, bt, cct) {
+  mutex_debug_impl(const std::string &n = std::string(), bool bt = false) :
+    mutex_debugging<mutex_debug_impl<Recursive> >(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 f5a05999bef06605d73f7265a7f823a84b170690..32244382055789e52ed5b51993e15f32306aafb6 100644 (file)
--- a/src/common/options.cc
+++ b/src/common/options.cc
@@ -4954,10 +4954,6 @@ std::vector<Option> get_global_options() {
                      "manager before this is reported as an ERR rather than "
                      "a WARN"),
 
-    Option("mutex_perf_counter", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
-    .set_default(false)
-    .set_description(""),
-
     Option("throttler_perf_counter", Option::TYPE_BOOL, Option::LEVEL_ADVANCED)
     .set_default(true)
     .set_description(""),

diff --git a/src/common/shared_mutex_debug.cc b/src/common/shared_mutex_debug.cc
index 83ed3f5abe56a10c45e81b505449e7ce3cb79408..c0a031a42d596748fd87278acad8ac714064467d 100644 (file)
--- a/src/common/shared_mutex_debug.cc
+++ b/src/common/shared_mutex_debug.cc
@@ -11,8 +11,7 @@ shared_mutex_debug::shared_mutex_debug(const std::string& n,
                                        bool track_lock,
                                        bool enable_lock_dep,
                                        bool prioritize_write)
-  : mutex_debugging_base{n, false /* backtrace */,
-                         nullptr /* cct for perf counter*/},
+  : mutex_debugging_base{n, false /* backtrace */},
     track(track_lock),
     lockdep(enable_lock_dep)
 {

diff --git a/src/os/filestore/FileJournal.h b/src/os/filestore/FileJournal.h
index a9af1a64b41f2be94c5189cde3b5e6ddd3eea2dd..96f0243bb5b4a023a3da91524c5feab2c671d19e 100644 (file)
--- a/src/os/filestore/FileJournal.h
+++ b/src/os/filestore/FileJournal.h
@@ -399,12 +399,12 @@ private:
   FileJournal(CephContext* cct, uuid_d fsid, Finisher *fin, Cond *sync_cond,
              const char *f, bool dio=false, bool ai=true, bool faio=false) :
     Journal(cct, fsid, fin, sync_cond),
-    finisher_lock("FileJournal::finisher_lock", false, true, false, cct),
+    finisher_lock("FileJournal::finisher_lock", false, true, false),
     journaled_seq(0),
     plug_journal_completions(false),
-    writeq_lock("FileJournal::writeq_lock", false, true, false, cct),
+    writeq_lock("FileJournal::writeq_lock", false, true, false),
     completions_lock(
-      "FileJournal::completions_lock", false, true, false, cct),
+      "FileJournal::completions_lock", false, true, false),
     fn(f),
     zero_buf(NULL),
     max_size(0), block_size(0),
@@ -425,7 +425,7 @@ private:
     fd(-1),
     writing_seq(0),
     throttle(cct->_conf->filestore_caller_concurrency),
-    write_lock("FileJournal::write_lock", false, true, false, cct),
+    write_lock("FileJournal::write_lock", false, true, false),
     write_stop(true),
     aio_stop(true),
     write_thread(this),

diff --git a/src/os/filestore/JournalingObjectStore.h b/src/os/filestore/JournalingObjectStore.h
index c5dac774ae38fbe5c78964a962a5264b9997481d..a289d0e86c6137982ac7842d0c9af74659f86886 100644 (file)
--- a/src/os/filestore/JournalingObjectStore.h
+++ b/src/os/filestore/JournalingObjectStore.h
@@ -34,7 +34,7 @@ protected:
     uint64_t op_submitted;
   public:
     SubmitManager(CephContext* cct) :
-      cct(cct), lock("JOS::SubmitManager::lock", false, true, false, cct),
+      cct(cct), lock("JOS::SubmitManager::lock", false, true, false),
       op_seq(0), op_submitted(0)
     {}
     uint64_t op_submit_start();
@@ -66,11 +66,11 @@ protected:
   public:
     ApplyManager(CephContext* cct, Journal *&j, Finisher &f) :
       cct(cct), journal(j), finisher(f),
-      apply_lock("JOS::ApplyManager::apply_lock", false, true, false, cct),
+      apply_lock("JOS::ApplyManager::apply_lock", false, true, false),
       blocked(false),
       open_ops(0),
       max_applied_seq(0),
-      com_lock("JOS::ApplyManager::com_lock", false, true, false, cct),
+      com_lock("JOS::ApplyManager::com_lock", false, true, false),
       committing_seq(0), committed_seq(0) {}
     void reset() {
       ceph_assert(open_ops == 0);

diff --git a/src/os/filestore/WBThrottle.cc b/src/os/filestore/WBThrottle.cc
index 4ce5256ab7d17ca4b6f5ffe10847d491a5b08c48..121ba7bc6a91f34f0253177f2e20320de93856b0 100644 (file)
--- a/src/os/filestore/WBThrottle.cc
+++ b/src/os/filestore/WBThrottle.cc
@@ -11,7 +11,7 @@ WBThrottle::WBThrottle(CephContext *cct) :
   cct(cct),
   logger(NULL),
   stopping(true),
-  lock("WBThrottle::lock", false, true, false, cct),
+  lock("WBThrottle::lock", false, true, false),
   fs(XFS)
 {
   {

diff --git a/src/osd/OSD.h b/src/osd/OSD.h
index 8039d9f69cbe625be7594a4b68562808fae56ea7..3ad1d9ec5e01152e55df4d3071de4e024eb6a978 100644 (file)
--- a/src/osd/OSD.h
+++ b/src/osd/OSD.h
@@ -1221,11 +1221,11 @@ struct OSDShard {
       osd(osd),
       shard_name(string("OSDShard.") + stringify(id)),
       sdata_wait_lock_name(shard_name + "::sdata_wait_lock"),
-      sdata_wait_lock(sdata_wait_lock_name.c_str(), false, true, false, cct),
+      sdata_wait_lock(sdata_wait_lock_name.c_str(), false, true, false),
       osdmap_lock_name(shard_name + "::osdmap_lock"),
       osdmap_lock(osdmap_lock_name.c_str(), false, false),
       shard_lock_name(shard_name + "::shard_lock"),
-      shard_lock(shard_lock_name.c_str(), false, true, false, cct),
+      shard_lock(shard_lock_name.c_str(), false, true, false),
       context_queue(sdata_wait_lock, sdata_cond) {
     if (opqueue == io_queue::weightedpriority) {
       pqueue = std::make_unique<