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: 2353421 Return-Path: X-Original-To: patchwork-kvm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id 066553FC8C for ; Wed, 27 Mar 2013 21:31:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754579Ab3C0Vbc (ORCPT ); Wed, 27 Mar 2013 17:31:32 -0400 Received: from mail.linux-iscsi.org ([67.23.28.174]:60056 "EHLO linux-iscsi.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754284Ab3C0Vbb (ORCPT ); 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) Subject: Re: [PATCH V3 WIP 3/3] disable vhost_verify_ring_mappings check From: "Nicholas A. Bellinger" To: "Michael S. Tsirkin" Cc: Stefan Hajnoczi , Asias He , qemu-devel@nongnu.org, kvm@vger.kernel.org, virtualization@lists.linux-foundation.org, target-devel@vger.kernel.org, Stefan Hajnoczi , Paolo Bonzini 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 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.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 --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html 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); }