From mboxrd@z Thu Jan 1 00:00:00 1970 From: Avi Kivity Subject: Re: pre-cleanup for SMP: move mmu cache to be VM bais Date: Thu, 31 May 2007 11:03:22 +0300 Message-ID: <465E814A.3070204@qumranet.com> References: <10EA09EFD8728347A513008B6B0DA77A0188858D@pdsmsx411.ccr.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: kvm-devel To: "Dong, Eddie" Return-path: In-Reply-To: <10EA09EFD8728347A513008B6B0DA77A0188858D-wq7ZOvIWXbNpB2pF5aRoyrfspsVTdybXVpNB7YpNyf8@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org Errors-To: kvm-devel-bounces-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org List-Id: kvm.vger.kernel.org Dong, Eddie wrote: > Move per VCPU mmu_memory_cache to be VM basis. > > Your opnion? > > Eddie > > diff --git a/drivers/kvm/kvm.h b/drivers/kvm/kvm.h > index 0632d0b..77989b4 100644 > --- a/drivers/kvm/kvm.h > +++ b/drivers/kvm/kvm.h > @@ -300,11 +300,6 @@ struct kvm_vcpu { > > struct kvm_mmu mmu; > > - struct kvm_mmu_memory_cache mmu_pte_chain_cache; > - struct kvm_mmu_memory_cache mmu_rmap_desc_cache; > - struct kvm_mmu_memory_cache mmu_page_cache; > - struct kvm_mmu_memory_cache mmu_page_header_cache; > - > gfn_t last_pt_write_gfn; > int last_pt_write_count; > > @@ -383,6 +378,11 @@ struct kvm { > unsigned long rmap_overflow; > struct list_head vm_list; > struct file *filp; > + > + struct kvm_mmu_memory_cache mmu_pte_chain_cache; > + struct kvm_mmu_memory_cache mmu_rmap_desc_cache; > + struct kvm_mmu_memory_cache mmu_page_cache; > + struct kvm_mmu_memory_cache mmu_page_header_cache; > }; > > This exposes a race: > > static int mmu_topup_memory_caches(struct kvm_vcpu *vcpu) > { > int r; > > r = __mmu_topup_memory_caches(vcpu, GFP_NOWAIT); > if (r < 0) { > spin_unlock(&vcpu->kvm->lock); > kvm_arch_ops->vcpu_put(vcpu); > r = __mmu_topup_memory_caches(vcpu, GFP_KERNEL); > kvm_arch_ops->vcpu_load(vcpu); > spin_lock(&vcpu->kvm->lock); > } > return r; > } If, after we've allocated but before we grab the lock back, another vcpu comes along and steals our memory, we can oom with no recovery. This can be fixed by having a while() in there, but I don't see a real need. The memory caches are local to the vcpu so they have some modest performance benefit (although it's probably not meausrable given all the other work this code path does). -- error compiling committee.c: too many arguments to function ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/