From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Michael S. Tsirkin" <mst@redhat.com>
Subject: Re: [PATCH v4 1/9] KVM: Fix srcu struct leakage
Date: Mon, 8 Nov 2010 19:00:37 +0200
Message-ID: <20101108170037.GH7962@redhat.com>
References: <cover.1289215310.git.jan.kiszka@siemens.com>
 <dcdb5d156a1664370b90cfbe1314514b1b4fda01.1289215310.git.jan.kiszka@siemens.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: Avi Kivity <avi@redhat.com>, Marcelo Tosatti <mtosatti@redhat.com>,
	kvm <kvm@vger.kernel.org>,
	Alex Williamson <alex.williamson@redhat.com>
To: Jan Kiszka <jan.kiszka@siemens.com>
Return-path: <kvm-owner@vger.kernel.org>
Received: from mx1.redhat.com ([209.132.183.28]:14159 "EHLO mx1.redhat.com"
	rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
	id S1752747Ab0KHRAp (ORCPT <rfc822;kvm@vger.kernel.org>);
	Mon, 8 Nov 2010 12:00:45 -0500
Content-Disposition: inline
In-Reply-To: <dcdb5d156a1664370b90cfbe1314514b1b4fda01.1289215310.git.jan.kiszka@siemens.com>
Sender: kvm-owner@vger.kernel.org
List-ID: <kvm.vger.kernel.org>

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