From a4e79ba67aa74b719025bd99bd6b4167c580c6dd Mon Sep 17 00:00:00 2001
From: Kefu Chai <kchai@redhat.com>
Date: Wed, 11 Nov 2020 19:14:06 +0800
Subject: [PATCH] crimson/osd: use with_head_obc() to replace
 get_or_load_head_obc

for better readability, and ensure that `lock.unlock()` is called when
an error is returned after the lock is acquired.

Signed-off-by: Kefu Chai <kchai@redhat.com>
---
 src/crimson/osd/object_context.h              |  2 ++
 src/crimson/osd/pg.cc                         |  3 ++
 src/crimson/osd/pg_recovery.cc                |  2 --
 .../osd/replicated_recovery_backend.cc        | 36 +++++++------------
 4 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/src/crimson/osd/object_context.h b/src/crimson/osd/object_context.h
index 72f271beb37..d2ee138ac4e 100644
--- a/src/crimson/osd/object_context.h
+++ b/src/crimson/osd/object_context.h
@@ -121,6 +121,8 @@ public:
       return seastar::with_lock(lock.for_read(), std::forward<Func>(func));
     case RWState::RWEXCL:
       return seastar::with_lock(lock.for_excl(), std::forward<Func>(func));
+    case RWState::RWNONE:
+      return seastar::futurize_invoke(func);
     default:
       assert(0 == "noop");
     }
diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index d21fbdc2c5d..e40ae947bb8 100644
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -835,6 +835,9 @@ PG::with_head_obc(hobject_t oid, with_obc_func_t&& func)
 template seastar::future<>
 PG::with_head_obc<RWState::RWREAD>(hobject_t, with_obc_func_t&&);
 
+template seastar::future<>
+PG::with_head_obc<RWState::RWNONE>(hobject_t, with_obc_func_t&&);
+
 PG::load_obc_ertr::future<
   std::pair<crimson::osd::ObjectContextRef, bool>>
 PG::get_or_load_head_obc(hobject_t oid)
diff --git a/src/crimson/osd/pg_recovery.cc b/src/crimson/osd/pg_recovery.cc
index f73bcec4609..14fc62565a4 100644
--- a/src/crimson/osd/pg_recovery.cc
+++ b/src/crimson/osd/pg_recovery.cc
@@ -323,8 +323,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;
-      // obc is loaded the excl lock
-      obc->put_lock_type(RWState::RWEXCL);
       ceph_assert_always(obc->get_recovery_read());
     }
     if (!pg->is_unreadable_object(soid)) {
diff --git a/src/crimson/osd/replicated_recovery_backend.cc b/src/crimson/osd/replicated_recovery_backend.cc
index 4f29568a1dc..edd2513e97a 100644
--- a/src/crimson/osd/replicated_recovery_backend.cc
+++ b/src/crimson/osd/replicated_recovery_backend.cc
@@ -641,29 +641,19 @@ seastar::future<bool> ReplicatedRecoveryBackend::_handle_pull_response(
   if (pi.recovery_info.version == eversion_t())
     pi.recovery_info.version = pop.version;
 
-  bool first = pi.recovery_progress.first;
-
-  return [this, &pi, first, &recovery_waiter, &pop] {
-    if (first) {
-      return pg.get_or_load_head_obc(pi.recovery_info.soid).safe_then(
-	[&pi, &recovery_waiter, &pop](auto p) {
-	auto& [obc, existed] = p;
-	pi.obc = obc;
-	recovery_waiter.obc = obc;
-	obc->obs.oi.decode(pop.attrset[OI_ATTR]);
-	pi.recovery_info.oi = obc->obs.oi;
-	return seastar::make_ready_future<>();
-      }, crimson::osd::PG::load_obc_ertr::all_same_way(
-	  [this, &pi](const std::error_code& e) {
-	  auto [obc, existed] = shard_services.obc_registry.get_cached_obc(
-				    pi.recovery_info.soid);
-	  pi.obc = obc;
-	  return seastar::make_ready_future<>();
-	})
-      );
-    }
-    return seastar::make_ready_future<>();
-  }().then([this, first, &pi, &pop, t, response]() mutable {
+  auto prepare_waiter = seastar::make_ready_future<>();
+  if (pi.recovery_progress.first) {
+    prepare_waiter = pg.with_head_obc<RWState::RWNONE>(
+      pi.recovery_info.soid, [&pi, &recovery_waiter, &pop](auto obc) {
+        pi.obc = obc;
+        recovery_waiter.obc = obc;
+        obc->obs.oi.decode(pop.attrset[OI_ATTR]);
+        pi.recovery_info.oi = obc->obs.oi;
+        return seastar::make_ready_future<>();
+      });
+  };
+  return prepare_waiter.then([this, first=pi.recovery_progress.first,
+			      &pi, &pop, t, response]() mutable {
     return seastar::do_with(interval_set<uint64_t>(),
 			    bufferlist(),
 			    interval_set<uint64_t>(),
-- 
2.39.5