From: Samuel Just Date: Wed, 12 May 2021 09:04:16 +0000 (+0000) Subject: crimson/os/seastore: invalidate transaction referencing invalid extents X-Git-Tag: v17.1.0~1567^2~8 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=5cdcaf5d2adaed65af46d98966db24343e0c4b9a;p=ceph.git crimson/os/seastore: invalidate transaction referencing invalid extents Modify read_set to retain a reverse mapping from extents back to transactions and use it to update Transaction::conflicted upon invalidation. Signed-off-by: Samuel Just --- diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 2dc005628150..19a874017c23 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -152,7 +152,14 @@ void Cache::replace_extent(CachedExtentRef next, CachedExtentRef prev) add_to_dirty(next); } - prev->state = CachedExtent::extent_state_t::INVALID; + invalidate(*prev); +} + +void Cache::invalidate(CachedExtent &extent) { + for (auto &&i: extent.transactions) { + i.t->conflicted = true; + } + extent.state = CachedExtent::extent_state_t::INVALID; } CachedExtentRef Cache::alloc_new_extent_by_type( @@ -223,14 +230,12 @@ std::optional Cache::try_construct_record(Transaction &t) LOG_PREFIX(Cache::try_construct_record); DEBUGT("enter", t); - // First, validate read set + // Should be valid due to interruptible future for (auto &i: t.read_set) { - if (i->state == CachedExtent::extent_state_t::INVALID) { - return std::nullopt; - } + assert(i.ref->is_valid()); } - DEBUGT("read_set validated", t); + t.read_set.clear(); record_t record; diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 2400f875c190..15fc98b729ad 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -587,6 +587,9 @@ private: /// Replace prev with next void replace_extent(CachedExtentRef next, CachedExtentRef prev); + /// Invalidate extent and mark affected transactions + void invalidate(CachedExtent &extent); + Transaction::get_extent_ret query_cache_for_extent( paddr_t offset, CachedExtentRef *out) { diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index a14e835fb6f1..c32d01ade073 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -17,6 +17,7 @@ namespace crimson::os::seastore { +class Transaction; class CachedExtent; using CachedExtentRef = boost::intrusive_ptr; @@ -38,6 +39,40 @@ namespace onode { class DummyNodeExtent; class TestReplayExtent; } + +template +class read_set_item_t { + boost::intrusive::list_member_hook<> list_hook; + using list_hook_options = boost::intrusive::member_hook< + read_set_item_t, + boost::intrusive::list_member_hook<>, + &read_set_item_t::list_hook>; + +public: + struct cmp_t { + using is_transparent = paddr_t; + bool operator()(const read_set_item_t &lhs, const read_set_item_t &rhs) const; + bool operator()(const paddr_t &lhs, const read_set_item_t &rhs) const; + bool operator()(const read_set_item_t &lhs, const paddr_t &rhs) const; + }; + + using list = boost::intrusive::list< + read_set_item_t, + list_hook_options>; + + T *t = nullptr; + CachedExtentRef ref; + + read_set_item_t(T *t, CachedExtentRef ref); + read_set_item_t(const read_set_item_t &) = delete; + read_set_item_t(read_set_item_t &&) = default; + ~read_set_item_t(); +}; +template +struct read_set_t : public std::set< + read_set_item_t, + typename read_set_item_t::cmp_t> {}; + class ExtentIndex; class CachedExtent : public boost::intrusive_ref_counter< CachedExtent, boost::thread_unsafe_counter> { @@ -286,6 +321,9 @@ public: virtual ~CachedExtent(); private: + template + friend class read_set_item_t; + friend struct paddr_cmp; friend struct ref_paddr_cmp; friend class ExtentIndex; @@ -351,6 +389,8 @@ private: } } + read_set_item_t::list transactions; + protected: CachedExtent(CachedExtent &&other) = delete; CachedExtent(ceph::bufferptr &&ptr) : ptr(std::move(ptr)) {} @@ -775,6 +815,35 @@ struct ref_laddr_cmp { } }; +template +read_set_item_t::read_set_item_t(T *t, CachedExtentRef ref) + : t(t), ref(ref) +{ + ref->transactions.push_back(*this); +} + +template +read_set_item_t::~read_set_item_t() +{ + ref->transactions.erase(ref->transactions.s_iterator_to(*this)); +} + +template +inline bool read_set_item_t::cmp_t::operator()( + const read_set_item_t &lhs, const read_set_item_t &rhs) const { + return lhs.ref->poffset < rhs.ref->poffset; +} +template +inline bool read_set_item_t::cmp_t::operator()( + const paddr_t &lhs, const read_set_item_t &rhs) const { + return lhs < rhs.ref->poffset; +} +template +inline bool read_set_item_t::cmp_t::operator()( + const read_set_item_t &lhs, const paddr_t &rhs) const { + return lhs.ref->poffset < rhs; +} + using lextent_set_t = addr_extent_set_base_t< laddr_t, LogicalCachedExtentRef, diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h index 7a46e5c29937..a8a7998988d7 100644 --- a/src/crimson/os/seastore/transaction.h +++ b/src/crimson/os/seastore/transaction.h @@ -16,6 +16,7 @@ namespace crimson::os::seastore { struct retired_extent_gate_t; class SeaStore; +class Transaction; /** * Transaction @@ -44,7 +45,7 @@ public: auto iter = read_set.find(addr); iter != read_set.end()) { if (out) - *out = CachedExtentRef(*iter); + *out = iter->ref; return get_extent_ret::PRESENT; } else { return get_extent_ret::ABSENT; @@ -73,8 +74,8 @@ public: void add_to_read_set(CachedExtentRef ref) { if (is_weak()) return; - ceph_assert(read_set.count(ref) == 0); - read_set.insert(ref); + auto [iter, inserted] = read_set.emplace(this, ref); + ceph_assert(inserted); } void add_fresh_extent(CachedExtentRef ref) { @@ -130,8 +131,8 @@ private: segment_off_t offset = 0; ///< relative offset of next block - pextent_set_t read_set; ///< set of extents read by paddr - ExtentIndex write_set; ///< set of extents written by paddr + read_set_t read_set; ///< set of extents read by paddr + ExtentIndex write_set; ///< set of extents written by paddr std::list fresh_block_list; ///< list of fresh blocks std::list mutated_block_list; ///< list of mutated blocks @@ -147,6 +148,8 @@ private: retired_extent_gate_t::token_t retired_gate_token; + bool conflicted = false; + public: Transaction( OrderingHandle &&handle,