From: Jason Dillaman Date: Fri, 25 Sep 2020 13:16:48 +0000 (-0400) Subject: librbd: pass write op object extents to copyup state machine X-Git-Tag: v16.1.0~833^2~9 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=09a90d9a40534bbc5b6b95a68144526548648263;p=ceph.git librbd: pass write op object extents to copyup state machine It will use these extents in a later commit to potentially reduce the parent image data that is processed for copyup portion of the write request. Signed-off-by: Jason Dillaman --- diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc index 64dd594526812..3f864322bc230 100644 --- a/src/librbd/io/CopyupRequest.cc +++ b/src/librbd/io/CopyupRequest.cc @@ -130,7 +130,8 @@ CopyupRequest::~CopyupRequest() { } template -void CopyupRequest::append_request(AbstractObjectWriteRequest *req) { +void CopyupRequest::append_request(AbstractObjectWriteRequest *req, + const Extents& object_extents) { std::lock_guard locker{m_lock}; auto cct = m_image_ctx->cct; @@ -138,6 +139,12 @@ void CopyupRequest::append_request(AbstractObjectWriteRequest *req) { << "append=" << m_append_request_permitted << dendl; if (m_append_request_permitted) { m_pending_requests.push_back(req); + + for (auto [offset, length] : object_extents) { + if (length > 0) { + m_write_object_extents.union_insert(offset, length); + } + } } else { m_restart_requests.push_back(req); } diff --git a/src/librbd/io/CopyupRequest.h b/src/librbd/io/CopyupRequest.h index 51368cc583797..7875a6dc9dc0e 100644 --- a/src/librbd/io/CopyupRequest.h +++ b/src/librbd/io/CopyupRequest.h @@ -6,6 +6,7 @@ #include "include/int_types.h" #include "include/buffer.h" +#include "include/interval_set.h" #include "common/ceph_mutex.h" #include "common/zipkin_trace.h" #include "librbd/io/AsyncOperation.h" @@ -39,7 +40,8 @@ public: const ZTracer::Trace &parent_trace); ~CopyupRequest(); - void append_request(AbstractObjectWriteRequest *req); + void append_request(AbstractObjectWriteRequest *req, + const Extents& object_extents); void send(); @@ -104,6 +106,8 @@ private: WriteRequests m_restart_requests; bool m_append_request_permitted = true; + interval_set m_write_object_extents; + void read_from_parent(); void handle_read_from_parent(int r); diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc index 1e7280d3a03eb..27223221faeab 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -564,13 +564,13 @@ void AbstractObjectWriteRequest::copyup() { this->m_parent_extents.clear(); // make sure to wait on this CopyupRequest - new_req->append_request(this); + new_req->append_request(this, std::move(get_copyup_overwrite_extents())); image_ctx->copyup_list[this->m_object_no] = new_req; image_ctx->copyup_list_lock.unlock(); new_req->send(); } else { - it->second->append_request(this); + it->second->append_request(this, std::move(get_copyup_overwrite_extents())); image_ctx->copyup_list_lock.unlock(); } } diff --git a/src/librbd/io/ObjectRequest.h b/src/librbd/io/ObjectRequest.h index 711b440bd3207..93ad1b9bf8863 100644 --- a/src/librbd/io/ObjectRequest.h +++ b/src/librbd/io/ObjectRequest.h @@ -199,6 +199,10 @@ protected: return r; } + virtual Extents get_copyup_overwrite_extents() const { + return {{m_object_off, m_object_len}}; + } + private: /** * @verbatim @@ -304,7 +308,6 @@ public: } else { m_discard_action = DISCARD_ACTION_TRUNCATE; } - this->m_object_len = 0; } else { m_discard_action = DISCARD_ACTION_REMOVE; } @@ -426,6 +429,10 @@ protected: int filter_write_result(int r) const override; + Extents get_copyup_overwrite_extents() const override { + return {}; + } + private: ceph::bufferlist m_cmp_bl; ceph::bufferlist m_write_bl; diff --git a/src/test/librbd/io/test_mock_CopyupRequest.cc b/src/test/librbd/io/test_mock_CopyupRequest.cc index 458010f89db73..93ee1c61c15f0 100644 --- a/src/test/librbd/io/test_mock_CopyupRequest.cc +++ b/src/test/librbd/io/test_mock_CopyupRequest.cc @@ -389,7 +389,7 @@ TEST_F(TestMockIoCopyupRequest, Standard) { auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request); + req->append_request(&mock_write_request, {}); req->send(); ASSERT_EQ(0, mock_write_request.ctx.wait()); @@ -447,7 +447,7 @@ TEST_F(TestMockIoCopyupRequest, StandardWithSnaps) { auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request); + req->append_request(&mock_write_request, {}); req->send(); ASSERT_EQ(0, mock_write_request.ctx.wait()); @@ -580,7 +580,7 @@ TEST_F(TestMockIoCopyupRequest, DeepCopy) { auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request); + req->append_request(&mock_write_request, {}); req->send(); ASSERT_EQ(0, mock_write_request.ctx.wait()); @@ -685,7 +685,7 @@ TEST_F(TestMockIoCopyupRequest, DeepCopyWithPostSnaps) { auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request); + req->append_request(&mock_write_request, {}); req->send(); ASSERT_EQ(0, mock_write_request.ctx.wait()); @@ -757,7 +757,7 @@ TEST_F(TestMockIoCopyupRequest, DeepCopyWithPreAndPostSnaps) { auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request); + req->append_request(&mock_write_request, {}); req->send(); ASSERT_EQ(0, mock_write_request.ctx.wait()); @@ -798,7 +798,7 @@ TEST_F(TestMockIoCopyupRequest, ZeroedCopyup) { auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request); + req->append_request(&mock_write_request, {}); req->send(); ASSERT_EQ(0, mock_write_request.ctx.wait()); @@ -873,7 +873,7 @@ TEST_F(TestMockIoCopyupRequest, NoOpCopyup) { auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request); + req->append_request(&mock_write_request, {}); req->send(); ASSERT_EQ(0, mock_write_request.ctx.wait()); @@ -922,12 +922,12 @@ TEST_F(TestMockIoCopyupRequest, RestartWrite) { mock_image_ctx.rados_api, *mock_image_ctx.get_data_io_context()); EXPECT_CALL(mock_io_ctx, write(ictx->get_object_name(0), _, 0, 0, _)) .WillOnce(WithoutArgs(Invoke([req, &mock_write_request2]() { - req->append_request(&mock_write_request2); + req->append_request(&mock_write_request2, {}); return 0; }))); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request1); + req->append_request(&mock_write_request1, {}); req->send(); ASSERT_EQ(0, mock_write_request1.ctx.wait()); @@ -964,7 +964,7 @@ TEST_F(TestMockIoCopyupRequest, ReadFromParentError) { auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request); + req->append_request(&mock_write_request, {}); req->send(); ASSERT_EQ(-EPERM, mock_write_request.ctx.wait()); @@ -1001,7 +1001,7 @@ TEST_F(TestMockIoCopyupRequest, DeepCopyError) { auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request); + req->append_request(&mock_write_request, {}); req->send(); ASSERT_EQ(-EPERM, mock_write_request.ctx.wait()); @@ -1042,7 +1042,7 @@ TEST_F(TestMockIoCopyupRequest, UpdateObjectMapError) { auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request); + req->append_request(&mock_write_request, {}); req->send(); ASSERT_EQ(-EINVAL, mock_write_request.ctx.wait()); @@ -1096,7 +1096,7 @@ TEST_F(TestMockIoCopyupRequest, CopyupError) { auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request); + req->append_request(&mock_write_request, {}); req->send(); ASSERT_EQ(-EPERM, mock_write_request.ctx.wait()); @@ -1142,7 +1142,7 @@ TEST_F(TestMockIoCopyupRequest, SparseCopyupNotSupported) { auto req = new MockCopyupRequest(&mock_image_ctx, 0, {{0, 4096}}, {}); mock_image_ctx.copyup_list[0] = req; - req->append_request(&mock_write_request); + req->append_request(&mock_write_request, {}); req->send(); ASSERT_EQ(0, mock_write_request.ctx.wait()); diff --git a/src/test/librbd/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc index dad8005731a7d..c19d3a62dc86b 100644 --- a/src/test/librbd/io/test_mock_ObjectRequest.cc +++ b/src/test/librbd/io/test_mock_ObjectRequest.cc @@ -37,7 +37,8 @@ namespace io { template <> struct CopyupRequest { MOCK_METHOD0(send, void()); - MOCK_METHOD1(append_request, void(AbstractObjectWriteRequest*)); + MOCK_METHOD2(append_request, void(AbstractObjectWriteRequest*, + const Extents&)); }; template <> @@ -244,10 +245,11 @@ struct TestMockIoObjectRequest : public TestMockFixture { void expect_copyup(MockCopyupRequest& mock_copyup_request, MockAbstractObjectWriteRequest** write_request, int r) { - EXPECT_CALL(mock_copyup_request, append_request(_)) - .WillOnce(Invoke([write_request](MockAbstractObjectWriteRequest *req) { - *write_request = req; - })); + EXPECT_CALL(mock_copyup_request, append_request(_, _)) + .WillOnce(WithArg<0>( + Invoke([write_request](MockAbstractObjectWriteRequest *req) { + *write_request = req; + }))); EXPECT_CALL(mock_copyup_request, send()) .WillOnce(Invoke([write_request, r]() { (*write_request)->handle_copyup(r);