From c7628de4d230cd08d536d7e0c35bf6822cc7146e Mon Sep 17 00:00:00 2001
From: chunmei <chunmei.liu@intel.com>
Date: Fri, 23 Jun 2023 04:25:53 +0000
Subject: [PATCH] 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>
---
 src/crimson/common/operation.h | 45 +++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 17 deletions(-)

diff --git a/src/crimson/common/operation.h b/src/crimson/common/operation.h
index f26a3e860bc..6df2c99fd2a 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 {}
-- 
2.39.5