From dd5452004047c31e0f3eb813d5dd0b50bd68e76f Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Thu, 19 Oct 2017 14:32:05 -0400 Subject: [PATCH] librbd: templatize io::CopyupRequest and io::ObjectRequest Signed-off-by: Jason Dillaman --- src/librbd/ImageCtx.h | 4 +- src/librbd/io/CopyupRequest.cc | 48 ++++++++++++++--------- src/librbd/io/CopyupRequest.h | 16 ++++++-- src/librbd/io/ObjectRequest.cc | 59 +++++++++++++++-------------- src/librbd/io/ObjectRequest.h | 6 +-- src/test/librbd/mock/MockImageCtx.h | 9 +++++ 6 files changed, 87 insertions(+), 55 deletions(-) diff --git a/src/librbd/ImageCtx.h b/src/librbd/ImageCtx.h index 910182a70dd..9479bae30a9 100644 --- a/src/librbd/ImageCtx.h +++ b/src/librbd/ImageCtx.h @@ -52,8 +52,8 @@ namespace librbd { namespace io { class AioCompletion; class AsyncOperation; + template class CopyupRequest; template class ImageRequestWQ; - class CopyupRequest; } namespace journal { struct Policy; } @@ -140,7 +140,7 @@ namespace librbd { Readahead readahead; uint64_t total_bytes_read; - std::map copyup_list; + std::map*> copyup_list; xlist async_ops; xlist*> async_requests; diff --git a/src/librbd/io/CopyupRequest.cc b/src/librbd/io/CopyupRequest.cc index 5f01b45e259..ac3cffb43b8 100644 --- a/src/librbd/io/CopyupRequest.cc +++ b/src/librbd/io/CopyupRequest.cc @@ -80,11 +80,11 @@ private: } // anonymous namespace - -CopyupRequest::CopyupRequest(ImageCtx *ictx, const std::string &oid, - uint64_t objectno, Extents &&image_extents, - const ZTracer::Trace &parent_trace) - : m_ictx(ictx), m_oid(oid), m_object_no(objectno), +template +CopyupRequest::CopyupRequest(I *ictx, const std::string &oid, + uint64_t objectno, Extents &&image_extents, + const ZTracer::Trace &parent_trace) + : m_ictx(util::get_image_ctx(ictx)), m_oid(oid), m_object_no(objectno), m_image_extents(image_extents), m_trace(util::create_trace(*m_ictx, "copy-up", parent_trace)), m_state(STATE_READ_FROM_PARENT) @@ -92,17 +92,20 @@ CopyupRequest::CopyupRequest(ImageCtx *ictx, const std::string &oid, m_async_op.start_op(*m_ictx); } -CopyupRequest::~CopyupRequest() { +template +CopyupRequest::~CopyupRequest() { assert(m_pending_requests.empty()); m_async_op.finish_op(); } -void CopyupRequest::append_request(ObjectRequest<> *req) { +template +void CopyupRequest::append_request(ObjectRequest *req) { ldout(m_ictx->cct, 20) << req << dendl; m_pending_requests.push_back(req); } -void CopyupRequest::complete_requests(int r) { +template +void CopyupRequest::complete_requests(int r) { while (!m_pending_requests.empty()) { vector *>::iterator it = m_pending_requests.begin(); ObjectRequest<> *req = *it; @@ -112,7 +115,8 @@ void CopyupRequest::complete_requests(int r) { } } -bool CopyupRequest::send_copyup() { +template +bool CopyupRequest::send_copyup() { bool add_copyup_op = !m_copyup_data.is_zero(); bool copy_on_read = m_pending_requests.empty(); if (!add_copyup_op && copy_on_read) { @@ -189,7 +193,8 @@ bool CopyupRequest::send_copyup() { return false; } -bool CopyupRequest::is_copyup_required() { +template +bool CopyupRequest::is_copyup_required() { bool noop = true; for (const ObjectRequest<> *req : m_pending_requests) { if (!req->is_op_payload_empty()) { @@ -201,7 +206,8 @@ bool CopyupRequest::is_copyup_required() { return (m_copyup_data.is_zero() && noop); } -void CopyupRequest::send() +template +void CopyupRequest::send() { m_state = STATE_READ_FROM_PARENT; AioCompletion *comp = AioCompletion::create_and_start( @@ -215,7 +221,8 @@ void CopyupRequest::send() ReadResult{&m_copyup_data}, 0, m_trace); } -void CopyupRequest::complete(int r) +template +void CopyupRequest::complete(int r) { if (should_complete(r)) { complete_requests(r); @@ -223,7 +230,8 @@ void CopyupRequest::complete(int r) } } -bool CopyupRequest::should_complete(int r) +template +bool CopyupRequest::should_complete(int r) { CephContext *cct = m_ictx->cct; ldout(cct, 20) << "oid " << m_oid @@ -277,17 +285,18 @@ bool CopyupRequest::should_complete(int r) return (r < 0); } -void CopyupRequest::remove_from_list() +template +void CopyupRequest::remove_from_list() { Mutex::Locker l(m_ictx->copyup_list_lock); - map::iterator it = - m_ictx->copyup_list.find(m_object_no); + auto it = m_ictx->copyup_list.find(m_object_no); assert(it != m_ictx->copyup_list.end()); m_ictx->copyup_list.erase(it); } -bool CopyupRequest::send_object_map_head() { +template +bool CopyupRequest::send_object_map_head() { CephContext *cct = m_ictx->cct; ldout(cct, 20) << dendl; @@ -346,7 +355,8 @@ bool CopyupRequest::send_object_map_head() { return send_object_map(); } -bool CopyupRequest::send_object_map() { +template +bool CopyupRequest::send_object_map() { // avoid possible recursive lock attempts if (m_snap_ids.empty()) { // no object map update required @@ -371,3 +381,5 @@ bool CopyupRequest::send_object_map() { } // namespace io } // namespace librbd + +template class librbd::io::CopyupRequest; diff --git a/src/librbd/io/CopyupRequest.h b/src/librbd/io/CopyupRequest.h index c4aa6a25956..6b0413bb4e9 100644 --- a/src/librbd/io/CopyupRequest.h +++ b/src/librbd/io/CopyupRequest.h @@ -26,13 +26,21 @@ namespace io { struct AioCompletion; template class ObjectRequest; +template class CopyupRequest { public: - CopyupRequest(ImageCtx *ictx, const std::string &oid, uint64_t objectno, + static CopyupRequest* create(ImageCtxT *ictx, const std::string &oid, + uint64_t objectno, Extents &&image_extents, + const ZTracer::Trace &parent_trace) { + return new CopyupRequest(ictx, oid, objectno, std::move(image_extents), + parent_trace); + } + + CopyupRequest(ImageCtxT *ictx, const std::string &oid, uint64_t objectno, Extents &&image_extents, const ZTracer::Trace &parent_trace); ~CopyupRequest(); - void append_request(ObjectRequest *req); + void append_request(ObjectRequest *req); void send(); @@ -84,7 +92,7 @@ private: State m_state; ceph::bufferlist m_copyup_data; - std::vector *> m_pending_requests; + std::vector *> m_pending_requests; std::atomic m_pending_copyups { 0 }; AsyncOperation m_async_op; @@ -107,4 +115,6 @@ private: } // namespace io } // namespace librbd +extern template class librbd::io::CopyupRequest; + #endif // CEPH_LIBRBD_IO_COPYUP_REQUEST_H diff --git a/src/librbd/io/ObjectRequest.cc b/src/librbd/io/ObjectRequest.cc index d6a3905deab..16e8aabf20d 100644 --- a/src/librbd/io/ObjectRequest.cc +++ b/src/librbd/io/ObjectRequest.cc @@ -31,6 +31,19 @@ namespace librbd { namespace io { +namespace { + +template +inline bool is_copy_on_read(I *ictx, librados::snap_t snap_id) { + assert(ictx->snap_lock.is_locked()); + return (ictx->clone_copy_on_read && + !ictx->read_only && snap_id == CEPH_NOSNAP && + (ictx->exclusive_lock == nullptr || + ictx->exclusive_lock->is_lock_owner())); +} + +} // anonymous namespace + template ObjectRequest* ObjectRequest::create_remove(I *ictx, const std::string &oid, @@ -121,7 +134,7 @@ ObjectRequest::create_compare_and_write(I *ictx, const std::string &oid, } template -ObjectRequest::ObjectRequest(ImageCtx *ictx, const std::string &oid, +ObjectRequest::ObjectRequest(I *ictx, const std::string &oid, uint64_t objectno, uint64_t off, uint64_t len, librados::snap_t snap_id, bool hide_enoent, const char *trace_name, @@ -186,14 +199,6 @@ bool ObjectRequest::compute_parent_extents() { return false; } -static inline bool is_copy_on_read(ImageCtx *ictx, librados::snap_t snap_id) { - assert(ictx->snap_lock.is_locked()); - return (ictx->clone_copy_on_read && - !ictx->read_only && snap_id == CEPH_NOSNAP && - (ictx->exclusive_lock == nullptr || - ictx->exclusive_lock->is_lock_owner())); -} - /** read **/ template @@ -204,8 +209,8 @@ ObjectReadRequest::ObjectReadRequest(I *ictx, const std::string &oid, int op_flags, const ZTracer::Trace &parent_trace, Context *completion) - : ObjectRequest(util::get_image_ctx(ictx), oid, objectno, offset, len, - snap_id, false, "read", parent_trace, completion), + : ObjectRequest(ictx, oid, objectno, offset, len, snap_id, false, "read", + parent_trace, completion), m_buffer_extents(be), m_tried_parent(false), m_sparse(sparse), m_op_flags(op_flags), m_state(LIBRBD_AIO_READ_FLAT) { guard_read(); @@ -214,7 +219,7 @@ ObjectReadRequest::ObjectReadRequest(I *ictx, const std::string &oid, template void ObjectReadRequest::guard_read() { - ImageCtx *image_ctx = this->m_ictx; + I *image_ctx = this->m_ictx; RWLock::RLocker snap_locker(image_ctx->snap_lock); RWLock::RLocker parent_locker(image_ctx->parent_lock); @@ -227,7 +232,7 @@ void ObjectReadRequest::guard_read() template bool ObjectReadRequest::should_complete(int r) { - ImageCtx *image_ctx = this->m_ictx; + I *image_ctx = this->m_ictx; ldout(image_ctx->cct, 20) << this->m_oid << " " << this->m_object_off << "~" << this->m_object_len << " r = " << r << dendl; @@ -302,7 +307,7 @@ bool ObjectReadRequest::should_complete(int r) template void ObjectReadRequest::send() { - ImageCtx *image_ctx = this->m_ictx; + I *image_ctx = this->m_ictx; ldout(image_ctx->cct, 20) << this->m_oid << " " << this->m_object_off << "~" << this->m_object_len << dendl; @@ -342,7 +347,7 @@ void ObjectReadRequest::send() { template void ObjectReadRequest::send_copyup() { - ImageCtx *image_ctx = this->m_ictx; + I *image_ctx = this->m_ictx; ldout(image_ctx->cct, 20) << this->m_oid << " " << this->m_object_off << "~" << this->m_object_len << dendl; @@ -357,11 +362,10 @@ void ObjectReadRequest::send_copyup() } Mutex::Locker copyup_locker(image_ctx->copyup_list_lock); - map::iterator it = - image_ctx->copyup_list.find(this->m_object_no); + auto it = image_ctx->copyup_list.find(this->m_object_no); if (it == image_ctx->copyup_list.end()) { // create and kick off a CopyupRequest - CopyupRequest *new_req = new CopyupRequest( + auto new_req = CopyupRequest::create( image_ctx, this->m_oid, this->m_object_no, std::move(this->m_parent_extents), this->m_trace); this->m_parent_extents.clear(); @@ -374,15 +378,15 @@ void ObjectReadRequest::send_copyup() template void ObjectReadRequest::read_from_parent(Extents&& parent_extents) { - ImageCtx *image_ctx = this->m_ictx; + I *image_ctx = this->m_ictx; AioCompletion *parent_completion = AioCompletion::create_and_start< - ObjectRequest >(this, image_ctx, AIO_TYPE_READ); + ObjectRequest >(this, util::get_image_ctx(image_ctx), AIO_TYPE_READ); ldout(image_ctx->cct, 20) << "parent completion " << parent_completion << " extents " << parent_extents << dendl; - ImageRequest<>::aio_read(image_ctx->parent, parent_completion, - std::move(parent_extents), - ReadResult{&m_read_data}, 0, this->m_trace); + ImageRequest::aio_read(image_ctx->parent, parent_completion, + std::move(parent_extents), + ReadResult{&m_read_data}, 0, this->m_trace); } /** write **/ @@ -568,13 +572,10 @@ void AbstractObjectWriteRequest::send_copyup() m_state = LIBRBD_AIO_WRITE_COPYUP; m_ictx->copyup_list_lock.Lock(); - map::iterator it = - m_ictx->copyup_list.find(m_object_no); + auto it = m_ictx->copyup_list.find(m_object_no); if (it == m_ictx->copyup_list.end()) { - CopyupRequest *new_req = new CopyupRequest(m_ictx, m_oid, - m_object_no, - std::move(m_parent_extents), - this->m_trace); + auto new_req = CopyupRequest<>::create( + m_ictx, m_oid, m_object_no, std::move(m_parent_extents), this->m_trace); m_parent_extents.clear(); // make sure to wait on this CopyupRequest diff --git a/src/librbd/io/ObjectRequest.h b/src/librbd/io/ObjectRequest.h index 9b6b5a9bc6d..9c3551db310 100644 --- a/src/librbd/io/ObjectRequest.h +++ b/src/librbd/io/ObjectRequest.h @@ -21,7 +21,7 @@ struct ImageCtx; namespace io { struct AioCompletion; -class CopyupRequest; +template class CopyupRequest; class ObjectRemoveRequest; class ObjectTruncateRequest; class ObjectWriteRequest; @@ -97,7 +97,7 @@ public: const ZTracer::Trace &parent_trace, Context *completion); - ObjectRequest(ImageCtx *ictx, const std::string &oid, + ObjectRequest(ImageCtxT *ictx, const std::string &oid, uint64_t objectno, uint64_t off, uint64_t len, librados::snap_t snap_id, bool hide_enoent, const char *trace_name, const ZTracer::Trace &parent_trace, @@ -129,7 +129,7 @@ public: protected: bool compute_parent_extents(); - ImageCtx *m_ictx; + ImageCtxT *m_ictx; std::string m_oid; uint64_t m_object_no, m_object_off, m_object_len; librados::snap_t m_snap_id; diff --git a/src/test/librbd/mock/MockImageCtx.h b/src/test/librbd/mock/MockImageCtx.h index dca86c17d2b..3b922819459 100644 --- a/src/test/librbd/mock/MockImageCtx.h +++ b/src/test/librbd/mock/MockImageCtx.h @@ -66,6 +66,7 @@ struct MockImageCtx { parent_lock(image_ctx.parent_lock), object_map_lock(image_ctx.object_map_lock), async_ops_lock(image_ctx.async_ops_lock), + copyup_list_lock(image_ctx.copyup_list_lock), order(image_ctx.order), size(image_ctx.size), features(image_ctx.features), @@ -92,6 +93,7 @@ struct MockImageCtx { concurrent_management_ops(image_ctx.concurrent_management_ops), blacklist_on_break_lock(image_ctx.blacklist_on_break_lock), blacklist_expire_seconds(image_ctx.blacklist_expire_seconds), + sparse_read_threshold_bytes(image_ctx.sparse_read_threshold_bytes), journal_order(image_ctx.journal_order), journal_splay_width(image_ctx.journal_splay_width), journal_commit_age(image_ctx.journal_commit_age), @@ -148,6 +150,7 @@ struct MockImageCtx { MOCK_CONST_METHOD0(get_current_size, uint64_t()); MOCK_CONST_METHOD1(get_image_size, uint64_t(librados::snap_t)); MOCK_CONST_METHOD1(get_object_count, uint64_t(librados::snap_t)); + MOCK_CONST_METHOD1(get_read_flags, int(librados::snap_t)); MOCK_CONST_METHOD2(get_snap_id, librados::snap_t(cls::rbd::SnapshotNamespace snap_namespace, std::string in_snap_name)); @@ -158,6 +161,8 @@ struct MockImageCtx { ParentSpec *pspec)); MOCK_CONST_METHOD2(get_parent_overlap, int(librados::snap_t in_snap_id, uint64_t *overlap)); + MOCK_CONST_METHOD2(prune_parent_extents, uint64_t(vector >& , + uint64_t)); MOCK_CONST_METHOD2(is_snap_protected, int(librados::snap_t in_snap_id, bool *is_protected)); @@ -247,6 +252,7 @@ struct MockImageCtx { RWLock &parent_lock; RWLock &object_map_lock; Mutex &async_ops_lock; + Mutex ©up_list_lock; uint8_t order; uint64_t size; @@ -268,6 +274,8 @@ struct MockImageCtx { xlist*> async_requests; std::list async_requests_waiters; + std::map*> copyup_list; + io::MockImageRequestWQ *io_work_queue; MockContextWQ *op_work_queue; @@ -292,6 +300,7 @@ struct MockImageCtx { int concurrent_management_ops; bool blacklist_on_break_lock; uint32_t blacklist_expire_seconds; + uint64_t sparse_read_threshold_bytes; uint8_t journal_order; uint8_t journal_splay_width; double journal_commit_age;