From nobody Sun Apr 28 18:41:59 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1511884146889213.27037191614136; Tue, 28 Nov 2017 07:49:06 -0800 (PST) Received: from localhost ([::1]:38549 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJi89-0008U4-N9 for importer@patchew.org; Tue, 28 Nov 2017 10:49:05 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33836) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJi3e-0004yo-34 for qemu-devel@nongnu.org; Tue, 28 Nov 2017 10:44:27 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eJi3Z-0005Ue-He for qemu-devel@nongnu.org; Tue, 28 Nov 2017 10:44:26 -0500 Received: from mx1.redhat.com ([209.132.183.28]:28500) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eJi3Q-0005SI-KY; Tue, 28 Nov 2017 10:44:12 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id B4E6976521; Tue, 28 Nov 2017 15:44:11 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-201.ams2.redhat.com [10.36.117.201]) by smtp.corp.redhat.com (Postfix) with ESMTP id C595A51DE6; Tue, 28 Nov 2017 15:44:09 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 28 Nov 2017 16:43:47 +0100 Message-Id: <20171128154350.21504-2-kwolf@redhat.com> In-Reply-To: <20171128154350.21504-1-kwolf@redhat.com> References: <20171128154350.21504-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.26]); Tue, 28 Nov 2017 15:44:11 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH for-2.11 1/4] Revert "coroutine: abort if we try to schedule or enter a pending coroutine" X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" This reverts commit 6133b39f3c36623425a6ede9e89d93175fde15cd. The commit checked conditions that would expose a bug, but there is no real reason to forbid them apart from the bug, which we'll fix in a minute. In particular, reentering a coroutine during co_aio_sleep_ns() is fine; the function is explicitly written to allow this. aio_co_schedule() can indeed conflict with direct coroutine invocations, but this is exactky what we want to fix, so remove that check again, too. Signed-off-by: Kevin Wolf Reviewed-by: Jeff Cody Reviewed-by: Stefan Hajnoczi Tested-by: Jeff Cody --- include/qemu/coroutine_int.h | 13 +++---------- util/async.c | 13 ------------- util/qemu-coroutine-sleep.c | 12 ------------ util/qemu-coroutine.c | 14 -------------- 4 files changed, 3 insertions(+), 49 deletions(-) diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h index 59e8406398..cb98892bba 100644 --- a/include/qemu/coroutine_int.h +++ b/include/qemu/coroutine_int.h @@ -46,21 +46,14 @@ struct Coroutine { =20 size_t locks_held; =20 - /* Only used when the coroutine has yielded. */ - AioContext *ctx; - - /* Used to catch and abort on illegal co-routine entry. - * Will contain the name of the function that had first - * scheduled the coroutine. */ - const char *scheduled; - - QSIMPLEQ_ENTRY(Coroutine) co_queue_next; - /* Coroutines that should be woken up when we yield or terminate. * Only used when the coroutine is running. */ QSIMPLEQ_HEAD(, Coroutine) co_queue_wakeup; =20 + /* Only used when the coroutine has yielded. */ + AioContext *ctx; + QSIMPLEQ_ENTRY(Coroutine) co_queue_next; QSLIST_ENTRY(Coroutine) co_scheduled_next; }; =20 diff --git a/util/async.c b/util/async.c index 4dd9d95a9e..0e1bd8780a 100644 --- a/util/async.c +++ b/util/async.c @@ -388,9 +388,6 @@ static void co_schedule_bh_cb(void *opaque) QSLIST_REMOVE_HEAD(&straight, co_scheduled_next); trace_aio_co_schedule_bh_cb(ctx, co); aio_context_acquire(ctx); - - /* Protected by write barrier in qemu_aio_coroutine_enter */ - atomic_set(&co->scheduled, NULL); qemu_coroutine_enter(co); aio_context_release(ctx); } @@ -441,16 +438,6 @@ fail: void aio_co_schedule(AioContext *ctx, Coroutine *co) { trace_aio_co_schedule(ctx, co); - const char *scheduled =3D atomic_cmpxchg(&co->scheduled, NULL, - __func__); - - if (scheduled) { - fprintf(stderr, - "%s: Co-routine was already scheduled in '%s'\n", - __func__, scheduled); - abort(); - } - QSLIST_INSERT_HEAD_ATOMIC(&ctx->scheduled_coroutines, co, co_scheduled_next); qemu_bh_schedule(ctx->co_schedule_bh); diff --git a/util/qemu-coroutine-sleep.c b/util/qemu-coroutine-sleep.c index 254349cdbb..9c5655041b 100644 --- a/util/qemu-coroutine-sleep.c +++ b/util/qemu-coroutine-sleep.c @@ -13,7 +13,6 @@ =20 #include "qemu/osdep.h" #include "qemu/coroutine.h" -#include "qemu/coroutine_int.h" #include "qemu/timer.h" #include "block/aio.h" =20 @@ -26,8 +25,6 @@ static void co_sleep_cb(void *opaque) { CoSleepCB *sleep_cb =3D opaque; =20 - /* Write of schedule protected by barrier write in aio_co_schedule */ - atomic_set(&sleep_cb->co->scheduled, NULL); aio_co_wake(sleep_cb->co); } =20 @@ -37,15 +34,6 @@ void coroutine_fn co_aio_sleep_ns(AioContext *ctx, QEMUC= lockType type, CoSleepCB sleep_cb =3D { .co =3D qemu_coroutine_self(), }; - - const char *scheduled =3D atomic_cmpxchg(&sleep_cb.co->scheduled, NULL, - __func__); - if (scheduled) { - fprintf(stderr, - "%s: Co-routine was already scheduled in '%s'\n", - __func__, scheduled); - abort(); - } sleep_cb.ts =3D aio_timer_new(ctx, type, SCALE_NS, co_sleep_cb, &sleep= _cb); timer_mod(sleep_cb.ts, qemu_clock_get_ns(type) + ns); qemu_coroutine_yield(); diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c index 9eff7fd450..d6095c1d5a 100644 --- a/util/qemu-coroutine.c +++ b/util/qemu-coroutine.c @@ -107,22 +107,8 @@ void qemu_aio_coroutine_enter(AioContext *ctx, Corouti= ne *co) Coroutine *self =3D qemu_coroutine_self(); CoroutineAction ret; =20 - /* Cannot rely on the read barrier for co in aio_co_wake(), as there a= re - * callers outside of aio_co_wake() */ - const char *scheduled =3D atomic_mb_read(&co->scheduled); - trace_qemu_aio_coroutine_enter(ctx, self, co, co->entry_arg); =20 - /* if the Coroutine has already been scheduled, entering it again will - * cause us to enter it twice, potentially even after the coroutine has - * been deleted */ - if (scheduled) { - fprintf(stderr, - "%s: Co-routine was already scheduled in '%s'\n", - __func__, scheduled); - abort(); - } - if (co->caller) { fprintf(stderr, "Co-routine re-entered recursively\n"); abort(); --=20 2.13.6 From nobody Sun Apr 28 18:41:59 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1511884195187745.4662480033504; Tue, 28 Nov 2017 07:49:55 -0800 (PST) Received: from localhost ([::1]:38551 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJi8n-0000YB-0c for importer@patchew.org; Tue, 28 Nov 2017 10:49:45 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33895) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJi3l-00055j-Fe for qemu-devel@nongnu.org; Tue, 28 Nov 2017 10:44:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eJi3f-0005Xv-Op for qemu-devel@nongnu.org; Tue, 28 Nov 2017 10:44:33 -0500 Received: from mx1.redhat.com ([209.132.183.28]:37076) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eJi3U-0005Sh-1y; Tue, 28 Nov 2017 10:44:16 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 2AFE2356FB; Tue, 28 Nov 2017 15:44:15 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-201.ams2.redhat.com [10.36.117.201]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0A8B251DE6; Tue, 28 Nov 2017 15:44:11 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 28 Nov 2017 16:43:48 +0100 Message-Id: <20171128154350.21504-3-kwolf@redhat.com> In-Reply-To: <20171128154350.21504-1-kwolf@redhat.com> References: <20171128154350.21504-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Tue, 28 Nov 2017 15:44:15 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH for-2.11 2/4] Revert "blockjob: do not allow coroutine double entry or entry-after-completion" X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" This reverts commit 4afeffc8572f40d8844b946a30c00b10da4442b1. This fixed the symptom of a bug rather than the root cause. Waking up a sleeping coroutine is generally fine, we just need to make it work correctly across AioContexts, which we'll do in a minute. Signed-off-by: Kevin Wolf Reviewed-by: Jeff Cody Reviewed-by: Stefan Hajnoczi Tested-by: Jeff Cody --- include/block/blockjob_int.h | 3 +-- blockjob.c | 7 ++----- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/include/block/blockjob_int.h b/include/block/blockjob_int.h index 43f3be2965..f13ad05c0d 100644 --- a/include/block/blockjob_int.h +++ b/include/block/blockjob_int.h @@ -143,8 +143,7 @@ void *block_job_create(const char *job_id, const BlockJ= obDriver *driver, * @ns: How many nanoseconds to stop for. * * Put the job to sleep (assuming that it wasn't canceled) for @ns - * nanoseconds. Canceling the job will not interrupt the wait, so the - * cancel will not process until the coroutine wakes up. + * nanoseconds. Canceling the job will interrupt the wait immediately. */ void block_job_sleep_ns(BlockJob *job, QEMUClockType type, int64_t ns); =20 diff --git a/blockjob.c b/blockjob.c index ff9a614531..3a0c49137e 100644 --- a/blockjob.c +++ b/blockjob.c @@ -797,14 +797,11 @@ void block_job_sleep_ns(BlockJob *job, QEMUClockType = type, int64_t ns) return; } =20 - /* We need to leave job->busy set here, because when we have - * put a coroutine to 'sleep', we have scheduled it to run in - * the future. We cannot enter that same coroutine again before - * it wakes and runs, otherwise we risk double-entry or entry after - * completion. */ + job->busy =3D false; if (!block_job_should_pause(job)) { co_aio_sleep_ns(blk_get_aio_context(job->blk), type, ns); } + job->busy =3D true; =20 block_job_pause_point(job); } --=20 2.13.6 From nobody Sun Apr 28 18:41:59 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) by mx.zohomail.com with SMTPS id 1511884055282713.7270167325971; Tue, 28 Nov 2017 07:47:35 -0800 (PST) Received: from localhost ([::1]:38540 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJi6Z-0007Il-Lp for importer@patchew.org; Tue, 28 Nov 2017 10:47:27 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33865) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJi3g-00050x-Fr for qemu-devel@nongnu.org; Tue, 28 Nov 2017 10:44:29 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eJi3f-0005Xk-F0 for qemu-devel@nongnu.org; Tue, 28 Nov 2017 10:44:28 -0500 Received: from mx1.redhat.com ([209.132.183.28]:60202) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eJi3Y-0005Tt-TU; Tue, 28 Nov 2017 10:44:21 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 09B36C0587C0; Tue, 28 Nov 2017 15:44:20 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-201.ams2.redhat.com [10.36.117.201]) by smtp.corp.redhat.com (Postfix) with ESMTP id 794A75FCA0; Tue, 28 Nov 2017 15:44:15 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 28 Nov 2017 16:43:49 +0100 Message-Id: <20171128154350.21504-4-kwolf@redhat.com> In-Reply-To: <20171128154350.21504-1-kwolf@redhat.com> References: <20171128154350.21504-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.32]); Tue, 28 Nov 2017 15:44:20 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH for-2.11 3/4] coroutine: Cancel aio_co_schedule() on direct entry X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" If a coroutine can be reentered from multiple possible sources, we need to be careful in the case that two sources try to reenter it at the same time. There are two different cases where this can happen: 1. A coroutine spawns multiple asynchronous jobs and waits for all of them to complete. In this case, multiple reentries are expected and the coroutine is probably looping around a yield, so entering it twice is generally fine (but entering it just once after all jobs have completed would be enough, too). Exception: When the loop condition becomes false and the first reenter already leaves the loop, we must not do a second reenter. 2. A coroutine yields and provides multiple alternative ways to be reentered. In this case, we must always only process the first reenter. Direct entry is not a problem. It requires that the AioContext locks for the coroutine context are held, which means that the coroutine has enough time to set some state that simply prevents the second caller from reentering the coroutine, too. Things get more interesting with aio_co_schedule() because the coroutine may be scheduled before it is (directly) entered from a second place. Then changing some state inside the coroutine doesn't help because it is already scheduled and the state won't be checked again. For this case, we'll cancel any pending aio_co_schedule() when the coroutine is actually entered. Reentering it once is enough for all cases explained above, and a requirement for part of them. Signed-off-by: Kevin Wolf Reviewed-by: Stefan Hajnoczi Tested-by: Jeff Cody --- include/qemu/coroutine_int.h | 1 + util/async.c | 15 ++++++++++++++- util/qemu-coroutine.c | 17 +++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/include/qemu/coroutine_int.h b/include/qemu/coroutine_int.h index cb98892bba..73fc35e54b 100644 --- a/include/qemu/coroutine_int.h +++ b/include/qemu/coroutine_int.h @@ -40,6 +40,7 @@ struct Coroutine { CoroutineEntry *entry; void *entry_arg; Coroutine *caller; + AioContext *scheduled; =20 /* Only used when the coroutine has terminated. */ QSLIST_ENTRY(Coroutine) pool_next; diff --git a/util/async.c b/util/async.c index 0e1bd8780a..dc249e9404 100644 --- a/util/async.c +++ b/util/async.c @@ -372,6 +372,7 @@ static bool event_notifier_poll(void *opaque) static void co_schedule_bh_cb(void *opaque) { AioContext *ctx =3D opaque; + AioContext *scheduled_ctx; QSLIST_HEAD(, Coroutine) straight, reversed; =20 QSLIST_MOVE_ATOMIC(&reversed, &ctx->scheduled_coroutines); @@ -388,7 +389,16 @@ static void co_schedule_bh_cb(void *opaque) QSLIST_REMOVE_HEAD(&straight, co_scheduled_next); trace_aio_co_schedule_bh_cb(ctx, co); aio_context_acquire(ctx); - qemu_coroutine_enter(co); + scheduled_ctx =3D atomic_mb_read(&co->scheduled); + if (scheduled_ctx =3D=3D ctx) { + qemu_coroutine_enter(co); + } else { + /* This must be a cancelled aio_co_schedule() because the coro= utine + * was already entered before this BH had a chance to run. If a + * coroutine is scheduled for two different AioContexts, somet= hing + * is very wrong. */ + assert(scheduled_ctx =3D=3D NULL); + } aio_context_release(ctx); } } @@ -438,6 +448,9 @@ fail: void aio_co_schedule(AioContext *ctx, Coroutine *co) { trace_aio_co_schedule(ctx, co); + + /* Set co->scheduled before the coroutine becomes visible in the list = */ + atomic_mb_set(&co->scheduled, ctx); QSLIST_INSERT_HEAD_ATOMIC(&ctx->scheduled_coroutines, co, co_scheduled_next); qemu_bh_schedule(ctx->co_schedule_bh); diff --git a/util/qemu-coroutine.c b/util/qemu-coroutine.c index d6095c1d5a..c515b3cb4a 100644 --- a/util/qemu-coroutine.c +++ b/util/qemu-coroutine.c @@ -122,6 +122,23 @@ void qemu_aio_coroutine_enter(AioContext *ctx, Corouti= ne *co) */ smp_wmb(); =20 + /* Make sure that a coroutine that can alternatively reentered from two + * different sources isn't reentered more than once when the first cal= ler + * uses aio_co_schedule() and the other one enters to coroutine direct= ly. + * This is achieved by cancelling the pending aio_co_schedule(). + * + * The other way round, if aio_co_schedule() would be called after this + * point, this would be a problem, too, but in practice it doesn't hap= pen + * because we're holding the AioContext lock here and aio_co_schedule() + * callers must do the same. This means that the coroutine just needs = to + * prevent other callers from calling aio_co_schedule() before it yiel= ds + * (e.g. block job coroutines by setting job->busy =3D true). + * + * We still want to ensure that the second case doesn't happen, so res= et + * co->scheduled only after setting co->caller to make the above check + * effective for the co_schedule_bh_cb() case. */ + atomic_set(&co->scheduled, NULL); + ret =3D qemu_coroutine_switch(self, co, COROUTINE_ENTER); =20 qemu_co_queue_run_restart(co); --=20 2.13.6 From nobody Sun Apr 28 18:41:59 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) client-ip=208.118.235.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists.gnu.org; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of gnu.org designates 208.118.235.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists.gnu.org (208.118.235.17 [208.118.235.17]) by mx.zohomail.com with SMTPS id 1511884279054894.2082193507312; Tue, 28 Nov 2017 07:51:19 -0800 (PST) Received: from localhost ([::1]:38566 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJiAD-0001nf-BP for importer@patchew.org; Tue, 28 Nov 2017 10:51:13 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34021) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1eJi3v-0005E6-5C for qemu-devel@nongnu.org; Tue, 28 Nov 2017 10:44:47 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1eJi3r-0005dP-Ae for qemu-devel@nongnu.org; Tue, 28 Nov 2017 10:44:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:48582) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1eJi3k-0005aB-Vc; Tue, 28 Nov 2017 10:44:33 -0500 Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 1496761476; Tue, 28 Nov 2017 15:44:32 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-117-201.ams2.redhat.com [10.36.117.201]) by smtp.corp.redhat.com (Postfix) with ESMTP id 534396198D; Tue, 28 Nov 2017 15:44:20 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Date: Tue, 28 Nov 2017 16:43:50 +0100 Message-Id: <20171128154350.21504-5-kwolf@redhat.com> In-Reply-To: <20171128154350.21504-1-kwolf@redhat.com> References: <20171128154350.21504-1-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Tue, 28 Nov 2017 15:44:32 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] [fuzzy] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PATCH for-2.11 4/4] block: Expect graph changes in bdrv_parent_drained_begin/end X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kwolf@redhat.com, famz@redhat.com, jcody@redhat.com, qemu-devel@nongnu.org, mreitz@redhat.com, stefanha@redhat.com, pbonzini@redhat.com Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: "Qemu-devel" X-ZohoMail: RSF_0 Z_629925259 SPT_0 Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" The .drained_begin/end callbacks can (directly or indirectly via aio_poll()) cause block nodes to be removed or the current BdrvChild to point to a different child node. Use QLIST_FOREACH_SAFE() to make sure we don't access invalid BlockDriverStates or accidentally continue iterating the parents of the new child node instead of the node we actually came from. Signed-off-by: Kevin Wolf Reviewed-by: Jeff Cody Reviewed-by: Stefan Hajnoczi Tested-by: Jeff Cody --- block/io.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/block/io.c b/block/io.c index 4fdf93a014..6773926fc1 100644 --- a/block/io.c +++ b/block/io.c @@ -42,9 +42,9 @@ static int coroutine_fn bdrv_co_do_pwrite_zeroes(BlockDri= verState *bs, =20 void bdrv_parent_drained_begin(BlockDriverState *bs) { - BdrvChild *c; + BdrvChild *c, *next; =20 - QLIST_FOREACH(c, &bs->parents, next_parent) { + QLIST_FOREACH_SAFE(c, &bs->parents, next_parent, next) { if (c->role->drained_begin) { c->role->drained_begin(c); } @@ -53,9 +53,9 @@ void bdrv_parent_drained_begin(BlockDriverState *bs) =20 void bdrv_parent_drained_end(BlockDriverState *bs) { - BdrvChild *c; + BdrvChild *c, *next; =20 - QLIST_FOREACH(c, &bs->parents, next_parent) { + QLIST_FOREACH_SAFE(c, &bs->parents, next_parent, next) { if (c->role->drained_end) { c->role->drained_end(c); } --=20 2.13.6