diff mbox

[libdrm,v2] amdgpu: Add memory over allocation test.

Message ID 1510592502-2230-1-git-send-email-andrey.grodzovsky@amd.com (mailing list archive)
State New, archived
Headers show

Commit Message

Andrey Grodzovsky Nov. 13, 2017, 5:01 p.m. UTC
Allocates 1 TB of memory. Test is disabled by default
since it's triggers OOM killer.

v2:
FIx the test to only alloc the BO and assert if return value
not equal to -ENOMEM and remove test disable on start.

Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
---
 tests/amdgpu/bo_tests.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

Comments

Christian König Nov. 14, 2017, 8:44 a.m. UTC | #1
Am 13.11.2017 um 18:01 schrieb Andrey Grodzovsky:
> Allocates 1 TB of memory. Test is disabled by default
> since it's triggers OOM killer.
>
> v2:
> FIx the test to only alloc the BO and assert if return value
> not equal to -ENOMEM and remove test disable on start.
>
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>

I was just about to scream that you need to fix the helper first, but 
then I saw that you now directly use amdgpu_bo_alloc().

The commit message could be changed and for the unlikely case that we 
actually can allocate a BO of 1TB size we should free it.

But apart from that the patch looks good to me and is Acked-by: 
Christian König <christian.koenig@amd.com>.

Regards,
Christian.

> ---
>   tests/amdgpu/bo_tests.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
>
> diff --git a/tests/amdgpu/bo_tests.c b/tests/amdgpu/bo_tests.c
> index 4545196..53e76c1 100644
> --- a/tests/amdgpu/bo_tests.c
> +++ b/tests/amdgpu/bo_tests.c
> @@ -47,6 +47,7 @@ static void amdgpu_bo_export_import(void);
>   static void amdgpu_bo_metadata(void);
>   static void amdgpu_bo_map_unmap(void);
>   static void amdgpu_memory_alloc(void);
> +static void amdgpu_mem_fail_alloc(void);
>   
>   CU_TestInfo bo_tests[] = {
>   	{ "Export/Import",  amdgpu_bo_export_import },
> @@ -55,6 +56,7 @@ CU_TestInfo bo_tests[] = {
>   #endif
>   	{ "CPU map/unmap",  amdgpu_bo_map_unmap },
>   	{ "Memory alloc Test",  amdgpu_memory_alloc },
> +	{ "Memory fail alloc Test",  amdgpu_mem_fail_alloc },
>   	CU_TEST_INFO_NULL,
>   };
>   
> @@ -244,3 +246,25 @@ static void amdgpu_memory_alloc(void)
>   	r = gpu_mem_free(bo, va_handle, bo_mc, 4096);
>   	CU_ASSERT_EQUAL(r, 0);
>   }
> +
> +static void amdgpu_mem_fail_alloc(void)
> +{
> +	amdgpu_bo_handle bo;
> +	int r;
> +	struct amdgpu_bo_alloc_request req = {0};
> +	amdgpu_bo_handle buf_handle;
> +
> +	/* Test impossible mem allocation, 1TB */
> +	req.alloc_size = 0xE8D4A51000;
> +	req.phys_alignment = 4096;
> +	req.preferred_heap = AMDGPU_GEM_DOMAIN_VRAM;
> +	req.flags = AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
> +
> +	r = amdgpu_bo_alloc(device_handle, &req, &buf_handle);
> +	CU_ASSERT_EQUAL(r, -ENOMEM);
> +
> +	if (!r) {
> +		r = amdgpu_bo_free(bo);
> +		CU_ASSERT_EQUAL(r, 0);
> +	}
> +}
Andrey Grodzovsky Nov. 14, 2017, 12:53 p.m. UTC | #2
On 11/14/2017 03:44 AM, Christian König wrote:
> Am 13.11.2017 um 18:01 schrieb Andrey Grodzovsky:
>> Allocates 1 TB of memory. Test is disabled by default
>> since it's triggers OOM killer.
>>
>> v2:
>> FIx the test to only alloc the BO and assert if return value
>> not equal to -ENOMEM and remove test disable on start.
>>
>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>
> I was just about to scream that you need to fix the helper first, but 
> then I saw that you now directly use amdgpu_bo_alloc().
>
> The commit message could be changed and for the unlikely case that we 
> actually can allocate a BO of 1TB size we should free it.

The patch already does this

