1 | The following changes since commit ca61fa4b803e5d0abaf6f1ceb690f23bb78a4def: | 1 | The following changes since commit 554623226f800acf48a2ed568900c1c968ec9a8b: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/quic/tags/pull-hex-20211006' into staging (2021-10-06 12:11:14 -0700) | 3 | Merge tag 'qemu-sparc-20220508' of https://github.com/mcayland/qemu into staging (2022-05-08 17:03:26 -0500) |
4 | 4 | ||
5 | are available in the Git repository at: | 5 | are available in the Git repository at: |
6 | 6 | ||
7 | https://gitlab.com/stefanha/qemu.git tags/block-pull-request | 7 | https://gitlab.com/stefanha/qemu.git tags/block-pull-request |
8 | 8 | ||
9 | for you to fetch changes up to 1cc7eada97914f090125e588497986f6f7900514: | 9 | for you to fetch changes up to 3dc584abeef0e1277c2de8c1c1974cb49444eb0a: |
10 | 10 | ||
11 | iothread: use IOThreadParamInfo in iothread_[set|get]_param() (2021-10-07 15:29:50 +0100) | 11 | virtio-scsi: move request-related items from .h to .c (2022-05-09 10:45:04 +0100) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Pull request | 14 | Pull request |
15 | 15 | ||
16 | - Add new thread-pool-min/thread-pool-max parameters to control the thread pool | ||
17 | used for async I/O. | ||
18 | |||
19 | - Fix virtio-scsi IOThread 100% CPU consumption QEMU 7.0 regression. | ||
20 | |||
16 | ---------------------------------------------------------------- | 21 | ---------------------------------------------------------------- |
17 | 22 | ||
18 | Stefano Garzarella (2): | 23 | Nicolas Saenz Julienne (3): |
19 | iothread: rename PollParamInfo to IOThreadParamInfo | 24 | Introduce event-loop-base abstract class |
20 | iothread: use IOThreadParamInfo in iothread_[set|get]_param() | 25 | util/main-loop: Introduce the main loop into QOM |
26 | util/event-loop-base: Introduce options to set the thread pool size | ||
21 | 27 | ||
22 | iothread.c | 28 +++++++++++++++------------- | 28 | Stefan Hajnoczi (6): |
23 | 1 file changed, 15 insertions(+), 13 deletions(-) | 29 | virtio-scsi: fix ctrl and event handler functions in dataplane mode |
30 | virtio-scsi: don't waste CPU polling the event virtqueue | ||
31 | virtio-scsi: clean up virtio_scsi_handle_event_vq() | ||
32 | virtio-scsi: clean up virtio_scsi_handle_ctrl_vq() | ||
33 | virtio-scsi: clean up virtio_scsi_handle_cmd_vq() | ||
34 | virtio-scsi: move request-related items from .h to .c | ||
35 | |||
36 | qapi/qom.json | 43 ++++++++-- | ||
37 | meson.build | 26 +++--- | ||
38 | include/block/aio.h | 10 +++ | ||
39 | include/block/thread-pool.h | 3 + | ||
40 | include/hw/virtio/virtio-scsi.h | 43 ---------- | ||
41 | include/hw/virtio/virtio.h | 1 + | ||
42 | include/qemu/main-loop.h | 10 +++ | ||
43 | include/sysemu/event-loop-base.h | 41 +++++++++ | ||
44 | include/sysemu/iothread.h | 6 +- | ||
45 | event-loop-base.c | 140 +++++++++++++++++++++++++++++++ | ||
46 | hw/scsi/virtio-scsi-dataplane.c | 2 +- | ||
47 | hw/scsi/virtio-scsi.c | 101 +++++++++++++++------- | ||
48 | hw/virtio/virtio.c | 13 +++ | ||
49 | iothread.c | 68 +++++---------- | ||
50 | util/aio-posix.c | 1 + | ||
51 | util/async.c | 20 +++++ | ||
52 | util/main-loop.c | 65 ++++++++++++++ | ||
53 | util/thread-pool.c | 55 +++++++++++- | ||
54 | 18 files changed, 505 insertions(+), 143 deletions(-) | ||
55 | create mode 100644 include/sysemu/event-loop-base.h | ||
56 | create mode 100644 event-loop-base.c | ||
24 | 57 | ||
25 | -- | 58 | -- |
26 | 2.31.1 | 59 | 2.35.1 |
27 | |||
28 | |||
29 | diff view generated by jsdifflib |
1 | From: Stefano Garzarella <sgarzare@redhat.com> | 1 | From: Nicolas Saenz Julienne <nsaenzju@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Commit 1793ad0247 ("iothread: add aio-max-batch parameter") added | 3 | Introduce the 'event-loop-base' abstract class, it'll hold the |
4 | a new parameter (aio-max-batch) to IOThread and used PollParamInfo | 4 | properties common to all event loops and provide the necessary hooks for |
5 | structure to handle it. | 5 | their creation and maintenance. Then have iothread inherit from it. |
6 | 6 | ||
7 | Since it is not a parameter of the polling mechanism, we rename the | 7 | EventLoopBaseClass is defined as user creatable and provides a hook for |
8 | structure to a more generic IOThreadParamInfo. | 8 | its children to attach themselves to the user creatable class 'complete' |
9 | 9 | function. It also provides an update_params() callback to propagate | |
10 | Suggested-by: Kevin Wolf <kwolf@redhat.com> | 10 | property changes onto its children. |
11 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> | 11 | |
12 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 12 | The new 'event-loop-base' class will live in the root directory. It is |
13 | Message-id: 20210727145936.147032-2-sgarzare@redhat.com | 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 | ||
14 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 39 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
15 | --- | 40 | --- |
16 | iothread.c | 14 +++++++------- | 41 | qapi/qom.json | 22 +++++-- |
17 | 1 file changed, 7 insertions(+), 7 deletions(-) | 42 | meson.build | 23 ++++--- |
18 | 43 | include/sysemu/event-loop-base.h | 36 +++++++++++ | |
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 | ||
52 | index XXXXXXX..XXXXXXX 100644 | ||
53 | --- a/qapi/qom.json | ||
54 | +++ b/qapi/qom.json | ||
55 | @@ -XXX,XX +XXX,XX @@ | ||
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; | ||
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); | ||
19 | diff --git a/iothread.c b/iothread.c | 339 | diff --git a/iothread.c b/iothread.c |
20 | index XXXXXXX..XXXXXXX 100644 | 340 | index XXXXXXX..XXXXXXX 100644 |
21 | --- a/iothread.c | 341 | --- a/iothread.c |
22 | +++ b/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; | ||
23 | @@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp) | 387 | @@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp) |
24 | typedef struct { | 388 | */ |
25 | const char *name; | 389 | iothread_init_gcontext(iothread); |
26 | ptrdiff_t offset; /* field's byte offset in IOThread struct */ | 390 | |
27 | -} PollParamInfo; | 391 | - iothread_set_aio_context_params(iothread, &local_error); |
28 | +} IOThreadParamInfo; | 392 | + iothread_set_aio_context_params(base, &local_error); |
29 | 393 | if (local_error) { | |
30 | -static PollParamInfo poll_max_ns_info = { | 394 | error_propagate(errp, local_error); |
31 | +static IOThreadParamInfo poll_max_ns_info = { | 395 | aio_context_unref(iothread->ctx); |
32 | "poll-max-ns", offsetof(IOThread, poll_max_ns), | 396 | @@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp) |
33 | }; | 397 | * to inherit. |
34 | -static PollParamInfo poll_grow_info = { | 398 | */ |
35 | +static IOThreadParamInfo poll_grow_info = { | 399 | thread_name = g_strdup_printf("IO %s", |
36 | "poll-grow", offsetof(IOThread, poll_grow), | 400 | - object_get_canonical_path_component(OBJECT(obj))); |
37 | }; | 401 | + object_get_canonical_path_component(OBJECT(base))); |
38 | -static PollParamInfo poll_shrink_info = { | 402 | qemu_thread_create(&iothread->thread, thread_name, iothread_run, |
39 | +static IOThreadParamInfo poll_shrink_info = { | 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 = { | ||
40 | "poll-shrink", offsetof(IOThread, poll_shrink), | 407 | "poll-shrink", offsetof(IOThread, poll_shrink), |
41 | }; | 408 | }; |
42 | -static PollParamInfo aio_max_batch_info = { | 409 | -static IOThreadParamInfo aio_max_batch_info = { |
43 | +static IOThreadParamInfo aio_max_batch_info = { | 410 | - "aio-max-batch", offsetof(IOThread, aio_max_batch), |
44 | "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 | - }, | ||
45 | }; | 477 | }; |
46 | 478 | ||
47 | @@ -XXX,XX +XXX,XX @@ static void iothread_get_param(Object *obj, Visitor *v, | 479 | static void iothread_register_types(void) |
48 | const char *name, void *opaque, Error **errp) | 480 | @@ -XXX,XX +XXX,XX @@ static int query_one_iothread(Object *object, void *opaque) |
49 | { | 481 | info->poll_max_ns = iothread->poll_max_ns; |
50 | IOThread *iothread = IOTHREAD(obj); | 482 | info->poll_grow = iothread->poll_grow; |
51 | - PollParamInfo *info = opaque; | 483 | info->poll_shrink = iothread->poll_shrink; |
52 | + IOThreadParamInfo *info = opaque; | 484 | - info->aio_max_batch = iothread->aio_max_batch; |
53 | int64_t *field = (void *)iothread + info->offset; | 485 | + info->aio_max_batch = iothread->parent_obj.aio_max_batch; |
54 | 486 | ||
55 | visit_type_int64(v, name, field, errp); | 487 | QAPI_LIST_APPEND(*tail, info); |
56 | @@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v, | 488 | return 0; |
57 | const char *name, void *opaque, Error **errp) | ||
58 | { | ||
59 | IOThread *iothread = IOTHREAD(obj); | ||
60 | - PollParamInfo *info = opaque; | ||
61 | + IOThreadParamInfo *info = opaque; | ||
62 | int64_t *field = (void *)iothread + info->offset; | ||
63 | int64_t value; | ||
64 | |||
65 | -- | 489 | -- |
66 | 2.31.1 | 490 | 2.35.1 |
67 | |||
68 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | 1 | From: Nicolas Saenz Julienne <nsaenzju@redhat.com> | |
2 | |||
3 | 'event-loop-base' provides basic property handling for all 'AioContext' | ||
4 | based event loops. So let's define a new 'MainLoopClass' that inherits | ||
5 | from it. This will permit tweaking the main loop's properties through | ||
6 | qapi as well as through the command line using the '-object' keyword[1]. | ||
7 | Only one instance of 'MainLoopClass' might be created at any time. | ||
8 | |||
9 | 'EventLoopBaseClass' learns a new callback, 'can_be_deleted()' so as to | ||
10 | mark 'MainLoop' as non-deletable. | ||
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> | ||
20 | --- | ||
21 | qapi/qom.json | 13 ++++++++ | ||
22 | meson.build | 3 +- | ||
23 | include/qemu/main-loop.h | 10 ++++++ | ||
24 | include/sysemu/event-loop-base.h | 1 + | ||
25 | event-loop-base.c | 13 ++++++++ | ||
26 | util/main-loop.c | 56 ++++++++++++++++++++++++++++++++ | ||
27 | 6 files changed, 95 insertions(+), 1 deletion(-) | ||
28 | |||
29 | diff --git a/qapi/qom.json b/qapi/qom.json | ||
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 @@ | ||
150 | #include "qemu/error-report.h" | ||
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; | ||
166 | + } | ||
167 | + | ||
168 | + aio_context_set_aio_params(qemu_aio_context, base->aio_max_batch, errp); | ||
169 | +} | ||
170 | + | ||
171 | +MainLoop *mloop; | ||
172 | + | ||
173 | +static void main_loop_init(EventLoopBase *base, Error **errp) | ||
174 | +{ | ||
175 | + MainLoop *m = MAIN_LOOP(base); | ||
176 | + | ||
177 | + if (mloop) { | ||
178 | + error_setg(errp, "only one main-loop instance allowed"); | ||
179 | + return; | ||
180 | + } | ||
181 | + | ||
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 | ||
219 | -- | ||
220 | 2.35.1 | diff view generated by jsdifflib |
1 | From: Stefano Garzarella <sgarzare@redhat.com> | 1 | From: Nicolas Saenz Julienne <nsaenzju@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Commit 0445409d74 ("iothread: generalize | 3 | The thread pool regulates itself: when idle, it kills threads until |
4 | iothread_set_param/iothread_get_param") moved common code to set and | 4 | empty, when in demand, it creates new threads until full. This behaviour |
5 | get IOThread parameters in two new functions. | 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 | These functions are called inside callbacks, so we don't need to use an | 10 | In order to mitigate this let's introduce a new 'EventLoopBase' |
8 | opaque pointer. Let's replace `void *opaque` parameter with | 11 | property to set the thread pool size. The threads will be created during |
9 | `IOThreadParamInfo *info`. | 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. | ||
10 | 16 | ||
11 | Suggested-by: Kevin Wolf <kwolf@redhat.com> | 17 | Signed-off-by: Nicolas Saenz Julienne <nsaenzju@redhat.com> |
12 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> | 18 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> |
13 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 19 | Acked-by: Markus Armbruster <armbru@redhat.com> |
14 | Message-id: 20210727145936.147032-3-sgarzare@redhat.com | 20 | Message-id: 20220425075723.20019-4-nsaenzju@redhat.com |
15 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 21 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
16 | --- | 22 | --- |
17 | iothread.c | 18 ++++++++++-------- | 23 | qapi/qom.json | 10 +++++- |
18 | 1 file changed, 10 insertions(+), 8 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(-) | ||
19 | 34 | ||
35 | diff --git a/qapi/qom.json b/qapi/qom.json | ||
36 | index XXXXXXX..XXXXXXX 100644 | ||
37 | --- a/qapi/qom.json | ||
38 | +++ b/qapi/qom.json | ||
39 | @@ -XXX,XX +XXX,XX @@ | ||
40 | # 0 means that the engine will use its default. | ||
41 | # (default: 0) | ||
42 | # | ||
43 | +# @thread-pool-min: minimum number of threads reserved in the thread pool | ||
44 | +# (default:0) | ||
45 | +# | ||
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, | ||
20 | diff --git a/iothread.c b/iothread.c | 176 | diff --git a/iothread.c b/iothread.c |
21 | index XXXXXXX..XXXXXXX 100644 | 177 | index XXXXXXX..XXXXXXX 100644 |
22 | --- a/iothread.c | 178 | --- a/iothread.c |
23 | +++ b/iothread.c | 179 | +++ b/iothread.c |
24 | @@ -XXX,XX +XXX,XX @@ static IOThreadParamInfo aio_max_batch_info = { | 180 | @@ -XXX,XX +XXX,XX @@ static void iothread_set_aio_context_params(EventLoopBase *base, Error **errp) |
25 | }; | 181 | aio_context_set_aio_params(iothread->ctx, |
26 | 182 | iothread->parent_obj.aio_max_batch, | |
27 | static void iothread_get_param(Object *obj, Visitor *v, | 183 | errp); |
28 | - const char *name, void *opaque, Error **errp) | 184 | + |
29 | + const char *name, IOThreadParamInfo *info, Error **errp) | 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) | ||
30 | { | 252 | { |
31 | IOThread *iothread = IOTHREAD(obj); | 253 | + ERRP_GUARD(); |
32 | - IOThreadParamInfo *info = opaque; | 254 | + |
33 | int64_t *field = (void *)iothread + info->offset; | 255 | if (!qemu_aio_context) { |
34 | 256 | error_setg(errp, "qemu aio context not ready"); | |
35 | visit_type_int64(v, name, field, errp); | ||
36 | } | ||
37 | |||
38 | static bool iothread_set_param(Object *obj, Visitor *v, | ||
39 | - const char *name, void *opaque, Error **errp) | ||
40 | + const char *name, IOThreadParamInfo *info, Error **errp) | ||
41 | { | ||
42 | IOThread *iothread = IOTHREAD(obj); | ||
43 | - IOThreadParamInfo *info = opaque; | ||
44 | int64_t *field = (void *)iothread + info->offset; | ||
45 | int64_t value; | ||
46 | |||
47 | @@ -XXX,XX +XXX,XX @@ static bool iothread_set_param(Object *obj, Visitor *v, | ||
48 | static void iothread_get_poll_param(Object *obj, Visitor *v, | ||
49 | const char *name, void *opaque, Error **errp) | ||
50 | { | ||
51 | + IOThreadParamInfo *info = opaque; | ||
52 | |||
53 | - iothread_get_param(obj, v, name, opaque, errp); | ||
54 | + iothread_get_param(obj, v, name, info, errp); | ||
55 | } | ||
56 | |||
57 | static void iothread_set_poll_param(Object *obj, Visitor *v, | ||
58 | const char *name, void *opaque, Error **errp) | ||
59 | { | ||
60 | IOThread *iothread = IOTHREAD(obj); | ||
61 | + IOThreadParamInfo *info = opaque; | ||
62 | |||
63 | - if (!iothread_set_param(obj, v, name, opaque, errp)) { | ||
64 | + if (!iothread_set_param(obj, v, name, info, errp)) { | ||
65 | return; | 257 | return; |
66 | } | 258 | } |
67 | 259 | ||
68 | @@ -XXX,XX +XXX,XX @@ static void iothread_set_poll_param(Object *obj, Visitor *v, | 260 | aio_context_set_aio_params(qemu_aio_context, base->aio_max_batch, errp); |
69 | static void iothread_get_aio_param(Object *obj, Visitor *v, | 261 | + if (*errp) { |
70 | const char *name, void *opaque, Error **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 | +{ | ||
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) | ||
71 | { | 308 | { |
72 | + IOThreadParamInfo *info = opaque; | 309 | ThreadPool *pool = opaque; |
73 | 310 | @@ -XXX,XX +XXX,XX @@ static void *worker_thread(void *opaque) | |
74 | - iothread_get_param(obj, v, name, opaque, errp); | 311 | ret = qemu_sem_timedwait(&pool->sem, 10000); |
75 | + iothread_get_param(obj, v, name, info, errp); | 312 | qemu_mutex_lock(&pool->lock); |
76 | } | 313 | pool->idle_threads--; |
77 | 314 | - } while (ret == -1 && !QTAILQ_EMPTY(&pool->request_list)); | |
78 | static void iothread_set_aio_param(Object *obj, Visitor *v, | 315 | - if (ret == -1 || pool->stopping) { |
79 | const char *name, void *opaque, Error **errp) | 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) | ||
80 | { | 354 | { |
81 | IOThread *iothread = IOTHREAD(obj); | 355 | if (!ctx) { |
82 | + IOThreadParamInfo *info = opaque; | 356 | @@ -XXX,XX +XXX,XX @@ static void thread_pool_init_one(ThreadPool *pool, AioContext *ctx) |
83 | 357 | qemu_mutex_init(&pool->lock); | |
84 | - if (!iothread_set_param(obj, v, name, opaque, errp)) { | 358 | qemu_cond_init(&pool->worker_stopped); |
85 | + if (!iothread_set_param(obj, v, name, info, errp)) { | 359 | qemu_sem_init(&pool->sem, 0); |
86 | return; | 360 | - pool->max_threads = 64; |
87 | } | 361 | pool->new_thread_bh = aio_bh_new(ctx, spawn_thread_bh_fn, pool); |
88 | 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) | ||
89 | -- | 370 | -- |
90 | 2.31.1 | 371 | 2.35.1 |
91 | |||
92 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | Commit f34e8d8b8d48d73f36a67b6d5e492ef9784b5012 ("virtio-scsi: prepare | ||
2 | virtio_scsi_handle_cmd for dataplane") prepared the virtio-scsi cmd | ||
3 | virtqueue handler function to be used in both the dataplane and | ||
4 | non-datpalane code paths. | ||
1 | 5 | ||
6 | It failed to convert the ctrl and event virtqueue handler functions, | ||
7 | which are not designed to be called from the dataplane code path but | ||
8 | will be since the ioeventfd is set up for those virtqueues when | ||
9 | dataplane starts. | ||
10 | |||
11 | Convert the ctrl and event virtqueue handler functions now so they | ||
12 | operate correctly when called from the dataplane code path. Avoid code | ||
13 | duplication by extracting this code into a helper function. | ||
14 | |||
15 | Fixes: f34e8d8b8d48d73f36a67b6d5e492ef9784b5012 ("virtio-scsi: prepare virtio_scsi_handle_cmd for dataplane") | ||
16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
17 | Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> | ||
18 | Message-id: 20220427143541.119567-2-stefanha@redhat.com | ||
19 | [Fixed s/by used/be used/ typo pointed out by Michael Tokarev | ||
20 | <mjt@tls.msk.ru>. | ||
21 | --Stefan] | ||
22 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
23 | --- | ||
24 | hw/scsi/virtio-scsi.c | 42 +++++++++++++++++++++++++++--------------- | ||
25 | 1 file changed, 27 insertions(+), 15 deletions(-) | ||
26 | |||
27 | diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c | ||
28 | index XXXXXXX..XXXXXXX 100644 | ||
29 | --- a/hw/scsi/virtio-scsi.c | ||
30 | +++ b/hw/scsi/virtio-scsi.c | ||
31 | @@ -XXX,XX +XXX,XX @@ bool virtio_scsi_handle_ctrl_vq(VirtIOSCSI *s, VirtQueue *vq) | ||
32 | return progress; | ||
33 | } | ||
34 | |||
35 | +/* | ||
36 | + * If dataplane is configured but not yet started, do so now and return true on | ||
37 | + * success. | ||
38 | + * | ||
39 | + * Dataplane is started by the core virtio code but virtqueue handler functions | ||
40 | + * can also be invoked when a guest kicks before DRIVER_OK, so this helper | ||
41 | + * function helps us deal with manually starting ioeventfd in that case. | ||
42 | + */ | ||
43 | +static bool virtio_scsi_defer_to_dataplane(VirtIOSCSI *s) | ||
44 | +{ | ||
45 | + if (!s->ctx || s->dataplane_started) { | ||
46 | + return false; | ||
47 | + } | ||
48 | + | ||
49 | + virtio_device_start_ioeventfd(&s->parent_obj.parent_obj); | ||
50 | + return !s->dataplane_fenced; | ||
51 | +} | ||
52 | + | ||
53 | static void virtio_scsi_handle_ctrl(VirtIODevice *vdev, VirtQueue *vq) | ||
54 | { | ||
55 | VirtIOSCSI *s = (VirtIOSCSI *)vdev; | ||
56 | |||
57 | - if (s->ctx) { | ||
58 | - virtio_device_start_ioeventfd(vdev); | ||
59 | - if (!s->dataplane_fenced) { | ||
60 | - return; | ||
61 | - } | ||
62 | + if (virtio_scsi_defer_to_dataplane(s)) { | ||
63 | + return; | ||
64 | } | ||
65 | + | ||
66 | virtio_scsi_acquire(s); | ||
67 | virtio_scsi_handle_ctrl_vq(s, vq); | ||
68 | virtio_scsi_release(s); | ||
69 | @@ -XXX,XX +XXX,XX @@ static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq) | ||
70 | /* use non-QOM casts in the data path */ | ||
71 | VirtIOSCSI *s = (VirtIOSCSI *)vdev; | ||
72 | |||
73 | - if (s->ctx && !s->dataplane_started) { | ||
74 | - virtio_device_start_ioeventfd(vdev); | ||
75 | - if (!s->dataplane_fenced) { | ||
76 | - return; | ||
77 | - } | ||
78 | + if (virtio_scsi_defer_to_dataplane(s)) { | ||
79 | + return; | ||
80 | } | ||
81 | + | ||
82 | virtio_scsi_acquire(s); | ||
83 | virtio_scsi_handle_cmd_vq(s, vq); | ||
84 | virtio_scsi_release(s); | ||
85 | @@ -XXX,XX +XXX,XX @@ static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq) | ||
86 | { | ||
87 | VirtIOSCSI *s = VIRTIO_SCSI(vdev); | ||
88 | |||
89 | - if (s->ctx) { | ||
90 | - virtio_device_start_ioeventfd(vdev); | ||
91 | - if (!s->dataplane_fenced) { | ||
92 | - return; | ||
93 | - } | ||
94 | + if (virtio_scsi_defer_to_dataplane(s)) { | ||
95 | + return; | ||
96 | } | ||
97 | + | ||
98 | virtio_scsi_acquire(s); | ||
99 | virtio_scsi_handle_event_vq(s, vq); | ||
100 | virtio_scsi_release(s); | ||
101 | -- | ||
102 | 2.35.1 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | The virtio-scsi event virtqueue is not emptied by its handler function. | ||
2 | This is typical for rx virtqueues where the device uses buffers when | ||
3 | some event occurs (e.g. a packet is received, an error condition | ||
4 | happens, etc). | ||
1 | 5 | ||
6 | Polling non-empty virtqueues wastes CPU cycles. We are not waiting for | ||
7 | new buffers to become available, we are waiting for an event to occur, | ||
8 | so it's a misuse of CPU resources to poll for buffers. | ||
9 | |||
10 | Introduce the new virtio_queue_aio_attach_host_notifier_no_poll() API, | ||
11 | which is identical to virtio_queue_aio_attach_host_notifier() except | ||
12 | that it does not poll the virtqueue. | ||
13 | |||
14 | Before this patch the following command-line consumed 100% CPU in the | ||
15 | IOThread polling and calling virtio_scsi_handle_event(): | ||
16 | |||
17 | $ qemu-system-x86_64 -M accel=kvm -m 1G -cpu host \ | ||
18 | --object iothread,id=iothread0 \ | ||
19 | --device virtio-scsi-pci,iothread=iothread0 \ | ||
20 | --blockdev file,filename=test.img,aio=native,cache.direct=on,node-name=drive0 \ | ||
21 | --device scsi-hd,drive=drive0 | ||
22 | |||
23 | After this patch CPU is no longer wasted. | ||
24 | |||
25 | Reported-by: Nir Soffer <nsoffer@redhat.com> | ||
26 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
27 | Tested-by: Nir Soffer <nsoffer@redhat.com> | ||
28 | Message-id: 20220427143541.119567-3-stefanha@redhat.com | ||
29 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
30 | --- | ||
31 | include/hw/virtio/virtio.h | 1 + | ||
32 | hw/scsi/virtio-scsi-dataplane.c | 2 +- | ||
33 | hw/virtio/virtio.c | 13 +++++++++++++ | ||
34 | 3 files changed, 15 insertions(+), 1 deletion(-) | ||
35 | |||
36 | diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h | ||
37 | index XXXXXXX..XXXXXXX 100644 | ||
38 | --- a/include/hw/virtio/virtio.h | ||
39 | +++ b/include/hw/virtio/virtio.h | ||
40 | @@ -XXX,XX +XXX,XX @@ EventNotifier *virtio_queue_get_host_notifier(VirtQueue *vq); | ||
41 | void virtio_queue_set_host_notifier_enabled(VirtQueue *vq, bool enabled); | ||
42 | void virtio_queue_host_notifier_read(EventNotifier *n); | ||
43 | void virtio_queue_aio_attach_host_notifier(VirtQueue *vq, AioContext *ctx); | ||
44 | +void virtio_queue_aio_attach_host_notifier_no_poll(VirtQueue *vq, AioContext *ctx); | ||
45 | void virtio_queue_aio_detach_host_notifier(VirtQueue *vq, AioContext *ctx); | ||
46 | VirtQueue *virtio_vector_first_queue(VirtIODevice *vdev, uint16_t vector); | ||
47 | VirtQueue *virtio_vector_next_queue(VirtQueue *vq); | ||
48 | diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c | ||
49 | index XXXXXXX..XXXXXXX 100644 | ||
50 | --- a/hw/scsi/virtio-scsi-dataplane.c | ||
51 | +++ b/hw/scsi/virtio-scsi-dataplane.c | ||
52 | @@ -XXX,XX +XXX,XX @@ int virtio_scsi_dataplane_start(VirtIODevice *vdev) | ||
53 | |||
54 | aio_context_acquire(s->ctx); | ||
55 | virtio_queue_aio_attach_host_notifier(vs->ctrl_vq, s->ctx); | ||
56 | - virtio_queue_aio_attach_host_notifier(vs->event_vq, s->ctx); | ||
57 | + virtio_queue_aio_attach_host_notifier_no_poll(vs->event_vq, s->ctx); | ||
58 | |||
59 | for (i = 0; i < vs->conf.num_queues; i++) { | ||
60 | virtio_queue_aio_attach_host_notifier(vs->cmd_vqs[i], s->ctx); | ||
61 | diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c | ||
62 | index XXXXXXX..XXXXXXX 100644 | ||
63 | --- a/hw/virtio/virtio.c | ||
64 | +++ b/hw/virtio/virtio.c | ||
65 | @@ -XXX,XX +XXX,XX @@ void virtio_queue_aio_attach_host_notifier(VirtQueue *vq, AioContext *ctx) | ||
66 | virtio_queue_host_notifier_aio_poll_end); | ||
67 | } | ||
68 | |||
69 | +/* | ||
70 | + * Same as virtio_queue_aio_attach_host_notifier() but without polling. Use | ||
71 | + * this for rx virtqueues and similar cases where the virtqueue handler | ||
72 | + * function does not pop all elements. When the virtqueue is left non-empty | ||
73 | + * polling consumes CPU cycles and should not be used. | ||
74 | + */ | ||
75 | +void virtio_queue_aio_attach_host_notifier_no_poll(VirtQueue *vq, AioContext *ctx) | ||
76 | +{ | ||
77 | + aio_set_event_notifier(ctx, &vq->host_notifier, true, | ||
78 | + virtio_queue_host_notifier_read, | ||
79 | + NULL, NULL); | ||
80 | +} | ||
81 | + | ||
82 | void virtio_queue_aio_detach_host_notifier(VirtQueue *vq, AioContext *ctx) | ||
83 | { | ||
84 | aio_set_event_notifier(ctx, &vq->host_notifier, true, NULL, NULL, NULL); | ||
85 | -- | ||
86 | 2.35.1 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | virtio_scsi_handle_event_vq() is only called from hw/scsi/virtio-scsi.c | ||
2 | now and its return value is no longer used. Remove the function | ||
3 | prototype from virtio-scsi.h and drop the return value. | ||
1 | 4 | ||
5 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
6 | Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> | ||
7 | Message-id: 20220427143541.119567-4-stefanha@redhat.com | ||
8 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
9 | --- | ||
10 | include/hw/virtio/virtio-scsi.h | 1 - | ||
11 | hw/scsi/virtio-scsi.c | 4 +--- | ||
12 | 2 files changed, 1 insertion(+), 4 deletions(-) | ||
13 | |||
14 | diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h | ||
15 | index XXXXXXX..XXXXXXX 100644 | ||
16 | --- a/include/hw/virtio/virtio-scsi.h | ||
17 | +++ b/include/hw/virtio/virtio-scsi.h | ||
18 | @@ -XXX,XX +XXX,XX @@ void virtio_scsi_common_realize(DeviceState *dev, | ||
19 | Error **errp); | ||
20 | |||
21 | void virtio_scsi_common_unrealize(DeviceState *dev); | ||
22 | -bool virtio_scsi_handle_event_vq(VirtIOSCSI *s, VirtQueue *vq); | ||
23 | bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq); | ||
24 | bool virtio_scsi_handle_ctrl_vq(VirtIOSCSI *s, VirtQueue *vq); | ||
25 | void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req); | ||
26 | diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c | ||
27 | index XXXXXXX..XXXXXXX 100644 | ||
28 | --- a/hw/scsi/virtio-scsi.c | ||
29 | +++ b/hw/scsi/virtio-scsi.c | ||
30 | @@ -XXX,XX +XXX,XX @@ void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev, | ||
31 | virtio_scsi_complete_req(req); | ||
32 | } | ||
33 | |||
34 | -bool virtio_scsi_handle_event_vq(VirtIOSCSI *s, VirtQueue *vq) | ||
35 | +static void virtio_scsi_handle_event_vq(VirtIOSCSI *s, VirtQueue *vq) | ||
36 | { | ||
37 | if (s->events_dropped) { | ||
38 | virtio_scsi_push_event(s, NULL, VIRTIO_SCSI_T_NO_EVENT, 0); | ||
39 | - return true; | ||
40 | } | ||
41 | - return false; | ||
42 | } | ||
43 | |||
44 | static void virtio_scsi_handle_event(VirtIODevice *vdev, VirtQueue *vq) | ||
45 | -- | ||
46 | 2.35.1 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | virtio_scsi_handle_ctrl_vq() is only called from hw/scsi/virtio-scsi.c | ||
2 | now and its return value is no longer used. Remove the function | ||
3 | prototype from virtio-scsi.h and drop the return value. | ||
1 | 4 | ||
5 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
6 | Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> | ||
7 | Message-id: 20220427143541.119567-5-stefanha@redhat.com | ||
8 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
9 | --- | ||
10 | include/hw/virtio/virtio-scsi.h | 1 - | ||
11 | hw/scsi/virtio-scsi.c | 5 +---- | ||
12 | 2 files changed, 1 insertion(+), 5 deletions(-) | ||
13 | |||
14 | diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h | ||
15 | index XXXXXXX..XXXXXXX 100644 | ||
16 | --- a/include/hw/virtio/virtio-scsi.h | ||
17 | +++ b/include/hw/virtio/virtio-scsi.h | ||
18 | @@ -XXX,XX +XXX,XX @@ void virtio_scsi_common_realize(DeviceState *dev, | ||
19 | |||
20 | void virtio_scsi_common_unrealize(DeviceState *dev); | ||
21 | bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq); | ||
22 | -bool virtio_scsi_handle_ctrl_vq(VirtIOSCSI *s, VirtQueue *vq); | ||
23 | void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req); | ||
24 | void virtio_scsi_free_req(VirtIOSCSIReq *req); | ||
25 | void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev, | ||
26 | diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c | ||
27 | index XXXXXXX..XXXXXXX 100644 | ||
28 | --- a/hw/scsi/virtio-scsi.c | ||
29 | +++ b/hw/scsi/virtio-scsi.c | ||
30 | @@ -XXX,XX +XXX,XX @@ static void virtio_scsi_handle_ctrl_req(VirtIOSCSI *s, VirtIOSCSIReq *req) | ||
31 | } | ||
32 | } | ||
33 | |||
34 | -bool virtio_scsi_handle_ctrl_vq(VirtIOSCSI *s, VirtQueue *vq) | ||
35 | +static void virtio_scsi_handle_ctrl_vq(VirtIOSCSI *s, VirtQueue *vq) | ||
36 | { | ||
37 | VirtIOSCSIReq *req; | ||
38 | - bool progress = false; | ||
39 | |||
40 | while ((req = virtio_scsi_pop_req(s, vq))) { | ||
41 | - progress = true; | ||
42 | virtio_scsi_handle_ctrl_req(s, req); | ||
43 | } | ||
44 | - return progress; | ||
45 | } | ||
46 | |||
47 | /* | ||
48 | -- | ||
49 | 2.35.1 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | virtio_scsi_handle_cmd_vq() is only called from hw/scsi/virtio-scsi.c | ||
2 | now and its return value is no longer used. Remove the function | ||
3 | prototype from virtio-scsi.h and drop the return value. | ||
1 | 4 | ||
5 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
6 | Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> | ||
7 | Message-id: 20220427143541.119567-6-stefanha@redhat.com | ||
8 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
9 | --- | ||
10 | include/hw/virtio/virtio-scsi.h | 1 - | ||
11 | hw/scsi/virtio-scsi.c | 5 +---- | ||
12 | 2 files changed, 1 insertion(+), 5 deletions(-) | ||
13 | |||
14 | diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h | ||
15 | index XXXXXXX..XXXXXXX 100644 | ||
16 | --- a/include/hw/virtio/virtio-scsi.h | ||
17 | +++ b/include/hw/virtio/virtio-scsi.h | ||
18 | @@ -XXX,XX +XXX,XX @@ void virtio_scsi_common_realize(DeviceState *dev, | ||
19 | Error **errp); | ||
20 | |||
21 | void virtio_scsi_common_unrealize(DeviceState *dev); | ||
22 | -bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq); | ||
23 | void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req); | ||
24 | void virtio_scsi_free_req(VirtIOSCSIReq *req); | ||
25 | void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev, | ||
26 | diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c | ||
27 | index XXXXXXX..XXXXXXX 100644 | ||
28 | --- a/hw/scsi/virtio-scsi.c | ||
29 | +++ b/hw/scsi/virtio-scsi.c | ||
30 | @@ -XXX,XX +XXX,XX @@ static void virtio_scsi_handle_cmd_req_submit(VirtIOSCSI *s, VirtIOSCSIReq *req) | ||
31 | scsi_req_unref(sreq); | ||
32 | } | ||
33 | |||
34 | -bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq) | ||
35 | +static void virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq) | ||
36 | { | ||
37 | VirtIOSCSIReq *req, *next; | ||
38 | int ret = 0; | ||
39 | bool suppress_notifications = virtio_queue_get_notification(vq); | ||
40 | - bool progress = false; | ||
41 | |||
42 | QTAILQ_HEAD(, VirtIOSCSIReq) reqs = QTAILQ_HEAD_INITIALIZER(reqs); | ||
43 | |||
44 | @@ -XXX,XX +XXX,XX @@ bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq) | ||
45 | } | ||
46 | |||
47 | while ((req = virtio_scsi_pop_req(s, vq))) { | ||
48 | - progress = true; | ||
49 | ret = virtio_scsi_handle_cmd_req_prepare(s, req); | ||
50 | if (!ret) { | ||
51 | QTAILQ_INSERT_TAIL(&reqs, req, next); | ||
52 | @@ -XXX,XX +XXX,XX @@ bool virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq) | ||
53 | QTAILQ_FOREACH_SAFE(req, &reqs, next, next) { | ||
54 | virtio_scsi_handle_cmd_req_submit(s, req); | ||
55 | } | ||
56 | - return progress; | ||
57 | } | ||
58 | |||
59 | static void virtio_scsi_handle_cmd(VirtIODevice *vdev, VirtQueue *vq) | ||
60 | -- | ||
61 | 2.35.1 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | There is no longer a need to expose the request and related APIs in | ||
2 | virtio-scsi.h since there are no callers outside virtio-scsi.c. | ||
1 | 3 | ||
4 | Note the block comment in VirtIOSCSIReq has been adjusted to meet the | ||
5 | coding style. | ||
6 | |||
7 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
8 | Reviewed-by: Paolo Bonzini <pbonzini@redhat.com> | ||
9 | Message-id: 20220427143541.119567-7-stefanha@redhat.com | ||
10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
11 | --- | ||
12 | include/hw/virtio/virtio-scsi.h | 40 ----------------------------- | ||
13 | hw/scsi/virtio-scsi.c | 45 ++++++++++++++++++++++++++++++--- | ||
14 | 2 files changed, 41 insertions(+), 44 deletions(-) | ||
15 | |||
16 | diff --git a/include/hw/virtio/virtio-scsi.h b/include/hw/virtio/virtio-scsi.h | ||
17 | index XXXXXXX..XXXXXXX 100644 | ||
18 | --- a/include/hw/virtio/virtio-scsi.h | ||
19 | +++ b/include/hw/virtio/virtio-scsi.h | ||
20 | @@ -XXX,XX +XXX,XX @@ struct VirtIOSCSI { | ||
21 | uint32_t host_features; | ||
22 | }; | ||
23 | |||
24 | -typedef struct VirtIOSCSIReq { | ||
25 | - /* Note: | ||
26 | - * - fields up to resp_iov are initialized by virtio_scsi_init_req; | ||
27 | - * - fields starting at vring are zeroed by virtio_scsi_init_req. | ||
28 | - * */ | ||
29 | - VirtQueueElement elem; | ||
30 | - | ||
31 | - VirtIOSCSI *dev; | ||
32 | - VirtQueue *vq; | ||
33 | - QEMUSGList qsgl; | ||
34 | - QEMUIOVector resp_iov; | ||
35 | - | ||
36 | - union { | ||
37 | - /* Used for two-stage request submission */ | ||
38 | - QTAILQ_ENTRY(VirtIOSCSIReq) next; | ||
39 | - | ||
40 | - /* Used for cancellation of request during TMFs */ | ||
41 | - int remaining; | ||
42 | - }; | ||
43 | - | ||
44 | - SCSIRequest *sreq; | ||
45 | - size_t resp_size; | ||
46 | - enum SCSIXferMode mode; | ||
47 | - union { | ||
48 | - VirtIOSCSICmdResp cmd; | ||
49 | - VirtIOSCSICtrlTMFResp tmf; | ||
50 | - VirtIOSCSICtrlANResp an; | ||
51 | - VirtIOSCSIEvent event; | ||
52 | - } resp; | ||
53 | - union { | ||
54 | - VirtIOSCSICmdReq cmd; | ||
55 | - VirtIOSCSICtrlTMFReq tmf; | ||
56 | - VirtIOSCSICtrlANReq an; | ||
57 | - } req; | ||
58 | -} VirtIOSCSIReq; | ||
59 | - | ||
60 | static inline void virtio_scsi_acquire(VirtIOSCSI *s) | ||
61 | { | ||
62 | if (s->ctx) { | ||
63 | @@ -XXX,XX +XXX,XX @@ void virtio_scsi_common_realize(DeviceState *dev, | ||
64 | Error **errp); | ||
65 | |||
66 | void virtio_scsi_common_unrealize(DeviceState *dev); | ||
67 | -void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req); | ||
68 | -void virtio_scsi_free_req(VirtIOSCSIReq *req); | ||
69 | -void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev, | ||
70 | - uint32_t event, uint32_t reason); | ||
71 | |||
72 | void virtio_scsi_dataplane_setup(VirtIOSCSI *s, Error **errp); | ||
73 | int virtio_scsi_dataplane_start(VirtIODevice *s); | ||
74 | diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c | ||
75 | index XXXXXXX..XXXXXXX 100644 | ||
76 | --- a/hw/scsi/virtio-scsi.c | ||
77 | +++ b/hw/scsi/virtio-scsi.c | ||
78 | @@ -XXX,XX +XXX,XX @@ | ||
79 | #include "hw/virtio/virtio-access.h" | ||
80 | #include "trace.h" | ||
81 | |||
82 | +typedef struct VirtIOSCSIReq { | ||
83 | + /* | ||
84 | + * Note: | ||
85 | + * - fields up to resp_iov are initialized by virtio_scsi_init_req; | ||
86 | + * - fields starting at vring are zeroed by virtio_scsi_init_req. | ||
87 | + */ | ||
88 | + VirtQueueElement elem; | ||
89 | + | ||
90 | + VirtIOSCSI *dev; | ||
91 | + VirtQueue *vq; | ||
92 | + QEMUSGList qsgl; | ||
93 | + QEMUIOVector resp_iov; | ||
94 | + | ||
95 | + union { | ||
96 | + /* Used for two-stage request submission */ | ||
97 | + QTAILQ_ENTRY(VirtIOSCSIReq) next; | ||
98 | + | ||
99 | + /* Used for cancellation of request during TMFs */ | ||
100 | + int remaining; | ||
101 | + }; | ||
102 | + | ||
103 | + SCSIRequest *sreq; | ||
104 | + size_t resp_size; | ||
105 | + enum SCSIXferMode mode; | ||
106 | + union { | ||
107 | + VirtIOSCSICmdResp cmd; | ||
108 | + VirtIOSCSICtrlTMFResp tmf; | ||
109 | + VirtIOSCSICtrlANResp an; | ||
110 | + VirtIOSCSIEvent event; | ||
111 | + } resp; | ||
112 | + union { | ||
113 | + VirtIOSCSICmdReq cmd; | ||
114 | + VirtIOSCSICtrlTMFReq tmf; | ||
115 | + VirtIOSCSICtrlANReq an; | ||
116 | + } req; | ||
117 | +} VirtIOSCSIReq; | ||
118 | + | ||
119 | static inline int virtio_scsi_get_lun(uint8_t *lun) | ||
120 | { | ||
121 | return ((lun[2] << 8) | lun[3]) & 0x3FFF; | ||
122 | @@ -XXX,XX +XXX,XX @@ static inline SCSIDevice *virtio_scsi_device_get(VirtIOSCSI *s, uint8_t *lun) | ||
123 | return scsi_device_get(&s->bus, 0, lun[1], virtio_scsi_get_lun(lun)); | ||
124 | } | ||
125 | |||
126 | -void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req) | ||
127 | +static void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req) | ||
128 | { | ||
129 | VirtIODevice *vdev = VIRTIO_DEVICE(s); | ||
130 | const size_t zero_skip = | ||
131 | @@ -XXX,XX +XXX,XX @@ void virtio_scsi_init_req(VirtIOSCSI *s, VirtQueue *vq, VirtIOSCSIReq *req) | ||
132 | memset((uint8_t *)req + zero_skip, 0, sizeof(*req) - zero_skip); | ||
133 | } | ||
134 | |||
135 | -void virtio_scsi_free_req(VirtIOSCSIReq *req) | ||
136 | +static void virtio_scsi_free_req(VirtIOSCSIReq *req) | ||
137 | { | ||
138 | qemu_iovec_destroy(&req->resp_iov); | ||
139 | qemu_sglist_destroy(&req->qsgl); | ||
140 | @@ -XXX,XX +XXX,XX @@ static void virtio_scsi_reset(VirtIODevice *vdev) | ||
141 | s->events_dropped = false; | ||
142 | } | ||
143 | |||
144 | -void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev, | ||
145 | - uint32_t event, uint32_t reason) | ||
146 | +static void virtio_scsi_push_event(VirtIOSCSI *s, SCSIDevice *dev, | ||
147 | + uint32_t event, uint32_t reason) | ||
148 | { | ||
149 | VirtIOSCSICommon *vs = VIRTIO_SCSI_COMMON(s); | ||
150 | VirtIOSCSIReq *req; | ||
151 | -- | ||
152 | 2.35.1 | diff view generated by jsdifflib |