1 | The following changes since commit 560009f2c8b57b7cdd31a5693ea86ab369382f49: | 1 | The following changes since commit 9cf289af47bcfae5c75de37d8e5d6fd23705322c: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging (2019-10-07 15:40:53 +0100) | 3 | Merge tag 'qga-pull-request' of gitlab.com:marcandre.lureau/qemu into staging (2022-05-04 03:42:49 -0700) |
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 4d804b5305ffb4d5fa414c38d4f1bdfb987c8d0b: | 9 | for you to fetch changes up to bef2e050d6a7feb865854c65570c496ac5a8cf53: |
10 | 10 | ||
11 | iotests/262: Switch source/dest VM launch order (2019-10-08 14:28:25 +0100) | 11 | util/event-loop-base: Introduce options to set the thread pool size (2022-05-04 17:02:19 +0100) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Pull request | 14 | Pull request |
15 | 15 | ||
16 | This pull request also contains the two commits from the previous pull request | 16 | Add new thread-pool-min/thread-pool-max parameters to control the thread pool |
17 | that was dropped due to a mingw compilation error. The compilation should now | 17 | used for async I/O. |
18 | be fixed. | ||
19 | 18 | ||
20 | ---------------------------------------------------------------- | 19 | ---------------------------------------------------------------- |
21 | 20 | ||
22 | Max Reitz (2): | 21 | Nicolas Saenz Julienne (3): |
23 | block: Skip COR for inactive nodes | 22 | Introduce event-loop-base abstract class |
24 | iotests/262: Switch source/dest VM launch order | 23 | util/main-loop: Introduce the main loop into QOM |
24 | util/event-loop-base: Introduce options to set the thread pool size | ||
25 | 25 | ||
26 | Sergio Lopez (1): | 26 | qapi/qom.json | 43 ++++++++-- |
27 | virtio-blk: schedule virtio_notify_config to run on main context | 27 | meson.build | 26 +++--- |
28 | 28 | include/block/aio.h | 10 +++ | |
29 | Vladimir Sementsov-Ogievskiy (1): | 29 | include/block/thread-pool.h | 3 + |
30 | util/ioc.c: try to reassure Coverity about qemu_iovec_init_extended | 30 | include/qemu/main-loop.h | 10 +++ |
31 | 31 | include/sysemu/event-loop-base.h | 41 +++++++++ | |
32 | block/io.c | 41 +++++++++++++++++++++++++------------- | 32 | include/sysemu/iothread.h | 6 +- |
33 | hw/block/virtio-blk.c | 16 ++++++++++++++- | 33 | event-loop-base.c | 140 +++++++++++++++++++++++++++++++ |
34 | util/iov.c | 5 +++-- | 34 | iothread.c | 68 +++++---------- |
35 | tests/qemu-iotests/262 | 12 +++++------ | 35 | util/aio-posix.c | 1 + |
36 | tests/qemu-iotests/262.out | 6 +++--- | 36 | util/async.c | 20 +++++ |
37 | 5 files changed, 54 insertions(+), 26 deletions(-) | 37 | util/main-loop.c | 65 ++++++++++++++ |
38 | util/thread-pool.c | 55 +++++++++++- | ||
39 | 13 files changed, 419 insertions(+), 69 deletions(-) | ||
40 | create mode 100644 include/sysemu/event-loop-base.h | ||
41 | create mode 100644 event-loop-base.c | ||
38 | 42 | ||
39 | -- | 43 | -- |
40 | 2.21.0 | 44 | 2.35.1 |
41 | |||
42 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
2 | 1 | ||
3 | Make it more obvious, that filling qiov corresponds to qiov allocation, | ||
4 | which in turn corresponds to total_niov calculation, based on mid_niov | ||
5 | (not mid_len). Still add an assertion to show that there should be no | ||
6 | difference. | ||
7 | |||
8 | [Added mingw "error: 'mid_iov' may be used uninitialized in this | ||
9 | function" compiler error fix suggested by Vladimir. | ||
10 | --Stefan] | ||
11 | |||
12 | Reported-by: Coverity (CID 1405302) | ||
13 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
14 | Message-id: 20190910090310.14032-1-vsementsov@virtuozzo.com | ||
15 | Suggested-by: Peter Maydell <peter.maydell@linaro.org> | ||
16 | Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
17 | Message-Id: <20190910090310.14032-1-vsementsov@virtuozzo.com> | ||
18 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
19 | |||
20 | fixup! util/ioc.c: try to reassure Coverity about qemu_iovec_init_extended | ||
21 | --- | ||
22 | util/iov.c | 5 +++-- | ||
23 | 1 file changed, 3 insertions(+), 2 deletions(-) | ||
24 | |||
25 | diff --git a/util/iov.c b/util/iov.c | ||
26 | index XXXXXXX..XXXXXXX 100644 | ||
27 | --- a/util/iov.c | ||
28 | +++ b/util/iov.c | ||
29 | @@ -XXX,XX +XXX,XX @@ void qemu_iovec_init_extended( | ||
30 | { | ||
31 | size_t mid_head, mid_tail; | ||
32 | int total_niov, mid_niov = 0; | ||
33 | - struct iovec *p, *mid_iov; | ||
34 | + struct iovec *p, *mid_iov = NULL; | ||
35 | |||
36 | if (mid_len) { | ||
37 | mid_iov = qiov_slice(mid_qiov, mid_offset, mid_len, | ||
38 | @@ -XXX,XX +XXX,XX @@ void qemu_iovec_init_extended( | ||
39 | p++; | ||
40 | } | ||
41 | |||
42 | - if (mid_len) { | ||
43 | + assert(!mid_niov == !mid_len); | ||
44 | + if (mid_niov) { | ||
45 | memcpy(p, mid_iov, mid_niov * sizeof(*p)); | ||
46 | p[0].iov_base = (uint8_t *)p[0].iov_base + mid_head; | ||
47 | p[0].iov_len -= mid_head; | ||
48 | -- | ||
49 | 2.21.0 | ||
50 | |||
51 | diff view generated by jsdifflib |
1 | From: Max Reitz <mreitz@redhat.com> | 1 | From: Nicolas Saenz Julienne <nsaenzju@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Launching the destination VM before the source VM gives us a regression | 3 | Introduce the 'event-loop-base' abstract class, it'll hold the |
4 | test for HEAD^: | 4 | properties common to all event loops and provide the necessary hooks for |
5 | 5 | their creation and maintenance. Then have iothread inherit from it. | |
6 | The guest device causes a read from the disk image through | 6 | |
7 | guess_disk_lchs(). This will not work if the first sector (containing | 7 | EventLoopBaseClass is defined as user creatable and provides a hook for |
8 | the partition table) is yet unallocated, we use COR, and the node is | 8 | its children to attach themselves to the user creatable class 'complete' |
9 | inactive. | 9 | function. It also provides an update_params() callback to propagate |
10 | 10 | property changes onto its children. | |
11 | By launching the source VM before the destination, however, the COR | 11 | |
12 | filter on the source will allocate that area in the image shared between | 12 | The new 'event-loop-base' class will live in the root directory. It is |
13 | both VMs, thus the problem will not become apparent. | 13 | built on its own using the 'link_whole' option (there are no direct |
14 | 14 | function dependencies between the class and its children, it all happens | |
15 | Switching the launch order causes the sector to still be unallocated | 15 | trough 'constructor' magic). And also imposes new compilation |
16 | when guess_disk_lchs() runs on the inactive node in the destination VM, | 16 | dependencies: |
17 | and thus we get our test case. | 17 | |
18 | 18 | qom <- event-loop-base <- blockdev (iothread.c) | |
19 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 19 | |
20 | Reviewed-by: Eric Blake <eblake@redhat.com> | 20 | And in subsequent patches: |
21 | Message-id: 20191001174827.11081-3-mreitz@redhat.com | 21 | |
22 | Message-Id: <20191001174827.11081-3-mreitz@redhat.com> | 22 | qom <- event-loop-base <- qemuutil (util/main-loop.c) |
23 | |||
24 | All this forced some amount of reordering in meson.build: | ||
25 | |||
26 | - Moved qom build definition before qemuutil. Doing it the other way | ||
27 | around (i.e. moving qemuutil after qom) isn't possible as a lot of | ||
28 | core libraries that live in between the two depend on it. | ||
29 | |||
30 | - Process the 'hw' subdir earlier, as it introduces files into the | ||
31 | 'qom' source set. | ||
32 | |||
33 | No functional changes intended. | ||
34 | |||
35 | Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com> | ||
36 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
37 | Acked-by: Markus Armbruster <armbru@redhat.com> | ||
38 | Message-id: 20220425075723.20019-2-nsaenzju@redhat.com | ||
23 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 39 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
24 | --- | 40 | --- |
25 | tests/qemu-iotests/262 | 12 ++++++------ | 41 | qapi/qom.json | 22 +++++-- |
26 | tests/qemu-iotests/262.out | 6 +++--- | 42 | meson.build | 23 ++++--- |
27 | 2 files changed, 9 insertions(+), 9 deletions(-) | 43 | include/sysemu/event-loop-base.h | 36 +++++++++++ |
28 | 44 | include/sysemu/iothread.h | 6 +- | |
29 | diff --git a/tests/qemu-iotests/262 b/tests/qemu-iotests/262 | 45 | event-loop-base.c | 104 +++++++++++++++++++++++++++++++ |
30 | index XXXXXXX..XXXXXXX 100755 | 46 | iothread.c | 65 ++++++------------- |
31 | --- a/tests/qemu-iotests/262 | 47 | 6 files changed, 192 insertions(+), 64 deletions(-) |
32 | +++ b/tests/qemu-iotests/262 | 48 | create mode 100644 include/sysemu/event-loop-base.h |
33 | @@ -XXX,XX +XXX,XX @@ with iotests.FilePath('img') as img_path, \ | 49 | create mode 100644 event-loop-base.c |
34 | 50 | ||
35 | os.mkfifo(fifo) | 51 | diff --git a/qapi/qom.json b/qapi/qom.json |
36 | |||
37 | - iotests.log('Launching source VM...') | ||
38 | - add_opts(vm_a) | ||
39 | - vm_a.launch() | ||
40 | - | ||
41 | - vm_a.enable_migration_events('A') | ||
42 | - | ||
43 | iotests.log('Launching destination VM...') | ||
44 | add_opts(vm_b) | ||
45 | vm_b.add_incoming("exec: cat '%s'" % (fifo)) | ||
46 | @@ -XXX,XX +XXX,XX @@ with iotests.FilePath('img') as img_path, \ | ||
47 | |||
48 | vm_b.enable_migration_events('B') | ||
49 | |||
50 | + iotests.log('Launching source VM...') | ||
51 | + add_opts(vm_a) | ||
52 | + vm_a.launch() | ||
53 | + | ||
54 | + vm_a.enable_migration_events('A') | ||
55 | + | ||
56 | iotests.log('Starting migration to B...') | ||
57 | iotests.log(vm_a.qmp('migrate', uri='exec:cat >%s' % (fifo))) | ||
58 | with iotests.Timeout(3, 'Migration does not complete'): | ||
59 | diff --git a/tests/qemu-iotests/262.out b/tests/qemu-iotests/262.out | ||
60 | index XXXXXXX..XXXXXXX 100644 | 52 | index XXXXXXX..XXXXXXX 100644 |
61 | --- a/tests/qemu-iotests/262.out | 53 | --- a/qapi/qom.json |
62 | +++ b/tests/qemu-iotests/262.out | 54 | +++ b/qapi/qom.json |
63 | @@ -XXX,XX +XXX,XX @@ | 55 | @@ -XXX,XX +XXX,XX @@ |
64 | -Launching source VM... | 56 | '*repeat': 'bool', |
65 | -Enabling migration QMP events on A... | 57 | '*grab-toggle': 'GrabToggleKeys' } } |
66 | -{"return": {}} | 58 | |
67 | Launching destination VM... | 59 | +## |
68 | Enabling migration QMP events on B... | 60 | +# @EventLoopBaseProperties: |
69 | {"return": {}} | 61 | +# |
70 | +Launching source VM... | 62 | +# Common properties for event loops |
71 | +Enabling migration QMP events on A... | 63 | +# |
72 | +{"return": {}} | 64 | +# @aio-max-batch: maximum number of requests in a batch for the AIO engine, |
73 | Starting migration to B... | 65 | +# 0 means that the engine will use its default. |
74 | {"return": {}} | 66 | +# (default: 0) |
75 | {"data": {"status": "setup"}, "event": "MIGRATION", "timestamp": {"microseconds": "USECS", "seconds": "SECS"}} | 67 | +# |
68 | +# Since: 7.1 | ||
69 | +## | ||
70 | +{ 'struct': 'EventLoopBaseProperties', | ||
71 | + 'data': { '*aio-max-batch': 'int' } } | ||
72 | + | ||
73 | ## | ||
74 | # @IothreadProperties: | ||
75 | # | ||
76 | @@ -XXX,XX +XXX,XX @@ | ||
77 | # algorithm detects it is spending too long polling without | ||
78 | # encountering events. 0 selects a default behaviour (default: 0) | ||
79 | # | ||
80 | -# @aio-max-batch: maximum number of requests in a batch for the AIO engine, | ||
81 | -# 0 means that the engine will use its default | ||
82 | -# (default:0, since 6.1) | ||
83 | +# The @aio-max-batch option is available since 6.1. | ||
84 | # | ||
85 | # Since: 2.0 | ||
86 | ## | ||
87 | { 'struct': 'IothreadProperties', | ||
88 | + 'base': 'EventLoopBaseProperties', | ||
89 | 'data': { '*poll-max-ns': 'int', | ||
90 | '*poll-grow': 'int', | ||
91 | - '*poll-shrink': 'int', | ||
92 | - '*aio-max-batch': 'int' } } | ||
93 | + '*poll-shrink': 'int' } } | ||
94 | |||
95 | ## | ||
96 | # @MemoryBackendProperties: | ||
97 | diff --git a/meson.build b/meson.build | ||
98 | index XXXXXXX..XXXXXXX 100644 | ||
99 | --- a/meson.build | ||
100 | +++ b/meson.build | ||
101 | @@ -XXX,XX +XXX,XX @@ subdir('qom') | ||
102 | subdir('authz') | ||
103 | subdir('crypto') | ||
104 | subdir('ui') | ||
105 | +subdir('hw') | ||
106 | |||
107 | |||
108 | if enable_modules | ||
109 | @@ -XXX,XX +XXX,XX @@ if enable_modules | ||
110 | modulecommon = declare_dependency(link_whole: libmodulecommon, compile_args: '-DBUILD_DSO') | ||
111 | endif | ||
112 | |||
113 | +qom_ss = qom_ss.apply(config_host, strict: false) | ||
114 | +libqom = static_library('qom', qom_ss.sources() + genh, | ||
115 | + dependencies: [qom_ss.dependencies()], | ||
116 | + name_suffix: 'fa') | ||
117 | +qom = declare_dependency(link_whole: libqom) | ||
118 | + | ||
119 | +event_loop_base = files('event-loop-base.c') | ||
120 | +event_loop_base = static_library('event-loop-base', sources: event_loop_base + genh, | ||
121 | + build_by_default: true) | ||
122 | +event_loop_base = declare_dependency(link_whole: event_loop_base, | ||
123 | + dependencies: [qom]) | ||
124 | + | ||
125 | stub_ss = stub_ss.apply(config_all, strict: false) | ||
126 | |||
127 | util_ss.add_all(trace_ss) | ||
128 | @@ -XXX,XX +XXX,XX @@ subdir('monitor') | ||
129 | subdir('net') | ||
130 | subdir('replay') | ||
131 | subdir('semihosting') | ||
132 | -subdir('hw') | ||
133 | subdir('tcg') | ||
134 | subdir('fpu') | ||
135 | subdir('accel') | ||
136 | @@ -XXX,XX +XXX,XX @@ qemu_syms = custom_target('qemu.syms', output: 'qemu.syms', | ||
137 | capture: true, | ||
138 | command: [undefsym, nm, '@INPUT@']) | ||
139 | |||
140 | -qom_ss = qom_ss.apply(config_host, strict: false) | ||
141 | -libqom = static_library('qom', qom_ss.sources() + genh, | ||
142 | - dependencies: [qom_ss.dependencies()], | ||
143 | - name_suffix: 'fa') | ||
144 | - | ||
145 | -qom = declare_dependency(link_whole: libqom) | ||
146 | - | ||
147 | authz_ss = authz_ss.apply(config_host, strict: false) | ||
148 | libauthz = static_library('authz', authz_ss.sources() + genh, | ||
149 | dependencies: [authz_ss.dependencies()], | ||
150 | @@ -XXX,XX +XXX,XX @@ libblockdev = static_library('blockdev', blockdev_ss.sources() + genh, | ||
151 | build_by_default: false) | ||
152 | |||
153 | blockdev = declare_dependency(link_whole: [libblockdev], | ||
154 | - dependencies: [block]) | ||
155 | + dependencies: [block, event_loop_base]) | ||
156 | |||
157 | qmp_ss = qmp_ss.apply(config_host, strict: false) | ||
158 | libqmp = static_library('qmp', qmp_ss.sources() + genh, | ||
159 | diff --git a/include/sysemu/event-loop-base.h b/include/sysemu/event-loop-base.h | ||
160 | new file mode 100644 | ||
161 | index XXXXXXX..XXXXXXX | ||
162 | --- /dev/null | ||
163 | +++ b/include/sysemu/event-loop-base.h | ||
164 | @@ -XXX,XX +XXX,XX @@ | ||
165 | +/* | ||
166 | + * QEMU event-loop backend | ||
167 | + * | ||
168 | + * Copyright (C) 2022 Red Hat Inc | ||
169 | + * | ||
170 | + * Authors: | ||
171 | + * Nicolas Saenz Julienne <nsaenzju@redhat.com> | ||
172 | + * | ||
173 | + * This work is licensed under the terms of the GNU GPL, version 2 or later. | ||
174 | + * See the COPYING file in the top-level directory. | ||
175 | + */ | ||
176 | +#ifndef QEMU_EVENT_LOOP_BASE_H | ||
177 | +#define QEMU_EVENT_LOOP_BASE_H | ||
178 | + | ||
179 | +#include "qom/object.h" | ||
180 | +#include "block/aio.h" | ||
181 | +#include "qemu/typedefs.h" | ||
182 | + | ||
183 | +#define TYPE_EVENT_LOOP_BASE "event-loop-base" | ||
184 | +OBJECT_DECLARE_TYPE(EventLoopBase, EventLoopBaseClass, | ||
185 | + EVENT_LOOP_BASE) | ||
186 | + | ||
187 | +struct EventLoopBaseClass { | ||
188 | + ObjectClass parent_class; | ||
189 | + | ||
190 | + void (*init)(EventLoopBase *base, Error **errp); | ||
191 | + void (*update_params)(EventLoopBase *base, Error **errp); | ||
192 | +}; | ||
193 | + | ||
194 | +struct EventLoopBase { | ||
195 | + Object parent; | ||
196 | + | ||
197 | + /* AioContext AIO engine parameters */ | ||
198 | + int64_t aio_max_batch; | ||
199 | +}; | ||
200 | +#endif | ||
201 | diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h | ||
202 | index XXXXXXX..XXXXXXX 100644 | ||
203 | --- a/include/sysemu/iothread.h | ||
204 | +++ b/include/sysemu/iothread.h | ||
205 | @@ -XXX,XX +XXX,XX @@ | ||
206 | #include "block/aio.h" | ||
207 | #include "qemu/thread.h" | ||
208 | #include "qom/object.h" | ||
209 | +#include "sysemu/event-loop-base.h" | ||
210 | |||
211 | #define TYPE_IOTHREAD "iothread" | ||
212 | |||
213 | struct IOThread { | ||
214 | - Object parent_obj; | ||
215 | + EventLoopBase parent_obj; | ||
216 | |||
217 | QemuThread thread; | ||
218 | AioContext *ctx; | ||
219 | @@ -XXX,XX +XXX,XX @@ struct IOThread { | ||
220 | int64_t poll_max_ns; | ||
221 | int64_t poll_grow; | ||
222 | int64_t poll_shrink; | ||
223 | - | ||
224 | - /* AioContext AIO engine parameters */ | ||
225 | - int64_t aio_max_batch; | ||
226 | }; | ||
227 | typedef struct IOThread IOThread; | ||
228 | |||
229 | diff --git a/event-loop-base.c b/event-loop-base.c | ||
230 | new file mode 100644 | ||
231 | index XXXXXXX..XXXXXXX | ||
232 | --- /dev/null | ||
233 | +++ b/event-loop-base.c | ||
234 | @@ -XXX,XX +XXX,XX @@ | ||
235 | +/* | ||
236 | + * QEMU event-loop base | ||
237 | + * | ||
238 | + * Copyright (C) 2022 Red Hat Inc | ||
239 | + * | ||
240 | + * Authors: | ||
241 | + * Stefan Hajnoczi <stefanha@redhat.com> | ||
242 | + * Nicolas Saenz Julienne <nsaenzju@redhat.com> | ||
243 | + * | ||
244 | + * This work is licensed under the terms of the GNU GPL, version 2 or later. | ||
245 | + * See the COPYING file in the top-level directory. | ||
246 | + */ | ||
247 | + | ||
248 | +#include "qemu/osdep.h" | ||
249 | +#include "qom/object_interfaces.h" | ||
250 | +#include "qapi/error.h" | ||
251 | +#include "sysemu/event-loop-base.h" | ||
252 | + | ||
253 | +typedef struct { | ||
254 | + const char *name; | ||
255 | + ptrdiff_t offset; /* field's byte offset in EventLoopBase struct */ | ||
256 | +} EventLoopBaseParamInfo; | ||
257 | + | ||
258 | +static EventLoopBaseParamInfo aio_max_batch_info = { | ||
259 | + "aio-max-batch", offsetof(EventLoopBase, aio_max_batch), | ||
260 | +}; | ||
261 | + | ||
262 | +static void event_loop_base_get_param(Object *obj, Visitor *v, | ||
263 | + const char *name, void *opaque, Error **errp) | ||
264 | +{ | ||
265 | + EventLoopBase *event_loop_base = EVENT_LOOP_BASE(obj); | ||
266 | + EventLoopBaseParamInfo *info = opaque; | ||
267 | + int64_t *field = (void *)event_loop_base + info->offset; | ||
268 | + | ||
269 | + visit_type_int64(v, name, field, errp); | ||
270 | +} | ||
271 | + | ||
272 | +static void event_loop_base_set_param(Object *obj, Visitor *v, | ||
273 | + const char *name, void *opaque, Error **errp) | ||
274 | +{ | ||
275 | + EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(obj); | ||
276 | + EventLoopBase *base = EVENT_LOOP_BASE(obj); | ||
277 | + EventLoopBaseParamInfo *info = opaque; | ||
278 | + int64_t *field = (void *)base + info->offset; | ||
279 | + int64_t value; | ||
280 | + | ||
281 | + if (!visit_type_int64(v, name, &value, errp)) { | ||
282 | + return; | ||
283 | + } | ||
284 | + | ||
285 | + if (value < 0) { | ||
286 | + error_setg(errp, "%s value must be in range [0, %" PRId64 "]", | ||
287 | + info->name, INT64_MAX); | ||
288 | + return; | ||
289 | + } | ||
290 | + | ||
291 | + *field = value; | ||
292 | + | ||
293 | + if (bc->update_params) { | ||
294 | + bc->update_params(base, errp); | ||
295 | + } | ||
296 | + | ||
297 | + return; | ||
298 | +} | ||
299 | + | ||
300 | +static void event_loop_base_complete(UserCreatable *uc, Error **errp) | ||
301 | +{ | ||
302 | + EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(uc); | ||
303 | + EventLoopBase *base = EVENT_LOOP_BASE(uc); | ||
304 | + | ||
305 | + if (bc->init) { | ||
306 | + bc->init(base, errp); | ||
307 | + } | ||
308 | +} | ||
309 | + | ||
310 | +static void event_loop_base_class_init(ObjectClass *klass, void *class_data) | ||
311 | +{ | ||
312 | + UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass); | ||
313 | + ucc->complete = event_loop_base_complete; | ||
314 | + | ||
315 | + object_class_property_add(klass, "aio-max-batch", "int", | ||
316 | + event_loop_base_get_param, | ||
317 | + event_loop_base_set_param, | ||
318 | + NULL, &aio_max_batch_info); | ||
319 | +} | ||
320 | + | ||
321 | +static const TypeInfo event_loop_base_info = { | ||
322 | + .name = TYPE_EVENT_LOOP_BASE, | ||
323 | + .parent = TYPE_OBJECT, | ||
324 | + .instance_size = sizeof(EventLoopBase), | ||
325 | + .class_size = sizeof(EventLoopBaseClass), | ||
326 | + .class_init = event_loop_base_class_init, | ||
327 | + .abstract = true, | ||
328 | + .interfaces = (InterfaceInfo[]) { | ||
329 | + { TYPE_USER_CREATABLE }, | ||
330 | + { } | ||
331 | + } | ||
332 | +}; | ||
333 | + | ||
334 | +static void register_types(void) | ||
335 | +{ | ||
336 | + type_register_static(&event_loop_base_info); | ||
337 | +} | ||
338 | +type_init(register_types); | ||
339 | diff --git a/iothread.c b/iothread.c | ||
340 | index XXXXXXX..XXXXXXX 100644 | ||
341 | --- a/iothread.c | ||
342 | +++ b/iothread.c | ||
343 | @@ -XXX,XX +XXX,XX @@ | ||
344 | #include "qemu/module.h" | ||
345 | #include "block/aio.h" | ||
346 | #include "block/block.h" | ||
347 | +#include "sysemu/event-loop-base.h" | ||
348 | #include "sysemu/iothread.h" | ||
349 | #include "qapi/error.h" | ||
350 | #include "qapi/qapi-commands-misc.h" | ||
351 | @@ -XXX,XX +XXX,XX @@ static void iothread_init_gcontext(IOThread *iothread) | ||
352 | iothread->main_loop = g_main_loop_new(iothread->worker_context, TRUE); | ||
353 | } | ||
354 | |||
355 | -static void iothread_set_aio_context_params(IOThread *iothread, Error **errp) | ||
356 | +static void iothread_set_aio_context_params(EventLoopBase *base, Error **errp) | ||
357 | { | ||
358 | + IOThread *iothread = IOTHREAD(base); | ||
359 | ERRP_GUARD(); | ||
360 | |||
361 | + if (!iothread->ctx) { | ||
362 | + return; | ||
363 | + } | ||
364 | + | ||
365 | aio_context_set_poll_params(iothread->ctx, | ||
366 | iothread->poll_max_ns, | ||
367 | iothread->poll_grow, | ||
368 | @@ -XXX,XX +XXX,XX @@ static void iothread_set_aio_context_params(IOThread *iothread, Error **errp) | ||
369 | } | ||
370 | |||
371 | aio_context_set_aio_params(iothread->ctx, | ||
372 | - iothread->aio_max_batch, | ||
373 | + iothread->parent_obj.aio_max_batch, | ||
374 | errp); | ||
375 | } | ||
376 | |||
377 | -static void iothread_complete(UserCreatable *obj, Error **errp) | ||
378 | + | ||
379 | +static void iothread_init(EventLoopBase *base, Error **errp) | ||
380 | { | ||
381 | Error *local_error = NULL; | ||
382 | - IOThread *iothread = IOTHREAD(obj); | ||
383 | + IOThread *iothread = IOTHREAD(base); | ||
384 | char *thread_name; | ||
385 | |||
386 | iothread->stopping = false; | ||
387 | @@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp) | ||
388 | */ | ||
389 | iothread_init_gcontext(iothread); | ||
390 | |||
391 | - iothread_set_aio_context_params(iothread, &local_error); | ||
392 | + iothread_set_aio_context_params(base, &local_error); | ||
393 | if (local_error) { | ||
394 | error_propagate(errp, local_error); | ||
395 | aio_context_unref(iothread->ctx); | ||
396 | @@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp) | ||
397 | * to inherit. | ||
398 | */ | ||
399 | thread_name = g_strdup_printf("IO %s", | ||
400 | - object_get_canonical_path_component(OBJECT(obj))); | ||
401 | + object_get_canonical_path_component(OBJECT(base))); | ||
402 | qemu_thread_create(&iothread->thread, thread_name, iothread_run, | ||
403 | iothread, QEMU_THREAD_JOINABLE); | ||
404 | g_free(thread_name); | ||
405 | @@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo poll_grow_info = { | ||
406 | static IOThreadParamInfo poll_shrink_info = { | ||
407 | "poll-shrink", offsetof(IOThread, poll_shrink), | ||
408 | }; | ||
409 | -static IOThreadParamInfo aio_max_batch_info = { | ||
410 | - "aio-max-batch", offsetof(IOThread, aio_max_batch), | ||
411 | -}; | ||
412 | |||
413 | static void iothread_get_param(Object *obj, Visitor *v, | ||
414 | const char *name, IOThreadParamInfo *info, Error **errp) | ||
415 | @@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v, | ||
416 | } | ||
417 | } | ||
418 | |||
419 | -static void iothread_get_aio_param(Object *obj, Visitor *v, | ||
420 | - const char *name, void *opaque, Error **errp) | ||
421 | -{ | ||
422 | - IOThreadParamInfo *info = opaque; | ||
423 | - | ||
424 | - iothread_get_param(obj, v, name, info, errp); | ||
425 | -} | ||
426 | - | ||
427 | -static void iothread_set_aio_param(Object *obj, Visitor *v, | ||
428 | - const char *name, void *opaque, Error **errp) | ||
429 | -{ | ||
430 | - IOThread *iothread = IOTHREAD(obj); | ||
431 | - IOThreadParamInfo *info = opaque; | ||
432 | - | ||
433 | - if (!iothread_set_param(obj, v, name, info, errp)) { | ||
434 | - return; | ||
435 | - } | ||
436 | - | ||
437 | - if (iothread->ctx) { | ||
438 | - aio_context_set_aio_params(iothread->ctx, | ||
439 | - iothread->aio_max_batch, | ||
440 | - errp); | ||
441 | - } | ||
442 | -} | ||
443 | - | ||
444 | static void iothread_class_init(ObjectClass *klass, void *class_data) | ||
445 | { | ||
446 | - UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass); | ||
447 | - ucc->complete = iothread_complete; | ||
448 | + EventLoopBaseClass *bc = EVENT_LOOP_BASE_CLASS(klass); | ||
449 | + | ||
450 | + bc->init = iothread_init; | ||
451 | + bc->update_params = iothread_set_aio_context_params; | ||
452 | |||
453 | object_class_property_add(klass, "poll-max-ns", "int", | ||
454 | iothread_get_poll_param, | ||
455 | @@ -XXX,XX +XXX,XX @@ static void iothread_class_init(ObjectClass *klass, void *class_data) | ||
456 | iothread_get_poll_param, | ||
457 | iothread_set_poll_param, | ||
458 | NULL, &poll_shrink_info); | ||
459 | - object_class_property_add(klass, "aio-max-batch", "int", | ||
460 | - iothread_get_aio_param, | ||
461 | - iothread_set_aio_param, | ||
462 | - NULL, &aio_max_batch_info); | ||
463 | } | ||
464 | |||
465 | static const TypeInfo iothread_info = { | ||
466 | .name = TYPE_IOTHREAD, | ||
467 | - .parent = TYPE_OBJECT, | ||
468 | + .parent = TYPE_EVENT_LOOP_BASE, | ||
469 | .class_init = iothread_class_init, | ||
470 | .instance_size = sizeof(IOThread), | ||
471 | .instance_init = iothread_instance_init, | ||
472 | .instance_finalize = iothread_instance_finalize, | ||
473 | - .interfaces = (InterfaceInfo[]) { | ||
474 | - {TYPE_USER_CREATABLE}, | ||
475 | - {} | ||
476 | - }, | ||
477 | }; | ||
478 | |||
479 | static void iothread_register_types(void) | ||
480 | @@ -XXX,XX +XXX,XX @@ static int query_one_iothread(Object *object, void *opaque) | ||
481 | info->poll_max_ns = iothread->poll_max_ns; | ||
482 | info->poll_grow = iothread->poll_grow; | ||
483 | info->poll_shrink = iothread->poll_shrink; | ||
484 | - info->aio_max_batch = iothread->aio_max_batch; | ||
485 | + info->aio_max_batch = iothread->parent_obj.aio_max_batch; | ||
486 | |||
487 | QAPI_LIST_APPEND(*tail, info); | ||
488 | return 0; | ||
76 | -- | 489 | -- |
77 | 2.21.0 | 490 | 2.35.1 |
78 | |||
79 | diff view generated by jsdifflib |
1 | From: Sergio Lopez <slp@redhat.com> | 1 | From: Nicolas Saenz Julienne <nsaenzju@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | virtio_notify_config() needs to acquire the global mutex, which isn't | 3 | 'event-loop-base' provides basic property handling for all 'AioContext' |
4 | allowed from an iothread, and may lead to a deadlock like this: | 4 | based event loops. So let's define a new 'MainLoopClass' that inherits |
5 | 5 | from it. This will permit tweaking the main loop's properties through | |
6 | - main thead | 6 | qapi as well as through the command line using the '-object' keyword[1]. |
7 | * Has acquired: qemu_global_mutex. | 7 | Only one instance of 'MainLoopClass' might be created at any time. |
8 | * Is trying the acquire: iothread AioContext lock via | 8 | |
9 | AIO_WAIT_WHILE (after aio_poll). | 9 | 'EventLoopBaseClass' learns a new callback, 'can_be_deleted()' so as to |
10 | 10 | mark 'MainLoop' as non-deletable. | |
11 | - iothread | 11 | |
12 | * Has acquired: AioContext lock. | 12 | [1] For example: |
13 | * Is trying to acquire: qemu_global_mutex (via | 13 | -object main-loop,id=main-loop,aio-max-batch=<value> |
14 | virtio_notify_config->prepare_mmio_access). | 14 | |
15 | 15 | Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com> | |
16 | If virtio_blk_resize() is called from an iothread, schedule | 16 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> |
17 | virtio_notify_config() to be run in the main context BH. | 17 | Acked-by: Markus Armbruster <armbru@redhat.com> |
18 | 18 | Message-id: 20220425075723.20019-3-nsaenzju@redhat.com | |
19 | [Removed unnecessary newline as suggested by Kevin Wolf | ||
20 | <kwolf@redhat.com>. | ||
21 | --Stefan] | ||
22 | |||
23 | Signed-off-by: Sergio Lopez <slp@redhat.com> | ||
24 | Reviewed-by: Kevin Wolf <kwolf@redhat.com> | ||
25 | Message-id: 20190916112411.21636-1-slp@redhat.com | ||
26 | Message-Id: <20190916112411.21636-1-slp@redhat.com> | ||
27 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 19 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
28 | --- | 20 | --- |
29 | hw/block/virtio-blk.c | 16 +++++++++++++++- | 21 | qapi/qom.json | 13 ++++++++ |
30 | 1 file changed, 15 insertions(+), 1 deletion(-) | 22 | meson.build | 3 +- |
31 | 23 | include/qemu/main-loop.h | 10 ++++++ | |
32 | diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c | 24 | include/sysemu/event-loop-base.h | 1 + |
33 | index XXXXXXX..XXXXXXX 100644 | 25 | event-loop-base.c | 13 ++++++++ |
34 | --- a/hw/block/virtio-blk.c | 26 | util/main-loop.c | 56 ++++++++++++++++++++++++++++++++ |
35 | +++ b/hw/block/virtio-blk.c | 27 | 6 files changed, 95 insertions(+), 1 deletion(-) |
36 | @@ -XXX,XX +XXX,XX @@ | 28 | |
37 | #include "qemu/iov.h" | 29 | diff --git a/qapi/qom.json b/qapi/qom.json |
38 | #include "qemu/module.h" | 30 | index XXXXXXX..XXXXXXX 100644 |
31 | --- a/qapi/qom.json | ||
32 | +++ b/qapi/qom.json | ||
33 | @@ -XXX,XX +XXX,XX @@ | ||
34 | '*poll-grow': 'int', | ||
35 | '*poll-shrink': 'int' } } | ||
36 | |||
37 | +## | ||
38 | +# @MainLoopProperties: | ||
39 | +# | ||
40 | +# Properties for the main-loop object. | ||
41 | +# | ||
42 | +# Since: 7.1 | ||
43 | +## | ||
44 | +{ 'struct': 'MainLoopProperties', | ||
45 | + 'base': 'EventLoopBaseProperties', | ||
46 | + 'data': {} } | ||
47 | + | ||
48 | ## | ||
49 | # @MemoryBackendProperties: | ||
50 | # | ||
51 | @@ -XXX,XX +XXX,XX @@ | ||
52 | { 'name': 'input-linux', | ||
53 | 'if': 'CONFIG_LINUX' }, | ||
54 | 'iothread', | ||
55 | + 'main-loop', | ||
56 | { 'name': 'memory-backend-epc', | ||
57 | 'if': 'CONFIG_LINUX' }, | ||
58 | 'memory-backend-file', | ||
59 | @@ -XXX,XX +XXX,XX @@ | ||
60 | 'input-linux': { 'type': 'InputLinuxProperties', | ||
61 | 'if': 'CONFIG_LINUX' }, | ||
62 | 'iothread': 'IothreadProperties', | ||
63 | + 'main-loop': 'MainLoopProperties', | ||
64 | 'memory-backend-epc': { 'type': 'MemoryBackendEpcProperties', | ||
65 | 'if': 'CONFIG_LINUX' }, | ||
66 | 'memory-backend-file': 'MemoryBackendFileProperties', | ||
67 | diff --git a/meson.build b/meson.build | ||
68 | index XXXXXXX..XXXXXXX 100644 | ||
69 | --- a/meson.build | ||
70 | +++ b/meson.build | ||
71 | @@ -XXX,XX +XXX,XX @@ libqemuutil = static_library('qemuutil', | ||
72 | sources: util_ss.sources() + stub_ss.sources() + genh, | ||
73 | dependencies: [util_ss.dependencies(), libm, threads, glib, socket, malloc, pixman]) | ||
74 | qemuutil = declare_dependency(link_with: libqemuutil, | ||
75 | - sources: genh + version_res) | ||
76 | + sources: genh + version_res, | ||
77 | + dependencies: [event_loop_base]) | ||
78 | |||
79 | if have_system or have_user | ||
80 | decodetree = generator(find_program('scripts/decodetree.py'), | ||
81 | diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h | ||
82 | index XXXXXXX..XXXXXXX 100644 | ||
83 | --- a/include/qemu/main-loop.h | ||
84 | +++ b/include/qemu/main-loop.h | ||
85 | @@ -XXX,XX +XXX,XX @@ | ||
86 | #define QEMU_MAIN_LOOP_H | ||
87 | |||
88 | #include "block/aio.h" | ||
89 | +#include "qom/object.h" | ||
90 | +#include "sysemu/event-loop-base.h" | ||
91 | |||
92 | #define SIG_IPI SIGUSR1 | ||
93 | |||
94 | +#define TYPE_MAIN_LOOP "main-loop" | ||
95 | +OBJECT_DECLARE_TYPE(MainLoop, MainLoopClass, MAIN_LOOP) | ||
96 | + | ||
97 | +struct MainLoop { | ||
98 | + EventLoopBase parent_obj; | ||
99 | +}; | ||
100 | +typedef struct MainLoop MainLoop; | ||
101 | + | ||
102 | /** | ||
103 | * qemu_init_main_loop: Set up the process so that it can run the main loop. | ||
104 | * | ||
105 | diff --git a/include/sysemu/event-loop-base.h b/include/sysemu/event-loop-base.h | ||
106 | index XXXXXXX..XXXXXXX 100644 | ||
107 | --- a/include/sysemu/event-loop-base.h | ||
108 | +++ b/include/sysemu/event-loop-base.h | ||
109 | @@ -XXX,XX +XXX,XX @@ struct EventLoopBaseClass { | ||
110 | |||
111 | void (*init)(EventLoopBase *base, Error **errp); | ||
112 | void (*update_params)(EventLoopBase *base, Error **errp); | ||
113 | + bool (*can_be_deleted)(EventLoopBase *base); | ||
114 | }; | ||
115 | |||
116 | struct EventLoopBase { | ||
117 | diff --git a/event-loop-base.c b/event-loop-base.c | ||
118 | index XXXXXXX..XXXXXXX 100644 | ||
119 | --- a/event-loop-base.c | ||
120 | +++ b/event-loop-base.c | ||
121 | @@ -XXX,XX +XXX,XX @@ static void event_loop_base_complete(UserCreatable *uc, Error **errp) | ||
122 | } | ||
123 | } | ||
124 | |||
125 | +static bool event_loop_base_can_be_deleted(UserCreatable *uc) | ||
126 | +{ | ||
127 | + EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(uc); | ||
128 | + EventLoopBase *backend = EVENT_LOOP_BASE(uc); | ||
129 | + | ||
130 | + if (bc->can_be_deleted) { | ||
131 | + return bc->can_be_deleted(backend); | ||
132 | + } | ||
133 | + | ||
134 | + return true; | ||
135 | +} | ||
136 | + | ||
137 | static void event_loop_base_class_init(ObjectClass *klass, void *class_data) | ||
138 | { | ||
139 | UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass); | ||
140 | ucc->complete = event_loop_base_complete; | ||
141 | + ucc->can_be_deleted = event_loop_base_can_be_deleted; | ||
142 | |||
143 | object_class_property_add(klass, "aio-max-batch", "int", | ||
144 | event_loop_base_get_param, | ||
145 | diff --git a/util/main-loop.c b/util/main-loop.c | ||
146 | index XXXXXXX..XXXXXXX 100644 | ||
147 | --- a/util/main-loop.c | ||
148 | +++ b/util/main-loop.c | ||
149 | @@ -XXX,XX +XXX,XX @@ | ||
39 | #include "qemu/error-report.h" | 150 | #include "qemu/error-report.h" |
40 | +#include "qemu/main-loop.h" | 151 | #include "qemu/queue.h" |
41 | #include "trace.h" | 152 | #include "qemu/compiler.h" |
42 | #include "hw/block/block.h" | 153 | +#include "qom/object.h" |
43 | #include "hw/qdev-properties.h" | 154 | |
44 | @@ -XXX,XX +XXX,XX @@ static int virtio_blk_load_device(VirtIODevice *vdev, QEMUFile *f, | 155 | #ifndef _WIN32 |
156 | #include <sys/wait.h> | ||
157 | @@ -XXX,XX +XXX,XX @@ int qemu_init_main_loop(Error **errp) | ||
45 | return 0; | 158 | return 0; |
46 | } | 159 | } |
47 | 160 | ||
48 | +static void virtio_resize_cb(void *opaque) | 161 | +static void main_loop_update_params(EventLoopBase *base, Error **errp) |
49 | +{ | 162 | +{ |
50 | + VirtIODevice *vdev = opaque; | 163 | + if (!qemu_aio_context) { |
51 | + | 164 | + error_setg(errp, "qemu aio context not ready"); |
52 | + assert(qemu_get_current_aio_context() == qemu_get_aio_context()); | 165 | + return; |
53 | + virtio_notify_config(vdev); | 166 | + } |
54 | +} | 167 | + |
55 | + | 168 | + aio_context_set_aio_params(qemu_aio_context, base->aio_max_batch, errp); |
56 | static void virtio_blk_resize(void *opaque) | 169 | +} |
57 | { | 170 | + |
58 | VirtIODevice *vdev = VIRTIO_DEVICE(opaque); | 171 | +MainLoop *mloop; |
59 | 172 | + | |
60 | - virtio_notify_config(vdev); | 173 | +static void main_loop_init(EventLoopBase *base, Error **errp) |
61 | + /* | 174 | +{ |
62 | + * virtio_notify_config() needs to acquire the global mutex, | 175 | + MainLoop *m = MAIN_LOOP(base); |
63 | + * so it can't be called from an iothread. Instead, schedule | 176 | + |
64 | + * it to be run in the main context BH. | 177 | + if (mloop) { |
65 | + */ | 178 | + error_setg(errp, "only one main-loop instance allowed"); |
66 | + aio_bh_schedule_oneshot(qemu_get_aio_context(), virtio_resize_cb, vdev); | 179 | + return; |
67 | } | 180 | + } |
68 | 181 | + | |
69 | static const BlockDevOps virtio_block_ops = { | 182 | + main_loop_update_params(base, errp); |
183 | + | ||
184 | + mloop = m; | ||
185 | + return; | ||
186 | +} | ||
187 | + | ||
188 | +static bool main_loop_can_be_deleted(EventLoopBase *base) | ||
189 | +{ | ||
190 | + return false; | ||
191 | +} | ||
192 | + | ||
193 | +static void main_loop_class_init(ObjectClass *oc, void *class_data) | ||
194 | +{ | ||
195 | + EventLoopBaseClass *bc = EVENT_LOOP_BASE_CLASS(oc); | ||
196 | + | ||
197 | + bc->init = main_loop_init; | ||
198 | + bc->update_params = main_loop_update_params; | ||
199 | + bc->can_be_deleted = main_loop_can_be_deleted; | ||
200 | +} | ||
201 | + | ||
202 | +static const TypeInfo main_loop_info = { | ||
203 | + .name = TYPE_MAIN_LOOP, | ||
204 | + .parent = TYPE_EVENT_LOOP_BASE, | ||
205 | + .class_init = main_loop_class_init, | ||
206 | + .instance_size = sizeof(MainLoop), | ||
207 | +}; | ||
208 | + | ||
209 | +static void main_loop_register_types(void) | ||
210 | +{ | ||
211 | + type_register_static(&main_loop_info); | ||
212 | +} | ||
213 | + | ||
214 | +type_init(main_loop_register_types) | ||
215 | + | ||
216 | static int max_priority; | ||
217 | |||
218 | #ifndef _WIN32 | ||
70 | -- | 219 | -- |
71 | 2.21.0 | 220 | 2.35.1 |
72 | |||
73 | diff view generated by jsdifflib |
1 | From: Max Reitz <mreitz@redhat.com> | 1 | From: Nicolas Saenz Julienne <nsaenzju@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | We must not write data to inactive nodes, and a COR is certainly | 3 | The thread pool regulates itself: when idle, it kills threads until |
4 | something we can simply not do without upsetting anyone. So skip COR | 4 | empty, when in demand, it creates new threads until full. This behaviour |
5 | operations on inactive nodes. | 5 | doesn't play well with latency sensitive workloads where the price of |
6 | creating a new thread is too high. For example, when paired with qemu's | ||
7 | '-mlock', or using safety features like SafeStack, creating a new thread | ||
8 | has been measured take multiple milliseconds. | ||
6 | 9 | ||
7 | Signed-off-by: Max Reitz <mreitz@redhat.com> | 10 | In order to mitigate this let's introduce a new 'EventLoopBase' |
8 | Reviewed-by: Eric Blake <eblake@redhat.com> | 11 | property to set the thread pool size. The threads will be created during |
9 | Message-id: 20191001174827.11081-2-mreitz@redhat.com | 12 | the pool's initialization or upon updating the property's value, remain |
10 | Message-Id: <20191001174827.11081-2-mreitz@redhat.com> | 13 | available during its lifetime regardless of demand, and destroyed upon |
14 | freeing it. A properly characterized workload will then be able to | ||
15 | configure the pool to avoid any latency spikes. | ||
16 | |||
17 | Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com> | ||
18 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
19 | Acked-by: Markus Armbruster <armbru@redhat.com> | ||
20 | Message-id: 20220425075723.20019-4-nsaenzju@redhat.com | ||
11 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 21 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
12 | --- | 22 | --- |
13 | block/io.c | 41 +++++++++++++++++++++++++++-------------- | 23 | qapi/qom.json | 10 +++++- |
14 | 1 file changed, 27 insertions(+), 14 deletions(-) | 24 | include/block/aio.h | 10 ++++++ |
25 | include/block/thread-pool.h | 3 ++ | ||
26 | include/sysemu/event-loop-base.h | 4 +++ | ||
27 | event-loop-base.c | 23 +++++++++++++ | ||
28 | iothread.c | 3 ++ | ||
29 | util/aio-posix.c | 1 + | ||
30 | util/async.c | 20 ++++++++++++ | ||
31 | util/main-loop.c | 9 ++++++ | ||
32 | util/thread-pool.c | 55 +++++++++++++++++++++++++++++--- | ||
33 | 10 files changed, 133 insertions(+), 5 deletions(-) | ||
15 | 34 | ||
16 | diff --git a/block/io.c b/block/io.c | 35 | diff --git a/qapi/qom.json b/qapi/qom.json |
17 | index XXXXXXX..XXXXXXX 100644 | 36 | index XXXXXXX..XXXXXXX 100644 |
18 | --- a/block/io.c | 37 | --- a/qapi/qom.json |
19 | +++ b/block/io.c | 38 | +++ b/qapi/qom.json |
20 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child, | 39 | @@ -XXX,XX +XXX,XX @@ |
21 | int max_transfer = MIN_NON_ZERO(bs->bl.max_transfer, | 40 | # 0 means that the engine will use its default. |
22 | BDRV_REQUEST_MAX_BYTES); | 41 | # (default: 0) |
23 | unsigned int progress = 0; | 42 | # |
24 | + bool skip_write; | 43 | +# @thread-pool-min: minimum number of threads reserved in the thread pool |
25 | 44 | +# (default:0) | |
26 | if (!drv) { | 45 | +# |
27 | return -ENOMEDIUM; | 46 | +# @thread-pool-max: maximum number of threads the thread pool can contain |
47 | +# (default:64) | ||
48 | +# | ||
49 | # Since: 7.1 | ||
50 | ## | ||
51 | { 'struct': 'EventLoopBaseProperties', | ||
52 | - 'data': { '*aio-max-batch': 'int' } } | ||
53 | + 'data': { '*aio-max-batch': 'int', | ||
54 | + '*thread-pool-min': 'int', | ||
55 | + '*thread-pool-max': 'int' } } | ||
56 | |||
57 | ## | ||
58 | # @IothreadProperties: | ||
59 | diff --git a/include/block/aio.h b/include/block/aio.h | ||
60 | index XXXXXXX..XXXXXXX 100644 | ||
61 | --- a/include/block/aio.h | ||
62 | +++ b/include/block/aio.h | ||
63 | @@ -XXX,XX +XXX,XX @@ struct AioContext { | ||
64 | QSLIST_HEAD(, Coroutine) scheduled_coroutines; | ||
65 | QEMUBH *co_schedule_bh; | ||
66 | |||
67 | + int thread_pool_min; | ||
68 | + int thread_pool_max; | ||
69 | /* Thread pool for performing work and receiving completion callbacks. | ||
70 | * Has its own locking. | ||
71 | */ | ||
72 | @@ -XXX,XX +XXX,XX @@ void aio_context_set_poll_params(AioContext *ctx, int64_t max_ns, | ||
73 | void aio_context_set_aio_params(AioContext *ctx, int64_t max_batch, | ||
74 | Error **errp); | ||
75 | |||
76 | +/** | ||
77 | + * aio_context_set_thread_pool_params: | ||
78 | + * @ctx: the aio context | ||
79 | + * @min: min number of threads to have readily available in the thread pool | ||
80 | + * @min: max number of threads the thread pool can contain | ||
81 | + */ | ||
82 | +void aio_context_set_thread_pool_params(AioContext *ctx, int64_t min, | ||
83 | + int64_t max, Error **errp); | ||
84 | #endif | ||
85 | diff --git a/include/block/thread-pool.h b/include/block/thread-pool.h | ||
86 | index XXXXXXX..XXXXXXX 100644 | ||
87 | --- a/include/block/thread-pool.h | ||
88 | +++ b/include/block/thread-pool.h | ||
89 | @@ -XXX,XX +XXX,XX @@ | ||
90 | |||
91 | #include "block/block.h" | ||
92 | |||
93 | +#define THREAD_POOL_MAX_THREADS_DEFAULT 64 | ||
94 | + | ||
95 | typedef int ThreadPoolFunc(void *opaque); | ||
96 | |||
97 | typedef struct ThreadPool ThreadPool; | ||
98 | @@ -XXX,XX +XXX,XX @@ BlockAIOCB *thread_pool_submit_aio(ThreadPool *pool, | ||
99 | int coroutine_fn thread_pool_submit_co(ThreadPool *pool, | ||
100 | ThreadPoolFunc *func, void *arg); | ||
101 | void thread_pool_submit(ThreadPool *pool, ThreadPoolFunc *func, void *arg); | ||
102 | +void thread_pool_update_params(ThreadPool *pool, struct AioContext *ctx); | ||
103 | |||
104 | #endif | ||
105 | diff --git a/include/sysemu/event-loop-base.h b/include/sysemu/event-loop-base.h | ||
106 | index XXXXXXX..XXXXXXX 100644 | ||
107 | --- a/include/sysemu/event-loop-base.h | ||
108 | +++ b/include/sysemu/event-loop-base.h | ||
109 | @@ -XXX,XX +XXX,XX @@ struct EventLoopBase { | ||
110 | |||
111 | /* AioContext AIO engine parameters */ | ||
112 | int64_t aio_max_batch; | ||
113 | + | ||
114 | + /* AioContext thread pool parameters */ | ||
115 | + int64_t thread_pool_min; | ||
116 | + int64_t thread_pool_max; | ||
117 | }; | ||
118 | #endif | ||
119 | diff --git a/event-loop-base.c b/event-loop-base.c | ||
120 | index XXXXXXX..XXXXXXX 100644 | ||
121 | --- a/event-loop-base.c | ||
122 | +++ b/event-loop-base.c | ||
123 | @@ -XXX,XX +XXX,XX @@ | ||
124 | #include "qemu/osdep.h" | ||
125 | #include "qom/object_interfaces.h" | ||
126 | #include "qapi/error.h" | ||
127 | +#include "block/thread-pool.h" | ||
128 | #include "sysemu/event-loop-base.h" | ||
129 | |||
130 | typedef struct { | ||
131 | @@ -XXX,XX +XXX,XX @@ typedef struct { | ||
132 | ptrdiff_t offset; /* field's byte offset in EventLoopBase struct */ | ||
133 | } EventLoopBaseParamInfo; | ||
134 | |||
135 | +static void event_loop_base_instance_init(Object *obj) | ||
136 | +{ | ||
137 | + EventLoopBase *base = EVENT_LOOP_BASE(obj); | ||
138 | + | ||
139 | + base->thread_pool_max = THREAD_POOL_MAX_THREADS_DEFAULT; | ||
140 | +} | ||
141 | + | ||
142 | static EventLoopBaseParamInfo aio_max_batch_info = { | ||
143 | "aio-max-batch", offsetof(EventLoopBase, aio_max_batch), | ||
144 | }; | ||
145 | +static EventLoopBaseParamInfo thread_pool_min_info = { | ||
146 | + "thread-pool-min", offsetof(EventLoopBase, thread_pool_min), | ||
147 | +}; | ||
148 | +static EventLoopBaseParamInfo thread_pool_max_info = { | ||
149 | + "thread-pool-max", offsetof(EventLoopBase, thread_pool_max), | ||
150 | +}; | ||
151 | |||
152 | static void event_loop_base_get_param(Object *obj, Visitor *v, | ||
153 | const char *name, void *opaque, Error **errp) | ||
154 | @@ -XXX,XX +XXX,XX @@ static void event_loop_base_class_init(ObjectClass *klass, void *class_data) | ||
155 | event_loop_base_get_param, | ||
156 | event_loop_base_set_param, | ||
157 | NULL, &aio_max_batch_info); | ||
158 | + object_class_property_add(klass, "thread-pool-min", "int", | ||
159 | + event_loop_base_get_param, | ||
160 | + event_loop_base_set_param, | ||
161 | + NULL, &thread_pool_min_info); | ||
162 | + object_class_property_add(klass, "thread-pool-max", "int", | ||
163 | + event_loop_base_get_param, | ||
164 | + event_loop_base_set_param, | ||
165 | + NULL, &thread_pool_max_info); | ||
166 | } | ||
167 | |||
168 | static const TypeInfo event_loop_base_info = { | ||
169 | .name = TYPE_EVENT_LOOP_BASE, | ||
170 | .parent = TYPE_OBJECT, | ||
171 | .instance_size = sizeof(EventLoopBase), | ||
172 | + .instance_init = event_loop_base_instance_init, | ||
173 | .class_size = sizeof(EventLoopBaseClass), | ||
174 | .class_init = event_loop_base_class_init, | ||
175 | .abstract = true, | ||
176 | diff --git a/iothread.c b/iothread.c | ||
177 | index XXXXXXX..XXXXXXX 100644 | ||
178 | --- a/iothread.c | ||
179 | +++ b/iothread.c | ||
180 | @@ -XXX,XX +XXX,XX @@ static void iothread_set_aio_context_params(EventLoopBase *base, Error **errp) | ||
181 | aio_context_set_aio_params(iothread->ctx, | ||
182 | iothread->parent_obj.aio_max_batch, | ||
183 | errp); | ||
184 | + | ||
185 | + aio_context_set_thread_pool_params(iothread->ctx, base->thread_pool_min, | ||
186 | + base->thread_pool_max, errp); | ||
187 | } | ||
188 | |||
189 | |||
190 | diff --git a/util/aio-posix.c b/util/aio-posix.c | ||
191 | index XXXXXXX..XXXXXXX 100644 | ||
192 | --- a/util/aio-posix.c | ||
193 | +++ b/util/aio-posix.c | ||
194 | @@ -XXX,XX +XXX,XX @@ | ||
195 | |||
196 | #include "qemu/osdep.h" | ||
197 | #include "block/block.h" | ||
198 | +#include "block/thread-pool.h" | ||
199 | #include "qemu/main-loop.h" | ||
200 | #include "qemu/rcu.h" | ||
201 | #include "qemu/rcu_queue.h" | ||
202 | diff --git a/util/async.c b/util/async.c | ||
203 | index XXXXXXX..XXXXXXX 100644 | ||
204 | --- a/util/async.c | ||
205 | +++ b/util/async.c | ||
206 | @@ -XXX,XX +XXX,XX @@ AioContext *aio_context_new(Error **errp) | ||
207 | |||
208 | ctx->aio_max_batch = 0; | ||
209 | |||
210 | + ctx->thread_pool_min = 0; | ||
211 | + ctx->thread_pool_max = THREAD_POOL_MAX_THREADS_DEFAULT; | ||
212 | + | ||
213 | return ctx; | ||
214 | fail: | ||
215 | g_source_destroy(&ctx->source); | ||
216 | @@ -XXX,XX +XXX,XX @@ void qemu_set_current_aio_context(AioContext *ctx) | ||
217 | assert(!get_my_aiocontext()); | ||
218 | set_my_aiocontext(ctx); | ||
219 | } | ||
220 | + | ||
221 | +void aio_context_set_thread_pool_params(AioContext *ctx, int64_t min, | ||
222 | + int64_t max, Error **errp) | ||
223 | +{ | ||
224 | + | ||
225 | + if (min > max || !max || min > INT_MAX || max > INT_MAX) { | ||
226 | + error_setg(errp, "bad thread-pool-min/thread-pool-max values"); | ||
227 | + return; | ||
228 | + } | ||
229 | + | ||
230 | + ctx->thread_pool_min = min; | ||
231 | + ctx->thread_pool_max = max; | ||
232 | + | ||
233 | + if (ctx->thread_pool) { | ||
234 | + thread_pool_update_params(ctx->thread_pool, ctx); | ||
235 | + } | ||
236 | +} | ||
237 | diff --git a/util/main-loop.c b/util/main-loop.c | ||
238 | index XXXXXXX..XXXXXXX 100644 | ||
239 | --- a/util/main-loop.c | ||
240 | +++ b/util/main-loop.c | ||
241 | @@ -XXX,XX +XXX,XX @@ | ||
242 | #include "sysemu/replay.h" | ||
243 | #include "qemu/main-loop.h" | ||
244 | #include "block/aio.h" | ||
245 | +#include "block/thread-pool.h" | ||
246 | #include "qemu/error-report.h" | ||
247 | #include "qemu/queue.h" | ||
248 | #include "qemu/compiler.h" | ||
249 | @@ -XXX,XX +XXX,XX @@ int qemu_init_main_loop(Error **errp) | ||
250 | |||
251 | static void main_loop_update_params(EventLoopBase *base, Error **errp) | ||
252 | { | ||
253 | + ERRP_GUARD(); | ||
254 | + | ||
255 | if (!qemu_aio_context) { | ||
256 | error_setg(errp, "qemu aio context not ready"); | ||
257 | return; | ||
28 | } | 258 | } |
29 | 259 | ||
260 | aio_context_set_aio_params(qemu_aio_context, base->aio_max_batch, errp); | ||
261 | + if (*errp) { | ||
262 | + return; | ||
263 | + } | ||
264 | + | ||
265 | + aio_context_set_thread_pool_params(qemu_aio_context, base->thread_pool_min, | ||
266 | + base->thread_pool_max, errp); | ||
267 | } | ||
268 | |||
269 | MainLoop *mloop; | ||
270 | diff --git a/util/thread-pool.c b/util/thread-pool.c | ||
271 | index XXXXXXX..XXXXXXX 100644 | ||
272 | --- a/util/thread-pool.c | ||
273 | +++ b/util/thread-pool.c | ||
274 | @@ -XXX,XX +XXX,XX @@ struct ThreadPool { | ||
275 | QemuMutex lock; | ||
276 | QemuCond worker_stopped; | ||
277 | QemuSemaphore sem; | ||
278 | - int max_threads; | ||
279 | QEMUBH *new_thread_bh; | ||
280 | |||
281 | /* The following variables are only accessed from one AioContext. */ | ||
282 | @@ -XXX,XX +XXX,XX @@ struct ThreadPool { | ||
283 | int new_threads; /* backlog of threads we need to create */ | ||
284 | int pending_threads; /* threads created but not running yet */ | ||
285 | bool stopping; | ||
286 | + int min_threads; | ||
287 | + int max_threads; | ||
288 | }; | ||
289 | |||
290 | +static inline bool back_to_sleep(ThreadPool *pool, int ret) | ||
291 | +{ | ||
30 | + /* | 292 | + /* |
31 | + * Do not write anything when the BDS is inactive. That is not | 293 | + * The semaphore timed out, we should exit the loop except when: |
32 | + * allowed, and it would not help. | 294 | + * - There is work to do, we raced with the signal. |
295 | + * - The max threads threshold just changed, we raced with the signal. | ||
296 | + * - The thread pool forces a minimum number of readily available threads. | ||
33 | + */ | 297 | + */ |
34 | + skip_write = (bs->open_flags & BDRV_O_INACTIVE); | 298 | + if (ret == -1 && (!QTAILQ_EMPTY(&pool->request_list) || |
35 | + | 299 | + pool->cur_threads > pool->max_threads || |
36 | /* FIXME We cannot require callers to have write permissions when all they | 300 | + pool->cur_threads <= pool->min_threads)) { |
37 | * are doing is a read request. If we did things right, write permissions | 301 | + return true; |
38 | * would be obtained anyway, but internally by the copy-on-read code. As | 302 | + } |
39 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn bdrv_co_do_copy_on_readv(BdrvChild *child, | 303 | + |
40 | while (cluster_bytes) { | 304 | + return false; |
41 | int64_t pnum; | 305 | +} |
42 | 306 | + | |
43 | - ret = bdrv_is_allocated(bs, cluster_offset, | 307 | static void *worker_thread(void *opaque) |
44 | - MIN(cluster_bytes, max_transfer), &pnum); | 308 | { |
45 | - if (ret < 0) { | 309 | ThreadPool *pool = opaque; |
46 | - /* Safe to treat errors in querying allocation as if | 310 | @@ -XXX,XX +XXX,XX @@ static void *worker_thread(void *opaque) |
47 | - * unallocated; we'll probably fail again soon on the | 311 | ret = qemu_sem_timedwait(&pool->sem, 10000); |
48 | - * read, but at least that will set a decent errno. | 312 | qemu_mutex_lock(&pool->lock); |
49 | - */ | 313 | pool->idle_threads--; |
50 | + if (skip_write) { | 314 | - } while (ret == -1 && !QTAILQ_EMPTY(&pool->request_list)); |
51 | + ret = 1; /* "already allocated", so nothing will be copied */ | 315 | - if (ret == -1 || pool->stopping) { |
52 | pnum = MIN(cluster_bytes, max_transfer); | 316 | + } while (back_to_sleep(pool, ret)); |
53 | - } | 317 | + if (ret == -1 || pool->stopping || |
54 | + } else { | 318 | + pool->cur_threads > pool->max_threads) { |
55 | + ret = bdrv_is_allocated(bs, cluster_offset, | 319 | break; |
56 | + MIN(cluster_bytes, max_transfer), &pnum); | 320 | } |
57 | + if (ret < 0) { | 321 | |
58 | + /* | 322 | @@ -XXX,XX +XXX,XX @@ void thread_pool_submit(ThreadPool *pool, ThreadPoolFunc *func, void *arg) |
59 | + * Safe to treat errors in querying allocation as if | 323 | thread_pool_submit_aio(pool, func, arg, NULL, NULL); |
60 | + * unallocated; we'll probably fail again soon on the | 324 | } |
61 | + * read, but at least that will set a decent errno. | 325 | |
62 | + */ | 326 | +void thread_pool_update_params(ThreadPool *pool, AioContext *ctx) |
63 | + pnum = MIN(cluster_bytes, max_transfer); | 327 | +{ |
64 | + } | 328 | + qemu_mutex_lock(&pool->lock); |
65 | 329 | + | |
66 | - /* Stop at EOF if the image ends in the middle of the cluster */ | 330 | + pool->min_threads = ctx->thread_pool_min; |
67 | - if (ret == 0 && pnum == 0) { | 331 | + pool->max_threads = ctx->thread_pool_max; |
68 | - assert(progress >= bytes); | 332 | + |
69 | - break; | 333 | + /* |
70 | - } | 334 | + * We either have to: |
71 | + /* Stop at EOF if the image ends in the middle of the cluster */ | 335 | + * - Increase the number available of threads until over the min_threads |
72 | + if (ret == 0 && pnum == 0) { | 336 | + * threshold. |
73 | + assert(progress >= bytes); | 337 | + * - Decrease the number of available threads until under the max_threads |
74 | + break; | 338 | + * threshold. |
75 | + } | 339 | + * - Do nothing. The current number of threads fall in between the min and |
76 | 340 | + * max thresholds. We'll let the pool manage itself. | |
77 | - assert(skip_bytes < pnum); | 341 | + */ |
78 | + assert(skip_bytes < pnum); | 342 | + for (int i = pool->cur_threads; i < pool->min_threads; i++) { |
79 | + } | 343 | + spawn_thread(pool); |
80 | 344 | + } | |
81 | if (ret <= 0) { | 345 | + |
82 | QEMUIOVector local_qiov; | 346 | + for (int i = pool->cur_threads; i > pool->max_threads; i--) { |
347 | + qemu_sem_post(&pool->sem); | ||
348 | + } | ||
349 | + | ||
350 | + qemu_mutex_unlock(&pool->lock); | ||
351 | +} | ||
352 | + | ||
353 | static void thread_pool_init_one(ThreadPool *pool, AioContext *ctx) | ||
354 | { | ||
355 | if (!ctx) { | ||
356 | @@ -XXX,XX +XXX,XX @@ static void thread_pool_init_one(ThreadPool *pool, AioContext *ctx) | ||
357 | qemu_mutex_init(&pool->lock); | ||
358 | qemu_cond_init(&pool->worker_stopped); | ||
359 | qemu_sem_init(&pool->sem, 0); | ||
360 | - pool->max_threads = 64; | ||
361 | pool->new_thread_bh = aio_bh_new(ctx, spawn_thread_bh_fn, pool); | ||
362 | |||
363 | QLIST_INIT(&pool->head); | ||
364 | QTAILQ_INIT(&pool->request_list); | ||
365 | + | ||
366 | + thread_pool_update_params(pool, ctx); | ||
367 | } | ||
368 | |||
369 | ThreadPool *thread_pool_new(AioContext *ctx) | ||
83 | -- | 370 | -- |
84 | 2.21.0 | 371 | 2.35.1 |
85 | |||
86 | diff view generated by jsdifflib |