1 | The following changes since commit d147f7e815f97cb477e223586bcb80c316ae10ea: | 1 | The following changes since commit 6c769690ac845fa62642a5f93b4e4bd906adab95: |
---|---|---|---|
2 | 2 | ||
3 | Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2017-10-03 16:27:24 +0100) | 3 | Merge remote-tracking branch 'remotes/vsementsov/tags/pull-simplebench-2021-05-04' into staging (2021-05-21 12:02:34 +0100) |
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 f708a5e71cba0d784e307334c07ade5f56f827ab: | 9 | for you to fetch changes up to 0a6f0c76a030710780ce10d6347a70f098024d21: |
10 | 10 | ||
11 | aio: fix assert when remove poll during destroy (2017-10-03 14:36:19 -0400) | 11 | coroutine-sleep: introduce qemu_co_sleep (2021-05-21 18:22:33 +0100) |
12 | |||
13 | ---------------------------------------------------------------- | ||
14 | Pull request | ||
15 | |||
16 | (Resent due to an email preparation mistake.) | ||
12 | 17 | ||
13 | ---------------------------------------------------------------- | 18 | ---------------------------------------------------------------- |
14 | 19 | ||
15 | ---------------------------------------------------------------- | 20 | Paolo Bonzini (6): |
21 | coroutine-sleep: use a stack-allocated timer | ||
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 | ||
16 | 28 | ||
17 | Peter Xu (4): | 29 | Philippe Mathieu-Daudé (1): |
18 | qom: provide root container for internal objs | 30 | bitops.h: Improve find_xxx_bit() documentation |
19 | iothread: provide helpers for internal use | ||
20 | iothread: export iothread_stop() | ||
21 | iothread: delay the context release to finalize | ||
22 | 31 | ||
23 | Stefan Hajnoczi (1): | 32 | Zenghui Yu (1): |
24 | aio: fix assert when remove poll during destroy | 33 | multi-process: Initialize variables declared with g_auto* |
25 | 34 | ||
26 | include/qom/object.h | 11 +++++++++++ | 35 | include/qemu/bitops.h | 15 ++++++-- |
27 | include/sysemu/iothread.h | 9 +++++++++ | 36 | include/qemu/coroutine.h | 27 ++++++++----- |
28 | iothread.c | 46 ++++++++++++++++++++++++++++++++++++---------- | 37 | block/block-copy.c | 10 ++--- |
29 | qom/object.c | 11 +++++++++++ | 38 | block/nbd.c | 14 +++---- |
30 | util/aio-posix.c | 9 ++++++++- | 39 | hw/remote/memory.c | 5 +-- |
31 | 5 files changed, 75 insertions(+), 11 deletions(-) | 40 | hw/remote/proxy.c | 3 +- |
41 | util/qemu-coroutine-sleep.c | 75 +++++++++++++++++++------------------ | ||
42 | 7 files changed, 79 insertions(+), 70 deletions(-) | ||
32 | 43 | ||
33 | -- | 44 | -- |
34 | 2.13.6 | 45 | 2.31.1 |
35 | 46 | ||
36 | diff view generated by jsdifflib |
New patch | |||
---|---|---|---|
1 | From: Zenghui Yu <yuzenghui@huawei.com> | ||
1 | 2 | ||
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 |
New patch | |||
---|---|---|---|
1 | From: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
1 | 2 | ||
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 |
New patch | |||
---|---|---|---|
1 | From: Paolo Bonzini <pbonzini@redhat.com> | ||
1 | 2 | ||
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 |
1 | From: Peter Xu <peterx@redhat.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | When gcontext is used with iothread, the context will be destroyed | 3 | Simplify the code by removing conditionals. qemu_co_sleep_ns |
4 | during iothread_stop(). That's not good since sometimes we would like | 4 | can simply point the argument to an on-stack temporary. |
5 | to keep the resources until iothread is destroyed, but we may want to | ||
6 | stop the thread before that point. | ||
7 | 5 | ||
8 | Delay the destruction of gcontext to iothread finalize. Then we can do: | 6 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
9 | 7 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | |
10 | iothread_stop(thread); | 8 | Message-id: 20210517100548.28806-3-pbonzini@redhat.com |
11 | some_cleanup_on_resources(); | ||
12 | iothread_destroy(thread); | ||
13 | |||
14 | We may need this patch if we want to run chardev IOs in iothreads and | ||
15 | hopefully clean them up correctly. For more specific information, | ||
16 | please see 2b316774f6 ("qemu-char: do not operate on sources from | ||
17 | finalize callbacks"). | ||
18 | |||
19 | Reviewed-by: Fam Zheng <famz@redhat.com> | ||
20 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
21 | Signed-off-by: Peter Xu <peterx@redhat.com> | ||
22 | Message-id: 20170928025958.1420-5-peterx@redhat.com | ||
23 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 9 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
24 | --- | 10 | --- |
25 | iothread.c | 6 ++++-- | 11 | include/qemu/coroutine.h | 5 +++-- |
26 | 1 file changed, 4 insertions(+), 2 deletions(-) | 12 | util/qemu-coroutine-sleep.c | 18 +++++------------- |
13 | 2 files changed, 8 insertions(+), 15 deletions(-) | ||
27 | 14 | ||
28 | diff --git a/iothread.c b/iothread.c | 15 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h |
29 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
30 | --- a/iothread.c | 17 | --- a/include/qemu/coroutine.h |
31 | +++ b/iothread.c | 18 | +++ b/include/qemu/coroutine.h |
32 | @@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque) | 19 | @@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleepState QemuCoSleepState; |
33 | g_main_loop_unref(loop); | 20 | |
34 | 21 | /** | |
35 | g_main_context_pop_thread_default(iothread->worker_context); | 22 | * Yield the coroutine for a given duration. During this yield, @sleep_state |
36 | - g_main_context_unref(iothread->worker_context); | 23 | - * (if not NULL) is set to an opaque pointer, which may be used for |
37 | - iothread->worker_context = NULL; | 24 | + * is set to an opaque pointer, which may be used for |
38 | } | 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, | ||
39 | } | 54 | } |
40 | 55 | ||
41 | @@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj) | 56 | aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state); |
42 | IOThread *iothread = IOTHREAD(obj); | 57 | - if (sleep_state) { |
43 | 58 | - *sleep_state = &state; | |
44 | iothread_stop(iothread); | 59 | - } |
45 | + if (iothread->worker_context) { | 60 | + *sleep_state = &state; |
46 | + g_main_context_unref(iothread->worker_context); | 61 | timer_mod(&state.ts, qemu_clock_get_ns(type) + ns); |
47 | + iothread->worker_context = NULL; | 62 | qemu_coroutine_yield(); |
48 | + } | 63 | - if (sleep_state) { |
49 | qemu_cond_destroy(&iothread->init_done_cond); | 64 | - /* |
50 | qemu_mutex_destroy(&iothread->init_done_lock); | 65 | - * Note that *sleep_state is cleared during qemu_co_sleep_wake |
51 | if (!iothread->ctx) { | 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 | } | ||
52 | -- | 74 | -- |
53 | 2.13.6 | 75 | 2.31.1 |
54 | 76 | ||
55 | diff view generated by jsdifflib |
1 | From: Peter Xu <peterx@redhat.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | So that internal iothread users can explicitly stop one iothread without | 3 | All callers of qemu_co_sleep_wake are checking whether they are passing |
4 | destroying it. | 4 | a NULL argument inside the pointer-to-pointer: do the check in |
5 | qemu_co_sleep_wake itself. | ||
5 | 6 | ||
6 | Since at it, fix iothread_stop() to allow it to be called multiple | 7 | As a side effect, qemu_co_sleep_wake can be called more than once and |
7 | times. Before this patch we may call iothread_stop() more than once on | 8 | it will only wake the coroutine once; after the first time, the argument |
8 | single iothread, while that may not be correct since qemu_thread_join() | 9 | will be set to NULL via *sleep_state->user_state_pointer. However, this |
9 | is not allowed to run twice. From manual of pthread_join(): | 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. | ||
10 | 12 | ||
11 | Joining with a thread that has previously been joined results in | 13 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
12 | undefined behavior. | 14 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
13 | 15 | Message-id: 20210517100548.28806-4-pbonzini@redhat.com | |
14 | Reviewed-by: Fam Zheng <famz@redhat.com> | ||
15 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
16 | Signed-off-by: Peter Xu <peterx@redhat.com> | ||
17 | Message-id: 20170928025958.1420-4-peterx@redhat.com | ||
18 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 16 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
19 | --- | 17 | --- |
20 | include/sysemu/iothread.h | 1 + | 18 | block/block-copy.c | 4 +--- |
21 | iothread.c | 24 ++++++++++++++++-------- | 19 | block/nbd.c | 8 ++------ |
22 | 2 files changed, 17 insertions(+), 8 deletions(-) | 20 | util/qemu-coroutine-sleep.c | 21 ++++++++++++--------- |
21 | 3 files changed, 15 insertions(+), 18 deletions(-) | ||
23 | 22 | ||
24 | diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h | 23 | diff --git a/block/block-copy.c b/block/block-copy.c |
25 | index XXXXXXX..XXXXXXX 100644 | 24 | index XXXXXXX..XXXXXXX 100644 |
26 | --- a/include/sysemu/iothread.h | 25 | --- a/block/block-copy.c |
27 | +++ b/include/sysemu/iothread.h | 26 | +++ b/block/block-copy.c |
28 | @@ -XXX,XX +XXX,XX @@ GMainContext *iothread_get_g_main_context(IOThread *iothread); | 27 | @@ -XXX,XX +XXX,XX @@ out: |
29 | * "query-iothreads". | 28 | |
30 | */ | 29 | void block_copy_kick(BlockCopyCallState *call_state) |
31 | IOThread *iothread_create(const char *id, Error **errp); | 30 | { |
32 | +void iothread_stop(IOThread *iothread); | 31 | - if (call_state->sleep_state) { |
33 | void iothread_destroy(IOThread *iothread); | 32 | - qemu_co_sleep_wake(call_state->sleep_state); |
34 | 33 | - } | |
35 | #endif /* IOTHREAD_H */ | 34 | + qemu_co_sleep_wake(call_state->sleep_state); |
36 | diff --git a/iothread.c b/iothread.c | 35 | } |
36 | |||
37 | /* | ||
38 | diff --git a/block/nbd.c b/block/nbd.c | ||
37 | index XXXXXXX..XXXXXXX 100644 | 39 | index XXXXXXX..XXXXXXX 100644 |
38 | --- a/iothread.c | 40 | --- a/block/nbd.c |
39 | +++ b/iothread.c | 41 | +++ b/block/nbd.c |
40 | @@ -XXX,XX +XXX,XX @@ static void *iothread_run(void *opaque) | 42 | @@ -XXX,XX +XXX,XX @@ static void coroutine_fn nbd_client_co_drain_begin(BlockDriverState *bs) |
41 | return NULL; | 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 | + } | ||
42 | } | 89 | } |
43 | 90 | ||
44 | -static int iothread_stop(Object *object, void *opaque) | 91 | static void co_sleep_cb(void *opaque) |
45 | +void iothread_stop(IOThread *iothread) | ||
46 | { | 92 | { |
47 | - IOThread *iothread; | 93 | - qemu_co_sleep_wake(opaque); |
48 | - | 94 | + QemuCoSleepState **sleep_state = opaque; |
49 | - iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD); | 95 | + qemu_co_sleep_wake(*sleep_state); |
50 | - if (!iothread || !iothread->ctx || iothread->stopping) { | 96 | } |
51 | - return 0; | 97 | |
52 | + if (!iothread->ctx || iothread->stopping) { | 98 | void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, |
53 | + return; | 99 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, |
100 | abort(); | ||
54 | } | 101 | } |
55 | iothread->stopping = true; | 102 | |
56 | aio_notify(iothread->ctx); | 103 | - aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state); |
57 | @@ -XXX,XX +XXX,XX @@ static int iothread_stop(Object *object, void *opaque) | 104 | + aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, sleep_state); |
58 | g_main_loop_quit(iothread->main_loop); | 105 | *sleep_state = &state; |
59 | } | 106 | timer_mod(&state.ts, qemu_clock_get_ns(type) + ns); |
60 | qemu_thread_join(&iothread->thread); | 107 | qemu_coroutine_yield(); |
61 | +} | ||
62 | + | ||
63 | +static int iothread_stop_iter(Object *object, void *opaque) | ||
64 | +{ | ||
65 | + IOThread *iothread; | ||
66 | + | ||
67 | + iothread = (IOThread *)object_dynamic_cast(object, TYPE_IOTHREAD); | ||
68 | + if (!iothread) { | ||
69 | + return 0; | ||
70 | + } | ||
71 | + iothread_stop(iothread); | ||
72 | return 0; | ||
73 | } | ||
74 | |||
75 | @@ -XXX,XX +XXX,XX @@ static void iothread_instance_finalize(Object *obj) | ||
76 | { | ||
77 | IOThread *iothread = IOTHREAD(obj); | ||
78 | |||
79 | - iothread_stop(obj, NULL); | ||
80 | + iothread_stop(iothread); | ||
81 | qemu_cond_destroy(&iothread->init_done_cond); | ||
82 | qemu_mutex_destroy(&iothread->init_done_lock); | ||
83 | if (!iothread->ctx) { | ||
84 | @@ -XXX,XX +XXX,XX @@ void iothread_stop_all(void) | ||
85 | aio_context_release(ctx); | ||
86 | } | ||
87 | |||
88 | - object_child_foreach(container, iothread_stop, NULL); | ||
89 | + object_child_foreach(container, iothread_stop_iter, NULL); | ||
90 | } | ||
91 | |||
92 | static gpointer iothread_g_main_context_init(gpointer opaque) | ||
93 | -- | 108 | -- |
94 | 2.13.6 | 109 | 2.31.1 |
95 | 110 | ||
96 | diff view generated by jsdifflib |
1 | After iothread is enabled internally inside QEMU with GMainContext, we | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | may encounter this warning when destroying the iothread: | ||
3 | 2 | ||
4 | (qemu-system-x86_64:19925): GLib-CRITICAL **: g_source_remove_poll: | 3 | This simplification is enabled by the previous patch. Now aio_co_wake |
5 | assertion '!SOURCE_DESTROYED (source)' failed | 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 | 6 | ||
7 | The problem is that g_source_remove_poll() does not allow to remove one | 7 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
8 | source from array if the source is detached from its owner | 8 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
9 | context. (peterx: which IMHO does not make much sense) | 9 | Message-id: 20210517100548.28806-5-pbonzini@redhat.com |
10 | |||
11 | Fix it on QEMU side by avoid calling g_source_remove_poll() if we know | ||
12 | the object is during destruction, and we won't leak anything after all | ||
13 | since the array will be gone soon cleanly even with that fd. | ||
14 | |||
15 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | ||
16 | Reviewed-by: Fam Zheng <famz@redhat.com> | ||
17 | Signed-off-by: Peter Xu <peterx@redhat.com> | ||
18 | Message-id: 20170928025958.1420-6-peterx@redhat.com | ||
19 | [peterx: write the commit message] | ||
20 | Signed-off-by: Peter Xu <peterx@redhat.com> | ||
21 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
22 | --- | 11 | --- |
23 | util/aio-posix.c | 9 ++++++++- | 12 | util/qemu-coroutine-sleep.c | 8 ++++---- |
24 | 1 file changed, 8 insertions(+), 1 deletion(-) | 13 | 1 file changed, 4 insertions(+), 4 deletions(-) |
25 | 14 | ||
26 | diff --git a/util/aio-posix.c b/util/aio-posix.c | 15 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c |
27 | index XXXXXXX..XXXXXXX 100644 | 16 | index XXXXXXX..XXXXXXX 100644 |
28 | --- a/util/aio-posix.c | 17 | --- a/util/qemu-coroutine-sleep.c |
29 | +++ b/util/aio-posix.c | 18 | +++ b/util/qemu-coroutine-sleep.c |
30 | @@ -XXX,XX +XXX,XX @@ void aio_set_fd_handler(AioContext *ctx, | 19 | @@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns"; |
31 | return; | 20 | |
32 | } | 21 | struct QemuCoSleepState { |
33 | 22 | Coroutine *co; | |
34 | - g_source_remove_poll(&ctx->source, &node->pfd); | 23 | - QEMUTimer ts; |
35 | + /* If the GSource is in the process of being destroyed then | 24 | QemuCoSleepState **user_state_pointer; |
36 | + * g_source_remove_poll() causes an assertion failure. Skip | 25 | }; |
37 | + * removal in that case, because glib cleans up its state during | 26 | |
38 | + * destruction anyway. | 27 | @@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) |
39 | + */ | 28 | |
40 | + if (!g_source_is_destroyed(&ctx->source)) { | 29 | assert(scheduled == qemu_co_sleep_ns__scheduled); |
41 | + g_source_remove_poll(&ctx->source, &node->pfd); | 30 | *sleep_state->user_state_pointer = NULL; |
42 | + } | 31 | - timer_del(&sleep_state->ts); |
43 | 32 | aio_co_wake(sleep_state->co); | |
44 | /* If the lock is held, just mark the node as deleted */ | 33 | } |
45 | if (qemu_lockcnt_count(&ctx->list_lock)) { | 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); | ||
46 | -- | 57 | -- |
47 | 2.13.6 | 58 | 2.31.1 |
48 | 59 | ||
49 | diff view generated by jsdifflib |
1 | From: Peter Xu <peterx@redhat.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | We have object_get_objects_root() to keep user created objects, however | 3 | Right now, users of qemu_co_sleep_ns_wakeable are simply passing |
4 | no place for objects that will be used internally. Create such a | 4 | a pointer to QemuCoSleepState by reference to the function. But |
5 | container for internal objects. | 5 | QemuCoSleepState really is just a Coroutine*; making the |
6 | 6 | content of the struct public is just as efficient and lets us | |
7 | CC: Andreas Färber <afaerber@suse.de> | 7 | skip the user_state_pointer indirection. |
8 | CC: Markus Armbruster <armbru@redhat.com> | 8 | |
9 | CC: Paolo Bonzini <pbonzini@redhat.com> | 9 | Since the usage is changed, take the occasion to rename the |
10 | Suggested-by: Daniel P. Berrange <berrange@redhat.com> | 10 | struct to QemuCoSleep. |
11 | Signed-off-by: Peter Xu <peterx@redhat.com> | 11 | |
12 | Reviewed-by: Fam Zheng <famz@redhat.com> | 12 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
13 | Message-id: 20170928025958.1420-2-peterx@redhat.com | 13 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
14 | Message-id: 20210517100548.28806-6-pbonzini@redhat.com | ||
14 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 15 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
15 | --- | 16 | --- |
16 | include/qom/object.h | 11 +++++++++++ | 17 | include/qemu/coroutine.h | 23 +++++++++++---------- |
17 | qom/object.c | 11 +++++++++++ | 18 | block/block-copy.c | 8 ++++---- |
18 | 2 files changed, 22 insertions(+) | 19 | block/nbd.c | 10 ++++----- |
19 | 20 | util/qemu-coroutine-sleep.c | 41 ++++++++++++++++--------------------- | |
20 | diff --git a/include/qom/object.h b/include/qom/object.h | 21 | 4 files changed, 39 insertions(+), 43 deletions(-) |
21 | index XXXXXXX..XXXXXXX 100644 | 22 | |
22 | --- a/include/qom/object.h | 23 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h |
23 | +++ b/include/qom/object.h | 24 | index XXXXXXX..XXXXXXX 100644 |
24 | @@ -XXX,XX +XXX,XX @@ Object *object_get_root(void); | 25 | --- a/include/qemu/coroutine.h |
25 | Object *object_get_objects_root(void); | 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; | ||
26 | 35 | ||
27 | /** | 36 | /** |
28 | + * object_get_internal_root: | 37 | - * Yield the coroutine for a given duration. During this yield, @sleep_state |
29 | + * | 38 | - * is set to an opaque pointer, which may be used for |
30 | + * Get the container object that holds internally used object | 39 | - * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the |
31 | + * instances. Any object which is put into this container must not be | 40 | - * timer fires. Don't save the obtained value to other variables and don't call |
32 | + * user visible, and it will not be exposed in the QOM tree. | 41 | - * qemu_co_sleep_wake from another aio context. |
33 | + * | 42 | + * Yield the coroutine for a given duration. Initializes @w so that, |
34 | + * Returns: the internal object container | 43 | + * during this yield, it can be passed to qemu_co_sleep_wake() to |
35 | + */ | 44 | + * terminate the sleep. |
36 | +Object *object_get_internal_root(void); | 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); | ||
37 | + | 50 | + |
38 | +/** | 51 | static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) |
39 | * object_get_canonical_path_component: | 52 | { |
40 | * | 53 | - QemuCoSleepState *unused = NULL; |
41 | * Returns: The final component in the object's canonical path. The canonical | 54 | - qemu_co_sleep_ns_wakeable(type, ns, &unused); |
42 | diff --git a/qom/object.c b/qom/object.c | 55 | + QemuCoSleep w = { 0 }; |
43 | index XXXXXXX..XXXXXXX 100644 | 56 | + qemu_co_sleep_ns_wakeable(&w, type, ns); |
44 | --- a/qom/object.c | 57 | } |
45 | +++ b/qom/object.c | 58 | |
46 | @@ -XXX,XX +XXX,XX @@ Object *object_get_objects_root(void) | 59 | /** |
47 | return container_get(object_get_root(), "/objects"); | 60 | @@ -XXX,XX +XXX,XX @@ static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) |
48 | } | 61 | * qemu_co_sleep_ns() and should be checked to be non-NULL before calling |
49 | 62 | * qemu_co_sleep_wake(). | |
50 | +Object *object_get_internal_root(void) | 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) | ||
51 | +{ | 154 | +{ |
52 | + static Object *internal_root; | 155 | Coroutine *co; |
53 | + | 156 | - QemuCoSleepState **user_state_pointer; |
54 | + if (!internal_root) { | 157 | -}; |
55 | + internal_root = object_new("container"); | 158 | |
56 | + } | 159 | -void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) |
57 | + | 160 | -{ |
58 | + return internal_root; | 161 | - if (sleep_state) { |
59 | +} | 162 | + co = w->to_wake; |
60 | + | 163 | + w->to_wake = NULL; |
61 | static void object_get_child_property(Object *obj, Visitor *v, | 164 | + if (co) { |
62 | const char *name, void *opaque, | 165 | /* Write of schedule protected by barrier write in aio_co_schedule */ |
63 | Error **errp) | 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 | } | ||
64 | -- | 222 | -- |
65 | 2.13.6 | 223 | 2.31.1 |
66 | 224 | ||
67 | diff view generated by jsdifflib |
1 | From: Peter Xu <peterx@redhat.com> | 1 | From: Paolo Bonzini <pbonzini@redhat.com> |
---|---|---|---|
2 | 2 | ||
3 | IOThread is a general framework that contains IO loop environment and a | 3 | Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake. |
4 | real thread behind. It's also good to be used internally inside qemu. | 4 | This makes the logic of qemu_co_sleep_ns_wakeable easy to understand. |
5 | Provide some helpers for it to create iothreads to be used internally. | ||
6 | 5 | ||
7 | Put all the internal used iothreads into the internal object container. | 6 | In the future we will introduce an API that can work even if the |
7 | sleep and wake happen from different threads. For now, initializing | ||
8 | w->to_wake after timer_mod is fine because the timer can only fire in | ||
9 | the same AioContext. | ||
8 | 10 | ||
9 | Reviewed-by: Fam Zheng <famz@redhat.com> | 11 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> |
10 | Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> | 12 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> |
11 | Signed-off-by: Peter Xu <peterx@redhat.com> | 13 | Message-id: 20210517100548.28806-7-pbonzini@redhat.com |
12 | Message-id: 20170928025958.1420-3-peterx@redhat.com | ||
13 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 14 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
14 | --- | 15 | --- |
15 | include/sysemu/iothread.h | 8 ++++++++ | 16 | include/qemu/coroutine.h | 5 +++++ |
16 | iothread.c | 16 ++++++++++++++++ | 17 | util/qemu-coroutine-sleep.c | 26 +++++++++++++++++++------- |
17 | 2 files changed, 24 insertions(+) | 18 | 2 files changed, 24 insertions(+), 7 deletions(-) |
18 | 19 | ||
19 | diff --git a/include/sysemu/iothread.h b/include/sysemu/iothread.h | 20 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h |
20 | index XXXXXXX..XXXXXXX 100644 | 21 | index XXXXXXX..XXXXXXX 100644 |
21 | --- a/include/sysemu/iothread.h | 22 | --- a/include/qemu/coroutine.h |
22 | +++ b/include/sysemu/iothread.h | 23 | +++ b/include/qemu/coroutine.h |
23 | @@ -XXX,XX +XXX,XX @@ AioContext *iothread_get_aio_context(IOThread *iothread); | 24 | @@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleep { |
24 | void iothread_stop_all(void); | 25 | void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, |
25 | GMainContext *iothread_get_g_main_context(IOThread *iothread); | 26 | QEMUClockType type, int64_t ns); |
26 | 27 | ||
27 | +/* | 28 | +/** |
28 | + * Helpers used to allocate iothreads for internal use. These | 29 | + * Yield the coroutine until the next call to qemu_co_sleep_wake. |
29 | + * iothreads will not be seen by monitor clients when query using | ||
30 | + * "query-iothreads". | ||
31 | + */ | 30 | + */ |
32 | +IOThread *iothread_create(const char *id, Error **errp); | 31 | +void coroutine_fn qemu_co_sleep(QemuCoSleep *w); |
33 | +void iothread_destroy(IOThread *iothread); | ||
34 | + | 32 | + |
35 | #endif /* IOTHREAD_H */ | 33 | static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) |
36 | diff --git a/iothread.c b/iothread.c | 34 | { |
35 | QemuCoSleep w = { 0 }; | ||
36 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | ||
37 | index XXXXXXX..XXXXXXX 100644 | 37 | index XXXXXXX..XXXXXXX 100644 |
38 | --- a/iothread.c | 38 | --- a/util/qemu-coroutine-sleep.c |
39 | +++ b/iothread.c | 39 | +++ b/util/qemu-coroutine-sleep.c |
40 | @@ -XXX,XX +XXX,XX @@ GMainContext *iothread_get_g_main_context(IOThread *iothread) | 40 | @@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque) |
41 | 41 | qemu_co_sleep_wake(w); | |
42 | return iothread->worker_context; | 42 | } |
43 | |||
44 | -void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | ||
45 | - QEMUClockType type, int64_t ns) | ||
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 | } | ||
56 | |||
57 | w->to_wake = co; | ||
58 | - aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w), | ||
59 | - timer_mod(&ts, qemu_clock_get_ns(type) + ns); | ||
60 | qemu_coroutine_yield(); | ||
61 | - timer_del(&ts); | ||
62 | |||
63 | /* w->to_wake is cleared before resuming this coroutine. */ | ||
64 | assert(w->to_wake == NULL); | ||
43 | } | 65 | } |
44 | + | 66 | + |
45 | +IOThread *iothread_create(const char *id, Error **errp) | 67 | +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, |
68 | + QEMUClockType type, int64_t ns) | ||
46 | +{ | 69 | +{ |
47 | + Object *obj; | 70 | + AioContext *ctx = qemu_get_current_aio_context(); |
71 | + QEMUTimer ts; | ||
48 | + | 72 | + |
49 | + obj = object_new_with_props(TYPE_IOTHREAD, | 73 | + aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w); |
50 | + object_get_internal_root(), | 74 | + timer_mod(&ts, qemu_clock_get_ns(type) + ns); |
51 | + id, errp, NULL); | ||
52 | + | 75 | + |
53 | + return IOTHREAD(obj); | 76 | + /* |
54 | +} | 77 | + * The timer will fire in the current AiOContext, so the callback |
55 | + | 78 | + * must happen after qemu_co_sleep yields and there is no race |
56 | +void iothread_destroy(IOThread *iothread) | 79 | + * between timer_mod and qemu_co_sleep. |
57 | +{ | 80 | + */ |
58 | + object_unparent(OBJECT(iothread)); | 81 | + qemu_co_sleep(w); |
82 | + timer_del(&ts); | ||
59 | +} | 83 | +} |
60 | -- | 84 | -- |
61 | 2.13.6 | 85 | 2.31.1 |
62 | 86 | ||
63 | diff view generated by jsdifflib |