1 | The following changes since commit 6cb4f6db4f4367faa33da85b15f75bbbd2bed2a6: | 1 | The following changes since commit c6a5fc2ac76c5ab709896ee1b0edd33685a67ed1: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/cleber/tags/python-next-pull-request' into staging (2019-03-07 16:16:02 +0000) | 3 | decodetree: Add --output-null for meson testing (2023-05-31 19:56:42 -0700) |
4 | 4 | ||
5 | are available in the Git repository at: | 5 | are available in the Git repository at: |
6 | 6 | ||
7 | git://github.com/stefanha/qemu.git tags/block-pull-request | 7 | https://gitlab.com/stefanha/qemu.git tags/block-pull-request |
8 | 8 | ||
9 | for you to fetch changes up to 6ca206204fa773c8626d59caf2a5676d6cc35f52: | 9 | for you to fetch changes up to 98b126f5e3228a346c774e569e26689943b401dd: |
10 | 10 | ||
11 | iothread: document about why we need explicit aio_poll() (2019-03-08 10:20:57 +0000) | 11 | qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa (2023-06-01 11:08:21 -0400) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Pull request | 14 | Pull request |
15 | 15 | ||
16 | - Stefano Garzarella's blkio block driver 'fd' parameter | ||
17 | - My thread-local blk_io_plug() series | ||
18 | |||
16 | ---------------------------------------------------------------- | 19 | ---------------------------------------------------------------- |
17 | 20 | ||
18 | Anastasiia Rusakova (1): | 21 | Stefan Hajnoczi (6): |
19 | hw/block/virtio-blk: Clean req->dev repetitions | 22 | block: add blk_io_plug_call() API |
23 | block/nvme: convert to blk_io_plug_call() API | ||
24 | block/blkio: convert to blk_io_plug_call() API | ||
25 | block/io_uring: convert to blk_io_plug_call() API | ||
26 | block/linux-aio: convert to blk_io_plug_call() API | ||
27 | block: remove bdrv_co_io_plug() API | ||
20 | 28 | ||
21 | Peter Xu (5): | 29 | Stefano Garzarella (2): |
22 | iothread: replace init_done_cond with a semaphore | 30 | block/blkio: use qemu_open() to support fd passing for virtio-blk |
23 | iothread: create the gcontext unconditionally | 31 | qapi: add '@fdset' feature for BlockdevOptionsVirtioBlkVhostVdpa |
24 | iothread: create main loop unconditionally | ||
25 | iothread: push gcontext earlier in the thread_fn | ||
26 | iothread: document about why we need explicit aio_poll() | ||
27 | 32 | ||
28 | Stefan Hajnoczi (1): | 33 | MAINTAINERS | 1 + |
29 | MAINTAINERS: add missing support status fields | 34 | qapi/block-core.json | 6 ++ |
30 | 35 | meson.build | 4 + | |
31 | MAINTAINERS | 3 ++ | 36 | include/block/block-io.h | 3 - |
32 | include/sysemu/iothread.h | 5 +-- | 37 | include/block/block_int-common.h | 11 --- |
33 | hw/block/virtio-blk.c | 16 ++++--- | 38 | include/block/raw-aio.h | 14 --- |
34 | iothread.c | 90 +++++++++++++++++++-------------------- | 39 | include/sysemu/block-backend-io.h | 13 +-- |
35 | 4 files changed, 57 insertions(+), 57 deletions(-) | 40 | block/blkio.c | 96 ++++++++++++------ |
41 | block/block-backend.c | 22 ----- | ||
42 | block/file-posix.c | 38 ------- | ||
43 | block/io.c | 37 ------- | ||
44 | block/io_uring.c | 44 ++++----- | ||
45 | block/linux-aio.c | 41 +++----- | ||
46 | block/nvme.c | 44 +++------ | ||
47 | block/plug.c | 159 ++++++++++++++++++++++++++++++ | ||
48 | hw/block/dataplane/xen-block.c | 8 +- | ||
49 | hw/block/virtio-blk.c | 4 +- | ||
50 | hw/scsi/virtio-scsi.c | 6 +- | ||
51 | block/meson.build | 1 + | ||
52 | block/trace-events | 6 +- | ||
53 | 20 files changed, 293 insertions(+), 265 deletions(-) | ||
54 | create mode 100644 block/plug.c | ||
36 | 55 | ||
37 | -- | 56 | -- |
38 | 2.20.1 | 57 | 2.40.1 |
39 | |||
40 | diff view generated by jsdifflib |
1 | This patch adds the "S:" line for areas of the codebase that currently | 1 | Introduce a new API for thread-local blk_io_plug() that does not |
---|---|---|---|
2 | lack a support status field. | 2 | traverse the block graph. The goal is to make blk_io_plug() multi-queue |
3 | 3 | friendly. | |
4 | Note that there are a few more areas that are more abstract and do not | 4 | |
5 | correspond to a specific set of files. They have not been modified. | 5 | Instead of having block drivers track whether or not we're in a plugged |
6 | 6 | section, provide an API that allows them to defer a function call until | |
7 | Cc: Alex Bennée <alex.bennee@linaro.org> | 7 | we're unplugged: blk_io_plug_call(fn, opaque). If blk_io_plug_call() is |
8 | called multiple times with the same fn/opaque pair, then fn() is only | ||
9 | called once at the end of the function - resulting in batching. | ||
10 | |||
11 | This patch introduces the API and changes blk_io_plug()/blk_io_unplug(). | ||
12 | blk_io_plug()/blk_io_unplug() no longer require a BlockBackend argument | ||
13 | because the plug state is now thread-local. | ||
14 | |||
15 | Later patches convert block drivers to blk_io_plug_call() and then we | ||
16 | can finally remove .bdrv_co_io_plug() once all block drivers have been | ||
17 | converted. | ||
18 | |||
8 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 19 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
9 | Reviewed-by: Thomas Huth <thuth@redhat.com> | 20 | Reviewed-by: Eric Blake <eblake@redhat.com> |
10 | Reviewed-by: Alex Bennée <alex.bennee@linaro.org> | 21 | Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> |
11 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | 22 | Acked-by: Kevin Wolf <kwolf@redhat.com> |
12 | Message-id: 20190301163518.20702-1-stefanha@redhat.com | 23 | Message-id: 20230530180959.1108766-2-stefanha@redhat.com |
13 | Message-Id: <20190301163518.20702-1-stefanha@redhat.com> | ||
14 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 24 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
15 | --- | 25 | --- |
16 | MAINTAINERS | 3 +++ | 26 | MAINTAINERS | 1 + |
17 | 1 file changed, 3 insertions(+) | 27 | include/sysemu/block-backend-io.h | 13 +-- |
28 | block/block-backend.c | 22 ----- | ||
29 | block/plug.c | 159 ++++++++++++++++++++++++++++++ | ||
30 | hw/block/dataplane/xen-block.c | 8 +- | ||
31 | hw/block/virtio-blk.c | 4 +- | ||
32 | hw/scsi/virtio-scsi.c | 6 +- | ||
33 | block/meson.build | 1 + | ||
34 | 8 files changed, 173 insertions(+), 41 deletions(-) | ||
35 | create mode 100644 block/plug.c | ||
18 | 36 | ||
19 | diff --git a/MAINTAINERS b/MAINTAINERS | 37 | diff --git a/MAINTAINERS b/MAINTAINERS |
20 | index XXXXXXX..XXXXXXX 100644 | 38 | index XXXXXXX..XXXXXXX 100644 |
21 | --- a/MAINTAINERS | 39 | --- a/MAINTAINERS |
22 | +++ b/MAINTAINERS | 40 | +++ b/MAINTAINERS |
23 | @@ -XXX,XX +XXX,XX @@ F: include/hw/tricore/ | 41 | @@ -XXX,XX +XXX,XX @@ F: util/aio-*.c |
24 | 42 | F: util/aio-*.h | |
25 | Multiarch Linux User Tests | 43 | F: util/fdmon-*.c |
26 | M: Alex Bennée <alex.bennee@linaro.org> | 44 | F: block/io.c |
27 | +S: Maintained | 45 | +F: block/plug.c |
28 | F: tests/tcg/multiarch/ | 46 | F: migration/block* |
29 | 47 | F: include/block/aio.h | |
30 | Guest CPU Cores (KVM): | 48 | F: include/block/aio-wait.h |
31 | @@ -XXX,XX +XXX,XX @@ F: qemu.sasl | 49 | diff --git a/include/sysemu/block-backend-io.h b/include/sysemu/block-backend-io.h |
32 | Coroutines | 50 | index XXXXXXX..XXXXXXX 100644 |
33 | M: Stefan Hajnoczi <stefanha@redhat.com> | 51 | --- a/include/sysemu/block-backend-io.h |
34 | M: Kevin Wolf <kwolf@redhat.com> | 52 | +++ b/include/sysemu/block-backend-io.h |
35 | +S: Maintained | 53 | @@ -XXX,XX +XXX,XX @@ void blk_iostatus_set_err(BlockBackend *blk, int error); |
36 | F: util/*coroutine* | 54 | int blk_get_max_iov(BlockBackend *blk); |
37 | F: include/qemu/coroutine* | 55 | int blk_get_max_hw_iov(BlockBackend *blk); |
38 | F: tests/test-coroutine.c | 56 | |
39 | @@ -XXX,XX +XXX,XX @@ F: .gitlab-ci.yml | 57 | -/* |
40 | Guest Test Compilation Support | 58 | - * blk_io_plug/unplug are thread-local operations. This means that multiple |
41 | M: Alex Bennée <alex.bennee@linaro.org> | 59 | - * IOThreads can simultaneously call plug/unplug, but the caller must ensure |
42 | R: Philippe Mathieu-Daudé <f4bug@amsat.org> | 60 | - * that each unplug() is called in the same IOThread of the matching plug(). |
43 | +S: Maintained | 61 | - */ |
44 | F: tests/tcg/Makefile | 62 | -void coroutine_fn blk_co_io_plug(BlockBackend *blk); |
45 | F: tests/tcg/Makefile.include | 63 | -void co_wrapper blk_io_plug(BlockBackend *blk); |
46 | L: qemu-devel@nongnu.org | 64 | - |
65 | -void coroutine_fn blk_co_io_unplug(BlockBackend *blk); | ||
66 | -void co_wrapper blk_io_unplug(BlockBackend *blk); | ||
67 | +void blk_io_plug(void); | ||
68 | +void blk_io_unplug(void); | ||
69 | +void blk_io_plug_call(void (*fn)(void *), void *opaque); | ||
70 | |||
71 | AioContext *blk_get_aio_context(BlockBackend *blk); | ||
72 | BlockAcctStats *blk_get_stats(BlockBackend *blk); | ||
73 | diff --git a/block/block-backend.c b/block/block-backend.c | ||
74 | index XXXXXXX..XXXXXXX 100644 | ||
75 | --- a/block/block-backend.c | ||
76 | +++ b/block/block-backend.c | ||
77 | @@ -XXX,XX +XXX,XX @@ void blk_add_insert_bs_notifier(BlockBackend *blk, Notifier *notify) | ||
78 | notifier_list_add(&blk->insert_bs_notifiers, notify); | ||
79 | } | ||
80 | |||
81 | -void coroutine_fn blk_co_io_plug(BlockBackend *blk) | ||
82 | -{ | ||
83 | - BlockDriverState *bs = blk_bs(blk); | ||
84 | - IO_CODE(); | ||
85 | - GRAPH_RDLOCK_GUARD(); | ||
86 | - | ||
87 | - if (bs) { | ||
88 | - bdrv_co_io_plug(bs); | ||
89 | - } | ||
90 | -} | ||
91 | - | ||
92 | -void coroutine_fn blk_co_io_unplug(BlockBackend *blk) | ||
93 | -{ | ||
94 | - BlockDriverState *bs = blk_bs(blk); | ||
95 | - IO_CODE(); | ||
96 | - GRAPH_RDLOCK_GUARD(); | ||
97 | - | ||
98 | - if (bs) { | ||
99 | - bdrv_co_io_unplug(bs); | ||
100 | - } | ||
101 | -} | ||
102 | - | ||
103 | BlockAcctStats *blk_get_stats(BlockBackend *blk) | ||
104 | { | ||
105 | IO_CODE(); | ||
106 | diff --git a/block/plug.c b/block/plug.c | ||
107 | new file mode 100644 | ||
108 | index XXXXXXX..XXXXXXX | ||
109 | --- /dev/null | ||
110 | +++ b/block/plug.c | ||
111 | @@ -XXX,XX +XXX,XX @@ | ||
112 | +/* SPDX-License-Identifier: GPL-2.0-or-later */ | ||
113 | +/* | ||
114 | + * Block I/O plugging | ||
115 | + * | ||
116 | + * Copyright Red Hat. | ||
117 | + * | ||
118 | + * This API defers a function call within a blk_io_plug()/blk_io_unplug() | ||
119 | + * section, allowing multiple calls to batch up. This is a performance | ||
120 | + * optimization that is used in the block layer to submit several I/O requests | ||
121 | + * at once instead of individually: | ||
122 | + * | ||
123 | + * blk_io_plug(); <-- start of plugged region | ||
124 | + * ... | ||
125 | + * blk_io_plug_call(my_func, my_obj); <-- deferred my_func(my_obj) call | ||
126 | + * blk_io_plug_call(my_func, my_obj); <-- another | ||
127 | + * blk_io_plug_call(my_func, my_obj); <-- another | ||
128 | + * ... | ||
129 | + * blk_io_unplug(); <-- end of plugged region, my_func(my_obj) is called once | ||
130 | + * | ||
131 | + * This code is actually generic and not tied to the block layer. If another | ||
132 | + * subsystem needs this functionality, it could be renamed. | ||
133 | + */ | ||
134 | + | ||
135 | +#include "qemu/osdep.h" | ||
136 | +#include "qemu/coroutine-tls.h" | ||
137 | +#include "qemu/notify.h" | ||
138 | +#include "qemu/thread.h" | ||
139 | +#include "sysemu/block-backend.h" | ||
140 | + | ||
141 | +/* A function call that has been deferred until unplug() */ | ||
142 | +typedef struct { | ||
143 | + void (*fn)(void *); | ||
144 | + void *opaque; | ||
145 | +} UnplugFn; | ||
146 | + | ||
147 | +/* Per-thread state */ | ||
148 | +typedef struct { | ||
149 | + unsigned count; /* how many times has plug() been called? */ | ||
150 | + GArray *unplug_fns; /* functions to call at unplug time */ | ||
151 | +} Plug; | ||
152 | + | ||
153 | +/* Use get_ptr_plug() to fetch this thread-local value */ | ||
154 | +QEMU_DEFINE_STATIC_CO_TLS(Plug, plug); | ||
155 | + | ||
156 | +/* Called at thread cleanup time */ | ||
157 | +static void blk_io_plug_atexit(Notifier *n, void *value) | ||
158 | +{ | ||
159 | + Plug *plug = get_ptr_plug(); | ||
160 | + g_array_free(plug->unplug_fns, TRUE); | ||
161 | +} | ||
162 | + | ||
163 | +/* This won't involve coroutines, so use __thread */ | ||
164 | +static __thread Notifier blk_io_plug_atexit_notifier; | ||
165 | + | ||
166 | +/** | ||
167 | + * blk_io_plug_call: | ||
168 | + * @fn: a function pointer to be invoked | ||
169 | + * @opaque: a user-defined argument to @fn() | ||
170 | + * | ||
171 | + * Call @fn(@opaque) immediately if not within a blk_io_plug()/blk_io_unplug() | ||
172 | + * section. | ||
173 | + * | ||
174 | + * Otherwise defer the call until the end of the outermost | ||
175 | + * blk_io_plug()/blk_io_unplug() section in this thread. If the same | ||
176 | + * @fn/@opaque pair has already been deferred, it will only be called once upon | ||
177 | + * blk_io_unplug() so that accumulated calls are batched into a single call. | ||
178 | + * | ||
179 | + * The caller must ensure that @opaque is not freed before @fn() is invoked. | ||
180 | + */ | ||
181 | +void blk_io_plug_call(void (*fn)(void *), void *opaque) | ||
182 | +{ | ||
183 | + Plug *plug = get_ptr_plug(); | ||
184 | + | ||
185 | + /* Call immediately if we're not plugged */ | ||
186 | + if (plug->count == 0) { | ||
187 | + fn(opaque); | ||
188 | + return; | ||
189 | + } | ||
190 | + | ||
191 | + GArray *array = plug->unplug_fns; | ||
192 | + if (!array) { | ||
193 | + array = g_array_new(FALSE, FALSE, sizeof(UnplugFn)); | ||
194 | + plug->unplug_fns = array; | ||
195 | + blk_io_plug_atexit_notifier.notify = blk_io_plug_atexit; | ||
196 | + qemu_thread_atexit_add(&blk_io_plug_atexit_notifier); | ||
197 | + } | ||
198 | + | ||
199 | + UnplugFn *fns = (UnplugFn *)array->data; | ||
200 | + UnplugFn new_fn = { | ||
201 | + .fn = fn, | ||
202 | + .opaque = opaque, | ||
203 | + }; | ||
204 | + | ||
205 | + /* | ||
206 | + * There won't be many, so do a linear search. If this becomes a bottleneck | ||
207 | + * then a binary search (glib 2.62+) or different data structure could be | ||
208 | + * used. | ||
209 | + */ | ||
210 | + for (guint i = 0; i < array->len; i++) { | ||
211 | + if (memcmp(&fns[i], &new_fn, sizeof(new_fn)) == 0) { | ||
212 | + return; /* already exists */ | ||
213 | + } | ||
214 | + } | ||
215 | + | ||
216 | + g_array_append_val(array, new_fn); | ||
217 | +} | ||
218 | + | ||
219 | +/** | ||
220 | + * blk_io_plug: Defer blk_io_plug_call() functions until blk_io_unplug() | ||
221 | + * | ||
222 | + * blk_io_plug/unplug are thread-local operations. This means that multiple | ||
223 | + * threads can simultaneously call plug/unplug, but the caller must ensure that | ||
224 | + * each unplug() is called in the same thread of the matching plug(). | ||
225 | + * | ||
226 | + * Nesting is supported. blk_io_plug_call() functions are only called at the | ||
227 | + * outermost blk_io_unplug(). | ||
228 | + */ | ||
229 | +void blk_io_plug(void) | ||
230 | +{ | ||
231 | + Plug *plug = get_ptr_plug(); | ||
232 | + | ||
233 | + assert(plug->count < UINT32_MAX); | ||
234 | + | ||
235 | + plug->count++; | ||
236 | +} | ||
237 | + | ||
238 | +/** | ||
239 | + * blk_io_unplug: Run any pending blk_io_plug_call() functions | ||
240 | + * | ||
241 | + * There must have been a matching blk_io_plug() call in the same thread prior | ||
242 | + * to this blk_io_unplug() call. | ||
243 | + */ | ||
244 | +void blk_io_unplug(void) | ||
245 | +{ | ||
246 | + Plug *plug = get_ptr_plug(); | ||
247 | + | ||
248 | + assert(plug->count > 0); | ||
249 | + | ||
250 | + if (--plug->count > 0) { | ||
251 | + return; | ||
252 | + } | ||
253 | + | ||
254 | + GArray *array = plug->unplug_fns; | ||
255 | + if (!array) { | ||
256 | + return; | ||
257 | + } | ||
258 | + | ||
259 | + UnplugFn *fns = (UnplugFn *)array->data; | ||
260 | + | ||
261 | + for (guint i = 0; i < array->len; i++) { | ||
262 | + fns[i].fn(fns[i].opaque); | ||
263 | + } | ||
264 | + | ||
265 | + /* | ||
266 | + * This resets the array without freeing memory so that appending is cheap | ||
267 | + * in the future. | ||
268 | + */ | ||
269 | + g_array_set_size(array, 0); | ||
270 | +} | ||
271 | diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c | ||
272 | index XXXXXXX..XXXXXXX 100644 | ||
273 | --- a/hw/block/dataplane/xen-block.c | ||
274 | +++ b/hw/block/dataplane/xen-block.c | ||
275 | @@ -XXX,XX +XXX,XX @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane) | ||
276 | * is below us. | ||
277 | */ | ||
278 | if (inflight_atstart > IO_PLUG_THRESHOLD) { | ||
279 | - blk_io_plug(dataplane->blk); | ||
280 | + blk_io_plug(); | ||
281 | } | ||
282 | while (rc != rp) { | ||
283 | /* pull request from ring */ | ||
284 | @@ -XXX,XX +XXX,XX @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane) | ||
285 | |||
286 | if (inflight_atstart > IO_PLUG_THRESHOLD && | ||
287 | batched >= inflight_atstart) { | ||
288 | - blk_io_unplug(dataplane->blk); | ||
289 | + blk_io_unplug(); | ||
290 | } | ||
291 | xen_block_do_aio(request); | ||
292 | if (inflight_atstart > IO_PLUG_THRESHOLD) { | ||
293 | if (batched >= inflight_atstart) { | ||
294 | - blk_io_plug(dataplane->blk); | ||
295 | + blk_io_plug(); | ||
296 | batched = 0; | ||
297 | } else { | ||
298 | batched++; | ||
299 | @@ -XXX,XX +XXX,XX @@ static bool xen_block_handle_requests(XenBlockDataPlane *dataplane) | ||
300 | } | ||
301 | } | ||
302 | if (inflight_atstart > IO_PLUG_THRESHOLD) { | ||
303 | - blk_io_unplug(dataplane->blk); | ||
304 | + blk_io_unplug(); | ||
305 | } | ||
306 | |||
307 | return done_something; | ||
308 | diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c | ||
309 | index XXXXXXX..XXXXXXX 100644 | ||
310 | --- a/hw/block/virtio-blk.c | ||
311 | +++ b/hw/block/virtio-blk.c | ||
312 | @@ -XXX,XX +XXX,XX @@ void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq) | ||
313 | bool suppress_notifications = virtio_queue_get_notification(vq); | ||
314 | |||
315 | aio_context_acquire(blk_get_aio_context(s->blk)); | ||
316 | - blk_io_plug(s->blk); | ||
317 | + blk_io_plug(); | ||
318 | |||
319 | do { | ||
320 | if (suppress_notifications) { | ||
321 | @@ -XXX,XX +XXX,XX @@ void virtio_blk_handle_vq(VirtIOBlock *s, VirtQueue *vq) | ||
322 | virtio_blk_submit_multireq(s, &mrb); | ||
323 | } | ||
324 | |||
325 | - blk_io_unplug(s->blk); | ||
326 | + blk_io_unplug(); | ||
327 | aio_context_release(blk_get_aio_context(s->blk)); | ||
328 | } | ||
329 | |||
330 | diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c | ||
331 | index XXXXXXX..XXXXXXX 100644 | ||
332 | --- a/hw/scsi/virtio-scsi.c | ||
333 | +++ b/hw/scsi/virtio-scsi.c | ||
334 | @@ -XXX,XX +XXX,XX @@ static int virtio_scsi_handle_cmd_req_prepare(VirtIOSCSI *s, VirtIOSCSIReq *req) | ||
335 | return -ENOBUFS; | ||
336 | } | ||
337 | scsi_req_ref(req->sreq); | ||
338 | - blk_io_plug(d->conf.blk); | ||
339 | + blk_io_plug(); | ||
340 | object_unref(OBJECT(d)); | ||
341 | return 0; | ||
342 | } | ||
343 | @@ -XXX,XX +XXX,XX @@ static void virtio_scsi_handle_cmd_req_submit(VirtIOSCSI *s, VirtIOSCSIReq *req) | ||
344 | if (scsi_req_enqueue(sreq)) { | ||
345 | scsi_req_continue(sreq); | ||
346 | } | ||
347 | - blk_io_unplug(sreq->dev->conf.blk); | ||
348 | + blk_io_unplug(); | ||
349 | scsi_req_unref(sreq); | ||
350 | } | ||
351 | |||
352 | @@ -XXX,XX +XXX,XX @@ static void virtio_scsi_handle_cmd_vq(VirtIOSCSI *s, VirtQueue *vq) | ||
353 | while (!QTAILQ_EMPTY(&reqs)) { | ||
354 | req = QTAILQ_FIRST(&reqs); | ||
355 | QTAILQ_REMOVE(&reqs, req, next); | ||
356 | - blk_io_unplug(req->sreq->dev->conf.blk); | ||
357 | + blk_io_unplug(); | ||
358 | scsi_req_unref(req->sreq); | ||
359 | virtqueue_detach_element(req->vq, &req->elem, 0); | ||
360 | virtio_scsi_free_req(req); | ||
361 | diff --git a/block/meson.build b/block/meson.build | ||
362 | index XXXXXXX..XXXXXXX 100644 | ||
363 | --- a/block/meson.build | ||
364 | +++ b/block/meson.build | ||
365 | @@ -XXX,XX +XXX,XX @@ block_ss.add(files( | ||
366 | 'mirror.c', | ||
367 | 'nbd.c', | ||
368 | 'null.c', | ||
369 | + 'plug.c', | ||
370 | 'qapi.c', | ||
371 | 'qcow2-bitmap.c', | ||
372 | 'qcow2-cache.c', | ||
47 | -- | 373 | -- |
48 | 2.20.1 | 374 | 2.40.1 |
49 | |||
50 | diff view generated by jsdifflib |
1 | From: Peter Xu <peterx@redhat.com> | 1 | Stop using the .bdrv_co_io_plug() API because it is not multi-queue |
---|---|---|---|
2 | block layer friendly. Use the new blk_io_plug_call() API to batch I/O | ||
3 | submission instead. | ||
2 | 4 | ||
3 | Only sending an init-done message using lock+cond seems an overkill to | 5 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
4 | me. Replacing it with a simpler semaphore. | 6 | Reviewed-by: Eric Blake <eblake@redhat.com> |
5 | 7 | Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> | |
6 | Meanwhile, init the semaphore unconditionally, then we can destroy it | 8 | Acked-by: Kevin Wolf <kwolf@redhat.com> |
7 | unconditionally too in finalize which seems cleaner. | 9 | Message-id: 20230530180959.1108766-3-stefanha@redhat.com |
8 | |||
9 | Signed-off-by: Peter Xu <peterx@redhat.com> | ||
10 | Message-id: 20190306115532.23025-2-peterx@redhat.com | ||
11 | Message-Id: <20190306115532.23025-2-peterx@redhat.com> | ||
12 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
13 | --- | 11 | --- |
14 | include/sysemu/iothread.h | 3 +-- | 12 | block/nvme.c | 44 ++++++++++++-------------------------------- |
15 | iothread.c | 17 ++++------------- | 13 | block/trace-events | 1 - |
16 | 2 files changed, 5 insertions(+), 15 deletions(-) | 14 | 2 files changed, 12 insertions(+), 33 deletions(-) |
17 | 15 | ||
18 | diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h | 16 | diff --git a/block/nvme.c b/block/nvme.c |
19 | index XXXXXXX..XXXXXXX 100644 | 17 | index XXXXXXX..XXXXXXX 100644 |
20 | --- a/include/sysemu/iothread.h | 18 | --- a/block/nvme.c |
21 | +++ b/include/sysemu/iothread.h | 19 | +++ b/block/nvme.c |
22 | @@ -XXX,XX +XXX,XX @@ typedef struct { | 20 | @@ -XXX,XX +XXX,XX @@ |
23 | GMainContext *worker_context; | 21 | #include "qemu/vfio-helpers.h" |
24 | GMainLoop *main_loop; | 22 | #include "block/block-io.h" |
25 | GOnce once; | 23 | #include "block/block_int.h" |
26 | - QemuMutex init_done_lock; | 24 | +#include "sysemu/block-backend.h" |
27 | - QemuCond init_done_cond; /* is thread initialization done? */ | 25 | #include "sysemu/replay.h" |
28 | + QemuSemaphore init_done_sem; /* is thread init done? */ | 26 | #include "trace.h" |
29 | bool stopping; /* has iothread_stop() been called? */ | 27 | |
30 | bool running; /* should iothread_run() continue? */ | 28 | @@ -XXX,XX +XXX,XX @@ struct BDRVNVMeState { |
31 | int thread_id; | 29 | int blkshift; |
32 | diff --git a/iothread.c b/iothread.c | 30 | |
33 | index XXXXXXX..XXXXXXX 100644 | 31 | uint64_t max_transfer; |
34 | --- a/iothread.c | 32 | - bool plugged; |
35 | +++ b/iothread.c | 33 | |
36 | @@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque) | 34 | bool supports_write_zeroes; |
37 | rcu_register_thread(); | 35 | bool supports_discard; |
38 | 36 | @@ -XXX,XX +XXX,XX @@ static void nvme_kick(NVMeQueuePair *q) | |
39 | my_iothread = iothread; | 37 | { |
40 | - qemu_mutex_lock(&iothread->init_done_lock); | 38 | BDRVNVMeState *s = q->s; |
41 | iothread->thread_id = qemu_get_thread_id(); | 39 | |
42 | - qemu_cond_signal(&iothread->init_done_cond); | 40 | - if (s->plugged || !q->need_kick) { |
43 | - qemu_mutex_unlock(&iothread->init_done_lock); | 41 | + if (!q->need_kick) { |
44 | + qemu_sem_post(&iothread->init_done_sem); | ||
45 | |||
46 | while (iothread->running) { | ||
47 | aio_poll(iothread->ctx, true); | ||
48 | @@ -XXX,XX +XXX,XX @@ static void iothread_instance_init(Object *obj) | ||
49 | |||
50 | iothread->poll_max_ns = IOTHREAD_POLL_MAX_NS_DEFAULT; | ||
51 | iothread->thread_id = -1; | ||
52 | + qemu_sem_init(&iothread->init_done_sem, 0); | ||
53 | } | ||
54 | |||
55 | static void iothread_instance_finalize(Object *obj) | ||
56 | @@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj) | ||
57 | |||
58 | iothread_stop(iothread); | ||
59 | |||
60 | - if (iothread->thread_id != -1) { | ||
61 | - qemu_cond_destroy(&iothread->init_done_cond); | ||
62 | - qemu_mutex_destroy(&iothread->init_done_lock); | ||
63 | - } | ||
64 | /* | ||
65 | * Before glib2 2.33.10, there is a glib2 bug that GSource context | ||
66 | * pointer may not be cleared even if the context has already been | ||
67 | @@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj) | ||
68 | g_main_context_unref(iothread->worker_context); | ||
69 | iothread->worker_context = NULL; | ||
70 | } | ||
71 | + qemu_sem_destroy(&iothread->init_done_sem); | ||
72 | } | ||
73 | |||
74 | static void iothread_complete(UserCreatable *obj, Error **errp) | ||
75 | @@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp) | ||
76 | return; | 42 | return; |
77 | } | 43 | } |
78 | 44 | trace_nvme_kick(s, q->index); | |
79 | - qemu_mutex_init(&iothread->init_done_lock); | 45 | @@ -XXX,XX +XXX,XX @@ static bool nvme_process_completion(NVMeQueuePair *q) |
80 | - qemu_cond_init(&iothread->init_done_cond); | 46 | NvmeCqe *c; |
81 | iothread->once = (GOnce) G_ONCE_INIT; | 47 | |
82 | 48 | trace_nvme_process_completion(s, q->index, q->inflight); | |
83 | /* This assumes we are called from a thread with useful CPU affinity for us | 49 | - if (s->plugged) { |
84 | @@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp) | 50 | - trace_nvme_process_completion_queue_plugged(s, q->index); |
85 | g_free(name); | 51 | - return false; |
86 | 52 | - } | |
87 | /* Wait for initialization to complete */ | 53 | |
88 | - qemu_mutex_lock(&iothread->init_done_lock); | 54 | /* |
89 | while (iothread->thread_id == -1) { | 55 | * Support re-entrancy when a request cb() function invokes aio_poll(). |
90 | - qemu_cond_wait(&iothread->init_done_cond, | 56 | @@ -XXX,XX +XXX,XX @@ static void nvme_trace_command(const NvmeCmd *cmd) |
91 | - &iothread->init_done_lock); | ||
92 | + qemu_sem_wait(&iothread->init_done_sem); | ||
93 | } | 57 | } |
94 | - qemu_mutex_unlock(&iothread->init_done_lock); | ||
95 | } | 58 | } |
96 | 59 | ||
97 | typedef struct { | 60 | +static void nvme_unplug_fn(void *opaque) |
61 | +{ | ||
62 | + NVMeQueuePair *q = opaque; | ||
63 | + | ||
64 | + QEMU_LOCK_GUARD(&q->lock); | ||
65 | + nvme_kick(q); | ||
66 | + nvme_process_completion(q); | ||
67 | +} | ||
68 | + | ||
69 | static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req, | ||
70 | NvmeCmd *cmd, BlockCompletionFunc cb, | ||
71 | void *opaque) | ||
72 | @@ -XXX,XX +XXX,XX @@ static void nvme_submit_command(NVMeQueuePair *q, NVMeRequest *req, | ||
73 | q->sq.tail * NVME_SQ_ENTRY_BYTES, cmd, sizeof(*cmd)); | ||
74 | q->sq.tail = (q->sq.tail + 1) % NVME_QUEUE_SIZE; | ||
75 | q->need_kick++; | ||
76 | - nvme_kick(q); | ||
77 | - nvme_process_completion(q); | ||
78 | + blk_io_plug_call(nvme_unplug_fn, q); | ||
79 | qemu_mutex_unlock(&q->lock); | ||
80 | } | ||
81 | |||
82 | @@ -XXX,XX +XXX,XX @@ static void nvme_attach_aio_context(BlockDriverState *bs, | ||
83 | } | ||
84 | } | ||
85 | |||
86 | -static void coroutine_fn nvme_co_io_plug(BlockDriverState *bs) | ||
87 | -{ | ||
88 | - BDRVNVMeState *s = bs->opaque; | ||
89 | - assert(!s->plugged); | ||
90 | - s->plugged = true; | ||
91 | -} | ||
92 | - | ||
93 | -static void coroutine_fn nvme_co_io_unplug(BlockDriverState *bs) | ||
94 | -{ | ||
95 | - BDRVNVMeState *s = bs->opaque; | ||
96 | - assert(s->plugged); | ||
97 | - s->plugged = false; | ||
98 | - for (unsigned i = INDEX_IO(0); i < s->queue_count; i++) { | ||
99 | - NVMeQueuePair *q = s->queues[i]; | ||
100 | - qemu_mutex_lock(&q->lock); | ||
101 | - nvme_kick(q); | ||
102 | - nvme_process_completion(q); | ||
103 | - qemu_mutex_unlock(&q->lock); | ||
104 | - } | ||
105 | -} | ||
106 | - | ||
107 | static bool nvme_register_buf(BlockDriverState *bs, void *host, size_t size, | ||
108 | Error **errp) | ||
109 | { | ||
110 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_nvme = { | ||
111 | .bdrv_detach_aio_context = nvme_detach_aio_context, | ||
112 | .bdrv_attach_aio_context = nvme_attach_aio_context, | ||
113 | |||
114 | - .bdrv_co_io_plug = nvme_co_io_plug, | ||
115 | - .bdrv_co_io_unplug = nvme_co_io_unplug, | ||
116 | - | ||
117 | .bdrv_register_buf = nvme_register_buf, | ||
118 | .bdrv_unregister_buf = nvme_unregister_buf, | ||
119 | }; | ||
120 | diff --git a/block/trace-events b/block/trace-events | ||
121 | index XXXXXXX..XXXXXXX 100644 | ||
122 | --- a/block/trace-events | ||
123 | +++ b/block/trace-events | ||
124 | @@ -XXX,XX +XXX,XX @@ nvme_kick(void *s, unsigned q_index) "s %p q #%u" | ||
125 | nvme_dma_flush_queue_wait(void *s) "s %p" | ||
126 | nvme_error(int cmd_specific, int sq_head, int sqid, int cid, int status) "cmd_specific %d sq_head %d sqid %d cid %d status 0x%x" | ||
127 | nvme_process_completion(void *s, unsigned q_index, int inflight) "s %p q #%u inflight %d" | ||
128 | -nvme_process_completion_queue_plugged(void *s, unsigned q_index) "s %p q #%u" | ||
129 | nvme_complete_command(void *s, unsigned q_index, int cid) "s %p q #%u cid %d" | ||
130 | nvme_submit_command(void *s, unsigned q_index, int cid) "s %p q #%u cid %d" | ||
131 | nvme_submit_command_raw(int c0, int c1, int c2, int c3, int c4, int c5, int c6, int c7) "%02x %02x %02x %02x %02x %02x %02x %02x" | ||
98 | -- | 132 | -- |
99 | 2.20.1 | 133 | 2.40.1 |
100 | |||
101 | diff view generated by jsdifflib |
1 | From: Peter Xu <peterx@redhat.com> | 1 | Stop using the .bdrv_co_io_plug() API because it is not multi-queue |
---|---|---|---|
2 | block layer friendly. Use the new blk_io_plug_call() API to batch I/O | ||
3 | submission instead. | ||
2 | 4 | ||
3 | In existing code we create the gcontext dynamically at the first | 5 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
4 | access of the gcontext from caller. That can bring some complexity | 6 | Reviewed-by: Eric Blake <eblake@redhat.com> |
5 | and potential races during using iothread. Since the context itself | 7 | Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> |
6 | is not that big a resource, and we won't have millions of iothread, | 8 | Acked-by: Kevin Wolf <kwolf@redhat.com> |
7 | let's simply create the gcontext unconditionally. | 9 | Message-id: 20230530180959.1108766-4-stefanha@redhat.com |
8 | |||
9 | This will also be a preparation work further to move the thread | ||
10 | context push operation earlier than before (now it's only pushed right | ||
11 | before we want to start running the gmainloop). | ||
12 | |||
13 | Removing the g_once since it's not necessary, while introducing a new | ||
14 | run_gcontext boolean to show whether we want to run the gcontext. | ||
15 | |||
16 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | ||
17 | Signed-off-by: Peter Xu <peterx@redhat.com> | ||
18 | Message-id: 20190306115532.23025-3-peterx@redhat.com | ||
19 | Message-Id: <20190306115532.23025-3-peterx@redhat.com> | ||
20 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
21 | --- | 11 | --- |
22 | include/sysemu/iothread.h | 2 +- | 12 | block/blkio.c | 43 ++++++++++++++++++++++++------------------- |
23 | iothread.c | 43 +++++++++++++++++++-------------------- | 13 | 1 file changed, 24 insertions(+), 19 deletions(-) |
24 | 2 files changed, 22 insertions(+), 23 deletions(-) | ||
25 | 14 | ||
26 | diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h | 15 | diff --git a/block/blkio.c b/block/blkio.c |
27 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
28 | --- a/include/sysemu/iothread.h | 17 | --- a/block/blkio.c |
29 | +++ b/include/sysemu/iothread.h | 18 | +++ b/block/blkio.c |
30 | @@ -XXX,XX +XXX,XX @@ typedef struct { | 19 | @@ -XXX,XX +XXX,XX @@ |
31 | 20 | #include "qemu/error-report.h" | |
32 | QemuThread thread; | 21 | #include "qapi/qmp/qdict.h" |
33 | AioContext *ctx; | 22 | #include "qemu/module.h" |
34 | + bool run_gcontext; /* whether we should run gcontext */ | 23 | +#include "sysemu/block-backend.h" |
35 | GMainContext *worker_context; | 24 | #include "exec/memory.h" /* for ram_block_discard_disable() */ |
36 | GMainLoop *main_loop; | 25 | |
37 | - GOnce once; | 26 | #include "block/block-io.h" |
38 | QemuSemaphore init_done_sem; /* is thread init done? */ | 27 | @@ -XXX,XX +XXX,XX @@ static void blkio_detach_aio_context(BlockDriverState *bs) |
39 | bool stopping; /* has iothread_stop() been called? */ | 28 | NULL, NULL, NULL); |
40 | bool running; /* should iothread_run() continue? */ | ||
41 | diff --git a/iothread.c b/iothread.c | ||
42 | index XXXXXXX..XXXXXXX 100644 | ||
43 | --- a/iothread.c | ||
44 | +++ b/iothread.c | ||
45 | @@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque) | ||
46 | * We must check the running state again in case it was | ||
47 | * changed in previous aio_poll() | ||
48 | */ | ||
49 | - if (iothread->running && atomic_read(&iothread->worker_context)) { | ||
50 | + if (iothread->running && atomic_read(&iothread->run_gcontext)) { | ||
51 | GMainLoop *loop; | ||
52 | |||
53 | g_main_context_push_thread_default(iothread->worker_context); | ||
54 | @@ -XXX,XX +XXX,XX @@ static void iothread_instance_init(Object *obj) | ||
55 | iothread->poll_max_ns = IOTHREAD_POLL_MAX_NS_DEFAULT; | ||
56 | iothread->thread_id = -1; | ||
57 | qemu_sem_init(&iothread->init_done_sem, 0); | ||
58 | + /* By default, we don't run gcontext */ | ||
59 | + atomic_set(&iothread->run_gcontext, 0); | ||
60 | } | 29 | } |
61 | 30 | ||
62 | static void iothread_instance_finalize(Object *obj) | 31 | -/* Call with s->blkio_lock held to submit I/O after enqueuing a new request */ |
63 | @@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj) | 32 | -static void blkio_submit_io(BlockDriverState *bs) |
64 | qemu_sem_destroy(&iothread->init_done_sem); | 33 | +/* |
34 | + * Called by blk_io_unplug() or immediately if not plugged. Called without | ||
35 | + * blkio_lock. | ||
36 | + */ | ||
37 | +static void blkio_unplug_fn(void *opaque) | ||
38 | { | ||
39 | - if (qatomic_read(&bs->io_plugged) == 0) { | ||
40 | - BDRVBlkioState *s = bs->opaque; | ||
41 | + BDRVBlkioState *s = opaque; | ||
42 | |||
43 | + WITH_QEMU_LOCK_GUARD(&s->blkio_lock) { | ||
44 | blkioq_do_io(s->blkioq, NULL, 0, 0, NULL); | ||
45 | } | ||
65 | } | 46 | } |
66 | 47 | ||
67 | +static void iothread_init_gcontext(IOThread *iothread) | 48 | +/* |
49 | + * Schedule I/O submission after enqueuing a new request. Called without | ||
50 | + * blkio_lock. | ||
51 | + */ | ||
52 | +static void blkio_submit_io(BlockDriverState *bs) | ||
68 | +{ | 53 | +{ |
69 | + GSource *source; | 54 | + BDRVBlkioState *s = bs->opaque; |
70 | + | 55 | + |
71 | + iothread->worker_context = g_main_context_new(); | 56 | + blk_io_plug_call(blkio_unplug_fn, s); |
72 | + source = aio_get_g_source(iothread_get_aio_context(iothread)); | ||
73 | + g_source_attach(source, iothread->worker_context); | ||
74 | + g_source_unref(source); | ||
75 | +} | 57 | +} |
76 | + | 58 | + |
77 | static void iothread_complete(UserCreatable *obj, Error **errp) | 59 | static int coroutine_fn |
60 | blkio_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes) | ||
78 | { | 61 | { |
79 | Error *local_error = NULL; | 62 | @@ -XXX,XX +XXX,XX @@ blkio_co_pdiscard(BlockDriverState *bs, int64_t offset, int64_t bytes) |
80 | @@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp) | 63 | |
81 | return; | 64 | WITH_QEMU_LOCK_GUARD(&s->blkio_lock) { |
65 | blkioq_discard(s->blkioq, offset, bytes, &cod, 0); | ||
66 | - blkio_submit_io(bs); | ||
82 | } | 67 | } |
83 | 68 | ||
84 | + /* | 69 | + blkio_submit_io(bs); |
85 | + * Init one GMainContext for the iothread unconditionally, even if | 70 | qemu_coroutine_yield(); |
86 | + * it's not used | 71 | return cod.ret; |
87 | + */ | 72 | } |
88 | + iothread_init_gcontext(iothread); | 73 | @@ -XXX,XX +XXX,XX @@ blkio_co_preadv(BlockDriverState *bs, int64_t offset, int64_t bytes, |
89 | + | 74 | |
90 | aio_context_set_poll_params(iothread->ctx, | 75 | WITH_QEMU_LOCK_GUARD(&s->blkio_lock) { |
91 | iothread->poll_max_ns, | 76 | blkioq_readv(s->blkioq, offset, iov, iovcnt, &cod, 0); |
92 | iothread->poll_grow, | 77 | - blkio_submit_io(bs); |
93 | @@ -XXX,XX +XXX,XX @@ static void iothread_complete(UserCreatable *obj, Error **errp) | ||
94 | return; | ||
95 | } | 78 | } |
96 | 79 | ||
97 | - iothread->once = (GOnce) G_ONCE_INIT; | 80 | + blkio_submit_io(bs); |
81 | qemu_coroutine_yield(); | ||
82 | |||
83 | if (use_bounce_buffer) { | ||
84 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkio_co_pwritev(BlockDriverState *bs, int64_t offset, | ||
85 | |||
86 | WITH_QEMU_LOCK_GUARD(&s->blkio_lock) { | ||
87 | blkioq_writev(s->blkioq, offset, iov, iovcnt, &cod, blkio_flags); | ||
88 | - blkio_submit_io(bs); | ||
89 | } | ||
90 | |||
91 | + blkio_submit_io(bs); | ||
92 | qemu_coroutine_yield(); | ||
93 | |||
94 | if (use_bounce_buffer) { | ||
95 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkio_co_flush(BlockDriverState *bs) | ||
96 | |||
97 | WITH_QEMU_LOCK_GUARD(&s->blkio_lock) { | ||
98 | blkioq_flush(s->blkioq, &cod, 0); | ||
99 | - blkio_submit_io(bs); | ||
100 | } | ||
101 | |||
102 | + blkio_submit_io(bs); | ||
103 | qemu_coroutine_yield(); | ||
104 | return cod.ret; | ||
105 | } | ||
106 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn blkio_co_pwrite_zeroes(BlockDriverState *bs, | ||
107 | |||
108 | WITH_QEMU_LOCK_GUARD(&s->blkio_lock) { | ||
109 | blkioq_write_zeroes(s->blkioq, offset, bytes, &cod, blkio_flags); | ||
110 | - blkio_submit_io(bs); | ||
111 | } | ||
112 | |||
113 | + blkio_submit_io(bs); | ||
114 | qemu_coroutine_yield(); | ||
115 | return cod.ret; | ||
116 | } | ||
117 | |||
118 | -static void coroutine_fn blkio_co_io_unplug(BlockDriverState *bs) | ||
119 | -{ | ||
120 | - BDRVBlkioState *s = bs->opaque; | ||
98 | - | 121 | - |
99 | /* This assumes we are called from a thread with useful CPU affinity for us | 122 | - WITH_QEMU_LOCK_GUARD(&s->blkio_lock) { |
100 | * to inherit. | 123 | - blkio_submit_io(bs); |
101 | */ | 124 | - } |
102 | @@ -XXX,XX +XXX,XX @@ IOThreadInfoList *qmp_query_iothreads(Error **errp) | ||
103 | return head; | ||
104 | } | ||
105 | |||
106 | -static gpointer iothread_g_main_context_init(gpointer opaque) | ||
107 | -{ | ||
108 | - AioContext *ctx; | ||
109 | - IOThread *iothread = opaque; | ||
110 | - GSource *source; | ||
111 | - | ||
112 | - iothread->worker_context = g_main_context_new(); | ||
113 | - | ||
114 | - ctx = iothread_get_aio_context(iothread); | ||
115 | - source = aio_get_g_source(ctx); | ||
116 | - g_source_attach(source, iothread->worker_context); | ||
117 | - g_source_unref(source); | ||
118 | - | ||
119 | - aio_notify(iothread->ctx); | ||
120 | - return NULL; | ||
121 | -} | 125 | -} |
122 | - | 126 | - |
123 | GMainContext *iothread_get_g_main_context(IOThread *iothread) | 127 | typedef enum { |
124 | { | 128 | BMRR_OK, |
125 | - g_once(&iothread->once, iothread_g_main_context_init, iothread); | 129 | BMRR_SKIP, |
126 | - | 130 | @@ -XXX,XX +XXX,XX @@ static void blkio_refresh_limits(BlockDriverState *bs, Error **errp) |
127 | + atomic_set(&iothread->run_gcontext, 1); | 131 | .bdrv_co_pwritev = blkio_co_pwritev, \ |
128 | + aio_notify(iothread->ctx); | 132 | .bdrv_co_flush_to_disk = blkio_co_flush, \ |
129 | return iothread->worker_context; | 133 | .bdrv_co_pwrite_zeroes = blkio_co_pwrite_zeroes, \ |
130 | } | 134 | - .bdrv_co_io_unplug = blkio_co_io_unplug, \ |
131 | 135 | .bdrv_refresh_limits = blkio_refresh_limits, \ | |
136 | .bdrv_register_buf = blkio_register_buf, \ | ||
137 | .bdrv_unregister_buf = blkio_unregister_buf, \ | ||
132 | -- | 138 | -- |
133 | 2.20.1 | 139 | 2.40.1 |
134 | |||
135 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | Stop using the .bdrv_co_io_plug() API because it is not multi-queue | ||
2 | block layer friendly. Use the new blk_io_plug_call() API to batch I/O | ||
3 | submission instead. | ||
1 | 4 | ||
5 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
6 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
7 | Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> | ||
8 | Acked-by: Kevin Wolf <kwolf@redhat.com> | ||
9 | Message-id: 20230530180959.1108766-5-stefanha@redhat.com | ||
10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
11 | --- | ||
12 | include/block/raw-aio.h | 7 ------- | ||
13 | block/file-posix.c | 10 ---------- | ||
14 | block/io_uring.c | 44 ++++++++++++++++------------------------- | ||
15 | block/trace-events | 5 ++--- | ||
16 | 4 files changed, 19 insertions(+), 47 deletions(-) | ||
17 | |||
18 | diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h | ||
19 | index XXXXXXX..XXXXXXX 100644 | ||
20 | --- a/include/block/raw-aio.h | ||
21 | +++ b/include/block/raw-aio.h | ||
22 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn luring_co_submit(BlockDriverState *bs, int fd, uint64_t offset, | ||
23 | QEMUIOVector *qiov, int type); | ||
24 | void luring_detach_aio_context(LuringState *s, AioContext *old_context); | ||
25 | void luring_attach_aio_context(LuringState *s, AioContext *new_context); | ||
26 | - | ||
27 | -/* | ||
28 | - * luring_io_plug/unplug work in the thread's current AioContext, therefore the | ||
29 | - * caller must ensure that they are paired in the same IOThread. | ||
30 | - */ | ||
31 | -void luring_io_plug(void); | ||
32 | -void luring_io_unplug(void); | ||
33 | #endif | ||
34 | |||
35 | #ifdef _WIN32 | ||
36 | diff --git a/block/file-posix.c b/block/file-posix.c | ||
37 | index XXXXXXX..XXXXXXX 100644 | ||
38 | --- a/block/file-posix.c | ||
39 | +++ b/block/file-posix.c | ||
40 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_io_plug(BlockDriverState *bs) | ||
41 | laio_io_plug(); | ||
42 | } | ||
43 | #endif | ||
44 | -#ifdef CONFIG_LINUX_IO_URING | ||
45 | - if (s->use_linux_io_uring) { | ||
46 | - luring_io_plug(); | ||
47 | - } | ||
48 | -#endif | ||
49 | } | ||
50 | |||
51 | static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs) | ||
52 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs) | ||
53 | laio_io_unplug(s->aio_max_batch); | ||
54 | } | ||
55 | #endif | ||
56 | -#ifdef CONFIG_LINUX_IO_URING | ||
57 | - if (s->use_linux_io_uring) { | ||
58 | - luring_io_unplug(); | ||
59 | - } | ||
60 | -#endif | ||
61 | } | ||
62 | |||
63 | static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs) | ||
64 | diff --git a/block/io_uring.c b/block/io_uring.c | ||
65 | index XXXXXXX..XXXXXXX 100644 | ||
66 | --- a/block/io_uring.c | ||
67 | +++ b/block/io_uring.c | ||
68 | @@ -XXX,XX +XXX,XX @@ | ||
69 | #include "block/raw-aio.h" | ||
70 | #include "qemu/coroutine.h" | ||
71 | #include "qapi/error.h" | ||
72 | +#include "sysemu/block-backend.h" | ||
73 | #include "trace.h" | ||
74 | |||
75 | /* Only used for assertions. */ | ||
76 | @@ -XXX,XX +XXX,XX @@ typedef struct LuringAIOCB { | ||
77 | } LuringAIOCB; | ||
78 | |||
79 | typedef struct LuringQueue { | ||
80 | - int plugged; | ||
81 | unsigned int in_queue; | ||
82 | unsigned int in_flight; | ||
83 | bool blocked; | ||
84 | @@ -XXX,XX +XXX,XX @@ static void luring_process_completions_and_submit(LuringState *s) | ||
85 | { | ||
86 | luring_process_completions(s); | ||
87 | |||
88 | - if (!s->io_q.plugged && s->io_q.in_queue > 0) { | ||
89 | + if (s->io_q.in_queue > 0) { | ||
90 | ioq_submit(s); | ||
91 | } | ||
92 | } | ||
93 | @@ -XXX,XX +XXX,XX @@ static void qemu_luring_poll_ready(void *opaque) | ||
94 | static void ioq_init(LuringQueue *io_q) | ||
95 | { | ||
96 | QSIMPLEQ_INIT(&io_q->submit_queue); | ||
97 | - io_q->plugged = 0; | ||
98 | io_q->in_queue = 0; | ||
99 | io_q->in_flight = 0; | ||
100 | io_q->blocked = false; | ||
101 | } | ||
102 | |||
103 | -void luring_io_plug(void) | ||
104 | +static void luring_unplug_fn(void *opaque) | ||
105 | { | ||
106 | - AioContext *ctx = qemu_get_current_aio_context(); | ||
107 | - LuringState *s = aio_get_linux_io_uring(ctx); | ||
108 | - trace_luring_io_plug(s); | ||
109 | - s->io_q.plugged++; | ||
110 | -} | ||
111 | - | ||
112 | -void luring_io_unplug(void) | ||
113 | -{ | ||
114 | - AioContext *ctx = qemu_get_current_aio_context(); | ||
115 | - LuringState *s = aio_get_linux_io_uring(ctx); | ||
116 | - assert(s->io_q.plugged); | ||
117 | - trace_luring_io_unplug(s, s->io_q.blocked, s->io_q.plugged, | ||
118 | - s->io_q.in_queue, s->io_q.in_flight); | ||
119 | - if (--s->io_q.plugged == 0 && | ||
120 | - !s->io_q.blocked && s->io_q.in_queue > 0) { | ||
121 | + LuringState *s = opaque; | ||
122 | + trace_luring_unplug_fn(s, s->io_q.blocked, s->io_q.in_queue, | ||
123 | + s->io_q.in_flight); | ||
124 | + if (!s->io_q.blocked && s->io_q.in_queue > 0) { | ||
125 | ioq_submit(s); | ||
126 | } | ||
127 | } | ||
128 | @@ -XXX,XX +XXX,XX @@ static int luring_do_submit(int fd, LuringAIOCB *luringcb, LuringState *s, | ||
129 | |||
130 | QSIMPLEQ_INSERT_TAIL(&s->io_q.submit_queue, luringcb, next); | ||
131 | s->io_q.in_queue++; | ||
132 | - trace_luring_do_submit(s, s->io_q.blocked, s->io_q.plugged, | ||
133 | - s->io_q.in_queue, s->io_q.in_flight); | ||
134 | - if (!s->io_q.blocked && | ||
135 | - (!s->io_q.plugged || | ||
136 | - s->io_q.in_flight + s->io_q.in_queue >= MAX_ENTRIES)) { | ||
137 | - ret = ioq_submit(s); | ||
138 | - trace_luring_do_submit_done(s, ret); | ||
139 | - return ret; | ||
140 | + trace_luring_do_submit(s, s->io_q.blocked, s->io_q.in_queue, | ||
141 | + s->io_q.in_flight); | ||
142 | + if (!s->io_q.blocked) { | ||
143 | + if (s->io_q.in_flight + s->io_q.in_queue >= MAX_ENTRIES) { | ||
144 | + ret = ioq_submit(s); | ||
145 | + trace_luring_do_submit_done(s, ret); | ||
146 | + return ret; | ||
147 | + } | ||
148 | + | ||
149 | + blk_io_plug_call(luring_unplug_fn, s); | ||
150 | } | ||
151 | return 0; | ||
152 | } | ||
153 | diff --git a/block/trace-events b/block/trace-events | ||
154 | index XXXXXXX..XXXXXXX 100644 | ||
155 | --- a/block/trace-events | ||
156 | +++ b/block/trace-events | ||
157 | @@ -XXX,XX +XXX,XX @@ file_paio_submit(void *acb, void *opaque, int64_t offset, int count, int type) " | ||
158 | # io_uring.c | ||
159 | luring_init_state(void *s, size_t size) "s %p size %zu" | ||
160 | luring_cleanup_state(void *s) "%p freed" | ||
161 | -luring_io_plug(void *s) "LuringState %p plug" | ||
162 | -luring_io_unplug(void *s, int blocked, int plugged, int queued, int inflight) "LuringState %p blocked %d plugged %d queued %d inflight %d" | ||
163 | -luring_do_submit(void *s, int blocked, int plugged, int queued, int inflight) "LuringState %p blocked %d plugged %d queued %d inflight %d" | ||
164 | +luring_unplug_fn(void *s, int blocked, int queued, int inflight) "LuringState %p blocked %d queued %d inflight %d" | ||
165 | +luring_do_submit(void *s, int blocked, int queued, int inflight) "LuringState %p blocked %d queued %d inflight %d" | ||
166 | luring_do_submit_done(void *s, int ret) "LuringState %p submitted to kernel %d" | ||
167 | luring_co_submit(void *bs, void *s, void *luringcb, int fd, uint64_t offset, size_t nbytes, int type) "bs %p s %p luringcb %p fd %d offset %" PRId64 " nbytes %zd type %d" | ||
168 | luring_process_completion(void *s, void *aiocb, int ret) "LuringState %p luringcb %p ret %d" | ||
169 | -- | ||
170 | 2.40.1 | diff view generated by jsdifflib |
1 | From: Peter Xu <peterx@redhat.com> | 1 | Stop using the .bdrv_co_io_plug() API because it is not multi-queue |
---|---|---|---|
2 | 2 | block layer friendly. Use the new blk_io_plug_call() API to batch I/O | |
3 | After consulting Paolo I know why we'd better keep the explicit | 3 | submission instead. |
4 | aio_poll() in iothread_run(). Document it directly into the code so | 4 | |
5 | that future readers will know the answer from day one. | 5 | Note that a dev_max_batch check is dropped in laio_io_unplug() because |
6 | 6 | the semantics of unplug_fn() are different from .bdrv_co_unplug(): | |
7 | Signed-off-by: Peter Xu <peterx@redhat.com> | 7 | 1. unplug_fn() is only called when the last blk_io_unplug() call occurs, |
8 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | 8 | not every time blk_io_unplug() is called. |
9 | Message-id: 20190306115532.23025-6-peterx@redhat.com | 9 | 2. unplug_fn() is per-thread, not per-BlockDriverState, so there is no |
10 | Message-Id: <20190306115532.23025-6-peterx@redhat.com> | 10 | way to get per-BlockDriverState fields like dev_max_batch. |
11 | |||
12 | Therefore this condition cannot be moved to laio_unplug_fn(). It is not | ||
13 | obvious that this condition affects performance in practice, so I am | ||
14 | removing it instead of trying to come up with a more complex mechanism | ||
15 | to preserve the condition. | ||
16 | |||
17 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
18 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
19 | Acked-by: Kevin Wolf <kwolf@redhat.com> | ||
20 | Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> | ||
21 | Message-id: 20230530180959.1108766-6-stefanha@redhat.com | ||
11 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 22 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
12 | --- | 23 | --- |
13 | iothread.c | 9 +++++++++ | 24 | include/block/raw-aio.h | 7 ------- |
14 | 1 file changed, 9 insertions(+) | 25 | block/file-posix.c | 28 ---------------------------- |
15 | 26 | block/linux-aio.c | 41 +++++++++++------------------------------ | |
16 | diff --git a/iothread.c b/iothread.c | 27 | 3 files changed, 11 insertions(+), 65 deletions(-) |
28 | |||
29 | diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h | ||
17 | index XXXXXXX..XXXXXXX 100644 | 30 | index XXXXXXX..XXXXXXX 100644 |
18 | --- a/iothread.c | 31 | --- a/include/block/raw-aio.h |
19 | +++ b/iothread.c | 32 | +++ b/include/block/raw-aio.h |
20 | @@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque) | 33 | @@ -XXX,XX +XXX,XX @@ int coroutine_fn laio_co_submit(int fd, uint64_t offset, QEMUIOVector *qiov, |
21 | qemu_sem_post(&iothread->init_done_sem); | 34 | |
22 | 35 | void laio_detach_aio_context(LinuxAioState *s, AioContext *old_context); | |
23 | while (iothread->running) { | 36 | void laio_attach_aio_context(LinuxAioState *s, AioContext *new_context); |
24 | + /* | 37 | - |
25 | + * Note: from functional-wise the g_main_loop_run() below can | 38 | -/* |
26 | + * already cover the aio_poll() events, but we can't run the | 39 | - * laio_io_plug/unplug work in the thread's current AioContext, therefore the |
27 | + * main loop unconditionally because explicit aio_poll() here | 40 | - * caller must ensure that they are paired in the same IOThread. |
28 | + * is faster than g_main_loop_run() when we do not need the | 41 | - */ |
29 | + * gcontext at all (e.g., pure block layer iothreads). In | 42 | -void laio_io_plug(void); |
30 | + * other words, when we want to run the gcontext with the | 43 | -void laio_io_unplug(uint64_t dev_max_batch); |
31 | + * iothread we need to pay some performance for functionality. | 44 | #endif |
32 | + */ | 45 | /* io_uring.c - Linux io_uring implementation */ |
33 | aio_poll(iothread->ctx, true); | 46 | #ifdef CONFIG_LINUX_IO_URING |
34 | 47 | diff --git a/block/file-posix.c b/block/file-posix.c | |
35 | /* | 48 | index XXXXXXX..XXXXXXX 100644 |
49 | --- a/block/file-posix.c | ||
50 | +++ b/block/file-posix.c | ||
51 | @@ -XXX,XX +XXX,XX @@ static int coroutine_fn raw_co_pwritev(BlockDriverState *bs, int64_t offset, | ||
52 | return raw_co_prw(bs, offset, bytes, qiov, QEMU_AIO_WRITE); | ||
53 | } | ||
54 | |||
55 | -static void coroutine_fn raw_co_io_plug(BlockDriverState *bs) | ||
56 | -{ | ||
57 | - BDRVRawState __attribute__((unused)) *s = bs->opaque; | ||
58 | -#ifdef CONFIG_LINUX_AIO | ||
59 | - if (s->use_linux_aio) { | ||
60 | - laio_io_plug(); | ||
61 | - } | ||
62 | -#endif | ||
63 | -} | ||
64 | - | ||
65 | -static void coroutine_fn raw_co_io_unplug(BlockDriverState *bs) | ||
66 | -{ | ||
67 | - BDRVRawState __attribute__((unused)) *s = bs->opaque; | ||
68 | -#ifdef CONFIG_LINUX_AIO | ||
69 | - if (s->use_linux_aio) { | ||
70 | - laio_io_unplug(s->aio_max_batch); | ||
71 | - } | ||
72 | -#endif | ||
73 | -} | ||
74 | - | ||
75 | static int coroutine_fn raw_co_flush_to_disk(BlockDriverState *bs) | ||
76 | { | ||
77 | BDRVRawState *s = bs->opaque; | ||
78 | @@ -XXX,XX +XXX,XX @@ BlockDriver bdrv_file = { | ||
79 | .bdrv_co_copy_range_from = raw_co_copy_range_from, | ||
80 | .bdrv_co_copy_range_to = raw_co_copy_range_to, | ||
81 | .bdrv_refresh_limits = raw_refresh_limits, | ||
82 | - .bdrv_co_io_plug = raw_co_io_plug, | ||
83 | - .bdrv_co_io_unplug = raw_co_io_unplug, | ||
84 | .bdrv_attach_aio_context = raw_aio_attach_aio_context, | ||
85 | |||
86 | .bdrv_co_truncate = raw_co_truncate, | ||
87 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_device = { | ||
88 | .bdrv_co_copy_range_from = raw_co_copy_range_from, | ||
89 | .bdrv_co_copy_range_to = raw_co_copy_range_to, | ||
90 | .bdrv_refresh_limits = raw_refresh_limits, | ||
91 | - .bdrv_co_io_plug = raw_co_io_plug, | ||
92 | - .bdrv_co_io_unplug = raw_co_io_unplug, | ||
93 | .bdrv_attach_aio_context = raw_aio_attach_aio_context, | ||
94 | |||
95 | .bdrv_co_truncate = raw_co_truncate, | ||
96 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_cdrom = { | ||
97 | .bdrv_co_pwritev = raw_co_pwritev, | ||
98 | .bdrv_co_flush_to_disk = raw_co_flush_to_disk, | ||
99 | .bdrv_refresh_limits = cdrom_refresh_limits, | ||
100 | - .bdrv_co_io_plug = raw_co_io_plug, | ||
101 | - .bdrv_co_io_unplug = raw_co_io_unplug, | ||
102 | .bdrv_attach_aio_context = raw_aio_attach_aio_context, | ||
103 | |||
104 | .bdrv_co_truncate = raw_co_truncate, | ||
105 | @@ -XXX,XX +XXX,XX @@ static BlockDriver bdrv_host_cdrom = { | ||
106 | .bdrv_co_pwritev = raw_co_pwritev, | ||
107 | .bdrv_co_flush_to_disk = raw_co_flush_to_disk, | ||
108 | .bdrv_refresh_limits = cdrom_refresh_limits, | ||
109 | - .bdrv_co_io_plug = raw_co_io_plug, | ||
110 | - .bdrv_co_io_unplug = raw_co_io_unplug, | ||
111 | .bdrv_attach_aio_context = raw_aio_attach_aio_context, | ||
112 | |||
113 | .bdrv_co_truncate = raw_co_truncate, | ||
114 | diff --git a/block/linux-aio.c b/block/linux-aio.c | ||
115 | index XXXXXXX..XXXXXXX 100644 | ||
116 | --- a/block/linux-aio.c | ||
117 | +++ b/block/linux-aio.c | ||
118 | @@ -XXX,XX +XXX,XX @@ | ||
119 | #include "qemu/event_notifier.h" | ||
120 | #include "qemu/coroutine.h" | ||
121 | #include "qapi/error.h" | ||
122 | +#include "sysemu/block-backend.h" | ||
123 | |||
124 | /* Only used for assertions. */ | ||
125 | #include "qemu/coroutine_int.h" | ||
126 | @@ -XXX,XX +XXX,XX @@ struct qemu_laiocb { | ||
127 | }; | ||
128 | |||
129 | typedef struct { | ||
130 | - int plugged; | ||
131 | unsigned int in_queue; | ||
132 | unsigned int in_flight; | ||
133 | bool blocked; | ||
134 | @@ -XXX,XX +XXX,XX @@ static void qemu_laio_process_completions_and_submit(LinuxAioState *s) | ||
135 | { | ||
136 | qemu_laio_process_completions(s); | ||
137 | |||
138 | - if (!s->io_q.plugged && !QSIMPLEQ_EMPTY(&s->io_q.pending)) { | ||
139 | + if (!QSIMPLEQ_EMPTY(&s->io_q.pending)) { | ||
140 | ioq_submit(s); | ||
141 | } | ||
142 | } | ||
143 | @@ -XXX,XX +XXX,XX @@ static void qemu_laio_poll_ready(EventNotifier *opaque) | ||
144 | static void ioq_init(LaioQueue *io_q) | ||
145 | { | ||
146 | QSIMPLEQ_INIT(&io_q->pending); | ||
147 | - io_q->plugged = 0; | ||
148 | io_q->in_queue = 0; | ||
149 | io_q->in_flight = 0; | ||
150 | io_q->blocked = false; | ||
151 | @@ -XXX,XX +XXX,XX @@ static uint64_t laio_max_batch(LinuxAioState *s, uint64_t dev_max_batch) | ||
152 | return max_batch; | ||
153 | } | ||
154 | |||
155 | -void laio_io_plug(void) | ||
156 | +static void laio_unplug_fn(void *opaque) | ||
157 | { | ||
158 | - AioContext *ctx = qemu_get_current_aio_context(); | ||
159 | - LinuxAioState *s = aio_get_linux_aio(ctx); | ||
160 | + LinuxAioState *s = opaque; | ||
161 | |||
162 | - s->io_q.plugged++; | ||
163 | -} | ||
164 | - | ||
165 | -void laio_io_unplug(uint64_t dev_max_batch) | ||
166 | -{ | ||
167 | - AioContext *ctx = qemu_get_current_aio_context(); | ||
168 | - LinuxAioState *s = aio_get_linux_aio(ctx); | ||
169 | - | ||
170 | - assert(s->io_q.plugged); | ||
171 | - s->io_q.plugged--; | ||
172 | - | ||
173 | - /* | ||
174 | - * Why max batch checking is performed here: | ||
175 | - * Another BDS may have queued requests with a higher dev_max_batch and | ||
176 | - * therefore in_queue could now exceed our dev_max_batch. Re-check the max | ||
177 | - * batch so we can honor our device's dev_max_batch. | ||
178 | - */ | ||
179 | - if (s->io_q.in_queue >= laio_max_batch(s, dev_max_batch) || | ||
180 | - (!s->io_q.plugged && | ||
181 | - !s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending))) { | ||
182 | + if (!s->io_q.blocked && !QSIMPLEQ_EMPTY(&s->io_q.pending)) { | ||
183 | ioq_submit(s); | ||
184 | } | ||
185 | } | ||
186 | @@ -XXX,XX +XXX,XX @@ static int laio_do_submit(int fd, struct qemu_laiocb *laiocb, off_t offset, | ||
187 | |||
188 | QSIMPLEQ_INSERT_TAIL(&s->io_q.pending, laiocb, next); | ||
189 | s->io_q.in_queue++; | ||
190 | - if (!s->io_q.blocked && | ||
191 | - (!s->io_q.plugged || | ||
192 | - s->io_q.in_queue >= laio_max_batch(s, dev_max_batch))) { | ||
193 | - ioq_submit(s); | ||
194 | + if (!s->io_q.blocked) { | ||
195 | + if (s->io_q.in_queue >= laio_max_batch(s, dev_max_batch)) { | ||
196 | + ioq_submit(s); | ||
197 | + } else { | ||
198 | + blk_io_plug_call(laio_unplug_fn, s); | ||
199 | + } | ||
200 | } | ||
201 | |||
202 | return 0; | ||
36 | -- | 203 | -- |
37 | 2.20.1 | 204 | 2.40.1 |
38 | |||
39 | diff view generated by jsdifflib |
1 | From: Peter Xu <peterx@redhat.com> | 1 | No block driver implements .bdrv_co_io_plug() anymore. Get rid of the |
---|---|---|---|
2 | 2 | function pointers. | |
3 | We were pushing the context until right before running the gmainloop. | ||
4 | Now since we have everything unconditionally, we can move this | ||
5 | earlier. | ||
6 | |||
7 | One benefit is that now it's done even before init_done_sem, so as | ||
8 | long as the iothread user calls iothread_create() and completes, we | ||
9 | know that the thread stack is ready. | ||
10 | |||
11 | Signed-off-by: Peter Xu <peterx@redhat.com> | ||
12 | Message-id: 20190306115532.23025-5-peterx@redhat.com | ||
13 | Message-Id: <20190306115532.23025-5-peterx@redhat.com> | ||
14 | |||
15 | [Tweaked comment wording as discussed with Peter Xu. | ||
16 | --Stefan] | ||
17 | 3 | ||
18 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 4 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
5 | Reviewed-by: Eric Blake <eblake@redhat.com> | ||
6 | Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> | ||
7 | Acked-by: Kevin Wolf <kwolf@redhat.com> | ||
8 | Message-id: 20230530180959.1108766-7-stefanha@redhat.com | ||
9 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
19 | --- | 10 | --- |
20 | iothread.c | 9 ++++++--- | 11 | include/block/block-io.h | 3 --- |
21 | 1 file changed, 6 insertions(+), 3 deletions(-) | 12 | include/block/block_int-common.h | 11 ---------- |
13 | block/io.c | 37 -------------------------------- | ||
14 | 3 files changed, 51 deletions(-) | ||
22 | 15 | ||
23 | diff --git a/iothread.c b/iothread.c | 16 | diff --git a/include/block/block-io.h b/include/block/block-io.h |
24 | index XXXXXXX..XXXXXXX 100644 | 17 | index XXXXXXX..XXXXXXX 100644 |
25 | --- a/iothread.c | 18 | --- a/include/block/block-io.h |
26 | +++ b/iothread.c | 19 | +++ b/include/block/block-io.h |
27 | @@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque) | 20 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn bdrv_co_leave(BlockDriverState *bs, AioContext *old_ctx); |
28 | IOThread *iothread = opaque; | 21 | |
29 | 22 | AioContext *child_of_bds_get_parent_aio_context(BdrvChild *c); | |
30 | rcu_register_thread(); | 23 | |
24 | -void coroutine_fn GRAPH_RDLOCK bdrv_co_io_plug(BlockDriverState *bs); | ||
25 | -void coroutine_fn GRAPH_RDLOCK bdrv_co_io_unplug(BlockDriverState *bs); | ||
31 | - | 26 | - |
32 | + /* | 27 | bool coroutine_fn GRAPH_RDLOCK |
33 | + * g_main_context_push_thread_default() must be called before anything | 28 | bdrv_co_can_store_new_dirty_bitmap(BlockDriverState *bs, const char *name, |
34 | + * in this new thread uses glib. | 29 | uint32_t granularity, Error **errp); |
35 | + */ | 30 | diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h |
36 | + g_main_context_push_thread_default(iothread->worker_context); | 31 | index XXXXXXX..XXXXXXX 100644 |
37 | my_iothread = iothread; | 32 | --- a/include/block/block_int-common.h |
38 | iothread->thread_id = qemu_get_thread_id(); | 33 | +++ b/include/block/block_int-common.h |
39 | qemu_sem_post(&iothread->init_done_sem); | 34 | @@ -XXX,XX +XXX,XX @@ struct BlockDriver { |
40 | @@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque) | 35 | void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_debug_event)( |
41 | * changed in previous aio_poll() | 36 | BlockDriverState *bs, BlkdebugEvent event); |
42 | */ | 37 | |
43 | if (iothread->running && atomic_read(&iothread->run_gcontext)) { | 38 | - /* io queue for linux-aio */ |
44 | - g_main_context_push_thread_default(iothread->worker_context); | 39 | - void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_io_plug)(BlockDriverState *bs); |
45 | g_main_loop_run(iothread->main_loop); | 40 | - void coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_io_unplug)( |
46 | - g_main_context_pop_thread_default(iothread->worker_context); | 41 | - BlockDriverState *bs); |
47 | } | 42 | - |
48 | } | 43 | bool (*bdrv_supports_persistent_dirty_bitmap)(BlockDriverState *bs); |
49 | 44 | ||
50 | + g_main_context_pop_thread_default(iothread->worker_context); | 45 | bool coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_can_store_new_dirty_bitmap)( |
51 | rcu_unregister_thread(); | 46 | @@ -XXX,XX +XXX,XX @@ struct BlockDriverState { |
52 | return NULL; | 47 | unsigned int in_flight; |
48 | unsigned int serialising_in_flight; | ||
49 | |||
50 | - /* | ||
51 | - * counter for nested bdrv_io_plug. | ||
52 | - * Accessed with atomic ops. | ||
53 | - */ | ||
54 | - unsigned io_plugged; | ||
55 | - | ||
56 | /* do we need to tell the quest if we have a volatile write cache? */ | ||
57 | int enable_write_cache; | ||
58 | |||
59 | diff --git a/block/io.c b/block/io.c | ||
60 | index XXXXXXX..XXXXXXX 100644 | ||
61 | --- a/block/io.c | ||
62 | +++ b/block/io.c | ||
63 | @@ -XXX,XX +XXX,XX @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t size) | ||
64 | return mem; | ||
53 | } | 65 | } |
66 | |||
67 | -void coroutine_fn bdrv_co_io_plug(BlockDriverState *bs) | ||
68 | -{ | ||
69 | - BdrvChild *child; | ||
70 | - IO_CODE(); | ||
71 | - assert_bdrv_graph_readable(); | ||
72 | - | ||
73 | - QLIST_FOREACH(child, &bs->children, next) { | ||
74 | - bdrv_co_io_plug(child->bs); | ||
75 | - } | ||
76 | - | ||
77 | - if (qatomic_fetch_inc(&bs->io_plugged) == 0) { | ||
78 | - BlockDriver *drv = bs->drv; | ||
79 | - if (drv && drv->bdrv_co_io_plug) { | ||
80 | - drv->bdrv_co_io_plug(bs); | ||
81 | - } | ||
82 | - } | ||
83 | -} | ||
84 | - | ||
85 | -void coroutine_fn bdrv_co_io_unplug(BlockDriverState *bs) | ||
86 | -{ | ||
87 | - BdrvChild *child; | ||
88 | - IO_CODE(); | ||
89 | - assert_bdrv_graph_readable(); | ||
90 | - | ||
91 | - assert(bs->io_plugged); | ||
92 | - if (qatomic_fetch_dec(&bs->io_plugged) == 1) { | ||
93 | - BlockDriver *drv = bs->drv; | ||
94 | - if (drv && drv->bdrv_co_io_unplug) { | ||
95 | - drv->bdrv_co_io_unplug(bs); | ||
96 | - } | ||
97 | - } | ||
98 | - | ||
99 | - QLIST_FOREACH(child, &bs->children, next) { | ||
100 | - bdrv_co_io_unplug(child->bs); | ||
101 | - } | ||
102 | -} | ||
103 | - | ||
104 | /* Helper that undoes bdrv_register_buf() when it fails partway through */ | ||
105 | static void GRAPH_RDLOCK | ||
106 | bdrv_register_buf_rollback(BlockDriverState *bs, void *host, size_t size, | ||
54 | -- | 107 | -- |
55 | 2.20.1 | 108 | 2.40.1 |
56 | |||
57 | diff view generated by jsdifflib |
1 | From: Anastasiia Rusakova <arusakova917@gmail.com> | 1 | From: Stefano Garzarella <sgarzare@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Some functions sometimes uses req->dev even though a local variable | 3 | Some virtio-blk drivers (e.g. virtio-blk-vhost-vdpa) supports the fd |
4 | VirtIOBlock* s = req->dev has already been defined. | 4 | passing. Let's expose this to the user, so the management layer |
5 | Updated places to use s everywhere in the file. | 5 | can pass the file descriptor of an already opened path. |
6 | 6 | ||
7 | Signed-off-by: Anastasiia Rusakova <arusakova917@gmail.com> | 7 | If the libblkio virtio-blk driver supports fd passing, let's always |
8 | Message-id: 20190307161925.4158-1-rusakova.nastasia@icloud.com | 8 | use qemu_open() to open the `path`, so we can handle fd passing |
9 | Message-Id: <20190307161925.4158-1-rusakova.nastasia@icloud.com> | 9 | from the management layer through the "/dev/fdset/N" special path. |
10 | |||
11 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
12 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> | ||
13 | Message-id: 20230530071941.8954-2-sgarzare@redhat.com | ||
10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 14 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
11 | --- | 15 | --- |
12 | hw/block/virtio-blk.c | 16 +++++++++------- | 16 | block/blkio.c | 53 ++++++++++++++++++++++++++++++++++++++++++--------- |
13 | 1 file changed, 9 insertions(+), 7 deletions(-) | 17 | 1 file changed, 44 insertions(+), 9 deletions(-) |
14 | 18 | ||
15 | diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c | 19 | diff --git a/block/blkio.c b/block/blkio.c |
16 | index XXXXXXX..XXXXXXX 100644 | 20 | index XXXXXXX..XXXXXXX 100644 |
17 | --- a/hw/block/virtio-blk.c | 21 | --- a/block/blkio.c |
18 | +++ b/hw/block/virtio-blk.c | 22 | +++ b/block/blkio.c |
19 | @@ -XXX,XX +XXX,XX @@ static void virtio_blk_rw_complete(void *opaque, int ret) | 23 | @@ -XXX,XX +XXX,XX @@ static int blkio_virtio_blk_common_open(BlockDriverState *bs, |
20 | } | 24 | { |
21 | 25 | const char *path = qdict_get_try_str(options, "path"); | |
22 | if (ret) { | 26 | BDRVBlkioState *s = bs->opaque; |
23 | - int p = virtio_ldl_p(VIRTIO_DEVICE(req->dev), &req->out.type); | 27 | - int ret; |
24 | + int p = virtio_ldl_p(VIRTIO_DEVICE(s), &req->out.type); | 28 | + bool fd_supported = false; |
25 | bool is_read = !(p & VIRTIO_BLK_T_OUT); | 29 | + int fd, ret; |
26 | /* Note that memory may be dirtied on read failure. If the | 30 | |
27 | * virtio request is not completed here, as is the case for | 31 | if (!path) { |
28 | @@ -XXX,XX +XXX,XX @@ static void virtio_blk_rw_complete(void *opaque, int ret) | 32 | error_setg(errp, "missing 'path' option"); |
29 | } | 33 | return -EINVAL; |
30 | |||
31 | virtio_blk_req_complete(req, VIRTIO_BLK_S_OK); | ||
32 | - block_acct_done(blk_get_stats(req->dev->blk), &req->acct); | ||
33 | + block_acct_done(blk_get_stats(s->blk), &req->acct); | ||
34 | virtio_blk_free_request(req); | ||
35 | } | 34 | } |
36 | aio_context_release(blk_get_aio_context(s->conf.conf.blk)); | 35 | |
37 | @@ -XXX,XX +XXX,XX @@ static int virtio_blk_handle_scsi_req(VirtIOBlockReq *req) | 36 | - ret = blkio_set_str(s->blkio, "path", path); |
38 | { | 37 | - qdict_del(options, "path"); |
39 | int status = VIRTIO_BLK_S_OK; | 38 | - if (ret < 0) { |
40 | struct virtio_scsi_inhdr *scsi = NULL; | 39 | - error_setg_errno(errp, -ret, "failed to set path: %s", |
41 | - VirtIODevice *vdev = VIRTIO_DEVICE(req->dev); | 40 | - blkio_get_error_msg()); |
42 | - VirtQueueElement *elem = &req->elem; | 41 | - return ret; |
43 | VirtIOBlock *blk = req->dev; | 42 | - } |
44 | + VirtIODevice *vdev = VIRTIO_DEVICE(blk); | 43 | - |
45 | + VirtQueueElement *elem = &req->elem; | 44 | if (!(flags & BDRV_O_NOCACHE)) { |
46 | 45 | error_setg(errp, "cache.direct=off is not supported"); | |
47 | #ifdef __linux__ | 46 | return -EINVAL; |
48 | int i; | 47 | } |
49 | @@ -XXX,XX +XXX,XX @@ static void virtio_blk_submit_multireq(BlockBackend *blk, MultiReqBuffer *mrb) | ||
50 | |||
51 | static void virtio_blk_handle_flush(VirtIOBlockReq *req, MultiReqBuffer *mrb) | ||
52 | { | ||
53 | - block_acct_start(blk_get_stats(req->dev->blk), &req->acct, 0, | ||
54 | + VirtIOBlock *s = req->dev; | ||
55 | + | 48 | + |
56 | + block_acct_start(blk_get_stats(s->blk), &req->acct, 0, | 49 | + if (blkio_get_int(s->blkio, "fd", &fd) == 0) { |
57 | BLOCK_ACCT_FLUSH); | 50 | + fd_supported = true; |
58 | 51 | + } | |
59 | /* | 52 | + |
60 | * Make sure all outstanding writes are posted to the backing device. | 53 | + /* |
61 | */ | 54 | + * If the libblkio driver supports fd passing, let's always use qemu_open() |
62 | if (mrb->is_write && mrb->num_reqs > 0) { | 55 | + * to open the `path`, so we can handle fd passing from the management |
63 | - virtio_blk_submit_multireq(req->dev->blk, mrb); | 56 | + * layer through the "/dev/fdset/N" special path. |
64 | + virtio_blk_submit_multireq(s->blk, mrb); | 57 | + */ |
65 | } | 58 | + if (fd_supported) { |
66 | - blk_aio_flush(req->dev->blk, virtio_blk_flush_complete, req); | 59 | + int open_flags; |
67 | + blk_aio_flush(s->blk, virtio_blk_flush_complete, req); | 60 | + |
61 | + if (flags & BDRV_O_RDWR) { | ||
62 | + open_flags = O_RDWR; | ||
63 | + } else { | ||
64 | + open_flags = O_RDONLY; | ||
65 | + } | ||
66 | + | ||
67 | + fd = qemu_open(path, open_flags, errp); | ||
68 | + if (fd < 0) { | ||
69 | + return -EINVAL; | ||
70 | + } | ||
71 | + | ||
72 | + ret = blkio_set_int(s->blkio, "fd", fd); | ||
73 | + if (ret < 0) { | ||
74 | + error_setg_errno(errp, -ret, "failed to set fd: %s", | ||
75 | + blkio_get_error_msg()); | ||
76 | + qemu_close(fd); | ||
77 | + return ret; | ||
78 | + } | ||
79 | + } else { | ||
80 | + ret = blkio_set_str(s->blkio, "path", path); | ||
81 | + if (ret < 0) { | ||
82 | + error_setg_errno(errp, -ret, "failed to set path: %s", | ||
83 | + blkio_get_error_msg()); | ||
84 | + return ret; | ||
85 | + } | ||
86 | + } | ||
87 | + | ||
88 | + qdict_del(options, "path"); | ||
89 | + | ||
90 | return 0; | ||
68 | } | 91 | } |
69 | 92 | ||
70 | static bool virtio_blk_sect_range_ok(VirtIOBlock *dev, | ||
71 | -- | 93 | -- |
72 | 2.20.1 | 94 | 2.40.1 |
73 | |||
74 | diff view generated by jsdifflib |
1 | From: Peter Xu <peterx@redhat.com> | 1 | From: Stefano Garzarella <sgarzare@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | Since we've have the gcontext always there, create the main loop | 3 | The virtio-blk-vhost-vdpa driver in libblkio 1.3.0 supports the fd |
4 | altogether. The iothread_run() is even cleaner. | 4 | passing through the new 'fd' property. |
5 | 5 | ||
6 | Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> | 6 | Since now we are using qemu_open() on '@path' if the virtio-blk driver |
7 | Signed-off-by: Peter Xu <peterx@redhat.com> | 7 | supports the fd passing, let's announce it. |
8 | Message-id: 20190306115532.23025-4-peterx@redhat.com | 8 | In this way, the management layer can pass the file descriptor of an |
9 | Message-Id: <20190306115532.23025-4-peterx@redhat.com> | 9 | already opened vhost-vdpa character device. This is useful especially |
10 | when the device can only be accessed with certain privileges. | ||
11 | |||
12 | Add the '@fdset' feature only when the virtio-blk-vhost-vdpa driver | ||
13 | in libblkio supports it. | ||
14 | |||
15 | Suggested-by: Markus Armbruster <armbru@redhat.com> | ||
16 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
17 | Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> | ||
18 | Message-id: 20230530071941.8954-3-sgarzare@redhat.com | ||
10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 19 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
11 | --- | 20 | --- |
12 | iothread.c | 12 +++--------- | 21 | qapi/block-core.json | 6 ++++++ |
13 | 1 file changed, 3 insertions(+), 9 deletions(-) | 22 | meson.build | 4 ++++ |
23 | 2 files changed, 10 insertions(+) | ||
14 | 24 | ||
15 | diff --git a/iothread.c b/iothread.c | 25 | diff --git a/qapi/block-core.json b/qapi/block-core.json |
16 | index XXXXXXX..XXXXXXX 100644 | 26 | index XXXXXXX..XXXXXXX 100644 |
17 | --- a/iothread.c | 27 | --- a/qapi/block-core.json |
18 | +++ b/iothread.c | 28 | +++ b/qapi/block-core.json |
19 | @@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque) | 29 | @@ -XXX,XX +XXX,XX @@ |
20 | * changed in previous aio_poll() | 30 | # |
21 | */ | 31 | # @path: path to the vhost-vdpa character device. |
22 | if (iothread->running && atomic_read(&iothread->run_gcontext)) { | 32 | # |
23 | - GMainLoop *loop; | 33 | +# Features: |
24 | - | 34 | +# @fdset: Member @path supports the special "/dev/fdset/N" path |
25 | g_main_context_push_thread_default(iothread->worker_context); | 35 | +# (since 8.1) |
26 | - iothread->main_loop = | 36 | +# |
27 | - g_main_loop_new(iothread->worker_context, TRUE); | 37 | # Since: 7.2 |
28 | - loop = iothread->main_loop; | 38 | ## |
29 | - | 39 | { 'struct': 'BlockdevOptionsVirtioBlkVhostVdpa', |
30 | g_main_loop_run(iothread->main_loop); | 40 | 'data': { 'path': 'str' }, |
31 | - iothread->main_loop = NULL; | 41 | + 'features': [ { 'name' :'fdset', |
32 | - g_main_loop_unref(loop); | 42 | + 'if': 'CONFIG_BLKIO_VHOST_VDPA_FD' } ], |
33 | - | 43 | 'if': 'CONFIG_BLKIO' } |
34 | g_main_context_pop_thread_default(iothread->worker_context); | 44 | |
35 | } | 45 | ## |
36 | } | 46 | diff --git a/meson.build b/meson.build |
37 | @@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj) | 47 | index XXXXXXX..XXXXXXX 100644 |
38 | if (iothread->worker_context) { | 48 | --- a/meson.build |
39 | g_main_context_unref(iothread->worker_context); | 49 | +++ b/meson.build |
40 | iothread->worker_context = NULL; | 50 | @@ -XXX,XX +XXX,XX @@ config_host_data.set('CONFIG_LZO', lzo.found()) |
41 | + g_main_loop_unref(iothread->main_loop); | 51 | config_host_data.set('CONFIG_MPATH', mpathpersist.found()) |
42 | + iothread->main_loop = NULL; | 52 | config_host_data.set('CONFIG_MPATH_NEW_API', mpathpersist_new_api) |
43 | } | 53 | config_host_data.set('CONFIG_BLKIO', blkio.found()) |
44 | qemu_sem_destroy(&iothread->init_done_sem); | 54 | +if blkio.found() |
45 | } | 55 | + config_host_data.set('CONFIG_BLKIO_VHOST_VDPA_FD', |
46 | @@ -XXX,XX +XXX,XX @@ static void iothread_init_gcontext(IOThread *iothread) | 56 | + blkio.version().version_compare('>=1.3.0')) |
47 | source = aio_get_g_source(iothread_get_aio_context(iothread)); | 57 | +endif |
48 | g_source_attach(source, iothread->worker_context); | 58 | config_host_data.set('CONFIG_CURL', curl.found()) |
49 | g_source_unref(source); | 59 | config_host_data.set('CONFIG_CURSES', curses.found()) |
50 | + iothread->main_loop = g_main_loop_new(iothread->worker_context, TRUE); | 60 | config_host_data.set('CONFIG_GBM', gbm.found()) |
51 | } | ||
52 | |||
53 | static void iothread_complete(UserCreatable *obj, Error **errp) | ||
54 | -- | 61 | -- |
55 | 2.20.1 | 62 | 2.40.1 |
56 | |||
57 | diff view generated by jsdifflib |