On Wed, Jun 25, 2025 at 09:37:18AM -0700, Nicolin Chen wrote:
On Wed, Jun 25, 2025 at 03:38:19AM +0000, Tian, Kevin wrote:
From: Nicolin Chen nicolinc@nvidia.com Sent: Saturday, June 14, 2025 3:15 PM
+int iommufd_access_notify_unmap(struct io_pagetable *iopt, unsigned long iova,
unsigned long length){ struct iommufd_ioas *ioas = container_of(iopt, struct iommufd_ioas, iopt); struct iommufd_access *access; unsigned long index;
int ret = 0;
xa_lock(&ioas->iopt.access_list); xa_for_each(&ioas->iopt.access_list, index, access) {
if (!access->ops || !access->ops->unmap) {ret = -EBUSY;goto unlock;}then accesses before this one have been notified to unpin the area while accesses afterwards are left unnotified.
in the end the unmap fails but with some side-effect incurred.
I'm not sure whether this intermediate state may lead to any undesired effect later. Just raise it in case you or Jason already thought about it.
That's a good point. When an access blocks the unmap, there is no unmap happening so no point in notifying devices for ops->unmap.
And, when the function is re-entered, there could be a duplicated ops->unmap call for those devices that are already notified once?
So, if we play safe, there can be a standalone xa_for_each to dig for !access->ops->unmap. And it could be a bit cleaner to add an iommufd_access_has_internal_use() to be called under those rwsems.
Correct: it has to be under the xa_lock. So, this pre-check needs to be done in this function.
Thanks Nicolin