Re: [Patch v1] drm/amdgpu: null check for hmm_pfns ptr before freeing it

["Christian König" <christian.koenig@amd.com>]

On 10/23/25 17:30, Kuehling, Felix wrote:
> 
> On 2025-10-23 03:48, Arunpravin Paneer Selvam wrote:
>> Acked-by: Arunpravin Paneer Selvam <Arunpravin.PaneerSelvam@amd.com>
>>
>> Regards,
>> Arun.
>> On 10/23/2025 12:28 PM, Sunil Khatri wrote:
>>> Due to low memory or when num of pages is too big to be
>>> accomodated, allocation could fail for pfn's.
>>>
>>> Chekc hmm_pfns for NULL before calling the kvfree for the it.
>>>
>>> Signed-off-by: Sunil Khatri <sunil.khatri@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c | 6 +++++-
>>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
>>> index d6f903a2d573..6ac206e2bc46 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_hmm.c
>>> @@ -286,7 +286,11 @@ void amdgpu_hmm_range_free(struct amdgpu_hmm_range *range)
>>>       if (!range)
>>>           return;
>>>   -    kvfree(range->hmm_range.hmm_pfns);
>>> +    if (range->hmm_range.hmm_pfns) {
>>> +        kvfree(range->hmm_range.hmm_pfns);
>>> +        range->hmm_range.hmm_pfns = NULL;
>>> +    }
> 
> NULL-checks before kfree and friends are unnecessary. There are actually static checkers that complain about such unnecessary NULL-checks. For example, see https://lkml.org/lkml/2024/8/11/168.
> 
> The same is also true for the standard libc free in usermode: https://stackoverflow.com/questions/1912325/checking-for-null-before-calling-free.
> 
> Finally, setting range->hmm_range.hmm_pfns = NULL is also unnecessary because you're about to free the whole range structure anyway.

Agree completely with Felix.

Sunil why do you think that this is necessary and blocking KFD for some reason?

Regards,
Christian.

> 
> Regards,
>   Felix
> 
> 
>>> +
>>>       amdgpu_bo_unref(&range->bo);
>>>       kfree(range);
>>>   }
>>