From: Yin Congmin <congmin.yin@intel.com>
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: v16.2.7~50^2~28
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=6793f3e64cc23d655dc2e1535e026d6443763d48;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 <congmin.yin@intel.com>
(cherry picked from commit eee12082ff77afd7a69c6261acf5c1f59f1f9fed)
---

diff --git a/src/librbd/cache/pwl/AbstractWriteLog.cc b/src/librbd/cache/pwl/AbstractWriteLog.cc
index 54d4f4d1a8e5..40b7fcb7daf3 100644
--- a/src/librbd/cache/pwl/AbstractWriteLog.cc
+++ b/src/librbd/cache/pwl/AbstractWriteLog.cc
@@ -1512,7 +1512,8 @@ bool AbstractWriteLog<I>::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<I>::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 fda9d8187e17..39795084c9a6 100644
--- a/src/librbd/cache/pwl/ssd/WriteLog.cc
+++ b/src/librbd/cache/pwl/ssd/WriteLog.cc
@@ -169,9 +169,12 @@ bool WriteLog<I>::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<I>::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<uint64_t, std::shared_ptr<SyncPointLogEntry>> sync_point_entries;