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

Scott Wood
2013-04-25T16:51:08Z

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