|
| 1 | +From 52599d99e735341759f86b69a28f314aaeb61229 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Stefan Hajnoczi <stefanha@redhat.com> |
| 3 | +Date: Wed, 14 Apr 2021 21:02:46 +0100 |
| 4 | +Subject: [PATCH 2/7] util/async: add a human-readable name to BHs for |
| 5 | + debugging |
| 6 | +MIME-Version: 1.0 |
| 7 | +Content-Type: text/plain; charset=UTF-8 |
| 8 | +Content-Transfer-Encoding: 8bit |
| 9 | +Content-Type: text/plain; charset = "utf-8" |
| 10 | +Content-Transfert-Encoding: 8bit |
| 11 | + |
| 12 | +It can be difficult to debug issues with BHs in production environments. |
| 13 | +Although BHs can usually be identified by looking up their ->cb() |
| 14 | +function pointer, this requires debug information for the program. It is |
| 15 | +also not possible to print human-readable diagnostics about BHs because |
| 16 | +they have no identifier. |
| 17 | + |
| 18 | +This patch adds a name to each BH. The name is not unique per instance |
| 19 | +but differentiates between cb() functions, which is usually enough. It's |
| 20 | +done by changing aio_bh_new() and friends to macros that stringify cb. |
| 21 | + |
| 22 | +The next patch will use the name field when reporting leaked BHs. |
| 23 | + |
| 24 | +Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> |
| 25 | +Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> |
| 26 | +Message-Id: <20210414200247.917496-2-stefanha@redhat.com> |
| 27 | + |
| 28 | + |
| 29 | +XCP-ng backport |
| 30 | +Backported-by: Thierry Escande <thierry.escande@vates.tech> |
| 31 | +Origin: https://gitlab.com/qemu-project/qemu/-/commit/0f08586c7171757d77c27ee6c606e8a1c44ac6e3 |
| 32 | +Notes: |
| 33 | +- Minor patch offset changes |
| 34 | +--- |
| 35 | + include/block/aio.h | 31 ++++++++++++++++++++++++++++--- |
| 36 | + include/qemu/main-loop.h | 4 +++- |
| 37 | + tests/ptimer-test-stubs.c | 2 +- |
| 38 | + util/async.c | 9 +++++++-- |
| 39 | + util/main-loop.c | 4 ++-- |
| 40 | + 5 files changed, 41 insertions(+), 9 deletions(-) |
| 41 | + |
| 42 | +diff --git a/include/block/aio.h b/include/block/aio.h |
| 43 | +index 6b0d52f..2d8dc2e 100644 |
| 44 | +--- a/include/block/aio.h |
| 45 | ++++ b/include/block/aio.h |
| 46 | +@@ -190,20 +190,45 @@ void aio_context_acquire(AioContext *ctx); |
| 47 | + /* Relinquish ownership of the AioContext. */ |
| 48 | + void aio_context_release(AioContext *ctx); |
| 49 | + |
| 50 | ++/** |
| 51 | ++ * aio_bh_schedule_oneshot_full: Allocate a new bottom half structure that will |
| 52 | ++ * run only once and as soon as possible. |
| 53 | ++ * |
| 54 | ++ * @name: A human-readable identifier for debugging purposes. |
| 55 | ++ */ |
| 56 | ++void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque, |
| 57 | ++ const char *name); |
| 58 | ++ |
| 59 | + /** |
| 60 | + * aio_bh_schedule_oneshot: Allocate a new bottom half structure that will run |
| 61 | + * only once and as soon as possible. |
| 62 | ++ * |
| 63 | ++ * A convenience wrapper for aio_bh_schedule_oneshot_full() that uses cb as the |
| 64 | ++ * name string. |
| 65 | + */ |
| 66 | +-void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque); |
| 67 | ++#define aio_bh_schedule_oneshot(ctx, cb, opaque) \ |
| 68 | ++ aio_bh_schedule_oneshot_full((ctx), (cb), (opaque), (stringify(cb))) |
| 69 | + |
| 70 | + /** |
| 71 | +- * aio_bh_new: Allocate a new bottom half structure. |
| 72 | ++ * aio_bh_new_full: Allocate a new bottom half structure. |
| 73 | + * |
| 74 | + * Bottom halves are lightweight callbacks whose invocation is guaranteed |
| 75 | + * to be wait-free, thread-safe and signal-safe. The #QEMUBH structure |
| 76 | + * is opaque and must be allocated prior to its use. |
| 77 | ++ * |
| 78 | ++ * @name: A human-readable identifier for debugging purposes. |
| 79 | ++ */ |
| 80 | ++QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque, |
| 81 | ++ const char *name); |
| 82 | ++ |
| 83 | ++/** |
| 84 | ++ * aio_bh_new: Allocate a new bottom half structure |
| 85 | ++ * |
| 86 | ++ * A convenience wrapper for aio_bh_new_full() that uses the cb as the name |
| 87 | ++ * string. |
| 88 | + */ |
| 89 | +-QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque); |
| 90 | ++#define aio_bh_new(ctx, cb, opaque) \ |
| 91 | ++ aio_bh_new_full((ctx), (cb), (opaque), (stringify(cb))) |
| 92 | + |
| 93 | + /** |
| 94 | + * aio_notify: Force processing of pending events. |
| 95 | +diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h |
| 96 | +index f6ba78e..b131a0e 100644 |
| 97 | +--- a/include/qemu/main-loop.h |
| 98 | ++++ b/include/qemu/main-loop.h |
| 99 | +@@ -299,7 +299,9 @@ void qemu_mutex_unlock_iothread(void); |
| 100 | + |
| 101 | + void qemu_fd_register(int fd); |
| 102 | + |
| 103 | +-QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque); |
| 104 | ++#define qemu_bh_new(cb, opaque) \ |
| 105 | ++ qemu_bh_new_full((cb), (opaque), (stringify(cb))) |
| 106 | ++QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name); |
| 107 | + void qemu_bh_schedule_idle(QEMUBH *bh); |
| 108 | + |
| 109 | + enum { |
| 110 | +diff --git a/tests/ptimer-test-stubs.c b/tests/ptimer-test-stubs.c |
| 111 | +index ed393d9..d82970c 100644 |
| 112 | +--- a/tests/ptimer-test-stubs.c |
| 113 | ++++ b/tests/ptimer-test-stubs.c |
| 114 | +@@ -107,7 +107,7 @@ int64_t qemu_clock_deadline_ns_all(QEMUClockType type, int attr_mask) |
| 115 | + return deadline; |
| 116 | + } |
| 117 | + |
| 118 | +-QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque) |
| 119 | ++QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name) |
| 120 | + { |
| 121 | + QEMUBH *bh = g_new(QEMUBH, 1); |
| 122 | + |
| 123 | +diff --git a/util/async.c b/util/async.c |
| 124 | +index b1fa531..a9262f5 100644 |
| 125 | +--- a/util/async.c |
| 126 | ++++ b/util/async.c |
| 127 | +@@ -38,6 +38,7 @@ |
| 128 | + |
| 129 | + struct QEMUBH { |
| 130 | + AioContext *ctx; |
| 131 | ++ const char *name; |
| 132 | + QEMUBHFunc *cb; |
| 133 | + void *opaque; |
| 134 | + QEMUBH *next; |
| 135 | +@@ -46,7 +47,8 @@ struct QEMUBH { |
| 136 | + bool deleted; |
| 137 | + }; |
| 138 | + |
| 139 | +-void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque) |
| 140 | ++void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb, |
| 141 | ++ void *opaque, const char *name) |
| 142 | + { |
| 143 | + QEMUBH *bh; |
| 144 | + bh = g_new(QEMUBH, 1); |
| 145 | +@@ -54,6 +56,7 @@ void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque) |
| 146 | + .ctx = ctx, |
| 147 | + .cb = cb, |
| 148 | + .opaque = opaque, |
| 149 | ++ .name = name, |
| 150 | + }; |
| 151 | + qemu_lockcnt_lock(&ctx->list_lock); |
| 152 | + bh->next = ctx->first_bh; |
| 153 | +@@ -66,7 +69,8 @@ void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque) |
| 154 | + aio_notify(ctx); |
| 155 | + } |
| 156 | + |
| 157 | +-QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque) |
| 158 | ++QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque, |
| 159 | ++ const char *name) |
| 160 | + { |
| 161 | + QEMUBH *bh; |
| 162 | + bh = g_new(QEMUBH, 1); |
| 163 | +@@ -74,6 +78,7 @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque) |
| 164 | + .ctx = ctx, |
| 165 | + .cb = cb, |
| 166 | + .opaque = opaque, |
| 167 | ++ .name = name, |
| 168 | + }; |
| 169 | + qemu_lockcnt_lock(&ctx->list_lock); |
| 170 | + bh->next = ctx->first_bh; |
| 171 | +diff --git a/util/main-loop.c b/util/main-loop.c |
| 172 | +index eda63fe..04d0474 100644 |
| 173 | +--- a/util/main-loop.c |
| 174 | ++++ b/util/main-loop.c |
| 175 | +@@ -527,9 +527,9 @@ void main_loop_wait(int nonblocking) |
| 176 | + |
| 177 | + /* Functions to operate on the main QEMU AioContext. */ |
| 178 | + |
| 179 | +-QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque) |
| 180 | ++QEMUBH *qemu_bh_new_full(QEMUBHFunc *cb, void *opaque, const char *name) |
| 181 | + { |
| 182 | +- return aio_bh_new(qemu_aio_context, cb, opaque); |
| 183 | ++ return aio_bh_new_full(qemu_aio_context, cb, opaque, name); |
| 184 | + } |
| 185 | + |
| 186 | + /* |
| 187 | +-- |
| 188 | +2.51.0 |
| 189 | + |
0 commit comments