1 | The following changes since commit 6c769690ac845fa62642a5f93b4e4bd906adab95: | 1 | The following changes since commit 887cba855bb6ff4775256f7968409281350b568c: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/vsementsov/tags/pull-simplebench-2021-05-04' into staging (2021-05-21 12:02:34 +0100) | 3 | configure: Fix cross-building for RISCV host (v5) (2023-07-11 17:56:09 +0100) |
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 0a6f0c76a030710780ce10d6347a70f098024d21: | 9 | for you to fetch changes up to 75dcb4d790bbe5327169fd72b185960ca58e2fa6: |
10 | 10 | ||
11 | coroutine-sleep: introduce qemu_co_sleep (2021-05-21 18:22:33 +0100) | 11 | virtio-blk: fix host notifier issues during dataplane start/stop (2023-07-12 15:20:32 -0400) |
12 | 12 | ||
13 | ---------------------------------------------------------------- | 13 | ---------------------------------------------------------------- |
14 | Pull request | 14 | Pull request |
15 | 15 | ||
16 | (Resent due to an email preparation mistake.) | ||
17 | |||
18 | ---------------------------------------------------------------- | 16 | ---------------------------------------------------------------- |
19 | 17 | ||
20 | Paolo Bonzini (6): | 18 | Stefan Hajnoczi (1): |
21 | coroutine-sleep: use a stack-allocated timer | 19 | virtio-blk: fix host notifier issues during dataplane start/stop |
22 | coroutine-sleep: disallow NULL QemuCoSleepState** argument | ||
23 | coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing | ||
24 | coroutine-sleep: move timer out of QemuCoSleepState | ||
25 | coroutine-sleep: replace QemuCoSleepState pointer with struct in the | ||
26 | API | ||
27 | coroutine-sleep: introduce qemu_co_sleep | ||
28 | 20 | ||
29 | Philippe Mathieu-Daudé (1): | 21 | hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++-------------- |
30 | bitops.h: Improve find_xxx_bit() documentation | 22 | 1 file changed, 38 insertions(+), 29 deletions(-) |
31 | |||
32 | Zenghui Yu (1): | ||
33 | multi-process: Initialize variables declared with g_auto* | ||
34 | |||
35 | include/qemu/bitops.h | 15 ++++++-- | ||
36 | include/qemu/coroutine.h | 27 ++++++++----- | ||
37 | block/block-copy.c | 10 ++--- | ||
38 | block/nbd.c | 14 +++---- | ||
39 | hw/remote/memory.c | 5 +-- | ||
40 | hw/remote/proxy.c | 3 +- | ||
41 | util/qemu-coroutine-sleep.c | 75 +++++++++++++++++++------------------ | ||
42 | 7 files changed, 79 insertions(+), 70 deletions(-) | ||
43 | 23 | ||
44 | -- | 24 | -- |
45 | 2.31.1 | 25 | 2.40.1 |
46 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Zenghui Yu <yuzenghui@huawei.com> | ||
2 | 1 | ||
3 | Quote docs/devel/style.rst (section "Automatic memory deallocation"): | ||
4 | |||
5 | * Variables declared with g_auto* MUST always be initialized, | ||
6 | otherwise the cleanup function will use uninitialized stack memory | ||
7 | |||
8 | Initialize @name properly to get rid of the compilation error (using | ||
9 | gcc-7.3.0 on CentOS): | ||
10 | |||
11 | ../hw/remote/proxy.c: In function 'pci_proxy_dev_realize': | ||
12 | /usr/include/glib-2.0/glib/glib-autocleanups.h:28:3: error: 'name' may be used uninitialized in this function [-Werror=maybe-uninitialized] | ||
13 | g_free (*pp); | ||
14 | ^~~~~~~~~~~~ | ||
15 | ../hw/remote/proxy.c:350:30: note: 'name' was declared here | ||
16 | g_autofree char *name; | ||
17 | ^~~~ | ||
18 | |||
19 | Signed-off-by: Zenghui Yu <yuzenghui@huawei.com> | ||
20 | Reviewed-by: Jagannathan Raman <jag.raman@oracle.com> | ||
21 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
22 | Reviewed-by: Miroslav Rezanina <mrezanin@redhat.com> | ||
23 | Message-id: 20210312112143.1369-1-yuzenghui@huawei.com | ||
24 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
25 | --- | ||
26 | hw/remote/memory.c | 5 ++--- | ||
27 | hw/remote/proxy.c | 3 +-- | ||
28 | 2 files changed, 3 insertions(+), 5 deletions(-) | ||
29 | |||
30 | diff --git a/hw/remote/memory.c b/hw/remote/memory.c | ||
31 | index XXXXXXX..XXXXXXX 100644 | ||
32 | --- a/hw/remote/memory.c | ||
33 | +++ b/hw/remote/memory.c | ||
34 | @@ -XXX,XX +XXX,XX @@ void remote_sysmem_reconfig(MPQemuMsg *msg, Error **errp) | ||
35 | |||
36 | remote_sysmem_reset(); | ||
37 | |||
38 | - for (region = 0; region < msg->num_fds; region++) { | ||
39 | - g_autofree char *name; | ||
40 | + for (region = 0; region < msg->num_fds; region++, suffix++) { | ||
41 | + g_autofree char *name = g_strdup_printf("remote-mem-%u", suffix); | ||
42 | subregion = g_new(MemoryRegion, 1); | ||
43 | - name = g_strdup_printf("remote-mem-%u", suffix++); | ||
44 | memory_region_init_ram_from_fd(subregion, NULL, | ||
45 | name, sysmem_info->sizes[region], | ||
46 | true, msg->fds[region], | ||
47 | diff --git a/hw/remote/proxy.c b/hw/remote/proxy.c | ||
48 | index XXXXXXX..XXXXXXX 100644 | ||
49 | --- a/hw/remote/proxy.c | ||
50 | +++ b/hw/remote/proxy.c | ||
51 | @@ -XXX,XX +XXX,XX @@ static void probe_pci_info(PCIDevice *dev, Error **errp) | ||
52 | PCI_BASE_ADDRESS_SPACE_IO : PCI_BASE_ADDRESS_SPACE_MEMORY; | ||
53 | |||
54 | if (size) { | ||
55 | - g_autofree char *name; | ||
56 | + g_autofree char *name = g_strdup_printf("bar-region-%d", i); | ||
57 | pdev->region[i].dev = pdev; | ||
58 | pdev->region[i].present = true; | ||
59 | if (type == PCI_BASE_ADDRESS_SPACE_MEMORY) { | ||
60 | pdev->region[i].memory = true; | ||
61 | } | ||
62 | - name = g_strdup_printf("bar-region-%d", i); | ||
63 | memory_region_init_io(&pdev->region[i].mr, OBJECT(pdev), | ||
64 | &proxy_mr_ops, &pdev->region[i], | ||
65 | name, size); | ||
66 | -- | ||
67 | 2.31.1 | ||
68 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
2 | 1 | ||
3 | Document the following functions return the bitmap size | ||
4 | if no matching bit is found: | ||
5 | |||
6 | - find_first_bit | ||
7 | - find_next_bit | ||
8 | - find_last_bit | ||
9 | - find_first_zero_bit | ||
10 | - find_next_zero_bit | ||
11 | |||
12 | Reviewed-by: Richard Henderson <richard.henderson@linaro.org> | ||
13 | Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
14 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
15 | Message-id: 20210510200758.2623154-2-philmd@redhat.com | ||
16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
17 | --- | ||
18 | include/qemu/bitops.h | 15 ++++++++++++--- | ||
19 | 1 file changed, 12 insertions(+), 3 deletions(-) | ||
20 | |||
21 | diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h | ||
22 | index XXXXXXX..XXXXXXX 100644 | ||
23 | --- a/include/qemu/bitops.h | ||
24 | +++ b/include/qemu/bitops.h | ||
25 | @@ -XXX,XX +XXX,XX @@ static inline int test_bit(long nr, const unsigned long *addr) | ||
26 | * @addr: The address to start the search at | ||
27 | * @size: The maximum size to search | ||
28 | * | ||
29 | - * Returns the bit number of the first set bit, or size. | ||
30 | + * Returns the bit number of the last set bit, | ||
31 | + * or @size if there is no set bit in the bitmap. | ||
32 | */ | ||
33 | unsigned long find_last_bit(const unsigned long *addr, | ||
34 | unsigned long size); | ||
35 | @@ -XXX,XX +XXX,XX @@ unsigned long find_last_bit(const unsigned long *addr, | ||
36 | * @addr: The address to base the search on | ||
37 | * @offset: The bitnumber to start searching at | ||
38 | * @size: The bitmap size in bits | ||
39 | + * | ||
40 | + * Returns the bit number of the next set bit, | ||
41 | + * or @size if there are no further set bits in the bitmap. | ||
42 | */ | ||
43 | unsigned long find_next_bit(const unsigned long *addr, | ||
44 | unsigned long size, | ||
45 | @@ -XXX,XX +XXX,XX @@ unsigned long find_next_bit(const unsigned long *addr, | ||
46 | * @addr: The address to base the search on | ||
47 | * @offset: The bitnumber to start searching at | ||
48 | * @size: The bitmap size in bits | ||
49 | + * | ||
50 | + * Returns the bit number of the next cleared bit, | ||
51 | + * or @size if there are no further clear bits in the bitmap. | ||
52 | */ | ||
53 | |||
54 | unsigned long find_next_zero_bit(const unsigned long *addr, | ||
55 | @@ -XXX,XX +XXX,XX @@ unsigned long find_next_zero_bit(const unsigned long *addr, | ||
56 | * @addr: The address to start the search at | ||
57 | * @size: The maximum size to search | ||
58 | * | ||
59 | - * Returns the bit number of the first set bit. | ||
60 | + * Returns the bit number of the first set bit, | ||
61 | + * or @size if there is no set bit in the bitmap. | ||
62 | */ | ||
63 | static inline unsigned long find_first_bit(const unsigned long *addr, | ||
64 | unsigned long size) | ||
65 | @@ -XXX,XX +XXX,XX @@ static inline unsigned long find_first_bit(const unsigned long *addr, | ||
66 | * @addr: The address to start the search at | ||
67 | * @size: The maximum size to search | ||
68 | * | ||
69 | - * Returns the bit number of the first cleared bit. | ||
70 | + * Returns the bit number of the first cleared bit, | ||
71 | + * or @size if there is no clear bit in the bitmap. | ||
72 | */ | ||
73 | static inline unsigned long find_first_zero_bit(const unsigned long *addr, | ||
74 | unsigned long size) | ||
75 | -- | ||
76 | 2.31.1 | ||
77 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
2 | 1 | ||
3 | The lifetime of the timer is well-known (it cannot outlive | ||
4 | qemu_co_sleep_ns_wakeable, because it's deleted by the time the | ||
5 | coroutine resumes), so it is not necessary to place it on the heap. | ||
6 | |||
7 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
8 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
9 | Message-id: 20210517100548.28806-2-pbonzini@redhat.com | ||
10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
11 | --- | ||
12 | util/qemu-coroutine-sleep.c | 9 ++++----- | ||
13 | 1 file changed, 4 insertions(+), 5 deletions(-) | ||
14 | |||
15 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/util/qemu-coroutine-sleep.c | ||
18 | +++ b/util/qemu-coroutine-sleep.c | ||
19 | @@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns"; | ||
20 | |||
21 | struct QemuCoSleepState { | ||
22 | Coroutine *co; | ||
23 | - QEMUTimer *ts; | ||
24 | + QEMUTimer ts; | ||
25 | QemuCoSleepState **user_state_pointer; | ||
26 | }; | ||
27 | |||
28 | @@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) | ||
29 | if (sleep_state->user_state_pointer) { | ||
30 | *sleep_state->user_state_pointer = NULL; | ||
31 | } | ||
32 | - timer_del(sleep_state->ts); | ||
33 | + timer_del(&sleep_state->ts); | ||
34 | aio_co_wake(sleep_state->co); | ||
35 | } | ||
36 | |||
37 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
38 | AioContext *ctx = qemu_get_current_aio_context(); | ||
39 | QemuCoSleepState state = { | ||
40 | .co = qemu_coroutine_self(), | ||
41 | - .ts = aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &state), | ||
42 | .user_state_pointer = sleep_state, | ||
43 | }; | ||
44 | |||
45 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
46 | abort(); | ||
47 | } | ||
48 | |||
49 | + aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state); | ||
50 | if (sleep_state) { | ||
51 | *sleep_state = &state; | ||
52 | } | ||
53 | - timer_mod(state.ts, qemu_clock_get_ns(type) + ns); | ||
54 | + timer_mod(&state.ts, qemu_clock_get_ns(type) + ns); | ||
55 | qemu_coroutine_yield(); | ||
56 | if (sleep_state) { | ||
57 | /* | ||
58 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
59 | */ | ||
60 | assert(*sleep_state == NULL); | ||
61 | } | ||
62 | - timer_free(state.ts); | ||
63 | } | ||
64 | -- | ||
65 | 2.31.1 | ||
66 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
2 | 1 | ||
3 | Simplify the code by removing conditionals. qemu_co_sleep_ns | ||
4 | can simply point the argument to an on-stack temporary. | ||
5 | |||
6 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
7 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
8 | Message-id: 20210517100548.28806-3-pbonzini@redhat.com | ||
9 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
10 | --- | ||
11 | include/qemu/coroutine.h | 5 +++-- | ||
12 | util/qemu-coroutine-sleep.c | 18 +++++------------- | ||
13 | 2 files changed, 8 insertions(+), 15 deletions(-) | ||
14 | |||
15 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/include/qemu/coroutine.h | ||
18 | +++ b/include/qemu/coroutine.h | ||
19 | @@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleepState QemuCoSleepState; | ||
20 | |||
21 | /** | ||
22 | * Yield the coroutine for a given duration. During this yield, @sleep_state | ||
23 | - * (if not NULL) is set to an opaque pointer, which may be used for | ||
24 | + * is set to an opaque pointer, which may be used for | ||
25 | * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the | ||
26 | * timer fires. Don't save the obtained value to other variables and don't call | ||
27 | * qemu_co_sleep_wake from another aio context. | ||
28 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
29 | QemuCoSleepState **sleep_state); | ||
30 | static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) | ||
31 | { | ||
32 | - qemu_co_sleep_ns_wakeable(type, ns, NULL); | ||
33 | + QemuCoSleepState *unused = NULL; | ||
34 | + qemu_co_sleep_ns_wakeable(type, ns, &unused); | ||
35 | } | ||
36 | |||
37 | /** | ||
38 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | ||
39 | index XXXXXXX..XXXXXXX 100644 | ||
40 | --- a/util/qemu-coroutine-sleep.c | ||
41 | +++ b/util/qemu-coroutine-sleep.c | ||
42 | @@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) | ||
43 | qemu_co_sleep_ns__scheduled, NULL); | ||
44 | |||
45 | assert(scheduled == qemu_co_sleep_ns__scheduled); | ||
46 | - if (sleep_state->user_state_pointer) { | ||
47 | - *sleep_state->user_state_pointer = NULL; | ||
48 | - } | ||
49 | + *sleep_state->user_state_pointer = NULL; | ||
50 | timer_del(&sleep_state->ts); | ||
51 | aio_co_wake(sleep_state->co); | ||
52 | } | ||
53 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
54 | } | ||
55 | |||
56 | aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state); | ||
57 | - if (sleep_state) { | ||
58 | - *sleep_state = &state; | ||
59 | - } | ||
60 | + *sleep_state = &state; | ||
61 | timer_mod(&state.ts, qemu_clock_get_ns(type) + ns); | ||
62 | qemu_coroutine_yield(); | ||
63 | - if (sleep_state) { | ||
64 | - /* | ||
65 | - * Note that *sleep_state is cleared during qemu_co_sleep_wake | ||
66 | - * before resuming this coroutine. | ||
67 | - */ | ||
68 | - assert(*sleep_state == NULL); | ||
69 | - } | ||
70 | + | ||
71 | + /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */ | ||
72 | + assert(*sleep_state == NULL); | ||
73 | } | ||
74 | -- | ||
75 | 2.31.1 | ||
76 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
2 | 1 | ||
3 | All callers of qemu_co_sleep_wake are checking whether they are passing | ||
4 | a NULL argument inside the pointer-to-pointer: do the check in | ||
5 | qemu_co_sleep_wake itself. | ||
6 | |||
7 | As a side effect, qemu_co_sleep_wake can be called more than once and | ||
8 | it will only wake the coroutine once; after the first time, the argument | ||
9 | will be set to NULL via *sleep_state->user_state_pointer. However, this | ||
10 | would not be safe unless co_sleep_cb keeps using the QemuCoSleepState* | ||
11 | directly, so make it go through the pointer-to-pointer instead. | ||
12 | |||
13 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
14 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
15 | Message-id: 20210517100548.28806-4-pbonzini@redhat.com | ||
16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
17 | --- | ||
18 | block/block-copy.c | 4 +--- | ||
19 | block/nbd.c | 8 ++------ | ||
20 | util/qemu-coroutine-sleep.c | 21 ++++++++++++--------- | ||
21 | 3 files changed, 15 insertions(+), 18 deletions(-) | ||
22 | |||
23 | diff --git a/block/block-copy.c b/block/block-copy.c | ||
24 | index XXXXXXX..XXXXXXX 100644 | ||
25 | --- a/block/block-copy.c | ||
26 | +++ b/block/block-copy.c | ||
27 | @@ -XXX,XX +XXX,XX @@ out: | ||
28 | |||
29 | void block_copy_kick(BlockCopyCallState *call_state) | ||
30 | { | ||
31 | - if (call_state->sleep_state) { | ||
32 | - qemu_co_sleep_wake(call_state->sleep_state); | ||
33 | - } | ||
34 | + qemu_co_sleep_wake(call_state->sleep_state); | ||
35 | } | ||
36 | |||
37 | /* | ||
38 | diff --git a/block/nbd.c b/block/nbd.c | ||
39 | index XXXXXXX..XXXXXXX 100644 | ||
40 | --- a/block/nbd.c | ||
41 | +++ b/block/nbd.c | ||
42 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs) | ||
43 | BDRVNBDState *s = (BDRVNBDState *)bs->opaque; | ||
44 | |||
45 | s->drained = true; | ||
46 | - if (s->connection_co_sleep_ns_state) { | ||
47 | - qemu_co_sleep_wake(s->connection_co_sleep_ns_state); | ||
48 | - } | ||
49 | + qemu_co_sleep_wake(s->connection_co_sleep_ns_state); | ||
50 | |||
51 | nbd_co_establish_connection_cancel(bs, false); | ||
52 | |||
53 | @@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs) | ||
54 | |||
55 | s->state = NBD_CLIENT_QUIT; | ||
56 | if (s->connection_co) { | ||
57 | - if (s->connection_co_sleep_ns_state) { | ||
58 | - qemu_co_sleep_wake(s->connection_co_sleep_ns_state); | ||
59 | - } | ||
60 | + qemu_co_sleep_wake(s->connection_co_sleep_ns_state); | ||
61 | nbd_co_establish_connection_cancel(bs, true); | ||
62 | } | ||
63 | if (qemu_in_coroutine()) { | ||
64 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | ||
65 | index XXXXXXX..XXXXXXX 100644 | ||
66 | --- a/util/qemu-coroutine-sleep.c | ||
67 | +++ b/util/qemu-coroutine-sleep.c | ||
68 | @@ -XXX,XX +XXX,XX @@ struct QemuCoSleepState { | ||
69 | |||
70 | void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) | ||
71 | { | ||
72 | - /* Write of schedule protected by barrier write in aio_co_schedule */ | ||
73 | - const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled, | ||
74 | - qemu_co_sleep_ns__scheduled, NULL); | ||
75 | + if (sleep_state) { | ||
76 | + /* Write of schedule protected by barrier write in aio_co_schedule */ | ||
77 | + const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled, | ||
78 | + qemu_co_sleep_ns__scheduled, NULL); | ||
79 | |||
80 | - assert(scheduled == qemu_co_sleep_ns__scheduled); | ||
81 | - *sleep_state->user_state_pointer = NULL; | ||
82 | - timer_del(&sleep_state->ts); | ||
83 | - aio_co_wake(sleep_state->co); | ||
84 | + assert(scheduled == qemu_co_sleep_ns__scheduled); | ||
85 | + *sleep_state->user_state_pointer = NULL; | ||
86 | + timer_del(&sleep_state->ts); | ||
87 | + aio_co_wake(sleep_state->co); | ||
88 | + } | ||
89 | } | ||
90 | |||
91 | static void co_sleep_cb(void *opaque) | ||
92 | { | ||
93 | - qemu_co_sleep_wake(opaque); | ||
94 | + QemuCoSleepState **sleep_state = opaque; | ||
95 | + qemu_co_sleep_wake(*sleep_state); | ||
96 | } | ||
97 | |||
98 | void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
99 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
100 | abort(); | ||
101 | } | ||
102 | |||
103 | - aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state); | ||
104 | + aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state); | ||
105 | *sleep_state = &state; | ||
106 | timer_mod(&state.ts, qemu_clock_get_ns(type) + ns); | ||
107 | qemu_coroutine_yield(); | ||
108 | -- | ||
109 | 2.31.1 | ||
110 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
2 | 1 | ||
3 | This simplification is enabled by the previous patch. Now aio_co_wake | ||
4 | will only be called once, therefore we do not care about a spurious | ||
5 | firing of the timer after a qemu_co_sleep_wake. | ||
6 | |||
7 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
8 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
9 | Message-id: 20210517100548.28806-5-pbonzini@redhat.com | ||
10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
11 | --- | ||
12 | util/qemu-coroutine-sleep.c | 8 ++++---- | ||
13 | 1 file changed, 4 insertions(+), 4 deletions(-) | ||
14 | |||
15 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | ||
16 | index XXXXXXX..XXXXXXX 100644 | ||
17 | --- a/util/qemu-coroutine-sleep.c | ||
18 | +++ b/util/qemu-coroutine-sleep.c | ||
19 | @@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns"; | ||
20 | |||
21 | struct QemuCoSleepState { | ||
22 | Coroutine *co; | ||
23 | - QEMUTimer ts; | ||
24 | QemuCoSleepState **user_state_pointer; | ||
25 | }; | ||
26 | |||
27 | @@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) | ||
28 | |||
29 | assert(scheduled == qemu_co_sleep_ns__scheduled); | ||
30 | *sleep_state->user_state_pointer = NULL; | ||
31 | - timer_del(&sleep_state->ts); | ||
32 | aio_co_wake(sleep_state->co); | ||
33 | } | ||
34 | } | ||
35 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
36 | QemuCoSleepState **sleep_state) | ||
37 | { | ||
38 | AioContext *ctx = qemu_get_current_aio_context(); | ||
39 | + QEMUTimer ts; | ||
40 | QemuCoSleepState state = { | ||
41 | .co = qemu_coroutine_self(), | ||
42 | .user_state_pointer = sleep_state, | ||
43 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
44 | abort(); | ||
45 | } | ||
46 | |||
47 | - aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state); | ||
48 | + aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state); | ||
49 | *sleep_state = &state; | ||
50 | - timer_mod(&state.ts, qemu_clock_get_ns(type) + ns); | ||
51 | + timer_mod(&ts, qemu_clock_get_ns(type) + ns); | ||
52 | qemu_coroutine_yield(); | ||
53 | + timer_del(&ts); | ||
54 | |||
55 | /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */ | ||
56 | assert(*sleep_state == NULL); | ||
57 | -- | ||
58 | 2.31.1 | ||
59 | diff view generated by jsdifflib |
Deleted patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
2 | 1 | ||
3 | Right now, users of qemu_co_sleep_ns_wakeable are simply passing | ||
4 | a pointer to QemuCoSleepState by reference to the function. But | ||
5 | QemuCoSleepState really is just a Coroutine*; making the | ||
6 | content of the struct public is just as efficient and lets us | ||
7 | skip the user_state_pointer indirection. | ||
8 | |||
9 | Since the usage is changed, take the occasion to rename the | ||
10 | struct to QemuCoSleep. | ||
11 | |||
12 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
13 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
14 | Message-id: 20210517100548.28806-6-pbonzini@redhat.com | ||
15 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
16 | --- | ||
17 | include/qemu/coroutine.h | 23 +++++++++++---------- | ||
18 | block/block-copy.c | 8 ++++---- | ||
19 | block/nbd.c | 10 ++++----- | ||
20 | util/qemu-coroutine-sleep.c | 41 ++++++++++++++++--------------------- | ||
21 | 4 files changed, 39 insertions(+), 43 deletions(-) | ||
22 | |||
23 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h | ||
24 | index XXXXXXX..XXXXXXX 100644 | ||
25 | --- a/include/qemu/coroutine.h | ||
26 | +++ b/include/qemu/coroutine.h | ||
27 | @@ -XXX,XX +XXX,XX @@ void qemu_co_rwlock_wrlock(CoRwlock *lock); | ||
28 | */ | ||
29 | void qemu_co_rwlock_unlock(CoRwlock *lock); | ||
30 | |||
31 | -typedef struct QemuCoSleepState QemuCoSleepState; | ||
32 | +typedef struct QemuCoSleep { | ||
33 | + Coroutine *to_wake; | ||
34 | +} QemuCoSleep; | ||
35 | |||
36 | /** | ||
37 | - * Yield the coroutine for a given duration. During this yield, @sleep_state | ||
38 | - * is set to an opaque pointer, which may be used for | ||
39 | - * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the | ||
40 | - * timer fires. Don't save the obtained value to other variables and don't call | ||
41 | - * qemu_co_sleep_wake from another aio context. | ||
42 | + * Yield the coroutine for a given duration. Initializes @w so that, | ||
43 | + * during this yield, it can be passed to qemu_co_sleep_wake() to | ||
44 | + * terminate the sleep. | ||
45 | */ | ||
46 | -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
47 | - QemuCoSleepState **sleep_state); | ||
48 | +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | ||
49 | + QEMUClockType type, int64_t ns); | ||
50 | + | ||
51 | static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) | ||
52 | { | ||
53 | - QemuCoSleepState *unused = NULL; | ||
54 | - qemu_co_sleep_ns_wakeable(type, ns, &unused); | ||
55 | + QemuCoSleep w = { 0 }; | ||
56 | + qemu_co_sleep_ns_wakeable(&w, type, ns); | ||
57 | } | ||
58 | |||
59 | /** | ||
60 | @@ -XXX,XX +XXX,XX @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) | ||
61 | * qemu_co_sleep_ns() and should be checked to be non-NULL before calling | ||
62 | * qemu_co_sleep_wake(). | ||
63 | */ | ||
64 | -void qemu_co_sleep_wake(QemuCoSleepState *sleep_state); | ||
65 | +void qemu_co_sleep_wake(QemuCoSleep *w); | ||
66 | |||
67 | /** | ||
68 | * Yield until a file descriptor becomes readable | ||
69 | diff --git a/block/block-copy.c b/block/block-copy.c | ||
70 | index XXXXXXX..XXXXXXX 100644 | ||
71 | --- a/block/block-copy.c | ||
72 | +++ b/block/block-copy.c | ||
73 | @@ -XXX,XX +XXX,XX @@ typedef struct BlockCopyCallState { | ||
74 | /* State */ | ||
75 | int ret; | ||
76 | bool finished; | ||
77 | - QemuCoSleepState *sleep_state; | ||
78 | + QemuCoSleep sleep; | ||
79 | bool cancelled; | ||
80 | |||
81 | /* OUT parameters */ | ||
82 | @@ -XXX,XX +XXX,XX @@ block_copy_dirty_clusters(BlockCopyCallState *call_state) | ||
83 | if (ns > 0) { | ||
84 | block_copy_task_end(task, -EAGAIN); | ||
85 | g_free(task); | ||
86 | - qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, ns, | ||
87 | - &call_state->sleep_state); | ||
88 | + qemu_co_sleep_ns_wakeable(&call_state->sleep, | ||
89 | + QEMU_CLOCK_REALTIME, ns); | ||
90 | continue; | ||
91 | } | ||
92 | } | ||
93 | @@ -XXX,XX +XXX,XX @@ out: | ||
94 | |||
95 | void block_copy_kick(BlockCopyCallState *call_state) | ||
96 | { | ||
97 | - qemu_co_sleep_wake(call_state->sleep_state); | ||
98 | + qemu_co_sleep_wake(&call_state->sleep); | ||
99 | } | ||
100 | |||
101 | /* | ||
102 | diff --git a/block/nbd.c b/block/nbd.c | ||
103 | index XXXXXXX..XXXXXXX 100644 | ||
104 | --- a/block/nbd.c | ||
105 | +++ b/block/nbd.c | ||
106 | @@ -XXX,XX +XXX,XX @@ typedef struct BDRVNBDState { | ||
107 | CoQueue free_sema; | ||
108 | Coroutine *connection_co; | ||
109 | Coroutine *teardown_co; | ||
110 | - QemuCoSleepState *connection_co_sleep_ns_state; | ||
111 | + QemuCoSleep reconnect_sleep; | ||
112 | bool drained; | ||
113 | bool wait_drained_end; | ||
114 | int in_flight; | ||
115 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs) | ||
116 | BDRVNBDState *s = (BDRVNBDState *)bs->opaque; | ||
117 | |||
118 | s->drained = true; | ||
119 | - qemu_co_sleep_wake(s->connection_co_sleep_ns_state); | ||
120 | + qemu_co_sleep_wake(&s->reconnect_sleep); | ||
121 | |||
122 | nbd_co_establish_connection_cancel(bs, false); | ||
123 | |||
124 | @@ -XXX,XX +XXX,XX @@ static void nbd_teardown_connection(BlockDriverState *bs) | ||
125 | |||
126 | s->state = NBD_CLIENT_QUIT; | ||
127 | if (s->connection_co) { | ||
128 | - qemu_co_sleep_wake(s->connection_co_sleep_ns_state); | ||
129 | + qemu_co_sleep_wake(&s->reconnect_sleep); | ||
130 | nbd_co_establish_connection_cancel(bs, true); | ||
131 | } | ||
132 | if (qemu_in_coroutine()) { | ||
133 | @@ -XXX,XX +XXX,XX @@ static coroutine_fn void nbd_co_reconnect_loop(BDRVNBDState *s) | ||
134 | } | ||
135 | bdrv_inc_in_flight(s->bs); | ||
136 | } else { | ||
137 | - qemu_co_sleep_ns_wakeable(QEMU_CLOCK_REALTIME, timeout, | ||
138 | - &s->connection_co_sleep_ns_state); | ||
139 | + qemu_co_sleep_ns_wakeable(&s->reconnect_sleep, | ||
140 | + QEMU_CLOCK_REALTIME, timeout); | ||
141 | if (s->drained) { | ||
142 | continue; | ||
143 | } | ||
144 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | ||
145 | index XXXXXXX..XXXXXXX 100644 | ||
146 | --- a/util/qemu-coroutine-sleep.c | ||
147 | +++ b/util/qemu-coroutine-sleep.c | ||
148 | @@ -XXX,XX +XXX,XX @@ | ||
149 | |||
150 | static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns"; | ||
151 | |||
152 | -struct QemuCoSleepState { | ||
153 | +void qemu_co_sleep_wake(QemuCoSleep *w) | ||
154 | +{ | ||
155 | Coroutine *co; | ||
156 | - QemuCoSleepState **user_state_pointer; | ||
157 | -}; | ||
158 | |||
159 | -void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) | ||
160 | -{ | ||
161 | - if (sleep_state) { | ||
162 | + co = w->to_wake; | ||
163 | + w->to_wake = NULL; | ||
164 | + if (co) { | ||
165 | /* Write of schedule protected by barrier write in aio_co_schedule */ | ||
166 | - const char *scheduled = qatomic_cmpxchg(&sleep_state->co->scheduled, | ||
167 | + const char *scheduled = qatomic_cmpxchg(&co->scheduled, | ||
168 | qemu_co_sleep_ns__scheduled, NULL); | ||
169 | |||
170 | assert(scheduled == qemu_co_sleep_ns__scheduled); | ||
171 | - *sleep_state->user_state_pointer = NULL; | ||
172 | - aio_co_wake(sleep_state->co); | ||
173 | + aio_co_wake(co); | ||
174 | } | ||
175 | } | ||
176 | |||
177 | static void co_sleep_cb(void *opaque) | ||
178 | { | ||
179 | - QemuCoSleepState **sleep_state = opaque; | ||
180 | - qemu_co_sleep_wake(*sleep_state); | ||
181 | + QemuCoSleep *w = opaque; | ||
182 | + qemu_co_sleep_wake(w); | ||
183 | } | ||
184 | |||
185 | -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
186 | - QemuCoSleepState **sleep_state) | ||
187 | +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | ||
188 | + QEMUClockType type, int64_t ns) | ||
189 | { | ||
190 | + Coroutine *co = qemu_coroutine_self(); | ||
191 | AioContext *ctx = qemu_get_current_aio_context(); | ||
192 | QEMUTimer ts; | ||
193 | - QemuCoSleepState state = { | ||
194 | - .co = qemu_coroutine_self(), | ||
195 | - .user_state_pointer = sleep_state, | ||
196 | - }; | ||
197 | |||
198 | - const char *scheduled = qatomic_cmpxchg(&state.co->scheduled, NULL, | ||
199 | - qemu_co_sleep_ns__scheduled); | ||
200 | + const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL, | ||
201 | + qemu_co_sleep_ns__scheduled); | ||
202 | if (scheduled) { | ||
203 | fprintf(stderr, | ||
204 | "%s: Co-routine was already scheduled in '%s'\n", | ||
205 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | ||
206 | abort(); | ||
207 | } | ||
208 | |||
209 | - aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state); | ||
210 | - *sleep_state = &state; | ||
211 | + w->to_wake = co; | ||
212 | + aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w), | ||
213 | timer_mod(&ts, qemu_clock_get_ns(type) + ns); | ||
214 | qemu_coroutine_yield(); | ||
215 | timer_del(&ts); | ||
216 | |||
217 | - /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */ | ||
218 | - assert(*sleep_state == NULL); | ||
219 | + /* w->to_wake is cleared before resuming this coroutine. */ | ||
220 | + assert(w->to_wake == NULL); | ||
221 | } | ||
222 | -- | ||
223 | 2.31.1 | ||
224 | diff view generated by jsdifflib |
1 | From: Paolo Bonzini <pbonzini@redhat.com> | 1 | The main loop thread can consume 100% CPU when using --device |
---|---|---|---|
2 | virtio-blk-pci,iothread=<iothread>. ppoll() constantly returns but | ||
3 | reading virtqueue host notifiers fails with EAGAIN. The file descriptors | ||
4 | are stale and remain registered with the AioContext because of bugs in | ||
5 | the virtio-blk dataplane start/stop code. | ||
2 | 6 | ||
3 | Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake. | 7 | The problem is that the dataplane start/stop code involves drain |
4 | This makes the logic of qemu_co_sleep_ns_wakeable easy to understand. | 8 | operations, which call virtio_blk_drained_begin() and |
9 | virtio_blk_drained_end() at points where the host notifier is not | ||
10 | operational: | ||
11 | - In virtio_blk_data_plane_start(), blk_set_aio_context() drains after | ||
12 | vblk->dataplane_started has been set to true but the host notifier has | ||
13 | not been attached yet. | ||
14 | - In virtio_blk_data_plane_stop(), blk_drain() and blk_set_aio_context() | ||
15 | drain after the host notifier has already been detached but with | ||
16 | vblk->dataplane_started still set to true. | ||
5 | 17 | ||
6 | In the future we will introduce an API that can work even if the | 18 | I would like to simplify ->ioeventfd_start/stop() to avoid interactions |
7 | sleep and wake happen from different threads. For now, initializing | 19 | with drain entirely, but couldn't find a way to do that. Instead, this |
8 | w->to_wake after timer_mod is fine because the timer can only fire in | 20 | patch accepts the fragile nature of the code and reorders it so that |
9 | the same AioContext. | 21 | vblk->dataplane_started is false during drain operations. This way the |
22 | virtio_blk_drained_begin() and virtio_blk_drained_end() calls don't | ||
23 | touch the host notifier. The result is that | ||
24 | virtio_blk_data_plane_start() and virtio_blk_data_plane_stop() have | ||
25 | complete control over the host notifier and stale file descriptors are | ||
26 | no longer left in the AioContext. | ||
10 | 27 | ||
11 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 28 | This patch fixes the 100% CPU consumption in the main loop thread and |
12 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | 29 | correctly moves host notifier processing to the IOThread. |
13 | Message-id: 20210517100548.28806-7-pbonzini@redhat.com | 30 | |
31 | Fixes: 1665d9326fd2 ("virtio-blk: implement BlockDevOps->drained_begin()") | ||
32 | Reported-by: Lukáš Doktor <ldoktor@redhat.com> | ||
33 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
34 | Tested-by: Lukas Doktor <ldoktor@redhat.com> | ||
35 | Message-id: 20230704151527.193586-1-stefanha@redhat.com | ||
14 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 36 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
15 | --- | 37 | --- |
16 | include/qemu/coroutine.h | 5 +++++ | 38 | hw/block/dataplane/virtio-blk.c | 67 +++++++++++++++++++-------------- |
17 | util/qemu-coroutine-sleep.c | 26 +++++++++++++++++++------- | 39 | 1 file changed, 38 insertions(+), 29 deletions(-) |
18 | 2 files changed, 24 insertions(+), 7 deletions(-) | ||
19 | 40 | ||
20 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h | 41 | diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c |
21 | index XXXXXXX..XXXXXXX 100644 | 42 | index XXXXXXX..XXXXXXX 100644 |
22 | --- a/include/qemu/coroutine.h | 43 | --- a/hw/block/dataplane/virtio-blk.c |
23 | +++ b/include/qemu/coroutine.h | 44 | +++ b/hw/block/dataplane/virtio-blk.c |
24 | @@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleep { | 45 | @@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev) |
25 | void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | 46 | |
26 | QEMUClockType type, int64_t ns); | 47 | memory_region_transaction_commit(); |
27 | 48 | ||
28 | +/** | 49 | - /* |
29 | + * Yield the coroutine until the next call to qemu_co_sleep_wake. | 50 | - * These fields are visible to the IOThread so we rely on implicit barriers |
30 | + */ | 51 | - * in aio_context_acquire() on the write side and aio_notify_accept() on |
31 | +void coroutine_fn qemu_co_sleep(QemuCoSleep *w); | 52 | - * the read side. |
53 | - */ | ||
54 | - s->starting = false; | ||
55 | - vblk->dataplane_started = true; | ||
56 | trace_virtio_blk_data_plane_start(s); | ||
57 | |||
58 | old_context = blk_get_aio_context(s->conf->conf.blk); | ||
59 | @@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev) | ||
60 | event_notifier_set(virtio_queue_get_host_notifier(vq)); | ||
61 | } | ||
62 | |||
63 | + /* | ||
64 | + * These fields must be visible to the IOThread when it processes the | ||
65 | + * virtqueue, otherwise it will think dataplane has not started yet. | ||
66 | + * | ||
67 | + * Make sure ->dataplane_started is false when blk_set_aio_context() is | ||
68 | + * called above so that draining does not cause the host notifier to be | ||
69 | + * detached/attached prematurely. | ||
70 | + */ | ||
71 | + s->starting = false; | ||
72 | + vblk->dataplane_started = true; | ||
73 | + smp_wmb(); /* paired with aio_notify_accept() on the read side */ | ||
32 | + | 74 | + |
33 | static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) | 75 | /* Get this show started by hooking up our callbacks */ |
34 | { | 76 | if (!blk_in_drain(s->conf->conf.blk)) { |
35 | QemuCoSleep w = { 0 }; | 77 | aio_context_acquire(s->ctx); |
36 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | 78 | @@ -XXX,XX +XXX,XX @@ int virtio_blk_data_plane_start(VirtIODevice *vdev) |
37 | index XXXXXXX..XXXXXXX 100644 | 79 | fail_guest_notifiers: |
38 | --- a/util/qemu-coroutine-sleep.c | 80 | vblk->dataplane_disabled = true; |
39 | +++ b/util/qemu-coroutine-sleep.c | 81 | s->starting = false; |
40 | @@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque) | 82 | - vblk->dataplane_started = true; |
41 | qemu_co_sleep_wake(w); | 83 | return -ENOSYS; |
42 | } | 84 | } |
43 | 85 | ||
44 | -void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | 86 | @@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev) |
45 | - QEMUClockType type, int64_t ns) | 87 | aio_wait_bh_oneshot(s->ctx, virtio_blk_data_plane_stop_bh, s); |
46 | +void coroutine_fn qemu_co_sleep(QemuCoSleep *w) | ||
47 | { | ||
48 | Coroutine *co = qemu_coroutine_self(); | ||
49 | - AioContext *ctx = qemu_get_current_aio_context(); | ||
50 | - QEMUTimer ts; | ||
51 | |||
52 | const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL, | ||
53 | qemu_co_sleep_ns__scheduled); | ||
54 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | ||
55 | } | 88 | } |
56 | 89 | ||
57 | w->to_wake = co; | 90 | + /* |
58 | - aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w), | 91 | + * Batch all the host notifiers in a single transaction to avoid |
59 | - timer_mod(&ts, qemu_clock_get_ns(type) + ns); | 92 | + * quadratic time complexity in address_space_update_ioeventfds(). |
60 | qemu_coroutine_yield(); | 93 | + */ |
61 | - timer_del(&ts); | 94 | + memory_region_transaction_begin(); |
62 | |||
63 | /* w->to_wake is cleared before resuming this coroutine. */ | ||
64 | assert(w->to_wake == NULL); | ||
65 | } | ||
66 | + | 95 | + |
67 | +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | 96 | + for (i = 0; i < nvqs; i++) { |
68 | + QEMUClockType type, int64_t ns) | 97 | + virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false); |
69 | +{ | 98 | + } |
70 | + AioContext *ctx = qemu_get_current_aio_context(); | ||
71 | + QEMUTimer ts; | ||
72 | + | ||
73 | + aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w); | ||
74 | + timer_mod(&ts, qemu_clock_get_ns(type) + ns); | ||
75 | + | 99 | + |
76 | + /* | 100 | + /* |
77 | + * The timer will fire in the current AiOContext, so the callback | 101 | + * The transaction expects the ioeventfds to be open when it |
78 | + * must happen after qemu_co_sleep yields and there is no race | 102 | + * commits. Do it now, before the cleanup loop. |
79 | + * between timer_mod and qemu_co_sleep. | ||
80 | + */ | 103 | + */ |
81 | + qemu_co_sleep(w); | 104 | + memory_region_transaction_commit(); |
82 | + timer_del(&ts); | 105 | + |
83 | +} | 106 | + for (i = 0; i < nvqs; i++) { |
107 | + virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i); | ||
108 | + } | ||
109 | + | ||
110 | + /* | ||
111 | + * Set ->dataplane_started to false before draining so that host notifiers | ||
112 | + * are not detached/attached anymore. | ||
113 | + */ | ||
114 | + vblk->dataplane_started = false; | ||
115 | + | ||
116 | aio_context_acquire(s->ctx); | ||
117 | |||
118 | /* Wait for virtio_blk_dma_restart_bh() and in flight I/O to complete */ | ||
119 | @@ -XXX,XX +XXX,XX @@ void virtio_blk_data_plane_stop(VirtIODevice *vdev) | ||
120 | |||
121 | aio_context_release(s->ctx); | ||
122 | |||
123 | - /* | ||
124 | - * Batch all the host notifiers in a single transaction to avoid | ||
125 | - * quadratic time complexity in address_space_update_ioeventfds(). | ||
126 | - */ | ||
127 | - memory_region_transaction_begin(); | ||
128 | - | ||
129 | - for (i = 0; i < nvqs; i++) { | ||
130 | - virtio_bus_set_host_notifier(VIRTIO_BUS(qbus), i, false); | ||
131 | - } | ||
132 | - | ||
133 | - /* | ||
134 | - * The transaction expects the ioeventfds to be open when it | ||
135 | - * commits. Do it now, before the cleanup loop. | ||
136 | - */ | ||
137 | - memory_region_transaction_commit(); | ||
138 | - | ||
139 | - for (i = 0; i < nvqs; i++) { | ||
140 | - virtio_bus_cleanup_host_notifier(VIRTIO_BUS(qbus), i); | ||
141 | - } | ||
142 | - | ||
143 | qemu_bh_cancel(s->bh); | ||
144 | notify_guest_bh(s); /* final chance to notify guest */ | ||
145 | |||
146 | /* Clean up guest notifier (irq) */ | ||
147 | k->set_guest_notifiers(qbus->parent, nvqs, false); | ||
148 | |||
149 | - vblk->dataplane_started = false; | ||
150 | s->stopping = false; | ||
151 | } | ||
84 | -- | 152 | -- |
85 | 2.31.1 | 153 | 2.40.1 |
86 | 154 | ||
155 | diff view generated by jsdifflib |