From: Xuehan Xu <xxhdx1985126@gmail.com>
Date: Fri, 18 Dec 2020 04:20:48 +0000 (+0800)
Subject: crimson/osd: don't get recovery read lock in PGRecvery::on_local_recover()
X-Git-Tag: v16.1.0~200^2~1
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=98d8d25b3990a70df58348b152b4531706f030c5;p=ceph.git

crimson/osd: don't get recovery read lock in PGRecvery::on_local_recover()

Now we've changed drop_recovery_read to only care about the recovery_read flag, so
we shouldn't get read lock when acquiring recovery read, otherwise there would be
a chance in which the read lock can't get released

Signed-off-by: Xuehan Xu <xxhdx1985126@gmail.com>
---

diff --git a/src/crimson/osd/pg_recovery.cc b/src/crimson/osd/pg_recovery.cc
index 0eeeee2b82d..c0ce0088b23 100644
--- a/src/crimson/osd/pg_recovery.cc
+++ b/src/crimson/osd/pg_recovery.cc
@@ -318,7 +318,6 @@ void PGRecovery::on_local_recover(
       auto& obc = pg->get_recovery_backend()->get_recovering(soid).obc; //TODO: move to pg backend?
       obc->obs.exists = true;
       obc->obs.oi = recovery_info.oi;
-      ceph_assert_always(obc->get_recovery_read());
     }
     if (!pg->is_unreadable_object(soid)) {
       pg->get_recovery_backend()->get_recovering(soid).set_readable();
diff --git a/src/crimson/osd/replicated_recovery_backend.cc b/src/crimson/osd/replicated_recovery_backend.cc
index d75e5c6ab1b..40cd06fc79b 100644
--- a/src/crimson/osd/replicated_recovery_backend.cc
+++ b/src/crimson/osd/replicated_recovery_backend.cc
@@ -27,24 +27,20 @@ seastar::future<> ReplicatedRecoveryBackend::recover_object(
   assert(is_recovering(soid));
   // start tracking the recovery of soid
   return maybe_pull_missing_obj(soid, need).then([this, soid, need] {
-    auto& recovery_waiter = recovering.at(soid);
-    if (recovery_waiter.obc) {
+    logger().debug("recover_object: loading obc: {}", soid);
+    return pg.with_head_obc<RWState::RWREAD>(soid,
+      [this, soid, need](auto obc) {
+      logger().debug("recover_object: loaded obc: {}", obc->obs.oi.soid);
+      auto& recovery_waiter = recovering.at(soid);
+      recovery_waiter.obc = obc;
+      recovery_waiter.obc->wait_recovery_read();
       return maybe_push_shards(soid, need);
-    } else {
-      logger().debug("recover_object: loading obc: {}", soid);
-      return pg.with_head_obc<RWState::RWREAD>(soid,
-        [this, soid, need, &recovery_waiter](auto obc) {
-        logger().debug("recover_object: loaded obc: {}", obc->obs.oi.soid);
-        recovery_waiter.obc = obc;
-        recovery_waiter.obc->wait_recovery_read();
-        return maybe_push_shards(soid, need);
-      }).handle_error(
-        crimson::osd::PG::load_obc_ertr::all_same_way([soid](auto& code) {
-        // TODO: may need eio handling?
-        logger().error("recover_object saw error code {}, ignoring object {}",
-                       code, soid);
-      }));
-    }
+    }).handle_error(
+      crimson::osd::PG::load_obc_ertr::all_same_way([soid](auto& code) {
+      // TODO: may need eio handling?
+      logger().error("recover_object saw error code {}, ignoring object {}",
+                     code, soid);
+    }));
   });
 }