os/bluestore: handle bnode removal race

One thread might try to drop a bnode while another does a lookup and
retakes a ref:

 1- drop bnode ref (-> 0)
 2- take bnode_set lock, do lookup
 2- take bnode ref (-> 1)
 2- drop lock
 1- take bnode_set lock
 1- remove from set
 1- drop lock

Fix this race by rechecking the ref count while we are holding the lock.

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

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 784432a4d68b3f83a2d79c716ef09d61321d575d..05eabf65d8797ab95722864dbfe7e1e47ef6012d 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -1069,8 +1069,11 @@ void BlueStore::Bnode::put()
 {
   if (--nref == 0) {
     dout(20) << __func__ << " removing self from set " << bnode_set << dendl;
-    bnode_set->remove(this);
-    delete this;
+    if (bnode_set->remove(this)) {
+      delete this;
+    } else {
+      dout(20) << __func__ << " lost race to remove myself from set" << dendl;
+    }
   }
 }
 

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 9b50d5a066a55c7a73afbac13de222f1584583a9..2df1044764630bdf86dfc3fd644c3fc5a4b96e84 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -456,9 +456,13 @@ public:
       std::lock_guard<std::mutex> l(lock);
       uset.insert(*b);
     }
-    void remove(Bnode *b) {
+    bool remove(Bnode *b) {
       std::lock_guard<std::mutex> l(lock);
-      uset.erase(*b);
+      if (b->nref == 0) {
+       uset.erase(*b);
+       return true;
+      }
+      return false;
     }
   };