From 6651dad143236cf1c938b30121737716e30e0952 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Wed, 19 Jan 2022 10:29:25 +0800 Subject: crimson/os/seastore/cache: misc cleanup Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/cache.cc | 37 +++++++++++++++---------------------- src/crimson/os/seastore/cache.h | 23 +++++++++-------------- 2 files changed, 24 insertions(+), 36 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 67273d7b0d6..3ee1d6ec838 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -616,7 +616,6 @@ void Cache::add_extent(CachedExtentRef ref) LOG_PREFIX(Cache::add_extent); assert(ref->is_valid()); extents.insert(*ref); - if (ref->is_dirty()) { add_to_dirty(ref); } else { @@ -680,18 +679,10 @@ void Cache::commit_retire_extent( { LOG_PREFIX(Cache::commit_retire_extent); DEBUGT("extent {}", t, *ref); - assert(ref->is_valid()); + remove_extent(ref); - // TODO: why does this duplicate remove_extent? - if (ref->is_dirty()) { - remove_from_dirty(ref); - } else { - lru.remove_from_lru(*ref); - } ref->dirty_from_or_retired_at = JOURNAL_SEQ_MAX; - invalidate_extent(t, *ref); - extents.erase(*ref); } void Cache::commit_replace_extent( @@ -851,7 +842,7 @@ CachedExtentRef Cache::alloc_new_extent_by_type( { switch (type) { case extent_types_t::ROOT: - assert(0 == "ROOT is never directly alloc'd"); + ceph_assert(0 == "ROOT is never directly alloc'd"); return CachedExtentRef(); case extent_types_t::LADDR_INTERNAL: return alloc_new_extent(t, length, delay); @@ -929,11 +920,10 @@ record_t Cache::prepare_record(Transaction &t) } DEBUGT("read_set validated", t); t.read_set.clear(); + t.write_set.clear(); record_t record; - t.write_set.clear(); - // Add new copy of mutated blocks, set_io_wait to block until written record.deltas.reserve(t.mutated_block_list.size()); for (auto &i: t.mutated_block_list) { @@ -954,6 +944,7 @@ record_t Cache::prepare_record(Transaction &t) assert(i->get_version() > 0); auto final_crc = i->get_crc32c(); if (i->get_type() == extent_types_t::ROOT) { + assert(t.root == i); root = t.root; DEBUGT("writing out root delta for {}", t, *t.root); record.push_back( @@ -1022,7 +1013,7 @@ record_t Cache::prepare_record(Transaction &t) i->prepare_write(); bl.append(i->get_bptr()); if (i->get_type() == extent_types_t::ROOT) { - assert(0 == "ROOT never gets written as a fresh block"); + ceph_assert(0 == "ROOT never gets written as a fresh block"); } assert(bl.length() == i->get_length()); @@ -1208,7 +1199,7 @@ Cache::close_ertr::future<> Cache::close() intrusive_ptr_release(ptr); } assert(stats.dirty_bytes == 0); - clear_lru(); + lru.clear(); return close_ertr::now(); } @@ -1297,16 +1288,18 @@ Cache::get_next_dirty_extents_ret Cache::get_next_dirty_extents( for (auto i = dirty.begin(); i != dirty.end() && bytes_so_far < max_bytes; ++i) { - if (i->get_dirty_from() != journal_seq_t() && i->get_dirty_from() < seq) { + auto dirty_from = i->get_dirty_from(); + ceph_assert(dirty_from != journal_seq_t() && + dirty_from != JOURNAL_SEQ_MAX && + dirty_from != NO_DELTAS); + if (dirty_from < seq) { DEBUGT("next {}", t, *i); - if (!(cand.empty() || - cand.back()->get_dirty_from() <= i->get_dirty_from())) { + if (!cand.empty() && cand.back()->get_dirty_from() > dirty_from) { ERRORT("last {}, next {}", t, *cand.back(), *i); + ceph_abort(); } - assert(cand.empty() || cand.back()->get_dirty_from() <= i->get_dirty_from()); bytes_so_far += i->get_length(); cand.push_back(&*i); - } else { break; } @@ -1337,7 +1330,7 @@ Cache::get_next_dirty_extents_ret Cache::get_next_dirty_extents( if (ext->get_type() == extent_types_t::ROOT) { if (t.root) { assert(&*t.root == &*ext); - assert(0 == "t.root would have to already be in the read set"); + ceph_assert(0 == "t.root would have to already be in the read set"); } else { assert(&*ext == &*root); t.root = root; @@ -1403,7 +1396,7 @@ Cache::get_extent_ertr::future Cache::_get_extent_by_type( switch (type) { case extent_types_t::ROOT: - assert(0 == "ROOT is never directly read"); + ceph_assert(0 == "ROOT is never directly read"); return get_extent_ertr::make_ready_future(); case extent_types_t::LADDR_INTERNAL: return get_extent( diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index a7de00d807d..515d62c698b 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -336,13 +336,6 @@ public: return get_extent(t, offset, length, [](T &){}); } - - /** - * get_extent_by_type - * - * Based on type, instantiate the correct concrete type - * and read in the extent at location offset~length. - */ private: // This is a workaround std::move_only_function not being available, // not really worth generalizing at this time. @@ -419,6 +412,12 @@ private: } public: + /** + * get_extent_by_type + * + * Based on type, instantiate the correct concrete type + * and read in the extent at location offset~length. + */ template get_extent_by_type_ret get_extent_by_type( Transaction &t, ///< [in] transaction @@ -466,10 +465,6 @@ public: return ret; } - void clear_lru() { - lru.clear(); - } - void mark_delayed_extent_inline( Transaction& t, LogicalCachedExtentRef& ref) { @@ -590,13 +585,13 @@ public: // journal replay should has been finished at this point, // Cache::root should have been inserted to the dirty list assert(root->is_dirty()); - std::vector dirty; + std::vector _dirty; for (auto &e : extents) { - dirty.push_back(CachedExtentRef(&e)); + _dirty.push_back(CachedExtentRef(&e)); } return seastar::do_with( std::forward(f), - std::move(dirty), + std::move(_dirty), [this, &t](auto &f, auto &refs) mutable { return trans_intr::do_for_each( -- cgit v1.2.1