crimson/.../object_context_loader: simplify obc loading

Because we just constructed the obc, we know that we can get an
exclusive lock without blocking.  Introduce
ObjectContext::load_then_with_lock to take an exclusive lock
unconditionally, load, downgrade (which we also know must be safe), and
then run the passed function.

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

diff --git a/src/crimson/osd/object_context.h b/src/crimson/osd/object_context.h
index b1a1e893827678ed82700d0dffc520be33e52849..ee2edaefe1dcbba909b9352f85b1a2d6b88fb361 100644 (file)
--- a/src/crimson/osd/object_context.h
+++ b/src/crimson/osd/object_context.h
@@ -269,6 +269,85 @@ public:
     }
   }
 
+  /**
+   * load_then_with_lock
+   *
+   * Takes two functions as arguments -- load_func to be invoked
+   * with an exclusive lock, and func to be invoked under the
+   * lock type specified by the Type template argument.
+   *
+   * Caller must ensure that *this is not already locked, presumably
+   * by invoking load_then_with_lock immediately after construction.
+   *
+   * @param [in] load_func Function to be invoked under excl lock
+   * @param [in] func Function to be invoked after load_func under
+   *             lock of type Type.
+   */
+  template<RWState::State Type, typename Func, typename Func2>
+  auto load_then_with_lock(Func &&load_func, Func2 &&func) {
+    class lock_state_t {
+      tri_mutex *lock = nullptr;
+      bool excl = false;
+
+    public:
+      lock_state_t(tri_mutex &lock) : lock(&lock), excl(true) {
+       ceph_assert(lock.try_lock_for_excl());
+      }
+      lock_state_t(lock_state_t &&o) : lock(o.lock), excl(o.excl) {
+       o.lock = nullptr;
+       o.excl = false;
+      }
+      lock_state_t() = delete;
+      lock_state_t &operator=(lock_state_t &&o) = delete;
+      lock_state_t(const lock_state_t &o) = delete;
+      lock_state_t &operator=(const lock_state_t &o) = delete;
+
+      void demote() {
+       ceph_assert(excl);
+       ceph_assert(lock);
+       if constexpr (Type == RWState::RWWRITE) {
+         lock->demote_to_write();
+       } else if constexpr (Type == RWState::RWREAD) {
+         lock->demote_to_read();
+       } else if constexpr (Type == RWState::RWNONE) {
+         lock->unlock_for_excl();
+       }
+       excl = false;
+      }
+
+      ~lock_state_t() {
+       if (!lock)
+         return;
+
+       if constexpr (Type == RWState::RWEXCL) {
+         lock->unlock_for_excl();
+       } else {
+         if (excl) {
+           lock->unlock_for_excl();
+           return;
+         }
+
+         if constexpr (Type == RWState::RWWRITE) {
+           lock->unlock_for_write();
+         } else if constexpr (Type == RWState::RWREAD) {
+           lock->unlock_for_read();
+         }
+       }
+      }
+    };
+
+    return seastar::do_with(
+      lock_state_t{lock},
+      [load_func=std::move(load_func), func=std::move(func)](auto &ls) mutable {
+       return std::invoke(
+         std::move(load_func)
+       ).si_then([func=std::move(func), &ls]() mutable {
+         ls.demote();
+         return std::invoke(std::move(func));
+       });
+      });
+  }
+
   bool empty() const {
     return !lock.is_acquired();
   }

diff --git a/src/crimson/osd/object_context_loader.cc b/src/crimson/osd/object_context_loader.cc
index 03bc1e1e349f03c8548d8ddea61f7eec761afe31..d099f41e15818fb292ba43dceffc6edf5ef54335 100644 (file)
--- a/src/crimson/osd/object_context_loader.cc
+++ b/src/crimson/osd/object_context_loader.cc
@@ -105,28 +105,43 @@ using crimson::common::local_conf;
     if constexpr (track) {
       obc->append_to(obc_set_accessing);
     }
-    return obc->with_lock<State, IOInterruptCondition>(
-      [existed=existed, obc=obc, func=std::move(func), this]() mutable {
-      return get_or_load_obc<State>(
-       obc, existed
-      ).safe_then_interruptible(std::move(func));
-    }).finally([FNAME, this, obc=std::move(obc)] {
-      DEBUGDPP("released object {}", dpp, obc->get_oid());
-      if constexpr (track) {
-       obc->remove_from(obc_set_accessing);
-      }
-    });
+    if (existed) {
+      return obc->with_lock<State, IOInterruptCondition>(
+       [func=std::move(func), obc=ObjectContextRef(obc)] {
+         return std::invoke(std::move(func), obc);
+       }
+      ).finally([FNAME, this, obc=ObjectContextRef(obc)] {
+       DEBUGDPP("released object {}", dpp, obc->get_oid());
+       if constexpr (track) {
+         obc->remove_from(obc_set_accessing);
+       }
+      });
+    } else {
+      return obc->load_then_with_lock<State> (
+       [this, obc=ObjectContextRef(obc)] {
+         return load_obc(obc);
+       },
+       [func=std::move(func), obc=ObjectContextRef(obc)] {
+         return std::invoke(std::move(func), obc);
+       }
+      ).finally([FNAME, this, obc=ObjectContextRef(obc)] {
+       DEBUGDPP("released object {}", dpp, obc->get_oid());
+       if constexpr (track) {
+         obc->remove_from(obc_set_accessing);
+       }
+      });
+    }
   }
 
 
-  ObjectContextLoader::load_obc_iertr::future<ObjectContextRef>
+  ObjectContextLoader::load_obc_iertr::future<>
   ObjectContextLoader::load_obc(ObjectContextRef obc)
   {
     LOG_PREFIX(ObjectContextLoader::load_obc);
     return backend.load_metadata(obc->get_oid())
     .safe_then_interruptible(
       [FNAME, this, obc=std::move(obc)](auto md)
-      -> load_obc_ertr::future<ObjectContextRef> {
+      -> load_obc_ertr::future<> {
       const hobject_t& oid = md->os.oi.soid;
       DEBUGDPP("loaded obs {} for {}", dpp, md->os.oi, oid);
       if (oid.is_head()) {
@@ -142,8 +157,8 @@ using crimson::common::local_conf;
         // See set_clone_ssc
         obc->set_clone_state(std::move(md->os));
       }
-      DEBUGDPP("returning obc {} for {}", dpp, obc->obs.oi, obc->obs.oi.soid);
-      return load_obc_ertr::make_ready_future<ObjectContextRef>(obc);
+      DEBUGDPP("loaded obc {} for {}", dpp, obc->obs.oi, obc->obs.oi.soid);
+      return seastar::now();
     });
   }
 

diff --git a/src/crimson/osd/object_context_loader.h b/src/crimson/osd/object_context_loader.h
index 0d7d7fdd935951447ea94be238f324fda98d4909..db27e81a3e48e00c5a89bd50522047896a32d202 100644 (file)
--- a/src/crimson/osd/object_context_loader.h
+++ b/src/crimson/osd/object_context_loader.h
@@ -96,7 +96,6 @@ private:
     return load_obc_iertr::make_ready_future<ObjectContextRef>(obc);
   }
 
-  load_obc_iertr::future<ObjectContextRef>
-  load_obc(ObjectContextRef obc);
+  load_obc_iertr::future<> load_obc(ObjectContextRef obc);
 };
 }