From 311bcf17d5fca40edd07b04d847434ade3e29410 Mon Sep 17 00:00:00 2001 From: Yin Congmin Date: Fri, 9 Apr 2021 23:01:52 +0800 Subject: [PATCH] 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) --- src/librbd/cache/pwl/AbstractWriteLog.cc | 5 ++++- src/librbd/cache/pwl/AbstractWriteLog.h | 2 +- src/librbd/cache/pwl/InitRequest.cc | 4 ++-- src/librbd/cache/pwl/rwl/WriteLog.cc | 15 ++++++++------- src/librbd/cache/pwl/rwl/WriteLog.h | 2 +- src/librbd/cache/pwl/ssd/WriteLog.cc | 6 ++++-- src/librbd/cache/pwl/ssd/WriteLog.h | 2 +- 7 files changed, 21 insertions(+), 15 deletions(-) diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc index 388f1dcee6494..7b8aa43025047 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 fb6a07992b9e4..c982d2631fe3f 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 ea00d4fbc7664..a6d0fb44a1de1 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 38d0e9131f8c3..80a83510c3163 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 1d1deb2c1fcb5..dabee07d742aa 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 68619fa6b3c98..e669dd6ce0020 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 62e1db9f35acf..d96436c988510 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; -- 2.39.5