RE: [PATCH 03/14] iommufd: Replace the hwpt->devices list with iommufd_group

Tian, Kevin
2023-03-08T07:30:01Z

> From: Jason Gunthorpe <jgg@nvidia.com>
> 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@nvidia.com>
> > > 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@nvidia.com>
> > > > > 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.