1
The following changes since commit 33f18cf7dca7741d3647d514040904ce83edd73d:
1
The following changes since commit 887cba855bb6ff4775256f7968409281350b568c:
2
2
3
Merge remote-tracking branch 'remotes/kraxel/tags/audio-20190821-pull-request' into staging (2019-08-21 15:18:50 +0100)
3
configure: Fix cross-building for RISCV host (v5) (2023-07-11 17:56:09 +0100)
4
4
5
are available in the Git repository at:
5
are available in the Git repository at:
6
6
7
https://github.com/stefanha/qemu.git tags/block-pull-request
7
https://gitlab.com/stefanha/qemu.git tags/block-pull-request
8
8
9
for you to fetch changes up to 5d4c1ed3d46d7e2010b389fe5f3376f605182ab0:
9
for you to fetch changes up to 75dcb4d790bbe5327169fd72b185960ca58e2fa6:
10
10
11
vhost-user-scsi: prevent using uninitialized vqs (2019-08-22 16:52:23 +0100)
11
virtio-blk: fix host notifier issues during dataplane start/stop (2023-07-12 15:20:32 -0400)
12
12
13
----------------------------------------------------------------
13
----------------------------------------------------------------
14
Pull request
14
Pull request
15
15
16
----------------------------------------------------------------
16
----------------------------------------------------------------
17
17
18
Raphael Norwitz (1):
18
Stefan Hajnoczi (1):
19
vhost-user-scsi: prevent using uninitialized vqs
19
virtio-blk: fix host notifier issues during dataplane start/stop
20
20
21
Stefan Hajnoczi (1):
21
hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
22
util/async: hold AioContext ref to prevent use-after-free
22
1 file changed, 38 insertions(+), 29 deletions(-)
23
24
hw/scsi/vhost-user-scsi.c | 2 +-
25
util/async.c | 8 ++++++++
26
2 files changed, 9 insertions(+), 1 deletion(-)
27
23
28
--
24
--
29
2.21.0
25
2.40.1
30
31
diff view generated by jsdifflib
Deleted patch
1
The tests/test-bdrv-drain /bdrv-drain/iothread/drain test case does the
2
following:
3
1
4
1. The preadv coroutine calls aio_bh_schedule_oneshot() and then yields.
5
2. The one-shot BH executes in another AioContext. All it does is call
6
aio_co_wakeup(preadv_co).
7
3. The preadv coroutine is re-entered and returns.
8
9
There is a race condition in aio_co_wake() where the preadv coroutine
10
returns and the test case destroys the preadv IOThread. aio_co_wake()
11
can still be running in the other AioContext and it performs an access
12
to the freed IOThread AioContext.
13
14
Here is the race in aio_co_schedule():
15
16
QSLIST_INSERT_HEAD_ATOMIC(&ctx->scheduled_coroutines,
17
co, co_scheduled_next);
18
<-- race: co may execute before we invoke qemu_bh_schedule()!
19
qemu_bh_schedule(ctx->co_schedule_bh);
20
21
So if co causes ctx to be freed then we're in trouble. Fix this problem
22
by holding a reference to ctx.
23
24
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
25
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
26
Message-id: 20190723190623.21537-1-stefanha@redhat.com
27
Message-Id: <20190723190623.21537-1-stefanha@redhat.com>
28
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
29
---
30
util/async.c | 8 ++++++++
31
1 file changed, 8 insertions(+)
32
33
diff --git a/util/async.c b/util/async.c
34
index XXXXXXX..XXXXXXX 100644
35
--- a/util/async.c
36
+++ b/util/async.c
37
@@ -XXX,XX +XXX,XX @@ void aio_co_schedule(AioContext *ctx, Coroutine *co)
38
abort();
39
}
40
41
+ /* The coroutine might run and release the last ctx reference before we
42
+ * invoke qemu_bh_schedule(). Take a reference to keep ctx alive until
43
+ * we're done.
44
+ */
45
+ aio_context_ref(ctx);
46
+
47
QSLIST_INSERT_HEAD_ATOMIC(&ctx->scheduled_coroutines,
48
co, co_scheduled_next);
49
qemu_bh_schedule(ctx->co_schedule_bh);
50
+
51
+ aio_context_unref(ctx);
52
}
53
54
void aio_co_wake(struct Coroutine *co)
55
--
56
2.21.0
57
58
diff view generated by jsdifflib
1
From: Raphael Norwitz <raphael.norwitz@nutanix.com>
1
The main loop thread can consume 100% CPU when using --device
2
virtio-blk-pci,iothread=<iothread>. ppoll() constantly returns but
3
reading virtqueue host notifiers fails with EAGAIN. The file descriptors
4
are stale and remain registered with the AioContext because of bugs in
5
the virtio-blk dataplane start/stop code.
2
6
3
Of the 3 virtqueues, seabios only sets cmd, leaving ctrl
7
The problem is that the dataplane start/stop code involves drain
4
and event without a physical address. This can cause
8
operations, which call virtio_blk_drained_begin() and
5
vhost_verify_ring_part_mapping to return ENOMEM, causing
9
virtio_blk_drained_end() at points where the host notifier is not
6
the following logs:
10
operational:
11
- In virtio_blk_data_plane_start(), blk_set_aio_context() drains after
12
vblk->dataplane_started has been set to true but the host notifier has
13
not been attached yet.
14
- In virtio_blk_data_plane_stop(), blk_drain() and blk_set_aio_context()
15
drain after the host notifier has already been detached but with
16
vblk->dataplane_started still set to true.
7
17
8
qemu-system-x86_64: Unable to map available ring for ring 0
18
I would like to simplify ->ioeventfd_start/stop() to avoid interactions
9
qemu-system-x86_64: Verify ring failure on region 0
19
with drain entirely, but couldn't find a way to do that. Instead, this
20
patch accepts the fragile nature of the code and reorders it so that
21
vblk->dataplane_started is false during drain operations. This way the
22
virtio_blk_drained_begin() and virtio_blk_drained_end() calls don't
23
touch the host notifier. The result is that
24
virtio_blk_data_plane_start() and virtio_blk_data_plane_stop() have
25
complete control over the host notifier and stale file descriptors are
26
no longer left in the AioContext.
10
27
11
The qemu commit e6cc11d64fc998c11a4dfcde8fda3fc33a74d844
28
This patch fixes the 100% CPU consumption in the main loop thread and
12
has already resolved the issue for vhost scsi devices but
29
correctly moves host notifier processing to the IOThread.
13
the fix was never applied to vhost-user scsi devices.
14
30
15
Signed-off-by: Raphael Norwitz <raphael.norwitz@nutanix.com>
31
Fixes: 1665d9326fd2 ("virtio-blk: implement BlockDevOps->drained_begin()")
16
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
32
Reported-by: Lukáš Doktor <ldoktor@redhat.com>
17
Message-id: 1560299717-177734-1-git-send-email-raphael.norwitz@nutanix.com
33
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
18
Message-Id: <1560299717-177734-1-git-send-email-raphael.norwitz@nutanix.com>
34
Tested-by: Lukas Doktor <ldoktor@redhat.com>
35
Message-id: 20230704151527.193586-1-stefanha@redhat.com
19
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
36
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
20
---
37
---
21
hw/scsi/vhost-user-scsi.c | 2 +-
38
hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
22
1 file changed, 1 insertion(+), 1 deletion(-)
39
1 file changed, 38 insertions(+), 29 deletions(-)
23
40
24
diff --git a/hw/scsi/vhost-user-scsi.c b/hw/scsi/vhost-user-scsi.c
41
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
25
index XXXXXXX..XXXXXXX 100644
42
index XXXXXXX..XXXXXXX 100644
26
--- a/hw/scsi/vhost-user-scsi.c
43
--- a/hw/block/dataplane/virtio-blk.c
27
+++ b/hw/scsi/vhost-user-scsi.c
44
+++ b/hw/block/dataplane/virtio-blk.c
28
@@ -XXX,XX +XXX,XX @@ static void vhost_user_scsi_realize(DeviceState *dev, Error **errp)
45
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
46
47
memory_region_transaction_commit();
48
49
- /*
50
- * These fields are visible to the IOThread so we rely on implicit barriers
51
- * in aio_context_acquire() on the write side and aio_notify_accept() on
52
- * the read side.
53
- */
54
- s->starting = false;
55
- vblk->dataplane_started = true;
56
trace_virtio_blk_data_plane_start(s);
57
58
old_context = blk_get_aio_context(s->conf->conf.blk);
59
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
60
event_notifier_set(virtio_queue_get_host_notifier(vq));
29
}
61
}
30
62
31
vsc->dev.nvqs = 2 + vs->conf.num_queues;
63
+ /*
32
- vsc->dev.vqs = g_new(struct vhost_virtqueue, vsc->dev.nvqs);
64
+ * These fields must be visible to the IOThread when it processes the
33
+ vsc->dev.vqs = g_new0(struct vhost_virtqueue, vsc->dev.nvqs);
65
+ * virtqueue, otherwise it will think dataplane has not started yet.
34
vsc->dev.vq_index = 0;
66
+ *
35
vsc->dev.backend_features = 0;
67
+ * Make sure ->dataplane_started is false when blk_set_aio_context() is
36
vqs = vsc->dev.vqs;
68
+ * called above so that draining does not cause the host notifier to be
69
+ * detached/attached prematurely.
70
+ */
71
+ s->starting = false;
72
+ vblk->dataplane_started = true;
73
+ smp_wmb(); /* paired with aio_notify_accept() on the read side */
74
+
75
/* Get this show started by hooking up our callbacks */
76
if (!blk_in_drain(s->conf->conf.blk)) {
77
aio_context_acquire(s->ctx);
78
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
79
fail_guest_notifiers:
80
vblk->dataplane_disabled = true;
81
s->starting = false;
82
- vblk->dataplane_started = true;
83
return -ENOSYS;
84
}
85
86
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
87
aio_wait_bh_oneshot(s->ctx, virtio_blk_data_plane_stop_bh, s);
88
}
89
90
+ /*
91
+ * Batch all the host notifiers in a single transaction to avoid
92
+ * quadratic time complexity in address_space_update_ioeventfds().
93
+ */
94
+ memory_region_transaction_begin();
95
+
96
+ for (i = 0; i < nvqs; i++) {
97
+ virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
98
+ }
99
+
100
+ /*
101
+ * The transaction expects the ioeventfds to be open when it
102
+ * commits. Do it now, before the cleanup loop.
103
+ */
104
+ memory_region_transaction_commit();
105
+
106
+ for (i = 0; i < nvqs; i++) {
107
+ virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i);
108
+ }
109
+
110
+ /*
111
+ * Set ->dataplane_started to false before draining so that host notifiers
112
+ * are not detached/attached anymore.
113
+ */
114
+ vblk->dataplane_started = false;
115
+
116
aio_context_acquire(s->ctx);
117
118
/* Wait for virtio_blk_dma_restart_bh() and in flight I/O to complete */
119
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
120
121
aio_context_release(s->ctx);
122
123
- /*
124
- * Batch all the host notifiers in a single transaction to avoid
125
- * quadratic time complexity in address_space_update_ioeventfds().
126
- */
127
- memory_region_transaction_begin();
128
-
129
- for (i = 0; i < nvqs; i++) {
130
- virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
131
- }
132
-
133
- /*
134
- * The transaction expects the ioeventfds to be open when it
135
- * commits. Do it now, before the cleanup loop.
136
- */
137
- memory_region_transaction_commit();
138
-
139
- for (i = 0; i < nvqs; i++) {
140
- virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i);
141
- }
142
-
143
qemu_bh_cancel(s->bh);
144
notify_guest_bh(s); /* final chance to notify guest */
145
146
/* Clean up guest notifier (irq) */
147
k->set_guest_notifiers(qbus->parent, nvqs, false);
148
149
- vblk->dataplane_started = false;
150
s->stopping = false;
151
}
37
--
152
--
38
2.21.0
153
2.40.1
39
154
40
155
diff view generated by jsdifflib