From: Yin Congmin Date: Tue, 22 Jun 2021 07:15:27 +0000 (+0800) Subject: librdb/cache/pwl/ssd: fix m_bytes_allocated exceeding m_bytes_allocated_cap X-Git-Tag: v17.1.0~1087^2~3 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=eee12082ff77afd7a69c6261acf5c1f59f1f9fed;p=ceph.git 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 --- diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc index 0dbe6d5a4632..884e60657060 100644 --- a/src/librbd/cache/pwl/AbstractWriteLog.cc +++ b/src/librbd/cache/pwl/AbstractWriteLog.cc @@ -1510,7 +1510,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; @@ -1520,7 +1521,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 b85e45544ee0..1b76ddc8f591 100644 --- a/src/librbd/cache/pwl/ssd/WriteLog.cc +++ b/src/librbd/cache/pwl/ssd/WriteLog.cc @@ -133,9 +133,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; @@ -216,8 +219,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;