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