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 6558c61e192c69e8d0da058f9380c651541612aa..c0f451bce05ff50389e65e10914c83fcf13cf824 100644 (file)
--- 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 3db3703530beffed8152de7985dde4d917e1550d..9b50d5a066a55c7a73afbac13de222f1584583a9 100644 (file)
--- 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;