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

[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;
>>   }