if (!r) {
         r = amdgpu_bo_free(bo);
         CU_ASSERT_EQUAL(r, 0);
     }

Thanks,
Andrey

>
> But apart from that the patch looks good to me and is Acked-by: 
> Christian König <christian.koenig@amd.com>.
>
> Regards,
> Christian.
>
>> ---
>>   tests/amdgpu/bo_tests.c | 24 ++++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/tests/amdgpu/bo_tests.c b/tests/amdgpu/bo_tests.c
>> index 4545196..53e76c1 100644
>> --- a/tests/amdgpu/bo_tests.c
>> +++ b/tests/amdgpu/bo_tests.c
>> @@ -47,6 +47,7 @@ static void amdgpu_bo_export_import(void);
>>   static void amdgpu_bo_metadata(void);
>>   static void amdgpu_bo_map_unmap(void);
>>   static void amdgpu_memory_alloc(void);
>> +static void amdgpu_mem_fail_alloc(void);
>>     CU_TestInfo bo_tests[] = {
>>       { "Export/Import",  amdgpu_bo_export_import },
>> @@ -55,6 +56,7 @@ CU_TestInfo bo_tests[] = {
>>   #endif
>>       { "CPU map/unmap",  amdgpu_bo_map_unmap },
>>       { "Memory alloc Test",  amdgpu_memory_alloc },
>> +    { "Memory fail alloc Test",  amdgpu_mem_fail_alloc },
>>       CU_TEST_INFO_NULL,
>>   };
>>   @@ -244,3 +246,25 @@ static void amdgpu_memory_alloc(void)
>>       r = gpu_mem_free(bo, va_handle, bo_mc, 4096);
>>       CU_ASSERT_EQUAL(r, 0);
>>   }
>> +
>> +static void amdgpu_mem_fail_alloc(void)
>> +{
>> +    amdgpu_bo_handle bo;
>> +    int r;
>> +    struct amdgpu_bo_alloc_request req = {0};
>> +    amdgpu_bo_handle buf_handle;
>> +
>> +    /* Test impossible mem allocation, 1TB */
>> +    req.alloc_size = 0xE8D4A51000;
>> +    req.phys_alignment = 4096;
>> +    req.preferred_heap = AMDGPU_GEM_DOMAIN_VRAM;
>> +    req.flags = AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
>> +
>> +    r = amdgpu_bo_alloc(device_handle, &req, &buf_handle);
>> +    CU_ASSERT_EQUAL(r, -ENOMEM);
>> +
>> +    if (!r) {
>> +        r = amdgpu_bo_free(bo);
>> +        CU_ASSERT_EQUAL(r, 0);
>> +    }
>> +}
>
>
Christian König Nov. 14, 2017, 1:25 p.m. UTC | #3
Am 14.11.2017 um 13:53 schrieb Andrey Grodzovsky:
>
>
> On 11/14/2017 03:44 AM, Christian König wrote:
>> Am 13.11.2017 um 18:01 schrieb Andrey Grodzovsky:
>>> Allocates 1 TB of memory. Test is disabled by default
>>> since it's triggers OOM killer.
>>>
>>> v2:
>>> FIx the test to only alloc the BO and assert if return value
>>> not equal to -ENOMEM and remove test disable on start.
>>>
>>> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
>>
>> I was just about to scream that you need to fix the helper first, but 
>> then I saw that you now directly use amdgpu_bo_alloc().
>>
>> The commit message could be changed and for the unlikely case that we 
>> actually can allocate a BO of 1TB size we should free it.
>
> The patch already does this
>
> if (!r) {
>         r = amdgpu_bo_free(bo);
>         CU_ASSERT_EQUAL(r, 0);
>     }

Not enough sleep last night.

Going to review and push the patch now.

Thanks,
Christian.

