Re: [PATCH rc 1/2] iommufd: Do not access the area pointer after unlocking

[Nicolin Chen <nicolinc@nvidia.com>]

On Mon, Jun 19, 2023 at 03:24:58PM -0300, Jason Gunthorpe wrote:

> Precompute what is needed to call the access function and do not check the
> area's num_accesses again as the pointer may not be valid anymore. Use a
> counter instead.

> @@ -458,6 +458,7 @@ static int iopt_unmap_iova_range(struct io_pagetable *iopt, unsigned long start,
>  {
>  	struct iopt_area *area;
>  	unsigned long unmapped_bytes = 0;
> +	unsigned int tries = 0;
>  	int rc = -ENOENT;
>  
>  	/*
> @@ -490,13 +491,17 @@ static int iopt_unmap_iova_range(struct io_pagetable *iopt, unsigned long start,
>  		 * without the pages->mutex.
>  		 */
>  		if (area->num_accesses) {
> +			size_t length = iopt_area_length(area);
> +
>  			start = area_first;
>  			area->prevent_access = true;
>  			up_write(&iopt->iova_rwsem);
>  			up_read(&iopt->domains_rwsem);
> -			iommufd_access_notify_unmap(iopt, area_first,
> -						    iopt_area_length(area));
> -			if (WARN_ON(READ_ONCE(area->num_accesses)))
> +
> +			iommufd_access_notify_unmap(iopt, area_first, length);
> +			/* Something is not responding to unmap requests. */
> +			tries++;
> +			if (WARN_ON(tries > 1000))

Hmm, just trying to understand this piece here...

IIUIC, the iommufd_access_notify_unmap() notifies the vfio device
to do a vfio_unpin_pages() that would do a "num_accesses--;". But
how does the counter work instead? And why 1000?

Btw, there seems to be a small typo in the comments above this
piece of code, though it's not in this patch:
s/iovam_rwsem/iova_rwsem

Thanks
Nicolin