os/bluestore: make Cache::lock a recursive_mutex

We need to remove items from the cache LRUs in their
destructors, and it is very awkward to ensure that all
releasing paths either hold the cache lock or have
already cleared the cache items out.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 8da99c9125d3a0ac545b42fb71ed8aed9254cbe9..0f15de97242970a71e760fe4ff47d6088b5a6632 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -516,7 +516,7 @@ void BlueStore::LRUCache::_touch_onode(OnodeRef& o)
 
 void BlueStore::LRUCache::trim(uint64_t onode_max, uint64_t buffer_max)
 {
-  std::lock_guard<std::mutex> l(lock);
+  std::lock_guard<std::recursive_mutex> l(lock);
 
   dout(20) << __func__ << " onodes " << onode_lru.size() << " / " << onode_max
           << " buffers " << buffer_size << " / " << buffer_max
@@ -709,7 +709,7 @@ void BlueStore::TwoQCache::_adjust_buffer_size(Buffer *b, int64_t delta)
 
 void BlueStore::TwoQCache::trim(uint64_t onode_max, uint64_t buffer_max)
 {
-  std::lock_guard<std::mutex> l(lock);
+  std::lock_guard<std::recursive_mutex> l(lock);
 
   dout(20) << __func__ << " onodes " << onode_lru.size() << " / " << onode_max
           << " buffers " << buffer_bytes << " / " << buffer_max
@@ -986,7 +986,7 @@ void BlueStore::BufferSpace::read(
   BlueStore::ready_regions_t& res,
   interval_set<uint64_t>& res_intervals)
 {
-  std::lock_guard<std::mutex> l(cache->lock);
+  std::lock_guard<std::recursive_mutex> l(cache->lock);
   res.clear();
   res_intervals.clear();
   uint64_t want_bytes = length;
@@ -1044,7 +1044,7 @@ void BlueStore::BufferSpace::read(
 
 void BlueStore::BufferSpace::finish_write(uint64_t seq)
 {
-  std::lock_guard<std::mutex> l(cache->lock);
+  std::lock_guard<std::recursive_mutex> l(cache->lock);
 
   auto i = writing_map.begin();
   while (i != writing_map.end()) {
@@ -1081,7 +1081,7 @@ void BlueStore::BufferSpace::finish_write(uint64_t seq)
 
 void BlueStore::OnodeSpace::add(const ghobject_t& oid, OnodeRef o)
 {
-  std::lock_guard<std::mutex> l(cache->lock);
+  std::lock_guard<std::recursive_mutex> l(cache->lock);
   dout(30) << __func__ << " " << oid << " " << o << dendl;
   assert(onode_map.count(oid) == 0);
   onode_map[oid] = o;
@@ -1090,7 +1090,7 @@ void BlueStore::OnodeSpace::add(const ghobject_t& oid, OnodeRef o)
 
 BlueStore::OnodeRef BlueStore::OnodeSpace::lookup(const ghobject_t& oid)
 {
-  std::lock_guard<std::mutex> l(cache->lock);
+  std::lock_guard<std::recursive_mutex> l(cache->lock);
   dout(30) << __func__ << dendl;
   ceph::unordered_map<ghobject_t,OnodeRef>::iterator p = onode_map.find(oid);
   if (p == onode_map.end()) {
@@ -1106,7 +1106,7 @@ BlueStore::OnodeRef BlueStore::OnodeSpace::lookup(const ghobject_t& oid)
 
 void BlueStore::OnodeSpace::clear()
 {
-  std::lock_guard<std::mutex> l(cache->lock);
+  std::lock_guard<std::recursive_mutex> l(cache->lock);
   dout(10) << __func__ << dendl;
   for (auto &p : onode_map) {
     cache->_rm_onode(p.second);
@@ -1119,7 +1119,7 @@ void BlueStore::OnodeSpace::rename(OnodeRef& oldo,
                                     const ghobject_t& new_oid,
                                     const string& new_okey)
 {
-  std::lock_guard<std::mutex> l(cache->lock);
+  std::lock_guard<std::recursive_mutex> l(cache->lock);
   dout(30) << __func__ << " " << old_oid << " -> " << new_oid << dendl;
   ceph::unordered_map<ghobject_t,OnodeRef>::iterator po, pn;
   po = onode_map.find(old_oid);
@@ -1148,7 +1148,7 @@ void BlueStore::OnodeSpace::rename(OnodeRef& oldo,
 
 bool BlueStore::OnodeSpace::map_any(std::function<bool(OnodeRef)> f)
 {
-  std::lock_guard<std::mutex> l(cache->lock);
+  std::lock_guard<std::recursive_mutex> l(cache->lock);
   dout(20) << __func__ << dendl;
   for (auto& i : onode_map) {
     if (f(i.second)) {
@@ -6882,7 +6882,7 @@ void BlueStore::_dump_extent_map(ExtentMap &em, int log_level)
       dout(log_level) << __func__ << "      csum: " << std::hex << v << std::dec
                      << dendl;
     }
-    std::lock_guard<std::mutex> l(e.blob->shared_blob->bc.cache->lock);
+    std::lock_guard<std::recursive_mutex> l(e.blob->shared_blob->bc.cache->lock);
     if (!e.blob->shared_blob->bc.empty()) {
       for (auto& i : e.blob->shared_blob->bc.buffer_map) {
        dout(log_level) << __func__ << "       0x" << std::hex << i.first

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index a8e30061f1b7d30d8da2c727b79877d861cae80d..2a836f8f72951b1eb6a4b0bab0ed7906bbe242e4 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -256,13 +256,13 @@ public:
 
     // return value is the highest cache_private of a trimmed buffer, or 0.
     int discard(uint64_t offset, uint64_t length) {
-      std::lock_guard<std::mutex> l(cache->lock);
+      std::lock_guard<std::recursive_mutex> l(cache->lock);
       return _discard(offset, length);
     }
     int _discard(uint64_t offset, uint64_t length);
 
     void write(uint64_t seq, uint64_t offset, bufferlist& bl, unsigned flags) {
-      std::lock_guard<std::mutex> l(cache->lock);
+      std::lock_guard<std::recursive_mutex> l(cache->lock);
       Buffer *b = new Buffer(this, Buffer::STATE_WRITING, seq, offset, bl,
                             flags);
       b->cache_private = _discard(offset, bl.length());
@@ -270,7 +270,7 @@ public:
     }
     void finish_write(uint64_t seq);
     void did_read(uint64_t offset, bufferlist& bl) {
-      std::lock_guard<std::mutex> l(cache->lock);
+      std::lock_guard<std::recursive_mutex> l(cache->lock);
       Buffer *b = new Buffer(this, Buffer::STATE_CLEAN, 0, offset, bl);
       b->cache_private = _discard(offset, bl.length());
       _add_buffer(b, 1, nullptr);
@@ -285,7 +285,7 @@ public:
     }
 
     void dump(Formatter *f) const {
-      std::lock_guard<std::mutex> l(cache->lock);
+      std::lock_guard<std::recursive_mutex> l(cache->lock);
       f->open_array_section("buffers");
       for (auto& i : buffer_map) {
        f->open_object_section("buffer");
@@ -676,7 +676,7 @@ public:
   /// a cache (shard) of onodes and buffers
   struct Cache {
     PerfCounters *logger;
-    std::mutex lock;                ///< protect lru and other structures
+    std::recursive_mutex lock;          ///< protect lru and other structures
 
     static Cache *create(string type, PerfCounters *logger);