1 | The following changes since commit 55a19ad8b2d0797e3a8fe90ab99a9bb713824059: | 1 | The following changes since commit 9cf289af47bcfae5c75de37d8e5d6fd23705322c: |
---|---|---|---|
2 | 2 | ||
3 | Update version for v2.9.0-rc1 release (2017-03-21 17:13:29 +0000) | 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/codyprime/qemu-kvm-jtc.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 600ac6a0ef5c06418446ef2f37407bddcc51b21c: | 9 | for you to fetch changes up to bef2e050d6a7feb865854c65570c496ac5a8cf53: |
10 | 10 | ||
11 | blockjob: add devops to blockjob backends (2017-03-22 13:26:27 -0400) | 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 | Block patches for 2.9 | 14 | Pull request |
15 | |||
16 | Add new thread-pool-min/thread-pool-max parameters to control the thread pool | ||
17 | used for async I/O. | ||
18 | |||
15 | ---------------------------------------------------------------- | 19 | ---------------------------------------------------------------- |
16 | 20 | ||
17 | John Snow (3): | 21 | Nicolas Saenz Julienne (3): |
18 | blockjob: add block_job_start_shim | 22 | Introduce event-loop-base abstract class |
19 | block-backend: add drained_begin / drained_end ops | 23 | util/main-loop: Introduce the main loop into QOM |
20 | blockjob: add devops to blockjob backends | 24 | util/event-loop-base: Introduce options to set the thread pool size |
21 | 25 | ||
22 | Paolo Bonzini (1): | 26 | qapi/qom.json | 43 ++++++++-- |
23 | blockjob: avoid recursive AioContext locking | 27 | meson.build | 26 +++--- |
24 | 28 | include/block/aio.h | 10 +++ | |
25 | block/block-backend.c | 24 ++++++++++++++-- | 29 | include/block/thread-pool.h | 3 + |
26 | blockjob.c | 63 ++++++++++++++++++++++++++++++++---------- | 30 | include/qemu/main-loop.h | 10 +++ |
27 | include/sysemu/block-backend.h | 8 ++++++ | 31 | include/sysemu/event-loop-base.h | 41 +++++++++ |
28 | 3 files changed, 79 insertions(+), 16 deletions(-) | 32 | include/sysemu/iothread.h | 6 +- |
33 | event-loop-base.c | 140 +++++++++++++++++++++++++++++++ | ||
34 | iothread.c | 68 +++++---------- | ||
35 | util/aio-posix.c | 1 + | ||
36 | util/async.c | 20 +++++ | ||
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 | ||
29 | 42 | ||
30 | -- | 43 | -- |
31 | 2.9.3 | 44 | 2.35.1 |
32 | |||
33 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
2 | 1 | ||
3 | Streaming or any other block job hangs when performed on a block device | ||
4 | that has a non-default iothread. This happens because the AioContext | ||
5 | is acquired twice by block_job_defer_to_main_loop_bh and then released | ||
6 | only once by BDRV_POLL_WHILE. (Insert rants on recursive mutexes, which | ||
7 | unfortunately are a temporary but necessary evil for iothreads at the | ||
8 | moment). | ||
9 | |||
10 | Luckily, the reason for the double acquisition is simple; the function | ||
11 | acquires the AioContext for both the job iothread and the BDS iothread, | ||
12 | in case the BDS iothread was changed while the job was running. It | ||
13 | is therefore enough to skip the second acquisition when the two | ||
14 | AioContexts are one and the same. | ||
15 | |||
16 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
17 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
18 | Reviewed-by: Jeff Cody <jcody@redhat.com> | ||
19 | Message-id: 1490118490-5597-1-git-send-email-pbonzini@redhat.com | ||
20 | Signed-off-by: Jeff Cody <jcody@redhat.com> | ||
21 | --- | ||
22 | blockjob.c | 8 ++++++-- | ||
23 | 1 file changed, 6 insertions(+), 2 deletions(-) | ||
24 | |||
25 | diff --git a/blockjob.c b/blockjob.c | ||
26 | index XXXXXXX..XXXXXXX 100644 | ||
27 | --- a/blockjob.c | ||
28 | +++ b/blockjob.c | ||
29 | @@ -XXX,XX +XXX,XX @@ static void block_job_defer_to_main_loop_bh(void *opaque) | ||
30 | |||
31 | /* Fetch BDS AioContext again, in case it has changed */ | ||
32 | aio_context = blk_get_aio_context(data->job->blk); | ||
33 | - aio_context_acquire(aio_context); | ||
34 | + if (aio_context != data->aio_context) { | ||
35 | + aio_context_acquire(aio_context); | ||
36 | + } | ||
37 | |||
38 | data->job->deferred_to_main_loop = false; | ||
39 | data->fn(data->job, data->opaque); | ||
40 | |||
41 | - aio_context_release(aio_context); | ||
42 | + if (aio_context != data->aio_context) { | ||
43 | + aio_context_release(aio_context); | ||
44 | + } | ||
45 | |||
46 | aio_context_release(data->aio_context); | ||
47 | |||
48 | -- | ||
49 | 2.9.3 | ||
50 | |||
51 | diff view generated by jsdifflib |
1 | From: John Snow <jsnow@redhat.com> | 1 | From: Nicolas Saenz Julienne <nsaenzju@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | This lets us hook into drained_begin and drained_end requests from the | 3 | Introduce the 'event-loop-base' abstract class, it'll hold the |
4 | backend level, which is particularly useful for making sure that all | 4 | properties common to all event loops and provide the necessary hooks for |
5 | jobs associated with a particular node (whether the source or the target) | 5 | their creation and maintenance. Then have iothread inherit from it. |
6 | receive a drain request. | 6 | |
7 | 7 | EventLoopBaseClass is defined as user creatable and provides a hook for | |
8 | Suggested-by: Kevin Wolf <kwolf@redhat.com> | 8 | its children to attach themselves to the user creatable class 'complete' |
9 | Signed-off-by: John Snow <jsnow@redhat.com> | 9 | function. It also provides an update_params() callback to propagate |
10 | Reviewed-by: Jeff Cody <jcody@redhat.com> | 10 | property changes onto its children. |
11 | Message-id: 20170316212351.13797-4-jsnow@redhat.com | 11 | |
12 | Signed-off-by: Jeff Cody <jcody@redhat.com> | 12 | The new 'event-loop-base' class will live in the root directory. It is |
13 | built on its own using the 'link_whole' option (there are no direct | ||
14 | function dependencies between the class and its children, it all happens | ||
15 | trough 'constructor' magic). And also imposes new compilation | ||
16 | dependencies: | ||
17 | |||
18 | qom <- event-loop-base <- blockdev (iothread.c) | ||
19 | |||
20 | And in subsequent patches: | ||
21 | |||
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 | ||
39 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
13 | --- | 40 | --- |
14 | blockjob.c | 29 ++++++++++++++++++++++++----- | 41 | qapi/qom.json | 22 +++++-- |
15 | 1 file changed, 24 insertions(+), 5 deletions(-) | 42 | meson.build | 23 ++++--- |
16 | 43 | include/sysemu/event-loop-base.h | 36 +++++++++++ | |
17 | diff --git a/blockjob.c b/blockjob.c | 44 | include/sysemu/iothread.h | 6 +- |
45 | event-loop-base.c | 104 +++++++++++++++++++++++++++++++ | ||
46 | iothread.c | 65 ++++++------------- | ||
47 | 6 files changed, 192 insertions(+), 64 deletions(-) | ||
48 | create mode 100644 include/sysemu/event-loop-base.h | ||
49 | create mode 100644 event-loop-base.c | ||
50 | |||
51 | diff --git a/qapi/qom.json b/qapi/qom.json | ||
18 | index XXXXXXX..XXXXXXX 100644 | 52 | index XXXXXXX..XXXXXXX 100644 |
19 | --- a/blockjob.c | 53 | --- a/qapi/qom.json |
20 | +++ b/blockjob.c | 54 | +++ b/qapi/qom.json |
21 | @@ -XXX,XX +XXX,XX @@ static const BdrvChildRole child_job = { | 55 | @@ -XXX,XX +XXX,XX @@ |
22 | .stay_at_node = true, | 56 | '*repeat': 'bool', |
57 | '*grab-toggle': 'GrabToggleKeys' } } | ||
58 | |||
59 | +## | ||
60 | +# @EventLoopBaseProperties: | ||
61 | +# | ||
62 | +# Common properties for event loops | ||
63 | +# | ||
64 | +# @aio-max-batch: maximum number of requests in a batch for the AIO engine, | ||
65 | +# 0 means that the engine will use its default. | ||
66 | +# (default: 0) | ||
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; | ||
23 | }; | 226 | }; |
24 | 227 | typedef struct IOThread IOThread; | |
25 | +static void block_job_drained_begin(void *opaque) | 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) | ||
26 | +{ | 264 | +{ |
27 | + BlockJob *job = opaque; | 265 | + EventLoopBase *event_loop_base = EVENT_LOOP_BASE(obj); |
28 | + block_job_pause(job); | 266 | + EventLoopBaseParamInfo *info = opaque; |
267 | + int64_t *field = (void *)event_loop_base + info->offset; | ||
268 | + | ||
269 | + visit_type_int64(v, name, field, errp); | ||
29 | +} | 270 | +} |
30 | + | 271 | + |
31 | +static void block_job_drained_end(void *opaque) | 272 | +static void event_loop_base_set_param(Object *obj, Visitor *v, |
273 | + const char *name, void *opaque, Error **errp) | ||
32 | +{ | 274 | +{ |
33 | + BlockJob *job = opaque; | 275 | + EventLoopBaseClass *bc = EVENT_LOOP_BASE_GET_CLASS(obj); |
34 | + block_job_resume(job); | 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; | ||
35 | +} | 298 | +} |
36 | + | 299 | + |
37 | +static const BlockDevOps block_job_dev_ops = { | 300 | +static void event_loop_base_complete(UserCreatable *uc, Error **errp) |
38 | + .drained_begin = block_job_drained_begin, | 301 | +{ |
39 | + .drained_end = block_job_drained_end, | 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 | + } | ||
40 | +}; | 332 | +}; |
41 | + | 333 | + |
42 | BlockJob *block_job_next(BlockJob *job) | 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) | ||
43 | { | 357 | { |
44 | if (!job) { | 358 | + IOThread *iothread = IOTHREAD(base); |
45 | @@ -XXX,XX +XXX,XX @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver, | 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) | ||
46 | } | 369 | } |
47 | 370 | ||
48 | job = g_malloc0(driver->instance_size); | 371 | aio_context_set_aio_params(iothread->ctx, |
49 | - error_setg(&job->blocker, "block device is in use by block job: %s", | 372 | - iothread->aio_max_batch, |
50 | - BlockJobType_lookup[driver->job_type]); | 373 | + iothread->parent_obj.aio_max_batch, |
51 | - block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort); | 374 | errp); |
52 | - bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker); | 375 | } |
53 | - | 376 | |
54 | job->driver = driver; | 377 | -static void iothread_complete(UserCreatable *obj, Error **errp) |
55 | job->id = g_strdup(job_id); | 378 | + |
56 | job->blk = blk; | 379 | +static void iothread_init(EventLoopBase *base, Error **errp) |
57 | @@ -XXX,XX +XXX,XX @@ void *block_job_create(const char *job_id, const BlockJobDriver *driver, | 380 | { |
58 | job->paused = true; | 381 | Error *local_error = NULL; |
59 | job->pause_count = 1; | 382 | - IOThread *iothread = IOTHREAD(obj); |
60 | job->refcnt = 1; | 383 | + IOThread *iothread = IOTHREAD(base); |
61 | + | 384 | char *thread_name; |
62 | + error_setg(&job->blocker, "block device is in use by block job: %s", | 385 | |
63 | + BlockJobType_lookup[driver->job_type]); | 386 | iothread->stopping = false; |
64 | + block_job_add_bdrv(job, "main node", bs, 0, BLK_PERM_ALL, &error_abort); | 387 | @@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp) |
65 | bs->job = job; | 388 | */ |
66 | 389 | iothread_init_gcontext(iothread); | |
67 | + blk_set_dev_ops(blk, &block_job_dev_ops, job); | 390 | |
68 | + bdrv_op_unblock(bs, BLOCK_OP_TYPE_DATAPLANE, job->blocker); | 391 | - iothread_set_aio_context_params(iothread, &local_error); |
69 | + | 392 | + iothread_set_aio_context_params(base, &local_error); |
70 | QLIST_INSERT_HEAD(&block_jobs, job, job_list); | 393 | if (local_error) { |
71 | 394 | error_propagate(errp, local_error); | |
72 | blk_add_aio_context_notifier(blk, block_job_attached_aio_context, | 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; | ||
73 | -- | 489 | -- |
74 | 2.9.3 | 490 | 2.35.1 |
75 | |||
76 | diff view generated by jsdifflib |
1 | From: John Snow <jsnow@redhat.com> | 1 | From: Nicolas Saenz Julienne <nsaenzju@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Allow block backends to forward drain requests to their devices/users. | 3 | 'event-loop-base' provides basic property handling for all 'AioContext' |
4 | The initial intended purpose for this patch is to allow BBs to forward | 4 | based event loops. So let's define a new 'MainLoopClass' that inherits |
5 | requests along to BlockJobs, which will want to pause if their associated | 5 | from it. This will permit tweaking the main loop's properties through |
6 | BB has entered a drained region. | 6 | qapi as well as through the command line using the '-object' keyword[1]. |
7 | 7 | Only one instance of 'MainLoopClass' might be created at any time. | |
8 | Signed-off-by: John Snow <jsnow@redhat.com> | 8 | |
9 | Reviewed-by: Jeff Cody <jcody@redhat.com> | 9 | 'EventLoopBaseClass' learns a new callback, 'can_be_deleted()' so as to |
10 | Message-id: 20170316212351.13797-3-jsnow@redhat.com | 10 | mark 'MainLoop' as non-deletable. |
11 | Signed-off-by: Jeff Cody <jcody@redhat.com> | 11 | |
12 | [1] For example: | ||
13 | -object main-loop,id=main-loop,aio-max-batch=<value> | ||
14 | |||
15 | Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com> | ||
16 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
17 | Acked-by: Markus Armbruster <armbru@redhat.com> | ||
18 | Message-id: 20220425075723.20019-3-nsaenzju@redhat.com | ||
19 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
12 | --- | 20 | --- |
13 | block/block-backend.c | 24 ++++++++++++++++++++++-- | 21 | qapi/qom.json | 13 ++++++++ |
14 | include/sysemu/block-backend.h | 8 ++++++++ | 22 | meson.build | 3 +- |
15 | 2 files changed, 30 insertions(+), 2 deletions(-) | 23 | include/qemu/main-loop.h | 10 ++++++ |
16 | 24 | include/sysemu/event-loop-base.h | 1 + | |
17 | diff --git a/block/block-backend.c b/block/block-backend.c | 25 | event-loop-base.c | 13 ++++++++ |
18 | index XXXXXXX..XXXXXXX 100644 | 26 | util/main-loop.c | 56 ++++++++++++++++++++++++++++++++ |
19 | --- a/block/block-backend.c | 27 | 6 files changed, 95 insertions(+), 1 deletion(-) |
20 | +++ b/block/block-backend.c | 28 | |
21 | @@ -XXX,XX +XXX,XX @@ struct BlockBackend { | 29 | diff --git a/qapi/qom.json b/qapi/qom.json |
22 | bool allow_write_beyond_eof; | 30 | index XXXXXXX..XXXXXXX 100644 |
23 | 31 | --- a/qapi/qom.json | |
24 | NotifierList remove_bs_notifiers, insert_bs_notifiers; | 32 | +++ b/qapi/qom.json |
25 | + | 33 | @@ -XXX,XX +XXX,XX @@ |
26 | + int quiesce_counter; | 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); | ||
27 | }; | 114 | }; |
28 | 115 | ||
29 | typedef struct BlockBackendAIOCB { | 116 | struct EventLoopBase { |
30 | @@ -XXX,XX +XXX,XX @@ void blk_set_dev_ops(BlockBackend *blk, const BlockDevOps *ops, | 117 | diff --git a/event-loop-base.c b/event-loop-base.c |
31 | void *opaque) | 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) | ||
32 | { | 138 | { |
33 | /* All drivers that use blk_set_dev_ops() are qdevified and we want to keep | 139 | UserCreatableClass *ucc = USER_CREATABLE_CLASS(klass); |
34 | - * it that way, so we can assume blk->dev is a DeviceState if blk->dev_ops | 140 | ucc->complete = event_loop_base_complete; |
35 | - * is set. */ | 141 | + ucc->can_be_deleted = event_loop_base_can_be_deleted; |
36 | + * it that way, so we can assume blk->dev, if present, is a DeviceState if | 142 | |
37 | + * blk->dev_ops is set. Non-device users may use dev_ops without device. */ | 143 | object_class_property_add(klass, "aio-max-batch", "int", |
38 | assert(!blk->legacy_dev); | 144 | event_loop_base_get_param, |
39 | 145 | diff --git a/util/main-loop.c b/util/main-loop.c | |
40 | blk->dev_ops = ops; | 146 | index XXXXXXX..XXXXXXX 100644 |
41 | blk->dev_opaque = opaque; | 147 | --- a/util/main-loop.c |
42 | + | 148 | +++ b/util/main-loop.c |
43 | + /* Are we currently quiesced? Should we enforce this right now? */ | 149 | @@ -XXX,XX +XXX,XX @@ |
44 | + if (blk->quiesce_counter && ops->drained_begin) { | 150 | #include "qemu/error-report.h" |
45 | + ops->drained_begin(opaque); | 151 | #include "qemu/queue.h" |
152 | #include "qemu/compiler.h" | ||
153 | +#include "qom/object.h" | ||
154 | |||
155 | #ifndef _WIN32 | ||
156 | #include <sys/wait.h> | ||
157 | @@ -XXX,XX +XXX,XX @@ int qemu_init_main_loop(Error **errp) | ||
158 | return 0; | ||
159 | } | ||
160 | |||
161 | +static void main_loop_update_params(EventLoopBase *base, Error **errp) | ||
162 | +{ | ||
163 | + if (!qemu_aio_context) { | ||
164 | + error_setg(errp, "qemu aio context not ready"); | ||
165 | + return; | ||
46 | + } | 166 | + } |
47 | } | 167 | + |
48 | 168 | + aio_context_set_aio_params(qemu_aio_context, base->aio_max_batch, errp); | |
49 | /* | 169 | +} |
50 | @@ -XXX,XX +XXX,XX @@ static void blk_root_drained_begin(BdrvChild *child) | 170 | + |
51 | { | 171 | +MainLoop *mloop; |
52 | BlockBackend *blk = child->opaque; | 172 | + |
53 | 173 | +static void main_loop_init(EventLoopBase *base, Error **errp) | |
54 | + if (++blk->quiesce_counter == 1) { | 174 | +{ |
55 | + if (blk->dev_ops && blk->dev_ops->drained_begin) { | 175 | + MainLoop *m = MAIN_LOOP(base); |
56 | + blk->dev_ops->drained_begin(blk->dev_opaque); | 176 | + |
57 | + } | 177 | + if (mloop) { |
178 | + error_setg(errp, "only one main-loop instance allowed"); | ||
179 | + return; | ||
58 | + } | 180 | + } |
59 | + | 181 | + |
60 | /* Note that blk->root may not be accessible here yet if we are just | 182 | + main_loop_update_params(base, errp); |
61 | * attaching to a BlockDriverState that is drained. Use child instead. */ | 183 | + |
62 | 184 | + mloop = m; | |
63 | @@ -XXX,XX +XXX,XX @@ static void blk_root_drained_begin(BdrvChild *child) | 185 | + return; |
64 | static void blk_root_drained_end(BdrvChild *child) | 186 | +} |
65 | { | 187 | + |
66 | BlockBackend *blk = child->opaque; | 188 | +static bool main_loop_can_be_deleted(EventLoopBase *base) |
67 | + assert(blk->quiesce_counter); | 189 | +{ |
68 | 190 | + return false; | |
69 | assert(blk->public.io_limits_disabled); | 191 | +} |
70 | --blk->public.io_limits_disabled; | 192 | + |
71 | + | 193 | +static void main_loop_class_init(ObjectClass *oc, void *class_data) |
72 | + if (--blk->quiesce_counter == 0) { | 194 | +{ |
73 | + if (blk->dev_ops && blk->dev_ops->drained_end) { | 195 | + EventLoopBaseClass *bc = EVENT_LOOP_BASE_CLASS(oc); |
74 | + blk->dev_ops->drained_end(blk->dev_opaque); | 196 | + |
75 | + } | 197 | + bc->init = main_loop_init; |
76 | + } | 198 | + bc->update_params = main_loop_update_params; |
77 | } | 199 | + bc->can_be_deleted = main_loop_can_be_deleted; |
78 | diff --git a/include/sysemu/block-backend.h b/include/sysemu/block-backend.h | 200 | +} |
79 | index XXXXXXX..XXXXXXX 100644 | 201 | + |
80 | --- a/include/sysemu/block-backend.h | 202 | +static const TypeInfo main_loop_info = { |
81 | +++ b/include/sysemu/block-backend.h | 203 | + .name = TYPE_MAIN_LOOP, |
82 | @@ -XXX,XX +XXX,XX @@ typedef struct BlockDevOps { | 204 | + .parent = TYPE_EVENT_LOOP_BASE, |
83 | * Runs when the size changed (e.g. monitor command block_resize) | 205 | + .class_init = main_loop_class_init, |
84 | */ | 206 | + .instance_size = sizeof(MainLoop), |
85 | void (*resize_cb)(void *opaque); | 207 | +}; |
86 | + /* | 208 | + |
87 | + * Runs when the backend receives a drain request. | 209 | +static void main_loop_register_types(void) |
88 | + */ | 210 | +{ |
89 | + void (*drained_begin)(void *opaque); | 211 | + type_register_static(&main_loop_info); |
90 | + /* | 212 | +} |
91 | + * Runs when the backend's last drain request ends. | 213 | + |
92 | + */ | 214 | +type_init(main_loop_register_types) |
93 | + void (*drained_end)(void *opaque); | 215 | + |
94 | } BlockDevOps; | 216 | static int max_priority; |
95 | 217 | ||
96 | /* This struct is embedded in (the private) BlockBackend struct and contains | 218 | #ifndef _WIN32 |
97 | -- | 219 | -- |
98 | 2.9.3 | 220 | 2.35.1 |
99 | |||
100 | diff view generated by jsdifflib |
1 | From: John Snow <jsnow@redhat.com> | 1 | From: Nicolas Saenz Julienne <nsaenzju@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | The purpose of this shim is to allow us to pause pre-started jobs. | 3 | The thread pool regulates itself: when idle, it kills threads until |
4 | The purpose of *that* is to allow us to buffer a pause request that | 4 | empty, when in demand, it creates new threads until full. This behaviour |
5 | will be able to take effect before the job ever does any work, allowing | 5 | doesn't play well with latency sensitive workloads where the price of |
6 | us to create jobs during a quiescent state (under which they will be | 6 | creating a new thread is too high. For example, when paired with qemu's |
7 | automatically paused), then resuming the jobs after the critical section | 7 | '-mlock', or using safety features like SafeStack, creating a new thread |
8 | in any order, either: | 8 | has been measured take multiple milliseconds. |
9 | 9 | ||
10 | (1) -block_job_start | 10 | In order to mitigate this let's introduce a new 'EventLoopBase' |
11 | -block_job_resume (via e.g. drained_end) | 11 | property to set the thread pool size. The threads will be created during |
12 | the pool's initialization or upon updating the property's value, remain | ||
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. | ||
12 | 16 | ||
13 | (2) -block_job_resume (via e.g. drained_end) | 17 | Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com> |
14 | -block_job_start | 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 | ||
21 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
22 | --- | ||
23 | qapi/qom.json | 10 +++++- | ||
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 | The problem that requires a startup wrapper is the idea that a job must | 35 | diff --git a/qapi/qom.json b/qapi/qom.json |
17 | start in the busy=true state only its first time-- all subsequent entries | 36 | index XXXXXXX..XXXXXXX 100644 |
18 | require busy to be false, and the toggling of this state is otherwise | 37 | --- a/qapi/qom.json |
19 | handled during existing pause and yield points. | 38 | +++ b/qapi/qom.json |
20 | 39 | @@ -XXX,XX +XXX,XX @@ | |
21 | The wrapper simply allows us to mandate that a job can "start," set busy | 40 | # 0 means that the engine will use its default. |
22 | to true, then immediately pause only if necessary. We could avoid | 41 | # (default: 0) |
23 | requiring a wrapper, but all jobs would need to do it, so it's been | 42 | # |
24 | factored out here. | 43 | +# @thread-pool-min: minimum number of threads reserved in the thread pool |
25 | 44 | +# (default:0) | |
26 | Signed-off-by: John Snow <jsnow@redhat.com> | 45 | +# |
27 | Reviewed-by: Jeff Cody <jcody@redhat.com> | 46 | +# @thread-pool-max: maximum number of threads the thread pool can contain |
28 | Message-id: 20170316212351.13797-2-jsnow@redhat.com | 47 | +# (default:64) |
29 | Signed-off-by: Jeff Cody <jcody@redhat.com> | 48 | +# |
30 | --- | 49 | # Since: 7.1 |
31 | blockjob.c | 26 +++++++++++++++++++------- | 50 | ## |
32 | 1 file changed, 19 insertions(+), 7 deletions(-) | 51 | { 'struct': 'EventLoopBaseProperties', |
33 | 52 | - 'data': { '*aio-max-batch': 'int' } } | |
34 | diff --git a/blockjob.c b/blockjob.c | 53 | + 'data': { '*aio-max-batch': 'int', |
35 | index XXXXXXX..XXXXXXX 100644 | 54 | + '*thread-pool-min': 'int', |
36 | --- a/blockjob.c | 55 | + '*thread-pool-max': 'int' } } |
37 | +++ b/blockjob.c | 56 | |
38 | @@ -XXX,XX +XXX,XX @@ static bool block_job_started(BlockJob *job) | 57 | ## |
39 | return job->co; | 58 | # @IothreadProperties: |
40 | } | 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); | ||
41 | 75 | ||
42 | +/** | 76 | +/** |
43 | + * All jobs must allow a pause point before entering their job proper. This | 77 | + * aio_context_set_thread_pool_params: |
44 | + * ensures that jobs can be paused prior to being started, then resumed later. | 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 | ||
45 | + */ | 81 | + */ |
46 | +static void coroutine_fn block_job_co_entry(void *opaque) | 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) | ||
47 | +{ | 136 | +{ |
48 | + BlockJob *job = opaque; | 137 | + EventLoopBase *base = EVENT_LOOP_BASE(obj); |
49 | + | 138 | + |
50 | + assert(job && job->driver && job->driver->start); | 139 | + base->thread_pool_max = THREAD_POOL_MAX_THREADS_DEFAULT; |
51 | + block_job_pause_point(job); | ||
52 | + job->driver->start(job); | ||
53 | +} | 140 | +} |
54 | + | 141 | + |
55 | void block_job_start(BlockJob *job) | 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) | ||
56 | { | 252 | { |
57 | assert(job && !block_job_started(job) && job->paused && | 253 | + ERRP_GUARD(); |
58 | - !job->busy && job->driver->start); | 254 | + |
59 | - job->co = qemu_coroutine_create(job->driver->start, job); | 255 | if (!qemu_aio_context) { |
60 | - if (--job->pause_count == 0) { | 256 | error_setg(errp, "qemu aio context not ready"); |
61 | - job->paused = false; | 257 | return; |
62 | - job->busy = true; | 258 | } |
63 | - qemu_coroutine_enter(job->co); | 259 | |
64 | - } | 260 | aio_context_set_aio_params(qemu_aio_context, base->aio_max_batch, errp); |
65 | + job->driver && job->driver->start); | 261 | + if (*errp) { |
66 | + job->co = qemu_coroutine_create(block_job_co_entry, job); | 262 | + return; |
67 | + job->pause_count--; | 263 | + } |
68 | + job->busy = true; | 264 | + |
69 | + job->paused = false; | 265 | + aio_context_set_thread_pool_params(qemu_aio_context, base->thread_pool_min, |
70 | + qemu_coroutine_enter(job->co); | 266 | + base->thread_pool_max, errp); |
71 | } | 267 | } |
72 | 268 | ||
73 | void block_job_ref(BlockJob *job) | 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 | +{ | ||
292 | + /* | ||
293 | + * The semaphore timed out, we should exit the loop except when: | ||
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. | ||
297 | + */ | ||
298 | + if (ret == -1 && (!QTAILQ_EMPTY(&pool->request_list) || | ||
299 | + pool->cur_threads > pool->max_threads || | ||
300 | + pool->cur_threads <= pool->min_threads)) { | ||
301 | + return true; | ||
302 | + } | ||
303 | + | ||
304 | + return false; | ||
305 | +} | ||
306 | + | ||
307 | static void *worker_thread(void *opaque) | ||
308 | { | ||
309 | ThreadPool *pool = opaque; | ||
310 | @@ -XXX,XX +XXX,XX @@ static void *worker_thread(void *opaque) | ||
311 | ret = qemu_sem_timedwait(&pool->sem, 10000); | ||
312 | qemu_mutex_lock(&pool->lock); | ||
313 | pool->idle_threads--; | ||
314 | - } while (ret == -1 && !QTAILQ_EMPTY(&pool->request_list)); | ||
315 | - if (ret == -1 || pool->stopping) { | ||
316 | + } while (back_to_sleep(pool, ret)); | ||
317 | + if (ret == -1 || pool->stopping || | ||
318 | + pool->cur_threads > pool->max_threads) { | ||
319 | break; | ||
320 | } | ||
321 | |||
322 | @@ -XXX,XX +XXX,XX @@ void thread_pool_submit(ThreadPool *pool, ThreadPoolFunc *func, void *arg) | ||
323 | thread_pool_submit_aio(pool, func, arg, NULL, NULL); | ||
324 | } | ||
325 | |||
326 | +void thread_pool_update_params(ThreadPool *pool, AioContext *ctx) | ||
327 | +{ | ||
328 | + qemu_mutex_lock(&pool->lock); | ||
329 | + | ||
330 | + pool->min_threads = ctx->thread_pool_min; | ||
331 | + pool->max_threads = ctx->thread_pool_max; | ||
332 | + | ||
333 | + /* | ||
334 | + * We either have to: | ||
335 | + * - Increase the number available of threads until over the min_threads | ||
336 | + * threshold. | ||
337 | + * - Decrease the number of available threads until under the max_threads | ||
338 | + * threshold. | ||
339 | + * - Do nothing. The current number of threads fall in between the min and | ||
340 | + * max thresholds. We'll let the pool manage itself. | ||
341 | + */ | ||
342 | + for (int i = pool->cur_threads; i < pool->min_threads; i++) { | ||
343 | + spawn_thread(pool); | ||
344 | + } | ||
345 | + | ||
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) | ||
74 | -- | 370 | -- |
75 | 2.9.3 | 371 | 2.35.1 |
76 | |||
77 | diff view generated by jsdifflib |