From: chunmei <chunmei.liu@intel.com>
Date: Fri, 23 Jun 2023 04:25:53 +0000 (+0000)
Subject: crimson/osd: remove seastar::now() to gurantee sequence in pipline
X-Git-Tag: v19.0.0~950^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=c7628de4d230cd08d536d7e0c35bf6822cc7146e;p=ceph.git

crimson/osd: remove seastar::now() to gurantee sequence in pipline

in debug build, if wait_barrier is seastar::now, will cause reactor
schedule the following continuation away, but can't schedule them back
in the same order as by which they are scheduled away, so the continuations
are out of order here.

Signed-off-by: chunmei <chunmei.liu@intel.com>
---

diff --git a/src/crimson/common/operation.h b/src/crimson/common/operation.h
index f26a3e860bc2..6df2c99fd2a0 100644
--- a/src/crimson/common/operation.h
+++ b/src/crimson/common/operation.h
@@ -476,7 +476,7 @@ public:
   using Ref = std::unique_ptr<PipelineExitBarrierI>;
 
   /// Waits for exit barrier
-  virtual seastar::future<> wait() = 0;
+  virtual std::optional<seastar::future<>> wait() = 0;
 
   /// Releases pipeline stage, can only be called after wait
   virtual void exit() = 0;
@@ -503,8 +503,8 @@ public:
 class PipelineHandle {
   PipelineExitBarrierI::Ref barrier;
 
-  auto wait_barrier() {
-    return barrier ? barrier->wait() : seastar::now();
+  std::optional<seastar::future<>> wait_barrier() {
+    return barrier ? barrier->wait() : std::nullopt;
   }
 
 public:
@@ -525,15 +525,26 @@ public:
   seastar::future<>
   enter(T &stage, typename T::BlockingEvent::template Trigger<OpT>&& t) {
     ceph_assert(stage.get_core() == seastar::this_shard_id());
-    return wait_barrier().then([this, &stage, t=std::move(t)] () mutable {
-      auto fut = t.maybe_record_blocking(stage.enter(t), stage);
-      exit();
-      return std::move(fut).then(
-        [this, t=std::move(t)](auto &&barrier_ref) mutable {
-        barrier = std::move(barrier_ref);
-        return seastar::now();
+    auto wait_fut = wait_barrier();
+    if (wait_fut.has_value()) {
+      return wait_fut.value().then([this, &stage, t=std::move(t)] () mutable {
+        auto fut = t.maybe_record_blocking(stage.enter(t), stage);
+        exit();
+        return std::move(fut).then(
+          [this, t=std::move(t)](auto &&barrier_ref) mutable {
+          barrier = std::move(barrier_ref);
+          return seastar::now();
+        });
       });
-    });
+    } else {
+        auto fut = t.maybe_record_blocking(stage.enter(t), stage);
+        exit();
+        return std::move(fut).then(
+          [this, t=std::move(t)](auto &&barrier_ref) mutable {
+          barrier = std::move(barrier_ref);
+          return seastar::now();
+        });
+    }
   }
 
   /**
@@ -542,7 +553,7 @@ public:
   seastar::future<> complete() {
     auto ret = wait_barrier();
     barrier.reset();
-    return ret;
+    return ret ? std::move(ret.value()) : seastar::now();
   }
 
   /**
@@ -578,8 +589,8 @@ class OrderedExclusivePhaseT : public PipelineStageIT<T> {
     ExitBarrier(OrderedExclusivePhaseT *phase, Operation::id_t id)
       : phase(phase), op_id(id) {}
 
-    seastar::future<> wait() final {
-      return seastar::now();
+    std::optional<seastar::future<>> wait() final {
+      return std::nullopt;
     }
 
     void exit() final {
@@ -681,7 +692,7 @@ private:
       seastar::future<> &&barrier,
       TriggerT& trigger) : phase(phase), barrier(std::move(barrier)), trigger(trigger) {}
 
-    seastar::future<> wait() final {
+    std::optional<seastar::future<>> wait() final {
       assert(phase);
       assert(barrier);
       auto ret = std::move(*barrier);
@@ -739,8 +750,8 @@ class UnorderedStageT : public PipelineStageIT<T> {
   public:
     ExitBarrier() = default;
 
-    seastar::future<> wait() final {
-      return seastar::now();
+    std::optional<seastar::future<>> wait() final {
+      return std::nullopt;
     }
 
     void exit() final {}