From a1b7312b73447536cd44f2f778b75a7d9f8a56f4 Mon Sep 17 00:00:00 2001 From: Jason Dillaman Date: Fri, 13 Oct 2017 14:44:40 -0400 Subject: [PATCH] librbd: combine trim state machine object map batch update states The PRE/POST states were previously divided into two halves for handling the copy-up batch and the direct removal batch. This can be simplified by just using a single PRE/POST for the entire deletion region. Signed-off-by: Jason Dillaman (cherry picked from commit 72ce4576fa2b562799a5bc78bd423cfabe097d67) Conflicts: src/librbd/operation/TrimRequest.cc: trivial resolution src/librbd/operation/TrimRequest.h: trivial resolution --- src/librbd/operation/TrimRequest.cc | 207 +++++++++------------------- src/librbd/operation/TrimRequest.h | 66 ++++----- 2 files changed, 95 insertions(+), 178 deletions(-) diff --git a/src/librbd/operation/TrimRequest.cc b/src/librbd/operation/TrimRequest.cc index 5e3500779df5f..a2d07e1cd173b 100644 --- a/src/librbd/operation/TrimRequest.cc +++ b/src/librbd/operation/TrimRequest.cc @@ -105,6 +105,7 @@ TrimRequest::TrimRequest(I &image_ctx, Context *on_finish, m_delete_off = MIN(new_num_periods * period, original_size); // first object we can delete free and clear m_delete_start = new_num_periods * image_ctx.get_stripe_count(); + m_delete_start_min = m_delete_start; m_num_objects = Striper::get_num_objects(image_ctx.layout, original_size); CephContext *cct = image_ctx.cct; @@ -131,33 +132,23 @@ bool TrimRequest::should_complete(int r) RWLock::RLocker owner_lock(image_ctx.owner_lock); switch (m_state) { - case STATE_PRE_COPYUP: - ldout(cct, 5) << " PRE_COPYUP" << dendl; + case STATE_PRE_TRIM: + ldout(cct, 5) << " PRE_TRIM" << dendl; send_copyup_objects(); break; case STATE_COPYUP_OBJECTS: ldout(cct, 5) << " COPYUP_OBJECTS" << dendl; - send_post_copyup(); - break; - - case STATE_POST_COPYUP: - ldout(cct, 5) << " POST_COPYUP" << dendl; - send_pre_remove(); - break; - - case STATE_PRE_REMOVE: - ldout(cct, 5) << " PRE_REMOVE" << dendl; send_remove_objects(); break; case STATE_REMOVE_OBJECTS: ldout(cct, 5) << " REMOVE_OBJECTS" << dendl; - send_post_remove(); + send_post_trim(); break; - case STATE_POST_REMOVE: - ldout(cct, 5) << " POST_OBJECTS" << dendl; + case STATE_POST_TRIM: + ldout(cct, 5) << " POST_TRIM" << dendl; send_clean_boundary(); break; @@ -180,147 +171,110 @@ bool TrimRequest::should_complete(int r) template void TrimRequest::send() { - send_pre_copyup(); + send_pre_trim(); } template -void TrimRequest::send_copyup_objects() { +void TrimRequest::send_pre_trim() { I &image_ctx = this->m_image_ctx; assert(image_ctx.owner_lock.is_locked()); - ldout(image_ctx.cct, 5) << this << " send_copyup_objects: " - << " start object=" << m_copyup_start << ", " - << " end object=" << m_copyup_end << dendl; - m_state = STATE_COPYUP_OBJECTS; + if (m_delete_start >= m_num_objects) { + send_clean_boundary(); + return; + } - ::SnapContext snapc; { RWLock::RLocker snap_locker(image_ctx.snap_lock); - RWLock::RLocker parent_locker(image_ctx.parent_lock); - snapc = image_ctx.snapc; - } - - Context *ctx = this->create_callback_context(); - typename AsyncObjectThrottle::ContextFactory context_factory( - boost::lambda::bind(boost::lambda::new_ptr >(), - boost::lambda::_1, &image_ctx, snapc, boost::lambda::_2)); - AsyncObjectThrottle *throttle = new AsyncObjectThrottle( - this, image_ctx, context_factory, ctx, &m_prog_ctx, m_copyup_start, - m_copyup_end); - throttle->start_ops(image_ctx.concurrent_management_ops); -} + if (image_ctx.object_map != nullptr) { + ldout(image_ctx.cct, 5) << this << " send_pre_trim: " + << " delete_start_min=" << m_delete_start_min + << " num_objects=" << m_num_objects << dendl; + m_state = STATE_PRE_TRIM; -template -void TrimRequest::send_remove_objects() { - I &image_ctx = this->m_image_ctx; - assert(image_ctx.owner_lock.is_locked()); + assert(image_ctx.exclusive_lock->is_lock_owner()); - ldout(image_ctx.cct, 5) << this << " send_remove_objects: " - << " delete_start=" << m_delete_start - << " num_objects=" << m_num_objects << dendl; - m_state = STATE_REMOVE_OBJECTS; + RWLock::WLocker object_map_locker(image_ctx.object_map_lock); + if (image_ctx.object_map->template aio_update >( + CEPH_NOSNAP, m_delete_start_min, m_num_objects, OBJECT_PENDING, + OBJECT_EXISTS, this)) { + return; + } + } + } - Context *ctx = this->create_callback_context(); - typename AsyncObjectThrottle::ContextFactory context_factory( - boost::lambda::bind(boost::lambda::new_ptr >(), - boost::lambda::_1, &image_ctx, boost::lambda::_2)); - AsyncObjectThrottle *throttle = new AsyncObjectThrottle( - this, image_ctx, context_factory, ctx, &m_prog_ctx, m_delete_start, - m_num_objects); - throttle->start_ops(image_ctx.concurrent_management_ops); + send_copyup_objects(); } template -void TrimRequest::send_pre_copyup() { +void TrimRequest::send_copyup_objects() { I &image_ctx = this->m_image_ctx; assert(image_ctx.owner_lock.is_locked()); - if (m_delete_start >= m_num_objects) { - send_clean_boundary(); - return; - } - + ::SnapContext snapc; bool has_snapshots; uint64_t parent_overlap; { RWLock::RLocker snap_locker(image_ctx.snap_lock); RWLock::RLocker parent_locker(image_ctx.parent_lock); + snapc = image_ctx.snapc; has_snapshots = !image_ctx.snaps.empty(); int r = image_ctx.get_parent_overlap(CEPH_NOSNAP, &parent_overlap); assert(r == 0); } // copyup is only required for portion of image that overlaps parent - m_copyup_end = Striper::get_num_objects(image_ctx.layout, parent_overlap); + uint64_t copyup_end = Striper::get_num_objects(image_ctx.layout, + parent_overlap); // TODO: protect against concurrent shrink and snap create? // skip to remove if no copyup is required. - if (m_copyup_end <= m_delete_start || !has_snapshots) { - send_pre_remove(); + if (copyup_end <= m_delete_start || !has_snapshots) { + send_remove_objects(); return; } - m_copyup_start = m_delete_start; - m_delete_start = m_copyup_end; - - { - RWLock::RLocker snap_locker(image_ctx.snap_lock); - if (image_ctx.object_map != nullptr) { - ldout(image_ctx.cct, 5) << this << " send_pre_copyup: " - << " copyup_start=" << m_copyup_start - << " copyup_end=" << m_copyup_end << dendl; - m_state = STATE_PRE_COPYUP; - - assert(image_ctx.exclusive_lock->is_lock_owner()); + uint64_t copyup_start = m_delete_start; + m_delete_start = copyup_end; - RWLock::WLocker object_map_locker(image_ctx.object_map_lock); - if (image_ctx.object_map->template aio_update >( - CEPH_NOSNAP, m_copyup_start, m_copyup_end, OBJECT_PENDING, - OBJECT_EXISTS, this)) { - return; - } - } - } + ldout(image_ctx.cct, 5) << this << " send_copyup_objects: " + << " start object=" << copyup_start << ", " + << " end object=" << copyup_end << dendl; + m_state = STATE_COPYUP_OBJECTS; - send_copyup_objects(); + Context *ctx = this->create_callback_context(); + typename AsyncObjectThrottle::ContextFactory context_factory( + boost::lambda::bind(boost::lambda::new_ptr >(), + boost::lambda::_1, &image_ctx, snapc, boost::lambda::_2)); + AsyncObjectThrottle *throttle = new AsyncObjectThrottle( + this, image_ctx, context_factory, ctx, &m_prog_ctx, copyup_start, + copyup_end); + throttle->start_ops(image_ctx.concurrent_management_ops); } template -void TrimRequest::send_pre_remove() { +void TrimRequest::send_remove_objects() { I &image_ctx = this->m_image_ctx; assert(image_ctx.owner_lock.is_locked()); - if (m_delete_start >= m_num_objects) { - send_clean_boundary(); - return; - } - - { - RWLock::RLocker snap_locker(image_ctx.snap_lock); - if (image_ctx.object_map != nullptr) { - ldout(image_ctx.cct, 5) << this << " send_pre_remove: " - << " delete_start=" << m_delete_start - << " num_objects=" << m_num_objects << dendl; - m_state = STATE_PRE_REMOVE; - - assert(image_ctx.exclusive_lock->is_lock_owner()); - // flag the objects as pending deletion - RWLock::WLocker object_map_locker(image_ctx.object_map_lock); - if (image_ctx.object_map->template aio_update >( - CEPH_NOSNAP, m_delete_start, m_num_objects, OBJECT_PENDING, - OBJECT_EXISTS, this)) { - return; - } - } - } + ldout(image_ctx.cct, 5) << this << " send_remove_objects: " + << " delete_start=" << m_delete_start + << " num_objects=" << m_num_objects << dendl; + m_state = STATE_REMOVE_OBJECTS; - // no object map update required - send_remove_objects(); + Context *ctx = this->create_callback_context(); + typename AsyncObjectThrottle::ContextFactory context_factory( + boost::lambda::bind(boost::lambda::new_ptr >(), + boost::lambda::_1, &image_ctx, boost::lambda::_2)); + AsyncObjectThrottle *throttle = new AsyncObjectThrottle( + this, image_ctx, context_factory, ctx, &m_prog_ctx, m_delete_start, + m_num_objects); + throttle->start_ops(image_ctx.concurrent_management_ops); } template -void TrimRequest::send_post_copyup() { +void TrimRequest::send_post_trim() { I &image_ctx = this->m_image_ctx; assert(image_ctx.owner_lock.is_locked()); @@ -328,50 +282,21 @@ void TrimRequest::send_post_copyup() { RWLock::RLocker snap_locker(image_ctx.snap_lock); if (image_ctx.object_map != nullptr) { ldout(image_ctx.cct, 5) << this << " send_post_copyup:" - << " copyup_start=" << m_copyup_start - << " copyup_end=" << m_copyup_end << dendl; - m_state = STATE_POST_COPYUP; - - assert(image_ctx.exclusive_lock->is_lock_owner()); - - RWLock::WLocker object_map_locker(image_ctx.object_map_lock); - if (image_ctx.object_map->template aio_update >( - CEPH_NOSNAP, m_copyup_start, m_copyup_end, OBJECT_NONEXISTENT, - OBJECT_PENDING, this)) { - return; - } - } - } - - send_pre_remove(); -} - -template -void TrimRequest::send_post_remove() { - I &image_ctx = this->m_image_ctx; - assert(image_ctx.owner_lock.is_locked()); - - { - RWLock::RLocker snap_locker(image_ctx.snap_lock); - if (image_ctx.object_map != nullptr) { - ldout(image_ctx.cct, 5) << this << " send_post_remove: " - << " delete_start=" << m_delete_start - << " num_objects=" << m_num_objects << dendl; - m_state = STATE_POST_REMOVE; + << " delete_start_min=" << m_delete_start_min + << " num_objects=" << m_num_objects << dendl; + m_state = STATE_POST_TRIM; assert(image_ctx.exclusive_lock->is_lock_owner()); - // flag the pending objects as removed RWLock::WLocker object_map_locker(image_ctx.object_map_lock); if (image_ctx.object_map->template aio_update >( - CEPH_NOSNAP, m_delete_start, m_num_objects, OBJECT_NONEXISTENT, + CEPH_NOSNAP, m_delete_start_min, m_num_objects, OBJECT_NONEXISTENT, OBJECT_PENDING, this)) { return; } } } - // no object map update required send_clean_boundary(); } diff --git a/src/librbd/operation/TrimRequest.h b/src/librbd/operation/TrimRequest.h index 814d180230978..363481344567e 100644 --- a/src/librbd/operation/TrimRequest.h +++ b/src/librbd/operation/TrimRequest.h @@ -33,46 +33,43 @@ protected: * * @verbatim * - * . . . . > STATE_FINISHED . . . . . . . . . - * | . . . . . . . . . . > . . . . . . . . . . - * | / . . - * STATE_PRE_COPYUP ---> STATE_COPYUP_OBJECTS . . - * | . . - * /-----------------------/ v . - * | . . - * v . . - * STATE_POST_COPYUP. . . > . . . - * | . . . . . . . . . . < . . . . . . . . . . - * | | . . - * v v v . - * STATE_PRE_REMOVE ---> STATE_REMOVE_OBJECTS . - * | . . . - * /-----------------------/ . . . . . . . . - * | . . . - * v v v v - * STATE_POST_REMOVE --> STATE_CLEAN_BOUNDARY ---> - * . ^ - * . . - * . . . . . . . . . . . . . . . . . . . . . . . - * - * @endverbatim + * . . . . . . . . . . . . . . . . . + * | . + * v (skip if not needed) . + * STATE_PRE_TRIM . + * | . + * v (skip if not needed) . + * STATE_COPYUP_OBJECTS . + * | . + * v (skip if not needed) . + * STATE_REMOVE_OBJECTS . + * | . + * v (skip if not needed) . + * STATE_POST_TRIM . + * | . + * v (skip if not needed) . + * STATE_CLEAN_BOUNDARY . + * | . + * v . + * STATE_FINISHED < . . . . . . . . . . . . . . . + * | + * v + * * * The _COPYUP_OBJECTS state is skipped if there is no parent overlap * within the new image size and the image does not have any snapshots. - * The _PRE_REMOVE/_POST_REMOVE states are skipped if the object map + * The _PRE_TRIM/_POST_TRIM states are skipped if the object map * isn't enabled. The _REMOVE_OBJECTS state is skipped if no whole objects * are removed. The _CLEAN_BOUNDARY state is skipped if no boundary * objects are cleaned. The state machine will immediately transition * to _FINISHED state if there are no bytes to trim. - */ + */ enum State { - STATE_PRE_COPYUP, + STATE_PRE_TRIM, STATE_COPYUP_OBJECTS, - STATE_POST_COPYUP, - STATE_PRE_REMOVE, STATE_REMOVE_OBJECTS, - STATE_POST_REMOVE, + STATE_POST_TRIM, STATE_CLEAN_BOUNDARY, STATE_FINISHED }; @@ -83,25 +80,20 @@ protected: private: uint64_t m_delete_start; + uint64_t m_delete_start_min = 0; uint64_t m_num_objects; uint64_t m_delete_off; uint64_t m_new_size; ProgressContext &m_prog_ctx; - uint64_t m_copyup_start; - uint64_t m_copyup_end; - TrimRequest(ImageCtxT &image_ctx, Context *on_finish, uint64_t original_size, uint64_t new_size, ProgressContext &prog_ctx); - void send_pre_copyup(); + void send_pre_trim(); void send_copyup_objects(); - void send_post_copyup(); - - void send_pre_remove(); void send_remove_objects(); - void send_post_remove(); + void send_post_trim(); void send_clean_boundary(); void send_finish(int r); -- 2.39.5