From: Mahati Chamarthy Date: Wed, 3 Feb 2021 15:03:50 +0000 (+0530) Subject: librbd/cache/pwl: Fix user request completion in X-Git-Tag: v17.1.0~3002^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=d8eb9391c9dd7425053bd91f79eea103ae90cc93;p=ceph.git librbd/cache/pwl: Fix user request completion in ... persist_on_flush mode Signed-off-by: Mahati Chamarthy --- diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc index e038a3fd65d1..228ea13ca421 100644 --- a/src/librbd/cache/pwl/AbstractWriteLog.cc +++ b/src/librbd/cache/pwl/AbstractWriteLog.cc @@ -1250,6 +1250,11 @@ void AbstractWriteLog::complete_op_log_entries(GenericLogOperations &&ops, if (op->reserved_allocated()) { published_reserves++; } + { + std::lock_guard locker(m_lock); + m_unpublished_reserves -= published_reserves; + m_dirty_log_entries.splice(m_dirty_log_entries.end(), dirty_entries); + } op->complete(result); m_perfcounter->tinc(l_librbd_pwl_log_op_dis_to_app_t, op->log_append_time - op->dispatch_time); @@ -1263,13 +1268,9 @@ void AbstractWriteLog::complete_op_log_entries(GenericLogOperations &&ops, log_entry->ram_entry.write_bytes); m_perfcounter->tinc(l_librbd_pwl_log_op_app_to_cmp_t, now - op->log_append_time); } - + // New entries may be flushable { std::lock_guard locker(m_lock); - m_unpublished_reserves -= published_reserves; - m_dirty_log_entries.splice(m_dirty_log_entries.end(), dirty_entries); - - /* New entries may be flushable */ wake_up(); } } diff --git a/src/librbd/cache/pwl/AbstractWriteLog.h b/src/librbd/cache/pwl/AbstractWriteLog.h index 60accc47ce1d..29b6c3b9c709 100644 --- a/src/librbd/cache/pwl/AbstractWriteLog.h +++ b/src/librbd/cache/pwl/AbstractWriteLog.h @@ -123,7 +123,8 @@ public: void release_write_lanes(C_BlockIORequestT *req); virtual bool alloc_resources(C_BlockIORequestT *req) = 0; virtual void setup_schedule_append( - pwl::GenericLogOperationsVector &ops, bool do_early_flush) = 0; + pwl::GenericLogOperationsVector &ops, bool do_early_flush, + C_BlockIORequestT *req) = 0; void schedule_append(pwl::GenericLogOperationsVector &ops); void schedule_append(pwl::GenericLogOperationSharedPtr op); void flush_new_sync_point(C_FlushRequestT *flush_req, diff --git a/src/librbd/cache/pwl/Request.cc b/src/librbd/cache/pwl/Request.cc index 28e15bb11fd4..2fa9643aba9c 100644 --- a/src/librbd/cache/pwl/Request.cc +++ b/src/librbd/cache/pwl/Request.cc @@ -262,7 +262,7 @@ bool C_WriteRequest::append_write_request(std::shared_ptr sync_poi template void C_WriteRequest::schedule_append() { ceph_assert(++m_appended == 1); - pwl.setup_schedule_append(this->op_set->operations, m_do_early_flush); + pwl.setup_schedule_append(this->op_set->operations, m_do_early_flush, this); } /** diff --git a/src/librbd/cache/pwl/rwl/WriteLog.cc b/src/librbd/cache/pwl/rwl/WriteLog.cc index ebe82c83fa9f..86d12c14f048 100644 --- a/src/librbd/cache/pwl/rwl/WriteLog.cc +++ b/src/librbd/cache/pwl/rwl/WriteLog.cc @@ -675,7 +675,8 @@ void WriteLog::enlist_op_flusher() template void WriteLog::setup_schedule_append( - pwl::GenericLogOperationsVector &ops, bool do_early_flush) { + pwl::GenericLogOperationsVector &ops, bool do_early_flush, + C_BlockIORequestT *req) { if (do_early_flush) { /* This caller is waiting for persist, so we'll use their thread to * expedite it */ diff --git a/src/librbd/cache/pwl/rwl/WriteLog.h b/src/librbd/cache/pwl/rwl/WriteLog.h index 39d63776eca1..dcdd9880aa89 100644 --- a/src/librbd/cache/pwl/rwl/WriteLog.h +++ b/src/librbd/cache/pwl/rwl/WriteLog.h @@ -83,7 +83,8 @@ protected: void process_work() override; void schedule_append_ops(pwl::GenericLogOperations &ops) override; void append_scheduled_ops(void) override; - void reserve_cache(C_BlockIORequestT *req, bool &alloc_succeeds, bool &no_space) override; + void reserve_cache(C_BlockIORequestT *req, + bool &alloc_succeeds, bool &no_space) override; void collect_read_extents( uint64_t read_buffer_offset, LogMapEntry map_entry, std::vector &log_entries_to_read, @@ -97,7 +98,8 @@ protected: bool alloc_resources(C_BlockIORequestT *req) override; void schedule_flush_and_append(pwl::GenericLogOperationsVector &ops) override; void setup_schedule_append( - pwl::GenericLogOperationsVector &ops, bool do_early_flush) override; + pwl::GenericLogOperationsVector &ops, bool do_early_flush, + C_BlockIORequestT *req) override; Context *construct_flush_entry_ctx( const std::shared_ptr log_entry) override; void initialize_pool(Context *on_finish, pwl::DeferredContexts &later) override; diff --git a/src/librbd/cache/pwl/ssd/WriteLog.cc b/src/librbd/cache/pwl/ssd/WriteLog.cc index 845883124891..611f320bc3ce 100644 --- a/src/librbd/cache/pwl/ssd/WriteLog.cc +++ b/src/librbd/cache/pwl/ssd/WriteLog.cc @@ -371,8 +371,13 @@ void WriteLog::schedule_append_ops(GenericLogOperations &ops) { template void WriteLog::setup_schedule_append(pwl::GenericLogOperationsVector &ops, - bool do_early_flush) { + bool do_early_flush, + C_BlockIORequestT *req) { this->schedule_append(ops); + if (this->get_persist_on_flush()) { + req->complete_user_request(0); + } + req->release_cell(); } template diff --git a/src/librbd/cache/pwl/ssd/WriteLog.h b/src/librbd/cache/pwl/ssd/WriteLog.h index e8236be76df3..64b4baa38bcd 100644 --- a/src/librbd/cache/pwl/ssd/WriteLog.h +++ b/src/librbd/cache/pwl/ssd/WriteLog.h @@ -50,7 +50,8 @@ public: bool alloc_resources(C_BlockIORequestT *req) override; void setup_schedule_append( - pwl::GenericLogOperationsVector &ops, bool do_early_flush) override; + pwl::GenericLogOperationsVector &ops, bool do_early_flush, + C_BlockIORequestT *req) override; void complete_user_request(Context *&user_req, int r) override; protected: diff --git a/src/test/librbd/cache/pwl/test_mock_SSDWriteLog.cc b/src/test/librbd/cache/pwl/test_mock_SSDWriteLog.cc index 63eebd0c28d4..658cb1de34a3 100644 --- a/src/test/librbd/cache/pwl/test_mock_SSDWriteLog.cc +++ b/src/test/librbd/cache/pwl/test_mock_SSDWriteLog.cc @@ -81,7 +81,7 @@ struct TestMockCacheSSDWriteLog : public TestMockFixture { ASSERT_EQ(present, state.present); ASSERT_EQ(empty, state.empty); ASSERT_EQ(clean, state.clean); - + ASSERT_EQ(host, state.host); ASSERT_EQ(path, state.path); ASSERT_EQ(size, state.size); @@ -127,7 +127,7 @@ TEST_F(TestMockCacheSSDWriteLog, init_state_write) { MockImageCacheStateSSD image_cache_state(&mock_image_ctx, mock_api); validate_cache_state(ictx, image_cache_state, false, true, true, "", "", 0); - + image_cache_state.empty = false; image_cache_state.clean = false; MockContextSSD finish_ctx; @@ -206,10 +206,10 @@ TEST_F(TestMockCacheSSDWriteLog, write) { ASSERT_EQ(0, open_image(m_image_name, &ictx)); MockImageCtx mock_image_ctx(*ictx); - MockImageWriteback mock_image_writeback(mock_image_ctx); - MockApi mock_api; - MockSSDWriteLog rwl( - mock_image_ctx, get_cache_state(mock_image_ctx, mock_api), + MockImageWriteback mock_image_writeback(mock_image_ctx); + MockApi mock_api; + MockSSDWriteLog rwl( + mock_image_ctx, get_cache_state(mock_image_ctx, mock_api), mock_image_writeback, mock_api); MockContextSSD finish_ctx1; @@ -609,6 +609,174 @@ TEST_F(TestMockCacheSSDWriteLog, invalidate) { ASSERT_EQ(0, finish_ctx3.wait()); } +TEST_F(TestMockCacheSSDWriteLog, flush) { + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockImageCtx mock_image_ctx(*ictx); + MockImageWriteback mock_image_writeback(mock_image_ctx); + MockApi mock_api; + MockSSDWriteLog rwl( + mock_image_ctx, get_cache_state(mock_image_ctx, mock_api), + mock_image_writeback, mock_api); + + expect_op_work_queue(mock_image_ctx); + expect_metadata_set(mock_image_ctx); + + MockContextSSD finish_ctx1; + expect_context_complete(finish_ctx1, 0); + rwl.init(&finish_ctx1); + ASSERT_EQ(0, finish_ctx1.wait()); + + MockContextSSD finish_ctx2; + expect_context_complete(finish_ctx2, 0); + Extents image_extents{{0, 4096}}; + bufferlist bl; + bl.append(std::string(4096, '1')); + bufferlist bl_copy = bl; + int fadvise_flags = 0; + rwl.write(std::move(image_extents), std::move(bl), fadvise_flags, &finish_ctx2); + ASSERT_EQ(0, finish_ctx2.wait()); + + MockContextSSD finish_ctx_flush; + expect_context_complete(finish_ctx_flush, 0); + rwl.flush(&finish_ctx_flush); + ASSERT_EQ(0, finish_ctx_flush.wait()); + + MockContextSSD finish_ctx3; + expect_context_complete(finish_ctx3, 0); + rwl.shut_down(&finish_ctx3); + + ASSERT_EQ(0, finish_ctx3.wait()); +} + +TEST_F(TestMockCacheSSDWriteLog, flush_source_shutdown) { + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockImageCtx mock_image_ctx(*ictx); + MockImageWriteback mock_image_writeback(mock_image_ctx); + MockApi mock_api; + MockSSDWriteLog rwl( + mock_image_ctx, get_cache_state(mock_image_ctx, mock_api), + mock_image_writeback, mock_api); + + expect_op_work_queue(mock_image_ctx); + expect_metadata_set(mock_image_ctx); + + MockContextSSD finish_ctx1; + expect_context_complete(finish_ctx1, 0); + rwl.init(&finish_ctx1); + ASSERT_EQ(0, finish_ctx1.wait()); + + MockContextSSD finish_ctx2; + expect_context_complete(finish_ctx2, 0); + Extents image_extents{{0, 4096}}; + bufferlist bl; + bl.append(std::string(4096, '1')); + int fadvise_flags = 0; + rwl.write(std::move(image_extents), std::move(bl), fadvise_flags, &finish_ctx2); + ASSERT_EQ(0, finish_ctx2.wait()); + + MockContextSSD finish_ctx_flush; + expect_context_complete(finish_ctx_flush, 0); + rwl.flush(io::FLUSH_SOURCE_SHUTDOWN, &finish_ctx_flush); + ASSERT_EQ(0, finish_ctx_flush.wait()); + + MockContextSSD finish_ctx3; + expect_context_complete(finish_ctx3, 0); + rwl.shut_down(&finish_ctx3); + ASSERT_EQ(0, finish_ctx3.wait()); +} + + +TEST_F(TestMockCacheSSDWriteLog, flush_source_internal) { + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockImageCtx mock_image_ctx(*ictx); + MockImageWriteback mock_image_writeback(mock_image_ctx); + MockApi mock_api; + MockSSDWriteLog rwl( + mock_image_ctx, get_cache_state(mock_image_ctx, mock_api), + mock_image_writeback, mock_api); + + expect_op_work_queue(mock_image_ctx); + expect_metadata_set(mock_image_ctx); + + MockContextSSD finish_ctx1; + expect_context_complete(finish_ctx1, 0); + rwl.init(&finish_ctx1); + ASSERT_EQ(0, finish_ctx1.wait()); + + MockContextSSD finish_ctx2; + expect_context_complete(finish_ctx2, 0); + Extents image_extents{{0, 4096}}; + bufferlist bl; + bl.append(std::string(4096, '1')); + int fadvise_flags = 0; + rwl.write(std::move(image_extents), std::move(bl), fadvise_flags, &finish_ctx2); + ASSERT_EQ(0, finish_ctx2.wait()); + + MockContextSSD finish_ctx_flush; + expect_context_complete(finish_ctx_flush, 0); + rwl.flush(io::FLUSH_SOURCE_INTERNAL, &finish_ctx_flush); + ASSERT_EQ(0, finish_ctx_flush.wait()); + + MockContextSSD finish_ctx3; + expect_context_complete(finish_ctx3, 0); + rwl.shut_down(&finish_ctx3); + ASSERT_EQ(0, finish_ctx3.wait()); +} + +TEST_F(TestMockCacheSSDWriteLog, flush_source_user) { + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockImageCtx mock_image_ctx(*ictx); + MockImageWriteback mock_image_writeback(mock_image_ctx); + MockApi mock_api; + MockSSDWriteLog rwl( + mock_image_ctx, get_cache_state(mock_image_ctx, mock_api), + mock_image_writeback, mock_api); + expect_op_work_queue(mock_image_ctx); + expect_metadata_set(mock_image_ctx); + + MockContextSSD finish_ctx1; + expect_context_complete(finish_ctx1, 0); + rwl.init(&finish_ctx1); + ASSERT_EQ(0, finish_ctx1.wait()); + + MockContextSSD finish_ctx2; + expect_context_complete(finish_ctx2, 0); + Extents image_extents{{0, 4096}}; + bufferlist bl; + bl.append(std::string(4096, '1')); + int fadvise_flags = 0; + rwl.write(std::move(image_extents), std::move(bl), fadvise_flags, &finish_ctx2); + ASSERT_EQ(0, finish_ctx2.wait()); + + usleep(10000); + MockContextSSD finish_ctx_flush; + expect_context_complete(finish_ctx_flush, 0); + rwl.flush(io::FLUSH_SOURCE_USER, &finish_ctx_flush); + ASSERT_EQ(0, finish_ctx_flush.wait()); + + MockContextSSD finish_ctx3; + expect_context_complete(finish_ctx3, 0); + Extents image_extents2{{0, 4096}}; + bufferlist bl2; + bl2.append(std::string(4096, '1')); + int fadvise_flags2 = 0; + rwl.write(std::move(image_extents2), std::move(bl2), fadvise_flags2, &finish_ctx3); + ASSERT_EQ(0, finish_ctx3.wait()); + + MockContextSSD finish_ctx4; + expect_context_complete(finish_ctx4, 0); + rwl.shut_down(&finish_ctx4); + ASSERT_EQ(0, finish_ctx4.wait()); +} + } // namespace pwl } // namespace cache } // namespace librbd