On Thu, Feb 09, 2023 at 03:13:08AM +0000, Tian, Kevin wrote: > > --- a/drivers/iommu/iommufd/device.c > > +++ b/drivers/iommu/iommufd/device.c > > @@ -509,11 +509,23 @@ int iommufd_access_set_ioas(struct > > iommufd_access *access, u32 ioas_id) > > iommufd_ref_to_users(obj); > > } > > > > + /* > > + * Set ioas to NULL to block any further iommufd_access_pin_pages(). > > + * iommufd_access_unpin_pages() can continue using access- > > >ioas_unpin. > > + */ > > + access->ioas = NULL; > > + > > if (cur_ioas) { > > + if (new_ioas) { > > + mutex_unlock(&access->ioas_lock); > > + access->ops->unmap(access->data, 0, ULONG_MAX); > > + mutex_lock(&access->ioas_lock); > > + } > > why does above only apply to a valid new_ioas? this is the cleanup on > cur_ioas then required even when new_ioas=NULL. Though it'd make sense to put it in the common path, our current detach routine doesn't call this unmap. If we do so, it'd become something new to the normal detach routine. Or does this mean the detach routine has been missing an unmap call so far? Thanks Nic