From bb58dce9e3ac2bb0fa6178bdea7957546c5efd3c Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Fri, 15 Jan 2021 11:25:12 +0800 Subject: [PATCH] crimson/onode-staged-tree: decouple NodeExtentMutable from NodeExtent Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/CMakeLists.txt | 1 - .../staged-fltree/node_delta_recorder.h | 3 +- .../staged-fltree/node_extent_accessor.h | 7 ++-- .../staged-fltree/node_extent_manager.h | 7 ++-- .../node_extent_manager/seastore.cc | 2 +- .../node_extent_manager/test_replay.h | 2 +- .../staged-fltree/node_extent_mutable.cc | 39 ------------------- .../staged-fltree/node_extent_mutable.h | 32 +++++++-------- 8 files changed, 26 insertions(+), 67 deletions(-) delete mode 100644 src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_mutable.cc diff --git a/src/crimson/os/seastore/CMakeLists.txt b/src/crimson/os/seastore/CMakeLists.txt index 77f8465cf9a6..ceffe209b4a4 100644 --- a/src/crimson/os/seastore/CMakeLists.txt +++ b/src/crimson/os/seastore/CMakeLists.txt @@ -18,7 +18,6 @@ add_library(crimson-seastore STATIC onode_manager/staged-fltree/node.cc onode_manager/staged-fltree/node_extent_manager.cc onode_manager/staged-fltree/node_extent_manager/seastore.cc - onode_manager/staged-fltree/node_extent_mutable.cc onode_manager/staged-fltree/node_impl.cc onode_manager/staged-fltree/stages/item_iterator_stage.cc onode_manager/staged-fltree/stages/key_layout.cc diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_delta_recorder.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_delta_recorder.h index d08a990151eb..4d323c05ce9a 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_delta_recorder.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_delta_recorder.h @@ -32,7 +32,8 @@ class DeltaRecorder { virtual node_type_t node_type() const = 0; virtual field_type_t field_type() const = 0; virtual void apply_delta(ceph::bufferlist::const_iterator&, - NodeExtentMutable&) = 0; + NodeExtentMutable&, + laddr_t) = 0; protected: DeltaRecorder() = default; diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h index 94782f50d4d9..6f1b95e499df 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_accessor.h @@ -99,7 +99,8 @@ class DeltaRecorderT final: public DeltaRecorder { node_type_t node_type() const override { return NODE_TYPE; } field_type_t field_type() const override { return FIELD_TYPE; } void apply_delta(ceph::bufferlist::const_iterator& delta, - NodeExtentMutable& node) override { + NodeExtentMutable& node, + laddr_t node_laddr) override { assert(is_empty()); node_stage_t stage(reinterpret_cast(node.get_read())); op_t op; @@ -169,12 +170,12 @@ class DeltaRecorderT final: public DeltaRecorder { } default: logger().error("OTree::Extent::Replay: got unknown op {} when replay {:#x}", - op, node.get_laddr()); + op, node_laddr); ceph_abort(); } } catch (buffer::error& e) { logger().error("OTree::Extent::Replay: got decode error {} when replay {:#x}", - e, node.get_laddr()); + e, node_laddr); ceph_abort(); } } diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h index 77b230e035fc..06a2f511f68f 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager.h @@ -41,7 +41,9 @@ class NodeExtent : public LogicalCachedExtent { NodeExtent(T&&... t) : LogicalCachedExtent(std::forward(t)...) {} NodeExtentMutable do_get_mutable() { - return NodeExtentMutable(*this); + assert(is_pending() || // during mutation + is_clean()); // during replay + return NodeExtentMutable(get_bptr().c_str(), get_length()); } /** @@ -51,9 +53,6 @@ class NodeExtent : public LogicalCachedExtent { * - CacheExtent::get_delta() -> ceph::bufferlist * - LogicalCachedExtent::apply_delta(const ceph::bufferlist) -> void */ - - private: - friend class NodeExtentMutable; }; using crimson::os::seastore::TransactionManager; diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.cc index 8d88485bf729..c3d9ba0c45b7 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.cc +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/seastore.cc @@ -81,7 +81,7 @@ void SeastoreNodeExtent::apply_delta(const ceph::bufferlist& bl) { auto node = do_get_mutable(); auto p = bl.cbegin(); while (p != bl.end()) { - recorder->apply_delta(p, node); + recorder->apply_delta(p, node, get_laddr()); } } diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/test_replay.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/test_replay.h index 240c8893281f..7f46ac96f015 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/test_replay.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_manager/test_replay.h @@ -30,7 +30,7 @@ class TestReplayExtent final: public NodeExtent { auto bl = recorder->get_delta(); assert(bl.length()); auto p = bl.cbegin(); - recorder->apply_delta(p, mut); + recorder->apply_delta(p, mut, 0u); assert(p == bl.end()); auto cmp = std::memcmp(get_read(), replayed_extent->get_read(), get_length()); ceph_assert(cmp == 0 && "replay mismatch!"); diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_mutable.cc b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_mutable.cc deleted file mode 100644 index 048c4000d3bb..000000000000 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_mutable.cc +++ /dev/null @@ -1,39 +0,0 @@ -// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:nil -*- -// vim: ts=8 sw=2 smarttab - -#include "node_extent_mutable.h" -#include "node_extent_manager.h" - -namespace crimson::os::seastore::onode { - -NodeExtentMutable::NodeExtentMutable(NodeExtent& extent) - : extent{extent} { - assert(extent.is_pending() || // during mutation - extent.is_clean()); // during replay -} - -const char* NodeExtentMutable::get_read() const { - assert(extent.is_pending() || // during mutation - extent.is_clean()); // during replay - return extent.get_bptr().c_str(); -} - -char* NodeExtentMutable::get_write() { - assert(extent.is_pending() || // during mutation - extent.is_clean()); // during replay - return extent.get_bptr().c_str(); -} - -extent_len_t NodeExtentMutable::get_length() const { - return extent.get_length(); -} - -laddr_t NodeExtentMutable::get_laddr() const { - return extent.get_laddr(); -} - -const char* NodeExtentMutable::buf_upper_bound() const { - return get_read() + get_length(); -} - -} diff --git a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_mutable.h b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_mutable.h index 52f10a013003..0a55020832e3 100644 --- a/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_mutable.h +++ b/src/crimson/os/seastore/onode_manager/staged-fltree/node_extent_mutable.h @@ -9,8 +9,6 @@ namespace crimson::os::seastore::onode { -class NodeExtent; - /** * NodeExtentMutable * @@ -21,8 +19,7 @@ class NodeExtent; class NodeExtentMutable { public: void copy_in_absolute(void* dst, const void* src, extent_len_t len) { - assert((char*)dst >= get_write()); - assert((char*)dst + len <= buf_upper_bound()); + assert(is_safe(dst, len)); std::memcpy(dst, src, len); } template @@ -44,11 +41,9 @@ class NodeExtentMutable { } void shift_absolute(const void* src, extent_len_t len, int offset) { - assert((const char*)src >= get_write()); - assert((const char*)src + len <= buf_upper_bound()); + assert(is_safe(src, len)); char* to = (char*)src + offset; - assert(to >= get_write()); - assert(to + len <= buf_upper_bound()); + assert(is_safe(to, len)); if (len != 0) { std::memmove(to, src, len); } @@ -59,20 +54,23 @@ class NodeExtentMutable { template void validate_inplace_update(const T& updated) { - assert((const char*)&updated >= get_write()); - assert((const char*)&updated + sizeof(T) <= buf_upper_bound()); + assert(is_safe(&updated, sizeof(T))); } - const char* get_read() const; - char* get_write(); - extent_len_t get_length() const; - laddr_t get_laddr() const; + const char* get_read() const { return p_start; } + char* get_write() { return p_start; } + extent_len_t get_length() const { return length; } private: - explicit NodeExtentMutable(NodeExtent&); - const char* buf_upper_bound() const; + NodeExtentMutable(char* p_start, extent_len_t length) + : p_start{p_start}, length{length} {} + bool is_safe(const void* src, extent_len_t len) const { + return ((const char*)src >= p_start) && + ((const char*)src + len <= p_start + length); + } - NodeExtent& extent; + char* p_start; + extent_len_t length; friend class NodeExtent; }; -- 2.47.3