> From: Jason Gunthorpe <jgg@xxxxxxxxxx> > Sent: Tuesday, March 7, 2023 9:53 PM > > On Tue, Mar 07, 2023 at 02:38:47AM +0000, Tian, Kevin wrote: > > > From: Jason Gunthorpe <jgg@xxxxxxxxxx> > > > Sent: Tuesday, March 7, 2023 4:22 AM > > > > > > On Thu, Mar 02, 2023 at 08:01:00AM +0000, Tian, Kevin wrote: > > > > > From: Jason Gunthorpe <jgg@xxxxxxxxxx> > > > > > Sent: Saturday, February 25, 2023 8:28 AM > > > > > > > > > > +struct iommufd_hw_pagetable * > > > > > +iommufd_hw_pagetable_detach(struct iommufd_device *idev) > > > > > { > > > > > - if (!iommufd_hw_pagetable_has_group(hwpt, idev->igroup)) > > > > > + struct iommufd_hw_pagetable *hwpt = idev->igroup->hwpt; > > > > > + > > > > > + lockdep_assert_held(&idev->igroup->lock); > > > > > + > > > > > + idev->igroup->devices--; > > > > > + if (!idev->igroup->devices) { > > > > > iommu_detach_group(hwpt->domain, idev->igroup->group); > > > > > + idev->igroup->hwpt = NULL; > > > > > > > > hwpt->obj.users should be decremented here instead of leaving it > > > > in iommufd_device_detach(). > > > > > > It is like this because eventually we can't call > > > iommufd_object_destroy_user() while holding the locks. > > > > > > So the lowest function returns the hwpt up the call chain and once > > > everything is unlocked then it calls iommufd_hw_pagetable_put() > > > > but don't we have unbalanced refcnt poke? > > Yes, the refcount should be incremented for every attached device > per device or per group? Now it's igroup tracking attached hwpt and each device holds a reference on the igroup. Then why do we want to further poke the refcnt per attached device? sounds it's still clearer to increment it at first device attach and decrement at last device detach.