On 2024/4/16 17:47, Tian, Kevin wrote:
From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
Sent: Tuesday, April 16, 2024 5:25 PM
On 2024/4/16 17:01, Tian, Kevin wrote:
From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
Sent: Friday, April 12, 2024 4:21 PM
+
+ rc = ida_get_lowest(&vdev->pasids, pasid, pasid);
+ if (rc == pasid)
+ return iommufd_device_pasid_replace(vdev-
iommufd_device,
+ pasid, pt_id);
+
+ rc = iommufd_device_pasid_attach(vdev->iommufd_device, pasid,
pt_id);
+ if (rc)
+ return rc;
+
+ rc = ida_alloc_range(&vdev->pasids, pasid, pasid, GFP_KERNEL);
+ if (rc < 0) {
+ iommufd_device_pasid_detach(vdev->iommufd_device,
pasid);
+ return rc;
+ }
I'd do simple operation (ida_alloc_range()) first before doing attach.
But that means we rely on the ida_alloc_range() to return -ENOSPC to
indicate the pasid is allocated, hence this attach is actually a
replacement. This is easy to be broken if ida_alloc_range() returns
-ENOSPC for other reasons in future.
ida_alloc_range() could fail for other reasons e.g. -ENOMEM.
in case I didn't make it clear I just meant to swap the order
between iommufd_device_pasid_attach() and ida_alloc_range().
replacement is still checked against ida_get_lowest().
aha, I see.
--
Regards,
Yi Liu
