From f6e65edf9b493e8f778d2531397ceddc32efb098 Mon Sep 17 00:00:00 2001 From: Ilya Dryomov Date: Mon, 15 Jul 2024 11:39:11 +0200 Subject: [PATCH] librbd: get rid of AIO_STATE_CALLBACK in AioCompletion After commit 002afa0fe375 ("librbd: avoid using lock within AIO completion where possible"), the only method whose behavior would change if AIO_STATE_CALLBACK is removed is is_complete() and it actually needs fixing anyway: because of state != AIO_STATE_PENDING test, is_complete() returns true both for AIO_STATE_CALLBACK and AIO_STATE_COMPLETE, while wait_for_complete() still blocks on AIO_STATE_CALLBACK and returns only on AIO_STATE_COMPLETE. These methods back public APIs, so this inconsistency is exposed to users. If we move to setting state to AIO_STATE_COMPLETE at the top of mark_complete_and_notify() (i.e. before event socket notification), the transient state for callbacks can be eliminated entirely and the inconsistency goes away. Signed-off-by: Ilya Dryomov --- src/librbd/io/AioCompletion.cc | 7 +++---- src/librbd/io/AioCompletion.h | 1 - 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/librbd/io/AioCompletion.cc b/src/librbd/io/AioCompletion.cc index 7164d98c2594e..5452edf5fd37d 100644 --- a/src/librbd/io/AioCompletion.cc +++ b/src/librbd/io/AioCompletion.cc @@ -97,7 +97,6 @@ void AioCompletion::complete() { } } - state = AIO_STATE_CALLBACK; if (complete_cb) { if (external_callback) { complete_external_callback(); @@ -238,7 +237,7 @@ void AioCompletion::complete_request(ssize_t r) bool AioCompletion::is_complete() { tracepoint(librbd, aio_is_complete_enter, this); - bool done = (this->state != AIO_STATE_PENDING); + bool done = (this->state == AIO_STATE_COMPLETE); tracepoint(librbd, aio_is_complete_exit, done); return done; } @@ -263,13 +262,13 @@ void AioCompletion::complete_external_callback() { } void AioCompletion::mark_complete_and_notify() { + state = AIO_STATE_COMPLETE; + if (ictx != nullptr && event_notify && ictx->event_socket.is_valid()) { ictx->event_socket_completions.push(this); ictx->event_socket.notify(); } - state = AIO_STATE_COMPLETE; - { std::unique_lock locker(lock); cond.notify_all(); diff --git a/src/librbd/io/AioCompletion.h b/src/librbd/io/AioCompletion.h index 9afced7e2be95..d98055878cd9a 100644 --- a/src/librbd/io/AioCompletion.h +++ b/src/librbd/io/AioCompletion.h @@ -40,7 +40,6 @@ namespace io { struct AioCompletion { typedef enum { AIO_STATE_PENDING = 0, - AIO_STATE_CALLBACK, AIO_STATE_COMPLETE, } aio_state_t; -- 2.39.5