librbd/cache/pwl: Fix user request completion in

... persist_on_flush mode

Signed-off-by: Mahati Chamarthy <mahati.chamarthy@intel.com>

diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc
index e038a3fd65d1b3138e30a8111854e73eab6d4507..228ea13ca421839704abc4b842079cae0c63683a 100644 (file)
--- a/src/librbd/cache/pwl/AbstractWriteLog.cc
+++ b/src/librbd/cache/pwl/AbstractWriteLog.cc
@@ -1250,6 +1250,11 @@ void AbstractWriteLog<I>::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<I>::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 60accc47ce1da0e348934146592a702559f29376..29b6c3b9c7090e89ccc3fcb2e1cddfb07f05e89a 100644 (file)
--- 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 28e15bb11fd4f6daab995b9b239f85583265072f..2fa9643aba9cefe6086073bc594f183482ef6afe 100644 (file)
--- a/src/librbd/cache/pwl/Request.cc
+++ b/src/librbd/cache/pwl/Request.cc
@@ -262,7 +262,7 @@ bool C_WriteRequest<T>::append_write_request(std::shared_ptr<SyncPoint> sync_poi
 template <typename T>
 void C_WriteRequest<T>::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 ebe82c83fa9fc86b9521b73a6a028a43d600c1b0..86d12c14f04810ad2624151397cc80eb964b19a3 100644 (file)
--- a/src/librbd/cache/pwl/rwl/WriteLog.cc
+++ b/src/librbd/cache/pwl/rwl/WriteLog.cc
@@ -675,7 +675,8 @@ void WriteLog<I>::enlist_op_flusher()
 
 template <typename I>
 void WriteLog<I>::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 39d63776eca1db02f09fb607204632cdd00d2d1d..dcdd9880aa89e5ede1315a70d845594c1f719037 100644 (file)
--- 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<GenericWriteLogEntry> map_entry,
       std::vector<WriteLogCacheEntry*> &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<pwl::GenericLogEntry> 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 8458831248913b139a68956a04b3240ffeaf35a5..611f320bc3ce4b49ffc68ee43454263ec71b85a4 100644 (file)
--- a/src/librbd/cache/pwl/ssd/WriteLog.cc
+++ b/src/librbd/cache/pwl/ssd/WriteLog.cc
@@ -371,8 +371,13 @@ void WriteLog<I>::schedule_append_ops(GenericLogOperations &ops) {
 
 template <typename I>
 void WriteLog<I>::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 <typename I>

diff --git a/src/librbd/cache/pwl/ssd/WriteLog.h b/src/librbd/cache/pwl/ssd/WriteLog.h
index e8236be76df3ce6e744e423d8efff796e71d9d2f..64b4baa38bcdde49b1d1c768898f6af9540f1bde 100644 (file)
--- 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 63eebd0c28d4541b2cfb029c545b66c128bd87bc..658cb1de34a3ad16444c3d3b64162d5b7304db0f 100644 (file)
--- 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