common: move lock out of PerfCounterCollection and rename it

extract PerfCounterCollectionImpl and move m_lock out
from perfcounters.cc to upper,then seastar osd can reuse
functions in perfcounters.cc.

Signed-off-by: chunmei Liu <chunmei.liu@intel.com>

diff --git a/src/common/CMakeLists.txt b/src/common/CMakeLists.txt
index 06b251a26e1bbcf70c338c7215e2250b8bd3fb79..9e1e942d1c425514a2ff89e99b07ca852632aacf 100644 (file)
--- a/src/common/CMakeLists.txt
+++ b/src/common/CMakeLists.txt
@@ -77,6 +77,7 @@ set(common_srcs
   options.cc
   page.cc
   perf_counters.cc
+  perf_counters_collection.cc
   perf_histogram.cc
   pick_address.cc
   reverse.c

diff --git a/src/common/Mutex.cc b/src/common/Mutex.cc
index f5c38826b8ddb405002f9397073709b57d2f2183..e029adc752788b79aa1bad59b0ced076ae1ef0b0 100644 (file)
--- a/src/common/Mutex.cc
+++ b/src/common/Mutex.cc
@@ -13,7 +13,6 @@
  */
 
 #include "common/Mutex.h"
-#include "common/perf_counters.h"
 #include "common/config.h"
 #include "common/Clock.h"
 #include "common/valgrind.h"

diff --git a/src/common/Throttle.h b/src/common/Throttle.h
index 89b986ac4e0e85e1a5aa8871314338681ac00e90..933ba7426a3173fa0e70bb02036487d12f83f0dc 100644 (file)
--- a/src/common/Throttle.h
+++ b/src/common/Throttle.h
@@ -16,7 +16,7 @@
 #include "common/ThrottleInterface.h"
 #include "common/Timer.h"
 #include "common/convenience.h"
-#include "common/perf_counters.h"
+#include "common/perf_counters_collection.h"
 
 /**
  * @class Throttle

diff --git a/src/common/ceph_context.cc b/src/common/ceph_context.cc
index 81245039fcf0f62299b021d60e33c20817a947fc..f88b38e8590c2f4d1b203ceac360d2dcfdfafe3c 100644 (file)
--- a/src/common/ceph_context.cc
+++ b/src/common/ceph_context.cc
@@ -13,6 +13,8 @@
  *
  */
 
+#include "common/ceph_context.h"
+
 #include <mutex>
 #include <iostream>
 
@@ -22,7 +24,6 @@
 
 #include "include/mempool.h"
 #include "common/admin_socket.h"
-#include "common/perf_counters.h"
 #include "common/code_environment.h"
 #include "common/Cond.h"
 #include "common/config.h"

diff --git a/src/common/ceph_context.h b/src/common/ceph_context.h
index e2e91bb4e182484635835d4b6148e6ace5a7d80b..388e1db980956e3692504f8fc1d10e59383221c1 100644 (file)
--- a/src/common/ceph_context.h
+++ b/src/common/ceph_context.h
@@ -35,14 +35,13 @@
 #include "common/config_proxy.h"
 #include "include/spinlock.h"
 #endif
+#include "common/perf_counters_collection.h"
 
 
 #include "crush/CrushLocation.h"
 
 class AdminSocket;
 class CephContextServiceThread;
-class PerfCountersCollection;
-class PerfCounters;
 class CephContextHook;
 class CephContextObs;
 class CryptoHandler;
@@ -70,7 +69,6 @@ public:
   CryptoRandom* random() const;
   PerfCountersCollection* get_perfcounters_collection();
   ceph::common::ConfigProxy& _conf;
-
   CephContext* get();
   void put();
 private:

diff --git a/src/common/perf_counters.cc b/src/common/perf_counters.cc
index dc7050fe4b93ec2f34a7d97a8fdaa2c9a593392e..7d64450546a109afd315355d69c93f21c8158b2e 100644 (file)
--- a/src/common/perf_counters.cc
+++ b/src/common/perf_counters.cc
@@ -19,21 +19,17 @@
 
 using std::ostringstream;
 
-PerfCountersCollection::PerfCountersCollection(CephContext *cct)
-  : m_cct(cct),
-    m_lock("PerfCountersCollection")
+PerfCountersCollectionImpl::PerfCountersCollectionImpl()
 {
 }
 
-PerfCountersCollection::~PerfCountersCollection()
+PerfCountersCollectionImpl::~PerfCountersCollectionImpl()
 {
   clear();
 }
 
-void PerfCountersCollection::add(class PerfCounters *l)
+void PerfCountersCollectionImpl::add(PerfCounters *l)
 {
-  std::lock_guard<Mutex> lck(m_lock);
-
   // make sure the name is unique
   perf_counters_set_t::iterator i;
   i = m_loggers.find(l);
@@ -57,10 +53,8 @@ void PerfCountersCollection::add(class PerfCounters *l)
   }
 }
 
