Re: [PATCH v4 1/6] kvm: add device control API

[Scott Wood <scottwood@freescale.com>]

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