From: Sage Weil <sage@redhat.com>
Date: Mon, 20 Jun 2016 15:35:38 +0000 (-0400)
Subject: os/bluestore: protect BnodeSet with mutex
X-Git-Tag: v11.0.0~70^2~3
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=2215e227a992e5a723109ebfed6462fcf4d6b823;p=ceph.git

os/bluestore: protect BnodeSet with mutex

The onodes (and thus bnodes) may not get destroyed with the collection
lock held, which means we cannot rely on that lock to protected the
bnodeset structure.  Give BnodeSet it's own mutex to protect itself.

While we are at it, make dummy a member so we can avoid constructing it
each time.  This is now safe since we are under the new lock.

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

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 6558c61e192c..c0f451bce05f 100644
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -1070,11 +1070,27 @@ void BlueStore::Bnode::put()
 {
   if (--nref == 0) {
     dout(20) << __func__ << " removing self from set " << bnode_set << dendl;
-    bnode_set->uset.erase(*this);
+    bnode_set->remove(this);
     delete this;
   }
 }
 
+// BnodeSet
+
+#undef dout_prefix
+#define dout_prefix *_dout << "bluestore.bnodeset(" << this << ") "
+
+BlueStore::BnodeRef BlueStore::BnodeSet::get(uint32_t hash)
+{
+  std::lock_guard<std::mutex> l(lock);
+  dummy.hash = hash;
+  auto p = uset.find(dummy);
+  if (p == uset.end()) {
+    return nullptr;
+  }
+  return &*p;
+}
+
 // Onode
 
 #undef dout_prefix
@@ -1113,37 +1129,36 @@ BlueStore::BnodeRef BlueStore::Collection::get_bnode(
   uint32_t hash
   )
 {
-  Bnode dummy(hash, string(), NULL);
-  auto p = bnode_set.uset.find(dummy);
-  if (p == bnode_set.uset.end()) {
-    spg_t pgid;
-    if (!cid.is_pg(&pgid))
-      pgid = spg_t();  // meta
-    string key;
-    get_bnode_key(pgid.shard, pgid.pool(), hash, &key);
-    BnodeRef e = new Bnode(hash, key, &bnode_set);
+  BnodeRef b = bnode_set.get(hash);
+  if (b) {
     dout(10) << __func__ << " hash " << std::hex << hash << std::dec
-	     << " created " << e << dendl;
-
-    bufferlist v;
-    int r = store->db->get(PREFIX_OBJ, key, &v);
-    if (r >= 0) {
-      assert(v.length() > 0);
-      bufferlist::iterator p = v.begin();
-      e->blob_map.decode(p, cache);
-      dout(10) << __func__ << " hash " << std::hex << hash << std::dec
-	       << " loaded blob_map " << e->blob_map << dendl;
-    } else {
-      dout(10) << __func__ << " hash " <<std::hex << hash << std::dec
-	       << " missed, new blob_map" << dendl;
-    }
-    bnode_set.uset.insert(*e);
-    return e;
-  } else {
+	     << " had " << b << dendl;
+    return b;
+  }
+
+  spg_t pgid;
+  if (!cid.is_pg(&pgid))
+    pgid = spg_t();  // meta
+  string key;
+  get_bnode_key(pgid.shard, pgid.pool(), hash, &key);
+  b = new Bnode(hash, key, &bnode_set);
+  dout(10) << __func__ << " hash " << std::hex << hash << std::dec
+	   << " created " << b << dendl;
+
+  bufferlist v;
+  int r = store->db->get(PREFIX_OBJ, key, &v);
+  if (r >= 0) {
+    assert(v.length() > 0);
+    bufferlist::iterator p = v.begin();
+    b->blob_map.decode(p, cache);
     dout(10) << __func__ << " hash " << std::hex << hash << std::dec
-	     << " had " << &*p << dendl;
-    return &*p;
+	     << " loaded blob_map " << b->blob_map << dendl;
+  } else {
+    dout(10) << __func__ << " hash " <<std::hex << hash << std::dec
+	     << " missed, new blob_map" << dendl;
   }
+  bnode_set.add(b.get());
+  return b;
 }
 
 BlueStore::OnodeRef BlueStore::Collection::get_onode(
diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 3db3703530be..9b50d5a066a5 100644
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -432,20 +432,34 @@ public:
     typedef boost::intrusive::unordered_set<Bnode>::bucket_type bucket_type;
     typedef boost::intrusive::unordered_set<Bnode>::bucket_traits bucket_traits;
 
+    std::mutex lock;
     unsigned num_buckets;
     vector<bucket_type> buckets;
-
     boost::intrusive::unordered_set<Bnode> uset;
 
+    Bnode dummy;  ///< dummy entry used for lookups.  protected by lock.
+
     explicit BnodeSet(unsigned n)
       : num_buckets(n),
 	buckets(n),
-	uset(bucket_traits(buckets.data(), num_buckets)) {
+	uset(bucket_traits(buckets.data(), num_buckets)),
+	dummy(0, string(), NULL) {
       assert(n > 0);
     }
     ~BnodeSet() {
       assert(uset.empty());
     }
+
+    BnodeRef get(uint32_t hash);
+
+    void add(Bnode *b) {
+      std::lock_guard<std::mutex> l(lock);
+      uset.insert(*b);
+    }
+    void remove(Bnode *b) {
+      std::lock_guard<std::mutex> l(lock);
+      uset.erase(*b);
+    }
   };
 
   struct OnodeSpace;