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 <dillaman@redhat.com>
(cherry picked from commit 72ce4576fa2b562799a5bc78bd423cfabe097d67)

Conflicts:
	src/librbd/operation/TrimRequest.cc: trivial resolution
	src/librbd/operation/TrimRequest.h: trivial resolution

diff --git a/src/librbd/operation/TrimRequest.cc b/src/librbd/operation/TrimRequest.cc
index 5e3500779df5f024b56227cd72bbf74284ca257f..a2d07e1cd173bdd702b0bc9c8b96a132e3322bb2 100644 (file)
--- a/src/librbd/operation/TrimRequest.cc
+++ b/src/librbd/operation/TrimRequest.cc
@@ -105,6 +105,7 @@ TrimRequest<I>::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<I>::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<I>::should_complete(int r)
 
 template <typename I>
 void TrimRequest<I>::send() {
-  send_pre_copyup();
+  send_pre_trim();
 }
 
 template<typename I>
-void TrimRequest<I>::send_copyup_objects() {
+void TrimRequest<I>::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<I>::ContextFactory context_factory(
-    boost::lambda::bind(boost::lambda::new_ptr<C_CopyupObject<I> >(),
-      boost::lambda::_1, &image_ctx, snapc, boost::lambda::_2));
-  AsyncObjectThrottle<I> *throttle = new AsyncObjectThrottle<I>(
-    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 <typename I>
-void TrimRequest<I>::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<AsyncRequest<I> >(
+            CEPH_NOSNAP, m_delete_start_min, m_num_objects, OBJECT_PENDING,
+            OBJECT_EXISTS, this)) {
+        return;
+      }
+    }
+  }
 
-  Context *ctx = this->create_callback_context();
-  typename AsyncObjectThrottle<I>::ContextFactory context_factory(
-    boost::lambda::bind(boost::lambda::new_ptr<C_RemoveObject<I> >(),
-      boost::lambda::_1, &image_ctx, boost::lambda::_2));
-  AsyncObjectThrottle<I> *throttle = new AsyncObjectThrottle<I>(
-    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<typename I>
-void TrimRequest<I>::send_pre_copyup() {
+void TrimRequest<I>::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<AsyncRequest<I> >(
-            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<I>::ContextFactory context_factory(
+    boost::lambda::bind(boost::lambda::new_ptr<C_CopyupObject<I> >(),
+      boost::lambda::_1, &image_ctx, snapc, boost::lambda::_2));
+  AsyncObjectThrottle<I> *throttle = new AsyncObjectThrottle<I>(
+    this, image_ctx, context_factory, ctx, &m_prog_ctx, copyup_start,
+    copyup_end);
+  throttle->start_ops(image_ctx.concurrent_management_ops);
 }
 
 template <typename I>
-void TrimRequest<I>::send_pre_remove() {
+void TrimRequest<I>::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<AsyncRequest<I> >(
-            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<I>::ContextFactory context_factory(
+    boost::lambda::bind(boost::lambda::new_ptr<C_RemoveObject<I> >(),
+      boost::lambda::_1, &image_ctx, boost::lambda::_2));
+  AsyncObjectThrottle<I> *throttle = new AsyncObjectThrottle<I>(
+    this, image_ctx, context_factory, ctx, &m_prog_ctx, m_delete_start,
+    m_num_objects);
+  throttle->start_ops(image_ctx.concurrent_management_ops);
 }
 
 template<typename I>
-void TrimRequest<I>::send_post_copyup() {
+void TrimRequest<I>::send_post_trim() {
   I &image_ctx = this->m_image_ctx;
   assert(image_ctx.owner_lock.is_locked());
 
@@ -328,50 +282,21 @@ void TrimRequest<I>::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<AsyncRequest<I> >(
-            CEPH_NOSNAP, m_copyup_start, m_copyup_end, OBJECT_NONEXISTENT,
-            OBJECT_PENDING, this)) {
-        return;
-      }
-    }
-  }
-
-  send_pre_remove();
-}
-
-template <typename I>
-void TrimRequest<I>::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<AsyncRequest<I> >(
-            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 814d1802309787139528c2a454cecb5080dbab8b..363481344567ec58c6788fa50087ac2b6631d03c 100644 (file)
--- a/src/librbd/operation/TrimRequest.h
+++ b/src/librbd/operation/TrimRequest.h
@@ -33,46 +33,43 @@ protected:
    *
    * @verbatim
    *
-   *     <start> . . . . > 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 ---> <finish>
-   *        .                                           ^
-   *        .                                           .
-   *        . . . . . . . . . . . . . . . . . . . . . . .
-   *
-   * @endverbatim
+   *     <start>  . . . . . . . . . . . . . . . . .
+   *        |                                     .
+   *        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
+   *    <finish>
    *
    * 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);