Re: [PATCH v5 1/3] vfio/type1: batch vfio_find_vpfn() in function vfio_unpin_pages_remote()

[Alex Williamson]

On Fri, 20 Jun 2025 11:23:42 +0800
lizhe.67@xxxxxxxxxxxxx wrote:

> From: Li Zhe <lizhe.67@xxxxxxxxxxxxx>
> 
> This patch is based on patch 'vfio/type1: optimize
> vfio_pin_pages_remote() for large folios'[1].

The above and the below link are only necessary in the cover letter, or
below the --- marker below, they don't really make sense in the
committed log.

Anyway, aside from that and one nit on 2/ (sent separately), the series
looks ok to me and I hope David and Jason will chime in with A-b/R-b
give the previous discussions.

Given the build bot error[1] I'd suggest resending all your work in a
single series, the previous map optimization and the unmap optimization
here.  That way the dependency is already included, and it's a good
nudge for acks.  Thanks,

Alex


[1]https://lore.kernel.org/all/202506250037.VfdBAPP3-lkp@xxxxxxxxx/

> 
> The function vpfn_pages() can help us determine the number of vpfn
> nodes on the vpfn rb tree within a specified range. This allows us
> to avoid searching for each vpfn individually in the function
> vfio_unpin_pages_remote(). This patch batches the vfio_find_vpfn()
> calls in function vfio_unpin_pages_remote().
> 
> [1]: https://lore.kernel.org/all/20250529064947.38433-1-lizhe.67@xxxxxxxxxxxxx/
> 
> Signed-off-by: Li Zhe <lizhe.67@xxxxxxxxxxxxx>
> ---
>  drivers/vfio/vfio_iommu_type1.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 28ee4b8d39ae..e952bf8bdfab 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -805,16 +805,12 @@ static long vfio_unpin_pages_remote(struct vfio_dma *dma, dma_addr_t iova,
>  				    unsigned long pfn, unsigned long npage,
>  				    bool do_accounting)
>  {
> -	long unlocked = 0, locked = 0;
> +	long unlocked = 0, locked = vpfn_pages(dma, iova, npage);
>  	long i;
>  
> -	for (i = 0; i < npage; i++, iova += PAGE_SIZE) {
> -		if (put_pfn(pfn++, dma->prot)) {
> +	for (i = 0; i < npage; i++)
> +		if (put_pfn(pfn++, dma->prot))
>  			unlocked++;
> -			if (vfio_find_vpfn(dma, iova))
> -				locked++;
> -		}
> -	}
>  
>  	if (do_accounting)
>  		vfio_lock_acct(dma, locked - unlocked, true);