From f21b08f9b91d84d518de205fd81824b49b91796f Mon Sep 17 00:00:00 2001
From: Patrick Donnelly <pdonnell@redhat.com>
Date: Mon, 8 Jul 2019 12:42:18 -0700
Subject: [PATCH] common/TrackedOp: make settings atomic

To avoid locks on configuration changes.

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
---
 src/common/TrackedOp.cc | 16 ++++++++--------
 src/common/TrackedOp.h  | 19 ++++++++-----------
 2 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/src/common/TrackedOp.cc b/src/common/TrackedOp.cc
index 975e58a2260..c0e60e73d5c 100644
--- a/src/common/TrackedOp.cc
+++ b/src/common/TrackedOp.cc
@@ -78,7 +78,7 @@ void OpHistory::_insert_delayed(const utime_t& now, TrackedOpRef op)
   double opduration = op->get_duration();
   duration.insert(make_pair(opduration, op));
   arrived.insert(make_pair(op->get_initiated(), op));
-  if (opduration >= history_slow_op_threshold)
+  if (opduration >= history_slow_op_threshold.load())
     slow_op.insert(make_pair(op->get_initiated(), op));
   cleanup(now);
 }
@@ -87,21 +87,21 @@ void OpHistory::cleanup(utime_t now)
 {
   while (arrived.size() &&
 	 (now - arrived.begin()->first >
-	  (double)(history_duration))) {
+	  (double)(history_duration.load()))) {
     duration.erase(make_pair(
 	arrived.begin()->second->get_duration(),
 	arrived.begin()->second));
     arrived.erase(arrived.begin());
   }
 
-  while (duration.size() > history_size) {
+  while (duration.size() > history_size.load()) {
     arrived.erase(make_pair(
 	duration.begin()->second->get_initiated(),
 	duration.begin()->second));
     duration.erase(duration.begin());
   }
 
-  while (slow_op.size() > history_slow_op_size) {
+  while (slow_op.size() > history_slow_op_size.load()) {
     slow_op.erase(make_pair(
 	slow_op.begin()->second->get_initiated(),
 	slow_op.begin()->second));
@@ -113,8 +113,8 @@ void OpHistory::dump_ops(utime_t now, Formatter *f, set<string> filters, bool by
   std::lock_guard history_lock(ops_history_lock);
   cleanup(now);
   f->open_object_section("op_history");
-  f->dump_int("size", history_size);
-  f->dump_int("duration", history_duration);
+  f->dump_int("size", history_size.load());
+  f->dump_int("duration", history_duration.load());
   {
     f->open_array_section("ops");
     auto dump_fn = [&f, &now, &filters](auto begin_iter, auto end_iter) {
@@ -182,8 +182,8 @@ void OpHistory::dump_slow_ops(utime_t now, Formatter *f, set<string> filters)
   std::lock_guard history_lock(ops_history_lock);
   cleanup(now);
   f->open_object_section("OpHistory slow ops");
-  f->dump_int("num to keep", history_slow_op_size);
-  f->dump_int("threshold to keep", history_slow_op_threshold);
+  f->dump_int("num to keep", history_slow_op_size.load());
+  f->dump_int("threshold to keep", history_slow_op_threshold.load());
   {
     f->open_array_section("Ops");
     for (set<pair<utime_t, TrackedOpRef> >::const_iterator i =
diff --git a/src/common/TrackedOp.h b/src/common/TrackedOp.h
index fb88718b420..9a545c894a2 100644
--- a/src/common/TrackedOp.h
+++ b/src/common/TrackedOp.h
@@ -59,19 +59,16 @@ class OpHistory {
   std::set<std::pair<utime_t, TrackedOpRef> > slow_op;
   ceph::mutex ops_history_lock = ceph::make_mutex("OpHistory::ops_history_lock");
   void cleanup(utime_t now);
-  uint32_t history_size;
-  uint32_t history_duration;
-  uint32_t history_slow_op_size;
-  uint32_t history_slow_op_threshold;
-  std::atomic_bool shutdown;
+  std::atomic_size_t history_size{0};
+  std::atomic_uint32_t history_duration{0};
+  std::atomic_size_t history_slow_op_size{0};
+  std::atomic_uint32_t history_slow_op_threshold{0};
+  std::atomic_bool shutdown{false};
   OpHistoryServiceThread opsvc;
   friend class OpHistoryServiceThread;
 
 public:
-  OpHistory()
-    : history_size(0), history_duration(0),
-      history_slow_op_size(0), history_slow_op_threshold(0),
-      shutdown(false), opsvc(this) {
+  OpHistory() : opsvc(this) {
     opsvc.create("OpHistorySvc");
   }
   ~OpHistory() {
@@ -91,11 +88,11 @@ public:
   void dump_ops(utime_t now, ceph::Formatter *f, std::set<std::string> filters = {""}, bool by_duration=false);
   void dump_slow_ops(utime_t now, ceph::Formatter *f, std::set<std::string> filters = {""});
   void on_shutdown();
-  void set_size_and_duration(uint32_t new_size, uint32_t new_duration) {
+  void set_size_and_duration(size_t new_size, uint32_t new_duration) {
     history_size = new_size;
     history_duration = new_duration;
   }
-  void set_slow_op_size_and_threshold(uint32_t new_size, uint32_t new_threshold) {
+  void set_slow_op_size_and_threshold(size_t new_size, uint32_t new_threshold) {
     history_slow_op_size = new_size;
     history_slow_op_threshold = new_threshold;
   }
-- 
2.39.5