Re: [PATCH v2] drm/amdkfd: Cal vram offset in TTM resource for each svm_migrate_copy_to_vram

[Felix Kuehling <felix.kuehling@xxxxxxx>]

Am 2023-03-01 um 11:34 schrieb Xiaogang.Chen:
> From: Xiaogang Chen <xiaogang.chen@xxxxxxx>
>
> svm_migrate_ram_to_vram migrates a prange from sys ram to vram. The prange may
> cross multiple vma. Need remember current dst vram offset in the TTM resource for
> each migration.
>
> Signed-off-by: Xiaogang Chen <Xiaogang.Chen@xxxxxxx>

The patch looks good to me. Is this a typo in the title: "Cal vram 
offset ..."? Should this be "Calculate vram offset ..."?

For a shorter title I'd suggest: "Fix BO offset for multi-VMA page 
migration".

Is a similar fix needed for migration in the other direction, VRAM to 
system memory?

Regards,
  Felix


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index 1c625433ff30..373cd7b0e1ca 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -294,7 +294,7 @@ static unsigned long svm_migrate_unsuccessful_pages(struct migrate_vma *migrate)
>   static int
>   svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
>   			 struct migrate_vma *migrate, struct dma_fence **mfence,
> -			 dma_addr_t *scratch)
> +			 dma_addr_t *scratch, uint64_t ttm_res_offset)
>   {
>   	uint64_t npages = migrate->npages;
>   	struct device *dev = adev->dev;
> @@ -304,8 +304,8 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
>   	uint64_t i, j;
>   	int r;
>   
> -	pr_debug("svms 0x%p [0x%lx 0x%lx]\n", prange->svms, prange->start,
> -		 prange->last);
> +	pr_debug("svms 0x%p [0x%lx 0x%lx 0x%lx]\n", prange->svms, prange->start,
> +		 prange->last, ttm_res_offset);
>   
>   	src = scratch;
>   	dst = (uint64_t *)(scratch + npages);
> @@ -316,7 +316,7 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
>   		goto out;
>   	}
>   
> -	amdgpu_res_first(prange->ttm_res, prange->offset << PAGE_SHIFT,
> +	amdgpu_res_first(prange->ttm_res, ttm_res_offset,
>   			 npages << PAGE_SHIFT, &cursor);
>   	for (i = j = 0; i < npages; i++) {
>   		struct page *spage;
> @@ -403,7 +403,7 @@ svm_migrate_copy_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
>   static long
>   svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
>   			struct vm_area_struct *vma, uint64_t start,
> -			uint64_t end, uint32_t trigger)
> +			uint64_t end, uint32_t trigger, uint64_t ttm_res_offset)
>   {
>   	struct kfd_process *p = container_of(prange->svms, struct kfd_process, svms);
>   	uint64_t npages = (end - start) >> PAGE_SHIFT;
> @@ -456,7 +456,7 @@ svm_migrate_vma_to_vram(struct amdgpu_device *adev, struct svm_range *prange,
>   	else
>   		pr_debug("0x%lx pages migrated\n", cpages);
>   
> -	r = svm_migrate_copy_to_vram(adev, prange, &migrate, &mfence, scratch);
> +	r = svm_migrate_copy_to_vram(adev, prange, &migrate, &mfence, scratch, ttm_res_offset);
>   	migrate_vma_pages(&migrate);
>   
>   	pr_debug("successful/cpages/npages 0x%lx/0x%lx/0x%lx\n",
> @@ -504,6 +504,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc,
>   	unsigned long addr, start, end;
>   	struct vm_area_struct *vma;
>   	struct amdgpu_device *adev;
> +	uint64_t ttm_res_offset;
>   	unsigned long cpages = 0;
>   	long r = 0;
>   
> @@ -524,6 +525,7 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc,
>   
>   	start = prange->start << PAGE_SHIFT;
>   	end = (prange->last + 1) << PAGE_SHIFT;
> +	ttm_res_offset = prange->offset << PAGE_SHIFT;
>   
>   	for (addr = start; addr < end;) {
>   		unsigned long next;
> @@ -533,13 +535,14 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc,
>   			break;
>   
>   		next = min(vma->vm_end, end);
> -		r = svm_migrate_vma_to_vram(adev, prange, vma, addr, next, trigger);
> +		r = svm_migrate_vma_to_vram(adev, prange, vma, addr, next, trigger, ttm_res_offset);
>   		if (r < 0) {
>   			pr_debug("failed %ld to migrate\n", r);
>   			break;
>   		} else {
>   			cpages += r;
>   		}
> +		ttm_res_offset += next - addr;
>   		addr = next;
>   	}
>