From c48a4ef96d181c39f4f7e4704f1926bba5069035 Mon Sep 17 00:00:00 2001
From: Sage Weil <sage@inktank.com>
Date: Wed, 14 May 2014 08:58:07 -0700
Subject: [PATCH] common/perf_counters: use second atomic to make counters safe
 to read

Add a second counter so that we can detect a race with an add/inc during
read, and retry.

Signed-off-by: Sage Weil <sage@inktank.com>
---
 src/common/perf_counters.cc | 38 +++++++++++++++++++++++++------------
 src/common/perf_counters.h  | 29 ++++++++++++++++++++++------
 2 files changed, 49 insertions(+), 18 deletions(-)

diff --git a/src/common/perf_counters.cc b/src/common/perf_counters.cc
index 06edb4199103..61d893454edf 100644
--- a/src/common/perf_counters.cc
+++ b/src/common/perf_counters.cc
@@ -106,9 +106,13 @@ void PerfCounters::inc(int idx, uint64_t amt)
   perf_counter_data_any_d& data(m_data[idx - m_lower_bound - 1]);
   if (!(data.type & PERFCOUNTER_U64))
     return;
-  data.u64.add(amt);
-  if (data.type & PERFCOUNTER_LONGRUNAVG)
+  if (data.type & PERFCOUNTER_LONGRUNAVG) {
     data.avgcount.inc();
+    data.u64.add(amt);
+    data.avgcount2.inc();
+  } else {
+    data.u64.add(amt);
+  }
 }
 
 void PerfCounters::dec(int idx, uint64_t amt)
@@ -136,8 +140,13 @@ void PerfCounters::set(int idx, uint64_t amt)
   if (!(data.type & PERFCOUNTER_U64))
     return;
   data.u64.set(amt);
-  if (data.type & PERFCOUNTER_LONGRUNAVG)
+  if (data.type & PERFCOUNTER_LONGRUNAVG) {
     data.avgcount.inc();
+    data.u64.set(amt);
+    data.avgcount2.inc();
+  } else {
+    data.u64.set(amt);
+  }
 }
 
 uint64_t PerfCounters::get(int idx) const
@@ -163,9 +172,13 @@ void PerfCounters::tinc(int idx, utime_t amt)
   perf_counter_data_any_d& data(m_data[idx - m_lower_bound - 1]);
   if (!(data.type & PERFCOUNTER_TIME))
     return;
-  data.u64.add(amt.to_nsec());
-  if (data.type & PERFCOUNTER_LONGRUNAVG)
+  if (data.type & PERFCOUNTER_LONGRUNAVG) {
     data.avgcount.inc();
+    data.u64.add(amt.to_nsec());
+    data.avgcount2.inc();
+  } else {
+    data.u64.add(amt.to_nsec());
+  }
 }
 
 void PerfCounters::tset(int idx, utime_t amt)
@@ -209,7 +222,8 @@ pair<uint64_t, uint64_t> PerfCounters::get_tavg_ms(int idx) const
     return make_pair(0, 0);
   if (!(data.type & PERFCOUNTER_LONGRUNAVG))
     return make_pair(0, 0);
-  return make_pair(data.avgcount.read(), data.u64.read()/1000000);
+  pair<uint64_t,uint64_t> a = data.read_avg();
+  return make_pair(a.second, a.first / 1000000ull);
 }
 
 void PerfCounters::dump_formatted(Formatter *f, bool schema)
@@ -229,15 +243,15 @@ void PerfCounters::dump_formatted(Formatter *f, bool schema)
     } else {
       if (d->type & PERFCOUNTER_LONGRUNAVG) {
 	f->open_object_section(d->name);
+	pair<uint64_t,uint64_t> a = d->read_avg();
 	if (d->type & PERFCOUNTER_U64) {
-	  f->dump_unsigned("avgcount", d->avgcount.read());
-	  f->dump_unsigned("sum", d->u64.read());
+	  f->dump_unsigned("avgcount", a.second);
+	  f->dump_unsigned("sum", a.first);
 	} else if (d->type & PERFCOUNTER_TIME) {
-	  f->dump_unsigned("avgcount", d->avgcount.read());
-	  uint64_t v = d->u64.read();
+	  f->dump_unsigned("avgcount", a.second);
 	  f->dump_format_unquoted("sum", "%"PRId64".%09"PRId64,
-				  v / 1000000000ull,
-				  v % 1000000000ull);
+				  a.first / 1000000000ull,
+				  a.first % 1000000000ull);
 	} else {
 	  assert(0);
 	}
diff --git a/src/common/perf_counters.h b/src/common/perf_counters.h
index f4f650771bcc..78c8684a2a58 100644
--- a/src/common/perf_counters.h
+++ b/src/common/perf_counters.h
@@ -115,14 +115,18 @@ private:
       : name(NULL),
 	type(PERFCOUNTER_NONE),
 	u64(0),
-	avgcount(0)
+	avgcount(0),
+	avgcount2(0)
     {}
     perf_counter_data_any_d(const perf_counter_data_any_d& other)
       : name(other.name),
 	type(other.type),
-	u64(other.u64.read()),
-	avgcount(other.avgcount.read())
-    {}
+	u64(other.u64.read()) {
+      pair<uint64_t,uint64_t> a = other.read_avg();
+      u64.set(a.first);
+      avgcount.set(a.second);
+      avgcount2.set(a.second);
+    }
 
     void write_schema_json(char *buf, size_t buf_sz) const;
     void  write_json(char *buf, size_t buf_sz) const;
@@ -131,14 +135,27 @@ private:
     enum perfcounter_type_d type;
     atomic64_t u64;
     atomic64_t avgcount;
+    atomic64_t avgcount2;
 
     perf_counter_data_any_d& operator=(const perf_counter_data_any_d& other) {
       name = other.name;
       type = other.type;
-      u64.set(other.u64.read());
-      avgcount.set(other.avgcount.read());
+      pair<uint64_t,uint64_t> a = other.read_avg();
+      u64.set(a.first);
+      avgcount.set(a.second);
+      avgcount2.set(a.second);
       return *this;
     }
+
+    /// read <sum, count> safely
+    pair<uint64_t,uint64_t> read_avg() const {
+      uint64_t sum, count;
+      do {
+	count = avgcount.read();
+	sum = u64.read();
+      } while (avgcount2.read() != count);
+      return make_pair(sum, count);
+    }
   };
   typedef std::vector<perf_counter_data_any_d> perf_counter_data_vec_t;
 
-- 
2.47.3