1
The following changes since commit 6157b0e19721aadb4c7fdcfe57b2924af6144b14:
1
The following changes since commit 887cba855bb6ff4775256f7968409281350b568c:
2
2
3
Merge remote-tracking branch 'remotes/vivier2/tags/linux-user-for-6.0-pull-=
3
configure: Fix cross-building for RISCV host (v5) (2023-07-11 17:56:09 +0100)
4
request' into staging (2021-03-14 17:47:49 +0000)
5
4
6
are available in the Git repository at:
5
are available in the Git repository at:
7
6
8
https://gitlab.com/stefanha/qemu.git tags/block-pull-request
7
https://gitlab.com/stefanha/qemu.git tags/block-pull-request
9
8
10
for you to fetch changes up to fb0b154c801e3447e505de420195fb7038695941:
9
for you to fetch changes up to 75dcb4d790bbe5327169fd72b185960ca58e2fa6:
11
10
12
virtio-blk: Respect discard granularity (2021-03-15 09:48:53 +0000)
11
virtio-blk: fix host notifier issues during dataplane start/stop (2023-07-12 15:20:32 -0400)
13
12
14
----------------------------------------------------------------
13
----------------------------------------------------------------
15
Pull request
14
Pull request
16
15
17
----------------------------------------------------------------
16
----------------------------------------------------------------
18
17
19
Akihiko Odaki (1):
18
Stefan Hajnoczi (1):
20
virtio-blk: Respect discard granularity
19
virtio-blk: fix host notifier issues during dataplane start/stop
21
20
22
include/hw/virtio/virtio-blk.h | 1 +
21
hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
23
hw/block/virtio-blk.c | 8 +++++++-
22
1 file changed, 38 insertions(+), 29 deletions(-)
24
hw/core/machine.c | 1 +
25
3 files changed, 9 insertions(+), 1 deletion(-)
26
23
27
--=20
24
--
28
2.29.2
25
2.40.1
29
diff view generated by jsdifflib
1
From: Akihiko Odaki <akihiko.odaki@gmail.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
Report the configured granularity for discard operation to the
7
The problem is that the dataplane start/stop code involves drain
4
guest. If this is not set use the block size.
8
operations, which call virtio_blk_drained_begin() and
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.
5
17
6
Since until now we have ignored the configured discard granularity
18
I would like to simplify ->ioeventfd_start/stop() to avoid interactions
7
and always reported the block size, let's add
19
with drain entirely, but couldn't find a way to do that. Instead, this
8
'report-discard-granularity' property and disable it for older
20
patch accepts the fragile nature of the code and reorders it so that
9
machine types to avoid migration issues.
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
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
28
This patch fixes the 100% CPU consumption in the main loop thread and
12
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
29
correctly moves host notifier processing to the IOThread.
30
31
Fixes: 1665d9326fd2 ("virtio-blk: implement BlockDevOps->drained_begin()")
32
Reported-by: Lukáš Doktor <ldoktor@redhat.com>
13
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
33
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
14
Message-Id: <20210225001239.47046-1-akihiko.odaki@gmail.com>
34
Tested-by: Lukas Doktor <ldoktor@redhat.com>
35
Message-id: 20230704151527.193586-1-stefanha@redhat.com
36
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
15
---
37
---
16
include/hw/virtio/virtio-blk.h | 1 +
38
hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
17
hw/block/virtio-blk.c | 8 +++++++-
39
1 file changed, 38 insertions(+), 29 deletions(-)
18
hw/core/machine.c | 1 +
19
3 files changed, 9 insertions(+), 1 deletion(-)
20
40
21
diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h
41
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
22
index XXXXXXX..XXXXXXX 100644
42
index XXXXXXX..XXXXXXX 100644
23
--- a/include/hw/virtio/virtio-blk.h
43
--- a/hw/block/dataplane/virtio-blk.c
24
+++ b/include/hw/virtio/virtio-blk.h
44
+++ b/hw/block/dataplane/virtio-blk.c
25
@@ -XXX,XX +XXX,XX @@ struct VirtIOBlkConf
45
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
26
uint16_t num_queues;
46
27
uint16_t queue_size;
47
memory_region_transaction_commit();
28
bool seg_max_adjust;
48
29
+ bool report_discard_granularity;
49
- /*
30
uint32_t max_discard_sectors;
50
- * These fields are visible to the IOThread so we rely on implicit barriers
31
uint32_t max_write_zeroes_sectors;
51
- * in aio_context_acquire() on the write side and aio_notify_accept() on
32
bool x_enable_wce_if_config_wce;
52
- * the read side.
33
diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c
53
- */
34
index XXXXXXX..XXXXXXX 100644
54
- s->starting = false;
35
--- a/hw/block/virtio-blk.c
55
- vblk->dataplane_started = true;
36
+++ b/hw/block/virtio-blk.c
56
trace_virtio_blk_data_plane_start(s);
37
@@ -XXX,XX +XXX,XX @@ static void virtio_blk_update_config(VirtIODevice *vdev, uint8_t *config)
57
38
blkcfg.wce = blk_enable_write_cache(s->blk);
58
old_context = blk_get_aio_context(s->conf->conf.blk);
39
virtio_stw_p(vdev, &blkcfg.num_queues, s->conf.num_queues);
59
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
40
if (virtio_has_feature(s->host_features, VIRTIO_BLK_F_DISCARD)) {
60
event_notifier_set(virtio_queue_get_host_notifier(vq));
41
+ uint32_t discard_granularity = conf->discard_granularity;
61
}
42
+ if (discard_granularity == -1 || !s->conf.report_discard_granularity) {
62
43
+ discard_granularity = blk_size;
63
+ /*
44
+ }
64
+ * These fields must be visible to the IOThread when it processes the
45
virtio_stl_p(vdev, &blkcfg.max_discard_sectors,
65
+ * virtqueue, otherwise it will think dataplane has not started yet.
46
s->conf.max_discard_sectors);
66
+ *
47
virtio_stl_p(vdev, &blkcfg.discard_sector_alignment,
67
+ * Make sure ->dataplane_started is false when blk_set_aio_context() is
48
- blk_size >> BDRV_SECTOR_BITS);
68
+ * called above so that draining does not cause the host notifier to be
49
+ discard_granularity >> BDRV_SECTOR_BITS);
69
+ * detached/attached prematurely.
50
/*
70
+ */
51
* We support only one segment per request since multiple segments
71
+ s->starting = false;
52
* are not widely used and there are no userspace APIs that allow
72
+ vblk->dataplane_started = true;
53
@@ -XXX,XX +XXX,XX @@ static Property virtio_blk_properties[] = {
73
+ smp_wmb(); /* paired with aio_notify_accept() on the read side */
54
IOThread *),
74
+
55
DEFINE_PROP_BIT64("discard", VirtIOBlock, host_features,
75
/* Get this show started by hooking up our callbacks */
56
VIRTIO_BLK_F_DISCARD, true),
76
if (!blk_in_drain(s->conf->conf.blk)) {
57
+ DEFINE_PROP_BOOL("report-discard-granularity", VirtIOBlock,
77
aio_context_acquire(s->ctx);
58
+ conf.report_discard_granularity, true),
78
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
59
DEFINE_PROP_BIT64("write-zeroes", VirtIOBlock, host_features,
79
fail_guest_notifiers:
60
VIRTIO_BLK_F_WRITE_ZEROES, true),
80
vblk->dataplane_disabled = true;
61
DEFINE_PROP_UINT32("max-discard-sectors", VirtIOBlock,
81
s->starting = false;
62
diff --git a/hw/core/machine.c b/hw/core/machine.c
82
- vblk->dataplane_started = true;
63
index XXXXXXX..XXXXXXX 100644
83
return -ENOSYS;
64
--- a/hw/core/machine.c
84
}
65
+++ b/hw/core/machine.c
85
66
@@ -XXX,XX +XXX,XX @@
86
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
67
GlobalProperty hw_compat_5_2[] = {
87
aio_wait_bh_oneshot(s->ctx, virtio_blk_data_plane_stop_bh, s);
68
{ "ICH9-LPC", "smm-compat", "on"},
88
}
69
{ "PIIX4_PM", "smm-compat", "on"},
89
70
+ { "virtio-blk-device", "report-discard-granularity", "off" },
90
+ /*
71
};
91
+ * Batch all the host notifiers in a single transaction to avoid
72
const size_t hw_compat_5_2_len = G_N_ELEMENTS(hw_compat_5_2);
92
+ * quadratic time complexity in address_space_update_ioeventfds().
73
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
}
74
--
152
--
75
2.29.2
153
2.40.1
76
154
155
diff view generated by jsdifflib