Re: [PATCH 5/5] vfio: Use cdev_device_add() instead of device_create()

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Oct 12, 2021 at 08:33:49AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > 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