DPDK patches and discussions
 help / color / mirror / Atom feed
* Re: [PATCH v1] app/test-gpudev: remove memory leaks in case of errors
  2021-11-17 20:44 [PATCH v1] app/test-gpudev: remove memory leaks in case of errors eagostini
@ 2021-11-17 16:33 ` David Marchand
  2021-11-18  1:23 ` [PATCH v2] " eagostini
  2021-11-25 23:03 ` [PATCH v3] app/test-gpudev: remove all memory leaks eagostini
  2 siblings, 0 replies; 5+ messages in thread
From: David Marchand @ 2021-11-17 16:33 UTC (permalink / raw)
  To: Elena Agostini; +Cc: dev, Thomas Monjalon

On Wed, Nov 17, 2021 at 1:34 PM <eagostini@nvidia.com> wrote:

Commitlog.

+ Fixes: tag.

>
> From: Elena Agostini <eagostini@nvidia.com>
>
> Signed-off-by: Elena Agostini <eagostini@nvidia.com>
> ---
>  app/test-gpudev/main.c | 41 ++++++++++++++++++++++++++++-------------
>  1 file changed, 28 insertions(+), 13 deletions(-)
>
> diff --git a/app/test-gpudev/main.c b/app/test-gpudev/main.c
> index c647e30de4..fe5c1e5b15 100644
> --- a/app/test-gpudev/main.c
> +++ b/app/test-gpudev/main.c
> @@ -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.


>
>         /* 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);

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


> +
> +       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);

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


> +
> +       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);

What about "mbufs" objects?


> +       printf("\nTEST: FAILED\n");
>         return -1;
>  }
>
> --
> 2.17.1
>



-- 
David Marchand


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

* [PATCH v1] app/test-gpudev: remove memory leaks in case of errors
@ 2021-11-17 20:44 eagostini
  2021-11-17 16:33 ` David Marchand
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: eagostini @ 2021-11-17 20:44 UTC (permalink / raw)
  To: dev; +Cc: Elena Agostini

From: Elena Agostini <eagostini@nvidia.com>

Signed-off-by: Elena Agostini <eagostini@nvidia.com>
---
 app/test-gpudev/main.c | 41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/app/test-gpudev/main.c b/app/test-gpudev/main.c
index c647e30de4..fe5c1e5b15 100644
--- a/app/test-gpudev/main.c
+++ b/app/test-gpudev/main.c
@@ -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;
 }
 
-- 
2.17.1


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

* [PATCH v2] app/test-gpudev: remove memory leaks in case of errors
  2021-11-17 20:44 [PATCH v1] app/test-gpudev: remove memory leaks in case of errors eagostini
  2021-11-17 16:33 ` David Marchand
@ 2021-11-18  1:23 ` eagostini
  2021-11-25 23:03 ` [PATCH v3] app/test-gpudev: remove all memory leaks eagostini
  2 siblings, 0 replies; 5+ messages in thread
From: eagostini @ 2021-11-18  1:23 UTC (permalink / raw)
  To: dev; +Cc: Elena Agostini

From: Elena Agostini <eagostini@nvidia.com>

Signed-off-by: Elena Agostini <eagostini@nvidia.com>
---
 app/test-gpudev/main.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/app/test-gpudev/main.c b/app/test-gpudev/main.c
index c647e30de4..250fba6427 100644
--- a/app/test-gpudev/main.c
+++ b/app/test-gpudev/main.c
@@ -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;
 }
-- 
2.17.1


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

* [PATCH v3] app/test-gpudev: remove all memory leaks
  2021-11-17 20:44 [PATCH v1] app/test-gpudev: remove memory leaks in case of errors eagostini
  2021-11-17 16:33 ` David Marchand
  2021-11-18  1:23 ` [PATCH v2] " eagostini
@ 2021-11-25 23:03 ` eagostini
  2021-11-26 12:17   ` Thomas Monjalon
  2 siblings, 1 reply; 5+ messages in thread
From: eagostini @ 2021-11-25 23:03 UTC (permalink / raw)
  To: dev; +Cc: Elena Agostini

From: Elena Agostini <eagostini@nvidia.com>

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 <eagostini@nvidia.com>
---
 app/test-gpudev/main.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/app/test-gpudev/main.c b/app/test-gpudev/main.c
index c647e30de4..5c1aa3d52f 100644
--- a/app/test-gpudev/main.c
+++ b/app/test-gpudev/main.c
@@ -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;
 }
-- 
2.17.1


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

* Re: [PATCH v3] app/test-gpudev: remove all memory leaks
  2021-11-25 23:03 ` [PATCH v3] app/test-gpudev: remove all memory leaks eagostini
@ 2021-11-26 12:17   ` Thomas Monjalon
  0 siblings, 0 replies; 5+ messages in thread
From: Thomas Monjalon @ 2021-11-26 12:17 UTC (permalink / raw)
  To: Elena Agostini; +Cc: dev

26/11/2021 00:03, eagostini@nvidia.com:
> From: Elena Agostini <eagostini@nvidia.com>
> 
> 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 <eagostini@nvidia.com>

Applied, thanks.




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

end of thread, other threads:[~2021-11-26 12:17 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-17 20:44 [PATCH v1] app/test-gpudev: remove memory leaks in case of errors eagostini
2021-11-17 16:33 ` David Marchand
2021-11-18  1:23 ` [PATCH v2] " eagostini
2021-11-25 23:03 ` [PATCH v3] app/test-gpudev: remove all memory leaks eagostini
2021-11-26 12:17   ` Thomas Monjalon

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git