[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH v2 for-7.1 4/9] nbd: keep send_mutex/free_sema handling outside n
From: |
Paolo Bonzini |
Subject: |
[PATCH v2 for-7.1 4/9] nbd: keep send_mutex/free_sema handling outside nbd_co_do_establish_connection |
Date: |
Thu, 14 Apr 2022 19:57:51 +0200 |
Elevate s->in_flight early so that other incoming requests will wait
on the CoQueue in nbd_co_send_request; restart them after getting back
from nbd_reconnect_attempt. This could be after the reconnect timer or
nbd_cancel_in_flight have cancelled the attempt, so there is no
need anymore to cancel the requests there.
nbd_co_send_request now handles both stopping and restarting pending
requests after a successful connection, and there is no need to
hold send_mutex in nbd_co_do_establish_connection. The current setup
is confusing because nbd_co_do_establish_connection is called both with
send_mutex taken and without it. Before the patch it uses free_sema which
(at least in theory...) is protected by send_mutex, after the patch it
does not anymore.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
block/coroutines.h | 4 +--
block/nbd.c | 66 ++++++++++++++++++++++------------------------
2 files changed, 33 insertions(+), 37 deletions(-)
diff --git a/block/coroutines.h b/block/coroutines.h
index b293e943c8..8f6e438ef3 100644
--- a/block/coroutines.h
+++ b/block/coroutines.h
@@ -59,7 +59,7 @@ int coroutine_fn bdrv_co_writev_vmstate(BlockDriverState *bs,
QEMUIOVector *qiov, int64_t pos);
int coroutine_fn
-nbd_co_do_establish_connection(BlockDriverState *bs, Error **errp);
+nbd_co_do_establish_connection(BlockDriverState *bs, bool blocking, Error
**errp);
int coroutine_fn
@@ -109,7 +109,7 @@ bdrv_common_block_status_above(BlockDriverState *bs,
BlockDriverState **file,
int *depth);
int generated_co_wrapper
-nbd_do_establish_connection(BlockDriverState *bs, Error **errp);
+nbd_do_establish_connection(BlockDriverState *bs, bool blocking, Error **errp);
int generated_co_wrapper
blk_do_preadv(BlockBackend *blk, int64_t offset, int64_t bytes,
diff --git a/block/nbd.c b/block/nbd.c
index a30603ce87..62dd338ef3 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -187,9 +187,6 @@ static void reconnect_delay_timer_cb(void *opaque)
if (qatomic_load_acquire(&s->state) == NBD_CLIENT_CONNECTING_WAIT) {
s->state = NBD_CLIENT_CONNECTING_NOWAIT;
nbd_co_establish_connection_cancel(s->conn);
- while (qemu_co_enter_next(&s->free_sema, NULL)) {
- /* Resume all queued requests */
- }
}
reconnect_delay_timer_del(s);
@@ -310,11 +307,10 @@ static int nbd_handle_updated_info(BlockDriverState *bs,
Error **errp)
}
int coroutine_fn nbd_co_do_establish_connection(BlockDriverState *bs,
- Error **errp)
+ bool blocking, Error **errp)
{
BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
int ret;
- bool blocking = nbd_client_connecting_wait(s);
IO_CODE();
assert(!s->ioc);
@@ -350,7 +346,6 @@ int coroutine_fn
nbd_co_do_establish_connection(BlockDriverState *bs,
/* successfully connected */
s->state = NBD_CLIENT_CONNECTED;
- qemu_co_queue_restart_all(&s->free_sema);
return 0;
}
@@ -358,25 +353,25 @@ int coroutine_fn
nbd_co_do_establish_connection(BlockDriverState *bs,
/* called under s->send_mutex */
static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState *s)
{
- assert(nbd_client_connecting(s));
- assert(s->in_flight == 0);
-
- if (nbd_client_connecting_wait(s) && s->reconnect_delay &&
- !s->reconnect_delay_timer)
- {
- /*
- * It's first reconnect attempt after switching to
- * NBD_CLIENT_CONNECTING_WAIT
- */
- reconnect_delay_timer_init(s,
- qemu_clock_get_ns(QEMU_CLOCK_REALTIME) +
- s->reconnect_delay * NANOSECONDS_PER_SECOND);
- }
+ bool blocking = nbd_client_connecting_wait(s);
/*
* Now we are sure that nobody is accessing the channel, and no one will
* try until we set the state to CONNECTED.
*/
+ assert(nbd_client_connecting(s));
+ assert(s->in_flight == 1);
+
+ if (blocking && !s->reconnect_delay_timer) {
+ /*
+ * It's the first reconnect attempt after switching to
+ * NBD_CLIENT_CONNECTING_WAIT
+ */
+ g_assert(s->reconnect_delay);
+ reconnect_delay_timer_init(s,
+ qemu_clock_get_ns(QEMU_CLOCK_REALTIME) +
+ s->reconnect_delay * NANOSECONDS_PER_SECOND);
+ }
/* Finalize previous connection if any */
if (s->ioc) {
@@ -387,7 +382,9 @@ static coroutine_fn void nbd_reconnect_attempt(BDRVNBDState
*s)
s->ioc = NULL;
}
- nbd_co_do_establish_connection(s->bs, NULL);
+ qemu_co_mutex_unlock(&s->send_mutex);
+ nbd_co_do_establish_connection(s->bs, blocking, NULL);
+ qemu_co_mutex_lock(&s->send_mutex);
/*
* The reconnect attempt is done (maybe successfully, maybe not), so
@@ -472,21 +469,21 @@ static int coroutine_fn
nbd_co_send_request(BlockDriverState *bs,
qemu_co_mutex_lock(&s->send_mutex);
while (s->in_flight == MAX_NBD_REQUESTS ||
- (!nbd_client_connected(s) && s->in_flight > 0))
- {
+ (!nbd_client_connected(s) && s->in_flight > 0)) {
qemu_co_queue_wait(&s->free_sema, &s->send_mutex);
}
- if (nbd_client_connecting(s)) {
- nbd_reconnect_attempt(s);
- }
-
- if (!nbd_client_connected(s)) {
- rc = -EIO;
- goto err;
- }
-
s->in_flight++;
+ if (!nbd_client_connected(s)) {
+ if (nbd_client_connecting(s)) {
+ nbd_reconnect_attempt(s);
+ qemu_co_queue_restart_all(&s->free_sema);
+ }
+ if (!nbd_client_connected(s)) {
+ rc = -EIO;
+ goto err;
+ }
+ }
for (i = 0; i < MAX_NBD_REQUESTS; i++) {
if (s->requests[i].coroutine == NULL) {
@@ -526,8 +523,8 @@ err:
nbd_channel_error(s, rc);
if (i != -1) {
s->requests[i].coroutine = NULL;
- s->in_flight--;
}
+ s->in_flight--;
qemu_co_queue_next(&s->free_sema);
}
qemu_co_mutex_unlock(&s->send_mutex);
@@ -1883,7 +1880,7 @@ static int nbd_open(BlockDriverState *bs, QDict *options,
int flags,
}
s->state = NBD_CLIENT_CONNECTING_WAIT;
- ret = nbd_do_establish_connection(bs, errp);
+ ret = nbd_do_establish_connection(bs, true, errp);
if (ret < 0) {
goto fail;
}
@@ -2049,7 +2046,6 @@ static void nbd_cancel_in_flight(BlockDriverState *bs)
if (s->state == NBD_CLIENT_CONNECTING_WAIT) {
s->state = NBD_CLIENT_CONNECTING_NOWAIT;
- qemu_co_queue_restart_all(&s->free_sema);
}
nbd_co_establish_connection_cancel(s->conn);
--
2.35.1
- [PATCH v2 for-7.1 0/9] nbd: actually make s->state thread-safe, Paolo Bonzini, 2022/04/14
- [PATCH v2 for-7.1 1/9] nbd: safeguard against waking up invalid coroutine, Paolo Bonzini, 2022/04/14
- [PATCH v2 for-7.1 2/9] nbd: mark more coroutine_fns, Paolo Bonzini, 2022/04/14
- [PATCH v2 for-7.1 3/9] nbd: remove peppering of nbd_client_connected, Paolo Bonzini, 2022/04/14
- [PATCH v2 for-7.1 4/9] nbd: keep send_mutex/free_sema handling outside nbd_co_do_establish_connection,
Paolo Bonzini <=
- [PATCH v2 for-7.1 5/9] nbd: use a QemuMutex to synchronize yanking, reconnection and coroutines, Paolo Bonzini, 2022/04/14
- [PATCH v2 for-7.1 7/9] nbd: move s->state under requests_lock, Paolo Bonzini, 2022/04/14