Re: [PATCH v2 2/3] drm/amdkfd: svm unmap use page aligned address

[Felix Kuehling <felix.kuehling@amd.com>]

On 2025-10-03 14:15, Philip Yang wrote:
> svm_range_unmap_from_gpus uses page aligned start, end address, the end
> address is inclusive.
>
> Fixes: 38c55f6719f7 ("drm/amdkfd: Handle lack of READ permissions in SVM mapping")
> Signed-off-by: Philip Yang <Philip.Yang@amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> index e8a15751c125..0aadd20be56a 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
> @@ -1723,10 +1723,9 @@ static int svm_range_validate_and_map(struct mm_struct *mm,
>   				svm_range_lock(prange);
>   				if (vma->vm_flags & VM_WRITE)
>   					pr_debug("VM_WRITE without VM_READ is not supported");
> -				s = max(start, prange->start);
> -				e = min(end, prange->last);
> -				if (e >= s)
> -					r = svm_range_unmap_from_gpus(prange, s, e,
> +				s = max(addr >> PAGE_SHIFT, prange->start);
> +				e = s + npages - 1;

This is confusing. The old code was clamping prange->start/last with 
start/end. start/end are based on map_start/map_end. Your updated code 
uses npages, which comes from the VMA, which may not start at the same 
address as the prange or the map_start. So I think just using npages 
here is incorrect.

You also completely removed the condition that s >= e. I think that's 
OK, since all callers make sure that map_start/map_end fall inside 
prange->start/last.

What are you actually trying to unmap here? Do you want to unmap the 
entire prange, the part of prange inside map_start..map_end, or the part 
of prange that overlaps with the VMA, or something else?

Regards,
   Felix


> +				r = svm_range_unmap_from_gpus(prange, s, e,
>   						       KFD_SVM_UNMAP_TRIGGER_UNMAP_FROM_CPU);
>   				svm_range_unlock(prange);
>   				/* If unmap returns non-zero, we'll bail on the next for loop