From 2d337fb122d147e32d027d1e7211cd4156a5b72b Mon Sep 17 00:00:00 2001 From: Jianpeng Ma Date: Fri, 20 Aug 2021 14:29:37 +0800 Subject: [PATCH] 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 --- src/librbd/cache/pwl/ssd/WriteLog.cc | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/librbd/cache/pwl/ssd/WriteLog.cc b/src/librbd/cache/pwl/ssd/WriteLog.cc index d6826086553..05458d4f757 100644 --- a/src/librbd/cache/pwl/ssd/WriteLog.cc +++ b/src/librbd/cache/pwl/ssd/WriteLog.cc @@ -666,9 +666,21 @@ bool WriteLog::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::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(pool_root); new_root->flushed_sync_gen = flushed_sync_gen; -- 2.47.3