On 9/29/2016 7:47 AM, Jike Song wrote:
> +Guangrong
>
> On 08/25/2016 11:53 AM, Kirti Wankhede wrote:
...
>> +static long vfio_iommu_type1_pin_pages(void *iommu_data,
>> + unsigned long *user_pfn,
>> + long npage, int prot,
>> + unsigned long *phys_pfn)
>> +{
>> + struct vfio_iommu *iommu = iommu_data;
>> + struct vfio_domain *domain;
>> + int i, j, ret;
>> + long retpage;
>> + unsigned long remote_vaddr;
>> + unsigned long *pfn = phys_pfn;
>> + struct vfio_dma *dma;
>> + bool do_accounting = false;
>> +
>> + if (!iommu || !user_pfn || !phys_pfn)
>> + return -EINVAL;
>> +
>> + mutex_lock(&iommu->lock);
>> +
>> + if (!iommu->local_domain) {
>> + ret = -EINVAL;
>> + goto pin_done;
>> + }
>> +
>> + domain = iommu->local_domain;
>> +
>> + /*
>> + * If iommu capable domain exist in the container then all pages are
>> + * already pinned and accounted. Accouting should be done if there is no
>> + * iommu capable domain in the container.
>> + */
>> + do_accounting = !IS_IOMMU_CAPABLE_DOMAIN_IN_CONTAINER(iommu);
>> +
>> + for (i = 0; i < npage; i++) {
>> + struct vfio_pfn *p;
>> + dma_addr_t iova;
>> +
>> + iova = user_pfn[i] << PAGE_SHIFT;
>> +
>> + dma = vfio_find_dma(iommu, iova, 0);
>> + if (!dma) {
>> + ret = -EINVAL;
>> + goto pin_unwind;
>> + }
>> +
>> + remote_vaddr = dma->vaddr + iova - dma->iova;
>> +
>> + retpage = __vfio_pin_pages_local(domain, remote_vaddr, prot,
>> + &pfn[i], do_accounting);
>
> Hi Kirti,
>
> Here you call __vfio_pin_pages_local() > vaddr_get_pfn() > GUP regardless
> whether the vaddr already pinned or not. That probably means, if the caller
> calls vfio_pin_pages() with a GPA for multiple times, you get memory leaks.
>
> GUP always increases the page refcnt.
>
> FWIW, I would like to have the pfn_list_lock implemented with key == iova,
> so you can always try to find the PFN for a given iova, and pin it only if
> not found.
>
I didn't get how there would be a memory leak.
Right, GUP increases refcnt, so if vfio_pin_pages() is called for
multiple types for same GPA, refcnt would be incremented. In
vfio_iommu_type1_pin_pages() pinned pages list is maintained with
ref_count. If pfn is already in list, ref_count is incremented and same
is used while unpining pages.
Kirti
