Hi Alex,
> From: Alex Williamson <alex.williamson@xxxxxxxxxx>
> Sent: Friday, January 20, 2023 4:36 AM
>
> On Thu, 19 Jan 2023 12:01:59 +0100
> Eric Auger <eric.auger@xxxxxxxxxx> wrote:
>
> > >
> > > -void vfio_device_close(struct vfio_device *device,
> > > - struct iommufd_ctx *iommufd)
> > > +void vfio_device_close(struct vfio_device_file *df)
> > > {
> > > + struct vfio_device *device = df->device;
> > > +
> > > mutex_lock(&device->dev_set->lock);
> > > vfio_assert_device_open(device);
> > > if (device->open_count == 1)
> > > - vfio_device_last_close(device, iommufd);
> > > + vfio_device_last_close(df);
> > > device->open_count--;
> > > mutex_unlock(&device->dev_set->lock);
> > > }
>
> I find it strange that the dev_set->lock has been moved to the caller
> for open, but not for close.
There is dev_set->lock for close. Is it? just as the above code snippet.
dev_set->lock is held before calling vfio_device_last_close.
> Like Eric suggests, this seems to be
> because vfio_device_file is usurping dev_set->lock to protect its own
> fields, but then those fields are set on open, cleared on the open
> error path, but not on close?? Thanks,
Yeah. The on close, the vfio_device_file will be freed. So there is
no clear on the vfio_device_file fields.
Regards,
Yi Liu
