ReplicatedPG: use register_on_finish rather than on_finish for flush/evict

Signed-off-by: Samuel Just <sjust@redhat.com>

diff --git a/src/osd/ReplicatedPG.cc b/src/osd/ReplicatedPG.cc
index 74efe97dd3555d1e232ffb784c71705da264f102..c9a5ada999218667bb2cade3a3138370228d293a 100644 (file)
--- a/src/osd/ReplicatedPG.cc
+++ b/src/osd/ReplicatedPG.cc
@@ -4459,7 +4459,7 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
          break;
        }
        if (oi.is_dirty()) {
-         result = start_flush(ctx->op, ctx->obc, false, NULL, NULL);
+         result = start_flush(ctx->op, ctx->obc, false, NULL, boost::none);
          if (result == -EINPROGRESS)
            result = -EAGAIN;
        } else {
@@ -4492,7 +4492,7 @@ int ReplicatedPG::do_osd_ops(OpContext *ctx, vector<OSDOp>& ops)
        }
        hobject_t missing;
        if (oi.is_dirty()) {
-         result = start_flush(ctx->op, ctx->obc, true, &missing, NULL);
+         result = start_flush(ctx->op, ctx->obc, true, &missing, boost::none);
          if (result == -EINPROGRESS)
            result = -EAGAIN;
        } else {
@@ -7760,7 +7760,7 @@ struct C_Flush : public Context {
 int ReplicatedPG::start_flush(
   OpRequestRef op, ObjectContextRef obc,
   bool blocking, hobject_t *pmissing,
-  Context *on_flush)
+  boost::optional<std::function<void()>> &&on_flush)
 {
   const object_info_t& oi = obc->obs.oi;
   const hobject_t& soid = oi.soid;
@@ -7922,7 +7922,7 @@ int ReplicatedPG::start_flush(
   fop->obc = obc;
   fop->flushed_version = oi.user_version;
   fop->blocking = blocking;
-  fop->on_flush = on_flush;
+  fop->on_flush = std::move(on_flush);
   fop->op = op;
 
   ObjectOperation o;
@@ -7991,9 +7991,8 @@ void ReplicatedPG::finish_flush(hobject_t oid, ceph_tid_t tid, int r)
       requeue_ops(fop->dup_ops);
     }
     if (fop->on_flush) {
-      Context *on_flush = fop->on_flush;
-      fop->on_flush = NULL;
-      on_flush->complete(-EBUSY);
+      (*(fop->on_flush))();
+      fop->on_flush = boost::none;
     }
     flush_ops.erase(oid);
     return;
@@ -8029,9 +8028,8 @@ int ReplicatedPG::try_flush_mark_clean(FlushOpRef fop)
       requeue_ops(fop->dup_ops);
     }
     if (fop->on_flush) {
-      Context *on_flush = fop->on_flush;
-      fop->on_flush = NULL;
-      on_flush->complete(-EBUSY);
+      (*(fop->on_flush))();
+      fop->on_flush = boost::none;
     }
     flush_ops.erase(oid);
     if (fop->blocking)
@@ -8060,9 +8058,8 @@ int ReplicatedPG::try_flush_mark_clean(FlushOpRef fop)
       agent_maybe_evict(obc, true)) {
     osd->logger->inc(l_osd_tier_clean);
     if (fop->on_flush) {
-      Context *on_flush = fop->on_flush;
-      fop->on_flush = NULL;
-      on_flush->complete(0);
+      (*(fop->on_flush))();
+      fop->on_flush = boost::none;
     }
     flush_ops.erase(oid);
     return 0;
@@ -8088,8 +8085,8 @@ int ReplicatedPG::try_flush_mark_clean(FlushOpRef fop)
   dout(10) << __func__ << " clearing DIRTY flag for " << oid << dendl;
   OpContextUPtr ctx = simple_opc_create(fop->obc);
 
-  ctx->on_finish = fop->on_flush;
-  fop->on_flush = NULL;
+  ctx->register_on_finish(*(fop->on_flush));
+  fop->on_flush = boost::none;
 
   ctx->lock_to_release = OpContext::W_LOCK;  // we took it above
   ctx->at_version = get_next_version();
@@ -8141,9 +8138,8 @@ void ReplicatedPG::cancel_flush(FlushOpRef fop, bool requeue)
     requeue_ops(fop->dup_ops);
   }
   if (fop->on_flush) {
-    Context *on_flush = fop->on_flush;
-    fop->on_flush = NULL;
-    on_flush->complete(-ECANCELED);
+    (*(fop->on_flush))();
+    fop->on_flush = boost::none;
   }
   flush_ops.erase(fop->obc->obs.oi.soid);
 }
