[PULL 0/1] Block patches
[PULL 0/1] Block patches
1
The following changes since commit 887cba855bb6ff4775256f7968409281350b568c:
1
The following changes since commit da1034094d375afe9e3d8ec8980550ea0f06f7e0:
2
2
3
configure: Fix cross-building for RISCV host (v5) (2023-07-11 17:56:09 +0100)
3
Merge tag 'for-upstream' of https://gitlab.com/bonzini/qemu into staging (2023-10-03 07:43:44 -0400)
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 75dcb4d790bbe5327169fd72b185960ca58e2fa6:
9
for you to fetch changes up to 9afa888ce0f816d0f2cfc95eebe4f49244c518af:
10
10
11
virtio-blk: fix host notifier issues during dataplane start/stop (2023-07-12 15:20:32 -0400)
11
osdep: set _FORTIFY_SOURCE=2 when optimization is enabled (2023-10-04 09:52:06 -0400)
12
12
13
----------------------------------------------------------------
13
----------------------------------------------------------------
14
Pull request
14
Pull request
15
15
16
----------------------------------------------------------------
16
----------------------------------------------------------------
17
17
18
Stefan Hajnoczi (1):
18
Daniel P. Berrangé (1):
19
virtio-blk: fix host notifier issues during dataplane start/stop
19
osdep: set _FORTIFY_SOURCE=2 when optimization is enabled
20
20
21
hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
21
meson.build | 10 ----------
22
1 file changed, 38 insertions(+), 29 deletions(-)
22
include/qemu/osdep.h | 4 ++++
23
util/coroutine-sigaltstack.c | 4 ++--
24
util/coroutine-ucontext.c | 4 ++--
25
4 files changed, 8 insertions(+), 14 deletions(-)
23
26
24
--
27
--
25
2.40.1
28
2.41.0
29
30
diff view generated by jsdifflib
[PULL 1/1] virtio-blk: fix host notifier issues during dataplane start/stop
[PULL 1/1] osdep: set _FORTIFY_SOURCE=2 when optimization is enabled
1
The main loop thread can consume 100% CPU when using --device
1
From: Daniel P. Berrangé <berrange@redhat.com>
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.
6
2
7
The problem is that the dataplane start/stop code involves drain
3
Currently we set _FORTIFY_SOURCE=2 as a compiler argument when the
8
operations, which call virtio_blk_drained_begin() and
4
meson 'optimization' setting is non-zero, the compiler is GCC and
9
virtio_blk_drained_end() at points where the host notifier is not
5
the target is Linux.
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.
17
6
18
I would like to simplify ->ioeventfd_start/stop() to avoid interactions
7
While the default QEMU optimization level is 2, user could override
19
with drain entirely, but couldn't find a way to do that. Instead, this
8
this by setting CFLAGS="-O0" or --extra-cflags="-O0" when running
20
patch accepts the fragile nature of the code and reorders it so that
9
configure and this won't be reflected in the meson 'optimization'
21
vblk->dataplane_started is false during drain operations. This way the
10
setting. As a result we try to enable _FORTIFY_SOURCE=2 and then the
22
virtio_blk_drained_begin() and virtio_blk_drained_end() calls don't
11
user gets compile errors as it only works with optimization.
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.
27
12
28
This patch fixes the 100% CPU consumption in the main loop thread and
13
Rather than trying to improve detection in meson, it is simpler to
29
correctly moves host notifier processing to the IOThread.
14
just check the __OPTIMIZE__ define from osdep.h.
30
15
31
Fixes: 1665d9326fd2 ("virtio-blk: implement BlockDevOps->drained_begin()")
16
The comment about being incompatible with clang appears to be
32
Reported-by: Lukáš Doktor <ldoktor@redhat.com>
17
outdated, as compilation works fine without excluding clang.
33
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
18
34
Tested-by: Lukas Doktor <ldoktor@redhat.com>
19
In the coroutine code we must set _FORTIFY_SOURCE=0 to stop the
35
Message-id: 20230704151527.193586-1-stefanha@redhat.com
20
logic in osdep.h then enabling it.
21
22
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
23
Message-id: 20231003091549.223020-1-berrange@redhat.com
36
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
24
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
37
---
25
---
38
hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++--------------
26
meson.build | 10 ----------
39
1 file changed, 38 insertions(+), 29 deletions(-)
27
include/qemu/osdep.h | 4 ++++
28
util/coroutine-sigaltstack.c | 4 ++--
29
util/coroutine-ucontext.c | 4 ++--
30
4 files changed, 8 insertions(+), 14 deletions(-)
40
31
41
diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c
32
diff --git a/meson.build b/meson.build
42
index XXXXXXX..XXXXXXX 100644
33
index XXXXXXX..XXXXXXX 100644
43
--- a/hw/block/dataplane/virtio-blk.c
34
--- a/meson.build
44
+++ b/hw/block/dataplane/virtio-blk.c
35
+++ b/meson.build
45
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
36
@@ -XXX,XX +XXX,XX @@ if 'cpp' in all_languages
46
37
qemu_cxxflags = ['-D__STDC_LIMIT_MACROS', '-D__STDC_CONSTANT_MACROS', '-D__STDC_FORMAT_MACROS'] + qemu_cflags
47
memory_region_transaction_commit();
38
endif
48
39
49
- /*
40
-# clang does not support glibc + FORTIFY_SOURCE (is it still true?)
50
- * These fields are visible to the IOThread so we rely on implicit barriers
41
-if get_option('optimization') != '0' and targetos == 'linux'
51
- * in aio_context_acquire() on the write side and aio_notify_accept() on
42
- if cc.get_id() == 'gcc'
52
- * the read side.
43
- qemu_cflags += ['-U_FORTIFY_SOURCE', '-D_FORTIFY_SOURCE=2']
53
- */
44
- endif
54
- s->starting = false;
45
- if 'cpp' in all_languages and cxx.get_id() == 'gcc'
55
- vblk->dataplane_started = true;
46
- qemu_cxxflags += ['-U_FORTIFY_SOURCE', '-D_FORTIFY_SOURCE=2']
56
trace_virtio_blk_data_plane_start(s);
47
- endif
57
48
-endif
58
old_context = blk_get_aio_context(s->conf->conf.blk);
49
-
59
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
50
add_project_arguments(qemu_cflags, native: false, language: 'c')
60
event_notifier_set(virtio_queue_get_host_notifier(vq));
51
add_project_arguments(cc.get_supported_arguments(warn_flags), native: false, language: 'c')
61
}
52
if 'cpp' in all_languages
62
53
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
63
+ /*
54
index XXXXXXX..XXXXXXX 100644
64
+ * These fields must be visible to the IOThread when it processes the
55
--- a/include/qemu/osdep.h
65
+ * virtqueue, otherwise it will think dataplane has not started yet.
56
+++ b/include/qemu/osdep.h
66
+ *
57
@@ -XXX,XX +XXX,XX @@
67
+ * Make sure ->dataplane_started is false when blk_set_aio_context() is
58
#ifndef QEMU_OSDEP_H
68
+ * called above so that draining does not cause the host notifier to be
59
#define QEMU_OSDEP_H
69
+ * detached/attached prematurely.
60
70
+ */
61
+#if !defined _FORTIFY_SOURCE && defined __OPTIMIZE__ && __OPTIMIZE__ && defined __linux__
71
+ s->starting = false;
62
+# define _FORTIFY_SOURCE 2
72
+ vblk->dataplane_started = true;
63
+#endif
73
+ smp_wmb(); /* paired with aio_notify_accept() on the read side */
74
+
64
+
75
/* Get this show started by hooking up our callbacks */
65
#include "config-host.h"
76
if (!blk_in_drain(s->conf->conf.blk)) {
66
#ifdef NEED_CPU_H
77
aio_context_acquire(s->ctx);
67
#include CONFIG_TARGET
78
@@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev)
68
diff --git a/util/coroutine-sigaltstack.c b/util/coroutine-sigaltstack.c
79
fail_guest_notifiers:
69
index XXXXXXX..XXXXXXX 100644
80
vblk->dataplane_disabled = true;
70
--- a/util/coroutine-sigaltstack.c
81
s->starting = false;
71
+++ b/util/coroutine-sigaltstack.c
82
- vblk->dataplane_started = true;
72
@@ -XXX,XX +XXX,XX @@
83
return -ENOSYS;
73
*/
84
}
74
85
75
/* XXX Is there a nicer way to disable glibc's stack check for longjmp? */
86
@@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev)
76
-#ifdef _FORTIFY_SOURCE
87
aio_wait_bh_oneshot(s->ctx, virtio_blk_data_plane_stop_bh, s);
77
#undef _FORTIFY_SOURCE
88
}
78
-#endif
89
79
+#define _FORTIFY_SOURCE 0
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
+
80
+
96
+ for (i = 0; i < nvqs; i++) {
81
#include "qemu/osdep.h"
97
+ virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false);
82
#include <pthread.h>
98
+ }
83
#include "qemu/coroutine_int.h"
84
diff --git a/util/coroutine-ucontext.c b/util/coroutine-ucontext.c
85
index XXXXXXX..XXXXXXX 100644
86
--- a/util/coroutine-ucontext.c
87
+++ b/util/coroutine-ucontext.c
88
@@ -XXX,XX +XXX,XX @@
89
*/
90
91
/* XXX Is there a nicer way to disable glibc's stack check for longjmp? */
92
-#ifdef _FORTIFY_SOURCE
93
#undef _FORTIFY_SOURCE
94
-#endif
95
+#define _FORTIFY_SOURCE 0
99
+
96
+
100
+ /*
97
#include "qemu/osdep.h"
101
+ * The transaction expects the ioeventfds to be open when it
98
#include <ucontext.h>
102
+ * commits. Do it now, before the cleanup loop.
99
#include "qemu/coroutine_int.h"
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
}
152
--
100
--
153
2.40.1
101
2.41.0
154
102
155
103
diff view generated by jsdifflib

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.