[PATCH 4/6] virtio-blk: restart s->rq reqs in vq AioContexts

Stefan Hajnoczi posted 6 patches 10 months, 1 week ago
Maintainers: Stefan Hajnoczi <stefanha@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>
There is a newer version of this series
[PATCH 4/6] virtio-blk: restart s->rq reqs in vq AioContexts
Posted by Stefan Hajnoczi 10 months, 1 week ago
A virtio-blk device with the iothread-vq-mapping parameter has
per-virtqueue AioContexts. It is not thread-safe to process s->rq
requests in the BlockBackend AioContext since that may be different from
the virtqueue's AioContext to which this request belongs. The code
currently races and could crash.

Adapt virtio_blk_dma_restart_cb() to first split s->rq into per-vq lists
and then schedule a BH each vq's AioContext as necessary. This way
requests are safely processed in their vq's AioContext.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 hw/block/virtio-blk.c | 44 ++++++++++++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e342cb2cfb..4525988d92 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -1156,16 +1156,11 @@ static void virtio_blk_handle_output(VirtIODevice *vdev, VirtQueue *vq)
 
 static void virtio_blk_dma_restart_bh(void *opaque)
 {
-    VirtIOBlock *s = opaque;
+    VirtIOBlockReq *req = opaque;
+    VirtIOBlock *s = req->dev; /* we're called with at least one request */
 
-    VirtIOBlockReq *req;
     MultiReqBuffer mrb = {};
 
-    WITH_QEMU_LOCK_GUARD(&s->rq_lock) {
-        req = s->rq;
-        s->rq = NULL;
-    }
-
     while (req) {
         VirtIOBlockReq *next = req->next;
         if (virtio_blk_handle_request(req, &mrb)) {
@@ -1195,16 +1190,43 @@ static void virtio_blk_dma_restart_cb(void *opaque, bool running,
                                       RunState state)
 {
     VirtIOBlock *s = opaque;
+    uint16_t num_queues = s->conf.num_queues;
 
     if (!running) {
         return;
     }
 
-    /* Paired with dec in virtio_blk_dma_restart_bh() */
-    blk_inc_in_flight(s->conf.conf.blk);
+    /* Split the device-wide s->rq request list into per-vq request lists */
+    g_autofree VirtIOBlockReq **vq_rq = g_new0(VirtIOBlockReq *, num_queues);
+    VirtIOBlockReq *rq;
 
-    aio_bh_schedule_oneshot(blk_get_aio_context(s->conf.conf.blk),
-            virtio_blk_dma_restart_bh, s);
+    WITH_QEMU_LOCK_GUARD(&s->rq_lock) {
+        rq = s->rq;
+        s->rq = NULL;
+    }
+
+    while (rq) {
+        VirtIOBlockReq *next = rq->next;
+        uint16_t idx = virtio_get_queue_index(rq->vq);
+
+        rq->next = vq_rq[idx];
+        vq_rq[idx] = rq;
+        rq = next;
+    }
+
+    /* Schedule a BH to submit the requests in each vq's AioContext */
+    for (uint16_t i = 0; i < num_queues; i++) {
+        if (!vq_rq[i]) {
+            continue;
+        }
+
+        /* Paired with dec in virtio_blk_dma_restart_bh() */
+        blk_inc_in_flight(s->conf.conf.blk);
+
+        aio_bh_schedule_oneshot(s->vq_aio_context[i],
+                                virtio_blk_dma_restart_bh,
+                                vq_rq[i]);
+    }
 }
 
 static void virtio_blk_reset(VirtIODevice *vdev)
-- 
2.43.0