From ec0fc71cdb579e31ff4e599c43f3918322e70ddf Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Mon, 15 Aug 2022 09:44:09 +0200 Subject: [PATCH] librbd: discard cache state if compare-and-write writes to disk Otherwise this is obviously broken with rbd_cache_policy = writeback or rbd_cache_policy = writethrough as the write is done on the OSD, beneath ObjectCacher. This went unnoticed because the original (and so far the only known) compare-and-write user, tcmu-runner rbd driver, disables the cache altogether. Signed-off-by: Ilya Dryomov --- .../cache/ObjectCacherObjectDispatch.cc | 29 +++++++++++++++---- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/src/librbd/cache/ObjectCacherObjectDispatch.cc b/src/librbd/cache/ObjectCacherObjectDispatch.cc index 81d5a71885b..822a053e143 100644 --- a/src/librbd/cache/ObjectCacherObjectDispatch.cc +++ b/src/librbd/cache/ObjectCacherObjectDispatch.cc @@ -389,18 +389,37 @@ bool ObjectCacherObjectDispatch::compare_and_write( // pass-through the compare-and-write request since it's not a supported // operation of the ObjectCacher + ObjectExtents object_extents; + object_extents.emplace_back(data_object_name(m_image_ctx, object_no), + object_no, object_off, cmp_data.length(), 0); + + // if compare succeeds, discard the cache state after changes are + // committed to disk + auto ctx = *on_finish; + *on_finish = new LambdaContext( + [this, object_extents, ctx](int r) { + // ObjectCacher doesn't provide a way to reliably invalidate + // extents: in case of a racing read (if the bh is in RX state), + // release_set() just returns while discard_set() populates the + // extent with zeroes. Neither is OK but the latter is better + // because it is at least deterministic... + if (r == 0) { + m_cache_lock.lock(); + m_object_cacher->discard_set(m_object_set, object_extents); + m_cache_lock.unlock(); + } + + ctx->complete(r); + }); + // ensure we aren't holding the cache lock post-flush on_dispatched = util::create_async_context_callback(*m_image_ctx, on_dispatched); - // flush any pending writes from the cache + // flush any pending writes from the cache before compare ZTracer::Trace trace(parent_trace); *dispatch_result = io::DISPATCH_RESULT_CONTINUE; - ObjectExtents object_extents; - object_extents.emplace_back(data_object_name(m_image_ctx, object_no), - object_no, object_off, cmp_data.length(), 0); - std::lock_guard cache_locker{m_cache_lock}; m_object_cacher->flush_set(m_object_set, object_extents, &trace, on_dispatched); -- 2.47.3