On 06/07/2017 16:55, Christian Borntraeger wrote:
> On 07/06/2017 04:45 PM, Christian Borntraeger wrote:
>> we access the memslots array via srcu. Mark it as such and
>> use the right access functions also for the freeing of
>> memory slots.
>>
>> Found by sparse:
>> ./include/linux/kvm_host.h:565:16: error: incompatible types in
>> comparison expression (different address spaces)
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
>> ---
>> include/linux/kvm_host.h | 2 +-
>> virt/kvm/kvm_main.c | 4 ++--
>> 2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
>> index bcd37b8..7c32905 100644
>> --- a/include/linux/kvm_host.h
>> +++ b/include/linux/kvm_host.h
>> @@ -390,7 +390,7 @@ struct kvm {
>> spinlock_t mmu_lock;
>> struct mutex slots_lock;
>> struct mm_struct *mm; /* userspace tied to this vm */
>> - struct kvm_memslots *memslots[KVM_ADDRESS_SPACE_NUM];
>> + struct kvm_memslots __rcu *memslots[KVM_ADDRESS_SPACE_NUM];
>> struct kvm_vcpu *vcpus[KVM_MAX_VCPUS];
>>
>> /*
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index fc2d583..d4418fa 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -707,7 +707,7 @@ static struct kvm *kvm_create_vm(unsigned long type)
>> for (i = 0; i < KVM_NR_BUSES; i++)
>> kfree(kvm->buses[i]);
>> for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)
>> - kvm_free_memslots(kvm, kvm->memslots[i]);
>> + kvm_free_memslots(kvm, rcu_access_pointer(kvm->memslots[i]));
>
> should these rather be rcu_dereference_protected ?
Yes, I think so.
Paolo
>> kvm_arch_free_vm(kvm);
>> mmdrop(current->mm);
>> return ERR_PTR(r);
>> @@ -753,7 +753,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
>> kvm_arch_destroy_vm(kvm);
>> kvm_destroy_devices(kvm);
>> for (i = 0; i < KVM_ADDRESS_SPACE_NUM; i++)
>> - kvm_free_memslots(kvm, kvm->memslots[i]);
>> + kvm_free_memslots(kvm, rcu_access_pointer(kvm->memslots[i]));
>> cleanup_srcu_struct(&kvm->irq_srcu);
>> cleanup_srcu_struct(&kvm->srcu);
>> kvm_arch_free_vm(kvm);
>>
>
