From: Ilya Dryomov <idryomov@gmail.com>
Date: Thu, 13 May 2021 11:11:57 +0000 (+0200)
Subject: librbd/cache/pwl/ssd: actually use first_{valid,free}_entry on recovery
X-Git-Tag: v17.1.0~1943^2~4
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=ef020b85fb16c1730fc08eadfd1b51d3c4cd019a;p=ceph.git

librbd/cache/pwl/ssd: actually use first_{valid,free}_entry on recovery

first_valid_entry and first_free_entry pointers are read from media
but not actually used: both m_first_valid_entry and m_first_free_entry
get assigned 0 (or garbage).  next_log_pos gets the same value as well
meaning that not only no recovery is attempted but the cache also gets
corrupted because DATA_RING_BUFFER_OFFSET is not applied.

Fixes: https://tracker.ceph.com/issues/50669
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---

diff --git a/src/librbd/cache/pwl/ssd/WriteLog.cc b/src/librbd/cache/pwl/ssd/WriteLog.cc
index 37855fac6864f..6d558d37f530f 100644
--- a/src/librbd/cache/pwl/ssd/WriteLog.cc
+++ b/src/librbd/cache/pwl/ssd/WriteLog.cc
@@ -210,16 +210,11 @@ void WriteLog<I>::load_existing_entries(pwl::DeferredContexts &later) {
   decode(superblock, p);
   ldout(cct,5) << "Decoded superblock" << dendl;
 
-  WriteLogPoolRoot current_pool_root = superblock.root;
-  uint64_t next_log_pos = pool_root.first_valid_entry;
-  uint64_t first_free_entry =  pool_root.first_free_entry;
-  uint64_t curr_log_pos;
-
-  pool_root = current_pool_root;
-  m_first_free_entry = first_free_entry;
-  m_first_valid_entry = next_log_pos;
-  this->m_flushed_sync_gen = current_pool_root.flushed_sync_gen;
-  this->m_log_pool_size = current_pool_root.pool_size;
+  pool_root = superblock.root;
+  this->m_log_pool_size = pool_root.pool_size;
+  this->m_flushed_sync_gen = pool_root.flushed_sync_gen;
+  this->m_first_valid_entry = pool_root.first_valid_entry;
+  this->m_first_free_entry = pool_root.first_free_entry;
 
   std::map<uint64_t, std::shared_ptr<SyncPointLogEntry>> sync_point_entries;
 
@@ -228,7 +223,8 @@ void WriteLog<I>::load_existing_entries(pwl::DeferredContexts &later) {
   // Iterate through the log_entries and append all the write_bytes
   // of each entry to fetch the pos of next 4k of log_entries. Iterate
   // through the log entries and append them to the in-memory vector
-  while (next_log_pos != first_free_entry) {
+  for (uint64_t next_log_pos = this->m_first_valid_entry;
+       next_log_pos != this->m_first_free_entry; ) {
     // read the entries from SSD cache and decode
     bufferlist bl_entries;
     ::IOContext ioctx_entry(cct, nullptr);
@@ -238,7 +234,7 @@ void WriteLog<I>::load_existing_entries(pwl::DeferredContexts &later) {
     auto pl = bl_entries.cbegin();
     decode(ssd_log_entries, pl);
     ldout(cct, 5) << "decoded ssd log entries" << dendl;
-    curr_log_pos = next_log_pos;
+    uint64_t curr_log_pos = next_log_pos;
     std::shared_ptr<GenericLogEntry> log_entry = nullptr;
 
     for (auto it = ssd_log_entries.begin(); it != ssd_log_entries.end(); ++it) {