From 0f08586c7171757d77c27ee6c606e8a1c44ac6e3 Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 14 Apr 2021 21:02:46 +0100 Subject: util/async: add a human-readable name to BHs for debugging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It can be difficult to debug issues with BHs in production environments. Although BHs can usually be identified by looking up their ->cb() function pointer, this requires debug information for the program. It is also not possible to print human-readable diagnostics about BHs because they have no identifier. This patch adds a name to each BH. The name is not unique per instance but differentiates between cb() functions, which is usually enough. It's done by changing aio_bh_new() and friends to macros that stringify cb. The next patch will use the name field when reporting leaked BHs. Signed-off-by: Stefan Hajnoczi Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20210414200247.917496-2-stefanha@redhat.com> --- util/async.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'util/async.c') diff --git a/util/async.c b/util/async.c index 5d9b7cc1eb..9a668996b8 100644 --- a/util/async.c +++ b/util/async.c @@ -57,6 +57,7 @@ enum { struct QEMUBH { AioContext *ctx; + const char *name; QEMUBHFunc *cb; void *opaque; QSLIST_ENTRY(QEMUBH) next; @@ -107,7 +108,8 @@ static QEMUBH *aio_bh_dequeue(BHList *head, unsigned *flags) return bh; } -void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque) +void aio_bh_schedule_oneshot_full(AioContext *ctx, QEMUBHFunc *cb, + void *opaque, const char *name) { QEMUBH *bh; bh = g_new(QEMUBH, 1); @@ -115,11 +117,13 @@ void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque) .ctx = ctx, .cb = cb, .opaque = opaque, + .name = name, }; aio_bh_enqueue(bh, BH_SCHEDULED | BH_ONESHOT); } -QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque) +QEMUBH *aio_bh_new_full(AioContext *ctx, QEMUBHFunc *cb, void *opaque, + const char *name) { QEMUBH *bh; bh = g_new(QEMUBH, 1); @@ -127,6 +131,7 @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque) .ctx = ctx, .cb = cb, .opaque = opaque, + .name = name, }; return bh; } -- cgit v1.2.3 From 023ca420ee3d4de76518d690afa98dcac33998ce Mon Sep 17 00:00:00 2001 From: Stefan Hajnoczi Date: Wed, 14 Apr 2021 21:02:47 +0100 Subject: util/async: print leaked BH name when AioContext finalizes BHs must be deleted before the AioContext is finalized. If not, it's a bug and probably indicates that some part of the program still expects the BH to run in the future. That can lead to memory leaks, inconsistent state, or just hangs. Unfortunately the assert(flags & BH_DELETED) call in aio_ctx_finalize() is difficult to debug because the assertion failure contains no information about the BH! Use the QEMUBH name field added in the previous patch to show a useful error when a leaked BH is detected. Suggested-by: Eric Ernst Signed-off-by: Stefan Hajnoczi Message-Id: <20210414200247.917496-3-stefanha@redhat.com> --- util/async.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) (limited to 'util/async.c') diff --git a/util/async.c b/util/async.c index 9a668996b8..9a41591319 100644 --- a/util/async.c +++ b/util/async.c @@ -344,8 +344,20 @@ aio_ctx_finalize(GSource *source) assert(QSIMPLEQ_EMPTY(&ctx->bh_slice_list)); while ((bh = aio_bh_dequeue(&ctx->bh_list, &flags))) { - /* qemu_bh_delete() must have been called on BHs in this AioContext */ - assert(flags & BH_DELETED); + /* + * qemu_bh_delete() must have been called on BHs in this AioContext. In + * many cases memory leaks, hangs, or inconsistent state occur when a + * BH is leaked because something still expects it to run. + * + * If you hit this, fix the lifecycle of the BH so that + * qemu_bh_delete() and any associated cleanup is called before the + * AioContext is finalized. + */ + if (unlikely(!(flags & BH_DELETED))) { + fprintf(stderr, "%s: BH '%s' leaked, aborting...\n", + __func__, bh->name); + abort(); + } g_free(bh); } -- cgit v1.2.3