| Message ID | 20221104195727.4629-3-ajderossi@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | vfio/pci: Check the device set open_count on reset | expand |
On Fri, 4 Nov 2022 12:57:26 -0700 Anthony DeRossi <ajderossi@gmail.com> wrote: > open_count is incremented before open_device() and decremented after > close_device() for each device in the set. This allows devices to > determine whether shared resources are in use without tracking them > manually or accessing the private open_count in vfio_device. > > Signed-off-by: Anthony DeRossi <ajderossi@gmail.com> > --- > drivers/vfio/vfio_main.c | 3 +++ > include/linux/vfio.h | 1 + > 2 files changed, 4 insertions(+) > > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c > index 9a4af880e941..6c65418fc7e3 100644 > --- a/drivers/vfio/vfio_main.c > +++ b/drivers/vfio/vfio_main.c > @@ -761,6 +761,7 @@ static struct file *vfio_device_open(struct vfio_device *device) > mutex_lock(&device->group->group_lock); > device->kvm = device->group->kvm; > > + device->dev_set->open_count++; Note that this is on the device first open path... > if (device->ops->open_device) { > ret = device->ops->open_device(device); > if (ret) > @@ -809,6 +810,7 @@ static struct file *vfio_device_open(struct vfio_device *device) > } > err_undo_count: > mutex_unlock(&device->group->group_lock); > + device->dev_set->open_count--; But this can be reached for non-first open faults. > device->open_count--; > if (device->open_count == 0 && device->kvm) > device->kvm = NULL; > @@ -1023,6 +1025,7 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep) > device->ops->close_device(device); > > vfio_device_container_unregister(device); > + device->dev_set->open_count--; This is on the last-close path, so aside from the above bug this is more of an open device counter across the device set rather than than a number of open device file descriptors counter as we have on each device. There's some complexity thinking about the difference between those for our scenario, it works, but requires a bit of deduction. For example, a device set might have a count of 1 here, but that one device could be opened multiple times. It works for the scenario you address in the next patch, but is maybe not as generically useful. Like it seems maybe you're going for by your more recent comment, I was thinking an interface rather than tracking a new field on the device set. Thanks, Alex > } > mutex_unlock(&device->group->group_lock); > device->open_count--; > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > index e7cebeb875dd..5becdcdf4ba2 100644 > --- a/include/linux/vfio.h > +++ b/include/linux/vfio.h > @@ -28,6 +28,7 @@ struct vfio_device_set { > struct mutex lock; > struct list_head device_list; > unsigned int device_count; > + unsigned int open_count; > }; > > struct vfio_device {
On Fri, Nov 4, 2022 at 8:59 PM Alex Williamson <alex.williamson@redhat.com> wrote: > Like it seems maybe you're going for by your more recent comment, I was > thinking an interface rather than tracking a new field on the device > set. Thanks for the feedback. I sent an updated series with this change. v5: https://lore.kernel.org/kvm/20221105224458.8180-1-ajderossi@gmail.com/ Anthony
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c index 9a4af880e941..6c65418fc7e3 100644 --- a/drivers/vfio/vfio_main.c +++ b/drivers/vfio/vfio_main.c @@ -761,6 +761,7 @@ static struct file *vfio_device_open(struct vfio_device *device) mutex_lock(&device->group->group_lock); device->kvm = device->group->kvm; + device->dev_set->open_count++; if (device->ops->open_device) { ret = device->ops->open_device(device); if (ret) @@ -809,6 +810,7 @@ static struct file *vfio_device_open(struct vfio_device *device) } err_undo_count: mutex_unlock(&device->group->group_lock); + device->dev_set->open_count--; device->open_count--; if (device->open_count == 0 && device->kvm) device->kvm = NULL; @@ -1023,6 +1025,7 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep) device->ops->close_device(device); vfio_device_container_unregister(device); + device->dev_set->open_count--; } mutex_unlock(&device->group->group_lock); device->open_count--; diff --git a/include/linux/vfio.h b/include/linux/vfio.h index e7cebeb875dd..5becdcdf4ba2 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -28,6 +28,7 @@ struct vfio_device_set { struct mutex lock; struct list_head device_list; unsigned int device_count; + unsigned int open_count; }; struct vfio_device {
open_count is incremented before open_device() and decremented after close_device() for each device in the set. This allows devices to determine whether shared resources are in use without tracking them manually or accessing the private open_count in vfio_device. Signed-off-by: Anthony DeRossi <ajderossi@gmail.com> --- drivers/vfio/vfio_main.c | 3 +++ include/linux/vfio.h | 1 + 2 files changed, 4 insertions(+)