> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Friday, September 23, 2022 8:06 AM
>
> diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
> index 56fab31f8e0ff8..039e3208d286fa 100644
> --- a/drivers/vfio/vfio.h
> +++ b/drivers/vfio/vfio.h
> @@ -41,7 +41,15 @@ enum vfio_group_type {
> struct vfio_group {
> struct device dev;
> struct cdev cdev;
> + /*
> + * When drivers is non-zero a driver is attached to the struct device
> + * that provided the iommu_group and thus the iommu_group is a
> valid
> + * pointer. When drivers is 0 the driver is being detached. Once users
> + * reaches 0 then the iommu_group is invalid.
> + */
> + refcount_t drivers;
While I agree all this patch is doing, the notation of 'drivers' here sounds
a bit confusing IMHO. Given all the paths where 'drivers' are populated
are related to device registration/unregistration, isn't it clearer to rename
it as 'devices' and make it clear that iommu_group is invalid once the last
device is unregistered from the group? This kind of makes sense to me
even without talking about the aspect of driver attach/detach...
> refcount_t users;
> + struct completion users_comp;
Now the only place poking 'users' is when a group is opened/closed,
while group->opened_file already plays the guard role. From this
angle 'users' sounds redundant now?
