Skip to contentSkip to conversation

app/test-gpudev: remove memory leaks in case of errors

PATCH Superseded [email protected] 3 participants 5 messages Nov 17, 2021 – Nov 26, 2021

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/1 app/test-gpudev: remove memory leaks in case of errors

From: Elena Agostini <[email protected]>

Signed-off-by Elena Agostini <[email protected]>
---
 app/test-gpudev/main.c | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)
app/test-gpudev/main.c +28 -13
index c647e30de4..fe5c1e5b15 100644
@@ -70,7 +70,7 @@ alloc_gpu_memory(uint16_t gpu_id)
 	size_t buf_bytes = 1024;
 	int ret;
 
-	printf("\n=======> TEST: Allocate GPU memory\n\n");
+	printf("\nTEST: Allocate GPU memory\n\n");
 
 	/* Alloc memory on GPU 0 */
 	ptr_1 = rte_gpu_mem_alloc(gpu_id, buf_bytes);

@@ -113,11 +113,18 @@ alloc_gpu_memory(uint16_t gpu_id)
 	}
 	printf("GPU memory 0x%p freed\n", ptr_1);
 
-	printf("\n=======> TEST: PASSED\n");
+	printf("\nTEST: PASSED\n");
 	return 0;
 
 error:
-	printf("\n=======> TEST: FAILED\n");
+
+	if (ptr_1 != NULL)
+		rte_gpu_mem_free(gpu_id, ptr_1);
+
+	if (ptr_2 != NULL)
+		rte_gpu_mem_free(gpu_id, ptr_2);
+
+	printf("\nTEST: FAILED\n");
 	return -1;
 }
 

@@ -128,7 +135,7 @@ register_cpu_memory(uint16_t gpu_id)
 	size_t buf_bytes = 1024;
 	int ret;
 
-	printf("\n=======> TEST: Register CPU memory\n\n");
+	printf("\nTEST: Register CPU memory\n\n");
 
 	/* Alloc memory on CPU visible from GPU 0 */
 	ptr = rte_zmalloc(NULL, buf_bytes, 0);

@@ -161,11 +168,15 @@ register_cpu_memory(uint16_t gpu_id)
 	}
 	printf("CPU memory 0x%p unregistered\n", ptr);
 
-	printf("\n=======> TEST: PASSED\n");
+	printf("\nTEST: PASSED\n");
 	return 0;
 
 error:
-	printf("\n=======> TEST: FAILED\n");
+
+	if (ptr != NULL)
+		rte_gpu_mem_unregister(gpu_id, ptr);
+
+	printf("\nTEST: FAILED\n");
 	return -1;
 }
 

@@ -177,7 +188,7 @@ create_update_comm_flag(uint16_t gpu_id)
 	uint32_t set_val;
 	uint32_t get_val;
 
-	printf("\n=======> TEST: Communication flag\n\n");
+	printf("\nTEST: Communication flag\n\n");
 
 	ret = rte_gpu_comm_create_flag(gpu_id, &devflag, RTE_GPU_COMM_FLAG_CPU);
 	if (ret < 0) {

@@ -223,11 +234,13 @@ create_update_comm_flag(uint16_t gpu_id)
 		goto error;
 	}
 
-	printf("\n=======> TEST: PASSED\n");
+	printf("\nTEST: PASSED\n");
 	return 0;
 
 error:
-	printf("\n=======> TEST: FAILED\n");
+
+	rte_gpu_comm_destroy_flag(&devflag);
+	printf("\nTEST: FAILED\n");
 	return -1;
 }
 

@@ -254,11 +267,11 @@ create_update_comm_list(uint16_t gpu_id)
 {
 	int ret = 0;
 	int i = 0;
-	struct rte_gpu_comm_list *comm_list;
+	struct rte_gpu_comm_list *comm_list = NULL;
 	uint32_t num_comm_items = 1024;
 	struct rte_mbuf *mbufs[10];
 
-	printf("\n=======> TEST: Communication list\n\n");
+	printf("\nTEST: Communication list\n\n");
 
 	comm_list = rte_gpu_comm_create_list(gpu_id, num_comm_items);
 	if (comm_list == NULL) {

@@ -323,11 +336,13 @@ create_update_comm_list(uint16_t gpu_id)
 	for (i = 0; i < 10; i++)
 		rte_free(mbufs[i]);
 
-	printf("\n=======> TEST: PASSED\n");
+	printf("\nTEST: PASSED\n");
 	return 0;
 
 error:
-	printf("\n=======> TEST: FAILED\n");
+
+	rte_gpu_comm_destroy_list(comm_list, num_comm_items);
+	printf("\nTEST: FAILED\n");
 	return -1;
 }
Signature · 1 line
2.17.1
#1 lore raw

On Wed, Nov 17, 2021 at 1:34 PM <[email protected]> wrote:

Commitlog.

+ Fixes: tag.

Quoted 13 lines

From: Elena Agostini <[email protected]>

Signed-off-by Elena Agostini <[email protected]>
---
 app/test-gpudev/main.c | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)
