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-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=refs%2Fpull%2F40718%2Fhead;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 --- diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc index 76e8e478e2bf..0b6254e3d9d6 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 56ecd6c1c209..9e23e00a66c3 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 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 0dba120f9310..4f563d739da1 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 7aea8573c917..17b0d4388dcd 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 725039a81f65..2e807b6fe4dd 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 64b4baa38bcd..9c3647220a04 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;