RE: [PATCH 05/13] kvm/vfio: Provide struct kvm_device_ops::release() insted of ::destroy()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> From: Eric Auger <eric.auger@xxxxxxxxxx>
> Sent: Thursday, January 19, 2023 5:13 PM
> 
> Hi Yi,
> 
> On 1/17/23 14:49, Yi Liu wrote:
> > This is to avoid a circular refcount problem between the kvm struct and
> > the device file. KVM modules holds device/group file reference when the
> > device/group is added and releases it per removal or the last kvm reference
> > is released. This reference model is ok for the group since there is no
> > kvm reference in the group paths.
> >
> > But it is a problem for device file since the vfio devices may get kvm
> > reference in the device open path and put it in the device file release.
> > e.g. Intel kvmgt. This would result in a circular issue since the kvm
> > side won't put the device file reference if kvm reference is not 0, while
> > the vfio device side needs to put kvm reference in the release callback.
> >
> > To solve this problem for device file, let vfio provide release() which
> > would be called once kvm file is closed, it won't depend on the last kvm
> > reference. Hence avoid circular refcount problem.
> >
> > Suggested-by: Kevin Tian <kevin.tian@xxxxxxxxx>
> > Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
> > ---
> >  virt/kvm/vfio.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c
> > index 0f54b9d308d7..525efe37ab6d 100644
> > --- a/virt/kvm/vfio.c
> > +++ b/virt/kvm/vfio.c
> > @@ -364,7 +364,7 @@ static int kvm_vfio_create(struct kvm_device *dev,
> u32 type);
> >  static struct kvm_device_ops kvm_vfio_ops = {
> >  	.name = "kvm-vfio",
> >  	.create = kvm_vfio_create,
> > -	.destroy = kvm_vfio_destroy,
> Is it safe to simply remove the destroy cb as it is called from
> kvm_destroy_vm/kvm_destroy_devices?
> 

According to the definition .release is considered as an alternative
method to free the device:

	/*
	 * Destroy is responsible for freeing dev.
	 *
	 * Destroy may be called before or after destructors are called
	 * on emulated I/O regions, depending on whether a reference is
	 * held by a vcpu or other kvm component that gets destroyed
	 * after the emulated I/O.
	 */
	void (*destroy)(struct kvm_device *dev);

	/*
	 * Release is an alternative method to free the device. It is
	 * called when the device file descriptor is closed. Once
	 * release is called, the destroy method will not be called
	 * anymore as the device is removed from the device list of
	 * the VM. kvm->lock is held.
	 */
	void (*release)(struct kvm_device *dev);

Did you see any specific problem of moving this stuff to release?





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux