librbd/cache/pwl: Fix dead lock issue when pwl initialization failed

when pwl initialization failed, 'AbstractWriteLog' will release itself
in callback, it hold guard lock and want to get lock to delete data,
which causes dead lock. This PR works by release image_cache outside
the callback function.

Signed-off-by: Yin Congmin <congmin.yin@intel.com>

diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc
index 76e8e478e2bf7487907ba7e680b871133a1f9f2a..0b6254e3d9d6608ec0e013ee1f7f8e26ad003199 100644 (file)
--- a/src/librbd/cache/pwl/AbstractWriteLog.cc
+++ b/src/librbd/cache/pwl/AbstractWriteLog.cc
@@ -529,7 +529,10 @@ void AbstractWriteLog<I>::pwl_init(Context *on_finish, DeferredContexts &later)
     on_finish->complete(-errno);
   }
 
-  initialize_pool(on_finish, later);
+  bool succeeded = initialize_pool(on_finish, later);
+  if (!succeeded) {
+    return ;
+  }
 
   ldout(cct,1) << "pool " << m_log_pool_name << " has " << m_total_log_entries
                << " log entries, " << m_free_log_entries << " of which are free."

diff --git a/src/librbd/cache/pwl/AbstractWriteLog.h b/src/librbd/cache/pwl/AbstractWriteLog.h
index 56ecd6c1c209111beee6bb702af15195a5dfe439..9e23e00a66c3f4e7698ffed0307dbab9e78395cf 100644 (file)
--- a/src/librbd/cache/pwl/AbstractWriteLog.h
+++ b/src/librbd/cache/pwl/AbstractWriteLog.h
@@ -365,7 +365,7 @@ protected:
   virtual void append_scheduled_ops(void) = 0;
   virtual void schedule_append_ops(pwl::GenericLogOperations &ops) = 0;
   virtual void remove_pool_file() = 0;
