RE: [PATCH v2 07/10] iommufd/device: Make hwpt_list list_add/del symmetric

[Tian, Kevin]

> From: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Sent: Saturday, February 11, 2023 5:17 AM
> 
> >
> > there is semantics change.
> >
> > here is the current code:
> >
> > 	case IOMMUFD_OBJ_HW_PAGETABLE: {
> > 		struct iommufd_hw_pagetable *hwpt =
> > 			container_of(pt_obj, struct iommufd_hw_pagetable,
> obj);
> >
> > 		rc = iommufd_device_do_attach(idev, hwpt);
> > 		if (rc)
> > 			goto out_put_pt_obj;
> >
> > 		mutex_lock(&hwpt->ioas->mutex);
> > 		list_add_tail(&hwpt->hwpt_item, &hwpt->ioas->hwpt_list);
> > 		mutex_unlock(&hwpt->ioas->mutex);
> > 		break;
> > 	}
> >
> > Above means every attach to hwpt will try to add the hwpt to the
> > list tail. Isn't it a bug?
> 
> Yes, that looks like a bug..
> 
> But this patch isn't the right way to fix that.
> 
> The HWPT should be permanently linked to the IOAS as long as it
> exists, and the linkage should happen when it is first created.
> 
> So attaching a HWPT to another device should never re-link it to the
> ioas, thus delete these lines here.
> 

yes, this makes more sense