On Tue, Oct 12, 2021 at 08:33:49AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> > Sent: Saturday, October 2, 2021 7:22 AM
> >
> [...]
>
> > +static void vfio_group_release(struct device *dev)
> > {
> > - struct vfio_group *group, *existing_group;
> > - struct device *dev;
> > - int ret, minor;
> > + struct vfio_group *group = container_of(dev, struct vfio_group, dev);
> > + struct vfio_unbound_dev *unbound, *tmp;
> > +
> > + list_for_each_entry_safe(unbound, tmp,
> > + &group->unbound_list, unbound_next) {
> > + list_del(&unbound->unbound_next);
> > + kfree(unbound);
> > + }
>
> move to vfio_group_put()? this is not paired with vfio_group_alloc()...
Lists are tricky for pairing analysis, the vfio_group_alloc() creates
an empty list and release restores the list to empty.
> > static int vfio_group_fops_open(struct inode *inode, struct file *filep)
> > {
> > - struct vfio_group *group;
> > + struct vfio_group *group =
> > + container_of(inode->i_cdev, struct vfio_group, cdev);
> > int opened;
>
> A curiosity question. According to cdev_device_del() any cdev already
> open will remain with their fops callable.
Correct
> What prevents vfio_group from being released after cdev_device_del()
> has been called? Is it because cdev open will hold a reference to
> device thus put_device() will not hit zero in vfio_group_put()?
Yes, that is right. The extra reference is hidden deep inside the FS
code and is actually a reference on the cdev struct, which in turn
holds a kobject parent reference on the struct device. It is
complicated under the covers, but from an API perspective if a struct
file exists then so does the vfio_group.
Jason
