All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit
@ 2021-06-02  8:30 Changfeng
  2021-06-02  8:33 ` Christian König
  2021-06-02  8:54 ` Das, Nirmoy
  0 siblings, 2 replies; 9+ messages in thread
From: Changfeng @ 2021-06-02  8:30 UTC (permalink / raw)
  To: Ray.Huang, amd-gfx, Nirmoy.Das, Christian.Koenig; +Cc: changzhu

From: changzhu <Changfeng.Zhu@amd.com>

From: Changfeng <Changfeng.Zhu@amd.com>

It will cause error when alloc memory larger than 128KB in
amdgpu_bo_create->kzalloc.

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@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;
 }
 
-- 
2.17.1

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit
  2021-06-02  8:30 [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit Changfeng
@ 2021-06-02  8:33 ` Christian König
  2021-06-02  8:54 ` Das, Nirmoy
  1 sibling, 0 replies; 9+ messages in thread
From: Christian König @ 2021-06-02  8:33 UTC (permalink / raw)
  To: Changfeng, Ray.Huang, amd-gfx, Nirmoy.Das

Am 02.06.21 um 10:30 schrieb Changfeng:
> From: changzhu <Changfeng.Zhu@amd.com>
>
> From: Changfeng <Changfeng.Zhu@amd.com>
>
> It will cause error when alloc memory larger than 128KB in
> amdgpu_bo_create->kzalloc.

Well NAK, in that case we should just switch to kvmalloc here.

Christian.

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit
  2021-06-02  8:30 [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit Changfeng
  2021-06-02  8:33 ` Christian König
@ 2021-06-02  8:54 ` Das, Nirmoy
  2021-06-02  8:57   ` Christian König
  1 sibling, 1 reply; 9+ messages in thread
From: Das, Nirmoy @ 2021-06-02  8:54 UTC (permalink / raw)
  To: Changfeng, Ray.Huang, amd-gfx, Christian.Koenig


On 6/2/2021 10:30 AM, Changfeng wrote:
> From: changzhu <Changfeng.Zhu@amd.com>
>
> From: Changfeng <Changfeng.Zhu@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 ?


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit
  2021-06-02  8:54 ` Das, Nirmoy
@ 2021-06-02  8:57   ` Christian König
  2021-06-02  9:00     ` Das, Nirmoy
  2021-06-02  9:10     ` Zhu, Changfeng
  0 siblings, 2 replies; 9+ messages in thread
From: Christian König @ 2021-06-02  8:57 UTC (permalink / raw)
  To: Das, Nirmoy, Changfeng, Ray.Huang, amd-gfx



Am 02.06.21 um 10:54 schrieb Das, Nirmoy:
>
> On 6/2/2021 10:30 AM, Changfeng wrote:
>> From: changzhu <Changfeng.Zhu@amd.com>
>>
>> From: Changfeng <Changfeng.Zhu@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@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit
  2021-06-02  8:57   ` Christian König
@ 2021-06-02  9:00     ` Das, Nirmoy
  2021-06-02  9:10     ` Zhu, Changfeng
  1 sibling, 0 replies; 9+ messages in thread
From: Das, Nirmoy @ 2021-06-02  9:00 UTC (permalink / raw)
  To: Christian König, Changfeng, Ray.Huang, amd-gfx


On 6/2/2021 10:57 AM, Christian König wrote:
>
>
> Am 02.06.21 um 10:54 schrieb Das, Nirmoy:
>>
>> On 6/2/2021 10:30 AM, Changfeng wrote:
>>> From: changzhu <Changfeng.Zhu@amd.com>
>>>
>>> From: Changfeng <Changfeng.Zhu@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.


Thanks for clarifying, Christian.


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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit
  2021-06-02  8:57   ` Christian König
  2021-06-02  9:00     ` Das, Nirmoy
@ 2021-06-02  9:10     ` Zhu, Changfeng
  2021-06-02  9:31       ` Das, Nirmoy
  2021-06-02  9:40       ` Christian König
  1 sibling, 2 replies; 9+ messages in thread
From: Zhu, Changfeng @ 2021-06-02  9:10 UTC (permalink / raw)
  To: Koenig, Christian, Das, Nirmoy, Huang, Ray, amd-gfx

[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@amd.com> 
Sent: Wednesday, June 2, 2021 4:57 PM
To: Das, Nirmoy <Nirmoy.Das@amd.com>; Zhu, Changfeng <Changfeng.Zhu@amd.com>; Huang, Ray <Ray.Huang@amd.com>; amd-gfx@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@amd.com>
>>
>> From: Changfeng <Changfeng.Zhu@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@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit
  2021-06-02  9:10     ` Zhu, Changfeng
@ 2021-06-02  9:31       ` Das, Nirmoy
  2021-06-02  9:40       ` Christian König
  1 sibling, 0 replies; 9+ messages in thread
From: Das, Nirmoy @ 2021-06-02  9:31 UTC (permalink / raw)
  To: Zhu, Changfeng, Koenig, Christian, Huang, Ray, amd-gfx


On 6/2/2021 11:10 AM, Zhu, Changfeng wrote:
> [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?


Only driver will access this BO struct, so kvmalloc will be fine.


Regards,

Nirmoy


>
> BR,
> Changfeng.
>
>
>
> -----Original Message---
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Wednesday, June 2, 2021 4:57 PM
> To: Das, Nirmoy <Nirmoy.Das@amd.com>; Zhu, Changfeng <Changfeng.Zhu@amd.com>; Huang, Ray <Ray.Huang@amd.com>; amd-gfx@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@amd.com>
>>>
>>> From: Changfeng <Changfeng.Zhu@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@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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit
  2021-06-02  9:10     ` Zhu, Changfeng
  2021-06-02  9:31       ` Das, Nirmoy
@ 2021-06-02  9:40       ` Christian König
  2021-06-02 10:02         ` Zhu, Changfeng
  1 sibling, 1 reply; 9+ messages in thread
From: Christian König @ 2021-06-02  9:40 UTC (permalink / raw)
  To: Zhu, Changfeng, Koenig, Christian, Das, Nirmoy, Huang, Ray, amd-gfx

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@amd.com>
> Sent: Wednesday, June 2, 2021 4:57 PM
> To: Das, Nirmoy <Nirmoy.Das@amd.com>; Zhu, Changfeng <Changfeng.Zhu@amd.com>; Huang, Ray <Ray.Huang@amd.com>; amd-gfx@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@amd.com>
>>>
>>> From: Changfeng <Changfeng.Zhu@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@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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit
  2021-06-02  9:40       ` Christian König
@ 2021-06-02 10:02         ` Zhu, Changfeng
  0 siblings, 0 replies; 9+ messages in thread
From: Zhu, Changfeng @ 2021-06-02 10:02 UTC (permalink / raw)
  To: Christian König, Koenig, Christian, Das, Nirmoy, Huang, Ray,
	amd-gfx

[AMD Official Use Only]

OK.

Thx, Chris and Das.

I'll try it and verify whether there are issues.

BR,
Changfeng.

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Christian König
Sent: Wednesday, June 2, 2021 5:41 PM
To: Zhu, Changfeng <Changfeng.Zhu@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Das, Nirmoy <Nirmoy.Das@amd.com>; Huang, Ray <Ray.Huang@amd.com>; amd-gfx@freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit

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@amd.com>
> Sent: Wednesday, June 2, 2021 4:57 PM
> To: Das, Nirmoy <Nirmoy.Das@amd.com>; Zhu, Changfeng 
> <Changfeng.Zhu@amd.com>; Huang, Ray <Ray.Huang@amd.com>; 
> amd-gfx@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@amd.com>
>>>
>>> From: Changfeng <Changfeng.Zhu@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@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@lists.freedesktop.org
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flist
> s.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7CCh
> angfeng.Zhu%40amd.com%7Cd36c08795213458131a008d925aa7ed1%7C3dd8961fe48
> 84e608e11a82d994e183d%7C0%7C0%7C637582236475879048%7CUnknown%7CTWFpbGZ
> sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7C1000&amp;sdata=0TrtpWThdlCk8Pt5AvlFELrZhkXMqkY85KsPMSwEQf4%3D&amp;
> reserved=0

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7CChangfeng.Zhu%40amd.com%7Cd36c08795213458131a008d925aa7ed1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637582236475879048%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=0TrtpWThdlCk8Pt5AvlFELrZhkXMqkY85KsPMSwEQf4%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-06-02 10:02 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-02  8:30 [PATCH] drm/amdgpu: take back kvmalloc_array for entries alloc because of kzalloc memory limit Changfeng
2021-06-02  8:33 ` Christian König
2021-06-02  8:54 ` Das, Nirmoy
2021-06-02  8:57   ` Christian König
2021-06-02  9:00     ` Das, Nirmoy
2021-06-02  9:10     ` Zhu, Changfeng
2021-06-02  9:31       ` Das, Nirmoy
2021-06-02  9:40       ` Christian König
2021-06-02 10:02         ` Zhu, Changfeng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.