> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Tuesday, February 28, 2023 3:06 AM
>
> On Mon, Feb 27, 2023 at 03:11:31AM -0800, Yi Liu wrote:
> > @@ -309,6 +310,13 @@ void vfio_unregister_group_dev(struct
> vfio_device *device)
> > bool interrupted = false;
> > long rc;
> >
> > + /*
> > + * Balances vfio_device_add in register path. Putting it as the
> > + * first operation in unregister to prevent registration refcount
> > + * from incrementing per cdev open.
> > + */
> > + vfio_device_del(device);
> > +
> > vfio_device_put_registration(device);
> > rc = try_wait_for_completion(&device->comp);
> > while (rc <= 0) {
> > @@ -334,9 +342,6 @@ void vfio_unregister_group_dev(struct vfio_device
> *device)
> >
> > vfio_device_group_unregister(device);
> >
> > - /* Balances device_add in register path */
> > - device_del(&device->device);
> > -
> > /* Balances vfio_device_set_group in register path */
> > vfio_device_remove_group(device);
>
> The same rational applies to vfio_device_group_unregister too, so it
> should be moved up as well.
You are right. User may get new registration refcount in below path
which can be in parallel with this vfio_unregister_group_dev() path.
Let me move it and refine the comment as well.
ioctl(group_fd, VFIO_GROUP_GET_DEVICE_FD, )
vfio_group_ioctl_get_device_fd()
-> vfio_device_get_from_name()
-> vfio_device_try_get_registration() -- refcount++
Thanks,
Yi Liu