-  virtual void initialize_pool(Context *on_finish,
+  virtual bool initialize_pool(Context *on_finish,
                                pwl::DeferredContexts &later) = 0;
   virtual void collect_read_extents(
       uint64_t read_buffer_offset, LogMapEntry<GenericWriteLogEntry> map_entry,

diff --git a/src/librbd/cache/pwl/InitRequest.cc b/src/librbd/cache/pwl/InitRequest.cc
index ea00d4fbc7664410a19b0f276c90a24d9030cd52..a6d0fb44a1de1b138da5a9a80ac11172ff72a3da 100644 (file)
--- a/src/librbd/cache/pwl/InitRequest.cc
+++ b/src/librbd/cache/pwl/InitRequest.cc
@@ -122,8 +122,8 @@ void InitRequest<I>::init_image_cache() {
   ldout(cct, 10) << dendl;
 
   using klass = InitRequest<I>;
-  Context *ctx = create_context_callback<
-    klass, &klass::handle_init_image_cache>(this);
+  Context *ctx = create_async_context_callback(m_image_ctx,
+    create_context_callback<klass, &klass::handle_init_image_cache>(this));
   m_image_cache->init(ctx);
 }
 

diff --git a/src/librbd/cache/pwl/rwl/WriteLog.cc b/src/librbd/cache/pwl/rwl/WriteLog.cc
index 0dba120f9310720a01978f9a04f1cd3728ddcb47..4f563d739da14fe66789aaad4709ce39e2ad4439 100644 (file)
--- a/src/librbd/cache/pwl/rwl/WriteLog.cc
+++ b/src/librbd/cache/pwl/rwl/WriteLog.cc
@@ -258,7 +258,7 @@ void WriteLog<I>::remove_pool_file() {
 }
 
 template <typename I>
-void WriteLog<I>::initialize_pool(Context *on_finish, pwl::DeferredContexts &later) {
+bool WriteLog<I>::initialize_pool(Context *on_finish, pwl::DeferredContexts &later) {
   CephContext *cct = m_image_ctx.cct;
   TOID(struct WriteLogPoolRoot) pool_root;
   ceph_assert(ceph_mutex_is_locked_by_me(m_lock));
@@ -275,7 +275,7 @@ void WriteLog<I>::initialize_pool(Context *on_finish, pwl::DeferredContexts &lat
       m_cache_state->empty = true;
       /* TODO: filter/replace errnos that are meaningless to the caller */
       on_finish->complete(-errno);
-      return;
+      return false;
     }
     m_cache_state->present = true;
     m_cache_state->clean = true;
@@ -292,7 +292,7 @@ void WriteLog<I>::initialize_pool(Context *on_finish, pwl::DeferredContexts &lat
     if (num_small_writes <= 2) {
       lderr(cct) << "num_small_writes needs to > 2" << dendl;
       on_finish->complete(-EINVAL);
-      return;
+      return false;
     }
     this->m_log_pool_actual_size = this->m_log_pool_config_size;
     this->m_bytes_allocated_cap = effective_pool_size;
@@ -319,7 +319,7 @@ void WriteLog<I>::initialize_pool(Context *on_finish, pwl::DeferredContexts &lat
       this->m_free_log_entries = 0;
       lderr(cct) << "failed to initialize pool (" << this->m_log_pool_name << ")" << dendl;
       on_finish->complete(-pmemobj_tx_errno());
-      return;
+      return false;
     } TX_FINALLY {
     } TX_END;
   } else {
@@ -331,7 +331,7 @@ void WriteLog<I>::initialize_pool(Context *on_finish, pwl::DeferredContexts &lat
       lderr(cct) << "failed to open pool (" << this->m_log_pool_name << "): "
                  << pmemobj_errormsg() << dendl;
       on_finish->complete(-errno);
-      return;
+      return false;
     }
     pool_root = POBJ_ROOT(m_log_pool, struct WriteLogPoolRoot);
     if (D_RO(pool_root)->header.layout_version != RWL_POOL_VERSION) {
@@ -340,13 +340,13 @@ void WriteLog<I>::initialize_pool(Context *on_finish, pwl::DeferredContexts &lat
                  << D_RO(pool_root)->header.layout_version
                  << " expected " << RWL_POOL_VERSION << dendl;
       on_finish->complete(-EINVAL);
-      return;
+      return false;
     }
     if (D_RO(pool_root)->block_size != MIN_WRITE_ALLOC_SIZE) {
       lderr(cct) << "Pool block size is " << D_RO(pool_root)->block_size
                  << " expected " << MIN_WRITE_ALLOC_SIZE << dendl;
       on_finish->complete(-EINVAL);
-      return;
+      return false;
     }
     this->m_log_pool_actual_size = D_RO(pool_root)->pool_size;
     this->m_flushed_sync_gen = D_RO(pool_root)->flushed_sync_gen;
@@ -370,6 +370,7 @@ void WriteLog<I>::initialize_pool(Context *on_finish, pwl::DeferredContexts &lat
     m_cache_state->clean = this->m_dirty_log_entries.empty();
     m_cache_state->empty = m_log_entries.empty();
   }
+  return true;
 }
 
 /*

diff --git a/src/librbd/cache/pwl/rwl/WriteLog.h b/src/librbd/cache/pwl/rwl/WriteLog.h
index 7aea8573c917323b1e5a8aee4b13a0fbe9cfe370..17b0d4388dcdb085e94eb07dc83f7b19d7422a24 100644 (file)
--- a/src/librbd/cache/pwl/rwl/WriteLog.h
+++ b/src/librbd/cache/pwl/rwl/WriteLog.h
@@ -103,7 +103,7 @@ protected:
       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;
+  bool initialize_pool(Context *on_finish, pwl::DeferredContexts &later) override;
   void write_data_to_buffer(
       std::shared_ptr<pwl::WriteLogEntry> ws_entry,
       pwl::WriteLogCacheEntry *pmem_entry) override;

diff --git a/src/librbd/cache/pwl/ssd/WriteLog.cc b/src/librbd/cache/pwl/ssd/WriteLog.cc
index 725039a81f659aba0492631666555ec4dd8c8fd8..2e807b6fe4dd06531bbc30d31ba07c3d79fbc73c 100644 (file)
--- a/src/librbd/cache/pwl/ssd/WriteLog.cc
+++ b/src/librbd/cache/pwl/ssd/WriteLog.cc
@@ -95,7 +95,7 @@ void WriteLog<I>::complete_read(
 }
 
 template <typename I>
-void WriteLog<I>::initialize_pool(Context *on_finish,
+bool WriteLog<I>::initialize_pool(Context *on_finish,
                                   pwl::DeferredContexts &later) {
   CephContext *cct = m_image_ctx.cct;
   ceph_assert(ceph_mutex_is_locked_by_me(m_lock));
@@ -117,7 +117,7 @@ void WriteLog<I>::initialize_pool(Context *on_finish,
       m_cache_state->empty = true;
       /* TODO: filter/replace errnos that are meaningless to the caller */
       on_finish->complete(-errno);
-      return;
+      return false;
     }
 
     bdev = BlockDevice::create(cct, this->m_log_pool_name, aio_cache_cb,
@@ -126,7 +126,7 @@ void WriteLog<I>::initialize_pool(Context *on_finish,
     if (r < 0) {
       delete bdev;
       on_finish->complete(-1);
-      return;
+      return false;
     }
     m_cache_state->present = true;
     m_cache_state->clean = true;
@@ -173,7 +173,7 @@ void WriteLog<I>::initialize_pool(Context *on_finish,
      if (r < 0) {
        delete bdev;
        on_finish->complete(r);
-       return;
+       return false;
      }
      load_existing_entries(later);
      if (m_first_free_entry < m_first_valid_entry) {
@@ -192,6 +192,7 @@ void WriteLog<I>::initialize_pool(Context *on_finish,
      m_cache_state->clean = this->m_dirty_log_entries.empty();
      m_cache_state->empty = m_log_entries.empty();
   }
+  return true;
 }
 
 template <typename I>

diff --git a/src/librbd/cache/pwl/ssd/WriteLog.h b/src/librbd/cache/pwl/ssd/WriteLog.h
index 64b4baa38bcdde49b1d1c768898f6af9540f1bde..9c3647220a04b15943d8c3501cd56d5d5bca7ef7 100644 (file)
--- a/src/librbd/cache/pwl/ssd/WriteLog.h
+++ b/src/librbd/cache/pwl/ssd/WriteLog.h
@@ -63,7 +63,7 @@ protected:
   using AbstractWriteLog<ImageCtxT>::m_first_valid_entry;
   using AbstractWriteLog<ImageCtxT>::m_bytes_allocated;
 
-  void initialize_pool(Context *on_finish,
+  bool initialize_pool(Context *on_finish,
                        pwl::DeferredContexts &later) override;
   void process_work() override;
   void append_scheduled_ops(void) override;