Re: [PATCH] vfio: Remove vfio_group dev_counter

[Yi Liu <yi.l.liu@intel.com>]

On 2022/8/16 20:29, Jason Gunthorpe wrote:
> On Tue, Aug 16, 2022 at 09:21:05AM +0800, Yi Liu wrote:
>> Hi Jason,
>>
>> On 2022/8/16 00:50, Jason Gunthorpe wrote:
>>> This counts the number of devices attached to a vfio_group, ie the number
>>> of items in the group->device_list.
>>
>> yes. This dev_counter is added to ensure only singleton vfio group supports
>> pin page. Although I don't think it is a good approach as it only counts the
>> registered devices.
> 
> Ah, I missed explaining this, lets try again:
> 
> vfio: Remove vfio_group dev_counter
> 
> This counts the number of devices attached to a vfio_group, ie the number
> of items in the group->device_list.
> 
> It has two purposes
>   - To ensure the vfio_device is opened

hmmm, surely vfio_pin_pages() needs to ensure vfio_device is opened.
But it's not the purpose of adding dev_counter. So I guess this line
can be removed. :)

>   - To assert the group is singleton because the dirty tracking code in the
>     type1 iommu has limitations
> 
> However, vfio_pin_pages() already calls vfio_assert_device_open() so the
> first is taken care of, and all callers of vfio_pin_pages() use now use
> vfio_register_emulated_iommu_dev() which guarentees single groups by
> constrution.
> 
> So delete dev_counter and leave a note that vfio_pin_pages() can only be
> used with vfio_register_emulated_iommu_dev().

oh, yes. The fake group is allocated in vfio_register_emulated_iommu_dev().
So the group is surely singleton. Maybe for simple just say
vfio_register_emulated_iommu_dev() guarantees the group is singleton, and
vfio_pin_pages() can only be used with vfio_register_emulated_iommu_dev().
So no need to use dev_counter in vfio_pin_pages(). Such commit message may
be more accurate and no unnecessary distraction. :-) btw. The change looks
good to me.

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

-- 
Regards,
Yi Liu