debian/patches/extra/0043-nbd-client-avoid-spurious-qio_channel_yield-re-entry.patch

   1 From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
   2 From: Eric Blake <eblake@redhat.com>
   3 Date: Wed, 27 Sep 2017 17:57:21 +0200
   4 Subject: [PATCH] nbd-client: avoid spurious qio_channel_yield() re-entry
   5
   6 RH-Author: Eric Blake <eblake@redhat.com>
   7 Message-id: <20170927175725.20023-4-eblake@redhat.com>
   8 Patchwork-id: 76671
   9 O-Subject: [RHEV-7.4.z qemu-kvm-rhev PATCH 3/7] nbd-client: avoid spurious qio_channel_yield() re-entry
  10 Bugzilla: 1495474
  11 RH-Acked-by: Max Reitz <mreitz@redhat.com>
  12 RH-Acked-by: Jeffrey Cody <jcody@redhat.com>
  13 RH-Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
  14
  15 From: Stefan Hajnoczi <stefanha@redhat.com>
  16
  17 The following scenario leads to an assertion failure in
  18 qio_channel_yield():
  19
  20 1. Request coroutine calls qio_channel_yield() successfully when sending
  21    would block on the socket.  It is now yielded.
  22 2. nbd_read_reply_entry() calls nbd_recv_coroutines_enter_all() because
  23    nbd_receive_reply() failed.
  24 3. Request coroutine is entered and returns from qio_channel_yield().
  25    Note that the socket fd handler has not fired yet so
  26    ioc->write_coroutine is still set.
  27 4. Request coroutine attempts to send the request body with nbd_rwv()
  28    but the socket would still block.  qio_channel_yield() is called
  29    again and assert(!ioc->write_coroutine) is hit.
  30
  31 The problem is that nbd_read_reply_entry() does not distinguish between
  32 request coroutines that are waiting to receive a reply and those that
  33 are not.
  34
  35 This patch adds a per-request bool receiving flag so
  36 nbd_read_reply_entry() can avoid spurious aio_wake() calls.
  37
  38 Reported-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
  39 Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
  40 Message-Id: <20170822125113.5025-1-stefanha@redhat.com>
  41 Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
  42 Tested-by: Eric Blake <eblake@redhat.com>
  43 Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
  44 Signed-off-by: Eric Blake <eblake@redhat.com>
  45 (cherry picked from commit 40f4a21895b5a7eae4011593837069f63460d983)
  46 Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
  47 ---
  48  block/nbd-client.c | 35 ++++++++++++++++++++++-------------
  49  block/nbd-client.h |  7 ++++++-
  50  2 files changed, 28 insertions(+), 14 deletions(-)
  51
  52 diff --git a/block/nbd-client.c b/block/nbd-client.c
  53 index 256dabeeef..f7bca3f996 100644
  54 --- a/block/nbd-client.c
  55 +++ b/block/nbd-client.c
  56 @@ -38,8 +38,10 @@ static void nbd_recv_coroutines_enter_all(NBDClientSession *s)
  57      int i;
  58
  59      for (i = 0; i < MAX_NBD_REQUESTS; i++) {
  60 -        if (s->recv_coroutine[i]) {
  61 -            aio_co_wake(s->recv_coroutine[i]);
  62 +        NBDClientRequest *req = &s->requests[i];
  63 +
  64 +        if (req->coroutine && req->receiving) {
  65 +            aio_co_wake(req->coroutine);
  66          }
  67      }
  68  }
  69 @@ -83,28 +85,28 @@ static coroutine_fn void nbd_read_reply_entry(void *opaque)
  70           * one coroutine is called until the reply finishes.
  71           */
  72          i = HANDLE_TO_INDEX(s, s->reply.handle);
  73 -        if (i >= MAX_NBD_REQUESTS || !s->recv_coroutine[i]) {
  74 +        if (i >= MAX_NBD_REQUESTS ||
  75 +            !s->requests[i].coroutine ||
  76 +            !s->requests[i].receiving) {
  77              break;
  78          }
  79
  80 -        /* We're woken up by the recv_coroutine itself.  Note that there
  81 +        /* We're woken up again by the request itself.  Note that there
  82           * is no race between yielding and reentering read_reply_co.  This
  83           * is because:
  84           *
  85 -         * - if recv_coroutine[i] runs on the same AioContext, it is only
  86 +         * - if the request runs on the same AioContext, it is only
  87           *   entered after we yield
  88           *
  89 -         * - if recv_coroutine[i] runs on a different AioContext, reentering
  90 +         * - if the request runs on a different AioContext, reentering
  91           *   read_reply_co happens through a bottom half, which can only
  92           *   run after we yield.
  93           */
  94 -        aio_co_wake(s->recv_coroutine[i]);
  95 +        aio_co_wake(s->requests[i].coroutine);
  96          qemu_coroutine_yield();
  97      }
  98
  99 -    if (ret < 0) {
 100 -        s->quit = true;
 101 -    }
 102 +    s->quit = true;
 103      nbd_recv_coroutines_enter_all(s);
 104      s->read_reply_co = NULL;
 105  }
 106 @@ -123,14 +125,17 @@ static int nbd_co_send_request(BlockDriverState *bs,
 107      s->in_flight++;
 108
 109      for (i = 0; i < MAX_NBD_REQUESTS; i++) {
 110 -        if (s->recv_coroutine[i] == NULL) {
 111 -            s->recv_coroutine[i] = qemu_coroutine_self();
 112 +        if (s->requests[i].coroutine == NULL) {
 113              break;
 114          }
 115      }
 116
 117      g_assert(qemu_in_coroutine());
 118      assert(i < MAX_NBD_REQUESTS);
 119 +
 120 +    s->requests[i].coroutine = qemu_coroutine_self();
 121 +    s->requests[i].receiving = false;
 122 +
 123      request->handle = INDEX_TO_HANDLE(s, i);
 124
 125      if (s->quit) {
 126 @@ -168,10 +173,13 @@ static void nbd_co_receive_reply(NBDClientSession *s,
 127                                   NBDReply *reply,
 128                                   QEMUIOVector *qiov)
 129  {
 130 +    int i = HANDLE_TO_INDEX(s, request->handle);
 131      int ret;
 132
 133      /* Wait until we're woken up by nbd_read_reply_entry.  */
 134 +    s->requests[i].receiving = true;
 135      qemu_coroutine_yield();
 136 +    s->requests[i].receiving = false;
 137      *reply = s->reply;
 138      if (reply->handle != request->handle || !s->ioc || s->quit) {
 139          reply->error = EIO;
 140 @@ -181,6 +189,7 @@ static void nbd_co_receive_reply(NBDClientSession *s,
 141                                 true);
 142              if (ret != request->len) {
 143                  reply->error = EIO;
 144 +                s->quit = true;
 145              }
 146          }
 147
 148 @@ -195,7 +204,7 @@ static void nbd_coroutine_end(BlockDriverState *bs,
 149      NBDClientSession *s = nbd_get_client_session(bs);
 150      int i = HANDLE_TO_INDEX(s, request->handle);
 151
 152 -    s->recv_coroutine[i] = NULL;
 153 +    s->requests[i].coroutine = NULL;
 154
 155      /* Kick the read_reply_co to get the next reply.  */
 156      if (s->read_reply_co) {
 157 diff --git a/block/nbd-client.h b/block/nbd-client.h
 158 index 9774a8ebbb..f97792ff49 100644
 159 --- a/block/nbd-client.h
 160 +++ b/block/nbd-client.h
 161 @@ -17,6 +17,11 @@
 162
 163  #define MAX_NBD_REQUESTS    16
 164
 165 +typedef struct {
 166 +    Coroutine *coroutine;
 167 +    bool receiving;         /* waiting for read_reply_co? */
 168 +} NBDClientRequest;
 169 +
 170  typedef struct NBDClientSession {
 171      QIOChannelSocket *sioc; /* The master data channel */
 172      QIOChannel *ioc; /* The current I/O channel which may differ (eg TLS) */
 173 @@ -28,7 +33,7 @@ typedef struct NBDClientSession {
 174      Coroutine *read_reply_co;
 175      int in_flight;
 176
 177 -    Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
 178 +    NBDClientRequest requests[MAX_NBD_REQUESTS];
 179      NBDReply reply;
 180      bool quit;
 181  } NBDClientSession;
 182 --
 183 2.11.0
 184