From 345ea53bd30426d4e6159d0866497d5a8bf6f327 Mon Sep 17 00:00:00 2001 From: Sage Weil Date: Sun, 10 Jun 2018 15:57:31 -0500 Subject: [PATCH] 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 --- src/os/bluestore/BlueStore.cc | 2 +- src/os/bluestore/BlueStore.h | 14 ++++++-------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 12edfbde5b81d..cabd7d220fb6d 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -1672,7 +1672,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 84693b081a770..78d89ee6d6faa 100644 --- a/src/os/bluestore/BlueStore.h +++ b/src/os/bluestore/BlueStore.h @@ -450,17 +450,15 @@ public: sb->coll = coll; } - void remove_last(SharedBlob *sb) { - std::lock_guard l(lock); - assert(sb->nref == 0); - assert(sb->get_parent() == this); - sb_map.erase(sb->get_sbid()); - } - void remove(SharedBlob *sb) { std::lock_guard 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() { -- 2.39.5