From 34b1ff73aa9623e2d53dc89cc93ccb11d95f1cdb Mon Sep 17 00:00:00 2001
From: xie xingguo <xie.xingguo@zte.com.cn>
Date: Mon, 5 Jun 2017 11:29:39 +0800
Subject: [PATCH] os/bluestore: fix false asserts in Cache::trim_all()

These asserts are true if we are going to shutdown the BlueStore instance.
But the caller can also be something like "ceph daemon out/osd.1.asok flush_store_cache",
which can fire these asserts as a result.

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>
---
 src/os/bluestore/BlueStore.cc | 23 +++++++++++++++++------
 src/os/bluestore/BlueStore.h  |  6 ++++++
 2 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index e5bccea2e94d4..c1ef67fb9e226 100644
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -747,8 +747,6 @@ void BlueStore::Cache::trim_all()
 {
   std::lock_guard<std::recursive_mutex> l(lock);
   _trim(0, 0);
-  assert(_get_num_onodes() == 0);
-  assert(_get_buffer_bytes() == 0);
 }
 
 void BlueStore::Cache::trim(
@@ -5045,7 +5043,7 @@ int BlueStore::_mount(bool kv_only)
  out_stop:
   _kv_stop();
  out_coll:
-  flush_cache();
+  _flush_cache();
  out_alloc:
   _close_alloc();
  out_fm:
@@ -5074,7 +5072,7 @@ int BlueStore::umount()
   dout(20) << __func__ << " stopping kv thread" << dendl;
   _kv_stop();
   _reap_collections();
-  flush_cache();
+  _flush_cache();
   dout(20) << __func__ << " closing" << dendl;
 
   mounted = false;
@@ -5697,7 +5695,7 @@ int BlueStore::fsck(bool deep)
 
  out_scan:
   mempool_thread.shutdown();
-  flush_cache();
+  _flush_cache();
  out_alloc:
   _close_alloc();
  out_fm:
@@ -11059,11 +11057,12 @@ void BlueStore::generate_db_histogram(Formatter *f)
 
 }
 
-void BlueStore::flush_cache()
+void BlueStore::_flush_cache()
 {
   dout(10) << __func__ << dendl;
   for (auto i : cache_shards) {
     i->trim_all();
+    assert(i->empty());
   }
   for (auto& p : coll_map) {
     assert(p.second->onode_map.empty());
@@ -11072,6 +11071,18 @@ void BlueStore::flush_cache()
   coll_map.clear();
 }
 
+// For external caller.
+// We use a best-effort policy instead, e.g.,
+// we don't care if there are still some pinned onodes/data in the cache
+// after this command is completed.
+void BlueStore::flush_cache()
+{
+  dout(10) << __func__ << dendl;
+  for (auto i : cache_shards) {
+    i->trim_all();
+  }
+}
+
 void BlueStore::_apply_padding(uint64_t head_pad,
 			       uint64_t tail_pad,
 			       bufferlist& bl,
diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index f2f3ace5dceaf..d95917ede65f0 100644
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -1082,6 +1082,11 @@ public:
 			   uint64_t *buffers,
 			   uint64_t *bytes) = 0;
 
+    bool empty() {
+      std::lock_guard<std::recursive_mutex> l(lock);
+      return _get_num_onodes() == 0 && _get_buffer_bytes() == 0;
+    }
+
 #ifdef DEBUG_CACHE
     virtual void _audit(const char *s) = 0;
 #else
@@ -2129,6 +2134,7 @@ public:
 
   void get_db_statistics(Formatter *f) override;
   void generate_db_histogram(Formatter *f) override;
+  void _flush_cache();
   void flush_cache() override;
   void dump_perf_counters(Formatter *f) override {
     f->open_object_section("perf_counters");
-- 
2.39.5