Re: [PATCH v10 08/18] iommufd/device: Replace device_list with device_array

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

On 2025/3/21 01:20, Jason Gunthorpe wrote:
> On Thu, Mar 20, 2025 at 06:47:34AM -0700, Yi Liu wrote:
>> @@ -298,6 +298,20 @@ u32 iommufd_device_to_id(struct iommufd_device *idev)
>>   }
>>   EXPORT_SYMBOL_NS_GPL(iommufd_device_to_id, "IOMMUFD");
>>   
>> +static int iommufd_group_device_num(struct iommufd_group *igroup)
>> +{
>> +	struct iommufd_device *idev;
>> +	unsigned long index;
>> +	int count = 0;
> 
> unsigned int and unsigned int return code too

got it.

>> +	rc = xa_insert(&igroup->attach->device_array, idev->obj.id, XA_ZERO_ENTRY,
>> +		       GFP_KERNEL);
> 
> Probably don't really care, but note the choice of obj.id here is
> going to waste some memory in the xarray. 0 based would be more
> memory efficient, but some of the other operations would be slower.

yes. The iommufd_device_is_attached() and the detach path might be slower
since it would need to loop all the devices. I also considered to store the
id for indexing the device_array. But the problem is idev can be stored in
multiple device_arrays. A single id does not just like a single list_head
cannot work. Seems no better choice...

> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
> 
> Jason

-- 
Regards,
Yi Liu