qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Roman Kagan <rvkagan@yandex-team.ru>
To: qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-block@nongnu.org, Max Reitz <mreitz@redhat.com>,
	yc-core@yandex-team.ru
Subject: [PATCH 4/7] block/nbd: transfer reconnection stuff across aio_context switch
Date: Mon, 15 Mar 2021 09:06:08 +0300	[thread overview]
Message-ID: <20210315060611.2989049-5-rvkagan@yandex-team.ru> (raw)
In-Reply-To: <20210315060611.2989049-1-rvkagan@yandex-team.ru>

Make varios pieces of reconnection logic correctly survive the
transition of the BDRVNBDState from one aio_context to another.  In
particular,

- cancel the reconnect_delay_timer and rearm it in the new context;
- cancel the sleep of the connection_co between reconnect attempt so
  that it continues in the new context;
- prevent the connection thread from delivering its status to the old
  context, and retartget it to the new context on attach.

None of these is needed at the moment because the aio_context switch
happens within a drained section and that effectively terminates the
reconnection logic on entry and starts it over on exit.  However, this
patch paves the way to keeping the reconnection process active across
the drained section (in a followup patch).

Signed-off-by: Roman Kagan <rvkagan@yandex-team.ru>
---
 block/nbd.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 42 insertions(+), 2 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 658b827d24..a6d713ba58 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -126,6 +126,7 @@ typedef struct BDRVNBDState {
     bool wait_in_flight;
 
     QEMUTimer *reconnect_delay_timer;
+    int64_t reconnect_expire_time_ns;
 
     NBDClientRequest requests[MAX_NBD_REQUESTS];
     NBDReply reply;
@@ -240,6 +241,7 @@ static void reconnect_delay_timer_init(BDRVNBDState *s, uint64_t expire_time_ns)
 static void nbd_client_detach_aio_context(BlockDriverState *bs)
 {
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
+    NBDConnectThread *thr = s->connect_thread;
 
     /*
      * This runs in the (old, about to be detached) aio context of the @bs so
@@ -247,8 +249,31 @@ static void nbd_client_detach_aio_context(BlockDriverState *bs)
      */
     assert(qemu_get_current_aio_context() == bdrv_get_aio_context(bs));
 
-    /* Timer is deleted in nbd_client_co_drain_begin() */
-    assert(!s->reconnect_delay_timer);
+    /*
+     * Make sure the connection thread doesn't try to deliver its status to the
+     * old context.
+     */
+    qemu_mutex_lock(&thr->mutex);
+    thr->bh_ctx = NULL;
+    qemu_mutex_unlock(&thr->mutex);
+
+    /*
+     * Preserve the expiration time of the reconnect_delay_timer in order to
+     * resume it on the new aio context.
+     */
+    s->reconnect_expire_time_ns = s->reconnect_delay_timer ?
+        timer_expire_time_ns(s->reconnect_delay_timer) : -1;
+    reconnect_delay_timer_del(s);
+
+    /*
+     * If the connection coroutine was sleeping between reconnect attempts,
+     * wake it up now and let it continue the process in the new aio context.
+     * This will distort the exponential back-off but that's probably ok.
+     */
+    if (s->connection_co_sleep_ns_state) {
+        qemu_co_sleep_wake(s->connection_co_sleep_ns_state);
+    }
+
     /*
      * If reconnect is in progress we may have no ->ioc.  It will be
      * re-instantiated in the proper aio context once the connection is
@@ -263,6 +288,7 @@ static void nbd_client_attach_aio_context_bh(void *opaque)
 {
     BlockDriverState *bs = opaque;
     BDRVNBDState *s = (BDRVNBDState *)bs->opaque;
+    NBDConnectThread *thr = s->connect_thread;
 
     /*
      * This runs in the (new, just attached) aio context of the @bs so
@@ -270,6 +296,20 @@ static void nbd_client_attach_aio_context_bh(void *opaque)
      */
     assert(qemu_get_current_aio_context() == bdrv_get_aio_context(bs));
 
+    if (nbd_client_connecting_wait(s) && s->reconnect_expire_time_ns >= 0) {
+        reconnect_delay_timer_init(s, s->reconnect_expire_time_ns);
+    }
+
+    /*
+     * If the connection thread hasn't completed connecting yet, make sure it
+     * can deliver its status in the new context.
+     */
+    qemu_mutex_lock(&thr->mutex);
+    if (thr->state == CONNECT_THREAD_RUNNING) {
+        thr->bh_ctx = qemu_get_current_aio_context();
+    }
+    qemu_mutex_unlock(&thr->mutex);
+
     if (s->connection_co) {
         /*
          * The node is still drained, so we know the coroutine has yielded in
-- 
2.30.2



  parent reply	other threads:[~2021-03-15  6:12 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-15  6:06 [PATCH 0/7] block/nbd: decouple reconnect from drain Roman Kagan
2021-03-15  6:06 ` [PATCH 1/7] block/nbd: avoid touching freed connect_thread Roman Kagan
2021-03-15 15:40   ` Vladimir Sementsov-Ogievskiy
2021-03-16 15:29     ` Roman Kagan
2021-04-06 16:21   ` Vladimir Sementsov-Ogievskiy
2021-03-15  6:06 ` [PATCH 2/7] block/nbd: use uniformly nbd_client_connecting_wait Roman Kagan
2021-03-15 15:48   ` Vladimir Sementsov-Ogievskiy
2021-03-15  6:06 ` [PATCH 3/7] block/nbd: assert attach/detach runs in the proper context Roman Kagan
2021-03-15 16:41   ` Vladimir Sementsov-Ogievskiy
2021-03-15 19:57     ` Roman Kagan
2021-03-15  6:06 ` Roman Kagan [this message]
2021-03-15  6:06 ` [PATCH 5/7] block/nbd: better document a case in nbd_co_establish_connection Roman Kagan
2021-03-15  6:06 ` [PATCH 6/7] block/nbd: decouple reconnect from drain Roman Kagan
2021-03-15 20:10   ` Vladimir Sementsov-Ogievskiy
2021-03-16 16:03     ` Roman Kagan
2021-03-16 18:09       ` Vladimir Sementsov-Ogievskiy
2021-03-26  6:16         ` Roman Kagan
2021-03-15  6:06 ` [PATCH 7/7] block/nbd: stop manipulating in_flight counter Roman Kagan
2021-03-15 20:15   ` Vladimir Sementsov-Ogievskiy
2021-03-16 16:08     ` Roman Kagan
2021-03-16 18:37       ` Vladimir Sementsov-Ogievskiy
2021-03-26  7:41         ` Roman Kagan
2021-03-15 19:45 ` [PATCH 0/7] block/nbd: decouple reconnect from drain Vladimir Sementsov-Ogievskiy
2021-03-16 15:52   ` Roman Kagan
2021-03-16 14:41 ` Eric Blake
2021-03-16 16:10   ` Roman Kagan
2021-03-17  8:35 ` Vladimir Sementsov-Ogievskiy
2021-03-26  8:07   ` Roman Kagan
2021-04-07  7:45   ` Roman Kagan
2021-04-07 10:13     ` Vladimir Sementsov-Ogievskiy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210315060611.2989049-5-rvkagan@yandex-team.ru \
    --to=rvkagan@yandex-team.ru \
    --cc=kwolf@redhat.com \
    --cc=mreitz@redhat.com \
    --cc=qemu-block@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=vsementsov@virtuozzo.com \
    --cc=yc-core@yandex-team.ru \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).