From patchwork Tue Feb 16 17:56:17 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 8330251 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 2636AC02AA for ; Tue, 16 Feb 2016 17:59:07 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 857A920268 for ; Tue, 16 Feb 2016 17:59:06 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id C8C6420266 for ; Tue, 16 Feb 2016 17:59:05 +0000 (UTC) Received: from localhost ([::1]:48944 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVjtx-00085k-61 for patchwork-qemu-devel@patchwork.kernel.org; Tue, 16 Feb 2016 12:59:05 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41077) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVjrj-0004Bj-K0 for qemu-devel@nongnu.org; Tue, 16 Feb 2016 12:56:48 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aVjre-0002q2-Fx for qemu-devel@nongnu.org; Tue, 16 Feb 2016 12:56:47 -0500 Received: from mx1.redhat.com ([209.132.183.28]:34375) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aVjrc-0002pD-ES; Tue, 16 Feb 2016 12:56:40 -0500 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id C5F77461C8; Tue, 16 Feb 2016 17:56:39 +0000 (UTC) Received: from donizetti.redhat.com (ovpn-112-51.ams2.redhat.com [10.36.112.51]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u1GHuSWS022072; Tue, 16 Feb 2016 12:56:38 -0500 From: Paolo Bonzini To: qemu-devel@nongnu.org Date: Tue, 16 Feb 2016 18:56:17 +0100 Message-Id: <1455645388-32401-6-git-send-email-pbonzini@redhat.com> In-Reply-To: <1455645388-32401-1-git-send-email-pbonzini@redhat.com> References: <1455645388-32401-1-git-send-email-pbonzini@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 209.132.183.28 Cc: qemu-block@nongnu.org, stefanha@redhat.com Subject: [Qemu-devel] [PATCH 05/16] mirror: use bottom half to re-enter coroutine X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP mirror is calling bdrv_drain from an AIO callback---more precisely, the bdrv_drain happens far away from the AIO callback, in the coroutine that the AIO callback enters. This used to be okay because bdrv_drain more or less tried to guess when all AIO callbacks were done; however it will cause a deadlock once bdrv_drain really checks for AIO callbacks to be complete. The situation here is admittedly underdefined, and Stefan pointed out that the same solution is found in many other places in the QEMU block layer, therefore I think this workaround is acceptable. Signed-off-by: Paolo Bonzini --- block/mirror.c | 23 +++++++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/block/mirror.c b/block/mirror.c index 2c0edfa..793c20c 100644 --- a/block/mirror.c +++ b/block/mirror.c @@ -71,6 +71,7 @@ typedef struct MirrorOp { QEMUIOVector qiov; int64_t sector_num; int nb_sectors; + QEMUBH *co_enter_bh; } MirrorOp; static BlockErrorAction mirror_error_action(MirrorBlockJob *s, bool read, @@ -86,6 +87,18 @@ static BlockErrorAction mirror_error_action(MirrorBlockJob *s, bool read, } } +static void mirror_bh_cb(void *opaque) +{ + MirrorOp *op = opaque; + MirrorBlockJob *s = op->s; + + qemu_bh_delete(op->co_enter_bh); + g_free(op); + if (s->waiting_for_io) { + qemu_coroutine_enter(s->common.co, NULL); + } +} + static void mirror_iteration_done(MirrorOp *op, int ret) { MirrorBlockJob *s = op->s; @@ -116,11 +129,13 @@ static void mirror_iteration_done(MirrorOp *op, int ret) } qemu_iovec_destroy(&op->qiov); - g_free(op); - if (s->waiting_for_io) { - qemu_coroutine_enter(s->common.co, NULL); - } + /* The I/O operation is not finished until the callback returns. + * If we call qemu_coroutine_enter here, there is the possibility + * of a deadlock when the coroutine calls bdrv_drained_begin. + */ + op->co_enter_bh = qemu_bh_new(mirror_bh_cb, op); + qemu_bh_schedule(op->co_enter_bh); } static void mirror_write_complete(void *opaque, int ret)