os/bluestore: fix SharedBlobSet deregister race

In commit 8c8944b2c45ca9dc5b8fd4db1590e1d24206c0b3 we fixed one narrow race
and introduced a new (probably less narrow) one:

A: put shared blob foo, nref = 0, start removing self from set
B: open_shared_blob -> lookup gets nullptr (nref==null), creates a new shared blob
  B: takes lock, sets sb_map[sbid] = newblob
A: gets lock, erases sb_map[sbid]
B: open_shared_blob -> lookup gets null, creates another new shared blob

Fix by only removing the sb_map entry for the nref=0 sb if it still points
to us.  If it doesn't, that means some new blob has shown up in its place.

Fixes: http://tracker.ceph.com/issues/24319
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 345ea53bd30426d4e6159d0866497d5a8bf6f327)

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index d000804b74115225706c9135dcd5671564e5543d..804011d9fd696a2909d5d1e7a7a57174f9c51854 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -1667,7 +1667,7 @@ void BlueStore::SharedBlob::put()
                             << " removing self from set " << get_parent()
                             << dendl;
     if (get_parent()) {
-      get_parent()->remove_last(this);
+      get_parent()->remove(this);
     }
     delete this;
   }

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index 29a531b62bd07a9029d5d444f1151904cfdd2fc4..387c22373243ba6c038d45dfb59d12081ec29f1c 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -444,17 +444,15 @@ public:
       sb->coll = coll;
     }
 
-    void remove_last(SharedBlob *sb) {
-      std::lock_guard<std::mutex> l(lock);
-      assert(sb->nref == 0);
-      assert(sb->get_parent() == this);
-      sb_map.erase(sb->get_sbid());
-    }
-
     void remove(SharedBlob *sb) {
       std::lock_guard<std::mutex> l(lock);
       assert(sb->get_parent() == this);
-      sb_map.erase(sb->get_sbid());
+      // only remove if it still points to us
+      auto p = sb_map.find(sb->get_sbid());
+      if (p != sb_map.end() &&
+         p->second == sb) {
+       sb_map.erase(p);
+      }
     }
 
     bool empty() {