On 10/27/25 15:40, Khatri, Sunil wrote: > > On 27-10-2025 07:58 pm, Christian König wrote: >> On 10/23/25 17:30, Kuehling, Felix wrote: >>> On 2025-10-23 03:48, Arunpravin Paneer Selvam wrote: >>>> Acked-by: Arunpravin Paneer Selvam <[email protected]> >>>> >>>> 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 <[email protected]> >>>>> --- >>>>> 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. > > KFD side reported the error of NULL dereference > > pfns = kvmalloc_array(npages, sizeof(*pfns), GFP_KERNEL); //fails if the size > is too big. > > Now when we free the memory in function amdgpu_hmm_range_free and try to do a > kvfree of the range->hmm_range.hmm_pfns which is NULL and we were seeing the > NULL dereference. > So i added a check to check for the memory to be valid ptr first before > calling kvfree. > > This actually fixed the issue but i do agree that *"setting > range->hmm_range.hmm_pfns = NULL could be avoided and that why i did not > added that check in the final patch that i merged" This is the final code > after this merge.* > > voidamdgpu_hmm_range_free(structamdgpu_hmm_range*range) > { > if(!range) > return; > if(range->hmm_range.hmm_pfns) > kvfree(range->hmm_range.hmm_pfns);
That makes absolutely no sense kvfree() should be able to accept a NULL pointer as parameter. So clear NAK to that change. What exactly does the backtrace look like? Regards, Christian. > amdgpu_bo_unref(&range->bo); > kfree(range); > } > > > Regards Sunil Khatri > >>> Regards, >>> Felix >>> >>> >>>>> + >>>>> amdgpu_bo_unref(&range->bo); >>>>> kfree(range); >>>>> }
