| Message ID | 6-v1-7612f88c19f5+2f21-iommufd_alloc_jgg@nvidia.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add iommufd physical device operations for replace and alloc hwpt | expand |
> From: Jason Gunthorpe <jgg@nvidia.com> > Sent: Saturday, February 25, 2023 8:28 AM > > @@ -481,11 +481,7 @@ void iommufd_device_detach(struct > iommufd_device *idev) > hwpt = iommufd_hw_pagetable_detach(idev); > mutex_unlock(&idev->igroup->lock); > > - if (hwpt->auto_domain) > - iommufd_object_destroy_user(idev->ictx, &hwpt->obj); > - else > - refcount_dec(&hwpt->obj.users); > - > + iommufd_hw_pagetable_put(idev->ictx, hwpt); > refcount_dec(&idev->obj.users); > } As commented in patch3 this should be called in iommufd_hw_pagetable_detach() when idev->igroup->hwpt is cleared.
On Thu, Mar 02, 2023 at 08:12:28AM +0000, Tian, Kevin wrote: > > From: Jason Gunthorpe <jgg@nvidia.com> > > Sent: Saturday, February 25, 2023 8:28 AM > > > > @@ -481,11 +481,7 @@ void iommufd_device_detach(struct > > iommufd_device *idev) > > hwpt = iommufd_hw_pagetable_detach(idev); > > mutex_unlock(&idev->igroup->lock); > > > > - if (hwpt->auto_domain) > > - iommufd_object_destroy_user(idev->ictx, &hwpt->obj); > > - else > > - refcount_dec(&hwpt->obj.users); > > - > > + iommufd_hw_pagetable_put(idev->ictx, hwpt); > > refcount_dec(&idev->obj.users); > > } > > As commented in patch3 this should be called in > iommufd_hw_pagetable_detach() when idev->igroup->hwpt is cleared. Same answer, has to be called after we unlock everything. The issue is not device_detach which has simple locking but iommufd_device_change_pt() in a later patch. Jason
diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c index ea34dc5b0fd1e4..590f84afa06503 100644 --- a/drivers/iommu/iommufd/device.c +++ b/drivers/iommu/iommufd/device.c @@ -481,11 +481,7 @@ void iommufd_device_detach(struct iommufd_device *idev) hwpt = iommufd_hw_pagetable_detach(idev); mutex_unlock(&idev->igroup->lock); - if (hwpt->auto_domain) - iommufd_object_destroy_user(idev->ictx, &hwpt->obj); - else - refcount_dec(&hwpt->obj.users); - + iommufd_hw_pagetable_put(idev->ictx, hwpt); refcount_dec(&idev->obj.users); } EXPORT_SYMBOL_NS_GPL(iommufd_device_detach, IOMMUFD); diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h index 0497d6190fd1b3..c0c80181d05ed1 100644 --- a/drivers/iommu/iommufd/iommufd_private.h +++ b/drivers/iommu/iommufd/iommufd_private.h @@ -260,6 +260,16 @@ struct iommufd_hw_pagetable * iommufd_hw_pagetable_detach(struct iommufd_device *idev); void iommufd_hw_pagetable_destroy(struct iommufd_object *obj); +static inline void iommufd_hw_pagetable_put(struct iommufd_ctx *ictx, + struct iommufd_hw_pagetable *hwpt) +{ + lockdep_assert_not_held(&hwpt->ioas->mutex); + if (hwpt->auto_domain) + iommufd_object_destroy_user(ictx, &hwpt->obj); + else + refcount_dec(&hwpt->obj.users); +} + struct iommufd_group { struct kref ref; struct mutex lock;
Next patch will need to call this from two places. Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> --- drivers/iommu/iommufd/device.c | 6 +----- drivers/iommu/iommufd/iommufd_private.h | 10 ++++++++++ 2 files changed, 11 insertions(+), 5 deletions(-)