From d80cff0dd665d660cd545c5a66d265b6bf732c73 Mon Sep 17 00:00:00 2001 From: Jonas Pfefferle Date: Fri, 5 Aug 2022 19:36:36 +0200 Subject: [PATCH] librbd: remove cmp&write sector size restriction This patch removes the compare and write max sector size len restriction. We can allow up to stripe unit size accesses if the access is aligned properly. To allow larger size compare and write requests in the journal we split the buffers like we do for writes now. Signed-off-by: Jonas Pfefferle --- src/librbd/Journal.cc | 43 +++++++++++++++ src/librbd/Journal.h | 5 ++ src/librbd/io/ImageRequest.cc | 15 ++--- src/test/librbd/io/test_mock_ImageRequest.cc | 4 ++ src/test/librbd/test_mock_Journal.cc | 58 ++++++++++++++++++++ 5 files changed, 116 insertions(+), 9 deletions(-) diff --git a/src/librbd/Journal.cc b/src/librbd/Journal.cc index 0cd38b22ada..8ddce2e8f7d 100644 --- a/src/librbd/Journal.cc +++ b/src/librbd/Journal.cc @@ -792,6 +792,49 @@ uint64_t Journal::append_write_event(uint64_t offset, size_t length, length, flush_entry, 0); } +template +uint64_t Journal::append_compare_and_write_event(uint64_t offset, + size_t length, + const bufferlist &cmp_bl, + const bufferlist &write_bl, + bool flush_entry) { + ceph_assert( + m_max_append_size > journal::AioCompareAndWriteEvent::get_fixed_size()); + uint64_t max_compare_and_write_data_size = + m_max_append_size - journal::AioCompareAndWriteEvent::get_fixed_size(); + // we need double the size because we store cmp and write buffers + max_compare_and_write_data_size /= 2; + + // ensure that the compare and write event fits within the journal entry + Bufferlists bufferlists; + uint64_t bytes_remaining = length; + uint64_t event_offset = 0; + do { + uint64_t event_length = std::min(bytes_remaining, + max_compare_and_write_data_size); + + bufferlist event_cmp_bl; + event_cmp_bl.substr_of(cmp_bl, event_offset, event_length); + bufferlist event_write_bl; + event_write_bl.substr_of(write_bl, event_offset, event_length); + journal::EventEntry event_entry( + journal::AioCompareAndWriteEvent(offset + event_offset, + event_length, + event_cmp_bl, + event_write_bl), + ceph_clock_now()); + + bufferlists.emplace_back(); + encode(event_entry, bufferlists.back()); + + event_offset += event_length; + bytes_remaining -= event_length; + } while (bytes_remaining > 0); + + return append_io_events(journal::EVENT_TYPE_AIO_COMPARE_AND_WRITE, + bufferlists, offset, length, flush_entry, -EILSEQ); +} + template uint64_t Journal::append_io_event(journal::EventEntry &&event_entry, uint64_t offset, size_t length, diff --git a/src/librbd/Journal.h b/src/librbd/Journal.h index 406c0e34cbf..1ef9ffa8830 100644 --- a/src/librbd/Journal.h +++ b/src/librbd/Journal.h @@ -136,6 +136,11 @@ public: uint64_t append_write_event(uint64_t offset, size_t length, const bufferlist &bl, bool flush_entry); + uint64_t append_compare_and_write_event(uint64_t offset, + size_t length, + const bufferlist &cmp_bl, + const bufferlist &write_bl, + bool flush_entry); uint64_t append_io_event(journal::EventEntry &&event_entry, uint64_t offset, size_t length, bool flush_entry, int filter_ret_val); diff --git a/src/librbd/io/ImageRequest.cc b/src/librbd/io/ImageRequest.cc index 5f8d2f0da39..aaf6e215edf 100644 --- a/src/librbd/io/ImageRequest.cc +++ b/src/librbd/io/ImageRequest.cc @@ -748,12 +748,11 @@ uint64_t ImageCompareAndWriteRequest::append_journal_event( uint64_t tid = 0; ceph_assert(this->m_image_extents.size() == 1); auto &extent = this->m_image_extents.front(); - journal::EventEntry event_entry( - journal::AioCompareAndWriteEvent(extent.first, extent.second, m_cmp_bl, - m_bl)); - tid = image_ctx.journal->append_io_event(std::move(event_entry), - extent.first, extent.second, - synchronous, -EILSEQ); + tid = image_ctx.journal->append_compare_and_write_event(extent.first, + extent.second, + m_cmp_bl, + m_bl, + synchronous); return tid; } @@ -800,11 +799,9 @@ int ImageCompareAndWriteRequest::prune_object_extents( return -EINVAL; I &image_ctx = this->m_image_ctx; - uint64_t sector_size = 512ULL; uint64_t su = image_ctx.layout.stripe_unit; auto& object_extent = object_extents->front(); - if (object_extent.offset % sector_size + object_extent.length > sector_size || - (su != 0 && (object_extent.offset % su + object_extent.length > su))) + if (su == 0 || (object_extent.offset % su + object_extent.length > su)) return -EINVAL; return 0; diff --git a/src/test/librbd/io/test_mock_ImageRequest.cc b/src/test/librbd/io/test_mock_ImageRequest.cc index d1390bcf85d..cd26fb65e70 100644 --- a/src/test/librbd/io/test_mock_ImageRequest.cc +++ b/src/test/librbd/io/test_mock_ImageRequest.cc @@ -18,6 +18,10 @@ struct MockTestImageCtx; struct MockTestJournal : public MockJournal { MOCK_METHOD4(append_write_event, uint64_t(uint64_t, size_t, const bufferlist &, bool)); + MOCK_METHOD5(append_compare_and_write_event, uint64_t(uint64_t, size_t, + const bufferlist &, + const bufferlist &, + bool)); MOCK_METHOD5(append_io_event_mock, uint64_t(const journal::EventEntry&, uint64_t, size_t, bool, int)); uint64_t append_io_event(journal::EventEntry &&event_entry, diff --git a/src/test/librbd/test_mock_Journal.cc b/src/test/librbd/test_mock_Journal.cc index b57c66e8907..2fe74d2fe46 100644 --- a/src/test/librbd/test_mock_Journal.cc +++ b/src/test/librbd/test_mock_Journal.cc @@ -463,6 +463,19 @@ public: return mock_journal->append_write_event(0, length, bl, false); } + uint64_t when_append_compare_and_write_event( + MockJournalImageCtx &mock_image_ctx, MockJournal *mock_journal, + uint64_t length) { + bufferlist cmp_bl; + cmp_bl.append_zero(length); + bufferlist write_bl; + write_bl.append_zero(length); + + std::shared_lock owner_locker{mock_image_ctx.owner_lock}; + return mock_journal->append_compare_and_write_event(0, length, cmp_bl, + write_bl, false); + } + uint64_t when_append_io_event(MockJournalImageCtx &mock_image_ctx, MockJournal *mock_journal, int filter_ret_val) { @@ -1125,6 +1138,51 @@ TEST_F(TestMockJournal, AppendWriteEvent) { expect_shut_down_journaler(mock_journaler); } +TEST_F(TestMockJournal, AppendCompareAndWriteEvent) { + REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); + + librbd::ImageCtx *ictx; + ASSERT_EQ(0, open_image(m_image_name, &ictx)); + + MockJournalImageCtx mock_image_ctx(*ictx); + MockJournal *mock_journal = new MockJournal(mock_image_ctx); + MockObjectDispatch mock_object_dispatch; + ::journal::MockJournaler mock_journaler; + MockJournalOpenRequest mock_open_request; + open_journal(mock_image_ctx, mock_journal, mock_object_dispatch, + mock_journaler, mock_open_request); + BOOST_SCOPE_EXIT_ALL(&) { + close_journal(mock_image_ctx, mock_journal, mock_journaler); + mock_journal->put(); + }; + + InSequence seq; + + ::journal::MockFuture mock_future; + Context *on_journal_safe = nullptr; + expect_append_journaler(mock_journaler); + expect_append_journaler(mock_journaler); + expect_append_journaler(mock_journaler); + expect_wait_future(mock_future, &on_journal_safe); + ASSERT_EQ(1U, when_append_compare_and_write_event(mock_image_ctx, + mock_journal, + 1 << 16)); + mock_journal->get_work_queue()->drain(); + + on_journal_safe->complete(0); + C_SaferCond event_ctx; + mock_journal->wait_event(1U, &event_ctx); + ASSERT_EQ(0, event_ctx.wait()); + + expect_future_committed(mock_journaler); + expect_future_committed(mock_journaler); + expect_future_committed(mock_journaler); + mock_journal->commit_io_event(1U, 0); + ictx->op_work_queue->drain(); + + expect_shut_down_journaler(mock_journaler); +} + TEST_F(TestMockJournal, EventCommitError) { REQUIRE_FEATURE(RBD_FEATURE_JOURNALING); -- 2.39.5