Re: [PATCH v2 3/4] iommufd: Destroy vdevice on idevice destroy

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Jun 25, 2025 at 02:11:40AM +0000, Tian, Kevin wrote:
> > From: Jason Gunthorpe <jgg@nvidia.com>
> > Sent: Wednesday, June 25, 2025 9:36 AM
> > 
> > On Tue, Jun 24, 2025 at 11:57:31PM +0000, Tian, Kevin wrote:
> > > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > > Sent: Tuesday, June 24, 2025 10:54 PM
> > > >
> > > > On Mon, Jun 23, 2025 at 05:49:45PM +0800, Xu Yilun wrote:
> > > > > +static void iommufd_device_remove_vdev(struct iommufd_device
> > *idev)
> > > > > +{
> > > > > +	bool vdev_removing = false;
> > > > > +
> > > > > +	mutex_lock(&idev->igroup->lock);
> > > > > +	if (idev->vdev) {
> > > > > +		struct iommufd_vdevice *vdev;
> > > > > +
> > > > > +		vdev = iommufd_get_vdevice(idev->ictx, idev->vdev->obj.id);
> > > > > +		if (IS_ERR(vdev)) {
> > > >
> > > > This incrs obj.users which will cause a concurrent
> > > > iommufd_object_remove() to fail with -EBUSY, which we are trying to
> > > > avoid.
> > >
> > > concurrent remove means a user-initiated IOMMU_DESTROY, for which
> > > failing with -EBUSY is expected as it doesn't wait for shortterm?
> > 
> > Yes a user IOMMU_DESTROY of the vdevice should not have a transient
> > EBUSY failure. Avoiding that is the purpose of the shorterm_users
> > mechanism.
> 
> hmm my understanding is the opposite.
> 
> currently iommufd_destroy() doesn't set REMOVE_WAIT_SHORTTERM:

Oh yes I got them mixed up.

> waiting shorterm_users is more for kernel destroy.

Yes, it is to make kernel destroy not fail

> Then iommufd_device_remove_vdev() will wait on idev->vdev to
> be NULL instead of calling iommufd_object_tombstone_user().

Ah, because the original version incrs users, not just
shortterm_users.

Jason