From 343b049a1328d39a69a8c4c9e9cb93ac6ac77280 Mon Sep 17 00:00:00 2001 From: Adam Kupczyk Date: Mon, 22 Mar 2021 11:20:11 +0100 Subject: [PATCH] os/bluestore: Make Onode::put/get resiliant to split_cache In OnodeCacheShard* ocs = c->get_onode_cache(); std::lock_guard l(ocs->lock); while waiting for lock, split_cache might have changed OnodeCacheShard. This will result in adding Onode to improper OnodeCacheShard. Such action is obviously bad, as we will operate in future (at least once) on different OnodeCacheShard then we got lock for. Particulary sensitive to this are _trim and split_cache functions, as they iterate over elements. Signed-off-by: Adam Kupczyk --- src/os/bluestore/BlueStore.cc | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index e2774f6625d..072d0c2aba5 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -3506,7 +3506,13 @@ BlueStore::BlobRef BlueStore::ExtentMap::split_blob( void BlueStore::Onode::get() { if (++nref >= 2 && !pinned) { OnodeCacheShard* ocs = c->get_onode_cache(); - std::lock_guard l(ocs->lock); + ocs->lock.lock(); + // It is possible that during waiting split_cache moved us to different OnodeCacheShard. + while (ocs != c->get_onode_cache()) { + ocs->lock.unlock(); + ocs = c->get_onode_cache(); + ocs->lock.lock(); + } bool was_pinned = pinned; pinned = nref >= 2; // additional increment for newly pinned instance @@ -3517,13 +3523,20 @@ void BlueStore::Onode::get() { if (cached && r) { ocs->_pin(this); } + ocs->lock.unlock(); } } void BlueStore::Onode::put() { int n = --nref; if (n == 2) { OnodeCacheShard* ocs = c->get_onode_cache(); - std::lock_guard l(ocs->lock); + ocs->lock.lock(); + // It is possible that during waiting split_cache moved us to different OnodeCacheShard. + while (ocs != c->get_onode_cache()) { + ocs->lock.unlock(); + ocs = c->get_onode_cache(); + ocs->lock.lock(); + } bool need_unpin = pinned; pinned = pinned && nref > 2; // intentionally use > not >= as we have // +1 due to pinned state @@ -3543,6 +3556,7 @@ void BlueStore::Onode::put() { if (need_unpin) { n = --nref; } + ocs->lock.unlock(); } if (n == 0) { delete this; -- 2.39.5