From b2c88820923ef033a31620e7d77f84e7bb695e9d Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Mon, 12 Sep 2022 21:05:44 +0200 Subject: [PATCH] librbd: return area from extents_to_file() Note that, as suggested by extents_to_file() signature, all returned image extents would pertain to the same area. This means that an area boundary must coincide with an object boundary. luks::FormatRequest is actually more strict: crypto header area size is set to a multiple of stripe period (i.e. one or more whole objects). Signed-off-by: Ilya Dryomov --- src/librbd/cache/ObjectCacherWriteback.cc | 17 +++---- src/librbd/crypto/CryptoObjectDispatch.cc | 5 +-- src/librbd/deep_copy/ObjectCopyRequest.cc | 9 ++-- src/librbd/deep_copy/ObjectCopyRequest.h | 1 + src/librbd/io/CopyupRequest.cc | 9 ++-- src/librbd/io/ImageRequest.cc | 6 +-- src/librbd/io/ObjectRequest.cc | 38 ++++++++-------- src/librbd/io/ObjectRequest.h | 5 ++- src/librbd/io/Utils.cc | 45 ++++++++++--------- src/librbd/io/Utils.h | 7 ++- src/librbd/journal/ObjectDispatch.cc | 7 ++- .../deep_copy/test_mock_ObjectCopyRequest.cc | 16 ++++--- src/test/librbd/io/test_mock_CopyupRequest.cc | 16 ++++--- src/test/librbd/io/test_mock_ImageRequest.cc | 16 ++++--- src/test/librbd/io/test_mock_ObjectRequest.cc | 16 ++++--- 15 files changed, 116 insertions(+), 97 deletions(-) diff --git a/src/librbd/cache/ObjectCacherWriteback.cc b/src/librbd/cache/ObjectCacherWriteback.cc index 8c0aaa7bcda59..008098f07b0e2 100644 --- a/src/librbd/cache/ObjectCacherWriteback.cc +++ b/src/librbd/cache/ObjectCacherWriteback.cc @@ -169,10 +169,10 @@ bool ObjectCacherWriteback::may_copy_on_write(const object_t& oid, uint64_t object_no = oid_to_object_no(oid.name, m_ictx->object_prefix); // reverse map this object extent onto the parent - vector > objectx; - io::util::extent_to_file( - m_ictx, object_no, 0, m_ictx->layout.object_size, objectx); - uint64_t object_overlap = m_ictx->prune_parent_extents(objectx, overlap); + auto [parent_extents, _] = io::util::object_to_area_extents( + m_ictx, object_no, {{0, m_ictx->layout.object_size}}); + uint64_t object_overlap = m_ictx->prune_parent_extents(parent_extents, + overlap); bool may = object_overlap > 0; ldout(m_ictx->cct, 10) << "may_copy_on_write " << oid << " " << read_off << "~" << read_len << " = " << may << dendl; @@ -230,8 +230,6 @@ void ObjectCacherWriteback::overwrite_extent(const object_t& oid, uint64_t off, uint64_t len, ceph_tid_t original_journal_tid, ceph_tid_t new_journal_tid) { - typedef std::vector > Extents; - ldout(m_ictx->cct, 20) << __func__ << ": " << oid << " " << off << "~" << len << " " << "journal_tid=" << original_journal_tid << ", " @@ -242,10 +240,9 @@ void ObjectCacherWriteback::overwrite_extent(const object_t& oid, uint64_t off, // all IO operations are flushed prior to closing the journal ceph_assert(original_journal_tid != 0 && m_ictx->journal != NULL); - Extents file_extents; - io::util::extent_to_file(m_ictx, object_no, off, len, file_extents); - for (Extents::iterator it = file_extents.begin(); - it != file_extents.end(); ++it) { + auto [image_extents, _] = io::util::object_to_area_extents(m_ictx, object_no, + {{off, len}}); + for (auto it = image_extents.begin(); it != image_extents.end(); ++it) { if (new_journal_tid != 0) { // ensure new journal event is safely committed to disk before // committing old event diff --git a/src/librbd/crypto/CryptoObjectDispatch.cc b/src/librbd/crypto/CryptoObjectDispatch.cc index eea60af4f1dd1..8a5ff82cded0a 100644 --- a/src/librbd/crypto/CryptoObjectDispatch.cc +++ b/src/librbd/crypto/CryptoObjectDispatch.cc @@ -619,9 +619,8 @@ int CryptoObjectDispatch::prepare_copyup( auto [aligned_off, aligned_len] = m_crypto->align( extent.get_off(), extent.get_len()); - io::Extents image_extents; - io::util::extent_to_file( - m_image_ctx, object_no, aligned_off, aligned_len, image_extents); + auto [image_extents, _] = io::util::object_to_area_extents( + m_image_ctx, object_no, {{aligned_off, aligned_len}}); ceph::bufferlist encrypted_bl; uint64_t position = 0; diff --git a/src/librbd/deep_copy/ObjectCopyRequest.cc b/src/librbd/deep_copy/ObjectCopyRequest.cc index e86ed5ea1c182..71e8f95a6a1c9 100644 --- a/src/librbd/deep_copy/ObjectCopyRequest.cc +++ b/src/librbd/deep_copy/ObjectCopyRequest.cc @@ -72,10 +72,11 @@ void ObjectCopyRequest::send() { template void ObjectCopyRequest::send_list_snaps() { // image extents are consistent across src and dst so compute once - io::util::extent_to_file( - m_dst_image_ctx, m_dst_object_number, 0, - m_dst_image_ctx->layout.object_size, m_image_extents); - ldout(m_cct, 20) << "image_extents=" << m_image_extents << dendl; + std::tie(m_image_extents, m_image_area) = io::util::object_to_area_extents( + m_dst_image_ctx, m_dst_object_number, + {{0, m_dst_image_ctx->layout.object_size}}); + ldout(m_cct, 20) << "image_extents=" << m_image_extents + << " area=" << m_image_area << dendl; auto ctx = create_async_context_callback( *m_src_image_ctx, create_context_callback< diff --git a/src/librbd/deep_copy/ObjectCopyRequest.h b/src/librbd/deep_copy/ObjectCopyRequest.h index 7a89e333e3af5..fc2f58cd3d838 100644 --- a/src/librbd/deep_copy/ObjectCopyRequest.h +++ b/src/librbd/deep_copy/ObjectCopyRequest.h @@ -116,6 +116,7 @@ private: std::string m_dst_oid; io::Extents m_image_extents; + io::ImageArea m_image_area = io::ImageArea::DATA; io::SnapshotDelta m_snapshot_delta; diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc index c614dd5c213ce..81847996d5a12 100644 --- a/src/librbd/io/CopyupRequest.cc +++ b/src/librbd/io/CopyupRequest.cc @@ -658,11 +658,10 @@ void CopyupRequest::compute_deep_copy_snap_ids() { if (parent_overlap == 0) { return false; } - std::vector> extents; - util::extent_to_file(m_image_ctx, m_object_no, 0, - m_image_ctx->layout.object_size, extents); - auto overlap = m_image_ctx->prune_parent_extents( - extents, parent_overlap); + auto [parent_extents, _] = util::object_to_area_extents( + m_image_ctx, m_object_no, {{0, m_image_ctx->layout.object_size}}); + auto overlap = m_image_ctx->prune_parent_extents(parent_extents, + parent_overlap); return overlap > 0; }); } diff --git a/src/librbd/io/ImageRequest.cc b/src/librbd/io/ImageRequest.cc index d00292e52d114..5b897a5ba209b 100644 --- a/src/librbd/io/ImageRequest.cc +++ b/src/librbd/io/ImageRequest.cc @@ -89,9 +89,9 @@ struct C_AssembleSnapshotDeltas : public C_AioRequest { SnapshotDelta* assembled_image_snapshot_delta) { for (auto& [key, object_extents] : object_snapshot_delta) { for (auto& object_extent : object_extents) { - Extents image_extents; - io::util::extent_to_file(image_ctx, object_no, object_extent.get_off(), - object_extent.get_len(), image_extents); + auto [image_extents, _] = io::util::object_to_area_extents( + image_ctx, object_no, + {{object_extent.get_off(), object_extent.get_len()}}); auto& intervals = (*image_snapshot_delta)[key]; auto& assembled_intervals = (*assembled_image_snapshot_delta)[key]; diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc index 87c3cd7dd5289..eaf6ecfb13d94 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -145,11 +145,13 @@ void ObjectRequest::add_write_hint(I& image_ctx, neorados::WriteOp* wr) { template bool ObjectRequest::compute_parent_extents(Extents *parent_extents, + ImageArea *area, bool read_request) { ceph_assert(ceph_mutex_is_locked(m_ictx->image_lock)); m_has_parent = false; parent_extents->clear(); + *area = ImageArea::DATA; uint64_t parent_overlap; int r = m_ictx->get_parent_overlap( @@ -170,13 +172,14 @@ bool ObjectRequest::compute_parent_extents(Extents *parent_extents, return false; } - io::util::extent_to_file(m_ictx, m_object_no, 0, m_ictx->layout.object_size, - *parent_extents); + std::tie(*parent_extents, *area) = io::util::object_to_area_extents( + m_ictx, m_object_no, {{0, m_ictx->layout.object_size}}); uint64_t object_overlap = m_ictx->prune_parent_extents(*parent_extents, parent_overlap); if (object_overlap > 0) { - ldout(m_ictx->cct, 20) << "overlap " << parent_overlap << " " - << "extents " << *parent_extents << dendl; + ldout(m_ictx->cct, 20) << "overlap=" << parent_overlap + << " extents=" << *parent_extents + << " area=" << *area << dendl; m_has_parent = !parent_extents->empty(); return true; } @@ -323,7 +326,8 @@ void ObjectReadRequest::copyup() { image_ctx->owner_lock.lock_shared(); image_ctx->image_lock.lock_shared(); Extents parent_extents; - if (!this->compute_parent_extents(&parent_extents, true) || + ImageArea area; + if (!this->compute_parent_extents(&parent_extents, &area, true) || (image_ctx->exclusive_lock != nullptr && !image_ctx->exclusive_lock->is_lock_owner())) { image_ctx->image_lock.unlock_shared(); @@ -384,7 +388,7 @@ void AbstractObjectWriteRequest::compute_parent_info() { I *image_ctx = this->m_ictx; std::shared_lock image_locker{image_ctx->image_lock}; - this->compute_parent_extents(&m_parent_extents, false); + this->compute_parent_extents(&m_parent_extents, &m_image_area, false); if (!this->has_parent() || (m_full_object && @@ -711,12 +715,11 @@ template int ObjectCompareAndWriteRequest::filter_write_result(int r) const { if (r <= -MAX_ERRNO) { I *image_ctx = this->m_ictx; - Extents image_extents; // object extent compare mismatch uint64_t offset = -MAX_ERRNO - r; - io::util::extent_to_file(image_ctx, this->m_object_no, offset, - this->m_object_len, image_extents); + auto [image_extents, _] = io::util::object_to_area_extents( + image_ctx, this->m_object_no, {{offset, this->m_object_len}}); ceph_assert(image_extents.size() == 1); if (m_mismatch_offset) { @@ -956,17 +959,15 @@ void ObjectListSnapsRequest::list_from_parent() { } // calculate reverse mapping onto the parent image - Extents parent_image_extents; - for (auto [object_off, object_len]: m_object_extents) { - io::util::extent_to_file(image_ctx, this->m_object_no, object_off, - object_len, parent_image_extents); - } + Extents parent_extents; + std::tie(parent_extents, m_image_area) = io::util::object_to_area_extents( + image_ctx, this->m_object_no, m_object_extents); uint64_t parent_overlap = 0; uint64_t object_overlap = 0; int r = image_ctx->get_parent_overlap(snap_id_end, &parent_overlap); if (r == 0) { - object_overlap = image_ctx->prune_parent_extents(parent_image_extents, + object_overlap = image_ctx->prune_parent_extents(parent_extents, parent_overlap); } @@ -982,14 +983,15 @@ void ObjectListSnapsRequest::list_from_parent() { &ObjectListSnapsRequest::handle_list_from_parent>(this); auto aio_comp = AioCompletion::create_and_start( ctx, librbd::util::get_image_ctx(image_ctx->parent), AIO_TYPE_GENERIC); - ldout(cct, 20) << "aio_comp=" << aio_comp<< ", " - << "parent_image_extents " << parent_image_extents << dendl; + ldout(cct, 20) << "completion=" << aio_comp + << " parent_extents=" << parent_extents + << " area=" << m_image_area << dendl; auto list_snaps_flags = ( m_list_snaps_flags | LIST_SNAPS_FLAG_IGNORE_ZEROED_EXTENTS); ImageListSnapsRequest req( - *image_ctx->parent, aio_comp, std::move(parent_image_extents), + *image_ctx->parent, aio_comp, std::move(parent_extents), {0, image_ctx->parent->snap_id}, list_snaps_flags, &m_parent_snapshot_delta, this->m_trace); req.send(); diff --git a/src/librbd/io/ObjectRequest.h b/src/librbd/io/ObjectRequest.h index 89ca224cc90e7..caf644023be7d 100644 --- a/src/librbd/io/ObjectRequest.h +++ b/src/librbd/io/ObjectRequest.h @@ -74,7 +74,8 @@ public: virtual const char *get_op_type() const = 0; protected: - bool compute_parent_extents(Extents *parent_extents, bool read_request); + bool compute_parent_extents(Extents *parent_extents, ImageArea *area, + bool read_request); ImageCtxT *m_ictx; uint64_t m_object_no; @@ -236,6 +237,7 @@ private: */ Extents m_parent_extents; + ImageArea m_image_area = ImageArea::DATA; bool m_object_may_exist = false; bool m_copyup_in_progress = false; bool m_guarding_migration_write = false; @@ -476,6 +478,7 @@ private: neorados::SnapSet m_snap_set; boost::system::error_code m_ec; + ImageArea m_image_area = ImageArea::DATA; SnapshotDelta m_parent_snapshot_delta; void list_snaps(); diff --git a/src/librbd/io/Utils.cc b/src/librbd/io/Utils.cc index 5b786ff8f52fd..ad725cc22f0c5 100644 --- a/src/librbd/io/Utils.cc +++ b/src/librbd/io/Utils.cc @@ -84,7 +84,7 @@ bool assemble_write_same_extent( } template -void read_parent(I *image_ctx, uint64_t object_no, ReadExtents* extents, +void read_parent(I *image_ctx, uint64_t object_no, ReadExtents* read_extents, librados::snap_t snap_id, const ZTracer::Trace &trace, Context* on_finish) { @@ -93,11 +93,12 @@ void read_parent(I *image_ctx, uint64_t object_no, ReadExtents* extents, std::shared_lock image_locker{image_ctx->image_lock}; // calculate reverse mapping onto the image - Extents parent_extents; - for (auto& extent: *extents) { - extent_to_file(image_ctx, object_no, extent.offset, extent.length, - parent_extents); + Extents extents; + for (const auto& extent : *read_extents) { + extents.emplace_back(extent.offset, extent.length); } + auto [parent_extents, area] = object_to_area_extents(image_ctx, object_no, + extents); uint64_t parent_overlap = 0; uint64_t object_overlap = 0; @@ -117,19 +118,20 @@ void read_parent(I *image_ctx, uint64_t object_no, ReadExtents* extents, ldout(cct, 20) << dendl; ceph::bufferlist* parent_read_bl; - if (extents->size() > 1) { + if (read_extents->size() > 1) { auto parent_comp = new ReadResult::C_ObjectReadMergedExtents( - cct, extents, on_finish); + cct, read_extents, on_finish); parent_read_bl = &parent_comp->bl; on_finish = parent_comp; } else { - parent_read_bl = &extents->front().bl; + parent_read_bl = &read_extents->front().bl; } auto comp = AioCompletion::create_and_start(on_finish, image_ctx->parent, AIO_TYPE_READ); - ldout(cct, 20) << "completion " << comp << ", extents " << parent_extents - << dendl; + ldout(cct, 20) << "completion=" << comp + << " parent_extents=" << parent_extents + << " area=" << area << dendl; auto req = io::ImageDispatchSpec::create_read( *image_ctx->parent, io::IMAGE_DISPATCH_LAYER_INTERNAL_START, comp, std::move(parent_extents), ReadResult{parent_read_bl}, @@ -195,13 +197,15 @@ void file_to_extents(I* image_ctx, uint64_t offset, uint64_t length, } template -void extent_to_file(I* image_ctx, uint64_t object_no, uint64_t offset, - uint64_t length, - std::vector >& extents) { - Striper::extent_to_file(image_ctx->cct, &image_ctx->layout, object_no, - offset, length, extents); - // TODO: return area - image_ctx->io_image_dispatcher->remap_to_logical(extents); +std::pair object_to_area_extents( + I* image_ctx, uint64_t object_no, const Extents& object_extents) { + Extents extents; + for (auto [off, len] : object_extents) { + Striper::extent_to_file(image_ctx->cct, &image_ctx->layout, object_no, off, + len, extents); + } + auto area = image_ctx->io_image_dispatcher->remap_to_logical(extents); + return {std::move(extents), area}; } template @@ -245,10 +249,9 @@ template void librbd::io::util::file_to_extents( librbd::ImageCtx *image_ctx, uint64_t offset, uint64_t length, uint64_t buffer_offset, striper::LightweightObjectExtents* object_extents); -template void librbd::io::util::extent_to_file( - librbd::ImageCtx *image_ctx, uint64_t object_no, uint64_t offset, - uint64_t length, - std::vector >& extents); +template auto librbd::io::util::object_to_area_extents( + librbd::ImageCtx* image_ctx, uint64_t object_no, const Extents& extents) + -> std::pair; template uint64_t librbd::io::util::area_to_raw_offset( const librbd::ImageCtx& image_ctx, uint64_t offset, ImageArea area); template auto librbd::io::util::raw_to_area_offset( diff --git a/src/librbd/io/Utils.h b/src/librbd/io/Utils.h index 21218a8eb5be4..16a14c105331a 100644 --- a/src/librbd/io/Utils.h +++ b/src/librbd/io/Utils.h @@ -32,7 +32,7 @@ bool assemble_write_same_extent(const LightweightObjectExtent &object_extent, template void read_parent(ImageCtxT *image_ctx, uint64_t object_no, - ReadExtents* extents, librados::snap_t snap_id, + ReadExtents* read_extents, librados::snap_t snap_id, const ZTracer::Trace &trace, Context* on_finish); template @@ -60,9 +60,8 @@ void file_to_extents(ImageCtxT *image_ctx, uint64_t offset, uint64_t length, striper::LightweightObjectExtents* object_extents); template -void extent_to_file(ImageCtxT *image_ctx, uint64_t object_no, uint64_t offset, - uint64_t length, - std::vector >& extents); +std::pair object_to_area_extents( + ImageCtxT* image_ctx, uint64_t object_no, const Extents& object_extents); template uint64_t area_to_raw_offset(const ImageCtxT& image_ctx, uint64_t offset, diff --git a/src/librbd/journal/ObjectDispatch.cc b/src/librbd/journal/ObjectDispatch.cc index 5623c635d8f62..e3659c221b119 100644 --- a/src/librbd/journal/ObjectDispatch.cc +++ b/src/librbd/journal/ObjectDispatch.cc @@ -51,10 +51,9 @@ struct C_CommitIOEvent : public Context { if (r >= 0 || (object_dispatch_flags & io::OBJECT_DISPATCH_FLAG_WILL_RETRY_ON_ERROR) == 0) { - io::Extents file_extents; - io::util::extent_to_file(image_ctx, object_no, object_off, object_len, - file_extents); - for (auto& extent : file_extents) { + auto [image_extents, _] = io::util::object_to_area_extents( + image_ctx, object_no, {{object_off, object_len}}); + for (const auto& extent : image_extents) { journal->commit_io_event_extent(journal_tid, extent.first, extent.second, r); } diff --git a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc index d8bb0678a1355..af0ebd28cced4 100644 --- a/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc +++ b/src/test/librbd/deep_copy/test_mock_ObjectCopyRequest.cc @@ -53,12 +53,16 @@ template <> void file_to_extents( 0, buffer_offset, object_extents); } -template <> void extent_to_file( - MockTestImageCtx* image_ctx, uint64_t object_no, uint64_t offset, - uint64_t length, - std::vector >& extents) { - Striper::extent_to_file(image_ctx->cct, &image_ctx->layout, object_no, - offset, length, extents); +template <> +std::pair object_to_area_extents( + MockTestImageCtx* image_ctx, uint64_t object_no, + const Extents& object_extents) { + Extents extents; + for (auto [off, len] : object_extents) { + Striper::extent_to_file(image_ctx->cct, &image_ctx->layout, object_no, off, + len, extents); + } + return {std::move(extents), ImageArea::DATA}; } } // namespace util diff --git a/src/test/librbd/io/test_mock_CopyupRequest.cc b/src/test/librbd/io/test_mock_CopyupRequest.cc index 8c2e07f4e5d78..14d15cea17958 100644 --- a/src/test/librbd/io/test_mock_CopyupRequest.cc +++ b/src/test/librbd/io/test_mock_CopyupRequest.cc @@ -92,12 +92,16 @@ template <> void file_to_extents( 0, buffer_offset, object_extents); } -template <> void extent_to_file( - MockTestImageCtx* image_ctx, uint64_t object_no, uint64_t offset, - uint64_t length, - std::vector >& extents) { - Striper::extent_to_file(image_ctx->cct, &image_ctx->layout, object_no, - offset, length, extents); +template <> +std::pair object_to_area_extents( + MockTestImageCtx* image_ctx, uint64_t object_no, + const Extents& object_extents) { + Extents extents; + for (auto [off, len] : object_extents) { + Striper::extent_to_file(image_ctx->cct, &image_ctx->layout, object_no, off, + len, extents); + } + return {std::move(extents), ImageArea::DATA}; } } // namespace util diff --git a/src/test/librbd/io/test_mock_ImageRequest.cc b/src/test/librbd/io/test_mock_ImageRequest.cc index cd26fb65e70ed..8487c4d5537a7 100644 --- a/src/test/librbd/io/test_mock_ImageRequest.cc +++ b/src/test/librbd/io/test_mock_ImageRequest.cc @@ -69,12 +69,16 @@ void file_to_extents( 0, buffer_offset, object_extents); } -template <> void extent_to_file( - MockTestImageCtx* image_ctx, uint64_t object_no, uint64_t offset, - uint64_t length, - std::vector >& extents) { - Striper::extent_to_file(image_ctx->cct, &image_ctx->layout, object_no, - offset, length, extents); +template <> +std::pair object_to_area_extents( + MockTestImageCtx* image_ctx, uint64_t object_no, + const Extents& object_extents) { + Extents extents; + for (auto [off, len] : object_extents) { + Striper::extent_to_file(image_ctx->cct, &image_ctx->layout, object_no, off, + len, extents); + } + return {std::move(extents), ImageArea::DATA}; } } // namespace util diff --git a/src/test/librbd/io/test_mock_ObjectRequest.cc b/src/test/librbd/io/test_mock_ObjectRequest.cc index c9fa33c43e1cf..f8dc2841fd37e 100644 --- a/src/test/librbd/io/test_mock_ObjectRequest.cc +++ b/src/test/librbd/io/test_mock_ObjectRequest.cc @@ -98,12 +98,16 @@ template <> void file_to_extents( 0, buffer_offset, object_extents); } -template <> void extent_to_file( - MockTestImageCtx* image_ctx, uint64_t object_no, uint64_t offset, - uint64_t length, - std::vector >& extents) { - Striper::extent_to_file(image_ctx->cct, &image_ctx->layout, object_no, - offset, length, extents); +template <> +std::pair object_to_area_extents( + MockTestImageCtx* image_ctx, uint64_t object_no, + const Extents& object_extents) { + Extents extents; + for (auto [off, len] : object_extents) { + Striper::extent_to_file(image_ctx->cct, &image_ctx->layout, object_no, off, + len, extents); + } + return {std::move(extents), ImageArea::DATA}; } namespace { -- 2.39.5