1 | The following changes since commit 6c769690ac845fa62642a5f93b4e4bd906adab95: | 1 | The following changes since commit 711c0418c8c1ce3a24346f058b001c4c5a2f0f81: |
---|---|---|---|
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 | Merge remote-tracking branch 'remotes/philmd/tags/mips-20210702' into staging (2021-07-04 14:04:12 +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 9f460c64e13897117f35ffb61f6f5e0102cabc70: |
10 | 10 | ||
11 | coroutine-sleep: introduce qemu_co_sleep (2021-05-21 18:22:33 +0100) | 11 | block/io: Merge discard request alignments (2021-07-06 14:28:55 +0100) |
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 | Akihiko Odaki (3): |
21 | coroutine-sleep: use a stack-allocated timer | 19 | block/file-posix: Optimize for macOS |
22 | coroutine-sleep: disallow NULL QemuCoSleepState** argument | 20 | block: Add backend_defaults property |
23 | coroutine-sleep: allow qemu_co_sleep_wake that wakes nothing | 21 | block/io: Merge discard request alignments |
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 | 22 | ||
29 | Philippe Mathieu-Daudé (1): | 23 | Stefan Hajnoczi (2): |
30 | bitops.h: Improve find_xxx_bit() documentation | 24 | util/async: add a human-readable name to BHs for debugging |
25 | util/async: print leaked BH name when AioContext finalizes | ||
31 | 26 | ||
32 | Zenghui Yu (1): | 27 | include/block/aio.h | 31 ++++++++++++++++++++++--- |
33 | multi-process: Initialize variables declared with g_auto* | 28 | include/hw/block/block.h | 3 +++ |
34 | 29 | include/qemu/main-loop.h | 4 +++- | |
35 | include/qemu/bitops.h | 15 ++++++-- | 30 | block/file-posix.c | 27 ++++++++++++++++++++-- |
36 | include/qemu/coroutine.h | 27 ++++++++----- | 31 | block/io.c | 2 ++ |
37 | block/block-copy.c | 10 ++--- | 32 | hw/block/block.c | 42 ++++++++++++++++++++++++++++++---- |
38 | block/nbd.c | 14 +++---- | 33 | tests/unit/ptimer-test-stubs.c | 2 +- |
39 | hw/remote/memory.c | 5 +-- | 34 | util/async.c | 25 ++++++++++++++++---- |
40 | hw/remote/proxy.c | 3 +- | 35 | util/main-loop.c | 4 ++-- |
41 | util/qemu-coroutine-sleep.c | 75 +++++++++++++++++++------------------ | 36 | tests/qemu-iotests/172.out | 38 ++++++++++++++++++++++++++++++ |
42 | 7 files changed, 79 insertions(+), 70 deletions(-) | 37 | 10 files changed, 161 insertions(+), 17 deletions(-) |
43 | 38 | ||
44 | -- | 39 | -- |
45 | 2.31.1 | 40 | 2.31.1 |
46 | 41 | 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 |
1 | From: Paolo Bonzini <pbonzini@redhat.com> | 1 | It can be difficult to debug issues with BHs in production environments. |
---|---|---|---|
2 | Although BHs can usually be identified by looking up their ->cb() | ||
3 | function pointer, this requires debug information for the program. It is | ||
4 | also not possible to print human-readable diagnostics about BHs because | ||
5 | they have no identifier. | ||
2 | 6 | ||
3 | Allow using QemuCoSleep to sleep forever until woken by qemu_co_sleep_wake. | 7 | This patch adds a name to each BH. The name is not unique per instance |
4 | This makes the logic of qemu_co_sleep_ns_wakeable easy to understand. | 8 | but differentiates between cb() functions, which is usually enough. It's |
9 | done by changing aio_bh_new() and friends to macros that stringify cb. | ||
5 | 10 | ||
6 | In the future we will introduce an API that can work even if the | 11 | The next patch will use the name field when reporting leaked BHs. |
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. | ||
10 | 12 | ||
11 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | ||
12 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | ||
13 | Message-id: 20210517100548.28806-7-pbonzini@redhat.com | ||
14 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 13 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
14 | Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> | ||
15 | Message-Id: <20210414200247.917496-2-stefanha@redhat.com> | ||
15 | --- | 16 | --- |
16 | include/qemu/coroutine.h | 5 +++++ | 17 | include/block/aio.h | 31 ++++++++++++++++++++++++++++--- |
17 | util/qemu-coroutine-sleep.c | 26 +++++++++++++++++++------- | 18 | include/qemu/main-loop.h | 4 +++- |
18 | 2 files changed, 24 insertions(+), 7 deletions(-) | 19 | tests/unit/ptimer-test-stubs.c | 2 +- |
20 | util/async.c | 9 +++++++-- | ||
21 | util/main-loop.c | 4 ++-- | ||
22 | 5 files changed, 41 insertions(+), 9 deletions(-) | ||
19 | 23 | ||
20 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h | 24 | diff --git a/include/block/aio.h b/include/block/aio.h |
21 | index XXXXXXX..XXXXXXX 100644 | 25 | index XXXXXXX..XXXXXXX 100644 |
22 | --- a/include/qemu/coroutine.h | 26 | --- a/include/block/aio.h |
23 | +++ b/include/qemu/coroutine.h | 27 | +++ b/include/block/aio.h |
24 | @@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleep { | 28 | @@ -XXX,XX +XXX,XX @@ void aio_context_acquire(AioContext *ctx); |
25 | void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | 29 | /* Relinquish ownership of the AioContext. */ |
26 | QEMUClockType type, int64_t ns); | 30 | void aio_context_release(AioContext *ctx); |
27 | 31 | ||
28 | +/** | 32 | +/** |
29 | + * Yield the coroutine until the next call to qemu_co_sleep_wake. | 33 | + * aio_bh_schedule_oneshot_full: Allocate a new bottom half structure that will |
34 | + * run only once and as soon as possible. | ||
35 | + * | ||
36 | + * @name: A human-readable identifier for debugging purposes. | ||
30 | + */ | 37 | + */ |
31 | +void coroutine_fn qemu_co_sleep(QemuCoSleep *w); | 38 | +void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque, |
39 | + const char *name); | ||
32 | + | 40 | + |
33 | static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) | 41 | /** |
42 | * aio_bh_schedule_oneshot: Allocate a new bottom half structure that will run | ||
43 | * only once and as soon as possible. | ||
44 | + * | ||
45 | + * A convenience wrapper for aio_bh_schedule_oneshot_full() that uses cb as the | ||
46 | + * name string. | ||
47 | */ | ||
48 | -void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque); | ||
49 | +#define aio_bh_schedule_oneshot(ctx, cb, opaque) \ | ||
50 | + aio_bh_schedule_oneshot_full((ctx), (cb), (opaque), (stringify(cb))) | ||
51 | |||
52 | /** | ||
53 | - * aio_bh_new: Allocate a new bottom half structure. | ||
54 | + * aio_bh_new_full: Allocate a new bottom half structure. | ||
55 | * | ||
56 | * Bottom halves are lightweight callbacks whose invocation is guaranteed | ||
57 | * to be wait-free, thread-safe and signal-safe. The #QEMUBH structure | ||
58 | * is opaque and must be allocated prior to its use. | ||
59 | + * | ||
60 | + * @name: A human-readable identifier for debugging purposes. | ||
61 | */ | ||
62 | -QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque); | ||
63 | +QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque, | ||
64 | + const char *name); | ||
65 | + | ||
66 | +/** | ||
67 | + * aio_bh_new: Allocate a new bottom half structure | ||
68 | + * | ||
69 | + * A convenience wrapper for aio_bh_new_full() that uses the cb as the name | ||
70 | + * string. | ||
71 | + */ | ||
72 | +#define aio_bh_new(ctx, cb, opaque) \ | ||
73 | + aio_bh_new_full((ctx), (cb), (opaque), (stringify(cb))) | ||
74 | |||
75 | /** | ||
76 | * aio_notify: Force processing of pending events. | ||
77 | diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h | ||
78 | index XXXXXXX..XXXXXXX 100644 | ||
79 | --- a/include/qemu/main-loop.h | ||
80 | +++ b/include/qemu/main-loop.h | ||
81 | @@ -XXX,XX +XXX,XX @@ void qemu_cond_timedwait_iothread(QemuCond *cond, int ms); | ||
82 | |||
83 | void qemu_fd_register(int fd); | ||
84 | |||
85 | -QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque); | ||
86 | +#define qemu_bh_new(cb, opaque) \ | ||
87 | + qemu_bh_new_full((cb), (opaque), (stringify(cb))) | ||
88 | +QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name); | ||
89 | void qemu_bh_schedule_idle(QEMUBH *bh); | ||
90 | |||
91 | enum { | ||
92 | diff --git a/tests/unit/ptimer-test-stubs.c b/tests/unit/ptimer-test-stubs.c | ||
93 | index XXXXXXX..XXXXXXX 100644 | ||
94 | --- a/tests/unit/ptimer-test-stubs.c | ||
95 | +++ b/tests/unit/ptimer-test-stubs.c | ||
96 | @@ -XXX,XX +XXX,XX @@ int64_t qemu_clock_deadline_ns_all(QEMUClockType type, int attr_mask) | ||
97 | return deadline; | ||
98 | } | ||
99 | |||
100 | -QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque) | ||
101 | +QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name) | ||
34 | { | 102 | { |
35 | QemuCoSleep w = { 0 }; | 103 | QEMUBH *bh = g_new(QEMUBH, 1); |
36 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | 104 | |
105 | diff --git a/util/async.c b/util/async.c | ||
37 | index XXXXXXX..XXXXXXX 100644 | 106 | index XXXXXXX..XXXXXXX 100644 |
38 | --- a/util/qemu-coroutine-sleep.c | 107 | --- a/util/async.c |
39 | +++ b/util/qemu-coroutine-sleep.c | 108 | +++ b/util/async.c |
40 | @@ -XXX,XX +XXX,XX @@ static void co_sleep_cb(void *opaque) | 109 | @@ -XXX,XX +XXX,XX @@ enum { |
41 | qemu_co_sleep_wake(w); | 110 | |
111 | struct QEMUBH { | ||
112 | AioContext *ctx; | ||
113 | + const char *name; | ||
114 | QEMUBHFunc *cb; | ||
115 | void *opaque; | ||
116 | QSLIST_ENTRY(QEMUBH) next; | ||
117 | @@ -XXX,XX +XXX,XX @@ static QEMUBH *aio_bh_dequeue(BHList *head, unsigned *flags) | ||
118 | return bh; | ||
42 | } | 119 | } |
43 | 120 | ||
44 | -void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | 121 | -void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque) |
45 | - QEMUClockType type, int64_t ns) | 122 | +void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb, |
46 | +void coroutine_fn qemu_co_sleep(QemuCoSleep *w) | 123 | + void *opaque, const char *name) |
47 | { | 124 | { |
48 | Coroutine *co = qemu_coroutine_self(); | 125 | QEMUBH *bh; |
49 | - AioContext *ctx = qemu_get_current_aio_context(); | 126 | bh = g_new(QEMUBH, 1); |
50 | - QEMUTimer ts; | 127 | @@ -XXX,XX +XXX,XX @@ void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque) |
51 | 128 | .ctx = ctx, | |
52 | const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL, | 129 | .cb = cb, |
53 | qemu_co_sleep_ns__scheduled); | 130 | .opaque = opaque, |
54 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | 131 | + .name = name, |
55 | } | 132 | }; |
56 | 133 | aio_bh_enqueue(bh, BH_SCHEDULED | BH_ONESHOT); | |
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); | ||
65 | } | 134 | } |
66 | + | 135 | |
67 | +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | 136 | -QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque) |
68 | + QEMUClockType type, int64_t ns) | 137 | +QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque, |
69 | +{ | 138 | + const char *name) |
70 | + AioContext *ctx = qemu_get_current_aio_context(); | 139 | { |
71 | + QEMUTimer ts; | 140 | QEMUBH *bh; |
72 | + | 141 | bh = g_new(QEMUBH, 1); |
73 | + aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w); | 142 | @@ -XXX,XX +XXX,XX @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque) |
74 | + timer_mod(&ts, qemu_clock_get_ns(type) + ns); | 143 | .ctx = ctx, |
75 | + | 144 | .cb = cb, |
76 | + /* | 145 | .opaque = opaque, |
77 | + * The timer will fire in the current AiOContext, so the callback | 146 | + .name = name, |
78 | + * must happen after qemu_co_sleep yields and there is no race | 147 | }; |
79 | + * between timer_mod and qemu_co_sleep. | 148 | return bh; |
80 | + */ | 149 | } |
81 | + qemu_co_sleep(w); | 150 | diff --git a/util/main-loop.c b/util/main-loop.c |
82 | + timer_del(&ts); | 151 | index XXXXXXX..XXXXXXX 100644 |
83 | +} | 152 | --- a/util/main-loop.c |
153 | +++ b/util/main-loop.c | ||
154 | @@ -XXX,XX +XXX,XX @@ void main_loop_wait(int nonblocking) | ||
155 | |||
156 | /* Functions to operate on the main QEMU AioContext. */ | ||
157 | |||
158 | -QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque) | ||
159 | +QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name) | ||
160 | { | ||
161 | - return aio_bh_new(qemu_aio_context, cb, opaque); | ||
162 | + return aio_bh_new_full(qemu_aio_context, cb, opaque, name); | ||
163 | } | ||
164 | |||
165 | /* | ||
84 | -- | 166 | -- |
85 | 2.31.1 | 167 | 2.31.1 |
86 | 168 | diff view generated by jsdifflib |
1 | From: Paolo Bonzini <pbonzini@redhat.com> | 1 | BHs must be deleted before the AioContext is finalized. If not, it's a |
---|---|---|---|
2 | bug and probably indicates that some part of the program still expects | ||
3 | the BH to run in the future. That can lead to memory leaks, inconsistent | ||
4 | state, or just hangs. | ||
2 | 5 | ||
3 | This simplification is enabled by the previous patch. Now aio_co_wake | 6 | Unfortunately the assert(flags & BH_DELETED) call in aio_ctx_finalize() |
4 | will only be called once, therefore we do not care about a spurious | 7 | is difficult to debug because the assertion failure contains no |
5 | firing of the timer after a qemu_co_sleep_wake. | 8 | information about the BH! |
6 | 9 | ||
7 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 10 | Use the QEMUBH name field added in the previous patch to show a useful |
8 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | 11 | error when a leaked BH is detected. |
9 | Message-id: 20210517100548.28806-5-pbonzini@redhat.com | 12 | |
13 | Suggested-by: Eric Ernst <eric.g.ernst@gmail.com> | ||
10 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 14 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
15 | Message-Id: <20210414200247.917496-3-stefanha@redhat.com> | ||
11 | --- | 16 | --- |
12 | util/qemu-coroutine-sleep.c | 8 ++++---- | 17 | util/async.c | 16 ++++++++++++++-- |
13 | 1 file changed, 4 insertions(+), 4 deletions(-) | 18 | 1 file changed, 14 insertions(+), 2 deletions(-) |
14 | 19 | ||
15 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | 20 | diff --git a/util/async.c b/util/async.c |
16 | index XXXXXXX..XXXXXXX 100644 | 21 | index XXXXXXX..XXXXXXX 100644 |
17 | --- a/util/qemu-coroutine-sleep.c | 22 | --- a/util/async.c |
18 | +++ b/util/qemu-coroutine-sleep.c | 23 | +++ b/util/async.c |
19 | @@ -XXX,XX +XXX,XX @@ static const char *qemu_co_sleep_ns__scheduled = "qemu_co_sleep_ns"; | 24 | @@ -XXX,XX +XXX,XX @@ aio_ctx_finalize(GSource *source) |
20 | 25 | assert(QSIMPLEQ_EMPTY(&ctx->bh_slice_list)); | |
21 | struct QemuCoSleepState { | 26 | |
22 | Coroutine *co; | 27 | while ((bh = aio_bh_dequeue(&ctx->bh_list, &flags))) { |
23 | - QEMUTimer ts; | 28 | - /* qemu_bh_delete() must have been called on BHs in this AioContext */ |
24 | QemuCoSleepState **user_state_pointer; | 29 | - assert(flags & BH_DELETED); |
25 | }; | 30 | + /* |
26 | 31 | + * qemu_bh_delete() must have been called on BHs in this AioContext. In | |
27 | @@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) | 32 | + * many cases memory leaks, hangs, or inconsistent state occur when a |
28 | 33 | + * BH is leaked because something still expects it to run. | |
29 | assert(scheduled == qemu_co_sleep_ns__scheduled); | 34 | + * |
30 | *sleep_state->user_state_pointer = NULL; | 35 | + * If you hit this, fix the lifecycle of the BH so that |
31 | - timer_del(&sleep_state->ts); | 36 | + * qemu_bh_delete() and any associated cleanup is called before the |
32 | aio_co_wake(sleep_state->co); | 37 | + * AioContext is finalized. |
38 | + */ | ||
39 | + if (unlikely(!(flags & BH_DELETED))) { | ||
40 | + fprintf(stderr, "%s: BH '%s' leaked, aborting...\n", | ||
41 | + __func__, bh->name); | ||
42 | + abort(); | ||
43 | + } | ||
44 | |||
45 | g_free(bh); | ||
33 | } | 46 | } |
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 | -- | 47 | -- |
58 | 2.31.1 | 48 | 2.31.1 |
59 | 49 | diff view generated by jsdifflib |
1 | From: Paolo Bonzini <pbonzini@redhat.com> | 1 | From: Akihiko Odaki <akihiko.odaki@gmail.com> |
---|---|---|---|
2 | 2 | ||
3 | Right now, users of qemu_co_sleep_ns_wakeable are simply passing | 3 | This commit introduces "punch hole" operation and optimizes transfer |
4 | a pointer to QemuCoSleepState by reference to the function. But | 4 | block size for macOS. |
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 | 5 | ||
9 | Since the usage is changed, take the occasion to rename the | 6 | Thanks to Konstantin Nazarov for detailed analysis of a flaw in an |
10 | struct to QemuCoSleep. | 7 | old version of this change: |
8 | https://gist.github.com/akihikodaki/87df4149e7ca87f18dc56807ec5a1bc5#gistcomment-3654667 | ||
11 | 9 | ||
12 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 10 | Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> |
13 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | 11 | Message-id: 20210705130458.97642-1-akihiko.odaki@gmail.com |
14 | Message-id: 20210517100548.28806-6-pbonzini@redhat.com | ||
15 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 12 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
16 | --- | 13 | --- |
17 | include/qemu/coroutine.h | 23 +++++++++++---------- | 14 | block/file-posix.c | 27 +++++++++++++++++++++++++-- |
18 | block/block-copy.c | 8 ++++---- | 15 | 1 file changed, 25 insertions(+), 2 deletions(-) |
19 | block/nbd.c | 10 ++++----- | ||
20 | util/qemu-coroutine-sleep.c | 41 ++++++++++++++++--------------------- | ||
21 | 4 files changed, 39 insertions(+), 43 deletions(-) | ||
22 | 16 | ||
23 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h | 17 | diff --git a/block/file-posix.c b/block/file-posix.c |
24 | index XXXXXXX..XXXXXXX 100644 | 18 | index XXXXXXX..XXXXXXX 100644 |
25 | --- a/include/qemu/coroutine.h | 19 | --- a/block/file-posix.c |
26 | +++ b/include/qemu/coroutine.h | 20 | +++ b/block/file-posix.c |
27 | @@ -XXX,XX +XXX,XX @@ void qemu_co_rwlock_wrlock(CoRwlock *lock); | 21 | @@ -XXX,XX +XXX,XX @@ |
28 | */ | 22 | #if defined(HAVE_HOST_BLOCK_DEVICE) |
29 | void qemu_co_rwlock_unlock(CoRwlock *lock); | 23 | #include <paths.h> |
30 | 24 | #include <sys/param.h> | |
31 | -typedef struct QemuCoSleepState QemuCoSleepState; | 25 | +#include <sys/mount.h> |
32 | +typedef struct QemuCoSleep { | 26 | #include <IOKit/IOKitLib.h> |
33 | + Coroutine *to_wake; | 27 | #include <IOKit/IOBSD.h> |
34 | +} QemuCoSleep; | 28 | #include <IOKit/storage/IOMediaBSDClient.h> |
35 | 29 | @@ -XXX,XX +XXX,XX @@ static void raw_refresh_limits(BlockDriverState *bs, Error **errp) | |
36 | /** | 30 | return; |
37 | - * Yield the coroutine for a given duration. During this yield, @sleep_state | 31 | } |
38 | - * is set to an opaque pointer, which may be used for | 32 | |
39 | - * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the | 33 | +#if defined(__APPLE__) && (__MACH__) |
40 | - * timer fires. Don't save the obtained value to other variables and don't call | 34 | + struct statfs buf; |
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 | + | 35 | + |
51 | static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) | 36 | + if (!fstatfs(s->fd, &buf)) { |
52 | { | 37 | + bs->bl.opt_transfer = buf.f_iosize; |
53 | - QemuCoSleepState *unused = NULL; | 38 | + bs->bl.pdiscard_alignment = buf.f_bsize; |
54 | - qemu_co_sleep_ns_wakeable(type, ns, &unused); | 39 | + } |
55 | + QemuCoSleep w = { 0 }; | 40 | +#endif |
56 | + qemu_co_sleep_ns_wakeable(&w, type, ns); | 41 | + |
57 | } | 42 | if (bs->sg || S_ISBLK(st.st_mode)) { |
58 | 43 | int ret = hdev_get_max_hw_transfer(s->fd, &st); | |
59 | /** | 44 | |
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: | 45 | @@ -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 | } | 46 | } |
175 | } | 47 | } |
176 | 48 | ||
177 | static void co_sleep_cb(void *opaque) | 49 | +#if defined(CONFIG_FALLOCATE) || defined(BLKZEROOUT) || defined(BLKDISCARD) |
50 | static int translate_err(int err) | ||
178 | { | 51 | { |
179 | - QemuCoSleepState **sleep_state = opaque; | 52 | if (err == -ENODEV || err == -ENOSYS || err == -EOPNOTSUPP || |
180 | - qemu_co_sleep_wake(*sleep_state); | 53 | @@ -XXX,XX +XXX,XX @@ static int translate_err(int err) |
181 | + QemuCoSleep *w = opaque; | 54 | } |
182 | + qemu_co_sleep_wake(w); | 55 | return err; |
183 | } | 56 | } |
184 | 57 | +#endif | |
185 | -void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | 58 | |
186 | - QemuCoSleepState **sleep_state) | 59 | #ifdef CONFIG_FALLOCATE |
187 | +void coroutine_fn qemu_co_sleep_ns_wakeable(QemuCoSleep *w, | 60 | static int do_fallocate(int fd, int mode, off_t offset, off_t len) |
188 | + QEMUClockType type, int64_t ns) | 61 | @@ -XXX,XX +XXX,XX @@ static int handle_aiocb_discard(void *opaque) |
189 | { | 62 | } |
190 | + Coroutine *co = qemu_coroutine_self(); | 63 | } while (errno == EINTR); |
191 | AioContext *ctx = qemu_get_current_aio_context(); | 64 | |
192 | QEMUTimer ts; | 65 | - ret = -errno; |
193 | - QemuCoSleepState state = { | 66 | + ret = translate_err(-errno); |
194 | - .co = qemu_coroutine_self(), | 67 | #endif |
195 | - .user_state_pointer = sleep_state, | 68 | } else { |
196 | - }; | 69 | #ifdef CONFIG_FALLOCATE_PUNCH_HOLE |
197 | 70 | ret = do_fallocate(s->fd, FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, | |
198 | - const char *scheduled = qatomic_cmpxchg(&state.co->scheduled, NULL, | 71 | aiocb->aio_offset, aiocb->aio_nbytes); |
199 | - qemu_co_sleep_ns__scheduled); | 72 | + ret = translate_err(-errno); |
200 | + const char *scheduled = qatomic_cmpxchg(&co->scheduled, NULL, | 73 | +#elif defined(__APPLE__) && (__MACH__) |
201 | + qemu_co_sleep_ns__scheduled); | 74 | + fpunchhole_t fpunchhole; |
202 | if (scheduled) { | 75 | + fpunchhole.fp_flags = 0; |
203 | fprintf(stderr, | 76 | + fpunchhole.reserved = 0; |
204 | "%s: Co-routine was already scheduled in '%s'\n", | 77 | + fpunchhole.fp_offset = aiocb->aio_offset; |
205 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | 78 | + fpunchhole.fp_length = aiocb->aio_nbytes; |
206 | abort(); | 79 | + if (fcntl(s->fd, F_PUNCHHOLE, &fpunchhole) == -1) { |
80 | + ret = errno == ENODEV ? -ENOTSUP : -errno; | ||
81 | + } else { | ||
82 | + ret = 0; | ||
83 | + } | ||
84 | #endif | ||
207 | } | 85 | } |
208 | 86 | ||
209 | - aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, sleep_state); | 87 | - ret = translate_err(ret); |
210 | - *sleep_state = &state; | 88 | if (ret == -ENOTSUP) { |
211 | + w->to_wake = co; | 89 | s->has_discard = false; |
212 | + aio_timer_init(ctx, &ts, type, SCALE_NS, co_sleep_cb, w), | 90 | } |
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 | -- | 91 | -- |
223 | 2.31.1 | 92 | 2.31.1 |
224 | 93 | diff view generated by jsdifflib |
1 | From: Paolo Bonzini <pbonzini@redhat.com> | 1 | From: Akihiko Odaki <akihiko.odaki@gmail.com> |
---|---|---|---|
2 | 2 | ||
3 | Simplify the code by removing conditionals. qemu_co_sleep_ns | 3 | backend_defaults property allow users to control if default block |
4 | can simply point the argument to an on-stack temporary. | 4 | properties should be decided with backend information. |
5 | 5 | ||
6 | Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> | 6 | If it is off, any backend information will be discarded, which is |
7 | Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> | 7 | suitable if you plan to perform live migration to a different disk backend. |
8 | Message-id: 20210517100548.28806-3-pbonzini@redhat.com | 8 | |
9 | If it is on, a block device may utilize backend information more | ||
10 | aggressively. | ||
11 | |||
12 | By default, it is auto, which uses backend information for block | ||
13 | sizes and ignores the others, which is consistent with the older | ||
14 | versions. | ||
15 | |||
16 | Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> | ||
17 | Message-id: 20210705130458.97642-2-akihiko.odaki@gmail.com | ||
9 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> | 18 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
10 | --- | 19 | --- |
11 | include/qemu/coroutine.h | 5 +++-- | 20 | include/hw/block/block.h | 3 +++ |
12 | util/qemu-coroutine-sleep.c | 18 +++++------------- | 21 | hw/block/block.c | 42 ++++++++++++++++++++++++++++++++++---- |
13 | 2 files changed, 8 insertions(+), 15 deletions(-) | 22 | tests/qemu-iotests/172.out | 38 ++++++++++++++++++++++++++++++++++ |
14 | 23 | 3 files changed, 79 insertions(+), 4 deletions(-) | |
15 | diff --git a/include/qemu/coroutine.h b/include/qemu/coroutine.h | 24 | |
25 | diff --git a/include/hw/block/block.h b/include/hw/block/block.h | ||
16 | index XXXXXXX..XXXXXXX 100644 | 26 | index XXXXXXX..XXXXXXX 100644 |
17 | --- a/include/qemu/coroutine.h | 27 | --- a/include/hw/block/block.h |
18 | +++ b/include/qemu/coroutine.h | 28 | +++ b/include/hw/block/block.h |
19 | @@ -XXX,XX +XXX,XX @@ typedef struct QemuCoSleepState QemuCoSleepState; | 29 | @@ -XXX,XX +XXX,XX @@ |
20 | 30 | ||
21 | /** | 31 | typedef struct BlockConf { |
22 | * Yield the coroutine for a given duration. During this yield, @sleep_state | 32 | BlockBackend *blk; |
23 | - * (if not NULL) is set to an opaque pointer, which may be used for | 33 | + OnOffAuto backend_defaults; |
24 | + * is set to an opaque pointer, which may be used for | 34 | uint32_t physical_block_size; |
25 | * qemu_co_sleep_wake(). Be careful, the pointer is set back to zero when the | 35 | uint32_t logical_block_size; |
26 | * timer fires. Don't save the obtained value to other variables and don't call | 36 | uint32_t min_io_size; |
27 | * qemu_co_sleep_wake from another aio context. | 37 | @@ -XXX,XX +XXX,XX @@ static inline unsigned int get_physical_block_exp(BlockConf *conf) |
28 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | 38 | } |
29 | QemuCoSleepState **sleep_state); | 39 | |
30 | static inline void coroutine_fn qemu_co_sleep_ns(QEMUClockType type, int64_t ns) | 40 | #define DEFINE_BLOCK_PROPERTIES_BASE(_state, _conf) \ |
41 | + DEFINE_PROP_ON_OFF_AUTO("backend_defaults", _state, \ | ||
42 | + _conf.backend_defaults, ON_OFF_AUTO_AUTO), \ | ||
43 | DEFINE_PROP_BLOCKSIZE("logical_block_size", _state, \ | ||
44 | _conf.logical_block_size), \ | ||
45 | DEFINE_PROP_BLOCKSIZE("physical_block_size", _state, \ | ||
46 | diff --git a/hw/block/block.c b/hw/block/block.c | ||
47 | index XXXXXXX..XXXXXXX 100644 | ||
48 | --- a/hw/block/block.c | ||
49 | +++ b/hw/block/block.c | ||
50 | @@ -XXX,XX +XXX,XX @@ bool blkconf_blocksizes(BlockConf *conf, Error **errp) | ||
31 | { | 51 | { |
32 | - qemu_co_sleep_ns_wakeable(type, ns, NULL); | 52 | BlockBackend *blk = conf->blk; |
33 | + QemuCoSleepState *unused = NULL; | 53 | BlockSizes blocksizes; |
34 | + qemu_co_sleep_ns_wakeable(type, ns, &unused); | 54 | - int backend_ret; |
35 | } | 55 | + BlockDriverState *bs; |
36 | 56 | + bool use_blocksizes; | |
37 | /** | 57 | + bool use_bs; |
38 | diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c | 58 | + |
59 | + switch (conf->backend_defaults) { | ||
60 | + case ON_OFF_AUTO_AUTO: | ||
61 | + use_blocksizes = !blk_probe_blocksizes(blk, &blocksizes); | ||
62 | + use_bs = false; | ||
63 | + break; | ||
64 | + | ||
65 | + case ON_OFF_AUTO_ON: | ||
66 | + use_blocksizes = !blk_probe_blocksizes(blk, &blocksizes); | ||
67 | + bs = blk_bs(blk); | ||
68 | + use_bs = bs; | ||
69 | + break; | ||
70 | + | ||
71 | + case ON_OFF_AUTO_OFF: | ||
72 | + use_blocksizes = false; | ||
73 | + use_bs = false; | ||
74 | + break; | ||
75 | + | ||
76 | + default: | ||
77 | + abort(); | ||
78 | + } | ||
79 | |||
80 | - backend_ret = blk_probe_blocksizes(blk, &blocksizes); | ||
81 | /* fill in detected values if they are not defined via qemu command line */ | ||
82 | if (!conf->physical_block_size) { | ||
83 | - if (!backend_ret) { | ||
84 | + if (use_blocksizes) { | ||
85 | conf->physical_block_size = blocksizes.phys; | ||
86 | } else { | ||
87 | conf->physical_block_size = BDRV_SECTOR_SIZE; | ||
88 | } | ||
89 | } | ||
90 | if (!conf->logical_block_size) { | ||
91 | - if (!backend_ret) { | ||
92 | + if (use_blocksizes) { | ||
93 | conf->logical_block_size = blocksizes.log; | ||
94 | } else { | ||
95 | conf->logical_block_size = BDRV_SECTOR_SIZE; | ||
96 | } | ||
97 | } | ||
98 | + if (use_bs) { | ||
99 | + if (!conf->opt_io_size) { | ||
100 | + conf->opt_io_size = bs->bl.opt_transfer; | ||
101 | + } | ||
102 | + if (conf->discard_granularity == -1) { | ||
103 | + if (bs->bl.pdiscard_alignment) { | ||
104 | + conf->discard_granularity = bs->bl.pdiscard_alignment; | ||
105 | + } else if (bs->bl.request_alignment != 1) { | ||
106 | + conf->discard_granularity = bs->bl.request_alignment; | ||
107 | + } | ||
108 | + } | ||
109 | + } | ||
110 | |||
111 | if (conf->logical_block_size > conf->physical_block_size) { | ||
112 | error_setg(errp, | ||
113 | diff --git a/tests/qemu-iotests/172.out b/tests/qemu-iotests/172.out | ||
39 | index XXXXXXX..XXXXXXX 100644 | 114 | index XXXXXXX..XXXXXXX 100644 |
40 | --- a/util/qemu-coroutine-sleep.c | 115 | --- a/tests/qemu-iotests/172.out |
41 | +++ b/util/qemu-coroutine-sleep.c | 116 | +++ b/tests/qemu-iotests/172.out |
42 | @@ -XXX,XX +XXX,XX @@ void qemu_co_sleep_wake(QemuCoSleepState *sleep_state) | 117 | @@ -XXX,XX +XXX,XX @@ Testing: |
43 | qemu_co_sleep_ns__scheduled, NULL); | 118 | dev: floppy, id "" |
44 | 119 | unit = 0 (0x0) | |
45 | assert(scheduled == qemu_co_sleep_ns__scheduled); | 120 | drive = "floppy0" |
46 | - if (sleep_state->user_state_pointer) { | 121 | + backend_defaults = "auto" |
47 | - *sleep_state->user_state_pointer = NULL; | 122 | logical_block_size = 512 (512 B) |
48 | - } | 123 | physical_block_size = 512 (512 B) |
49 | + *sleep_state->user_state_pointer = NULL; | 124 | min_io_size = 0 (0 B) |
50 | timer_del(&sleep_state->ts); | 125 | @@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 |
51 | aio_co_wake(sleep_state->co); | 126 | dev: floppy, id "" |
52 | } | 127 | unit = 0 (0x0) |
53 | @@ -XXX,XX +XXX,XX @@ void coroutine_fn qemu_co_sleep_ns_wakeable(QEMUClockType type, int64_t ns, | 128 | drive = "floppy0" |
54 | } | 129 | + backend_defaults = "auto" |
55 | 130 | logical_block_size = 512 (512 B) | |
56 | aio_timer_init(ctx, &state.ts, type, SCALE_NS, co_sleep_cb, &state); | 131 | physical_block_size = 512 (512 B) |
57 | - if (sleep_state) { | 132 | min_io_size = 0 (0 B) |
58 | - *sleep_state = &state; | 133 | @@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 |
59 | - } | 134 | dev: floppy, id "" |
60 | + *sleep_state = &state; | 135 | unit = 1 (0x1) |
61 | timer_mod(&state.ts, qemu_clock_get_ns(type) + ns); | 136 | drive = "floppy1" |
62 | qemu_coroutine_yield(); | 137 | + backend_defaults = "auto" |
63 | - if (sleep_state) { | 138 | logical_block_size = 512 (512 B) |
64 | - /* | 139 | physical_block_size = 512 (512 B) |
65 | - * Note that *sleep_state is cleared during qemu_co_sleep_wake | 140 | min_io_size = 0 (0 B) |
66 | - * before resuming this coroutine. | 141 | @@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 |
67 | - */ | 142 | dev: floppy, id "" |
68 | - assert(*sleep_state == NULL); | 143 | unit = 0 (0x0) |
69 | - } | 144 | drive = "floppy0" |
70 | + | 145 | + backend_defaults = "auto" |
71 | + /* qemu_co_sleep_wake clears *sleep_state before resuming this coroutine. */ | 146 | logical_block_size = 512 (512 B) |
72 | + assert(*sleep_state == NULL); | 147 | physical_block_size = 512 (512 B) |
73 | } | 148 | min_io_size = 0 (0 B) |
149 | @@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2.2 | ||
150 | dev: floppy, id "" | ||
151 | unit = 1 (0x1) | ||
152 | drive = "floppy1" | ||
153 | + backend_defaults = "auto" | ||
154 | logical_block_size = 512 (512 B) | ||
155 | physical_block_size = 512 (512 B) | ||
156 | min_io_size = 0 (0 B) | ||
157 | @@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -fdb TEST_DIR/t.qcow2.2 | ||
158 | dev: floppy, id "" | ||
159 | unit = 0 (0x0) | ||
160 | drive = "floppy0" | ||
161 | + backend_defaults = "auto" | ||
162 | logical_block_size = 512 (512 B) | ||
163 | physical_block_size = 512 (512 B) | ||
164 | min_io_size = 0 (0 B) | ||
165 | @@ -XXX,XX +XXX,XX @@ Testing: -fdb | ||
166 | dev: floppy, id "" | ||
167 | unit = 1 (0x1) | ||
168 | drive = "floppy1" | ||
169 | + backend_defaults = "auto" | ||
170 | logical_block_size = 512 (512 B) | ||
171 | physical_block_size = 512 (512 B) | ||
172 | min_io_size = 0 (0 B) | ||
173 | @@ -XXX,XX +XXX,XX @@ Testing: -fdb | ||
174 | dev: floppy, id "" | ||
175 | unit = 0 (0x0) | ||
176 | drive = "floppy0" | ||
177 | + backend_defaults = "auto" | ||
178 | logical_block_size = 512 (512 B) | ||
179 | physical_block_size = 512 (512 B) | ||
180 | min_io_size = 0 (0 B) | ||
181 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 | ||
182 | dev: floppy, id "" | ||
183 | unit = 0 (0x0) | ||
184 | drive = "floppy0" | ||
185 | + backend_defaults = "auto" | ||
186 | logical_block_size = 512 (512 B) | ||
187 | physical_block_size = 512 (512 B) | ||
188 | min_io_size = 0 (0 B) | ||
189 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1 | ||
190 | dev: floppy, id "" | ||
191 | unit = 1 (0x1) | ||
192 | drive = "floppy1" | ||
193 | + backend_defaults = "auto" | ||
194 | logical_block_size = 512 (512 B) | ||
195 | physical_block_size = 512 (512 B) | ||
196 | min_io_size = 0 (0 B) | ||
197 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2,index=1 | ||
198 | dev: floppy, id "" | ||
199 | unit = 0 (0x0) | ||
200 | drive = "floppy0" | ||
201 | + backend_defaults = "auto" | ||
202 | logical_block_size = 512 (512 B) | ||
203 | physical_block_size = 512 (512 B) | ||
204 | min_io_size = 0 (0 B) | ||
205 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t | ||
206 | dev: floppy, id "" | ||
207 | unit = 1 (0x1) | ||
208 | drive = "floppy1" | ||
209 | + backend_defaults = "auto" | ||
210 | logical_block_size = 512 (512 B) | ||
211 | physical_block_size = 512 (512 B) | ||
212 | min_io_size = 0 (0 B) | ||
213 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=floppy,file=TEST_DIR/t | ||
214 | dev: floppy, id "" | ||
215 | unit = 0 (0x0) | ||
216 | drive = "floppy0" | ||
217 | + backend_defaults = "auto" | ||
218 | logical_block_size = 512 (512 B) | ||
219 | physical_block_size = 512 (512 B) | ||
220 | min_io_size = 0 (0 B) | ||
221 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0 | ||
222 | dev: floppy, id "" | ||
223 | unit = 0 (0x0) | ||
224 | drive = "none0" | ||
225 | + backend_defaults = "auto" | ||
226 | logical_block_size = 512 (512 B) | ||
227 | physical_block_size = 512 (512 B) | ||
228 | min_io_size = 0 (0 B) | ||
229 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,unit=1 | ||
230 | dev: floppy, id "" | ||
231 | unit = 1 (0x1) | ||
232 | drive = "none0" | ||
233 | + backend_defaults = "auto" | ||
234 | logical_block_size = 512 (512 B) | ||
235 | physical_block_size = 512 (512 B) | ||
236 | min_io_size = 0 (0 B) | ||
237 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco | ||
238 | dev: floppy, id "" | ||
239 | unit = 1 (0x1) | ||
240 | drive = "none1" | ||
241 | + backend_defaults = "auto" | ||
242 | logical_block_size = 512 (512 B) | ||
243 | physical_block_size = 512 (512 B) | ||
244 | min_io_size = 0 (0 B) | ||
245 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qco | ||
246 | dev: floppy, id "" | ||
247 | unit = 0 (0x0) | ||
248 | drive = "none0" | ||
249 | + backend_defaults = "auto" | ||
250 | logical_block_size = 512 (512 B) | ||
251 | physical_block_size = 512 (512 B) | ||
252 | min_io_size = 0 (0 B) | ||
253 | @@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl | ||
254 | dev: floppy, id "" | ||
255 | unit = 1 (0x1) | ||
256 | drive = "none0" | ||
257 | + backend_defaults = "auto" | ||
258 | logical_block_size = 512 (512 B) | ||
259 | physical_block_size = 512 (512 B) | ||
260 | min_io_size = 0 (0 B) | ||
261 | @@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl | ||
262 | dev: floppy, id "" | ||
263 | unit = 0 (0x0) | ||
264 | drive = "floppy0" | ||
265 | + backend_defaults = "auto" | ||
266 | logical_block_size = 512 (512 B) | ||
267 | physical_block_size = 512 (512 B) | ||
268 | min_io_size = 0 (0 B) | ||
269 | @@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl | ||
270 | dev: floppy, id "" | ||
271 | unit = 1 (0x1) | ||
272 | drive = "none0" | ||
273 | + backend_defaults = "auto" | ||
274 | logical_block_size = 512 (512 B) | ||
275 | physical_block_size = 512 (512 B) | ||
276 | min_io_size = 0 (0 B) | ||
277 | @@ -XXX,XX +XXX,XX @@ Testing: -fda TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl | ||
278 | dev: floppy, id "" | ||
279 | unit = 0 (0x0) | ||
280 | drive = "floppy0" | ||
281 | + backend_defaults = "auto" | ||
282 | logical_block_size = 512 (512 B) | ||
283 | physical_block_size = 512 (512 B) | ||
284 | min_io_size = 0 (0 B) | ||
285 | @@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl | ||
286 | dev: floppy, id "" | ||
287 | unit = 0 (0x0) | ||
288 | drive = "none0" | ||
289 | + backend_defaults = "auto" | ||
290 | logical_block_size = 512 (512 B) | ||
291 | physical_block_size = 512 (512 B) | ||
292 | min_io_size = 0 (0 B) | ||
293 | @@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl | ||
294 | dev: floppy, id "" | ||
295 | unit = 1 (0x1) | ||
296 | drive = "floppy1" | ||
297 | + backend_defaults = "auto" | ||
298 | logical_block_size = 512 (512 B) | ||
299 | physical_block_size = 512 (512 B) | ||
300 | min_io_size = 0 (0 B) | ||
301 | @@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl | ||
302 | dev: floppy, id "" | ||
303 | unit = 0 (0x0) | ||
304 | drive = "none0" | ||
305 | + backend_defaults = "auto" | ||
306 | logical_block_size = 512 (512 B) | ||
307 | physical_block_size = 512 (512 B) | ||
308 | min_io_size = 0 (0 B) | ||
309 | @@ -XXX,XX +XXX,XX @@ Testing: -fdb TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.qcow2.2 -device fl | ||
310 | dev: floppy, id "" | ||
311 | unit = 1 (0x1) | ||
312 | drive = "floppy1" | ||
313 | + backend_defaults = "auto" | ||
314 | logical_block_size = 512 (512 B) | ||
315 | physical_block_size = 512 (512 B) | ||
316 | min_io_size = 0 (0 B) | ||
317 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q | ||
318 | dev: floppy, id "" | ||
319 | unit = 1 (0x1) | ||
320 | drive = "none0" | ||
321 | + backend_defaults = "auto" | ||
322 | logical_block_size = 512 (512 B) | ||
323 | physical_block_size = 512 (512 B) | ||
324 | min_io_size = 0 (0 B) | ||
325 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q | ||
326 | dev: floppy, id "" | ||
327 | unit = 0 (0x0) | ||
328 | drive = "floppy0" | ||
329 | + backend_defaults = "auto" | ||
330 | logical_block_size = 512 (512 B) | ||
331 | physical_block_size = 512 (512 B) | ||
332 | min_io_size = 0 (0 B) | ||
333 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q | ||
334 | dev: floppy, id "" | ||
335 | unit = 1 (0x1) | ||
336 | drive = "none0" | ||
337 | + backend_defaults = "auto" | ||
338 | logical_block_size = 512 (512 B) | ||
339 | physical_block_size = 512 (512 B) | ||
340 | min_io_size = 0 (0 B) | ||
341 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=floppy,file=TEST_DIR/t.qcow2 -drive if=none,file=TEST_DIR/t.q | ||
342 | dev: floppy, id "" | ||
343 | unit = 0 (0x0) | ||
344 | drive = "floppy0" | ||
345 | + backend_defaults = "auto" | ||
346 | logical_block_size = 512 (512 B) | ||
347 | physical_block_size = 512 (512 B) | ||
348 | min_io_size = 0 (0 B) | ||
349 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -global floppy.drive=none0 -device | ||
350 | dev: floppy, id "" | ||
351 | unit = 0 (0x0) | ||
352 | drive = "none0" | ||
353 | + backend_defaults = "auto" | ||
354 | logical_block_size = 512 (512 B) | ||
355 | physical_block_size = 512 (512 B) | ||
356 | min_io_size = 0 (0 B) | ||
357 | @@ -XXX,XX +XXX,XX @@ Testing: -device floppy | ||
358 | dev: floppy, id "" | ||
359 | unit = 0 (0x0) | ||
360 | drive = "" | ||
361 | + backend_defaults = "auto" | ||
362 | logical_block_size = 512 (512 B) | ||
363 | physical_block_size = 512 (512 B) | ||
364 | min_io_size = 0 (0 B) | ||
365 | @@ -XXX,XX +XXX,XX @@ Testing: -device floppy,drive-type=120 | ||
366 | dev: floppy, id "" | ||
367 | unit = 0 (0x0) | ||
368 | drive = "" | ||
369 | + backend_defaults = "auto" | ||
370 | logical_block_size = 512 (512 B) | ||
371 | physical_block_size = 512 (512 B) | ||
372 | min_io_size = 0 (0 B) | ||
373 | @@ -XXX,XX +XXX,XX @@ Testing: -device floppy,drive-type=144 | ||
374 | dev: floppy, id "" | ||
375 | unit = 0 (0x0) | ||
376 | drive = "" | ||
377 | + backend_defaults = "auto" | ||
378 | logical_block_size = 512 (512 B) | ||
379 | physical_block_size = 512 (512 B) | ||
380 | min_io_size = 0 (0 B) | ||
381 | @@ -XXX,XX +XXX,XX @@ Testing: -device floppy,drive-type=288 | ||
382 | dev: floppy, id "" | ||
383 | unit = 0 (0x0) | ||
384 | drive = "" | ||
385 | + backend_defaults = "auto" | ||
386 | logical_block_size = 512 (512 B) | ||
387 | physical_block_size = 512 (512 B) | ||
388 | min_io_size = 0 (0 B) | ||
389 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t | ||
390 | dev: floppy, id "" | ||
391 | unit = 0 (0x0) | ||
392 | drive = "none0" | ||
393 | + backend_defaults = "auto" | ||
394 | logical_block_size = 512 (512 B) | ||
395 | physical_block_size = 512 (512 B) | ||
396 | min_io_size = 0 (0 B) | ||
397 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,drive-t | ||
398 | dev: floppy, id "" | ||
399 | unit = 0 (0x0) | ||
400 | drive = "none0" | ||
401 | + backend_defaults = "auto" | ||
402 | logical_block_size = 512 (512 B) | ||
403 | physical_block_size = 512 (512 B) | ||
404 | min_io_size = 0 (0 B) | ||
405 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,logical | ||
406 | dev: floppy, id "" | ||
407 | unit = 0 (0x0) | ||
408 | drive = "none0" | ||
409 | + backend_defaults = "auto" | ||
410 | logical_block_size = 512 (512 B) | ||
411 | physical_block_size = 512 (512 B) | ||
412 | min_io_size = 0 (0 B) | ||
413 | @@ -XXX,XX +XXX,XX @@ Testing: -drive if=none,file=TEST_DIR/t.qcow2 -device floppy,drive=none0,physica | ||
414 | dev: floppy, id "" | ||
415 | unit = 0 (0x0) | ||
416 | drive = "none0" | ||
417 | + backend_defaults = "auto" | ||
418 | logical_block_size = 512 (512 B) | ||
419 | physical_block_size = 512 (512 B) | ||
420 | min_io_size = 0 (0 B) | ||
74 | -- | 421 | -- |
75 | 2.31.1 | 422 | 2.31.1 |
76 | 423 | diff view generated by jsdifflib |
1 | From: Paolo Bonzini <pbonzini@redhat.com> | 1 | From: Akihiko Odaki <akihiko.odaki@gmail.com> |
---|---|---|---|
2 | 2 | ||
3 | All callers of qemu_co_sleep_wake are checking whether they are passing | 3 | Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> |
4 | a NULL argument inside the pointer-to-pointer: do the check in | 4 | Message-id: 20210705130458.97642-3-akihiko.odaki@gmail.com |
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> | 5 | Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
17 | --- | 6 | --- |
18 | block/block-copy.c | 4 +--- | 7 | block/io.c | 2 ++ |
19 | block/nbd.c | 8 ++------ | 8 | 1 file changed, 2 insertions(+) |
20 | util/qemu-coroutine-sleep.c | 21 ++++++++++++--------- | ||
21 | 3 files changed, 15 insertions(+), 18 deletions(-) | ||
22 | 9 | ||
23 | diff --git a/block/block-copy.c b/block/block-copy.c | 10 | diff --git a/block/io.c b/block/io.c |
24 | index XXXXXXX..XXXXXXX 100644 | 11 | index XXXXXXX..XXXXXXX 100644 |
25 | --- a/block/block-copy.c | 12 | --- a/block/io.c |
26 | +++ b/block/block-copy.c | 13 | +++ b/block/io.c |
27 | @@ -XXX,XX +XXX,XX @@ out: | 14 | @@ -XXX,XX +XXX,XX @@ void bdrv_parent_drained_begin_single(BdrvChild *c, bool poll) |
28 | 15 | ||
29 | void block_copy_kick(BlockCopyCallState *call_state) | 16 | static void bdrv_merge_limits(BlockLimits *dst, const BlockLimits *src) |
30 | { | 17 | { |
31 | - if (call_state->sleep_state) { | 18 | + dst->pdiscard_alignment = MAX(dst->pdiscard_alignment, |
32 | - qemu_co_sleep_wake(call_state->sleep_state); | 19 | + src->pdiscard_alignment); |
33 | - } | 20 | dst->opt_transfer = MAX(dst->opt_transfer, src->opt_transfer); |
34 | + qemu_co_sleep_wake(call_state->sleep_state); | 21 | dst->max_transfer = MIN_NON_ZERO(dst->max_transfer, src->max_transfer); |
35 | } | 22 | dst->max_hw_transfer = MIN_NON_ZERO(dst->max_hw_transfer, |
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 | -- | 23 | -- |
109 | 2.31.1 | 24 | 2.31.1 |
110 | 25 | diff view generated by jsdifflib |