From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [PATCH 2/2] KVM: Convert read-only users of vm_list to RCU Date: Thu, 10 Feb 2011 12:31:55 +0100 Message-ID: <4D53CCAB.8040204@siemens.com> References: <4D512EF7.8040409@siemens.com> <4D512F3B.1080107@siemens.com> <4D53BB02.20206@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Cc: Marcelo Tosatti , kvm , Zachary Amsden To: Avi Kivity Return-path: Received: from thoth.sbs.de ([192.35.17.2]:18969 "EHLO thoth.sbs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751969Ab1BJLcK (ORCPT ); Thu, 10 Feb 2011 06:32:10 -0500 In-Reply-To: <4D53BB02.20206@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 2011-02-10 11:16, Avi Kivity wrote: > On 02/08/2011 01:55 PM, Jan Kiszka wrote: >> Only for walking the list of VMs, we do not need to hold the preemption >> disabling kvm_lock. Convert stat services, the cpufreq callback and >> mmu_shrink to RCU. For the latter, special care is required to >> synchronize its list_move_tail with kvm_destroy_vm. >> >> >> diff --git a/arch/x86/kvm/mmu.c b/arch/x86/kvm/mmu.c >> index b6a9963..e9d0ed8 100644 >> --- a/arch/x86/kvm/mmu.c >> +++ b/arch/x86/kvm/mmu.c >> @@ -3587,9 +3587,9 @@ static int mmu_shrink(struct shrinker *shrink, int nr_to_scan, gfp_t gfp_mask) >> if (nr_to_scan == 0) >> goto out; >> >> - raw_spin_lock(&kvm_lock); >> + rcu_read_lock(); >> >> - list_for_each_entry(kvm,&vm_list, vm_list) { >> + list_for_each_entry_rcu(kvm,&vm_list, vm_list) { >> int idx, freed_pages; >> LIST_HEAD(invalid_list); > > Have to #include rculist.h, OK. > and to change all list operations on vm_list > to rcu variants. Not sure if we have such variants for all cases... > >> >> @@ -3607,10 +3607,14 @@ static int mmu_shrink(struct shrinker *shrink, int nr_to_scan, gfp_t gfp_mask) >> spin_unlock(&kvm->mmu_lock); >> srcu_read_unlock(&kvm->srcu, idx); >> } >> - if (kvm_freed) >> - list_move_tail(&kvm_freed->vm_list,&vm_list); >> + if (kvm_freed) { >> + raw_spin_lock(&kvm_lock); >> + if (!kvm->deleted) >> + list_move_tail(&kvm_freed->vm_list,&vm_list); > > There is no list_move_tail_rcu(). ...specifically not for this one. > > Why check kvm->deleted? it's in the process of being torn down anyway, > it doesn't matter if mmu_shrink or kvm_destroy_vm pulls the trigger. kvm_destroy_vm removes a vm from the list while mmu_shrink is running. Then mmu_shrink's list_move_tail will re-add that vm to the list tail again (unless already the removal in move_tail produces a crash). Jan -- Siemens AG, Corporate Technology, CT T DE IT 1 Corporate Competence Center Embedded Linux