From: Samuel Just Date: Tue, 12 May 2020 04:02:07 +0000 (-0700) Subject: crimson: distinguish record and block relative paddrs X-Git-Tag: v16.1.0~2167^2~13 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=1ac405bceccbec7d548ecff7b18ab37148894899;p=ceph.git crimson: distinguish record and block relative paddrs Blocks get read independently of the surrounding record, so paddr's embedded directly in a block need to refer to other blocks within the same record by a block_relative addr relative to the block's own offset. By contrast, deltas to existing blocks need to use record_relative addrs relative to the first block of the record. This patch distinguishes the two kinds of relative paddr (mainly for debugging purposes) and adapts cache, journal, etc to use the appropriate types. Signed-off-by: Samuel Just --- diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index cfb820490427..45f2ebf69337 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -182,7 +182,7 @@ Cache::replay_delta(paddr_t record_base, const delta_info_t &delta) { if (delta.type == extent_types_t::ROOT_LOCATION) { auto root_location = delta.paddr.is_relative() - ? record_base.add_relative(delta.paddr) + ? record_base.add_record_relative(delta.paddr) : delta.paddr; logger().debug("replay_delta: found root addr {}", root_location); return get_extent( diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index ed86526f26a5..f0b5faf96aee 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -61,7 +61,7 @@ class Transaction { void add_fresh_extent(CachedExtentRef ref) { fresh_block_list.push_back(ref); - ref->set_paddr(make_relative_paddr(offset)); + ref->set_paddr(make_record_relative_paddr(offset)); offset += ref->get_length(); write_set.insert(*ref); } diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index 0e7bfa4bdd4c..a2038ca691b7 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -309,7 +309,8 @@ protected: } else if (is_mutation_pending()) { return addr; } else { - ceph_assert(get_paddr().is_relative()); + ceph_assert(is_initial_pending()); + ceph_assert(get_paddr().is_record_relative()); return addr - get_paddr(); } } diff --git a/src/crimson/os/seastore/journal.cc b/src/crimson/os/seastore/journal.cc index 2437fa748fb9..88d3e238a877 100644 --- a/src/crimson/os/seastore/journal.cc +++ b/src/crimson/os/seastore/journal.cc @@ -336,8 +336,7 @@ Journal::replay_segment( deltas, [this, &delta_handler, record_start](auto &info) { return delta_handler( - record_start.add_relative( - make_relative_paddr(block_size)), + record_start.add_offset(block_size), info); }); }).safe_then([] { diff --git a/src/crimson/os/seastore/journal.h b/src/crimson/os/seastore/journal.h index aed11b54d1de..8f9f92be465d 100644 --- a/src/crimson/os/seastore/journal.h +++ b/src/crimson/os/seastore/journal.h @@ -148,7 +148,7 @@ public: auto ret = next_record_addr(); return write_record(rsize, std::move(record) ).safe_then([this, ret] { - return ret.add_relative(make_relative_paddr(block_size)); + return ret.add_offset(block_size); }); }); } diff --git a/src/crimson/os/seastore/seastore_types.cc b/src/crimson/os/seastore/seastore_types.cc index af8516e2eacc..d68f4b06b4c2 100644 --- a/src/crimson/os/seastore/seastore_types.cc +++ b/src/crimson/os/seastore/seastore_types.cc @@ -9,8 +9,10 @@ std::ostream &segment_to_stream(std::ostream &out, const segment_id_t &t) { if (t == NULL_SEG_ID) return out << "NULL_SEG"; - else if (t == REL_SEG_ID) - return out << "REL_SEG"; + else if (t == BLOCK_REL_SEG_ID) + return out << "BLOCK_REL_SEG"; + else if (t == RECORD_REL_SEG_ID) + return out << "RECORD_REL_SEG"; else return out << t; } diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h index 8cd4c4a631e2..b21820e36bfd 100644 --- a/src/crimson/os/seastore/seastore_types.h +++ b/src/crimson/os/seastore/seastore_types.h @@ -19,8 +19,10 @@ using segment_id_t = uint32_t; constexpr segment_id_t NULL_SEG_ID = std::numeric_limits::max() - 1; /* Used to denote relative paddr_t */ -constexpr segment_id_t REL_SEG_ID = +constexpr segment_id_t RECORD_REL_SEG_ID = std::numeric_limits::max() - 2; +constexpr segment_id_t BLOCK_REL_SEG_ID = + std::numeric_limits::max() - 3; std::ostream &segment_to_stream(std::ostream &, const segment_id_t &t); @@ -43,13 +45,38 @@ using record_delta_idx_t = uint32_t; constexpr record_delta_idx_t NULL_DELTA_IDX = std::numeric_limits::max(); -// offset on disk, see SegmentManager +/** + * paddr_t + * + * offset on disk, see SegmentManager + * + * May be absolute, record_relative, or block_relative. + * + * Blocks get read independently of the surrounding record, + * so paddrs embedded directly within a block need to refer + * to other blocks within the same record by a block_relative + * addr relative to the block's own offset. By contrast, + * deltas to existing blocks need to use record_relative + * addrs relative to the first block of the record. + * + * Fresh extents during a transaction are refered to by + * record_relative paddrs. + */ struct paddr_t { segment_id_t segment = NULL_SEG_ID; segment_off_t offset = NULL_SEG_OFF; bool is_relative() const { - return segment == REL_SEG_ID; + return segment == RECORD_REL_SEG_ID || + segment == BLOCK_REL_SEG_ID; + } + + bool is_record_relative() const { + return segment == RECORD_REL_SEG_ID; + } + + bool is_block_relative() const { + return segment == BLOCK_REL_SEG_ID; } paddr_t add_offset(segment_off_t o) const { @@ -58,21 +85,47 @@ struct paddr_t { paddr_t add_relative(paddr_t o) const { assert(o.is_relative()); - assert(!is_relative()); return paddr_t{segment, offset + o.offset}; } + paddr_t add_block_relative(paddr_t o) const { + // special version mainly for documentation purposes + assert(o.is_block_relative()); + return add_relative(o); + } + + paddr_t add_record_relative(paddr_t o) const { + // special version mainly for documentation purposes + assert(o.is_record_relative()); + return add_relative(o); + } + + /** + * paddr_t::operator- + * + * Only defined for record_relative paddr_ts. Yields a + * block_relative address. + */ paddr_t operator-(paddr_t rhs) const { - assert(rhs.is_relative() && is_relative()); + assert(rhs.is_record_relative() && is_record_relative()); return paddr_t{ - REL_SEG_ID, + BLOCK_REL_SEG_ID, offset - rhs.offset }; } + /** + * maybe_relative_to + * + * Helper for the case where an in-memory paddr_t may be + * either block_relative or absolute (not record_relative). + * + * base must be either absolute or record_relative. + */ paddr_t maybe_relative_to(paddr_t base) const { - if (is_relative()) - return base.add_relative(*this); + assert(!base.is_block_relative()); + if (is_block_relative()) + return base.add_block_relative(*this); else return *this; } @@ -87,8 +140,12 @@ struct paddr_t { WRITE_CMP_OPERATORS_2(paddr_t, segment, offset) WRITE_EQ_OPERATORS_2(paddr_t, segment, offset) constexpr paddr_t P_ADDR_NULL = paddr_t{}; -constexpr paddr_t make_relative_paddr(segment_off_t off) { - return paddr_t{REL_SEG_ID, off}; +constexpr paddr_t P_ADDR_MIN = paddr_t{0, 0}; +constexpr paddr_t make_record_relative_paddr(segment_off_t off) { + return paddr_t{RECORD_REL_SEG_ID, off}; +} +constexpr paddr_t make_block_relative_paddr(segment_off_t off) { + return paddr_t{BLOCK_REL_SEG_ID, off}; } std::ostream &operator<<(std::ostream &out, const paddr_t &rhs); diff --git a/src/test/crimson/seastore/test_seastore_journal.cc b/src/test/crimson/seastore/test_seastore_journal.cc index 02c22e84c727..4a901d86080d 100644 --- a/src/test/crimson/seastore/test_seastore_journal.cc +++ b/src/test/crimson/seastore/test_seastore_journal.cc @@ -27,7 +27,7 @@ struct record_validator_t { record_validator_t(T&&... record) : record(std::forward(record)...) {} void validate(SegmentManager &manager) { - paddr_t addr = make_relative_paddr(0); + paddr_t addr = make_record_relative_paddr(0); for (auto &&block : record.extents) { auto test = manager.read( record_final_offset.add_relative(addr),