From nobody Mon Apr 29 22:11:50 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1607971000; cv=none; d=zohomail.com; s=zohoarc; b=Jj1Ytovxc8m+PJcYesqJL2WaXWrxvuLWdURITec9SAfhlog/lS+CU0Fsak8a/xdhyc77wEe8StULFbXOG3yinmsAytGWk0MR5qk0ZVY1hMKIqEdf3dzbbpvrO6Cmc293v7sPDpCE2CwhA4bUZ26jKC/KB/zC+aVsiw4/0xOQgCA= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1607971000; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=+Eya5ky4YkXegn79a2KsK7rMFFfw80cpB9O81T1hhcs=; b=TEHkCLnuTRjMtVd49Y3FJfJrwS+v1qz0QXAn6XVLF3NS8eR8RzDYumHtiph0Q6r3VHqRu2chco+EOvksj5WCgOWPfah8XtInMZhnUjokYoukBNOat3OXHqxtF+R8/Uh/1twOrZnXOAhcmubhcxaEAksp8UeGLBpu9EvXjSNzXSA= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1607971000229669.5093444975022; Mon, 14 Dec 2020 10:36:40 -0800 (PST) Received: from list by lists.xenproject.org with outflank-mailman.52461.91844 (Exim 4.92) (envelope-from ) id 1kosht-0003d8-S2; Mon, 14 Dec 2020 18:36:25 +0000 Received: by outflank-mailman (output) from mailman id 52461.91844; Mon, 14 Dec 2020 18:36:25 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1koshs-0003bh-Fx; Mon, 14 Dec 2020 18:36:24 +0000 Received: by outflank-mailman (input) for mailman id 52461; Mon, 14 Dec 2020 17:05:44 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1korI8-0001vO-1f for xen-devel@lists.xenproject.org; Mon, 14 Dec 2020 17:05:44 +0000 Received: from us-smtp-delivery-124.mimecast.com (unknown [63.128.21.124]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTP id 76bc8c39-8d8b-419e-a334-5f0d38c62da0; Mon, 14 Dec 2020 17:05:42 +0000 (UTC) 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-580-s8NqP3znNFanUNxldhXiHw-1; Mon, 14 Dec 2020 12:05:38 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 548C3B8122; Mon, 14 Dec 2020 17:05:37 +0000 (UTC) Received: from toolbox.redhat.com (ovpn-112-231.rdu2.redhat.com [10.10.112.231]) by smtp.corp.redhat.com (Postfix) with ESMTP id 50A455D6AB; Mon, 14 Dec 2020 17:05:34 +0000 (UTC) X-Outflank-Mailman: Message body and most headers restored to incoming version X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 76bc8c39-8d8b-419e-a334-5f0d38c62da0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607965542; 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: in-reply-to:in-reply-to:references:references; bh=+Eya5ky4YkXegn79a2KsK7rMFFfw80cpB9O81T1hhcs=; b=D8nfXyPen1o7Wnf4yvy+Rd7RUlMUAVWGCfe9xBxy2E3ybg/9xGw3zfDtsfFHNtmg3enqjU 3Oz+MUgAS/NFfi74odgd45oEm9ZN17vaI04tpuPxGAeWM+ymkXjSas/nvkknBq3I2RazsO s2Qv27XHqZrZg4S5HIMoRwzGsPEBhj0= X-MC-Unique: s8NqP3znNFanUNxldhXiHw-1 From: Sergio Lopez To: qemu-devel@nongnu.org Cc: Kevin Wolf , Stefano Stabellini , qemu-block@nongnu.org, Anthony Perard , xen-devel@lists.xenproject.org, Paul Durrant , "Michael S. Tsirkin" , Stefan Hajnoczi , Fam Zheng , Eric Blake , Paolo Bonzini , Max Reitz , Sergio Lopez Subject: [PATCH v2 1/4] block: Honor blk_set_aio_context() context requirements Date: Mon, 14 Dec 2020 18:05:16 +0100 Message-Id: <20201214170519.223781-2-slp@redhat.com> In-Reply-To: <20201214170519.223781-1-slp@redhat.com> References: <20201214170519.223781-1-slp@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=slp@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" The documentation for bdrv_set_aio_context_ignore() states this: * The caller must own the AioContext lock for the old AioContext of bs, bu= t it * must not own the AioContext lock for new_context (unless new_context is = the * same as the current context of bs). As blk_set_aio_context() makes use of this function, this rule also applies to it. Fix all occurrences where this rule wasn't honored. Suggested-by: Kevin Wolf Signed-off-by: Sergio Lopez Reviewed-by: Kevin Wolf --- hw/block/dataplane/virtio-blk.c | 4 ++++ hw/block/dataplane/xen-block.c | 7 ++++++- hw/scsi/virtio-scsi.c | 6 ++++-- 3 files changed, 14 insertions(+), 3 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-bl= k.c index 37499c5564..e9050c8987 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -172,6 +172,7 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev) VirtIOBlockDataPlane *s =3D vblk->dataplane; BusState *qbus =3D BUS(qdev_get_parent_bus(DEVICE(vblk))); VirtioBusClass *k =3D VIRTIO_BUS_GET_CLASS(qbus); + AioContext *old_context; unsigned i; unsigned nvqs =3D s->conf->num_queues; Error *local_err =3D NULL; @@ -214,7 +215,10 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev) vblk->dataplane_started =3D true; trace_virtio_blk_data_plane_start(s); =20 + old_context =3D blk_get_aio_context(s->conf->conf.blk); + aio_context_acquire(old_context); r =3D blk_set_aio_context(s->conf->conf.blk, s->ctx, &local_err); + aio_context_release(old_context); if (r < 0) { error_report_err(local_err); goto fail_guest_notifiers; diff --git a/hw/block/dataplane/xen-block.c b/hw/block/dataplane/xen-block.c index 71c337c7b7..3675f8deaf 100644 --- a/hw/block/dataplane/xen-block.c +++ b/hw/block/dataplane/xen-block.c @@ -725,6 +725,7 @@ void xen_block_dataplane_start(XenBlockDataPlane *datap= lane, { ERRP_GUARD(); XenDevice *xendev =3D dataplane->xendev; + AioContext *old_context; unsigned int ring_size; unsigned int i; =20 @@ -808,10 +809,14 @@ void xen_block_dataplane_start(XenBlockDataPlane *dat= aplane, goto stop; } =20 - aio_context_acquire(dataplane->ctx); + old_context =3D blk_get_aio_context(dataplane->blk); + aio_context_acquire(old_context); /* If other users keep the BlockBackend in the iothread, that's ok */ blk_set_aio_context(dataplane->blk, dataplane->ctx, NULL); + aio_context_release(old_context); + /* Only reason for failure is a NULL channel */ + aio_context_acquire(dataplane->ctx); xen_device_set_event_channel_context(xendev, dataplane->event_channel, dataplane->ctx, &error_abort); aio_context_release(dataplane->ctx); diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 3db9a8aae9..7a347ceac5 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -821,15 +821,17 @@ static void virtio_scsi_hotplug(HotplugHandler *hotpl= ug_dev, DeviceState *dev, VirtIODevice *vdev =3D VIRTIO_DEVICE(hotplug_dev); VirtIOSCSI *s =3D VIRTIO_SCSI(vdev); SCSIDevice *sd =3D SCSI_DEVICE(dev); + AioContext *old_context; int ret; =20 if (s->ctx && !s->dataplane_fenced) { if (blk_op_is_blocked(sd->conf.blk, BLOCK_OP_TYPE_DATAPLANE, errp)= ) { return; } - virtio_scsi_acquire(s); + old_context =3D blk_get_aio_context(sd->conf.blk); + aio_context_acquire(old_context); ret =3D blk_set_aio_context(sd->conf.blk, s->ctx, errp); - virtio_scsi_release(s); + aio_context_release(old_context); if (ret < 0) { return; } --=20 2.26.2 From nobody Mon Apr 29 22:11:50 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1607971000; cv=none; d=zohomail.com; s=zohoarc; b=NcvCyXAWzi5THridvbWazVMPlqQtAcMHg/NPvSsN7Hx8BAzjjo5Ciq3uGBkdtvy4gyWTsguQwJxqygnb5gih0BoCOi8FFm1cMYDKW0w1U3HcNZrk6jkVSrYnomauZfsiTZux+4tt6rBo/ORo2Oq+x+wxRSgRi/80uk2EIvfhBds= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1607971000; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=/WpOYjMEI+uu6Ezu621PcgD7x1p7LtSJUlUJzLyqhGc=; b=K4ROMFZbb6B7U4nswru8bpe6kM9JxF6hIhaR095xqR5c3fLTswfe/e9eFK8TKVfR+2SALQC4/SGkkv/bYzql+VrvSXFao6+4o4d7sHFsuA4U+WUAVXGgHvurEQlmtycNk1hNe9EQfSWteei3q8ayPWAz1S3WCcf26zBKib738zQ= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1607971000031186.21970959284715; Mon, 14 Dec 2020 10:36:40 -0800 (PST) Received: from list by lists.xenproject.org with outflank-mailman.52463.91851 (Exim 4.92) (envelope-from ) id 1koshv-0003he-1M; Mon, 14 Dec 2020 18:36:27 +0000 Received: by outflank-mailman (output) from mailman id 52463.91851; Mon, 14 Dec 2020 18:36:26 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1koshu-0003g8-5h; Mon, 14 Dec 2020 18:36:26 +0000 Received: by outflank-mailman (input) for mailman id 52463; Mon, 14 Dec 2020 17:06:38 +0000 Received: from all-amaz-eas1.inumbo.com ([34.197.232.57] helo=us1-amaz-eas2.inumbo.com) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1korJ0-0001x2-3B for xen-devel@lists.xenproject.org; Mon, 14 Dec 2020 17:06:38 +0000 Received: from us-smtp-delivery-124.mimecast.com (unknown [63.128.21.124]) by us1-amaz-eas2.inumbo.com (Halon) with ESMTP id 9ad1bd6e-18cc-453b-bac9-306d71786958; Mon, 14 Dec 2020 17:06:37 +0000 (UTC) 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-244-D0CmhjvTOY6pEL_0tz-PPA-1; Mon, 14 Dec 2020 12:06:33 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7C6511012EA3; Mon, 14 Dec 2020 17:06:19 +0000 (UTC) Received: from toolbox.redhat.com (ovpn-112-231.rdu2.redhat.com [10.10.112.231]) by smtp.corp.redhat.com (Postfix) with ESMTP id A9AEB5D6AB; Mon, 14 Dec 2020 17:05:37 +0000 (UTC) X-Outflank-Mailman: Message body and most headers restored to incoming version X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: 9ad1bd6e-18cc-453b-bac9-306d71786958 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607965597; 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: in-reply-to:in-reply-to:references:references; bh=/WpOYjMEI+uu6Ezu621PcgD7x1p7LtSJUlUJzLyqhGc=; b=GDKALjNQdTUuK4lCz8sOK7Ds7EM8Kw3WgME4H6+oOwUCaDYsXjzGjQ6rxn81lQy3+saEZ3 rfdLTGdkOZMOh7w9JHKG5sSnKtqupk0VUeCHYo5HcJ1tjZ4PE2KE1vlZv+aPVirnLBGGyv NuwUIM78Rklg4sjogSAFOTZzTeFjTRI= X-MC-Unique: D0CmhjvTOY6pEL_0tz-PPA-1 From: Sergio Lopez To: qemu-devel@nongnu.org Cc: Kevin Wolf , Stefano Stabellini , qemu-block@nongnu.org, Anthony Perard , xen-devel@lists.xenproject.org, Paul Durrant , "Michael S. Tsirkin" , Stefan Hajnoczi , Fam Zheng , Eric Blake , Paolo Bonzini , Max Reitz , Sergio Lopez Subject: [PATCH v2 2/4] block: Avoid processing BDS twice in bdrv_set_aio_context_ignore() Date: Mon, 14 Dec 2020 18:05:17 +0100 Message-Id: <20201214170519.223781-3-slp@redhat.com> In-Reply-To: <20201214170519.223781-1-slp@redhat.com> References: <20201214170519.223781-1-slp@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=slp@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" While processing the parents of a BDS, one of the parents may process the child that's doing the tail recursion, which leads to a BDS being processed twice. This is especially problematic for the aio_notifiers, as they might attempt to work on both the old and the new AIO contexts. To avoid this, add the BDS pointer to the ignore list, and check the child BDS pointer while iterating over the children. Signed-off-by: Sergio Lopez --- block.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/block.c b/block.c index f1cedac362..bc8a66ab6e 100644 --- a/block.c +++ b/block.c @@ -6465,12 +6465,17 @@ void bdrv_set_aio_context_ignore(BlockDriverState *= bs, bdrv_drained_begin(bs); =20 QLIST_FOREACH(child, &bs->children, next) { - if (g_slist_find(*ignore, child)) { + if (g_slist_find(*ignore, child) || g_slist_find(*ignore, child->b= s)) { continue; } *ignore =3D g_slist_prepend(*ignore, child); bdrv_set_aio_context_ignore(child->bs, new_context, ignore); } + /* + * Add a reference to this BS to the ignore list, so its + * parents won't attempt to process it again. + */ + *ignore =3D g_slist_prepend(*ignore, bs); QLIST_FOREACH(child, &bs->parents, next_parent) { if (g_slist_find(*ignore, child)) { continue; --=20 2.26.2 From nobody Mon Apr 29 22:11:50 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) client-ip=192.237.175.120; envelope-from=xen-devel-bounces@lists.xenproject.org; helo=lists.xenproject.org; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1607971004; cv=none; d=zohomail.com; s=zohoarc; b=X44UlW3NjYrSxu9UAT7cPbKw+TXV5QRbjrV067+rlUtaJHeGZwpfeu69nAp1KVq9XQZk/+6LHtb5fIcSki5u7K39vqfe170iAOtUwAibuapek+wKMs0qJpfvMs8+enURz+92DbF0dq3GQR6LSRMBBfPmxk0aF6JwZtnWXD6/qcE= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1607971004; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=399Y9fIGtHICB7zQVl2y/q4KwqXDdZ2nv2mnaFw6NX8=; b=DxkPov9R+qbJuHWewY5F+nbL34uNpk/vpwSXkCbQqZXZIIIALA5p9dTB68fwTBzpUeekOJ045W2To/ROVb6OSSyx2EEC0m3hq8r0aMFuRoJDlW74sejiZFGmXOaI3Ur7jNv35dp6RM5w8O95tAyBenXV/mhDXsedwEw0czUHsDM= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of lists.xenproject.org designates 192.237.175.120 as permitted sender) smtp.mailfrom=xen-devel-bounces@lists.xenproject.org; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) by mx.zohomail.com with SMTPS id 1607971004110690.292932810052; Mon, 14 Dec 2020 10:36:44 -0800 (PST) Received: from list by lists.xenproject.org with outflank-mailman.52465.91866 (Exim 4.92) (envelope-from ) id 1koshw-0003mG-LT; Mon, 14 Dec 2020 18:36:28 +0000 Received: by outflank-mailman (output) from mailman id 52465.91866; Mon, 14 Dec 2020 18:36:28 +0000 Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1koshv-0003ks-PK; Mon, 14 Dec 2020 18:36:27 +0000 Received: by outflank-mailman (input) for mailman id 52465; Mon, 14 Dec 2020 17:06:44 +0000 Received: from us1-rack-iad1.inumbo.com ([172.99.69.81]) by lists.xenproject.org with esmtp (Exim 4.92) (envelope-from ) id 1korJ6-0001xX-BH for xen-devel@lists.xenproject.org; Mon, 14 Dec 2020 17:06:44 +0000 Received: from us-smtp-delivery-124.mimecast.com (unknown [216.205.24.124]) by us1-rack-iad1.inumbo.com (Halon) with ESMTP id d7c0dce5-848f-4709-b476-4e994209fe41; Mon, 14 Dec 2020 17:06:43 +0000 (UTC) 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-499-761DnAutO9qHxmbuLzXzFA-1; Mon, 14 Dec 2020 12:06:39 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A75508AB3B9; Mon, 14 Dec 2020 17:06:22 +0000 (UTC) Received: from toolbox.redhat.com (ovpn-112-231.rdu2.redhat.com [10.10.112.231]) by smtp.corp.redhat.com (Postfix) with ESMTP id 72BDC62A25; Mon, 14 Dec 2020 17:06:19 +0000 (UTC) X-Outflank-Mailman: Message body and most headers restored to incoming version X-BeenThere: xen-devel@lists.xenproject.org List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Errors-To: xen-devel-bounces@lists.xenproject.org Precedence: list Sender: "Xen-devel" X-Inumbo-ID: d7c0dce5-848f-4709-b476-4e994209fe41 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607965603; 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: in-reply-to:in-reply-to:references:references; bh=399Y9fIGtHICB7zQVl2y/q4KwqXDdZ2nv2mnaFw6NX8=; b=ZChTdDjNcRvbYd+NUF68br9MV+RSDoVzS55RPFrBdR8jNQHTk+MBVi35kLHmGvR25RIBIv 1qxJ0eN90i8PlZO+mz2B0OAAi4Ga89Y55VtLcCRvG9J5CPgeIssPk2UZlXyTVRM5YDyZDj DJrDhtNGwu/JqgExCOyRsS+UcFbCFiU= X-MC-Unique: 761DnAutO9qHxmbuLzXzFA-1 From: Sergio Lopez To: qemu-devel@nongnu.org Cc: Kevin Wolf , Stefano Stabellini , qemu-block@nongnu.org, Anthony Perard , xen-devel@lists.xenproject.org, Paul Durrant , "Michael S. Tsirkin" , Stefan Hajnoczi , Fam Zheng , Eric Blake , Paolo Bonzini , Max Reitz , Sergio Lopez Subject: [PATCH v2 3/4] nbd/server: Quiesce coroutines on context switch Date: Mon, 14 Dec 2020 18:05:18 +0100 Message-Id: <20201214170519.223781-4-slp@redhat.com> In-Reply-To: <20201214170519.223781-1-slp@redhat.com> References: <20201214170519.223781-1-slp@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=slp@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" When switching between AIO contexts we need to me make sure that both recv_coroutine and send_coroutine are not scheduled to run. Otherwise, QEMU may crash while attaching the new context with an error like this one: aio_co_schedule: Co-routine was already scheduled in 'aio_co_schedule' To achieve this we need a local implementation of 'qio_channel_readv_all_eof' named 'nbd_read_eof' (a trick already done by 'nbd/client.c') that allows us to interrupt the operation and to know when recv_coroutine is yielding. With this in place, we delegate detaching the AIO context to the owning context with a BH ('nbd_aio_detach_bh') scheduled using 'aio_wait_bh_oneshot'. This BH signals that we need to quiesce the channel by setting 'client->quiescing' to 'true', and either waits for the coroutine to finish using AIO_WAIT_WHILE or, if it's yielding in 'nbd_read_eof', actively enters the coroutine to interrupt it. RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=3D1900326 Signed-off-by: Sergio Lopez Reviewed-by: Eric Blake --- nbd/server.c | 120 +++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 106 insertions(+), 14 deletions(-) diff --git a/nbd/server.c b/nbd/server.c index 613ed2634a..7229f487d2 100644 --- a/nbd/server.c +++ b/nbd/server.c @@ -132,6 +132,9 @@ struct NBDClient { CoMutex send_lock; Coroutine *send_coroutine; =20 + bool read_yielding; + bool quiescing; + QTAILQ_ENTRY(NBDClient) next; int nb_requests; bool closing; @@ -1352,14 +1355,60 @@ static coroutine_fn int nbd_negotiate(NBDClient *cl= ient, Error **errp) return 0; } =20 -static int nbd_receive_request(QIOChannel *ioc, NBDRequest *request, +/* nbd_read_eof + * Tries to read @size bytes from @ioc. This is a local implementation of + * qio_channel_readv_all_eof. We have it here because we need it to be + * interruptible and to know when the coroutine is yielding. + * Returns 1 on success + * 0 on eof, when no data was read (errp is not set) + * negative errno on failure (errp is set) + */ +static inline int coroutine_fn +nbd_read_eof(NBDClient *client, void *buffer, size_t size, Error **errp) +{ + bool partial =3D false; + + assert(size); + while (size > 0) { + struct iovec iov =3D { .iov_base =3D buffer, .iov_len =3D size }; + ssize_t len; + + len =3D qio_channel_readv(client->ioc, &iov, 1, errp); + if (len =3D=3D QIO_CHANNEL_ERR_BLOCK) { + client->read_yielding =3D true; + qio_channel_yield(client->ioc, G_IO_IN); + client->read_yielding =3D false; + if (client->quiescing) { + return -EAGAIN; + } + continue; + } else if (len < 0) { + return -EIO; + } else if (len =3D=3D 0) { + if (partial) { + error_setg(errp, + "Unexpected end-of-file before all bytes were r= ead"); + return -EIO; + } else { + return 0; + } + } + + partial =3D true; + size -=3D len; + buffer =3D (uint8_t *) buffer + len; + } + return 1; +} + +static int nbd_receive_request(NBDClient *client, NBDRequest *request, Error **errp) { uint8_t buf[NBD_REQUEST_SIZE]; uint32_t magic; int ret; =20 - ret =3D nbd_read(ioc, buf, sizeof(buf), "request", errp); + ret =3D nbd_read_eof(client, buf, sizeof(buf), errp); if (ret < 0) { return ret; } @@ -1480,11 +1529,37 @@ static void blk_aio_attached(AioContext *ctx, void = *opaque) =20 QTAILQ_FOREACH(client, &exp->clients, next) { qio_channel_attach_aio_context(client->ioc, ctx); + + assert(client->recv_coroutine =3D=3D NULL); + assert(client->send_coroutine =3D=3D NULL); + + if (client->quiescing) { + client->quiescing =3D false; + nbd_client_receive_next_request(client); + } + } +} + +static void nbd_aio_detach_bh(void *opaque) +{ + NBDExport *exp =3D opaque; + NBDClient *client; + + QTAILQ_FOREACH(client, &exp->clients, next) { + qio_channel_detach_aio_context(client->ioc); + client->quiescing =3D true; + if (client->recv_coroutine) { - aio_co_schedule(ctx, client->recv_coroutine); + if (client->read_yielding) { + qemu_aio_coroutine_enter(exp->common.ctx, + client->recv_coroutine); + } else { + AIO_WAIT_WHILE(exp->common.ctx, client->recv_coroutine != =3D NULL); + } } + if (client->send_coroutine) { - aio_co_schedule(ctx, client->send_coroutine); + AIO_WAIT_WHILE(exp->common.ctx, client->send_coroutine !=3D NU= LL); } } } @@ -1492,13 +1567,10 @@ static void blk_aio_attached(AioContext *ctx, void = *opaque) static void blk_aio_detach(void *opaque) { NBDExport *exp =3D opaque; - NBDClient *client; =20 trace_nbd_blk_aio_detach(exp->name, exp->common.ctx); =20 - QTAILQ_FOREACH(client, &exp->clients, next) { - qio_channel_detach_aio_context(client->ioc); - } + aio_wait_bh_oneshot(exp->common.ctx, nbd_aio_detach_bh, exp); =20 exp->common.ctx =3D NULL; } @@ -2151,20 +2223,23 @@ static int nbd_co_send_bitmap(NBDClient *client, ui= nt64_t handle, =20 /* nbd_co_receive_request * Collect a client request. Return 0 if request looks valid, -EIO to drop - * connection right away, and any other negative value to report an error = to - * the client (although the caller may still need to disconnect after repo= rting - * the error). + * connection right away, -EAGAIN to indicate we were interrupted and the + * channel should be quiesced, and any other negative value to report an e= rror + * to the client (although the caller may still need to disconnect after + * reporting the error). */ static int nbd_co_receive_request(NBDRequestData *req, NBDRequest *request, Error **errp) { NBDClient *client =3D req->client; int valid_flags; + int ret; =20 g_assert(qemu_in_coroutine()); assert(client->recv_coroutine =3D=3D qemu_coroutine_self()); - if (nbd_receive_request(client->ioc, request, errp) < 0) { - return -EIO; + ret =3D nbd_receive_request(client, request, errp); + if (ret < 0) { + return ret; } =20 trace_nbd_co_receive_request_decode_type(request->handle, request->typ= e, @@ -2507,6 +2582,17 @@ static coroutine_fn void nbd_trip(void *opaque) return; } =20 + if (client->quiescing) { + /* + * We're switching between AIO contexts. Don't attempt to receive = a new + * request and kick the main context which may be waiting for us. + */ + nbd_client_put(client); + client->recv_coroutine =3D NULL; + aio_wait_kick(); + return; + } + req =3D nbd_request_get(client); ret =3D nbd_co_receive_request(req, &request, &local_err); client->recv_coroutine =3D NULL; @@ -2519,6 +2605,11 @@ static coroutine_fn void nbd_trip(void *opaque) goto done; } =20 + if (ret =3D=3D -EAGAIN) { + assert(client->quiescing); + goto done; + } + nbd_client_receive_next_request(client); if (ret =3D=3D -EIO) { goto disconnect; @@ -2565,7 +2656,8 @@ disconnect: =20 static void nbd_client_receive_next_request(NBDClient *client) { - if (!client->recv_coroutine && client->nb_requests < MAX_NBD_REQUESTS)= { + if (!client->recv_coroutine && client->nb_requests < MAX_NBD_REQUESTS = && + !client->quiescing) { nbd_client_get(client); client->recv_coroutine =3D qemu_coroutine_create(nbd_trip, client); aio_co_schedule(client->exp->common.ctx, client->recv_coroutine); --=20 2.26.2 From nobody Mon Apr 29 22:11:50 2024 Delivered-To: importer@patchew.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=1607972289; cv=none; d=zohomail.com; s=zohoarc; b=Wel4RTMVyXEky4ND8otN5VS+zzUMC0gJ/orqTgW2UzO6NdoG72KGjiweo3/8dlEhsovUe6IRgFVDbrPS7sxiz0xWSQBwaXwGuLXfyn6lNCBWafeognthPFccET8Y4ncoAGDNeHAjf+lhSegJvCZpQWFAR8IU0SPy0DqAppb74JM= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1607972289; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=bR0d1JArrAy1qnHujAAvCo8e9rCfTiOxzdxX6U9l434=; b=hWj4QdZJ8mZ++EYypSbbBnxjIF3O33lXqV/tDv6N4IlKpaA7PZn52OIeD/7W5nN0W7VpH2aMvyBM90iZ08cMecB0+6tlTGut3hsS89OL6kphGdhhJjrxVvjEh+yDLAvxVOEDAsLcwsef1yubAF8EsHV1GZg3O/DGLeUCsY+a/P8= 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 1607972289566794.2761027019066; Mon, 14 Dec 2020 10:58:09 -0800 (PST) Received: from localhost ([::1]:43558 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1korNz-0000ZW-CT for importer@patchew.org; Mon, 14 Dec 2020 12:11:47 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:53338) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1korJF-0002t7-B9 for qemu-devel@nongnu.org; Mon, 14 Dec 2020 12:06:53 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:28929) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1korJD-0005dU-CL for qemu-devel@nongnu.org; Mon, 14 Dec 2020 12:06:53 -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-557-_8-ouXvpNqK8vTFc4U2Pjg-1; Mon, 14 Dec 2020 12:06:47 -0500 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 0BAA0800685; Mon, 14 Dec 2020 17:06:26 +0000 (UTC) Received: from toolbox.redhat.com (ovpn-112-231.rdu2.redhat.com [10.10.112.231]) by smtp.corp.redhat.com (Postfix) with ESMTP id F3A39669FC; Mon, 14 Dec 2020 17:06:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607965610; 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: in-reply-to:in-reply-to:references:references; bh=bR0d1JArrAy1qnHujAAvCo8e9rCfTiOxzdxX6U9l434=; b=A17hCsS2dLfOAdeLek1rUNWKlSiON6lRiCofKLHx/32gKFAprl3Q7BCsU4qXnAFxpiuCvX w+llJLsolyD6t1Ci3p8KhnAtf7i4+RWdwmXlLOvEil/OOOpZ+t2LdrX6n5b1i64KIQjkGS se/SAZIwC8lWaQ+osAjiAtu2+3aZ2oM= X-MC-Unique: _8-ouXvpNqK8vTFc4U2Pjg-1 From: Sergio Lopez To: qemu-devel@nongnu.org Subject: [PATCH v2 4/4] block: Close block exports in two steps Date: Mon, 14 Dec 2020 18:05:19 +0100 Message-Id: <20201214170519.223781-5-slp@redhat.com> In-Reply-To: <20201214170519.223781-1-slp@redhat.com> References: <20201214170519.223781-1-slp@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=slp@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable 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; Received-SPF: pass client-ip=216.205.24.124; envelope-from=slp@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action 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 , Stefano Stabellini , Sergio Lopez , qemu-block@nongnu.org, Paul Durrant , "Michael S. Tsirkin" , Max Reitz , Stefan Hajnoczi , Paolo Bonzini , Anthony Perard , xen-devel@lists.xenproject.org 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" There's a cross-dependency between closing the block exports and draining the block layer. The latter needs that we close all export's client connections to ensure they won't queue more requests, but the exports may have coroutines yielding in the block layer, which implies they can't be fully closed until we drain it. To break this cross-dependency, this change adds a "bool wait" argument to blk_exp_close_all() and blk_exp_close_all_type(), so callers can decide whether they want to wait for the exports to be fully quiesced, or just return after requesting them to shut down. Then, in bdrv_close_all we make two calls, one without waiting to close all client connections, and another after draining the block layer, this time waiting for the exports to be fully quiesced. RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=3D1900505 Signed-off-by: Sergio Lopez --- block.c | 20 +++++++++++++++++++- block/export/export.c | 10 ++++++---- blockdev-nbd.c | 2 +- include/block/export.h | 4 ++-- qemu-nbd.c | 2 +- stubs/blk-exp-close-all.c | 2 +- 6 files changed, 30 insertions(+), 10 deletions(-) diff --git a/block.c b/block.c index bc8a66ab6e..41db70ac07 100644 --- a/block.c +++ b/block.c @@ -4472,13 +4472,31 @@ static void bdrv_close(BlockDriverState *bs) void bdrv_close_all(void) { assert(job_next(NULL) =3D=3D NULL); - blk_exp_close_all(); + + /* + * There's a cross-dependency between closing the block exports and + * draining the block layer. The latter needs that we close all export= 's + * client connections to ensure they won't queue more requests, but the + * exports may have coroutines yielding in the block layer, which impl= ies + * they can't be fully closed until we drain it. + * + * Make a first call to close all export's client connections, without + * waiting for each export to be fully quiesced. + */ + blk_exp_close_all(false); =20 /* Drop references from requests still in flight, such as canceled blo= ck * jobs whose AIO context has not been polled yet */ bdrv_drain_all(); =20 blk_remove_all_bs(); + + /* + * Make a second call to shut down the exports, this time waiting for = them + * to be fully quiesced. + */ + blk_exp_close_all(true); + blockdev_close_all_bdrv_states(); =20 assert(QTAILQ_EMPTY(&all_bdrv_states)); diff --git a/block/export/export.c b/block/export/export.c index bad6f21b1c..0124ebd9f9 100644 --- a/block/export/export.c +++ b/block/export/export.c @@ -280,7 +280,7 @@ static bool blk_exp_has_type(BlockExportType type) } =20 /* type =3D=3D BLOCK_EXPORT_TYPE__MAX for all types */ -void blk_exp_close_all_type(BlockExportType type) +void blk_exp_close_all_type(BlockExportType type, bool wait) { BlockExport *exp, *next; =20 @@ -293,12 +293,14 @@ void blk_exp_close_all_type(BlockExportType type) blk_exp_request_shutdown(exp); } =20 - AIO_WAIT_WHILE(NULL, blk_exp_has_type(type)); + if (wait) { + AIO_WAIT_WHILE(NULL, blk_exp_has_type(type)); + } } =20 -void blk_exp_close_all(void) +void blk_exp_close_all(bool wait) { - blk_exp_close_all_type(BLOCK_EXPORT_TYPE__MAX); + blk_exp_close_all_type(BLOCK_EXPORT_TYPE__MAX, wait); } =20 void qmp_block_export_add(BlockExportOptions *export, Error **errp) diff --git a/blockdev-nbd.c b/blockdev-nbd.c index d8443d235b..d71d4da7c2 100644 --- a/blockdev-nbd.c +++ b/blockdev-nbd.c @@ -266,7 +266,7 @@ void qmp_nbd_server_stop(Error **errp) return; } =20 - blk_exp_close_all_type(BLOCK_EXPORT_TYPE_NBD); + blk_exp_close_all_type(BLOCK_EXPORT_TYPE_NBD, true); =20 nbd_server_free(nbd_server); nbd_server =3D NULL; diff --git a/include/block/export.h b/include/block/export.h index 7feb02e10d..71c25928ce 100644 --- a/include/block/export.h +++ b/include/block/export.h @@ -83,7 +83,7 @@ BlockExport *blk_exp_find(const char *id); void blk_exp_ref(BlockExport *exp); void blk_exp_unref(BlockExport *exp); void blk_exp_request_shutdown(BlockExport *exp); -void blk_exp_close_all(void); -void blk_exp_close_all_type(BlockExportType type); +void blk_exp_close_all(bool wait); +void blk_exp_close_all_type(BlockExportType type, bool wait); =20 #endif diff --git a/qemu-nbd.c b/qemu-nbd.c index a7075c5419..928f4466f6 100644 --- a/qemu-nbd.c +++ b/qemu-nbd.c @@ -1122,7 +1122,7 @@ int main(int argc, char **argv) do { main_loop_wait(false); if (state =3D=3D TERMINATE) { - blk_exp_close_all(); + blk_exp_close_all(true); state =3D TERMINATED; } } while (state !=3D TERMINATED); diff --git a/stubs/blk-exp-close-all.c b/stubs/blk-exp-close-all.c index 1c71316763..ecd0ce611f 100644 --- a/stubs/blk-exp-close-all.c +++ b/stubs/blk-exp-close-all.c @@ -2,6 +2,6 @@ #include "block/export.h" =20 /* Only used in programs that support block exports (libblockdev.fa) */ -void blk_exp_close_all(void) +void blk_exp_close_all(bool wait) { } --=20 2.26.2