> 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?