@@ -11743,17 +11739,6 @@ void ReplicatedPG::agent_load_hit_sets()
   }
 }
 
-struct C_AgentFlushStartStop : public Context {
-  ReplicatedPGRef pg;
-  hobject_t oid;
-  C_AgentFlushStartStop(ReplicatedPG *p, hobject_t o) : pg(p), oid(o) {
-    pg->osd->agent_start_op(oid);
-  }
-  void finish(int r) {
-    pg->osd->agent_finish_op(oid);
-  }
-};
-
 bool ReplicatedPG::agent_maybe_flush(ObjectContextRef& obc)
 {
   if (!obc->obs.oi.is_dirty()) {
@@ -11795,12 +11780,18 @@ bool ReplicatedPG::agent_maybe_flush(ObjectContextRef& obc)
   // FIXME: flush anything dirty, regardless of what distribution of
   // ages we expect.
 
-  Context *on_flush = new C_AgentFlushStartStop(this, obc->obs.oi.soid);
+  hobject_t oid = obc->obs.oi.soid;
+  osd->agent_start_op(oid);
+  // no need to capture a pg ref, can't outlive fop or ctx
+  std::function<void()> on_flush = [this, oid]() {
+    osd->agent_finish_op(oid);
+  };
+
   int result = start_flush(
     OpRequestRef(), obc, false, NULL,
     on_flush);
   if (result != -EINPROGRESS) {
-    on_flush->complete(result);
+    on_flush();
     dout(10) << __func__ << " start_flush() failed " << obc->obs.oi
       << " with " << result << dendl;
     osd->logger->inc(l_osd_agent_skip);
@@ -11811,16 +11802,6 @@ bool ReplicatedPG::agent_maybe_flush(ObjectContextRef& obc)
   return true;
 }
 
-struct C_AgentEvictStartStop : public Context {
-  ReplicatedPGRef pg;
-  explicit C_AgentEvictStartStop(ReplicatedPG *p) : pg(p) {
-    pg->osd->agent_start_evict_op();
-  }
-  void finish(int r) {
-    pg->osd->agent_finish_evict_op();
-  }
-};
-
 bool ReplicatedPG::agent_maybe_evict(ObjectContextRef& obc, bool after_flush)
 {
   const hobject_t& soid = obc->obs.oi.soid;
@@ -11896,8 +11877,13 @@ bool ReplicatedPG::agent_maybe_evict(ObjectContextRef& obc, bool after_flush)
 
   dout(10) << __func__ << " evicting " << obc->obs.oi << dendl;
   OpContextUPtr ctx = simple_opc_create(obc);
-  Context *on_evict = new C_AgentEvictStartStop(this);
-  ctx->on_finish = on_evict;
+
+  osd->agent_start_evict_op();
+  ctx->register_on_finish(
+    [this]() {
+      osd->agent_finish_evict_op();
+    });
+
   ctx->lock_to_release = OpContext::W_LOCK;
   ctx->at_version = get_next_version();
   assert(ctx->new_obs.exists);

diff --git a/src/osd/ReplicatedPG.h b/src/osd/ReplicatedPG.h
index f4560a6521089b22e8cfb4df0a8e55b4dc388c13..b131f66fbc55cdff9fb9ea2b3a40dc405846eb8e 100644 (file)
--- a/src/osd/ReplicatedPG.h
+++ b/src/osd/ReplicatedPG.h
@@ -247,12 +247,11 @@ public:
     int rval;                   ///< copy-from result
     bool blocking;              ///< whether we are blocking updates
     bool removal;               ///< we are removing the backend object
-    Context *on_flush;          ///< callback, may be null
+    boost::optional<std::function<void()>> on_flush; ///< callback, may be null
 
     FlushOp()
       : flushed_version(0), objecter_tid(0), rval(0),
-       blocking(false), removal(false),
-       on_flush(NULL) {}
+       blocking(false), removal(false) {}
     ~FlushOp() { assert(!on_flush); }
   };
   typedef boost::shared_ptr<FlushOp> FlushOpRef;
@@ -1432,7 +1431,7 @@ protected:
   int start_flush(
     OpRequestRef op, ObjectContextRef obc,
     bool blocking, hobject_t *pmissing,
-    Context *on_flush);
+    boost::optional<std::function<void()>> &&on_flush);
   void finish_flush(hobject_t oid, ceph_tid_t tid, int r);
   int try_flush_mark_clean(FlushOpRef fop);
   void cancel_flush(FlushOpRef fop, bool requeue);