From patchwork Thu Mar 24 17:06:21 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Bonzini X-Patchwork-Id: 8663051 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 7D4F7C0553 for ; Thu, 24 Mar 2016 17:06:43 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 9BDFD20379 for ; Thu, 24 Mar 2016 17:06:42 +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 CD65D20351 for ; Thu, 24 Mar 2016 17:06:36 +0000 (UTC) Received: from localhost ([::1]:51966 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aj8iS-00072o-5P for patchwork-qemu-devel@patchwork.kernel.org; Thu, 24 Mar 2016 13:06:36 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40215) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aj8iK-000710-91 for qemu-devel@nongnu.org; Thu, 24 Mar 2016 13:06:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aj8iG-0000GT-4b for qemu-devel@nongnu.org; Thu, 24 Mar 2016 13:06:28 -0400 Received: from mail-wm0-x244.google.com ([2a00:1450:400c:c09::244]:33005) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aj8iF-0000GO-OK for qemu-devel@nongnu.org; Thu, 24 Mar 2016 13:06:24 -0400 Received: by mail-wm0-x244.google.com with SMTP id u125so14288382wmg.0 for ; Thu, 24 Mar 2016 10:06:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=diMWrvzzwgEhzS97QK+1mO6FNINhssxXQJrDBnWVUVo=; b=L7daH7HJTDh0jyVnFZOup5+1FAVFnbx09jVDlxdbgXgYcJhllzMCoe2wpnWtlBiTbD K1ly6bSfvY/QoFPb9kdtsYsrfoGCHo68+efZG7CyiFO4IVQz124CMkSUVqy593ef0Xsg FqUrCP2/ljoTpkLGdxNzS8/mut3tUSQ8lNYRtrmN1h4uDdpz7rLFD42BoIN5HzVSvk+P i5Zwnh45FApSjO3faAid4nUc59ZM00rri5NAEIzAhf/bPLSeAvDljdelP4pccBx9ADMx nuI4e57yJs6eGuysu3Wgi68U3L/50NDaN7jnSiQD6wL8xZvi40TKRbTPQZ2qwkMHcXri t+2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:subject:to:references:cc:from:message-id :date:user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=diMWrvzzwgEhzS97QK+1mO6FNINhssxXQJrDBnWVUVo=; b=P52aQM0pulFAWZ4ouSfOIrGbu6K4M2jwaxgImG3tMBp3VfQi/5dve1Bd0Q/yYrl7U2 6SVSMKpTWco+mHAULl/bFlzNEc8Rj0hb6YU9d+eg1LNdWC/ibTdliu1iCn5a5haXbHTv YlOFV2KzsxKA0Itd3C6Tju+OzD/MjRgZqm/Bsob6UXaubohqPOGyt9Ru7G500rLSbV+o PGp56TepMKoNIPuvXdZlZm9JayEF8n/gVOFrD7NAk20NwTJvRVLhqWATncUKZu127GE0 IEypdrBnKEkTSReY01fIEVFqwF5ix4VNLiRyKjs2nO5hl3JBmhcy7JyGMPx/SPf0fGBs t/BA== X-Gm-Message-State: AD7BkJIQnUi0IwgqABVn/ISgcVoLF8iArjqv9jm1Ng4wPKysuE+hZ6tEZ2SbIxJfu+21Cw== X-Received: by 10.194.120.229 with SMTP id lf5mr12037601wjb.151.1458839182998; Thu, 24 Mar 2016 10:06:22 -0700 (PDT) Received: from [192.168.10.150] (94-39-161-17.adsl-ull.clienti.tiscali.it. [94.39.161.17]) by smtp.googlemail.com with ESMTPSA id xx3sm8222859wjc.32.2016.03.24.10.06.21 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 24 Mar 2016 10:06:22 -0700 (PDT) To: Cornelia Huck , qemu-devel@nongnu.org References: <1458836125-73613-1-git-send-email-cornelia.huck@de.ibm.com> From: Paolo Bonzini Message-ID: <56F41E8D.5000609@redhat.com> Date: Thu, 24 Mar 2016 18:06:21 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <1458836125-73613-1-git-send-email-cornelia.huck@de.ibm.com> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 2a00:1450:400c:c09::244 Cc: tubo@linux.vnet.ibm.com, borntraeger@de.ibm.com, famz@redhat.com, stefanha@redhat.com, mst@redhat.com Subject: Re: [Qemu-devel] [PATCH 0/6] virtio: refactor host notifiers 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.8 required=5.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=ham 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 On 24/03/2016 17:15, Cornelia Huck wrote: > Here's the next version of my refactoring of the virtio host notifiers. > This one actually survives a bit of testing for me (reboot loop). > > As this patchset fixes a latent bug exposed by the recent dataplane > changes (we have a deassigned ioeventfd for a short period of time > during dataplane start, which leads to the virtqueue handler being > called in both the vcpu thread and the iothread simultaneously), I'd > like to see this in 2.6. Tested-by: Paolo Bonzini Resisted 6 minutes versus 10 seconds. At about 2.5 seconds per reboot, that means the failure happened at the fourth reboot before, and resisted about 150 reboots with your patches. My testcase was to add "systemd.unit=reboot.target" to a Fedora 21's kernel command line and run the following ./+build/x86_64-softmmu/qemu-system-x86_64 --enable-kvm -m 512 \ -smp 4 -serial mon:stdio -display none -object iothread,id=io \ -drive if=none,id=hd,file=/vm/virt_test/images/jeos-21-64.qcow2 \ -device virtio-blk-pci,drive=hd,iothread=io \ -drive if=none,file=null-co://,id=n1 \ -drive if=none,file=null-co://,id=n2 \ -drive if=none,file=null-co://,id=n3 \ -drive if=none,file=null-co://,id=n4 \ -drive if=none,file=null-co://,id=n5 \ -drive if=none,file=null-co://,id=n6 \ -drive if=none,file=null-co://,id=n7 \ -drive if=none,file=null-co://,id=n8 \ -device virtio-blk-pci,iothread=io,drive=n1 \ -device virtio-blk-pci,iothread=io,drive=n2 \ -device virtio-blk-pci,iothread=io,drive=n3 \ -device virtio-blk-pci,iothread=io,drive=n4 \ -device virtio-blk-pci,iothread=io,drive=n5 \ -device virtio-blk-pci,iothread=io,drive=n6 \ -device virtio-blk-pci,iothread=io,drive=n7 \ -device virtio-blk-pci,iothread=io,drive=n8 with the assertion patch applied: Thanks for your help! Paolo > Changes from RFC: > - Fixed some silly errors (checking for !disabled instead of disabled, > virtio_ccw_stop_ioeventfd() calling virtio_bus_start_ioeventfd()). > - Completely reworked set_host_notifier(): We only want to set/unset > the actual handler function and don't want to do anything to the > ioeventfd backing, so reduce the function to actually doing only > that. > - With the change above, we can lose the 'assign' parameter in > virtio_bus_stop_ioeventfd() again. > - Added more comments that hopefully make it clearer what is going on. > > I'd appreciate it if people could give it some testing; I'll be back > to look at the fallout after Easter. > > Cornelia Huck (6): > virtio-bus: common ioeventfd infrastructure > virtio-bus: have callers tolerate new host notifier api > virtio-ccw: convert to ioeventfd callbacks > virtio-pci: convert to ioeventfd callbacks > virtio-mmio: convert to ioeventfd callbacks > virtio-bus: remove old set_host_notifier callback > > hw/block/dataplane/virtio-blk.c | 6 +- > hw/s390x/virtio-ccw.c | 133 ++++++++++++++-------------------------- > hw/scsi/virtio-scsi-dataplane.c | 9 ++- > hw/virtio/vhost.c | 13 ++-- > hw/virtio/virtio-bus.c | 132 +++++++++++++++++++++++++++++++++++++++ > hw/virtio/virtio-mmio.c | 128 +++++++++++++------------------------- > hw/virtio/virtio-pci.c | 124 +++++++++++++------------------------ > include/hw/virtio/virtio-bus.h | 31 +++++++++- > 8 files changed, 303 insertions(+), 273 deletions(-) > diff --git a/hw/block/virtio-blk.c b/hw/block/virtio-blk.c index cb710f1..d0b8248 100644 --- a/hw/block/virtio-blk.c +++ b/hw/block/virtio-blk.c @@ -591,6 +591,7 @@ return; } + assert(atomic_fetch_inc(&s->reentrancy_test) == 0); blk_io_plug(s->blk); while ((req = virtio_blk_get_request(s))) { @@ -602,6 +603,7 @@ } blk_io_unplug(s->blk); + atomic_dec(&s->reentrancy_test); } static void virtio_blk_dma_restart_bh(void *opaque) diff --git a/include/hw/virtio/virtio-blk.h b/include/hw/virtio/virtio-blk.h index ae84d92..5cb66cd 100644 --- a/include/hw/virtio/virtio-blk.h +++ b/include/hw/virtio/virtio-blk.h @@ -54,6 +54,7 @@ typedef struct VirtIOBlock { bool original_wce; VMChangeStateEntry *change; bool dataplane_started; + int reentrancy_test; struct VirtIOBlockDataPlane *dataplane; } VirtIOBlock;