app/test-gpudev/main.c +0 -0
index c647e30de4..fe5c1e5b15 100644

app/test-gpudev/main.c alloc_gpu_memory(uint16_t gpu_id) · line 73

Quoted 5 lines
app/test-gpudev/main.c +1 -1
@@ -70,7 +70,7 @@ alloc_gpu_memory(uint16_t gpu_id)
        size_t buf_bytes = 1024;
        int ret;
 
-       printf("\n=======> TEST: Allocate GPU memory\n\n");
+       printf("\nTEST: Allocate GPU memory\n\n");

This hunk (and others removing =====>) is not related to fixing leaks. Please drop.

Quoted 6 lines
app/test-gpudev/main.c +0 -0
        /* Alloc memory on GPU 0 */
        ptr_1 = rte_gpu_mem_alloc(gpu_id, buf_bytes);

@@ -113,11 +113,18 @@ alloc_gpu_memory(uint16_t gpu_id)
        }
        printf("GPU memory 0x%p freed\n", ptr_1);

app/test-gpudev/main.c alloc_gpu_memory(uint16_t gpu_id) · line 122

Quoted 10 lines
app/test-gpudev/main.c +4 -2
 
-       printf("\n=======> TEST: PASSED\n");
+       printf("\nTEST: PASSED\n");
        return 0;
 
 error:
-       printf("\n=======> TEST: FAILED\n");
+
+       if (ptr_1 != NULL)
+               rte_gpu_mem_free(gpu_id, ptr_1);

rte_gpu_mem_free() should handle NULL pointer.
Maybe something to change in lib/gpudev itself.

Quoted 20 lines
app/test-gpudev/main.c +6 -1
+
+       if (ptr_2 != NULL)
+               rte_gpu_mem_free(gpu_id, ptr_2);
+
+       printf("\nTEST: FAILED\n");
        return -1;
 }
 

@@ -128,7 +135,7 @@ register_cpu_memory(uint16_t gpu_id)
        size_t buf_bytes = 1024;
        int ret;
 
-       printf("\n=======> TEST: Register CPU memory\n\n");
+       printf("\nTEST: Register CPU memory\n\n");
 
        /* Alloc memory on CPU visible from GPU 0 */
        ptr = rte_zmalloc(NULL, buf_bytes, 0);

@@ -161,11 +168,15 @@ register_cpu_memory(uint16_t gpu_id)
        }
        printf("CPU memory 0x%p unregistered\n", ptr);

app/test-gpudev/main.c register_cpu_memory(uint16_t gpu_id) · line 177

Quoted 10 lines
app/test-gpudev/main.c +4 -2
 
-       printf("\n=======> TEST: PASSED\n");
+       printf("\nTEST: PASSED\n");
        return 0;
 
 error:
-       printf("\n=======> TEST: FAILED\n");
+
+       if (ptr != NULL)
+               rte_gpu_mem_unregister(gpu_id, ptr);

I'd expect a rte_free(ptr); here.

Quoted 46 lines
app/test-gpudev/main.c +9 -5
+
+       printf("\nTEST: FAILED\n");
        return -1;
 }
 

@@ -177,7 +188,7 @@ create_update_comm_flag(uint16_t gpu_id)
        uint32_t set_val;
        uint32_t get_val;
 
-       printf("\n=======> TEST: Communication flag\n\n");
+       printf("\nTEST: Communication flag\n\n");
 
        ret = rte_gpu_comm_create_flag(gpu_id, &devflag, RTE_GPU_COMM_FLAG_CPU);
        if (ret < 0) {

@@ -223,11 +234,13 @@ create_update_comm_flag(uint16_t gpu_id)
                goto error;
        }
 
-       printf("\n=======> TEST: PASSED\n");
+       printf("\nTEST: PASSED\n");
        return 0;
 
 error:
-       printf("\n=======> TEST: FAILED\n");
+
+       rte_gpu_comm_destroy_flag(&devflag);
+       printf("\nTEST: FAILED\n");
        return -1;
 }
 

@@ -254,11 +267,11 @@ create_update_comm_list(uint16_t gpu_id)
 {
        int ret = 0;
        int i = 0;
-       struct rte_gpu_comm_list *comm_list;
+       struct rte_gpu_comm_list *comm_list = NULL;
        uint32_t num_comm_items = 1024;
        struct rte_mbuf *mbufs[10];
 
-       printf("\n=======> TEST: Communication list\n\n");
+       printf("\nTEST: Communication list\n\n");
 
        comm_list = rte_gpu_comm_create_list(gpu_id, num_comm_items);
        if (comm_list == NULL) {

@@ -323,11 +336,13 @@ create_update_comm_list(uint16_t gpu_id)
        for (i = 0; i < 10; i++)

app/test-gpudev/main.c create_update_comm_list(uint16_t gpu_id) · line 344

Quoted 10 lines
app/test-gpudev/main.c +3 -2
                rte_free(mbufs[i]);
 
-       printf("\n=======> TEST: PASSED\n");
+       printf("\nTEST: PASSED\n");
        return 0;
 
 error:
-       printf("\n=======> TEST: FAILED\n");
+
+       rte_gpu_comm_destroy_list(comm_list, num_comm_items);

What about "mbufs" objects?

Quoted 7 lines
app/test-gpudev/main.c +1 -1
+       printf("\nTEST: FAILED\n");
        return -1;
 }
 
--

2.17.1

Signature · 1 line
David Marchand
author
#2 lore raw

PATCH v2 1/1 app/test-gpudev: remove memory leaks in case of errors

In reply to [email protected]

From: Elena Agostini <[email protected]>

Signed-off-by Elena Agostini <[email protected]>
---
 app/test-gpudev/main.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)
