From f926b548b1ab7a8691386a6d0ac19bb72993cee5 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 (cherry picked from commit 343b049a1328d39a69a8c4c9e9cb93ac6ac77280) --- 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 65cdc34aa8d37..3f4377463ff40 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