From nobody Sun May 5 15:08:08 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1582135317; cv=none; d=zohomail.com; s=zohoarc; b=KJ3bFG+UN6voOIJzKWZgjkGO9tho3u3jloyO90n/NJXP5pfA96OSgE/SrgzWMpSQCXfJqoU9K5Vuh729gp70OX5PyVyjBU4NHqF6YgWleoIm8/Qzryton9Ww5vLlIQPSUupQxofpfJpYhxO0fDnjD0mvz+vJN4oxKDUqFUbOu+Y= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1582135317; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=deNTBYqUy33T+P3rGlVgVGPpYeM7lEAdm080kyCI4cc=; b=fMwERpriah4l16qJPqoYauZuzK2HkZMYms7WwdjHTVaiAeAh7LtYA3+vFrUYlWGezmZpW8BTfP0/WqR18rPPncGVEP7QhCSA+sZQB/we+totAG0LpgTkfy+S4WQwBP/Za0l4QGtykAS00qEtkiezEffR9m03/wy04LN5TWRPuHA= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=fail; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org; dmarc=fail header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1582135317601844.0827898440679; Wed, 19 Feb 2020 10:01:57 -0800 (PST) Received: from localhost ([::1]:57792 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1j4TfY-0002WK-Dt for importer@patchew.org; Wed, 19 Feb 2020 13:01:56 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:54241) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1j4TcC-0005qX-GD for qemu-devel@nongnu.org; Wed, 19 Feb 2020 12:58:30 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1j4TcA-00014O-A1 for qemu-devel@nongnu.org; Wed, 19 Feb 2020 12:58:28 -0500 Received: from us-smtp-2.mimecast.com ([207.211.31.81]:58894 helo=us-smtp-delivery-1.mimecast.com) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1j4TcA-000143-5N for qemu-devel@nongnu.org; Wed, 19 Feb 2020 12:58:26 -0500 Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-425-vPKJgjFPNwiNoH7uwWodHQ-1; Wed, 19 Feb 2020 12:58:20 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 9408C8017CC; Wed, 19 Feb 2020 17:58:19 +0000 (UTC) Received: from localhost (unknown [10.36.118.184]) by smtp.corp.redhat.com (Postfix) with ESMTP id 4AEC75C651; Wed, 19 Feb 2020 17:58:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1582135105; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=deNTBYqUy33T+P3rGlVgVGPpYeM7lEAdm080kyCI4cc=; b=AQrVlVCWGkdbu+YqLCOPSdCJBZ3U7W2DkxbsZ28OwvCDKnN84+7rvzs2+68YenMjwgG3sM /9IEcKEd0+Wwr5OhmImXNogvuSZ4IsbOdcc3ULxJM0hs0gwqKcwac9mUBmtwTO2l0ivp9o gI4V16CckV50VrwBNFUz00iSoRUsiiA= From: Stefan Hajnoczi To: qemu-devel@nongnu.org Subject: [PATCH v2] util/async: make bh_aio_poll() O(1) Date: Wed, 19 Feb 2020 17:53:48 +0000 Message-Id: <20200219175348.1161536-1-stefanha@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-MC-Unique: vPKJgjFPNwiNoH7uwWodHQ-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 207.211.31.81 X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Kevin Wolf , Fam Zheng , qemu-block@nongnu.org, Max Reitz , Stefan Hajnoczi , Paolo Bonzini Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail-DKIM: fail (Header signature does not verify) Content-Type: text/plain; charset="utf-8" The ctx->first_bh list contains all created BHs, including those that are not scheduled. The list is iterated by the event loop and therefore has O(n) time complexity with respected to the number of created BHs. Rewrite BHs so that only scheduled or deleted BHs are enqueued. Only BHs that actually require action will be iterated. One semantic change is required: qemu_bh_delete() enqueues the BH and therefore invokes aio_notify(). The tests/test-aio.c:test_source_bh_delete_from_cb() test case assumed that g_main_context_iteration(NULL, false) returns false after qemu_bh_delete() but it now returns true for one iteration. Fix up the test case. This patch makes aio_compute_timeout() and aio_bh_poll() drop from a CPU profile reported by perf-top(1). Previously they combined to 9% CPU utilization when AioContext polling is commented out and the guest has 2 virtio-blk,num-queues=3D1 and 99 virtio-blk,num-queues=3D32 devices. Signed-off-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini --- v2: * Use QSLIST for BHs and QSIMPLEQ for BHListSlices [Paolo] (Note that I replaced bh =3D atomic_rcu_read(&first_bh) with QSLIST_FOREACH(&bh_list) so there is no memory ordering but I think this is safe.) * Comment clarifications [Paolo] --- include/block/aio.h | 20 +++- tests/test-aio.c | 3 +- util/async.c | 236 +++++++++++++++++++++++++------------------- 3 files changed, 157 insertions(+), 102 deletions(-) diff --git a/include/block/aio.h b/include/block/aio.h index 7ba9bd7874..1a2ce9ca26 100644 --- a/include/block/aio.h +++ b/include/block/aio.h @@ -51,6 +51,19 @@ struct ThreadPool; struct LinuxAioState; struct LuringState; =20 +/* + * Each aio_bh_poll() call carves off a slice of the BH list, so that newly + * scheduled BHs are not processed until the next aio_bh_poll() call. All + * active aio_bh_poll() calls chain their slices together in a list, so th= at + * nested aio_bh_poll() calls process all scheduled bottom halves. + */ +typedef QSLIST_HEAD(, QEMUBH) BHList; +typedef struct BHListSlice BHListSlice; +struct BHListSlice { + BHList bh_list; + QSIMPLEQ_ENTRY(BHListSlice) next; +}; + struct AioContext { GSource source; =20 @@ -91,8 +104,11 @@ struct AioContext { */ QemuLockCnt list_lock; =20 - /* Anchor of the list of Bottom Halves belonging to the context */ - struct QEMUBH *first_bh; + /* Bottom Halves pending aio_bh_poll() processing */ + BHList bh_list; + + /* Chained BH list slices for each nested aio_bh_poll() call */ + QSIMPLEQ_HEAD(, BHListSlice) bh_slice_list; =20 /* Used by aio_notify. * diff --git a/tests/test-aio.c b/tests/test-aio.c index 86fb73b3d5..8a46078463 100644 --- a/tests/test-aio.c +++ b/tests/test-aio.c @@ -615,7 +615,8 @@ static void test_source_bh_delete_from_cb(void) g_assert_cmpint(data1.n, =3D=3D, data1.max); g_assert(data1.bh =3D=3D NULL); =20 - g_assert(!g_main_context_iteration(NULL, false)); + assert(g_main_context_iteration(NULL, false)); + assert(!g_main_context_iteration(NULL, false)); } =20 static void test_source_bh_delete_from_cb_many(void) diff --git a/util/async.c b/util/async.c index c192a24a61..2c8499cc29 100644 --- a/util/async.c +++ b/util/async.c @@ -36,16 +36,76 @@ /***********************************************************/ /* bottom halves (can be seen as timers which expire ASAP) */ =20 +/* QEMUBH::flags values */ +enum { + /* Already enqueued and waiting for aio_bh_poll() */ + BH_PENDING =3D (1 << 0), + + /* Invoke the callback */ + BH_SCHEDULED =3D (1 << 1), + + /* Delete without invoking callback */ + BH_DELETED =3D (1 << 2), + + /* Delete after invoking callback */ + BH_ONESHOT =3D (1 << 3), + + /* Schedule periodically when the event loop is idle */ + BH_IDLE =3D (1 << 4), +}; + struct QEMUBH { AioContext *ctx; QEMUBHFunc *cb; void *opaque; - QEMUBH *next; - bool scheduled; - bool idle; - bool deleted; + QSLIST_ENTRY(QEMUBH) next; + unsigned flags; }; =20 +/* Called concurrently from any thread */ +static void aio_bh_enqueue(QEMUBH *bh, unsigned new_flags) +{ + AioContext *ctx =3D bh->ctx; + unsigned old_flags; + + /* + * The memory barrier implicit in atomic_fetch_or makes sure that: + * 1. idle & any writes needed by the callback are done before the + * locations are read in the aio_bh_poll. + * 2. ctx is loaded before the callback has a chance to execute and bh + * could be freed. + */ + old_flags =3D atomic_fetch_or(&bh->flags, BH_PENDING | new_flags); + if (!(old_flags & BH_PENDING)) { + QSLIST_INSERT_HEAD_ATOMIC(&ctx->bh_list, bh, next); + } + + aio_notify(ctx); +} + +/* Only called from aio_bh_poll() and aio_ctx_finalize() */ +static QEMUBH *aio_bh_dequeue(BHList *head, unsigned *flags) +{ + QEMUBH *bh =3D QSLIST_FIRST(head); + + if (!bh) { + return NULL; + } + + QSLIST_REMOVE_HEAD(head, next); + + /* + * The atomic_and is paired with aio_bh_enqueue(). The implicit memory + * barrier ensures that the callback sees all writes done by the sched= uling + * thread. It also ensures that the scheduling thread sees the cleared + * flag before bh->cb has run, and thus will call aio_notify again if + * necessary. + */ + *flags =3D atomic_fetch_and(&bh->flags, + ~(BH_PENDING | BH_SCHEDULED | BH_IDLE)); + return bh; +} + void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFunc *cb, void *opaque) { QEMUBH *bh; @@ -55,15 +115,7 @@ void aio_bh_schedule_oneshot(AioContext *ctx, QEMUBHFun= c *cb, void *opaque) .cb =3D cb, .opaque =3D opaque, }; - qemu_lockcnt_lock(&ctx->list_lock); - bh->next =3D ctx->first_bh; - bh->scheduled =3D 1; - bh->deleted =3D 1; - /* Make sure that the members are ready before putting bh into list */ - smp_wmb(); - ctx->first_bh =3D bh; - qemu_lockcnt_unlock(&ctx->list_lock); - aio_notify(ctx); + aio_bh_enqueue(bh, BH_SCHEDULED | BH_ONESHOT); } =20 QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, void *opaque) @@ -75,12 +127,6 @@ QEMUBH *aio_bh_new(AioContext *ctx, QEMUBHFunc *cb, voi= d *opaque) .cb =3D cb, .opaque =3D opaque, }; - qemu_lockcnt_lock(&ctx->list_lock); - bh->next =3D ctx->first_bh; - /* Make sure that the members are ready before putting bh into list */ - smp_wmb(); - ctx->first_bh =3D bh; - qemu_lockcnt_unlock(&ctx->list_lock); return bh; } =20 @@ -89,91 +135,56 @@ void aio_bh_call(QEMUBH *bh) bh->cb(bh->opaque); } =20 -/* Multiple occurrences of aio_bh_poll cannot be called concurrently. - * The count in ctx->list_lock is incremented before the call, and is - * not affected by the call. - */ +/* Multiple occurrences of aio_bh_poll cannot be called concurrently. */ int aio_bh_poll(AioContext *ctx) { - QEMUBH *bh, **bhp, *next; - int ret; - bool deleted =3D false; - - ret =3D 0; - for (bh =3D atomic_rcu_read(&ctx->first_bh); bh; bh =3D next) { - next =3D atomic_rcu_read(&bh->next); - /* The atomic_xchg is paired with the one in qemu_bh_schedule. The - * implicit memory barrier ensures that the callback sees all writ= es - * done by the scheduling thread. It also ensures that the schedu= ling - * thread sees the zero before bh->cb has run, and thus will call - * aio_notify again if necessary. - */ - if (atomic_xchg(&bh->scheduled, 0)) { + BHListSlice slice; + BHListSlice *s; + int ret =3D 0; + + QSLIST_MOVE_ATOMIC(&slice.bh_list, &ctx->bh_list); + QSIMPLEQ_INSERT_TAIL(&ctx->bh_slice_list, &slice, next); + + while ((s =3D QSIMPLEQ_FIRST(&ctx->bh_slice_list))) { + QEMUBH *bh; + unsigned flags; + + bh =3D aio_bh_dequeue(&s->bh_list, &flags); + if (!bh) { + QSIMPLEQ_REMOVE_HEAD(&ctx->bh_slice_list, next); + continue; + } + + if ((flags & (BH_SCHEDULED | BH_DELETED)) =3D=3D BH_SCHEDULED) { /* Idle BHs don't count as progress */ - if (!bh->idle) { + if (!(flags & BH_IDLE)) { ret =3D 1; } - bh->idle =3D 0; aio_bh_call(bh); } - if (bh->deleted) { - deleted =3D true; + if (flags & (BH_DELETED | BH_ONESHOT)) { + g_free(bh); } } =20 - /* remove deleted bhs */ - if (!deleted) { - return ret; - } - - if (qemu_lockcnt_dec_if_lock(&ctx->list_lock)) { - bhp =3D &ctx->first_bh; - while (*bhp) { - bh =3D *bhp; - if (bh->deleted && !bh->scheduled) { - *bhp =3D bh->next; - g_free(bh); - } else { - bhp =3D &bh->next; - } - } - qemu_lockcnt_inc_and_unlock(&ctx->list_lock); - } return ret; } =20 void qemu_bh_schedule_idle(QEMUBH *bh) { - bh->idle =3D 1; - /* Make sure that idle & any writes needed by the callback are done - * before the locations are read in the aio_bh_poll. - */ - atomic_mb_set(&bh->scheduled, 1); + aio_bh_enqueue(bh, BH_SCHEDULED | BH_IDLE); } =20 void qemu_bh_schedule(QEMUBH *bh) { - AioContext *ctx; - - ctx =3D bh->ctx; - bh->idle =3D 0; - /* The memory barrier implicit in atomic_xchg makes sure that: - * 1. idle & any writes needed by the callback are done before the - * locations are read in the aio_bh_poll. - * 2. ctx is loaded before scheduled is set and the callback has a cha= nce - * to execute. - */ - if (atomic_xchg(&bh->scheduled, 1) =3D=3D 0) { - aio_notify(ctx); - } + aio_bh_enqueue(bh, BH_SCHEDULED); } =20 - /* This func is async. */ void qemu_bh_cancel(QEMUBH *bh) { - atomic_mb_set(&bh->scheduled, 0); + atomic_and(&bh->flags, ~BH_SCHEDULED); } =20 /* This func is async.The bottom half will do the delete action at the fin= ial @@ -181,21 +192,16 @@ void qemu_bh_cancel(QEMUBH *bh) */ void qemu_bh_delete(QEMUBH *bh) { - bh->scheduled =3D 0; - bh->deleted =3D 1; + aio_bh_enqueue(bh, BH_DELETED); } =20 -int64_t -aio_compute_timeout(AioContext *ctx) +static int64_t aio_compute_bh_timeout(BHList *head, int timeout) { - int64_t deadline; - int timeout =3D -1; QEMUBH *bh; =20 - for (bh =3D atomic_rcu_read(&ctx->first_bh); bh; - bh =3D atomic_rcu_read(&bh->next)) { - if (bh->scheduled) { - if (bh->idle) { + QSLIST_FOREACH(bh, head, next) { + if ((bh->flags & (BH_SCHEDULED | BH_DELETED)) =3D=3D BH_SCHEDULED)= { + if (bh->flags & BH_IDLE) { /* idle bottom halves will be polled at least * every 10ms */ timeout =3D 10000000; @@ -207,6 +213,28 @@ aio_compute_timeout(AioContext *ctx) } } =20 + return timeout; +} + +int64_t +aio_compute_timeout(AioContext *ctx) +{ + BHListSlice *s; + int64_t deadline; + int timeout =3D -1; + + timeout =3D aio_compute_bh_timeout(&ctx->bh_list, timeout); + if (timeout =3D=3D 0) { + return 0; + } + + QSIMPLEQ_FOREACH(s, &ctx->bh_slice_list, next) { + timeout =3D aio_compute_bh_timeout(&s->bh_list, timeout); + if (timeout =3D=3D 0) { + return 0; + } + } + deadline =3D timerlistgroup_deadline_ns(&ctx->tlg); if (deadline =3D=3D 0) { return 0; @@ -237,15 +265,24 @@ aio_ctx_check(GSource *source) { AioContext *ctx =3D (AioContext *) source; QEMUBH *bh; + BHListSlice *s; =20 atomic_and(&ctx->notify_me, ~1); aio_notify_accept(ctx); =20 - for (bh =3D ctx->first_bh; bh; bh =3D bh->next) { - if (bh->scheduled) { + QSLIST_FOREACH(bh, &ctx->bh_list, next) { + if ((bh->flags & (BH_SCHEDULED | BH_DELETED)) =3D=3D BH_SCHEDULED)= { return true; } } + + QSIMPLEQ_FOREACH(s, &ctx->bh_slice_list, next) { + QSLIST_FOREACH(bh, &s->bh_list, next) { + if ((bh->flags & (BH_SCHEDULED | BH_DELETED)) =3D=3D BH_SCHEDU= LED) { + return true; + } + } + } return aio_pending(ctx) || (timerlistgroup_deadline_ns(&ctx->tlg) =3D= =3D 0); } =20 @@ -265,6 +302,8 @@ static void aio_ctx_finalize(GSource *source) { AioContext *ctx =3D (AioContext *) source; + QEMUBH *bh; + unsigned flags; =20 thread_pool_free(ctx->thread_pool); =20 @@ -287,18 +326,15 @@ aio_ctx_finalize(GSource *source) assert(QSLIST_EMPTY(&ctx->scheduled_coroutines)); qemu_bh_delete(ctx->co_schedule_bh); =20 - qemu_lockcnt_lock(&ctx->list_lock); - assert(!qemu_lockcnt_count(&ctx->list_lock)); - while (ctx->first_bh) { - QEMUBH *next =3D ctx->first_bh->next; + /* There must be no aio_bh_poll() calls going on */ + assert(QSIMPLEQ_EMPTY(&ctx->bh_slice_list)); =20 + while ((bh =3D aio_bh_dequeue(&ctx->bh_list, &flags))) { /* qemu_bh_delete() must have been called on BHs in this AioContex= t */ - assert(ctx->first_bh->deleted); + assert(flags & BH_DELETED); =20 - g_free(ctx->first_bh); - ctx->first_bh =3D next; + g_free(bh); } - qemu_lockcnt_unlock(&ctx->list_lock); =20 aio_set_event_notifier(ctx, &ctx->notifier, false, NULL, NULL); event_notifier_cleanup(&ctx->notifier); @@ -445,6 +481,8 @@ AioContext *aio_context_new(Error **errp) AioContext *ctx; =20 ctx =3D (AioContext *) g_source_new(&aio_source_funcs, sizeof(AioConte= xt)); + QSLIST_INIT(&ctx->bh_list); + QSIMPLEQ_INIT(&ctx->bh_slice_list); aio_context_setup(ctx); =20 ret =3D event_notifier_init(&ctx->notifier, false); --=20 2.24.1