> From: Jason Gunthorpe <j...@nvidia.com>
> 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

Reply via email to