From: Xuehan Xu Date: Fri, 13 May 2022 08:29:23 +0000 (+0800) Subject: crimson/os/seastore/cache: don't index already removed backref entries in Cache:... X-Git-Tag: v18.0.0~763^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=beecaddd6c1c8f238c969c0500d46f7b54322a33;p=ceph.git crimson/os/seastore/cache: don't index already removed backref entries in Cache::backref_buffer This is needed by extent splitting, and can avoid inserting/removing unnecessary backrefs Signed-off-by: Xuehan Xu --- diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.cc b/src/crimson/os/seastore/backref/btree_backref_manager.cc index 235cbc32ac709..9c0f2ba524125 100644 --- a/src/crimson/os/seastore/backref/btree_backref_manager.cc +++ b/src/crimson/os/seastore/backref/btree_backref_manager.cc @@ -218,17 +218,17 @@ BtreeBackrefManager::merge_cached_backrefs( auto &backref_buffer = cache.get_backref_buffer(); if (backref_buffer) { return seastar::do_with( - backref_buffer->backrefs.begin(), + backref_buffer->backrefs_by_seq.begin(), JOURNAL_SEQ_NULL, [this, &t, &limit, &backref_buffer, max](auto &iter, auto &inserted_to) { return trans_intr::repeat( [&iter, this, &t, &limit, &backref_buffer, max, &inserted_to]() -> merge_cached_backrefs_iertr::future { - if (iter == backref_buffer->backrefs.end()) + if (iter == backref_buffer->backrefs_by_seq.end()) return seastar::make_ready_future( seastar::stop_iteration::yes); auto &seq = iter->first; - auto &backref_list = iter->second; + auto &backref_list = iter->second.br_list; LOG_PREFIX(BtreeBackrefManager::merge_cached_backrefs); DEBUGT("seq {}, limit {}, num_fresh_backref {}" , t, seq, limit, t.get_num_fresh_backref()); @@ -238,22 +238,22 @@ BtreeBackrefManager::merge_cached_backrefs( backref_list, [this, &t](auto &backref) { LOG_PREFIX(BtreeBackrefManager::merge_cached_backrefs); - if (backref->laddr != L_ADDR_NULL) { + if (backref.laddr != L_ADDR_NULL) { DEBUGT("new mapping: {}~{} -> {}", - t, backref->paddr, backref->len, backref->laddr); + t, backref.paddr, backref.len, backref.laddr); return new_mapping( t, - backref->paddr, - backref->len, - backref->laddr, - backref->type).si_then([](auto &&pin) { + backref.paddr, + backref.len, + backref.laddr, + backref.type).si_then([](auto &&pin) { return seastar::now(); }); } else { - DEBUGT("remove mapping: {}", t, backref->paddr); + DEBUGT("remove mapping: {}", t, backref.paddr); return remove_mapping( t, - backref->paddr).si_then([](auto&&) { + backref.paddr).si_then([](auto&&) { return seastar::now(); }).handle_error_interruptible( crimson::ct_error::input_output_error::pass_further(), @@ -502,4 +502,8 @@ BtreeBackrefManager::retrieve_backref_extents( }); } +bool BtreeBackrefManager::backref_should_be_removed(paddr_t paddr) { + return cache.backref_should_be_removed(paddr); +} + } // namespace crimson::os::seastore::backref diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.h b/src/crimson/os/seastore/backref/btree_backref_manager.h index 0a3395270378d..ddfae7ec48c53 100644 --- a/src/crimson/os/seastore/backref/btree_backref_manager.h +++ b/src/crimson/os/seastore/backref/btree_backref_manager.h @@ -126,6 +126,8 @@ public: void cache_new_backref_extent(paddr_t paddr, extent_types_t type) final; + bool backref_should_be_removed(paddr_t paddr) final; + private: SegmentManagerGroup &sm_group; Cache &cache; diff --git a/src/crimson/os/seastore/backref_manager.h b/src/crimson/os/seastore/backref_manager.h index ab9d5108b89bd..f3f661086d59c 100644 --- a/src/crimson/os/seastore/backref_manager.h +++ b/src/crimson/os/seastore/backref_manager.h @@ -102,6 +102,8 @@ public: paddr_t start, paddr_t end) = 0; + virtual bool backref_should_be_removed(paddr_t paddr) = 0; + using retrieve_backref_extents_iertr = trans_iertr< crimson::errorator< crimson::ct_error::input_output_error> diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 9183e5cd0bb8b..7c6872e65093e 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -1297,40 +1297,53 @@ void Cache::backref_batch_update( LOG_PREFIX(Cache::backref_batch_update); DEBUG("inserting {} entries", list.size()); if (!backref_buffer) { - backref_buffer = std::make_unique(); + backref_buffer = std::make_unique(); } // backref_buf_entry_t::laddr == L_ADDR_NULL means erase for (auto &ent : list) { if (ent->laddr == L_ADDR_NULL) { - auto [it, insert] = backref_remove_set.insert(*ent); - boost::ignore_unused(insert); + auto insert_set_iter = backref_inserted_set.find( + ent->paddr, backref_buf_entry_t::cmp_t()); + if (insert_set_iter == backref_inserted_set.end()) { + // backref to be removed isn't in the backref buffer, + // it must be in the backref tree. + auto [it, insert] = backref_remove_set.insert(*ent); + boost::ignore_unused(insert); #ifndef NDEBUG - if (!insert) { - ERROR("backref_remove_set already contains {}", ent->paddr); - } + if (!insert) { + ERROR("backref_remove_set already contains {}", ent->paddr); + } #endif - assert(insert); - } else { -#ifndef NDEBUG - auto r = backref_remove_set.erase(ent->paddr, backref_buf_entry_t::cmp_t()); - if (r) { - ERROR("backref_remove_set contains: {}", ent->paddr); + assert(insert); + } else { + // the backref insertion hasn't been applied to the + // backref tree + auto seq = insert_set_iter->seq; + auto it = backref_buffer->backrefs_by_seq.find(seq); + ceph_assert(it != backref_buffer->backrefs_by_seq.end()); + auto &backref_buf = it->second; + assert(insert_set_iter->backref_buf_hook.is_linked()); + backref_buf.br_list.erase( + backref_buf_entry_t::list_t::s_iterator_to(*insert_set_iter)); + backref_inserted_set.erase(insert_set_iter); } - assert(!r); -#endif + } else { auto [it, insert] = backref_inserted_set.insert(*ent); boost::ignore_unused(insert); assert(insert); } } - auto iter = backref_buffer->backrefs.find(seq); - if (iter == backref_buffer->backrefs.end()) { - backref_buffer->backrefs.emplace( + auto iter = backref_buffer->backrefs_by_seq.find(seq); + if (iter == backref_buffer->backrefs_by_seq.end()) { + backref_buffer->backrefs_by_seq.emplace( seq, std::move(list)); } else { - iter->second.insert( - iter->second.end(), + for (auto &ref : list) { + iter->second.br_list.push_back(*ref); + } + iter->second.backrefs.insert( + iter->second.backrefs.end(), std::make_move_iterator(list.begin()), std::make_move_iterator(list.end())); } diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index f78934defa3c8..316953ed43dc9 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -47,12 +47,12 @@ struct backref_buf_entry_t { len(alloc_blk.len), type(alloc_blk.type) {} - const paddr_t paddr = P_ADDR_NULL; - const laddr_t laddr = L_ADDR_NULL; - const extent_len_t len = 0; - const extent_types_t type = + paddr_t paddr = P_ADDR_NULL; + laddr_t laddr = L_ADDR_NULL; + extent_len_t len = 0; + extent_types_t type = extent_types_t::ROOT; - const journal_seq_t seq; + journal_seq_t seq; friend bool operator< ( const backref_buf_entry_t &l, const backref_buf_entry_t &r) { @@ -68,19 +68,35 @@ struct backref_buf_entry_t { const backref_buf_entry_t &r) { return l.paddr == r.paddr; } - using hook_t = + using set_hook_t = boost::intrusive::set_member_hook< boost::intrusive::link_mode< boost::intrusive::auto_unlink>>; - hook_t backref_set_hook; + set_hook_t backref_set_hook; + + using list_hook_t = + boost::intrusive::list_member_hook< + boost::intrusive::link_mode< + boost::intrusive::auto_unlink>>; + list_hook_t backref_buf_hook; + using backref_set_member_options = boost::intrusive::member_hook< backref_buf_entry_t, - hook_t, + set_hook_t, &backref_buf_entry_t::backref_set_hook>; using set_t = boost::intrusive::set< backref_buf_entry_t, backref_set_member_options, boost::intrusive::constant_time_size>; + + using backref_list_member_options = boost::intrusive::member_hook< + backref_buf_entry_t, + list_hook_t, + &backref_buf_entry_t::backref_buf_hook>; + using list_t = boost::intrusive::list< + backref_buf_entry_t, + backref_list_member_options, + boost::intrusive::constant_time_size>; struct cmp_t { using is_transparent = paddr_t; bool operator()( @@ -100,10 +116,20 @@ struct backref_buf_entry_t { using backref_buf_entry_ref = std::unique_ptr; -struct backref_buffer_t { - std::map> backrefs; +struct backref_buf_t { + backref_buf_t(std::vector &&refs) : backrefs(std::move(refs)) { + for (auto &ref : backrefs) { + br_list.push_back(*ref); + } + } + std::vector backrefs; + backref_buf_entry_t::list_t br_list; }; -using backref_buffer_ref = std::unique_ptr; + +struct backref_cache_t { + std::map backrefs_by_seq; +}; +using backref_cache_ref = std::unique_ptr; /** * Cache @@ -529,7 +555,7 @@ private: } } - backref_buffer_ref backref_buffer; + backref_cache_ref backref_buffer; // backrefs that needs to be inserted into the backref tree backref_buf_entry_t::set_t backref_inserted_set; backref_buf_entry_t::set_t backref_remove_set; // backrefs needs to be removed @@ -589,6 +615,11 @@ private: return *it; } + bool backref_should_be_removed(paddr_t addr) { + return backref_remove_set.find( + addr, backref_buf_entry_t::cmp_t()) != backref_remove_set.end(); + } + const backref_buf_entry_t::set_t& get_backrefs() { return backref_inserted_set; } @@ -597,7 +628,7 @@ private: return backref_remove_set; } - backref_buffer_ref& get_backref_buffer() { + backref_cache_ref& get_backref_buffer() { return backref_buffer; } @@ -639,11 +670,11 @@ public: void trim_backref_bufs(const journal_seq_t &trim_to) { LOG_PREFIX(Cache::trim_backref_bufs); SUBDEBUG(seastore_cache, "trimming to {}", trim_to); - if (backref_buffer && !backref_buffer->backrefs.empty()) { - assert(backref_buffer->backrefs.rbegin()->first >= trim_to); - auto iter = backref_buffer->backrefs.upper_bound(trim_to); - backref_buffer->backrefs.erase( - backref_buffer->backrefs.begin(), iter); + if (backref_buffer && !backref_buffer->backrefs_by_seq.empty()) { + assert(backref_buffer->backrefs_by_seq.rbegin()->first >= trim_to); + auto iter = backref_buffer->backrefs_by_seq.upper_bound(trim_to); + backref_buffer->backrefs_by_seq.erase( + backref_buffer->backrefs_by_seq.begin(), iter); } } @@ -889,8 +920,8 @@ public: std::optional get_oldest_backref_dirty_from() const { LOG_PREFIX(Cache::get_oldest_backref_dirty_from); journal_seq_t backref_oldest = JOURNAL_SEQ_NULL; - if (backref_buffer && !backref_buffer->backrefs.empty()) { - backref_oldest = backref_buffer->backrefs.begin()->first; + if (backref_buffer && !backref_buffer->backrefs_by_seq.empty()) { + backref_oldest = backref_buffer->backrefs_by_seq.begin()->first; } if (backref_oldest == JOURNAL_SEQ_NULL) { SUBDEBUG(seastore_cache, "backref_oldest: null"); diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc index ef582006e7a1a..c553f58d298af 100644 --- a/src/crimson/os/seastore/segment_cleaner.cc +++ b/src/crimson/os/seastore/segment_cleaner.cc @@ -1271,11 +1271,10 @@ void SegmentCleaner::mark_space_used( void SegmentCleaner::mark_space_free( paddr_t addr, - extent_len_t len, - bool force) + extent_len_t len) { LOG_PREFIX(SegmentCleaner::mark_space_free); - if (!init_complete && !force) { + if (!init_complete) { return; } if (addr.get_addr_type() != addr_types_t::SEGMENT) { diff --git a/src/crimson/os/seastore/segment_cleaner.h b/src/crimson/os/seastore/segment_cleaner.h index 9467bc7506094..c28e7b3686eaf 100644 --- a/src/crimson/os/seastore/segment_cleaner.h +++ b/src/crimson/os/seastore/segment_cleaner.h @@ -796,8 +796,7 @@ public: void mark_space_free( paddr_t addr, - extent_len_t len, - bool force = false); + extent_len_t len); SpaceTrackerIRef get_empty_space_tracker() const { return space_tracker->make_empty(); diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 10185dbb34a40..269e1a074149f 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -134,7 +134,8 @@ TransactionManager::mount_ertr::future<> TransactionManager::mount() t, addr, len); - if (addr.is_real()) { + if (addr.is_real() && + !backref_manager->backref_should_be_removed(addr)) { segment_cleaner->mark_space_used( addr, len , @@ -163,14 +164,6 @@ TransactionManager::mount_ertr::future<> TransactionManager::mount() seastar::lowres_system_clock::time_point(), true); } - auto &del_backrefs = backref_manager->get_cached_backref_removals(); - DEBUG("marking {} backrefs free", del_backrefs.size()); - for (auto &del_backref : del_backrefs) { - segment_cleaner->mark_space_free( - del_backref.paddr, - del_backref.len, - true); - } return seastar::now(); }); }); diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc index f8ef2b399c4eb..d650fff95ddd9 100644 --- a/src/test/crimson/seastore/test_transaction_manager.cc +++ b/src/test/crimson/seastore/test_transaction_manager.cc @@ -402,8 +402,9 @@ struct transaction_manager_test_t : [this, &tracker](auto &t) { return backref_manager->scan_mapped_space( t, - [&tracker](auto offset, auto len, depth_t) { - if (offset.get_addr_type() == addr_types_t::SEGMENT) { + [&tracker, this](auto offset, auto len, depth_t) { + if (offset.get_addr_type() == addr_types_t::SEGMENT && + !backref_manager->backref_should_be_removed(offset)) { logger().debug("check_usage: tracker alloc {}~{}", offset, len); tracker->allocate( @@ -423,17 +424,6 @@ struct transaction_manager_test_t : backref.len); } } - auto &del_backrefs = backref_manager->get_cached_backref_removals(); - for (auto &del_backref : del_backrefs) { - if (del_backref.paddr.get_addr_type() == addr_types_t::SEGMENT) { - logger().debug("check_usage: by backref, tracker release {}~{}", - del_backref.paddr, del_backref.len); - tracker->release( - del_backref.paddr.as_seg_paddr().get_segment_id(), - del_backref.paddr.as_seg_paddr().get_segment_off(), - del_backref.len); - } - } return seastar::now(); }); }).unsafe_get0();