os/bluestore: bring back cache change protection in finish_write

Signed-off-by: Pere Diaz Bou <pere-altea@hotmail.com>

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 371bb1ada886b1bc3ba0b0a3c7e7e183b571e4b9..a5b1fafe569179f55e8383bdc2f5f2e14c1c2b5d 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -1810,38 +1810,50 @@ void BlueStore::BufferSpace::read(
   cache->logger->inc(l_bluestore_buffer_miss_bytes, miss_bytes);
 }
 
-void BlueStore::BufferSpace::_finish_write(BufferCacheShard* cache, uint64_t seq)
+void BlueStore::BufferSpace::_finish_write(OnodeRef onode, uint64_t seq)
 {
-  std::lock_guard l(cache->lock);
-  ldout(cache->cct, 20) << __func__ << " seq=" << seq << dendl;
-  auto i = writing.begin();
-  while (i != writing.end()) {
-    if (i->seq > seq) {
-      break;
-    }
-    if (i->seq < seq) {
-      ++i;
+  while (true) {
+    BufferCacheShard *cache = onode->c->cache;
+    std::lock_guard l(cache->lock);
+    if (cache != onode->c->cache) {
+      ldout(cache->cct, 20) << __func__
+              << " raced with sb cache update, was " << cache
+              << ", now " << onode->c->cache << ", retrying"
+              << dendl;
       continue;
     }
+    ldout(cache->cct, 20) << __func__ << " seq=" << seq << dendl;
+    auto i = writing.begin();
+    while (i != writing.end()) {
+      if (i->seq > seq) {
+        break;
+      }
+      if (i->seq < seq) {
+        ++i;
+        continue;
+      }
 
-    Buffer *b = &*i;
-    ceph_assert(b->is_writing());
+      Buffer *b = &*i;
+      ceph_assert(b->is_writing());
 
-    if (b->flags & Buffer::FLAG_NOCACHE) {
-      writing.erase(i++);
-      ldout(cache->cct, 20) << __func__ << " discard " << *b << dendl;
-      buffer_map.erase(b->offset);
-    } else {
-      b->state = Buffer::STATE_CLEAN;
-      writing.erase(i++);
-      b->maybe_rebuild();
-      b->data.reassign_to_mempool(mempool::mempool_bluestore_cache_data);
-      cache->_add(b, 1, nullptr);
-      ldout(cache->cct, 20) << __func__ << " added " << *b << dendl;
+      if (b->flags & Buffer::FLAG_NOCACHE) {
+        writing.erase(i++);
+        ldout(cache->cct, 20) << __func__ << " discard " << *b << dendl;
+        buffer_map.erase(b->offset);
+      } else {
+        b->state = Buffer::STATE_CLEAN;
+        writing.erase(i++);
+        b->maybe_rebuild();
+        b->data.reassign_to_mempool(mempool::mempool_bluestore_cache_data);
+        cache->_add(b, 1, nullptr);
+        ldout(cache->cct, 20) << __func__ << " added " << *b << dendl;
+      }
     }
+    cache->_trim();
+    cache->_audit("finish_write end");
+
+    break;
   }
-  cache->_trim();
-  cache->_audit("finish_write end");
 }
 
 /*
@@ -13761,10 +13773,10 @@ void BlueStore::_txc_finish(TransContext *txc)
 
       }
       for (auto &dependent_onode : dependent_onodes) {
-        dependent_onode->bc._finish_write(dependent_onode->c->cache, seq);
+        dependent_onode->bc._finish_write(dependent_onode, seq);
       }
     }
-    onode->bc._finish_write(onode->c->cache, seq);
+    onode->bc._finish_write(onode, seq);
   }
   txc->buffers_written.clear();
 
@@ -18121,7 +18133,7 @@ void BlueStore::_shutdown_cache()
     std::unique_lock l(p.second->lock);
     for (auto &[seq, onodes] : p.second->deferred_seq_dependencies) {
       for (auto &onode : onodes) {
-        onode->bc._finish_write(onode->c->cache, seq);
+        onode->bc._finish_write(onode, seq);
       }
     }
     p.second->deferred_seq_dependencies.clear();

diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
index aaa9c71ffa5b310ce1e3aeb82f19ee418efc9886..87e0e0f402613d61a0ea8781410ea5fdc93e3a24 100644 (file)
--- a/src/os/bluestore/BlueStore.h
+++ b/src/os/bluestore/BlueStore.h
@@ -499,7 +499,7 @@ public:
                   nullptr);
       cache->_trim();
     }
-    void _finish_write(BufferCacheShard* cache, uint64_t seq);
+    void _finish_write(OnodeRef onode, uint64_t seq);
     void did_read(BufferCacheShard* cache, uint32_t offset, uint32_t length, ceph::buffer::list&& bl) {
       std::lock_guard l(cache->lock);
       uint16_t cache_private = _discard(cache, offset, bl.length());