From e8e1acb1d5154b749d251efa88b45e8ad3edb2bb Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 8 Sep 2016 11:51:34 -0400 Subject: [PATCH] librbd: ignore cache busy errors when shrinking an image Signed-off-by: Jason Dillaman (cherry picked from commit 4ce663845679dc35f2f15b893c6f988c4a60b25b) Conflicts: src/test/librbd/operation/test_mock_ResizeRequest.cc: when_resize does not have the allow_shrink argument because d1f2c557b2c039730baca9efa3f5244bc19dcb1a has not been backported --- src/librbd/operation/ResizeRequest.cc | 39 ++++++++++++++++++- src/librbd/operation/ResizeRequest.h | 8 +++- src/test/librbd/mock/MockImageCtx.h | 1 + .../operation/test_mock_ResizeRequest.cc | 32 ++++++++++++++- 4 files changed, 76 insertions(+), 4 deletions(-) diff --git a/src/librbd/operation/ResizeRequest.cc b/src/librbd/operation/ResizeRequest.cc index 79fae3930a155..c046e79abb210 100644 --- a/src/librbd/operation/ResizeRequest.cc +++ b/src/librbd/operation/ResizeRequest.cc @@ -172,6 +172,38 @@ Context *ResizeRequest::handle_trim_image(int *result) { return nullptr; } +template +void ResizeRequest::send_flush_cache() { + I &image_ctx = this->m_image_ctx; + if (image_ctx.object_cacher == nullptr) { + send_trim_image(); + return; + } + + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << this << " " << __func__ << dendl; + + RWLock::RLocker owner_locker(image_ctx.owner_lock); + image_ctx.flush_cache(create_async_context_callback( + image_ctx, create_context_callback< + ResizeRequest, &ResizeRequest::handle_flush_cache>(this))); +} + +template +Context *ResizeRequest::handle_flush_cache(int *result) { + I &image_ctx = this->m_image_ctx; + CephContext *cct = image_ctx.cct; + ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl; + + if (*result < 0) { + lderr(cct) << "failed to flush cache: " << cpp_strerror(*result) << dendl; + return this->create_context_finisher(*result); + } + + send_invalidate_cache(); + return nullptr; +} + template void ResizeRequest::send_invalidate_cache() { I &image_ctx = this->m_image_ctx; @@ -192,7 +224,10 @@ Context *ResizeRequest::handle_invalidate_cache(int *result) { CephContext *cct = image_ctx.cct; ldout(cct, 5) << this << " " << __func__ << ": r=" << *result << dendl; - if (*result < 0) { + // ignore busy error -- writeback was successfully flushed so we might be + // wasting some cache space for trimmed objects, but they will get purged + // eventually. Most likely cause of the issue was a in-flight cache read + if (*result < 0 && *result != -EBUSY) { lderr(cct) << "failed to invalidate cache: " << cpp_strerror(*result) << dendl; return this->create_context_finisher(*result); @@ -215,7 +250,7 @@ Context *ResizeRequest::send_grow_object_map() { if (m_original_size == m_new_size) { return this->create_context_finisher(0); } else if (m_new_size < m_original_size) { - send_invalidate_cache(); + send_flush_cache(); return nullptr; } diff --git a/src/librbd/operation/ResizeRequest.h b/src/librbd/operation/ResizeRequest.h index 400bde33316ff..d1c778b9c8c80 100644 --- a/src/librbd/operation/ResizeRequest.h +++ b/src/librbd/operation/ResizeRequest.h @@ -79,7 +79,10 @@ private: * | STATE_UPDATE_HEADER ----------------------------\ * | | * | (shrink) | - * |\--------> STATE_INVALIDATE_CACHE | + * |\--------> STATE_FLUSH_CACHE | + * | | | + * | v | + * | STATE_INVALIDATE_CACHE | * | | | * | v | * | STATE_TRIM_IMAGE | @@ -119,6 +122,9 @@ private: Context *send_append_op_event(); Context *handle_append_op_event(int *result); + void send_flush_cache(); + Context *handle_flush_cache(int *result); + void send_invalidate_cache(); Context *handle_invalidate_cache(int *result); diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h index f7283a72bc6de..2a8b98e28f228 100644 --- a/src/test/librbd/mock/MockImageCtx.h +++ b/src/test/librbd/mock/MockImageCtx.h @@ -157,6 +157,7 @@ struct MockImageCtx { MOCK_METHOD1(flush_async_operations, void(Context *)); MOCK_METHOD1(flush_copyup, void(Context *)); + MOCK_METHOD1(flush_cache, void(Context *)); MOCK_METHOD1(invalidate_cache, void(Context *)); MOCK_METHOD1(shut_down_cache, void(Context *)); diff --git a/src/test/librbd/operation/test_mock_ResizeRequest.cc b/src/test/librbd/operation/test_mock_ResizeRequest.cc index c7733cf5565be..e1998ed692a4f 100644 --- a/src/test/librbd/operation/test_mock_ResizeRequest.cc +++ b/src/test/librbd/operation/test_mock_ResizeRequest.cc @@ -116,6 +116,11 @@ public: .WillOnce(FinishRequest(&mock_trim_request, r, &mock_image_ctx)); } + void expect_flush_cache(MockImageCtx &mock_image_ctx, int r) { + EXPECT_CALL(mock_image_ctx, flush_cache(_)).WillOnce(CompleteContext(r, NULL)); + expect_op_work_queue(mock_image_ctx); + } + void expect_invalidate_cache(MockImageCtx &mock_image_ctx, int r) { EXPECT_CALL(mock_image_ctx, invalidate_cache(_)) .WillOnce(CompleteContext(r, NULL)); @@ -202,6 +207,7 @@ TEST_F(TestMockOperationResizeRequest, ShrinkSuccess) { expect_unblock_writes(mock_image_ctx); MockTrimRequest mock_trim_request; + expect_flush_cache(mock_image_ctx, 0); expect_invalidate_cache(mock_image_ctx, 0); expect_trim(mock_image_ctx, mock_trim_request, 0); expect_block_writes(mock_image_ctx, 0); @@ -263,12 +269,35 @@ TEST_F(TestMockOperationResizeRequest, TrimError) { expect_unblock_writes(mock_image_ctx); MockTrimRequest mock_trim_request; - expect_invalidate_cache(mock_image_ctx, 0); + expect_flush_cache(mock_image_ctx, 0); + expect_invalidate_cache(mock_image_ctx, -EBUSY); expect_trim(mock_image_ctx, mock_trim_request, -EINVAL); expect_commit_op_event(mock_image_ctx, -EINVAL); ASSERT_EQ(-EINVAL, when_resize(mock_image_ctx, ictx->size / 2, 0, false)); } +TEST_F(TestMockOperationResizeRequest, FlushCacheError) { + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockImageCtx mock_image_ctx(*ictx); + MockExclusiveLock mock_exclusive_lock; + MockJournal mock_journal; + MockObjectMap mock_object_map; + initialize_features(ictx, mock_image_ctx, mock_exclusive_lock, mock_journal, + mock_object_map); + + InSequence seq; + expect_block_writes(mock_image_ctx, 0); + expect_append_op_event(mock_image_ctx, true, 0); + expect_unblock_writes(mock_image_ctx); + + MockTrimRequest mock_trim_request; + expect_flush_cache(mock_image_ctx, -EINVAL); + expect_commit_op_event(mock_image_ctx, -EINVAL); + ASSERT_EQ(-EINVAL, when_resize(mock_image_ctx, ictx->size / 2, 0, false)); +} + TEST_F(TestMockOperationResizeRequest, InvalidateCacheError) { librbd::ImageCtx *ictx; ASSERT_EQ(0, open_image(m_image_name, &ictx)); @@ -286,6 +315,7 @@ TEST_F(TestMockOperationResizeRequest, InvalidateCacheError) { expect_unblock_writes(mock_image_ctx); MockTrimRequest mock_trim_request; + expect_flush_cache(mock_image_ctx, 0); expect_invalidate_cache(mock_image_ctx, -EINVAL); expect_commit_op_event(mock_image_ctx, -EINVAL); ASSERT_EQ(-EINVAL, when_resize(mock_image_ctx, ictx->size / 2, 0, false)); -- 2.39.5