app/test-gpudev/main.c +12 -1
index c647e30de4..250fba6427 100644
@@ -117,6 +117,10 @@ alloc_gpu_memory(uint16_t gpu_id)
 	return 0;
 
 error:
+
+	rte_gpu_mem_free(gpu_id, ptr_1);
+	rte_gpu_mem_free(gpu_id, ptr_2);
+
 	printf("\n=======> TEST: FAILED\n");
 	return -1;
 }

@@ -165,6 +169,9 @@ register_cpu_memory(uint16_t gpu_id)
 	return 0;
 
 error:
+
+	rte_gpu_mem_unregister(gpu_id, ptr);
+	rte_free(ptr);
 	printf("\n=======> TEST: FAILED\n");
 	return -1;
 }

@@ -227,6 +234,8 @@ create_update_comm_flag(uint16_t gpu_id)
 	return 0;
 
 error:
+
+	rte_gpu_comm_destroy_flag(&devflag);
 	printf("\n=======> TEST: FAILED\n");
 	return -1;
 }

@@ -254,7 +263,7 @@ create_update_comm_list(uint16_t gpu_id)
 {
 	int ret = 0;
 	int i = 0;
-	struct rte_gpu_comm_list *comm_list;
+	struct rte_gpu_comm_list *comm_list = NULL;
 	uint32_t num_comm_items = 1024;
 	struct rte_mbuf *mbufs[10];
 

@@ -327,6 +336,8 @@ create_update_comm_list(uint16_t gpu_id)
 	return 0;
 
 error:
+
+	rte_gpu_comm_destroy_list(comm_list, num_comm_items);
 	printf("\n=======> TEST: FAILED\n");
 	return -1;
 }
Signature · 1 line
2.17.1
author
#3 lore raw

PATCH v3 1/1 app/test-gpudev: remove all memory leaks

In reply to [email protected]

From: Elena Agostini <[email protected]>

Remove all memory leaks in case of errors in
test-gpudev application.

Fixes e818c4e2bf50 ("gpudev: add memory API")
Fixes c7ebd65c1372 ("gpudev: add communication list")
Signed-off-by Elena Agostini <[email protected]>
---
 app/test-gpudev/main.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
app/test-gpudev/main.c +16 -1
index c647e30de4..5c1aa3d52f 100644
@@ -117,6 +117,10 @@ alloc_gpu_memory(uint16_t gpu_id)
 	return 0;
 
 error:
+
+	rte_gpu_mem_free(gpu_id, ptr_1);
+	rte_gpu_mem_free(gpu_id, ptr_2);
+
 	printf("\n=======> TEST: FAILED\n");
 	return -1;
 }

@@ -161,10 +165,15 @@ register_cpu_memory(uint16_t gpu_id)
 	}
 	printf("CPU memory 0x%p unregistered\n", ptr);
 
+	rte_free(ptr);
+
 	printf("\n=======> TEST: PASSED\n");
 	return 0;
 
 error:
+
+	rte_gpu_mem_unregister(gpu_id, ptr);
+	rte_free(ptr);
 	printf("\n=======> TEST: FAILED\n");
 	return -1;
 }

@@ -227,6 +236,8 @@ create_update_comm_flag(uint16_t gpu_id)
 	return 0;
 
 error:
+
+	rte_gpu_comm_destroy_flag(&devflag);
 	printf("\n=======> TEST: FAILED\n");
 	return -1;
 }

@@ -254,7 +265,7 @@ create_update_comm_list(uint16_t gpu_id)
 {
 	int ret = 0;
 	int i = 0;
-	struct rte_gpu_comm_list *comm_list;
+	struct rte_gpu_comm_list *comm_list = NULL;
 	uint32_t num_comm_items = 1024;
 	struct rte_mbuf *mbufs[10];
 

@@ -327,6 +338,10 @@ create_update_comm_list(uint16_t gpu_id)
 	return 0;
 
 error:
+
+	rte_gpu_comm_destroy_list(comm_list, num_comm_items);
+	for (i = 0; i < 10; i++)
+		rte_free(mbufs[i]);
 	printf("\n=======> TEST: FAILED\n");
 	return -1;
 }
Signature · 1 line
2.17.1