Re: [PATCH v11 07/22] vfio iommu type1: Update argument of vaddr_get_pfn()

[Alexey Kardashevskiy <aik@xxxxxxxxx>]

On 05/11/16 08:10, Kirti Wankhede wrote:
> Update arguments of vaddr_get_pfn() to take struct mm_struct *mm as input
> argument.
> 
> Signed-off-by: Kirti Wankhede <kwankhede@xxxxxxxxxx>
> Signed-off-by: Neo Jia <cjia@xxxxxxxxxx>
> Change-Id: I885fd4cd4a9f66f4ee2c1caf58267464ec239f52
> ---
>  drivers/vfio/vfio_iommu_type1.c | 30 +++++++++++++++++++++++-------
>  1 file changed, 23 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 02b302d0b7de..653386e80e85 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -226,20 +226,36 @@ static int put_pfn(unsigned long pfn, int prot)
>  	return 0;
>  }
>  
> -static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
> +static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
> +			 int prot, unsigned long *pfn)
>  {
>  	struct page *page[1];
>  	struct vm_area_struct *vma;
>  	int ret = -EFAULT;
>  
> -	if (get_user_pages_fast(vaddr, 1, !!(prot & IOMMU_WRITE), page) == 1) {
> +	if (mm == current->mm)


afaik the rule is if one branch of "if" uses curly braces, the other should
do so too.


> +		ret = get_user_pages_fast(vaddr, 1, !!(prot & IOMMU_WRITE),
> +					  page);
> +	else {
> +		unsigned int flags = 0;
> +
> +		if (prot & IOMMU_WRITE)
> +			flags |= FOLL_WRITE;
> +
> +		down_read(&mm->mmap_sem);
> +		ret = get_user_pages_remote(NULL, mm, vaddr, 1, flags, page,
> +					    NULL);
> +		up_read(&mm->mmap_sem);


This chunk is not just about passing mm everywhere, it would be nice to see
in the commit log why this change is in this patch (may be it was commented
already, and I just missed it?).


> +	}
> +
> +	if (ret == 1) {
>  		*pfn = page_to_pfn(page[0]);
>  		return 0;
>  	}
>  
> -	down_read(&current->mm->mmap_sem);
> +	down_read(&mm->mmap_sem);
>  
> -	vma = find_vma_intersection(current->mm, vaddr, vaddr + 1);
> +	vma = find_vma_intersection(mm, vaddr, vaddr + 1);
>  
>  	if (vma && vma->vm_flags & VM_PFNMAP) {
>  		*pfn = ((vaddr - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff;
> @@ -247,7 +263,7 @@ static int vaddr_get_pfn(unsigned long vaddr, int prot, unsigned long *pfn)
>  			ret = 0;
>  	}
>  
> -	up_read(&current->mm->mmap_sem);
> +	up_read(&mm->mmap_sem);
>  
>  	return ret;
>  }
> @@ -268,7 +284,7 @@ static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
>  	if (!current->mm)
>  		return -ENODEV;
>  
> -	ret = vaddr_get_pfn(vaddr, prot, pfn_base);
> +	ret = vaddr_get_pfn(current->mm, vaddr, prot, pfn_base);
>  	if (ret)
>  		return ret;
>  
> @@ -291,7 +307,7 @@ static long __vfio_pin_pages_remote(unsigned long vaddr, long npage,
>  	for (i = 1, vaddr += PAGE_SIZE; i < npage; i++, vaddr += PAGE_SIZE) {
>  		unsigned long pfn = 0;
>  
> -		ret = vaddr_get_pfn(vaddr, prot, &pfn);
> +		ret = vaddr_get_pfn(current->mm, vaddr, prot, &pfn);
>  		if (ret)
>  			break;
>  
> 


-- 
Alexey

Attachment: signature.asc
Description: OpenPGP digital signature