From: David Zafman <dzafman@redhat.com>
Date: Tue, 12 Jul 2016 00:58:22 +0000 (-0700)
Subject: CLEANUP: Move locking into dump_ops_in_flight()/dump_historic_ops()
X-Git-Tag: v0.94.10~11^2~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=ce58688537d41e0662949f34b3595459581e74fc;p=ceph.git

CLEANUP: Move locking into dump_ops_in_flight()/dump_historic_ops()

Signed-off-by: David Zafman <dzafman@redhat.com>
(cherry picked from commit 17c645f70708134f50374cadb5505b0babe2265a)

Conflicts:
	src/mds/MDSRank.cc (variation of code exists in mds/MDS.cc)
	src/mon/Monitor.cc (no equivalent code here)
---

diff --git a/src/common/TrackedOp.cc b/src/common/TrackedOp.cc
index e55ad3e6d06d..c5d36dfe2589 100644
--- a/src/common/TrackedOp.cc
+++ b/src/common/TrackedOp.cc
@@ -87,14 +87,21 @@ void OpHistory::dump_ops(utime_t now, Formatter *f)
   f->close_section();
 }
 
-void OpTracker::dump_historic_ops(Formatter *f)
+bool OpTracker::dump_historic_ops(Formatter *f)
 {
+  if (!tracking_enabled)
+    return false;
+
   utime_t now = ceph_clock_now(cct);
   history.dump_ops(now, f);
+  return true;
 }
 
-void OpTracker::dump_ops_in_flight(Formatter *f, bool print_only_blocked)
+bool OpTracker::dump_ops_in_flight(Formatter *f, bool print_only_blocked)
 {
+  if (!tracking_enabled)
+    return false;
+
   f->open_object_section("ops_in_flight"); // overall dump
   uint64_t total_ops_in_flight = 0;
   f->open_array_section("ops"); // list of TrackedOps
@@ -119,6 +126,7 @@ void OpTracker::dump_ops_in_flight(Formatter *f, bool print_only_blocked)
   } else
     f->dump_int("num_ops", total_ops_in_flight);
   f->close_section(); // overall dump
+  return true;
 }
 
 void OpTracker::register_inflight_op(xlist<TrackedOp*>::item *i)
diff --git a/src/common/TrackedOp.h b/src/common/TrackedOp.h
index 0f406a20ce10..fb21517d58f0 100644
--- a/src/common/TrackedOp.h
+++ b/src/common/TrackedOp.h
@@ -97,8 +97,8 @@ public:
   void set_history_size_and_duration(uint32_t new_size, uint32_t new_duration) {
     history.set_size_and_duration(new_size, new_duration);
   }
-  void dump_ops_in_flight(Formatter *f, bool print_only_blocked=false);
-  void dump_historic_ops(Formatter *f);
+  bool dump_ops_in_flight(Formatter *f, bool print_only_blocked=false);
+  bool dump_historic_ops(Formatter *f);
   void register_inflight_op(xlist<TrackedOp*>::item *i);
   void unregister_inflight_op(TrackedOp *i);
 
diff --git a/src/mds/MDS.cc b/src/mds/MDS.cc
index 66232acbb9df..cc64a3e16110 100644
--- a/src/mds/MDS.cc
+++ b/src/mds/MDS.cc
@@ -242,16 +242,12 @@ bool MDS::asok_command(string command, cmdmap_t& cmdmap, string format,
       f->dump_string("error", "mds_not_active");
     } else if (command == "dump_ops_in_flight" ||
 	       command == "ops") {
-      if (!op_tracker.tracking_enabled) {
+      if (!op_tracker.dump_ops_in_flight(f)) {
         ss << "op_tracker tracking is not enabled";
-      } else {
-        op_tracker.dump_ops_in_flight(f);
       }
     } else if (command == "dump_historic_ops") {
-      if (!op_tracker.tracking_enabled) {
+      if (!op_tracker.dump_historic_ops(f)) {
 	ss << "op_tracker tracking is not enabled";
-      } else {
-        op_tracker.dump_historic_ops(f);
       }
     } else if (command == "osdmap barrier") {
       int64_t target_epoch = 0;
diff --git a/src/osd/OSD.cc b/src/osd/OSD.cc
index 3892b366f12f..8dd1600a2ac9 100644
--- a/src/osd/OSD.cc
+++ b/src/osd/OSD.cc
@@ -1685,27 +1685,19 @@ bool OSD::asok_command(string command, cmdmap_t& cmdmap, string format,
     store->sync_and_flush();
   } else if (command == "dump_ops_in_flight" ||
 	     command == "ops") {
-    if (!op_tracker.tracking_enabled) {
+    if (!op_tracker.dump_ops_in_flight(f)) {
       ss << "op_tracker tracking is not enabled now, so no ops are tracked currently, even those get stuck. \
 	Please enable \"osd_enable_op_tracker\", and the tracker will start to track new ops received afterwards.";
-    } else {
-      op_tracker.dump_ops_in_flight(f);
     }
   } else if (command == "dump_blocked_ops") {
-    RWLock::RLocker l(op_tracker.lock);
-    if (!op_tracker.tracking_enabled) {
+    if (!op_tracker.dump_ops_in_flight(f, true)) {
       ss << "op_tracker tracking is not enabled now, so no ops are tracked currently, even those get stuck. \
 	Please enable \"osd_enable_op_tracker\", and the tracker will start to track new ops received afterwards.";
-    } else {
-      op_tracker.dump_ops_in_flight(f, true);
     }
-
   } else if (command == "dump_historic_ops") {
-    if (!op_tracker.tracking_enabled) {
+    if (!op_tracker.dump_historic_ops(f)) {
       ss << "op_tracker tracking is not enabled now, so no ops are tracked currently, even those get stuck. \
 	Please enable \"osd_enable_op_tracker\", and the tracker will start to track new ops received afterwards.";
-    } else {
-      op_tracker.dump_historic_ops(f);
     }
   } else if (command == "dump_op_pq_state") {
     f->open_object_section("pq");