1
The following changes since commit ca61fa4b803e5d0abaf6f1ceb690f23bb78a4def:
1
The following changes since commit 887cba855bb6ff4775256f7968409281350b568c:
2
2
3
Merge remote-tracking branch 'remotes/quic/tags/pull-hex-20211006' into staging (2021-10-06 12:11:14 -0700)
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://gitlab.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 1cc7eada97914f090125e588497986f6f7900514:
9
for you to fetch changes up to 75dcb4d790bbe5327169fd72b185960ca58e2fa6:
10
10
11
iothread: use IOThreadParamInfo in iothread_[set|get]_param() (2021-10-07 15:29:50 +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
Stefano Garzarella (2):
18
Stefan Hajnoczi (1):
19
iothread: rename PollParamInfo to IOThreadParamInfo
19
virtio-blk: fix host notifier issues during dataplane start/stop
20
iothread: use IOThreadParamInfo in iothread_[set|get]_param()
21
20
22
iothread.c | 28 +++++++++++++++-------------
21
hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
23
1 file changed, 15 insertions(+), 13 deletions(-)
22
1 file changed, 38 insertions(+), 29 deletions(-)
24
23
25
--
24
--
26
2.31.1
25
2.40.1
27
28
29
diff view generated by jsdifflib
Deleted patch
1
From: Stefano Garzarella <sgarzare@redhat.com>
2
1
3
Commit 1793ad0247 ("iothread: add aio-max-batch parameter") added
4
a new parameter (aio-max-batch) to IOThread and used PollParamInfo
5
structure to handle it.
6
7
Since it is not a parameter of the polling mechanism, we rename the
8
structure to a more generic IOThreadParamInfo.
9
10
Suggested-by: Kevin Wolf <kwolf@redhat.com>
11
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
12
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
13
Message-id: 20210727145936.147032-2-sgarzare@redhat.com
14
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
15
---
16
iothread.c | 14 +++++++-------
17
1 file changed, 7 insertions(+), 7 deletions(-)
18
19
diff --git a/iothread.c b/iothread.c
20
index XXXXXXX..XXXXXXX 100644
21
--- a/iothread.c
22
+++ b/iothread.c
23
@@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp)
24
typedef struct {
25
const char *name;
26
ptrdiff_t offset; /* field's byte offset in IOThread struct */
27
-} PollParamInfo;
28
+} IOThreadParamInfo;
29
30
-static PollParamInfo poll_max_ns_info = {
31
+static IOThreadParamInfo poll_max_ns_info = {
32
"poll-max-ns", offsetof(IOThread, poll_max_ns),
33
};
34
-static PollParamInfo poll_grow_info = {
35
+static IOThreadParamInfo poll_grow_info = {
36
"poll-grow", offsetof(IOThread, poll_grow),
37
};
38
-static PollParamInfo poll_shrink_info = {
39
+static IOThreadParamInfo poll_shrink_info = {
40
"poll-shrink", offsetof(IOThread, poll_shrink),
41
};
42
-static PollParamInfo aio_max_batch_info = {
43
+static IOThreadParamInfo aio_max_batch_info = {
44
"aio-max-batch", offsetof(IOThread, aio_max_batch),
45
};
46
47
@@ -XXX,XX +XXX,XX @@ static void iothread_get_param(Object *obj, Visitor *v,
48
const char *name, void *opaque, Error **errp)
49
{
50
IOThread *iothread = IOTHREAD(obj);
51
- PollParamInfo *info = opaque;
52
+ IOThreadParamInfo *info = opaque;
53
int64_t *field = (void *)iothread + info->offset;
54
55
visit_type_int64(v, name, field, errp);
56
@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
57
const char *name, void *opaque, Error **errp)
58
{
59
IOThread *iothread = IOTHREAD(obj);
60
- PollParamInfo *info = opaque;
61
+ IOThreadParamInfo *info = opaque;
62
int64_t *field = (void *)iothread + info->offset;
63
int64_t value;
64
65
--
66
2.31.1
67
68
diff view generated by jsdifflib
1
From: Stefano Garzarella <sgarzare@redhat.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
Commit 0445409d74 ("iothread: generalize
7
The problem is that the dataplane start/stop code involves drain
4
iothread_set_param/iothread_get_param") moved common code to set and
8
operations, which call virtio_blk_drained_begin() and
5
get IOThread parameters in two new functions.
9
virtio_blk_drained_end() at points where the host notifier is not
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.
6
17
7
These functions are called inside callbacks, so we don't need to use an
18
I would like to simplify ->ioeventfd_start/stop() to avoid interactions
8
opaque pointer. Let's replace `void *opaque` parameter with
19
with drain entirely, but couldn't find a way to do that. Instead, this
9
`IOThreadParamInfo *info`.
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
Suggested-by: Kevin Wolf <kwolf@redhat.com>
28
This patch fixes the 100% CPU consumption in the main loop thread and
12
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
29
correctly moves host notifier processing to the IOThread.
13
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
30
14
Message-id: 20210727145936.147032-3-sgarzare@redhat.com
31
Fixes: 1665d9326fd2 ("virtio-blk: implement BlockDevOps->drained_begin()")
32
Reported-by: Lukáš Doktor <ldoktor@redhat.com>
33
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
34
Tested-by: Lukas Doktor <ldoktor@redhat.com>
35
Message-id: 20230704151527.193586-1-stefanha@redhat.com
15
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
36
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
16
---
37
---
17
iothread.c | 18 ++++++++++--------
38
hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
18
1 file changed, 10 insertions(+), 8 deletions(-)
39
1 file changed, 38 insertions(+), 29 deletions(-)
19
40
20
diff --git a/iothread.c b/iothread.c
41
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
21
index XXXXXXX..XXXXXXX 100644
42
index XXXXXXX..XXXXXXX 100644
22
--- a/iothread.c
43
--- a/hw/block/dataplane/virtio-blk.c
23
+++ b/iothread.c
44
+++ b/hw/block/dataplane/virtio-blk.c
24
@@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo aio_max_batch_info = {
45
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
25
};
46
26
47
memory_region_transaction_commit();
27
static void iothread_get_param(Object *obj, Visitor *v,
48
28
- const char *name, void *opaque, Error **errp)
49
- /*
29
+ const char *name, IOThreadParamInfo *info, Error **errp)
50
- * These fields are visible to the IOThread so we rely on implicit barriers
30
{
51
- * in aio_context_acquire() on the write side and aio_notify_accept() on
31
IOThread *iothread = IOTHREAD(obj);
52
- * the read side.
32
- IOThreadParamInfo *info = opaque;
53
- */
33
int64_t *field = (void *)iothread + info->offset;
54
- s->starting = false;
34
55
- vblk->dataplane_started = true;
35
visit_type_int64(v, name, field, errp);
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));
61
}
62
63
+ /*
64
+ * These fields must be visible to the IOThread when it processes the
65
+ * virtqueue, otherwise it will think dataplane has not started yet.
66
+ *
67
+ * Make sure ->dataplane_started is false when blk_set_aio_context() is
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;
36
}
84
}
37
85
38
static bool iothread_set_param(Object *obj, Visitor *v,
86
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
39
- const char *name, void *opaque, Error **errp)
87
aio_wait_bh_oneshot(s->ctx, virtio_blk_data_plane_stop_bh, s);
40
+ const char *name, IOThreadParamInfo *info, Error **errp)
88
}
41
{
89
42
IOThread *iothread = IOTHREAD(obj);
90
+ /*
43
- IOThreadParamInfo *info = opaque;
91
+ * Batch all the host notifiers in a single transaction to avoid
44
int64_t *field = (void *)iothread + info->offset;
92
+ * quadratic time complexity in address_space_update_ioeventfds().
45
int64_t value;
93
+ */
46
94
+ memory_region_transaction_begin();
47
@@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v,
95
+
48
static void iothread_get_poll_param(Object *obj, Visitor *v,
96
+ for (i = 0; i < nvqs; i++) {
49
const char *name, void *opaque, Error **errp)
97
+ virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
50
{
98
+ }
51
+ IOThreadParamInfo *info = opaque;
99
+
52
100
+ /*
53
- iothread_get_param(obj, v, name, opaque, errp);
101
+ * The transaction expects the ioeventfds to be open when it
54
+ iothread_get_param(obj, v, name, info, errp);
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;
55
}
151
}
56
57
static void iothread_set_poll_param(Object *obj, Visitor *v,
58
const char *name, void *opaque, Error **errp)
59
{
60
IOThread *iothread = IOTHREAD(obj);
61
+ IOThreadParamInfo *info = opaque;
62
63
- if (!iothread_set_param(obj, v, name, opaque, errp)) {
64
+ if (!iothread_set_param(obj, v, name, info, errp)) {
65
return;
66
}
67
68
@@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v,
69
static void iothread_get_aio_param(Object *obj, Visitor *v,
70
const char *name, void *opaque, Error **errp)
71
{
72
+ IOThreadParamInfo *info = opaque;
73
74
- iothread_get_param(obj, v, name, opaque, errp);
75
+ iothread_get_param(obj, v, name, info, errp);
76
}
77
78
static void iothread_set_aio_param(Object *obj, Visitor *v,
79
const char *name, void *opaque, Error **errp)
80
{
81
IOThread *iothread = IOTHREAD(obj);
82
+ IOThreadParamInfo *info = opaque;
83
84
- if (!iothread_set_param(obj, v, name, opaque, errp)) {
85
+ if (!iothread_set_param(obj, v, name, info, errp)) {
86
return;
87
}
88
89
--
152
--
90
2.31.1
153
2.40.1
91
154
92
155
diff view generated by jsdifflib