Re: [PATCH v5 10/29] iommufd: Abstract iopt_pin_pages and iopt_unpin_pages helpers

[Nicolin Chen <nicolinc@nvidia.com>]

On Thu, Jun 05, 2025 at 04:40:34PM -0300, Jason Gunthorpe wrote:
> On Thu, Jun 05, 2025 at 10:04:35AM -0700, Nicolin Chen wrote:
> > On Thu, Jun 05, 2025 at 12:16:48PM -0300, Jason Gunthorpe wrote:
> > > On Wed, Jun 04, 2025 at 09:11:07PM -0700, Nicolin Chen wrote:
> > > 
> > > > I found the entire ictx would be locked by iommufd_access_create(),
> > > > then the release fop couldn't even get invoked to destroy objects.
> > > 
> > > Yes, that makes sense..
> > > 
> > > It looks to me like you can safely leave ictx as NULL instead of
> > > adding a flag? That would be nicer than leaving a unrefcounted
> > > pointer floating around..
> > 
> > Hmm, there are a few iommufd_get_object calls using access->ictx
> > in iommufd_access_attach() and iommufd_access_destroy().
> 
> I counted:
> 
> iommufd_access_change_ioas_id
>  * Don't call this
> iommufd_access_destroy_object
>  * Don't put if null
> iommufd_access_create
>  * Don't set it
> iommufd_access_destroy
>  * Call iommufd_object_destroy_user directly
> iommufd_access_notify_unmap
>  * Check for null access->ops->unmap and skip the lock_obj/put_obj

Yea. And I added a set of lighter "_internal" helpers so the caller
side looks consistent:

	access = iommufd_access_create_internal(viommu->ictx);
	rc = iommufd_access_attach_internal(access, viommu->hwpt->ioas);
	...
	iommufd_access_detach_internal(access);
	iommufd_access_destroy_internal(ictx, access);

Thanks
Nicolin