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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxx>
Sent: Wednesday, June 2, 2021 4:57 PM
To: Das, Nirmoy <Nirmoy.Das@xxxxxxx>; Zhu, Changfeng <Changfeng.Zhu@xxxxxxx>; Huang, Ray <Ray.Huang@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxx
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@xxxxxxx>

From: Changfeng <Changfeng.Zhu@xxxxxxx>

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@xxxxxxx>
---
   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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux