> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Tuesday, October 12, 2021 8:05 PM
>
> 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.
items are added to this list after vfio_create_group() (in the start of
vfio_unregister_group_dev()). So I feel it makes more sense to move
it back to empty before put_device() in vfio_group_put(). But not a
strong opinion...
>
> > > 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.
>
Make sense. Thanks for explanation.
