Re: [PATCH 2/2] KVM: Convert read-only users of vm_list to RCU

Avi Kivity
2011-02-10T10:16:40Z

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, and to change all list operations on vm_list
to rcu variants.

>
> @@ -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().

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.

> + raw_spin_unlock(&kvm_lock);
> + }
>
> - raw_spin_unlock(&kvm_lock);
> + rcu_read_unlock();
>

--
error compiling committee.c: too many arguments to function