From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <scottwood@freescale.com>
Date: Thu, 25 Apr 2013 16:51:08 +0000
Subject: Re: [PATCH v4 1/6] kvm: add device control API
Message-Id: <1366908668.30341.1@snotra>
List-Id: <kvm-ppc.vger.kernel.org>
References: <1364954273-18196-1-git-send-email-scottwood@freescale.com>
	<1365811727-24431-1-git-send-email-scottwood@freescale.com>
	<1365811727-24431-2-git-send-email-scottwood@freescale.com>
	<20130425094324.GY12401@redhat.com>
	<5A22B05F-C1C5-439F-8AC6-D16137A9D086@suse.de>
In-Reply-To: <5A22B05F-C1C5-439F-8AC6-D16137A9D086@suse.de> (from
	agraf@suse.de on Thu Apr 25 05:47:39 2013)
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: Alexander Graf <agraf@suse.de>
Cc: Gleb Natapov <gleb@redhat.com>, kvm-ppc@vger.kernel.org, kvm@vger.kernel.org, paulus@samba.org

On 04/25/2013 05:47:39 AM, Alexander Graf wrote:
> 
> On 25.04.2013, at 11:43, Gleb Natapov wrote:
> 
> >> +void kvm_device_put(struct kvm_device *dev)
> >> +{
> >> +	if (atomic_dec_and_test(&dev->users))
> >> +		dev->ops->destroy(dev);
> >> +}
> >> +
> >> +static int kvm_device_release(struct inode *inode, struct file  
> *filp)
> >> +{
> >> +	struct kvm_device *dev = filp->private_data;
> >> +	struct kvm *kvm = dev->kvm;
> >> +
> >> +	kvm_device_put(dev);
> >> +	kvm_put_kvm(kvm);
> > We may put kvm only if users goes to zero, otherwise kvm can be
> > freed while something holds a reference to a device. Why not make
> > kvm_device_put() do it?
> 
> Nice catch. I'll change the patch so it does the kvm_put_kvm inside  
> kvm_device_put's destroy branch.

No, please don't.  The KVM reference being "put" here is associated  
with the file descriptor, not with the MPIC object.  If you make that  
change I think you'll have circular references and thus a memory leak,  
because the vcpus can hold a reference to the MPIC object.

-Scott