[PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit

[Christian König]

Hi Changfeng,

well that's a funny mix-up :)

The flags describe the backing store requirements, e.g. caching, 
contiguous etc etc...

But the allocation if for the housekeeping structure inside the kernel 
and is not related to the backing store of this BO.

Just switching the BO structure to be allocated using kvzalloc/kvfree 
should be enough.

Thanks,
Christian.

Am 02.06.21 um 11:10 schrieb Zhu, Changfeng:
> [AMD Official Use Only]
>
> Hi Chris,
>
> Actually, I think about switching kzalloc to kvmalloc in amdgpu_bo_create.
> However, I observe bp.flags = AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS in amdgpu_vm_pt_create.
>
> Does it matter we switch kzalloc to kvmalloc if there is a physical continuous memory request when creating bo? Such as AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS?
>
> BR,
> Changfeng.
>
>
>
> -----Original Message---
> From: Koenig, Christian <Christian.Koenig at amd.com>
> Sent: Wednesday, June 2, 2021 4:57 PM
> To: Das, Nirmoy <Nirmoy.Das at amd.com>; Zhu, Changfeng <Changfeng.Zhu at amd.com>; Huang, Ray <Ray.Huang at amd.com>; amd-gfx at freedesktop.org
> Subject: Re: [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit
>
>
>
> Am 02.06.21 um 10:54 schrieb Das, Nirmoy:
>> On 6/2/2021 10:30 AM, Changfeng wrote:
>>> From: changzhu <Changfeng.Zhu at amd.com>
>>>
>>> From: Changfeng <Changfeng.Zhu at amd.com>
>>>
>>> It will cause error when alloc memory larger than 128KB in
>>> amdgpu_bo_create->kzalloc.
>>
>> I wonder why I didn't see the error on my machine. Is there any config
>> I might be missing ?
> VM page table layout depends on hardware generation, APU vs dGPU and kernel command line settings.
>
> I think we just need to switch amdgpu_bo_create() from kzalloc to kvmalloc (and kfree to kvfree in amdgpu_bo_destroy of course).
>
> Shouldn't be more than a two line patch.
>
> Regards,
> Christian.
>
>>
>> Thanks,
>>
>> Nirmoy
>>
>>> Call Trace:
>>>      alloc_pages_current+0x6a/0xe0
>>>      kmalloc_order+0x32/0xb0
>>>      kmalloc_order_trace+0x1e/0x80
>>>      __kmalloc+0x249/0x2d0
>>>      amdgpu_bo_create+0x102/0x500 [amdgpu]
>>>      ? xas_create+0x264/0x3e0
>>>      amdgpu_bo_create_vm+0x32/0x60 [amdgpu]
>>>      amdgpu_vm_pt_create+0xf5/0x260 [amdgpu]
>>>      amdgpu_vm_init+0x1fd/0x4d0 [amdgpu]
>>>
>>> Change-Id: I29e479db45ead37c39449e856599fd4f6a0e34ce
>>> Signed-off-by: Changfeng <Changfeng.Zhu at amd.com>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 27
>>> +++++++++++++++-----------
>>>    1 file changed, 16 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> index 1923f035713a..714d613d020b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
>>> @@ -894,6 +894,10 @@ static int amdgpu_vm_pt_create(struct
>>> amdgpu_device *adev,
>>>            num_entries = 0;
>>>          bp.bo_ptr_size = struct_size((*vmbo), entries, num_entries);
>>> +    if (bp.bo_ptr_size > 32*AMDGPU_GPU_PAGE_SIZE) {
>>> +        DRM_INFO("Can't alloc memory larger than 128KB by using
>>> kzalloc in amdgpu_bo_create\n");
>>> +        bp.bo_ptr_size = sizeof(struct amdgpu_bo_vm);
>>> +    }
>>>          if (vm->use_cpu_for_update)
>>>            bp.flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>>> @@ -965,15 +969,19 @@ static int amdgpu_vm_alloc_pts(struct
>>> amdgpu_device *adev,
>>>        struct amdgpu_bo_vm *pt;
>>>        int r;
>>>    -    if (entry->base.bo) {
>>> -        if (cursor->level < AMDGPU_VM_PTB)
>>> -            entry->entries =
>>> -                to_amdgpu_bo_vm(entry->base.bo)->entries;
>>> -        else
>>> -            entry->entries = NULL;
>>> -        return 0;
>>> +    if (cursor->level < AMDGPU_VM_PTB && !entry->entries) {
>>> +        unsigned num_entries;
>>> +        num_entries = amdgpu_vm_num_entries(adev, cursor->level);
>>> +        entry->entries = kvmalloc_array(num_entries,
>>> +                        sizeof(*entry->entries),
>>> +                        GFP_KERNEL | __GFP_ZERO);
>>> +        if (!entry->entries)
>>> +            return -ENOMEM;
>>>        }
>>>    +    if (entry->base.bo)
>>> +        return 0;
>>> +
>>>        r = amdgpu_vm_pt_create(adev, vm, cursor->level, immediate,
>>> &pt);
>>>        if (r)
>>>            return r;
>>> @@ -984,10 +992,6 @@ static int amdgpu_vm_alloc_pts(struct
>>> amdgpu_device *adev,
>>>        pt_bo = &pt->bo;
>>>        pt_bo->parent = amdgpu_bo_ref(cursor->parent->base.bo);
>>>        amdgpu_vm_bo_base_init(&entry->base, vm, pt_bo);
>>> -    if (cursor->level < AMDGPU_VM_PTB)
>>> -        entry->entries = pt->entries;
>>> -    else
>>> -        entry->entries = NULL;
>>>          r = amdgpu_vm_clear_bo(adev, vm, pt, immediate);
>>>        if (r)
>>> @@ -1017,6 +1021,7 @@ static void amdgpu_vm_free_table(struct
>>> amdgpu_vm_pt *entry)
>>>            amdgpu_bo_unref(&shadow);
>>>            amdgpu_bo_unref(&entry->base.bo);
>>>        }
>>> +    kvfree(entry->entries);
>>>        entry->entries = NULL;
>>>    }
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx