From: Emanuele Giuseppe Esposito <eesposit@redhat.com>
To: qemu-block@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Hanna Reitz <hreitz@redhat.com>,
Stefan Hajnoczi <stefanha@redhat.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
Paolo Bonzini <pbonzini@redhat.com>, Fam Zheng <fam@euphon.net>,
qemu-devel@nongnu.org,
Emanuele Giuseppe Esposito <eesposit@redhat.com>
Subject: [PATCH 1/8] virtio_queue_aio_attach_host_notifier: remove AioContext lock
Date: Thu, 9 Jun 2022 10:37:20 -0400 [thread overview]
Message-ID: <20220609143727.1151816-2-eesposit@redhat.com> (raw)
In-Reply-To: <20220609143727.1151816-1-eesposit@redhat.com>
virtio_queue_aio_attach_host_notifier() and
virtio_queue_aio_attach_host_notifier_nopoll() run always in the
main loop, so there is no need to protect them with AioContext
lock.
On the other side, virtio_queue_aio_detach_host_notifier() runs
in a bh in the iothread context, but it is always scheduled
(thus serialized) by the main loop. Therefore removing the
AioContext lock is safe, but unfortunately we can't do it
right now since bdrv_set_aio_context() and
aio_wait_bh_oneshot() still need to have it.
Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
---
hw/block/dataplane/virtio-blk.c | 14 ++++++++++++--
hw/block/virtio-blk.c | 2 ++
hw/scsi/virtio-scsi-dataplane.c | 12 ++++++++++--
3 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
index 49276e46f2..f9224f23d2 100644
--- a/hw/block/dataplane/virtio-blk.c
+++ b/hw/block/dataplane/virtio-blk.c
@@ -167,6 +167,8 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
Error *local_err = NULL;
int r;
+ GLOBAL_STATE_CODE();
+
if (vblk->dataplane_started || s->starting) {
return 0;
}
@@ -243,13 +245,11 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
}
/* Get this show started by hooking up our callbacks */
- aio_context_acquire(s->ctx);
for (i = 0; i < nvqs; i++) {
VirtQueue *vq = virtio_get_queue(s->vdev, i);
virtio_queue_aio_attach_host_notifier(vq, s->ctx);
}
- aio_context_release(s->ctx);
return 0;
fail_aio_context:
@@ -304,6 +304,8 @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
unsigned i;
unsigned nvqs = s->conf->num_queues;
+ GLOBAL_STATE_CODE();
+
if (!vblk->dataplane_started || s->stopping) {
return;
}
@@ -318,6 +320,14 @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
trace_virtio_blk_data_plane_stop(s);
aio_context_acquire(s->ctx);
+ /*
+ * TODO: virtio_blk_data_plane_stop_bh() does not need the AioContext lock,
+ * because even though virtio_queue_aio_detach_host_notifier() runs in
+ * Iothread context, such calls are serialized by the BQL held (this
+ * function runs in the main loop).
+ * On the other side, virtio_queue_aio_attach_host_notifier* always runs
+ * in the main loop, therefore it doesn't need the AioContext lock.
+ */
aio_wait_bh_oneshot(s->ctx, virtio_blk_data_plane_stop_bh, s);
/* Drain and try to switch bs back to the QEMU main loop. If other users
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
index e9ba752f6b..8d0590cc76 100644
--- a/hw/block/virtio-blk.c
+++ b/hw/block/virtio-blk.c
@@ -121,6 +121,8 @@ static void virtio_blk_rw_complete(void *opaque, int ret)
VirtIOBlock *s = next->dev;
VirtIODevice *vdev = VIRTIO_DEVICE(s);
+ IO_CODE();
+
aio_context_acquire(blk_get_aio_context(s->conf.conf.blk));
while (next) {
VirtIOBlockReq *req = next;
diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c
index 8bb6e6acfc..7080e9caa9 100644
--- a/hw/scsi/virtio-scsi-dataplane.c
+++ b/hw/scsi/virtio-scsi-dataplane.c
@@ -91,6 +91,8 @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev)
VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(vdev);
VirtIOSCSI *s = VIRTIO_SCSI(vdev);
+ GLOBAL_STATE_CODE();
+
if (s->dataplane_started ||
s->dataplane_starting ||
s->dataplane_fenced) {
@@ -136,7 +138,6 @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev)
memory_region_transaction_commit();
- aio_context_acquire(s->ctx);
virtio_queue_aio_attach_host_notifier(vs->ctrl_vq, s->ctx);
virtio_queue_aio_attach_host_notifier_no_poll(vs->event_vq, s->ctx);
@@ -146,7 +147,6 @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev)
s->dataplane_starting = false;
s->dataplane_started = true;
- aio_context_release(s->ctx);
return 0;
fail_host_notifiers:
@@ -193,6 +193,14 @@ void virtio_scsi_dataplane_stop(VirtIODevice *vdev)
s->dataplane_stopping = true;
aio_context_acquire(s->ctx);
+ /*
+ * TODO: virtio_scsi_dataplane_stop_bh() does not need the AioContext lock,
+ * because even though virtio_queue_aio_detach_host_notifier() runs in
+ * Iothread context, such calls are serialized by the BQL held (this
+ * function runs in the main loop).
+ * On the other side, virtio_queue_aio_attach_host_notifier* always runs
+ * in the main loop, therefore it doesn't need the AioContext lock.
+ */
aio_wait_bh_oneshot(s->ctx, virtio_scsi_dataplane_stop_bh, s);
aio_context_release(s->ctx);
--
2.31.1
next prev parent reply other threads:[~2022-06-09 15:38 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-06-09 14:37 [PATCH 0/8] virtio-blk: removal of AioContext lock Emanuele Giuseppe Esposito
2022-06-09 14:37 ` Emanuele Giuseppe Esposito [this message]
2022-07-05 14:11 ` [PATCH 1/8] virtio_queue_aio_attach_host_notifier: remove " Stefan Hajnoczi
2022-07-08 9:01 ` Emanuele Giuseppe Esposito
2022-07-12 12:47 ` Stefan Hajnoczi
2022-06-09 14:37 ` [PATCH 2/8] block-backend: enable_write_cache should be atomic Emanuele Giuseppe Esposito
2022-07-05 14:16 ` Stefan Hajnoczi
2022-06-09 14:37 ` [PATCH 3/8] virtio_blk_process_queued_requests: always run in a bh Emanuele Giuseppe Esposito
2022-07-05 14:23 ` Stefan Hajnoczi
2022-07-08 9:07 ` Emanuele Giuseppe Esposito
2022-07-12 12:18 ` Stefan Hajnoczi
2022-06-09 14:37 ` [PATCH 4/8] virtio: categorize callbacks in GS Emanuele Giuseppe Esposito
2022-06-16 16:50 ` Michael S. Tsirkin
2022-07-05 14:25 ` Stefan Hajnoczi
2022-06-09 14:37 ` [PATCH 5/8] virtio-blk: mark GLOBAL_STATE_CODE functions Emanuele Giuseppe Esposito
2022-07-05 14:27 ` Stefan Hajnoczi
2022-06-09 14:37 ` [PATCH 6/8] virtio-blk: mark IO_CODE functions Emanuele Giuseppe Esposito
2022-07-05 14:39 ` Stefan Hajnoczi
2022-07-08 9:19 ` Emanuele Giuseppe Esposito
2022-07-12 12:26 ` Stefan Hajnoczi
2022-06-09 14:37 ` [PATCH 7/8] VirtIOBlock: protect rq with its own lock Emanuele Giuseppe Esposito
2022-07-05 14:45 ` Stefan Hajnoczi
2022-07-08 9:33 ` Emanuele Giuseppe Esposito
2022-07-08 11:22 ` Emanuele Giuseppe Esposito
2022-07-12 12:34 ` Stefan Hajnoczi
2022-07-12 12:29 ` Stefan Hajnoczi
2022-06-09 14:37 ` [PATCH 8/8] virtio-blk: remove unnecessary AioContext lock from function already safe Emanuele Giuseppe Esposito
2022-07-05 14:48 ` Stefan Hajnoczi
-- strict thread matches above, loose matches on Subject: below --
2022-11-08 21:19 [PATCH 0/8] virtio-blk: remove AioContext lock Stefan Hajnoczi
2022-11-08 21:19 ` [PATCH 1/8] virtio_queue_aio_attach_host_notifier: " Stefan Hajnoczi
2022-11-11 12:17 ` Emanuele Giuseppe Esposito
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=20220609143727.1151816-2-eesposit@redhat.com \
--to=eesposit@redhat.com \
--cc=fam@euphon.net \
--cc=hreitz@redhat.com \
--cc=kwolf@redhat.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=qemu-block@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=stefanha@redhat.com \
/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).