From patchwork Mon Jun 20 14:05:28 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stefan Hajnoczi X-Patchwork-Id: 9187621 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 8D89F6075E for ; Mon, 20 Jun 2016 14:24:29 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 790301FE7B for ; Mon, 20 Jun 2016 14:24:29 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6B9D227813; Mon, 20 Jun 2016 14:24:29 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 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.wl.linuxfoundation.org (Postfix) with ESMTPS id DA2891FE7B for ; Mon, 20 Jun 2016 14:24:28 +0000 (UTC) Received: from localhost ([::1]:43939 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bF07m-0002s4-RI for patchwork-qemu-devel@patchwork.kernel.org; Mon, 20 Jun 2016 10:24:26 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:47818) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bEzqD-000865-CA for qemu-devel@nongnu.org; Mon, 20 Jun 2016 10:06:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bEzqA-00079P-Bx for qemu-devel@nongnu.org; Mon, 20 Jun 2016 10:06:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:34111) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bEzq9-00079E-Ux for qemu-devel@nongnu.org; Mon, 20 Jun 2016 10:06:14 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 416D23B717; Mon, 20 Jun 2016 14:06:13 +0000 (UTC) Received: from localhost (ovpn-112-59.ams2.redhat.com [10.36.112.59]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u5KE6B5w027907; Mon, 20 Jun 2016 10:06:12 -0400 From: Stefan Hajnoczi To: Date: Mon, 20 Jun 2016 15:05:28 +0100 Message-Id: <1466431531-17806-18-git-send-email-stefanha@redhat.com> In-Reply-To: <1466431531-17806-1-git-send-email-stefanha@redhat.com> References: <1466431531-17806-1-git-send-email-stefanha@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.26 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Mon, 20 Jun 2016 14:06:13 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 17/20] block: use safe iteration over AioContext notifiers 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: Peter Maydell , Stefan Hajnoczi Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP It's possible that an AioContext notifier user was close to finishing when .detach_aio_context() or .attached_aio_context() is called. In that case they may call bdrv_remove_aio_context_notifier() during the callback. Use safe iteration to avoid crashing when the notifier list is modified during iteration. We must not only handle the case where the current aio notifier is removed during a callback but also the one where any other aio notifier is removed. The next patch adds an AioContext notifier for block jobs and they really could be terminating just as .detach_aio_context() is invoked. Signed-off-by: Stefan Hajnoczi Reviewed-by: Paolo Bonzini Reviewed-by: Fam Zheng Message-id: 1466096189-6477-6-git-send-email-stefanha@redhat.com --- block.c | 46 ++++++++++++++++++++++++++++++++++++---------- include/block/block_int.h | 2 ++ 2 files changed, 38 insertions(+), 10 deletions(-) diff --git a/block.c b/block.c index b331eb9..c0ccc27 100644 --- a/block.c +++ b/block.c @@ -3609,18 +3609,34 @@ AioContext *bdrv_get_aio_context(BlockDriverState *bs) return bs->aio_context; } +static void bdrv_do_remove_aio_context_notifier(BdrvAioNotifier *ban) +{ + QLIST_REMOVE(ban, list); + g_free(ban); +} + void bdrv_detach_aio_context(BlockDriverState *bs) { - BdrvAioNotifier *baf; + BdrvAioNotifier *baf, *baf_tmp; BdrvChild *child; if (!bs->drv) { return; } - QLIST_FOREACH(baf, &bs->aio_notifiers, list) { - baf->detach_aio_context(baf->opaque); + assert(!bs->walking_aio_notifiers); + bs->walking_aio_notifiers = true; + QLIST_FOREACH_SAFE(baf, &bs->aio_notifiers, list, baf_tmp) { + if (baf->deleted) { + bdrv_do_remove_aio_context_notifier(baf); + } else { + baf->detach_aio_context(baf->opaque); + } } + /* Never mind iterating again to check for ->deleted. bdrv_close() will + * remove remaining aio notifiers if we aren't called again. + */ + bs->walking_aio_notifiers = false; if (bs->drv->bdrv_detach_aio_context) { bs->drv->bdrv_detach_aio_context(bs); @@ -3635,7 +3651,7 @@ void bdrv_detach_aio_context(BlockDriverState *bs) void bdrv_attach_aio_context(BlockDriverState *bs, AioContext *new_context) { - BdrvAioNotifier *ban; + BdrvAioNotifier *ban, *ban_tmp; BdrvChild *child; if (!bs->drv) { @@ -3651,9 +3667,16 @@ void bdrv_attach_aio_context(BlockDriverState *bs, bs->drv->bdrv_attach_aio_context(bs, new_context); } - QLIST_FOREACH(ban, &bs->aio_notifiers, list) { - ban->attached_aio_context(new_context, ban->opaque); + assert(!bs->walking_aio_notifiers); + bs->walking_aio_notifiers = true; + QLIST_FOREACH_SAFE(ban, &bs->aio_notifiers, list, ban_tmp) { + if (ban->deleted) { + bdrv_do_remove_aio_context_notifier(ban); + } else { + ban->attached_aio_context(new_context, ban->opaque); + } } + bs->walking_aio_notifiers = false; } void bdrv_set_aio_context(BlockDriverState *bs, AioContext *new_context) @@ -3695,11 +3718,14 @@ void bdrv_remove_aio_context_notifier(BlockDriverState *bs, QLIST_FOREACH_SAFE(ban, &bs->aio_notifiers, list, ban_next) { if (ban->attached_aio_context == attached_aio_context && ban->detach_aio_context == detach_aio_context && - ban->opaque == opaque) + ban->opaque == opaque && + ban->deleted == false) { - QLIST_REMOVE(ban, list); - g_free(ban); - + if (bs->walking_aio_notifiers) { + ban->deleted = true; + } else { + bdrv_do_remove_aio_context_notifier(ban); + } return; } } diff --git a/include/block/block_int.h b/include/block/block_int.h index 688c6be..2057156 100644 --- a/include/block/block_int.h +++ b/include/block/block_int.h @@ -361,6 +361,7 @@ typedef struct BdrvAioNotifier { void (*detach_aio_context)(void *opaque); void *opaque; + bool deleted; QLIST_ENTRY(BdrvAioNotifier) list; } BdrvAioNotifier; @@ -427,6 +428,7 @@ struct BlockDriverState { * BDS may register themselves in this list to be notified of changes * regarding this BDS's context */ QLIST_HEAD(, BdrvAioNotifier) aio_notifiers; + bool walking_aio_notifiers; /* to make removal during iteration safe */ char filename[PATH_MAX]; char backing_file[PATH_MAX]; /* if non zero, the image is a diff of