f63d76a2 modified the lock() variants on tri_mutex so that the obc
loading pathway wouldn't invoke .then() on returned future known
statically to be ready. Now that the loading pathway uses demotion
mechanisms that cannot block and do not return futures, we no longer
have any users like that and can drop the extra std::nullopt
possibility.
In a larger sense, if lock() *can* return a non-ready future in a
particular pathway, there's no semantic difference between returning
std::optional<future<>> and future<> as the caller would still have to
deal with a possible non-ready future return even if std::nullopt is
also possible. If the pathway can be demonstrated statically to be
non-blocking, as with the obc loading mechanism, we really want to use a
mechanism that obviously cannot block rather relying on a mechanism with
a return signature of std::optional<future<>> to return std::nullopt.
Signed-off-by: Samuel Just <sjust@redhat.com>
SET_SUBSYS(osd);
//TODO: SET_SUBSYS(crimson_tri_mutex);
-std::optional<seastar::future<>>
-read_lock::lock()
+seastar::future<> read_lock::lock()
{
return static_cast<tri_mutex*>(this)->lock_for_read();
}
static_cast<tri_mutex*>(this)->unlock_for_read();
}
-std::optional<seastar::future<>>
-write_lock::lock()
+seastar::future<> write_lock::lock()
{
return static_cast<tri_mutex*>(this)->lock_for_write();
}
static_cast<tri_mutex*>(this)->unlock_for_write();
}
-std::optional<seastar::future<>>
-excl_lock::lock()
+seastar::future<> excl_lock::lock()
{
return static_cast<tri_mutex*>(this)->lock_for_excl();
}
assert(!is_acquired());
}
-std::optional<seastar::future<>>
-tri_mutex::lock_for_read()
+seastar::future<> tri_mutex::lock_for_read()
{
LOG_PREFIX(tri_mutex::lock_for_read());
DEBUGDPP("", *this);
if (try_lock_for_read()) {
DEBUGDPP("lock_for_read successfully", *this);
- return std::nullopt;
+ return seastar::now();
}
DEBUGDPP("can't lock_for_read, adding to waiters", *this);
waiters.emplace_back(seastar::promise<>(), type_t::read, name);
++readers;
}
-std::optional<seastar::future<>>
-tri_mutex::lock_for_write()
+seastar::future<> tri_mutex::lock_for_write()
{
LOG_PREFIX(tri_mutex::lock_for_write());
DEBUGDPP("", *this);
if (try_lock_for_write()) {
DEBUGDPP("lock_for_write successfully", *this);
- return std::nullopt;
+ return seastar::now();
}
DEBUGDPP("can't lock_for_write, adding to waiters", *this);
waiters.emplace_back(seastar::promise<>(), type_t::write, name);
}
// for exclusive users
-std::optional<seastar::future<>>
-tri_mutex::lock_for_excl()
+seastar::future<> tri_mutex::lock_for_excl()
{
LOG_PREFIX(tri_mutex::lock_for_excl());
DEBUGDPP("", *this);
if (try_lock_for_excl()) {
DEBUGDPP("lock_for_excl, successfully", *this);
- return std::nullopt;
+ return seastar::now();
}
DEBUGDPP("can't lock_for_excl, adding to waiters", *this);
waiters.emplace_back(seastar::promise<>(), type_t::exclusive, name);
#pragma once
-#include <optional>
-
#include <seastar/core/future.hh>
#include <seastar/core/circular_buffer.hh>
#include "crimson/common/log.h"
class read_lock {
public:
- std::optional<seastar::future<>> lock();
+ seastar::future<> lock();
void unlock();
};
class write_lock {
public:
- std::optional<seastar::future<>> lock();
+ seastar::future<> lock();
void unlock();
};
class excl_lock {
public:
- std::optional<seastar::future<>> lock();
+ seastar::future<> lock();
void unlock();
};
}
// for shared readers
- std::optional<seastar::future<>> lock_for_read();
+ seastar::future<> lock_for_read();
bool try_lock_for_read() noexcept;
void unlock_for_read();
void demote_to_read();
}
// for shared writers
- std::optional<seastar::future<>> lock_for_write();
+ seastar::future<> lock_for_write();
bool try_lock_for_write() noexcept;
void unlock_for_write();
void demote_to_write();
}
// for exclusive users
- std::optional<seastar::future<>> lock_for_excl();
+ seastar::future<> lock_for_excl();
bool try_lock_for_excl() noexcept;
void unlock_for_excl();
bool is_excl_acquired() const {
private:
template <typename Lock, typename Func>
auto _with_lock(Lock& lock, Func&& func) {
- auto maybe_fut = lock.lock();
- return seastar::futurize_invoke([
- maybe_fut=std::move(maybe_fut),
- func=std::forward<Func>(func),
- obc=Ref(this),
- &lock]() mutable {
- if (maybe_fut) {
- return std::move(*maybe_fut
- ).then([func=std::forward<Func>(func), obc, &lock]() mutable {
- return seastar::futurize_invoke(
- func
- ).finally([&lock, obc] {
- /* We chain the finally block here because it's possible for
- * *maybe_fut from lock.lock() above to fail due to a call to
- * ObjectContext::interrupt, which calls tri_mutex::abort.
- * In the event of such an error, the lock isn't actually taken
- * and calling unlock() would be incorrect. */
- lock.unlock();
- });
- });
- } else {
- // atomically calling func upon locking
- return seastar::futurize_invoke(
- func
- ).finally([&lock, obc] {
- lock.unlock();
- });
- }
+ return lock.lock(
+ ).then([&lock, func=std::forward<Func>(func), obc=Ref(this)]() mutable {
+ return seastar::futurize_invoke(
+ func
+ ).finally([&lock, obc=std::move(obc)] {
+ /* We chain the finally block here because it's possible for lock.lock()
+ * above to fail due to a call to ObjectContext::interrupt, which calls
+ * tri_mutex::abort. In the event of such an error, the lock isn't
+ * actually taken and calling unlock() would be incorrect. */
+ lock.unlock();
+ });
});
}