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

[Jan Kiszka <jan.kiszka@siemens.com>]

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