Re: [PATCH 1/4] drm/amdgpu: fix amdgpu_hmm_range_get_pages

[vitaly prosyak <vprosyak@amd.com>]

[-- Attachment #1: Type: text/plain, Size: 2708 bytes --]

Series is:
Reviewed-by: Vitaly Prosyak vitaly.prosyak@amd.com
Tested-by: Vitaly Prosyak vitaly.prosyak@amd.com

On 2026-05-12 12:27, Christian König wrote:
> The notifier sequence must only be read once or otherwise we could work
> with invalid pages.
>
> While at it also fix the coding style, e.g. drop the pre-initialized
> return value and use the common define for 2G range.
>
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
> index 90d26d820bac..eb470e252399 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
> @@ -51,8 +51,6 @@
>  #include "amdgpu_amdkfd.h"
>  #include "amdgpu_hmm.h"
>  
> -#define MAX_WALK_BYTE	(2UL << 30)
> -
>  /**
>   * amdgpu_hmm_invalidate_gfx - callback to notify about mm change
>   *
> @@ -170,11 +168,13 @@ int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,
>  			       void *owner,
>  			       struct amdgpu_hmm_range *range)
>  {
> -	unsigned long end;
> +	const u64 max_bytes = SZ_2G;
> +
> +	struct hmm_range *hmm_range = &range->hmm_range;
>  	unsigned long timeout;
>  	unsigned long *pfns;
> -	int r = 0;
> -	struct hmm_range *hmm_range = &range->hmm_range;
> +	unsigned long end;
> +	int r;
>  
>  	pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL);
>  	if (unlikely(!pfns)) {
> @@ -191,8 +191,9 @@ int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,
>  	end = start + npages * PAGE_SIZE;
>  	hmm_range->dev_private_owner = owner;
>  
> +	hmm_range->notifier_seq = mmu_interval_read_begin(notifier);
>  	do {
> -		hmm_range->end = min(hmm_range->start + MAX_WALK_BYTE, end);
> +		hmm_range->end = min(hmm_range->start + max_bytes, end);
>  
>  		pr_debug("hmm range: start = 0x%lx, end = 0x%lx",
>  			hmm_range->start, hmm_range->end);
> @@ -200,7 +201,6 @@ int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,
>  		timeout = jiffies + msecs_to_jiffies(HMM_RANGE_DEFAULT_TIMEOUT);
>  
>  retry:
> -		hmm_range->notifier_seq = mmu_interval_read_begin(notifier);
>  		r = hmm_range_fault(hmm_range);
>  		if (unlikely(r)) {
>  			if (r == -EBUSY && !time_after(jiffies, timeout))
> @@ -210,7 +210,7 @@ int amdgpu_hmm_range_get_pages(struct mmu_interval_notifier *notifier,
>  
>  		if (hmm_range->end == end)
>  			break;
> -		hmm_range->hmm_pfns += MAX_WALK_BYTE >> PAGE_SHIFT;
> +		hmm_range->hmm_pfns += max_bytes >> PAGE_SHIFT;
>  		hmm_range->start = hmm_range->end;
>  	} while (hmm_range->end < end);
>  

[-- Attachment #2: Type: text/html, Size: 5040 bytes --]