Re: [PATCH] drm/amdkfd: remap unaligned svm ranges that have split

[Felix Kuehling <felix.kuehling@xxxxxxx>]

On 2023-10-19 16:51, Alex Sierra wrote:
> Split SVM ranges that have been mapped into 2MB page table entries,
> require to be remap in case the split has happened in a non-aligned
> VA.
> [WHY]:
> This condition causes the 2MB page table entries be split into 4KB
> PTEs.
>
> Signed-off-by: Alex Sierra <alex.sierra@xxxxxxx>

Reviewed-by: Felix Kuehling <Felix.Kuehling@xxxxxxx>


> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 43 +++++++++++++++++++++-------
>   1 file changed, 32 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index 7b81233bc9ae..aa2996d6f818 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1104,26 +1104,32 @@ svm_range_split(struct svm_range *prange, uint64_t start, uint64_t last,
>   }
>   
>   static int
> -svm_range_split_tail(struct svm_range *prange,
> -		     uint64_t new_last, struct list_head *insert_list)
> +svm_range_split_tail(struct svm_range *prange, uint64_t new_last,
> +		     struct list_head *insert_list, struct list_head *remap_list)
>   {
>   	struct svm_range *tail;
>   	int r = svm_range_split(prange, prange->start, new_last, &tail);
>   
> -	if (!r)
> +	if (!r) {
>   		list_add(&tail->list, insert_list);
> +		if (!IS_ALIGNED(new_last + 1, 1UL << prange->granularity))
> +			list_add(&tail->update_list, remap_list);
> +	}
>   	return r;
>   }
>   
>   static int
> -svm_range_split_head(struct svm_range *prange,
> -		     uint64_t new_start, struct list_head *insert_list)
> +svm_range_split_head(struct svm_range *prange, uint64_t new_start,
> +		     struct list_head *insert_list, struct list_head *remap_list)
>   {
>   	struct svm_range *head;
>   	int r = svm_range_split(prange, new_start, prange->last, &head);
>   
> -	if (!r)
> +	if (!r) {
>   		list_add(&head->list, insert_list);
> +		if (!IS_ALIGNED(new_start, 1UL << prange->granularity))
> +			list_add(&head->update_list, remap_list);
> +	}
>   	return r;
>   }
>   
> @@ -2113,7 +2119,7 @@ static int
>   svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
>   	      uint32_t nattr, struct kfd_ioctl_svm_attribute *attrs,
>   	      struct list_head *update_list, struct list_head *insert_list,
> -	      struct list_head *remove_list)
> +	      struct list_head *remove_list, struct list_head *remap_list)
>   {
>   	unsigned long last = start + size - 1UL;
>   	struct svm_range_list *svms = &p->svms;
> @@ -2129,6 +2135,7 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
>   	INIT_LIST_HEAD(insert_list);
>   	INIT_LIST_HEAD(remove_list);
>   	INIT_LIST_HEAD(&new_list);
> +	INIT_LIST_HEAD(remap_list);
>   
>   	node = interval_tree_iter_first(&svms->objects, start, last);
>   	while (node) {
> @@ -2153,6 +2160,7 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
>   			struct svm_range *old = prange;
>   
>   			prange = svm_range_clone(old);
> +
>   			if (!prange) {
>   				r = -ENOMEM;
>   				goto out;
> @@ -2161,18 +2169,17 @@ svm_range_add(struct kfd_process *p, uint64_t start, uint64_t size,
>   			list_add(&old->update_list, remove_list);
>   			list_add(&prange->list, insert_list);
>   			list_add(&prange->update_list, update_list);
> -
>   			if (node->start < start) {
>   				pr_debug("change old range start\n");
>   				r = svm_range_split_head(prange, start,
> -							 insert_list);
> +							 insert_list, remap_list);
>   				if (r)
>   					goto out;
>   			}
>   			if (node->last > last) {
>   				pr_debug("change old range last\n");
>   				r = svm_range_split_tail(prange, last,
> -							 insert_list);
> +							 insert_list, remap_list);
>   				if (r)
>   					goto out;
>   			}
> @@ -3565,6 +3572,7 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm,
>   	struct list_head update_list;
>   	struct list_head insert_list;
>   	struct list_head remove_list;
> +	struct list_head remap_list;
>   	struct svm_range_list *svms;
>   	struct svm_range *prange;
>   	struct svm_range *next;
> @@ -3596,7 +3604,7 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm,
>   
>   	/* Add new range and split existing ranges as needed */
>   	r = svm_range_add(p, start, size, nattr, attrs, &update_list,
> -			  &insert_list, &remove_list);
> +			  &insert_list, &remove_list, &remap_list);
>   	if (r) {
>   		mutex_unlock(&svms->lock);
>   		mmap_write_unlock(mm);
> @@ -3661,6 +3669,19 @@ svm_range_set_attr(struct kfd_process *p, struct mm_struct *mm,
>   			ret = r;
>   	}
>   
> +	list_for_each_entry(prange, &remap_list, update_list) {
> +		pr_debug("Remapping prange 0x%p [0x%lx 0x%lx]\n",
> +			 prange, prange->start, prange->last);
> +		mutex_lock(&prange->migrate_mutex);
> +		r = svm_range_validate_and_map(mm, prange, MAX_GPU_INSTANCE,
> +					       true, true, prange->mapped_to_gpu);
> +		if (r)
> +			pr_debug("failed %d on remap svm range\n", r);
> +		mutex_unlock(&prange->migrate_mutex);
> +		if (r)
> +			ret = r;
> +	}
> +
>   	dynamic_svm_range_dump(svms);
>   
>   	mutex_unlock(&svms->lock);