| Message ID | 20221105224458.8180-3-ajderossi@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | vfio/pci: Check the device set open count on reset | expand |
On Sat, 5 Nov 2022 15:44:57 -0700 Anthony DeRossi <ajderossi@gmail.com> wrote: > The open count of a device set is the sum of the open counts of all > devices in the set. Drivers can use this value 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 | 11 +++++++++++ > include/linux/vfio.h | 1 + > 2 files changed, 12 insertions(+) > > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c > index 9a4af880e941..ab34faabcebb 100644 > --- a/drivers/vfio/vfio_main.c > +++ b/drivers/vfio/vfio_main.c > @@ -125,6 +125,17 @@ static void vfio_release_device_set(struct vfio_device *device) > xa_unlock(&vfio_device_set_xa); > } > > +unsigned int vfio_device_set_open_count(struct vfio_device_set *dev_set) > +{ > + struct vfio_device *cur; > + unsigned int open_count = 0; This can only be called while holding the dev_set->lock, so we should have an assert here: lockdep_assert_held(&dev_set->lock); The series looks ok to me otherwise, hopefully we'll get some additional reviews. Thanks, Alex > + > + list_for_each_entry(cur, &dev_set->device_list, dev_set_list) > + open_count += cur->open_count; > + return open_count; > +} > +EXPORT_SYMBOL_GPL(vfio_device_set_open_count); > + > /* > * Group objects - create, release, get, put, search > */ > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > index e7cebeb875dd..fdd393f70b19 100644 > --- a/include/linux/vfio.h > +++ b/include/linux/vfio.h > @@ -189,6 +189,7 @@ int vfio_register_emulated_iommu_dev(struct vfio_device *device); > void vfio_unregister_group_dev(struct vfio_device *device); > > int vfio_assign_device_set(struct vfio_device *device, void *set_id); > +unsigned int vfio_device_set_open_count(struct vfio_device_set *dev_set); > > int vfio_mig_get_next_state(struct vfio_device *device, > enum vfio_device_mig_state cur_fsm,
On Sat, Nov 05, 2022 at 03:44:57PM -0700, Anthony DeRossi wrote: > The open count of a device set is the sum of the open counts of all > devices in the set. Drivers can use this value 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 | 11 +++++++++++ > include/linux/vfio.h | 1 + > 2 files changed, 12 insertions(+) > > +unsigned int vfio_device_set_open_count(struct vfio_device_set *dev_set) > +{ > + struct vfio_device *cur; > + unsigned int open_count = 0; I'd probably just make this a bool 'vfio_device_set_last_close()' And roll in the < 1 logic too Nothing will ever need to know the number of fds open across the set. But this is fine as written Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> Jason
> From: Anthony DeRossi <ajderossi@gmail.com> > Sent: Sunday, November 6, 2022 6:45 AM > > The open count of a device set is the sum of the open counts of all > devices in the set. Drivers can use this value 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> Apart from remarks from Alex/Jason, good to me: Reviewed-by: Kevin Tian <kevin.tian@intel.com>
On Tue, 8 Nov 2022 20:48:08 -0400 Jason Gunthorpe <jgg@ziepe.ca> wrote: > On Sat, Nov 05, 2022 at 03:44:57PM -0700, Anthony DeRossi wrote: > > The open count of a device set is the sum of the open counts of all > > devices in the set. Drivers can use this value 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 | 11 +++++++++++ > > include/linux/vfio.h | 1 + > > 2 files changed, 12 insertions(+) > > > > > +unsigned int vfio_device_set_open_count(struct vfio_device_set *dev_set) > > +{ > > + struct vfio_device *cur; > > + unsigned int open_count = 0; > > I'd probably just make this a bool > > 'vfio_device_set_last_close()' > > And roll in the < 1 logic too > > Nothing will ever need to know the number of fds open across the set. 'last_close' presumes the caller though, which seems bad form. It's possible there are use cases for this in a 'first_open' scenario too. Something along the lines of 'singleton_open', but that's a horrible name, so we might as well just provide the count since we already have it. Thanks, Alex > But this is fine as written > > Reviewed-by: Jason Gunthorpe <jgg@nvidia.com> > > Jason >
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c index 9a4af880e941..ab34faabcebb 100644 --- a/drivers/vfio/vfio_main.c +++ b/drivers/vfio/vfio_main.c @@ -125,6 +125,17 @@ static void vfio_release_device_set(struct vfio_device *device) xa_unlock(&vfio_device_set_xa); } +unsigned int vfio_device_set_open_count(struct vfio_device_set *dev_set) +{ + struct vfio_device *cur; + unsigned int open_count = 0; + + list_for_each_entry(cur, &dev_set->device_list, dev_set_list) + open_count += cur->open_count; + return open_count; +} +EXPORT_SYMBOL_GPL(vfio_device_set_open_count); + /* * Group objects - create, release, get, put, search */ diff --git a/include/linux/vfio.h b/include/linux/vfio.h index e7cebeb875dd..fdd393f70b19 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -189,6 +189,7 @@ int vfio_register_emulated_iommu_dev(struct vfio_device *device); void vfio_unregister_group_dev(struct vfio_device *device); int vfio_assign_device_set(struct vfio_device *device, void *set_id); +unsigned int vfio_device_set_open_count(struct vfio_device_set *dev_set); int vfio_mig_get_next_state(struct vfio_device *device, enum vfio_device_mig_state cur_fsm,
The open count of a device set is the sum of the open counts of all devices in the set. Drivers can use this value 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 | 11 +++++++++++ include/linux/vfio.h | 1 + 2 files changed, 12 insertions(+)