From patchwork Mon Dec 14 17:05:16 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Lopez X-Patchwork-Id: 11972513 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id BA61CC4361B for ; Mon, 14 Dec 2020 17:13:45 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3994022515 for ; Mon, 14 Dec 2020 17:13:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3994022515 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:49008 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1korPq-0002qV-1h for qemu-devel@archiver.kernel.org; Mon, 14 Dec 2020 12:13:42 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:52942) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1korIA-0001aX-CP for qemu-devel@nongnu.org; Mon, 14 Dec 2020 12:05:46 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:44577) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1korI7-0005Ux-4T for qemu-devel@nongnu.org; Mon, 14 Dec 2020 12:05:46 -0500 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= 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 X-MC-Unique: s8NqP3znNFanUNxldhXiHw-1 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) From: Sergio Lopez To: qemu-devel@nongnu.org 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 Received-SPF: pass client-ip=63.128.21.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_H4=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+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" The documentation for bdrv_set_aio_context_ignore() states this: * The caller must own the AioContext lock for the old AioContext of bs, but 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-blk.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 = vblk->dataplane; BusState *qbus = BUS(qdev_get_parent_bus(DEVICE(vblk))); VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus); + AioContext *old_context; unsigned i; unsigned nvqs = s->conf->num_queues; Error *local_err = NULL; @@ -214,7 +215,10 @@ int virtio_blk_data_plane_start(VirtIODevice *vdev) vblk->dataplane_started = true; trace_virtio_blk_data_plane_start(s); + old_context = blk_get_aio_context(s->conf->conf.blk); + aio_context_acquire(old_context); r = 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 *dataplane, { ERRP_GUARD(); XenDevice *xendev = dataplane->xendev; + AioContext *old_context; unsigned int ring_size; unsigned int i; @@ -808,10 +809,14 @@ void xen_block_dataplane_start(XenBlockDataPlane *dataplane, goto stop; } - aio_context_acquire(dataplane->ctx); + old_context = 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 *hotplug_dev, DeviceState *dev, VirtIODevice *vdev = VIRTIO_DEVICE(hotplug_dev); VirtIOSCSI *s = VIRTIO_SCSI(vdev); SCSIDevice *sd = SCSI_DEVICE(dev); + AioContext *old_context; int ret; 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 = blk_get_aio_context(sd->conf.blk); + aio_context_acquire(old_context); ret = blk_set_aio_context(sd->conf.blk, s->ctx, errp); - virtio_scsi_release(s); + aio_context_release(old_context); if (ret < 0) { return; } From patchwork Mon Dec 14 17:05:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Lopez X-Patchwork-Id: 11972507 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B253BC4361B for ; Mon, 14 Dec 2020 17:08:15 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2C63F22510 for ; Mon, 14 Dec 2020 17:08:14 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2C63F22510 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:33400 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1korKY-0004RE-2c for qemu-devel@archiver.kernel.org; Mon, 14 Dec 2020 12:08:14 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:53212) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1korJ2-0002ZB-CX for qemu-devel@nongnu.org; Mon, 14 Dec 2020 12:06:41 -0500 Received: from us-smtp-delivery-124.mimecast.com ([63.128.21.124]:40003) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1korIz-0005bD-7q for qemu-devel@nongnu.org; Mon, 14 Dec 2020 12:06:40 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607965595; 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=XOVULEIEztPPiWzmO3yAaQsuuahBcplObOAWvg53dr7I0atWr2bQqXjfEI7rMMP6pJYe5i ovS1GQjb0j7wdL8VJHrKqsjGgGkK4nzKv3prx7Kd3Q7IbAO/FqQmybg4WmWQcDdzmF3AeR Uqz8Lm1uQiEWlbFGdxpMCgn181CEfG8= 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 X-MC-Unique: D0CmhjvTOY6pEL_0tz-PPA-1 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) From: Sergio Lopez To: qemu-devel@nongnu.org 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 Received-SPF: pass client-ip=63.128.21.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_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham 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+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" 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); QLIST_FOREACH(child, &bs->children, next) { - if (g_slist_find(*ignore, child)) { + if (g_slist_find(*ignore, child) || g_slist_find(*ignore, child->bs)) { continue; } *ignore = 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 = g_slist_prepend(*ignore, bs); QLIST_FOREACH(child, &bs->parents, next_parent) { if (g_slist_find(*ignore, child)) { continue; From patchwork Mon Dec 14 17:05:18 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Lopez X-Patchwork-Id: 11972517 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1B10BC4361B for ; Mon, 14 Dec 2020 17:17:37 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5B72222515 for ; Mon, 14 Dec 2020 17:17:36 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5B72222515 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:58438 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1korTb-0006x9-7c for qemu-devel@archiver.kernel.org; Mon, 14 Dec 2020 12:17:35 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]:53298) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1korJC-0002ju-K8 for qemu-devel@nongnu.org; Mon, 14 Dec 2020 12:06:50 -0500 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:58093) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.90_1) (envelope-from ) id 1korJ8-0005cy-MX for qemu-devel@nongnu.org; Mon, 14 Dec 2020 12:06:48 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1607965605; 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=XGtHvEPw7Jfb3yHh43PltROd989+BxPcV6mWJuV9PknM6Y6tYdjQC1HT7qyJa/Z0rPZ5Hu XElu1c9/ciRQHdtPsDYyQtHgpu3v10H/gQE7RyMzW6goYl98PMLcAfF11gsgIrwGjdi1Vw GHuW3ygR/2NypZS9grdwHxVzCpmXiOg= 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 X-MC-Unique: 761DnAutO9qHxmbuLzXzFA-1 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) From: Sergio Lopez To: qemu-devel@nongnu.org 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 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+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" 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=1900326 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; + 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 *client, Error **errp) return 0; } -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 = false; + + assert(size); + while (size > 0) { + struct iovec iov = { .iov_base = buffer, .iov_len = size }; + ssize_t len; + + len = qio_channel_readv(client->ioc, &iov, 1, errp); + if (len == QIO_CHANNEL_ERR_BLOCK) { + client->read_yielding = true; + qio_channel_yield(client->ioc, G_IO_IN); + client->read_yielding = false; + if (client->quiescing) { + return -EAGAIN; + } + continue; + } else if (len < 0) { + return -EIO; + } else if (len == 0) { + if (partial) { + error_setg(errp, + "Unexpected end-of-file before all bytes were read"); + return -EIO; + } else { + return 0; + } + } + + partial = true; + size -= len; + buffer = (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; - ret = nbd_read(ioc, buf, sizeof(buf), "request", errp); + ret = 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) QTAILQ_FOREACH(client, &exp->clients, next) { qio_channel_attach_aio_context(client->ioc, ctx); + + assert(client->recv_coroutine == NULL); + assert(client->send_coroutine == NULL); + + if (client->quiescing) { + client->quiescing = false; + nbd_client_receive_next_request(client); + } + } +} + +static void nbd_aio_detach_bh(void *opaque) +{ + NBDExport *exp = opaque; + NBDClient *client; + + QTAILQ_FOREACH(client, &exp->clients, next) { + qio_channel_detach_aio_context(client->ioc); + client->quiescing = 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 != NULL); + } } + if (client->send_coroutine) { - aio_co_schedule(ctx, client->send_coroutine); + AIO_WAIT_WHILE(exp->common.ctx, client->send_coroutine != NULL); } } } @@ -1492,13 +1567,10 @@ static void blk_aio_attached(AioContext *ctx, void *opaque) static void blk_aio_detach(void *opaque) { NBDExport *exp = opaque; - NBDClient *client; trace_nbd_blk_aio_detach(exp->name, exp->common.ctx); - 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); exp->common.ctx = NULL; } @@ -2151,20 +2223,23 @@ static int nbd_co_send_bitmap(NBDClient *client, uint64_t handle, /* 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 reporting - * 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 error + * 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 = req->client; int valid_flags; + int ret; g_assert(qemu_in_coroutine()); assert(client->recv_coroutine == qemu_coroutine_self()); - if (nbd_receive_request(client->ioc, request, errp) < 0) { - return -EIO; + ret = nbd_receive_request(client, request, errp); + if (ret < 0) { + return ret; } trace_nbd_co_receive_request_decode_type(request->handle, request->type, @@ -2507,6 +2582,17 @@ static coroutine_fn void nbd_trip(void *opaque) return; } + 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 = NULL; + aio_wait_kick(); + return; + } + req = nbd_request_get(client); ret = nbd_co_receive_request(req, &request, &local_err); client->recv_coroutine = NULL; @@ -2519,6 +2605,11 @@ static coroutine_fn void nbd_trip(void *opaque) goto done; } + if (ret == -EAGAIN) { + assert(client->quiescing); + goto done; + } + nbd_client_receive_next_request(client); if (ret == -EIO) { goto disconnect; @@ -2565,7 +2656,8 @@ disconnect: 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 = qemu_coroutine_create(nbd_trip, client); aio_co_schedule(client->exp->common.ctx, client->recv_coroutine); From patchwork Mon Dec 14 17:05:19 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sergio Lopez X-Patchwork-Id: 11972511 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1C2A4C2BB40 for ; Mon, 14 Dec 2020 17:11:46 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 56003224DF for ; Mon, 14 Dec 2020 17:11:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 56003224DF Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:43558 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1korNw-0000ZW-6T for qemu-devel@archiver.kernel.org; Mon, 14 Dec 2020 12:11:44 -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 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= 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 X-MC-Unique: _8-ouXvpNqK8vTFc4U2Pjg-1 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) 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 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+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" 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=1900505 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) == 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 implies + * 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); /* Drop references from requests still in flight, such as canceled block * jobs whose AIO context has not been polled yet */ bdrv_drain_all(); 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(); 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) } /* type == 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; @@ -293,12 +293,14 @@ void blk_exp_close_all_type(BlockExportType type) blk_exp_request_shutdown(exp); } - AIO_WAIT_WHILE(NULL, blk_exp_has_type(type)); + if (wait) { + AIO_WAIT_WHILE(NULL, blk_exp_has_type(type)); + } } -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); } 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; } - blk_exp_close_all_type(BLOCK_EXPORT_TYPE_NBD); + blk_exp_close_all_type(BLOCK_EXPORT_TYPE_NBD, true); nbd_server_free(nbd_server); nbd_server = 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); #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 == TERMINATE) { - blk_exp_close_all(); + blk_exp_close_all(true); state = TERMINATED; } } while (state != 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" /* Only used in programs that support block exports (libblockdev.fa) */ -void blk_exp_close_all(void) +void blk_exp_close_all(bool wait) { }