From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: [RFC] need to improve slot creation/destruction? -- Re: [RFC][PATCH] srcu: Implement call_srcu() Date: Thu, 09 Feb 2012 16:23:05 +0200 Message-ID: <4F33D6C9.2070403@redhat.com> References: <1328016724.2446.229.camel@twins> <4F27F0E6.1040309@redhat.com> <1328017807.2446.230.camel@twins> <20120131222447.GH2391@linux.vnet.ibm.com> <1328091749.2760.34.camel@laptop> <4F29178A.1090306@redhat.com> <4F2918D5.4050104@redhat.com> <20120201135020.GB18998@amt.cnet> <20120209004320.5772daa0472aef4700dab1b6@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Cc: mtosatti@redhat.com, kvm@vger.kernel.org To: Takuya Yoshikawa Return-path: Received: from mx1.redhat.com ([209.132.183.28]:33665 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753780Ab2BIOXI (ORCPT ); Thu, 9 Feb 2012 09:23:08 -0500 In-Reply-To: <20120209004320.5772daa0472aef4700dab1b6@gmail.com> Sender: kvm-owner@vger.kernel.org List-ID: On 02/08/2012 05:43 PM, Takuya Yoshikawa wrote: > [Dropped non-kvm members from cc] > > Marcelo Tosatti wrote: > > > VGABIOS mode constantly destroys and creates 0xa0000 slot, so > > performance is required for KVM_SET_MEM too (it can probably be fixed in > > qemu, but older qemu's must be supported). > > Apart from srcu, I see some problems concerning slot creation/destruction: > heavy shadow flush (and extra write protection). > > > Look at __kvm_set_memory_region(). > > 1. When we invalidate a memory slot, we call kvm_arch_flush_shadow() and > zap all shadow pages, not restricted to that slot. > > /* From this point no new shadow pages pointing to a deleted > * memslot will be created. > * > * validation of sp->gfn happens in: > * - gfn_to_hva (kvm_read_guest, gfn_to_pfn) > * - kvm_is_visible_gfn (mmu_check_roots) > */ > kvm_arch_flush_shadow(kvm); The workloads that exercise slot removal heavily usually do so in a tight loop, so flushing all shadow is not too bad (not much to flush). > > 2. When we create(and shift?) a memory slot, we call kvm_arch_flush_shadow() > to clear all mmio sptes, again not restricted to that slot. > > /* > * If the new memory slot is created, we need to clear all > * mmio sptes. > */ > if (npages && old.base_gfn != mem->guest_phys_addr >> PAGE_SHIFT) > kvm_arch_flush_shadow(kvm); This is pretty rare outside the previous scenario (memory/pci hotplug). > > 3. In addition to these, we do write-protect all pages in that slot in > kvm_arch_commit_memory_region() every time. > > > For 1: I made a patch to restrict the flush to that slot by using sp->slot_bitmap. > (seems working here) > > For 2: I think we can do the same: not 100% sure yet because I am still > struggling with the "mmio spte optimization" code. (really hacky ...) > > For 3: I think doing both "write protection" and "shadow flush" is unnecessary. Maybe it only needs to be done if the only change is enabling dirty logging. > BTW do we really need fast slot creation/destruction? Not really, but it's good to have infrastructure that copes with different workloads. If the patches keep the code simple I think it's a good thing to have. -- error compiling committee.c: too many arguments to function