On 04/25/2013 01:22:04 PM, Gleb Natapov wrote:
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?
Where will that kvm pointer be used, after all the file descriptors go
away and the vcpus stop running? mmio_mapped guards against unmapping
the MMIO if it's already been unmapped due to KVM destruction. We
don't have any timers or other delayed work.
Well, I do see one place, that Alex added -- the NULLing out of
dev->kvm->arch.mpic, which didn't exist in my patchset.
> 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?
MPIC holds reference on KVM, vcpu holds reference on MPIC, and vcpu is
not destroyed until KVM is destroyed.
-Scott
--
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