-void PerfCountersCollection::remove(class PerfCounters *l)
+void PerfCountersCollectionImpl::remove(PerfCounters *l)
 {
-  std::lock_guard<Mutex> lck(m_lock);
-
   for (unsigned int i = 0; i < l->m_data.size(); ++i) {
     PerfCounters::perf_counter_data_any_d &data = l->m_data[i];
 
@@ -76,22 +70,21 @@ void PerfCountersCollection::remove(class PerfCounters *l)
   m_loggers.erase(i);
 }
 
-void PerfCountersCollection::clear()
+void PerfCountersCollectionImpl::clear()
 {
-  std::lock_guard<Mutex> lck(m_lock);
   perf_counters_set_t::iterator i = m_loggers.begin();
   perf_counters_set_t::iterator i_end = m_loggers.end();
   for (; i != i_end; ) {
+    delete *i;
     m_loggers.erase(i++);
   }
 
   by_path.clear();
 }
 
-bool PerfCountersCollection::reset(const std::string &name)
+bool PerfCountersCollectionImpl::reset(const std::string &name)
 {
   bool result = false;
-  std::lock_guard<Mutex> lck(m_lock);
   perf_counters_set_t::iterator i = m_loggers.begin();
   perf_counters_set_t::iterator i_end = m_loggers.end();
 
@@ -128,14 +121,13 @@ bool PerfCountersCollection::reset(const std::string &name)
  * @param histograms if true, dump histogram values,
  *                   if false dump all non-histogram counters
  */
-void PerfCountersCollection::dump_formatted_generic(
+void PerfCountersCollectionImpl::dump_formatted_generic(
     Formatter *f,
     bool schema,
     bool histograms,
     const std::string &logger,
     const std::string &counter)
 {
-  std::lock_guard<Mutex> lck(m_lock);
   f->open_object_section("perfcounter_collection");
   
   for (perf_counters_set_t::iterator l = m_loggers.begin();
@@ -148,11 +140,9 @@ void PerfCountersCollection::dump_formatted_generic(
   f->close_section();
 }
 
-void PerfCountersCollection::with_counters(std::function<void(
-      const PerfCountersCollection::CounterMap &)> fn) const
+void PerfCountersCollectionImpl::with_counters(std::function<void(
+      const PerfCountersCollectionImpl::CounterMap &)> fn) const
 {
-  std::lock_guard<Mutex> lck(m_lock);
-
   fn(by_path);
 }
 
@@ -164,8 +154,10 @@ PerfCounters::~PerfCounters()
 
 void PerfCounters::inc(int idx, uint64_t amt)
 {
+#ifndef WITH_SEASTAR
   if (!m_cct->_conf->perf)
     return;
+#endif
 
   ceph_assert(idx > m_lower_bound);
   ceph_assert(idx < m_upper_bound);
@@ -183,8 +175,10 @@ void PerfCounters::inc(int idx, uint64_t amt)
 
 void PerfCounters::dec(int idx, uint64_t amt)
 {
+#ifndef WITH_SEASTAR
   if (!m_cct->_conf->perf)
     return;
+#endif
 
   ceph_assert(idx > m_lower_bound);
   ceph_assert(idx < m_upper_bound);
@@ -197,8 +191,10 @@ void PerfCounters::dec(int idx, uint64_t amt)
 
 void PerfCounters::set(int idx, uint64_t amt)
 {
+#ifndef WITH_SEASTAR
   if (!m_cct->_conf->perf)
     return;
+#endif
 
   ceph_assert(idx > m_lower_bound);
   ceph_assert(idx < m_upper_bound);
@@ -219,8 +215,10 @@ void PerfCounters::set(int idx, uint64_t amt)
 
 uint64_t PerfCounters::get(int idx) const
 {
+#ifndef WITH_SEASTAR
   if (!m_cct->_conf->perf)
     return 0;
+#endif
 
   ceph_assert(idx > m_lower_bound);
   ceph_assert(idx < m_upper_bound);
@@ -232,8 +230,10 @@ uint64_t PerfCounters::get(int idx) const
 
 void PerfCounters::tinc(int idx, utime_t amt)
 {
+#ifndef WITH_SEASTAR
   if (!m_cct->_conf->perf)
     return;
+#endif
 
   ceph_assert(idx > m_lower_bound);
   ceph_assert(idx < m_upper_bound);
@@ -251,8 +251,10 @@ void PerfCounters::tinc(int idx, utime_t amt)
 
 void PerfCounters::tinc(int idx, ceph::timespan amt)
 {
+#ifndef WITH_SEASTAR
   if (!m_cct->_conf->perf)
     return;
+#endif
 
   ceph_assert(idx > m_lower_bound);
   ceph_assert(idx < m_upper_bound);
@@ -270,8 +272,10 @@ void PerfCounters::tinc(int idx, ceph::timespan amt)
 
 void PerfCounters::tset(int idx, utime_t amt)
 {
+#ifndef WITH_SEASTAR
   if (!m_cct->_conf->perf)
     return;
+#endif
 
   ceph_assert(idx > m_lower_bound);
   ceph_assert(idx < m_upper_bound);
@@ -285,8 +289,10 @@ void PerfCounters::tset(int idx, utime_t amt)
 
 utime_t PerfCounters::tget(int idx) const
 {
+#ifndef WITH_SEASTAR
   if (!m_cct->_conf->perf)
     return utime_t();
+#endif
 
   ceph_assert(idx > m_lower_bound);
   ceph_assert(idx < m_upper_bound);
@@ -299,8 +305,10 @@ utime_t PerfCounters::tget(int idx) const
 
 void PerfCounters::hinc(int idx, int64_t x, int64_t y)
 {
+#ifndef WITH_SEASTAR
   if (!m_cct->_conf->perf)
     return;
+#endif
 
   ceph_assert(idx > m_lower_bound);
   ceph_assert(idx < m_upper_bound);
@@ -314,8 +322,10 @@ void PerfCounters::hinc(int idx, int64_t x, int64_t y)
 
 pair<uint64_t, uint64_t> PerfCounters::get_tavg_ns(int idx) const
 {
+#ifndef WITH_SEASTAR
   if (!m_cct->_conf->perf)
     return make_pair(0, 0);
+#endif
 
   ceph_assert(idx > m_lower_bound);
   ceph_assert(idx < m_upper_bound);
@@ -463,9 +473,13 @@ PerfCounters::PerfCounters(CephContext *cct, const std::string &name,
   : m_cct(cct),
     m_lower_bound(lower_bound),
     m_upper_bound(upper_bound),
+#ifndef WITH_SEASTAR
     m_name(name.c_str()),
     m_lock_name(std::string("PerfCounters::") + name.c_str()),
-    m_lock(m_lock_name.c_str())
+    m_lock(ceph::make_mutex(m_lock_name))
+#else
+    m_name(name.c_str())
+#endif
 {
   m_data.resize(upper_bound - lower_bound - 1);
 }

diff --git a/src/common/perf_counters.h b/src/common/perf_counters.h
index 57e1a157c57a22a005562a50134d63d5e6628569..8a2f62a3d3fb48ecf88d333325ae93649597157a 100644 (file)
--- a/src/common/perf_counters.h
+++ b/src/common/perf_counters.h
@@ -25,12 +25,12 @@
 
 #include "common/perf_histogram.h"
 #include "include/utime.h"
-#include "common/Mutex.h"
-#include "common/ceph_context.h"
+#include "common/ceph_mutex.h"
 #include "common/ceph_time.h"
 
 class CephContext;
 class PerfCountersBuilder;
+class PerfCounters;
 
 enum perfcounter_type_d : uint8_t
 {
@@ -282,17 +282,21 @@ private:
   int m_lower_bound;
   int m_upper_bound;
   std::string m_name;
+#ifndef WITH_SEASTAR
   const std::string m_lock_name;
+#endif
 
   int prio_adjust = 0;
 
+#ifndef WITH_SEASTAR
   /** Protects m_data */
-  mutable Mutex m_lock;
+  ceph::mutex m_lock;
+#endif
 
   perf_counter_data_vec_t m_data;
 
   friend class PerfCountersBuilder;
-  friend class PerfCountersCollection;
+  friend class PerfCountersCollectionImpl;
 };
 
 class SortPerfCountersByName {
@@ -305,15 +309,15 @@ public:
 typedef std::set <PerfCounters*, SortPerfCountersByName> perf_counters_set_t;
 
 /*
- * PerfCountersCollection manages PerfCounters objects for a Ceph process.
+ * PerfCountersCollectionImp manages PerfCounters objects for a Ceph process.
  */
-class PerfCountersCollection
+class PerfCountersCollectionImpl
 {
 public:
-  PerfCountersCollection(CephContext *cct);
-  ~PerfCountersCollection();
-  void add(class PerfCounters *l);
-  void remove(class PerfCounters *l);
+  PerfCountersCollectionImpl();
+  ~PerfCountersCollectionImpl();
+  void add(PerfCounters *l);
+  void remove(PerfCounters *l);
   void clear();
   bool reset(const std::string &name);
 
@@ -348,16 +352,9 @@ private:
                               const std::string &logger = "",
                               const std::string &counter = "");
 
-  CephContext *m_cct;
-
-  /** Protects m_loggers */
-  mutable Mutex m_lock;
-
   perf_counters_set_t m_loggers;
 
   CounterMap by_path; 
-
-  friend class PerfCountersCollectionTest;
 };
 
 
@@ -380,19 +377,4 @@ public:
 };
 
 
-class PerfCountersDeleter {
-  CephContext* cct;
-
-public:
-  PerfCountersDeleter() noexcept : cct(nullptr) {}
-  PerfCountersDeleter(CephContext* cct) noexcept : cct(cct) {}
-  void operator()(PerfCounters* p) noexcept {
-    if (cct)
-      cct->get_perfcounters_collection()->remove(p);
-    delete p;
-  }
-};
-
-using PerfCountersRef = std::unique_ptr<PerfCounters, PerfCountersDeleter>;
-
 #endif

diff --git a/src/common/perf_counters_collection.cc b/src/common/perf_counters_collection.cc
new file mode 100644 (file)
index 0000000..c8a7ec0
--- /dev/null
+++ b/src/common/perf_counters_collection.cc
@@ -0,0 +1,60 @@
+#include "common/perf_counters_collection.h"
+#include "common/ceph_mutex.h"
+#include "common/ceph_context.h"
+
+/* PerfcounterCollection hold the lock for PerfCounterCollectionImp */
+PerfCountersCollection::PerfCountersCollection(CephContext *cct)
+  : m_cct(cct),
+    m_lock(ceph::make_mutex("PerfCountersCollection"))
+{
+}
+PerfCountersCollection::~PerfCountersCollection()
+{
+  clear();
+}
+void PerfCountersCollection::add(PerfCounters *l)
+{
+  std::lock_guard lck(m_lock);
+  perf_impl.add(l);
+}
+void PerfCountersCollection::remove(PerfCounters *l)
+{
+  std::lock_guard lck(m_lock);
+  perf_impl.remove(l);
+}
+void PerfCountersCollection::clear()
+{
+  std::lock_guard lck(m_lock);
+  perf_impl.clear();
+}
+bool PerfCountersCollection::reset(const std::string &name)
+{
+  std::lock_guard lck(m_lock);
+  return perf_impl.reset(name);
+}
+void PerfCountersCollection::dump_formatted(ceph::Formatter *f, bool schema,
+                      const std::string &logger,
+                      const std::string &counter)
+{
+  std::lock_guard lck(m_lock);
+  perf_impl.dump_formatted(f,schema,logger,counter);
+}
+void PerfCountersCollection::dump_formatted_histograms(ceph::Formatter *f, bool schema,
+                                 const std::string &logger,
+                                 const std::string &counter)
+{
+  std::lock_guard lck(m_lock);
+  perf_impl.dump_formatted_histograms(f,schema,logger,counter);
+}
+void PerfCountersCollection::with_counters(std::function<void(const PerfCountersCollectionImpl::CounterMap &)> fn) const
+{
+  std::lock_guard lck(m_lock);
+  perf_impl.with_counters(fn);
+}
+void PerfCountersDeleter::operator()(PerfCounters* p) noexcept
+{
+  if (cct)
+    cct->get_perfcounters_collection()->remove(p);
+  delete p;
+}
+

diff --git a/src/common/perf_counters_collection.h b/src/common/perf_counters_collection.h
new file mode 100644 (file)
index 0000000..53a8f34
--- /dev/null
+++ b/src/common/perf_counters_collection.h
@@ -0,0 +1,46 @@
+#pragma once
+
+#include "common/perf_counters.h"
+#include "common/ceph_mutex.h"
+
+class CephContext;
+
+class PerfCountersCollection
+{
+  CephContext *m_cct;
+
+  /** Protects perf_impl->m_loggers */
+  mutable ceph::mutex m_lock;
+  PerfCountersCollectionImpl perf_impl;
+public:
+  PerfCountersCollection(CephContext *cct);
+  ~PerfCountersCollection();
+  void add(PerfCounters *l);
+  void remove(PerfCounters *l);
+  void clear();
+  bool reset(const std::string &name);
+
+  void dump_formatted(ceph::Formatter *f, bool schema,
+                      const std::string &logger = "",
+                      const std::string &counter = "");
+  void dump_formatted_histograms(ceph::Formatter *f, bool schema,
+                                 const std::string &logger = "",
+                                 const std::string &counter = "");
+
+  void with_counters(std::function<void(const PerfCountersCollectionImpl::CounterMap &)>) const;
+
+  friend class PerfCountersCollectionTest;
+};
+
+class PerfCountersDeleter {
+  CephContext* cct;
+
+public:
+  PerfCountersDeleter() noexcept : cct(nullptr) {}
+  PerfCountersDeleter(CephContext* cct) noexcept : cct(cct) {}
+  void operator()(PerfCounters* p) noexcept;
+};
+
+using PerfCountersRef = std::unique_ptr<PerfCounters, PerfCountersDeleter>;
+
+

diff --git a/src/mgr/MgrClient.cc b/src/mgr/MgrClient.cc
index 4bddbe2a19ce65ba2f4cce9a2a86a6d1737aec1d..5f2a62a465d137cc606d63020fb1c5f1202a8224 100644 (file)
--- a/src/mgr/MgrClient.cc
+++ b/src/mgr/MgrClient.cc
@@ -257,7 +257,7 @@ void MgrClient::_send_report()
   auto pcc = cct->get_perfcounters_collection();
 
   pcc->with_counters([this, report](
-        const PerfCountersCollection::CounterMap &by_path)
+        const PerfCountersCollectionImpl::CounterMap &by_path)
   {
     // Helper for checking whether a counter should be included
     auto include_counter = [this](

diff --git a/src/test/perf_counters.cc b/src/test/perf_counters.cc
index 87ce7163cdeadb61570b08ce795dd736d88a5cdb..d56adef8e365be5b52b4f84023e248abf00bf7f9 100644 (file)
--- a/src/test/perf_counters.cc
+++ b/src/test/perf_counters.cc
@@ -16,7 +16,7 @@
                            // now, this include has to come before the others.
 
 
-#include "common/perf_counters.h"
+#include "common/perf_counters_collection.h"
 #include "common/admin_socket_client.h"
 #include "common/ceph_context.h"
 #include "common/config.h"