Re: [PATCH v4 1/9] KVM: Fix srcu struct leakage

Jan Kiszka
2010-11-08T17:32:54Z

Am 08.11.2010 18:00, Michael S. Tsirkin wrote:
> On Mon, Nov 08, 2010 at 12:21:45PM +0100, Jan Kiszka wrote:
>> Clean up the srcu struct on vm destruction and refactor its release on
>> early errors.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>
> Yay, I suspected something's wrong with error handling in srcu.
>
> Acked-by: Michael S. Tsirkin <mst@redhat.com>
>
> This one actually has nothing to do with assignment,

Yes, it is just mechanically needed (and I found it while hacking in
more SRCU).

Jan

> and it looks like it's needed for -stable and 2.6.37.
> Avi/Marcelo?
>
>> ---
>> virt/kvm/kvm_main.c | 15 +++++++--------
>> 1 files changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
>> index 4111a4b..6cfcde7 100644
>> --- a/virt/kvm/kvm_main.c
>> +++ b/virt/kvm/kvm_main.c
>> @@ -401,23 +401,19 @@ static struct kvm *kvm_create_vm(void)
>> r = -ENOMEM;
>> kvm->memslots = kzalloc(sizeof(struct kvm_memslots), GFP_KERNEL);
>> if (!kvm->memslots)
>> - goto out_err;
>> + goto out_err_nosrcu;
>> if (init_srcu_struct(&kvm->srcu))
>> - goto out_err;
>> + goto out_err_nosrcu;
>> for (i = 0; i < KVM_NR_BUSES; i++) {
>> kvm->buses[i] = kzalloc(sizeof(struct kvm_io_bus),
>> GFP_KERNEL);
>> - if (!kvm->buses[i]) {
>> - cleanup_srcu_struct(&kvm->srcu);
>> + if (!kvm->buses[i])
>> goto out_err;
>> - }
>> }
>>
>> r = kvm_init_mmu_notifier(kvm);
>> - if (r) {
>> - cleanup_srcu_struct(&kvm->srcu);
>> + if (r)
>> goto out_err;
>> - }
>>
>> kvm->mm = current->mm;
>> atomic_inc(&kvm->mm->mm_count);
>> @@ -435,6 +431,8 @@ out:
>> return kvm;
>>
>> out_err:
>> + cleanup_srcu_struct(&kvm->srcu);
>> +out_err_nosrcu:
>> hardware_disable_all();
>> out_err_nodisable:
>> for (i = 0; i < KVM_NR_BUSES; i++)
>> @@ -513,6 +511,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
>> #else
>> kvm_arch_flush_shadow(kvm);
>> #endif
>> + cleanup_srcu_struct(&kvm->srcu);
>> kvm_arch_destroy_vm(kvm);
>> hardware_disable_all();
>> mmdrop(mm);
>> --
>> 1.7.1

--
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux