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

Michael S. Tsirkin
2010-11-08T17:00:45Z

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,
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