From patchwork Wed Mar 27 21:31:27 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Nicholas A. Bellinger" X-Patchwork-Id: 231840 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 6ED902C00AE for ; Thu, 28 Mar 2013 08:31:59 +1100 (EST) Received: from localhost ([::1]:60764 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UKxwr-00059T-Mt for incoming@patchwork.ozlabs.org; Wed, 27 Mar 2013 17:31:57 -0400 Received: from eggs.gnu.org ([208.118.235.92]:45446) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UKxwT-000560-Vr for qemu-devel@nongnu.org; Wed, 27 Mar 2013 17:31:35 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UKxwR-0007xC-HJ for qemu-devel@nongnu.org; Wed, 27 Mar 2013 17:31:33 -0400 Received: from mail.linux-iscsi.org ([67.23.28.174]:36920 helo=linux-iscsi.org) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UKxwR-0007x6-Ae for qemu-devel@nongnu.org; Wed, 27 Mar 2013 17:31:31 -0400 Received: from [192.168.1.68] (75-37-193-228.lightspeed.lsatca.sbcglobal.net [75.37.193.228]) (using SSLv3 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) (Authenticated sender: nab) by linux-iscsi.org (Postfix) with ESMTPSA id 7909322D9D0; Wed, 27 Mar 2013 21:20:27 +0000 (UTC) From: "Nicholas A. Bellinger" To: "Michael S. Tsirkin" In-Reply-To: <20130320095140.GA16615@redhat.com> References: <1363653285-23776-1-git-send-email-asias@redhat.com> <1363653285-23776-4-git-send-email-asias@redhat.com> <20130319084057.GB24393@stefanha-thinkpad.redhat.com> <1363744628.13070.28.camel@haakon2.linux-iscsi.org> <20130320095140.GA16615@redhat.com> Date: Wed, 27 Mar 2013 14:31:27 -0700 Message-ID: <1364419887.17698.19.camel@haakon2.linux-iscsi.org> Mime-Version: 1.0 X-Mailer: Evolution 2.30.3 X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.4.x-2.6.x [generic] X-Received-From: 67.23.28.174 Cc: kvm@vger.kernel.org, Stefan Hajnoczi , qemu-devel@nongnu.org, virtualization@lists.linux-foundation.org, target-devel@vger.kernel.org, Stefan Hajnoczi , Paolo Bonzini , Asias He Subject: Re: [Qemu-devel] [PATCH V3 WIP 3/3] disable vhost_verify_ring_mappings check 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On Wed, 2013-03-20 at 11:51 +0200, Michael S. Tsirkin wrote: > On Tue, Mar 19, 2013 at 06:57:08PM -0700, Nicholas A. Bellinger wrote: > > On Tue, 2013-03-19 at 09:40 +0100, Stefan Hajnoczi wrote: > > > On Tue, Mar 19, 2013 at 08:34:45AM +0800, Asias He wrote: > > > > --- > > > > hw/vhost.c | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/hw/vhost.c b/hw/vhost.c > > > > index 4d6aee3..0c52ec4 100644 > > > > --- a/hw/vhost.c > > > > +++ b/hw/vhost.c > > > > @@ -421,10 +421,12 @@ static void vhost_set_memory(MemoryListener *listener, > > > > return; > > > > } > > > > > > > > +#if 0 > > > > if (dev->started) { > > > > r = vhost_verify_ring_mappings(dev, start_addr, size); > > > > assert(r >= 0); > > > > } > > > > +#endif > > > > > > Please add a comment to explain why. > > > > > > > Btw, the output that Asias added in the failure case at the behest of > > MST is here: > > > > http://www.spinics.net/lists/target-devel/msg04077.html > > Yes I suspected we could get l > ring_size, but this is > not the case here. > Hi MST & Co, A quick update here.. So this issue appears to be related to performing the vhost_verify_ring_mappings() call after vhost_dev_unassign_memory() has been invoked with vhost_set_memory(..., add=false). AFAICT from the logs below, things appear to work as expected when vhost_verify_ring_mappings() is called only for the vhost_set_memory(..., add=true) case. Calling vhost_verify_ring_mappings() when dev->started == true + vhost_set_memory(..., add=false) appears to be a bug caused by fallout from: commit 24f4fe345c1b80bab1ee18573914123d8028a9e6 Author: Michael S. Tsirkin Date: Tue Dec 25 17:41:07 2012 +0200 vhost: set started flag while start is in progress I'm including the following patch in the forth-coming vhost-scsi series. Please let me know if you have any concerns. Thanks! --nab vhost_set_memory: section: 0x7f2249986b60 section->size: 2146697216 add: 0 Before vhost_verify_ring_mappings: start_addr: c0000 size: 2146697216 Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028 Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028 Checking vq: 2 ring_phys: ee000 ring_size: 5124 >>>>>>>>>>>>>>>>>>. Calling l: 5124 for start_addr: c0000 for vq 2 Unable to map ring buffer for ring 2 l: 4096 ring_size: 5124 vhost_set_memory: section: 0x7f2249986aa0 section->size: 32768 add: 1 Before vhost_verify_ring_mappings: start_addr: c0000 size: 32768 Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028 Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028 Checking vq: 2 ring_phys: ee000 ring_size: 5124 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 2 ring_phys: ee000 ring_size: 5124 vhost_set_memory: section: 0x7f2249986aa0 section->size: 2146664448 add: 1 Before vhost_verify_ring_mappings: start_addr: c8000 size: 2146664448 Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028 Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028 Checking vq: 2 ring_phys: ee000 ring_size: 5124 >>>>>>>>>>>>>>>>>>. Calling l: 5124 for start_addr: c8000 for vq 2 vhost_set_memory: section: 0x7f2249986b60 section->size: 32768 add: 0 Before vhost_verify_ring_mappings: start_addr: c0000 size: 32768 Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028 Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028 Checking vq: 2 ring_phys: ee000 ring_size: 5124 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 2 ring_phys: ee000 ring_size: 5124 vhost_set_memory: section: 0x7f2249986b60 section->size: 2146664448 add: 0 Before vhost_verify_ring_mappings: start_addr: c8000 size: 2146664448 Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028 Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028 Checking vq: 2 ring_phys: ee000 ring_size: 5124 >>>>>>>>>>>>>>>>>>. Calling l: 5124 for start_addr: c8000 for vq 2 Unable to map ring buffer for ring 2 l: 0 ring_size: 5124 vhost_set_memory: section: 0x7f2249986aa0 section->size: 36864 add: 1 Before vhost_verify_ring_mappings: start_addr: c0000 size: 36864 Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028 Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028 Checking vq: 2 ring_phys: ee000 ring_size: 5124 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 2 ring_phys: ee000 ring_size: 5124 vhost_set_memory: section: 0x7f2249986aa0 section->size: 2146660352 add: 1 Before vhost_verify_ring_mappings: start_addr: c9000 size: 2146660352 Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028 Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028 Checking vq: 2 ring_phys: ee000 ring_size: 5124 >>>>>>>>>>>>>>>>>>. Calling l: 5124 for start_addr: c9000 for vq 2 vhost_set_memory: section: 0x7f2249986b60 section->size: 2146660352 add: 0 Before vhost_verify_ring_mappings: start_addr: c9000 size: 2146660352 Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028 Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028 Checking vq: 2 ring_phys: ee000 ring_size: 5124 >>>>>>>>>>>>>>>>>>. Calling l: 5124 for start_addr: c9000 for vq 2 Unable to map ring buffer for ring 2 l: 0 ring_size: 5124 vhost_set_memory: section: 0x7f2249986aa0 section->size: 159744 add: 1 Before vhost_verify_ring_mappings: start_addr: c9000 size: 159744 Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028 Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028 Checking vq: 2 ring_phys: ee000 ring_size: 5124 >>>>>>>>>>>>>>>>>>. Calling l: 5124 for start_addr: c9000 for vq 2 vhost_set_memory: section: 0x7f2249986aa0 section->size: 65536 add: 1 Before vhost_verify_ring_mappings: start_addr: f0000 size: 65536 Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028 Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028 Checking vq: 2 ring_phys: ee000 ring_size: 5124 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 2 ring_phys: ee000 ring_size: 5124 vhost_set_memory: section: 0x7f2249986aa0 section->size: 2146435072 add: 1 Before vhost_verify_ring_mappings: start_addr: 100000 size: 2146435072 Checking vq: 0 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 0 ring_phys: 0 ring_size: 1028 Checking vq: 1 ring_phys: 0 ring_size: 1028 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 1 ring_phys: 0 ring_size: 1028 Checking vq: 2 ring_phys: ee000 ring_size: 5124 >>>>>>>>>>>>>>>>>>. Got ranges_overlap for vq: 2 ring_phys: ee000 ring_size: 5124 diff --git a/hw/vhost.c b/hw/vhost.c index 4d6aee3..687a689 100644 --- a/hw/vhost.c +++ b/hw/vhost.c @@ -421,7 +421,7 @@ static void vhost_set_memory(MemoryListener *listener, return; } - if (dev->started) { + if (dev->started && add) { r = vhost_verify_ring_mappings(dev, start_addr, size); assert(r >= 0); }