From: Jianpeng Ma <jianpeng.ma@intel.com>
Date: Fri, 20 Aug 2021 06:29:37 +0000 (+0800)
Subject: librbd/cache/pwl/ssd: fix first_valid_entry calculation in retire_entries()
X-Git-Tag: v17.1.0~1006^2
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=2d337fb122d147e32d027d1e7211cd4156a5b72b;p=ceph.git

librbd/cache/pwl/ssd: fix first_valid_entry calculation in retire_entries()

Consider one control_block which cotain multi encode(WriteLogCacheEntry):
Log1: WriteLogEntry
Log2: WriteLogEntry
Log3: Non-WriteLogEntry
For this case, currently calc method is: control_block_pos + sizeof(control_block).
But in fact, it should: control_block_pos + sizeof(control_block) +
data_length(Log1 + Log2).

Wrong first_valid_entry will persist to superblock and restart to read.
This cause read wrong position and when decode(WriteLogCacheEntry) it
will report bug.

Fixes: https://tracker.ceph.com/issues/52323
Signed-off-by: Jianpeng Ma <jianpeng.ma@intel.com>
---

diff --git a/src/librbd/cache/pwl/ssd/WriteLog.cc b/src/librbd/cache/pwl/ssd/WriteLog.cc
index d68260865532..05458d4f7573 100644
--- a/src/librbd/cache/pwl/ssd/WriteLog.cc
+++ b/src/librbd/cache/pwl/ssd/WriteLog.cc
@@ -666,9 +666,21 @@ bool WriteLog<I>::retire_entries(const unsigned long int frees_per_tx) {
             this->m_blocks_to_log_entries.remove_log_entry(write_entry);
           }
         }
+
+        ldout(cct, 20) << "span with " << retiring_subentries.size()
+                       << " entries: control_block_pos=" << control_block_pos
+                       << " data_length=" << data_length
+                       << dendl;
         retiring_entries.insert(
             retiring_entries.end(), retiring_subentries.begin(),
             retiring_subentries.end());
+
+        first_valid_entry = control_block_pos + data_length +
+            MIN_WRITE_ALLOC_SSD_SIZE;
+        if (first_valid_entry >= this->m_log_pool_size) {
+          first_valid_entry = first_valid_entry % this->m_log_pool_size +
+              DATA_RING_BUFFER_OFFSET;
+        }
       } else {
         break;
       }
@@ -686,18 +698,6 @@ bool WriteLog<I>::retire_entries(const unsigned long int frees_per_tx) {
       flushed_sync_gen = this->m_flushed_sync_gen;
     }
 
-    //calculate new first_valid_entry based on last entry to retire
-    auto entry = retiring_entries.back();
-    if (entry->is_write_entry() || entry->is_writesame_entry()) {
-      first_valid_entry = entry->ram_entry.write_data_pos +
-        entry->get_aligned_data_size();
-    } else {
-      first_valid_entry = entry->log_entry_index + MIN_WRITE_ALLOC_SSD_SIZE;
-    }
-    if (first_valid_entry >= this->m_log_pool_size) {
-      first_valid_entry = first_valid_entry % this->m_log_pool_size +
-          DATA_RING_BUFFER_OFFSET;
-    }
     ceph_assert(first_valid_entry != initial_first_valid_entry);
     auto new_root = std::make_shared<WriteLogPoolRoot>(pool_root);
     new_root->flushed_sync_gen = flushed_sync_gen;