> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Thursday, February 23, 2023 5:03 AM
>
> +static int iommufd_device_do_attach(struct iommufd_device *idev,
> + struct iommufd_hw_pagetable *hwpt)
> +{
> + int rc;
> +
> + mutex_lock(&hwpt->devices_lock);
> + rc = iommufd_hw_pagetable_attach(hwpt, idev);
> + if (rc)
> + goto out_unlock;
>
> idev->hwpt = hwpt;
> refcount_inc(&hwpt->obj.users);
> + /* hwpt->devices is all domains that have been attached */
> list_add(&idev->devices_item, &hwpt->devices);
s/domains/devices/
but I didn't see what additional info this comment actually
give in this place. It's there in a function name xxx_attach then
certainly every device in that list has been attached.
> +
> + rc = iopt_table_add_domain(&hwpt->ioas->iopt, hwpt->domain);
> + if (rc)
> + goto out_detach;
> +
> + /* ioas->hwpt_list is hwpts after iopt_table_add_domain() */
> + list_add_tail(&hwpt->hwpt_item, &hwpt->ioas->hwpt_list);
again the comment is meaningless
otherwise looks good to me,
Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx>