>
>
> Thanks,
> Andrey
>
>>
>> But apart from that the patch looks good to me and is Acked-by: 
>> Christian König <christian.koenig@amd.com>.
>>
>> Regards,
>> Christian.
>>
>>> ---
>>>   tests/amdgpu/bo_tests.c | 24 ++++++++++++++++++++++++
>>>   1 file changed, 24 insertions(+)
>>>
>>> diff --git a/tests/amdgpu/bo_tests.c b/tests/amdgpu/bo_tests.c
>>> index 4545196..53e76c1 100644
>>> --- a/tests/amdgpu/bo_tests.c
>>> +++ b/tests/amdgpu/bo_tests.c
>>> @@ -47,6 +47,7 @@ static void amdgpu_bo_export_import(void);
>>>   static void amdgpu_bo_metadata(void);
>>>   static void amdgpu_bo_map_unmap(void);
>>>   static void amdgpu_memory_alloc(void);
>>> +static void amdgpu_mem_fail_alloc(void);
>>>     CU_TestInfo bo_tests[] = {
>>>       { "Export/Import",  amdgpu_bo_export_import },
>>> @@ -55,6 +56,7 @@ CU_TestInfo bo_tests[] = {
>>>   #endif
>>>       { "CPU map/unmap",  amdgpu_bo_map_unmap },
>>>       { "Memory alloc Test",  amdgpu_memory_alloc },
>>> +    { "Memory fail alloc Test",  amdgpu_mem_fail_alloc },
>>>       CU_TEST_INFO_NULL,
>>>   };
>>>   @@ -244,3 +246,25 @@ static void amdgpu_memory_alloc(void)
>>>       r = gpu_mem_free(bo, va_handle, bo_mc, 4096);
>>>       CU_ASSERT_EQUAL(r, 0);
>>>   }
>>> +
>>> +static void amdgpu_mem_fail_alloc(void)
>>> +{
>>> +    amdgpu_bo_handle bo;
>>> +    int r;
>>> +    struct amdgpu_bo_alloc_request req = {0};
>>> +    amdgpu_bo_handle buf_handle;
>>> +
>>> +    /* Test impossible mem allocation, 1TB */
>>> +    req.alloc_size = 0xE8D4A51000;
>>> +    req.phys_alignment = 4096;
>>> +    req.preferred_heap = AMDGPU_GEM_DOMAIN_VRAM;
>>> +    req.flags = AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
>>> +
>>> +    r = amdgpu_bo_alloc(device_handle, &req, &buf_handle);
>>> +    CU_ASSERT_EQUAL(r, -ENOMEM);
>>> +
>>> +    if (!r) {
>>> +        r = amdgpu_bo_free(bo);
>>> +        CU_ASSERT_EQUAL(r, 0);
>>> +    }
>>> +}
>>
>>
>
diff mbox

Patch

diff --git a/tests/amdgpu/bo_tests.c b/tests/amdgpu/bo_tests.c
index 4545196..53e76c1 100644
--- a/tests/amdgpu/bo_tests.c
+++ b/tests/amdgpu/bo_tests.c
@@ -47,6 +47,7 @@  static void amdgpu_bo_export_import(void);
 static void amdgpu_bo_metadata(void);
 static void amdgpu_bo_map_unmap(void);
 static void amdgpu_memory_alloc(void);
+static void amdgpu_mem_fail_alloc(void);
 
 CU_TestInfo bo_tests[] = {
 	{ "Export/Import",  amdgpu_bo_export_import },
@@ -55,6 +56,7 @@  CU_TestInfo bo_tests[] = {
 #endif
 	{ "CPU map/unmap",  amdgpu_bo_map_unmap },
 	{ "Memory alloc Test",  amdgpu_memory_alloc },
+	{ "Memory fail alloc Test",  amdgpu_mem_fail_alloc },
 	CU_TEST_INFO_NULL,
 };
 
@@ -244,3 +246,25 @@  static void amdgpu_memory_alloc(void)
 	r = gpu_mem_free(bo, va_handle, bo_mc, 4096);
 	CU_ASSERT_EQUAL(r, 0);
 }
+
+static void amdgpu_mem_fail_alloc(void)
+{
+	amdgpu_bo_handle bo;
+	int r;
+	struct amdgpu_bo_alloc_request req = {0};
+	amdgpu_bo_handle buf_handle;
+
+	/* Test impossible mem allocation, 1TB */
+	req.alloc_size = 0xE8D4A51000;
+	req.phys_alignment = 4096;
+	req.preferred_heap = AMDGPU_GEM_DOMAIN_VRAM;
+	req.flags = AMDGPU_GEM_CREATE_NO_CPU_ACCESS;
+
+	r = amdgpu_bo_alloc(device_handle, &req, &buf_handle);
+	CU_ASSERT_EQUAL(r, -ENOMEM);
+
+	if (!r) {
+		r = amdgpu_bo_free(bo);
+		CU_ASSERT_EQUAL(r, 0);
+	}
+}