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: v16.2.7~50^2~32 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=311bcf17d5fca40edd07b04d847434ade3e29410;p=ceph.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 (cherry picked from commit 937af36e204791554708632245b4bca1d52f49a6) --- diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc index 388f1dcee649..7b8aa4302504 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) return; } - 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 fb6a07992b9e..c982d2631fe3 100644 --- a/src/librbd/cache/pwl/AbstractWriteLog.h +++ b/src/librbd/cache/pwl/AbstractWriteLog.h @@ -364,7 +364,7 @@ protected: virtual void append_scheduled_ops(void) = 0; virtual void schedule_append_ops(pwl::GenericLogOperations &ops, C_BlockIORequestT *req) = 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 ea00d4fbc766..a6d0fb44a1de 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 38d0e9131f8c..80a83510c316 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_bytes_allocated_cap = effective_pool_size; /* Log ring empty */ @@ -318,7 +318,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 { @@ -330,7 +330,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) { @@ -339,13 +339,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_size = D_RO(pool_root)->pool_size; this->m_flushed_sync_gen = D_RO(pool_root)->flushed_sync_gen; @@ -369,6 +369,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 1d1deb2c1fcb..dabee07d742a 100644 --- a/src/librbd/cache/pwl/rwl/WriteLog.h +++ b/src/librbd/cache/pwl/rwl/WriteLog.h @@ -104,7 +104,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 68619fa6b3c9..e669dd6ce002 100644 --- a/src/librbd/cache/pwl/ssd/WriteLog.cc +++ b/src/librbd/cache/pwl/ssd/WriteLog.cc @@ -106,7 +106,8 @@ void WriteLog::complete_read( } template -int WriteLog::create_and_open_bdev() { +bool WriteLog::initialize_pool(Context *on_finish, + pwl::DeferredContexts &later) { CephContext *cct = m_image_ctx.cct; bdev = BlockDevice::create(cct, this->m_log_pool_name, aio_cache_cb, @@ -155,7 +156,7 @@ bool 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; } r = create_and_open_bdev(); @@ -204,6 +205,7 @@ bool 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 62e1db9f35ac..d96436c98851 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;