On Thu, Apr 25, 2013 at 11:51:08AM -0500, Scott Wood wrote:
> 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.
Is it so? Device holds a pointer to kvm, so it increments kvm reference
to make sure the pointer is valid. What prevents kvm from been destroyed
while device is still in use in current code?
> 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.
>
How circular reference can be created?
--
Gleb.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
