From 6793f3e64cc23d655dc2e1535e026d6443763d48 Mon Sep 17 00:00:00 2001 From: Yin Congmin Date: Tue, 22 Jun 2021 15:15:27 +0800 Subject: [PATCH] librdb/cache/pwl/ssd: fix m_bytes_allocated exceeding m_bytes_allocated_cap SSD cache mode uses m_bytes_allocated and m_bytes_allocated_cap to control whether the allocation is successful. m_bytes_allocated may exceeding m_bytes_allocated_cap due to the multi-thread under no lock. When adding to m_bytes_allocated, check again to solve this issue. On the other hand, m_bytes_allocated_cap should be less than the actual ring buffer size. keep m_first_free_entry == m_first_valid_entry only means the cache is empty. Fixes: https://tracker.ceph.com/issues/50670 Signed-off-by: Yin Congmin (cherry picked from commit eee12082ff77afd7a69c6261acf5c1f59f1f9fed) --- src/librbd/cache/pwl/AbstractWriteLog.cc | 4 ++-- src/librbd/cache/pwl/ssd/WriteLog.cc | 13 ++++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc index 54d4f4d1a8e58..40b7fcb7daf3f 100644 --- a/src/librbd/cache/pwl/AbstractWriteLog.cc +++ b/src/librbd/cache/pwl/AbstractWriteLog.cc @@ -1512,7 +1512,8 @@ bool AbstractWriteLog::check_allocation( /* We need one free log entry per extent (each is a separate entry), and * one free "lane" for remote replication. */ if ((m_free_lanes >= num_lanes) && - (m_free_log_entries >= num_log_entries)) { + (m_free_log_entries >= num_log_entries) && + (m_bytes_allocated_cap >= m_bytes_allocated + bytes_allocated)) { m_free_lanes -= num_lanes; m_free_log_entries -= num_log_entries; m_unpublished_reserves += num_unpublished_reserves; @@ -1522,7 +1523,6 @@ bool AbstractWriteLog::check_allocation( if (req->has_io_waited_for_buffers()) { req->set_io_waited_for_buffers(false); } - } else { alloc_succeeds = false; } diff --git a/src/librbd/cache/pwl/ssd/WriteLog.cc b/src/librbd/cache/pwl/ssd/WriteLog.cc index fda9d8187e176..39795084c9a6a 100644 --- a/src/librbd/cache/pwl/ssd/WriteLog.cc +++ b/src/librbd/cache/pwl/ssd/WriteLog.cc @@ -169,9 +169,12 @@ bool WriteLog::initialize_pool(Context *on_finish, m_cache_state->empty = true; /* new pool, calculate and store metadata */ - /* Size of ring buffer */ - this->m_bytes_allocated_cap = - this->m_log_pool_size - DATA_RING_BUFFER_OFFSET; + /* Keep ring buffer at least MIN_WRITE_ALLOC_SSD_SIZE bytes free. + * In this way, when all ring buffer spaces are allocated, + * m_first_free_entry and m_first_valid_entry will not be equal. + * Equal only means the cache is empty. */ + this->m_bytes_allocated_cap = this->m_log_pool_size - + DATA_RING_BUFFER_OFFSET - MIN_WRITE_ALLOC_SSD_SIZE; /* Log ring empty */ m_first_free_entry = DATA_RING_BUFFER_OFFSET; m_first_valid_entry = DATA_RING_BUFFER_OFFSET; @@ -257,8 +260,8 @@ void WriteLog::load_existing_entries(pwl::DeferredContexts &later) { this->m_first_valid_entry = pool_root.first_valid_entry; this->m_first_free_entry = pool_root.first_free_entry; - this->m_bytes_allocated_cap = - this->m_log_pool_size - DATA_RING_BUFFER_OFFSET; + this->m_bytes_allocated_cap = this->m_log_pool_size - + DATA_RING_BUFFER_OFFSET - MIN_WRITE_ALLOC_SSD_SIZE; std::map> sync_point_entries; -- 2.47.3