From: Yin Congmin Date: Fri, 9 Apr 2021 15:01:52 +0000 (+0800) Subject: librbd/cache/pwl: Fix dead lock issue when pwl initialization failed X-Git-Tag: v17.1.0~2247^2 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=937af36e204791554708632245b4bca1d52f49a6;p=ceph-ci.git 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 --- diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc index 76e8e478e2b..0b6254e3d9d 100644 --- a/src/librbd/cache/pwl/AbstractWriteLog.cc +++ b/src/librbd/cache/pwl/AbstractWriteLog.cc @@ -529,7 +529,10 @@ void AbstractWriteLog::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 56ecd6c1c20..9e23e00a66c 100644 --- 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 map_entry, diff --git a/src/librbd/cache/pwl/InitRequest.cc b/src/librbd/cache/pwl/InitRequest.cc index ea00d4fbc76..a6d0fb44a1d 100644 --- a/src/librbd/cache/pwl/InitRequest.cc +++ b/src/librbd/cache/pwl/InitRequest.cc @@ -122,8 +122,8 @@ void InitRequest::init_image_cache() { ldout(cct, 10) << dendl; using klass = InitRequest; - Context *ctx = create_context_callback< - klass, &klass::handle_init_image_cache>(this); + Context *ctx = create_async_context_callback(m_image_ctx, + create_context_callback(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 0dba120f931..4f563d739da 100644 --- a/src/librbd/cache/pwl/rwl/WriteLog.cc +++ b/src/librbd/cache/pwl/rwl/WriteLog.cc @@ -258,7 +258,7 @@ void WriteLog::remove_pool_file() { } template -void WriteLog::initialize_pool(Context *on_finish, pwl::DeferredContexts &later) { +bool WriteLog::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::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::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::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::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::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::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 7aea8573c91..17b0d4388dc 100644 --- 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 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 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 725039a81f6..2e807b6fe4d 100644 --- a/src/librbd/cache/pwl/ssd/WriteLog.cc +++ b/src/librbd/cache/pwl/ssd/WriteLog.cc @@ -95,7 +95,7 @@ void WriteLog::complete_read( } template -void WriteLog::initialize_pool(Context *on_finish, +bool WriteLog::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::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::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::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::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 diff --git a/src/librbd/cache/pwl/ssd/WriteLog.h b/src/librbd/cache/pwl/ssd/WriteLog.h index 64b4baa38bc..9c3647220a0 100644 --- a/src/librbd/cache/pwl/ssd/WriteLog.h +++ b/src/librbd/cache/pwl/ssd/WriteLog.h @@ -63,7 +63,7 @@ protected: using AbstractWriteLog::m_first_valid_entry; using AbstractWriteLog::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;