DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] test_distributor.c: prevent memory leakages from the pool
@ 2020-04-13  9:19 ` Sarosh Arif
  2020-04-15  1:15   ` Lukasz Wojciechowski
  2020-04-15  6:42   ` [dpdk-dev] [PATCH v2] app/test/test_distributor.c: " Sarosh Arif
  0 siblings, 2 replies; 16+ messages in thread
From: Sarosh Arif @ 2020-04-13  9:19 UTC (permalink / raw)
  To: david.hunt; +Cc: dev, Sarosh Arif

rte_mempool_get_bulk is used to get bufs/many_bufs from the pool,
but at some locations when test passes/fails the bufs/many_bufs are
not returned back to the pool.
Due to this, multiple executions of distributor_autotest gives the
following error message: Error getting mbufs from pool.
To resolve this issue rte_mempool_put_bulk is used whenever the test
passes/fails and returns.   

Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
---
 app/test/test_distributor.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
index ba1f81cf8..8608b4ce8 100644
--- a/app/test/test_distributor.c
+++ b/app/test/test_distributor.c
@@ -128,6 +128,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 		printf("Line %d: Error, not all packets flushed. "
 				"Expected %u, got %u\n",
 				__LINE__, BURST, total_packet_count());
+		rte_mempool_put_bulk(p, (void *)bufs, BURST);
 		return -1;
 	}
 
@@ -153,6 +154,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 			printf("Line %d: Error, not all packets flushed. "
 					"Expected %u, got %u\n",
 					__LINE__, BURST, total_packet_count());
+			rte_mempool_put_bulk(p, (void *)bufs, BURST);
 			return -1;
 		}
 
@@ -179,6 +181,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 		printf("Line %d: Error, not all packets flushed. "
 				"Expected %u, got %u\n",
 				__LINE__, BURST, total_packet_count());
+		rte_mempool_put_bulk(p, (void *)bufs, BURST);
 		return -1;
 	}
 
@@ -233,6 +236,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 	if (num_returned != BIG_BATCH) {
 		printf("line %d: Missing packets, expected %d\n",
 				__LINE__, num_returned);
+		rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
 		return -1;
 	}
 
@@ -247,6 +251,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 
 		if (j == BIG_BATCH) {
 			printf("Error: could not find source packet #%u\n", i);
+			rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
 			return -1;
 		}
 	}
@@ -327,10 +332,12 @@ sanity_test_with_mbuf_alloc(struct worker_params *wp, struct rte_mempool *p)
 		printf("Line %u: Packet count is incorrect, %u, expected %u\n",
 				__LINE__, total_packet_count(),
 				(1<<ITER_POWER));
+		rte_mempool_put_bulk(p, (void *)bufs, BURST);
 		return -1;
 	}
 
 	printf("Sanity test with mbuf alloc/free passed\n\n");
+	rte_mempool_put_bulk(p, (void *)bufs, BURST);
 	return 0;
 }
 
@@ -450,10 +457,14 @@ sanity_test_with_worker_shutdown(struct worker_params *wp,
 		printf("Line %d: Error, not all packets flushed. "
 				"Expected %u, got %u\n",
 				__LINE__, BURST * 2, total_packet_count());
+		for (i = 0; i < 2; i++)
+			rte_mempool_put_bulk(p, (void *)bufs, BURST);
 		return -1;
 	}
 
 	printf("Sanity test with worker shutdown passed\n\n");
+	for (i = 0; i < 2; i++)
+		rte_mempool_put_bulk(p, (void *)bufs, BURST);
 	return 0;
 }
 
@@ -503,10 +514,12 @@ test_flush_with_worker_shutdown(struct worker_params *wp,
 		printf("Line %d: Error, not all packets flushed. "
 				"Expected %u, got %u\n",
 				__LINE__, BURST, total_packet_count());
+		rte_mempool_put_bulk(p, (void *)bufs, BURST);
 		return -1;
 	}
 
 	printf("Flush test with worker shutdown passed\n\n");
+	rte_mempool_put_bulk(p, (void *)bufs, BURST);
 	return 0;
 }
 
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH] test_distributor.c: prevent memory leakages from the pool
  2020-04-13  9:19 ` [dpdk-dev] [PATCH] test_distributor.c: prevent memory leakages from the pool Sarosh Arif
@ 2020-04-15  1:15   ` Lukasz Wojciechowski
  2020-04-15  6:42   ` [dpdk-dev] [PATCH v2] app/test/test_distributor.c: " Sarosh Arif
  1 sibling, 0 replies; 16+ messages in thread
From: Lukasz Wojciechowski @ 2020-04-15  1:15 UTC (permalink / raw)
  To: Sarosh Arif, david.hunt; +Cc: dev

Hi, Sarosh

I believe commit message title should begin with app/test not the file name

Other comments inline

W dniu 13.04.2020 o 11:19, Sarosh Arif pisze:
> rte_mempool_get_bulk is used to get bufs/many_bufs from the pool,
> but at some locations when test passes/fails the bufs/many_bufs are
> not returned back to the pool.
> Due to this, multiple executions of distributor_autotest gives the
> following error message: Error getting mbufs from pool.
> To resolve this issue rte_mempool_put_bulk is used whenever the test
> passes/fails and returns.
>
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> ---
>   app/test/test_distributor.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
>
> diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
> index ba1f81cf8..8608b4ce8 100644
> --- a/app/test/test_distributor.c
> +++ b/app/test/test_distributor.c
> @@ -128,6 +128,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   		printf("Line %d: Error, not all packets flushed. "
>   				"Expected %u, got %u\n",
>   				__LINE__, BURST, total_packet_count());
> +		rte_mempool_put_bulk(p, (void *)bufs, BURST);
>   		return -1;
>   	}
>   
> @@ -153,6 +154,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   			printf("Line %d: Error, not all packets flushed. "
>   					"Expected %u, got %u\n",
>   					__LINE__, BURST, total_packet_count());
> +			rte_mempool_put_bulk(p, (void *)bufs, BURST);
>   			return -1;
>   		}
>   
> @@ -179,6 +181,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   		printf("Line %d: Error, not all packets flushed. "
>   				"Expected %u, got %u\n",
>   				__LINE__, BURST, total_packet_count());
> +		rte_mempool_put_bulk(p, (void *)bufs, BURST);
>   		return -1;
>   	}
>   
> @@ -233,6 +236,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   	if (num_returned != BIG_BATCH) {
>   		printf("line %d: Missing packets, expected %d\n",
>   				__LINE__, num_returned);
> +		rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
>   		return -1;
>   	}
>   
> @@ -247,6 +251,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   
>   		if (j == BIG_BATCH) {
>   			printf("Error: could not find source packet #%u\n", i);
> +			rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
>   			return -1;
>   		}
>   	}
> @@ -327,10 +332,12 @@ sanity_test_with_mbuf_alloc(struct worker_params *wp, struct rte_mempool *p)
>   		printf("Line %u: Packet count is incorrect, %u, expected %u\n",
>   				__LINE__, total_packet_count(),
>   				(1<<ITER_POWER));
> +		rte_mempool_put_bulk(p, (void *)bufs, BURST);
This modification can cause double free of mbufs, as they are freed 
already with rte_pktmbuf_free in handle_work_with_free_mbufs (line 290).
Even if you assume that this condition will be run when not all of the 
packets were processed by worker, you don't know which were and which 
were not processed.
Please remove it from your patch.
>   		return -1;
>   	}
>   
>   	printf("Sanity test with mbuf alloc/free passed\n\n");
> +	rte_mempool_put_bulk(p, (void *)bufs, BURST);

This modification causes double free of mbufs, as they are freed already 
with rte_pktmbuf_free in handle_work_with_free_mbufs (line 290).
Please remove it from your patch.

>   	return 0;
>   }
>   

> @@ -450,10 +457,14 @@ sanity_test_with_worker_shutdown(struct worker_params *wp,
>   		printf("Line %d: Error, not all packets flushed. "
>   				"Expected %u, got %u\n",
>   				__LINE__, BURST * 2, total_packet_count());
> +		for (i = 0; i < 2; i++)
> +			rte_mempool_put_bulk(p, (void *)bufs, BURST);
This modification can cause double free of mbufs, as they are freed 
already with rte_pktmbuf_free in handle_work_for_shutdown_test (line 367).
Please remove it from your patch.
>   		return -1;
>   	}
>   
>   	printf("Sanity test with worker shutdown passed\n\n");
> +	for (i = 0; i < 2; i++)
> +		rte_mempool_put_bulk(p, (void *)bufs, BURST);
This modification causes double free of mbufs, as they are freed already 
with rte_pktmbuf_free in handle_work_for_shutdown_test (line 367).
Please remove it from your patch.
>   	return 0;
>   }
>   
> @@ -503,10 +514,12 @@ test_flush_with_worker_shutdown(struct worker_params *wp,
>   		printf("Line %d: Error, not all packets flushed. "
>   				"Expected %u, got %u\n",
>   				__LINE__, BURST, total_packet_count());
> +		rte_mempool_put_bulk(p, (void *)bufs, BURST);
This modification can cause double free of mbufs, as they are freed 
already with rte_pktmbuf_free in handle_work_for_shutdown_test (line 367).
Please remove it from your patch.
>   		return -1;
>   	}
>   
>   	printf("Flush test with worker shutdown passed\n\n");
> +	rte_mempool_put_bulk(p, (void *)bufs, BURST);
This modification causes double free of mbufs, as they are freed already 
with rte_pktmbuf_free in handle_work_for_shutdown_test (line 367).
Please remove it from your patch.
>   	return 0;
>   }
>   
Generally there are 3 handlers for workers:
* handle_work
* handle_work_with_free_mbufs
* handle_work_for_shutdown_test

The first one doesn't do anything with mbufs and sens it back to the 
test, where they should be released.
In the other 2 handlers mbufs are released, so they shouldn't be put 
back to mempool in test function.

Keeping this simple rules will be the asy way to clean the code and 
manage pool object properly.

Please consider also, that the there are still few places, where this 
rules are broken and there is one more tricky place: quit_workers 
function, which is called in all tests.

You can easily test proper usage of mempool by defining 
RTE_LIBRTE_MEMPOOL_DEBUG to 1


-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com


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

* [dpdk-dev] [PATCH v2] app/test/test_distributor.c: prevent memory leakages from the pool
  2020-04-13  9:19 ` [dpdk-dev] [PATCH] test_distributor.c: prevent memory leakages from the pool Sarosh Arif
  2020-04-15  1:15   ` Lukasz Wojciechowski
@ 2020-04-15  6:42   ` Sarosh Arif
  2020-04-15  6:52     ` Lukasz Wojciechowski
  2020-09-08 10:22     ` [dpdk-dev] [v3 PATCH] test_distributor: " Sarosh Arif
  1 sibling, 2 replies; 16+ messages in thread
From: Sarosh Arif @ 2020-04-15  6:42 UTC (permalink / raw)
  To: dev; +Cc: Sarosh Arif

v2:
remove double freeing of mbufs

Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
---
 app/test/test_distributor.c | 9 +++++++
 1 file changed, 7 insertions(+)

diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
index ba1f81cf8..5e972bb2e 100644
--- a/app/test/test_distributor.c
+++ b/app/test/test_distributor.c
@@ -128,6 +128,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 		printf("Line %d: Error, not all packets flushed. "
 				"Expected %u, got %u\n",
 				__LINE__, BURST, total_packet_count());
+		rte_mempool_put_bulk(p, (void *)bufs, BURST);
 		return -1;
 	}
 
@@ -153,6 +154,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 			printf("Line %d: Error, not all packets flushed. "
 					"Expected %u, got %u\n",
 					__LINE__, BURST, total_packet_count());
+			rte_mempool_put_bulk(p, (void *)bufs, BURST);
 			return -1;
 		}
 
@@ -179,6 +181,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 		printf("Line %d: Error, not all packets flushed. "
 				"Expected %u, got %u\n",
 				__LINE__, BURST, total_packet_count());
+		rte_mempool_put_bulk(p, (void *)bufs, BURST);
 		return -1;
 	}
 
@@ -233,6 +236,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 	if (num_returned != BIG_BATCH) {
 		printf("line %d: Missing packets, expected %d\n",
 				__LINE__, num_returned);
+		rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
 		return -1;
 	}
 
@@ -247,6 +251,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 
 		if (j == BIG_BATCH) {
 			printf("Error: could not find source packet #%u\n", i);
+			rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
 			return -1;
 		}
 	}

2.17.1


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

* Re: [dpdk-dev] [PATCH v2] app/test/test_distributor.c: prevent memory leakages from the pool
  2020-04-15  6:42   ` [dpdk-dev] [PATCH v2] app/test/test_distributor.c: " Sarosh Arif
@ 2020-04-15  6:52     ` Lukasz Wojciechowski
  2020-04-15  7:06       ` Sarosh Arif
  2020-09-08 10:22     ` [dpdk-dev] [v3 PATCH] test_distributor: " Sarosh Arif
  1 sibling, 1 reply; 16+ messages in thread
From: Lukasz Wojciechowski @ 2020-04-15  6:52 UTC (permalink / raw)
  To: Sarosh Arif, dev


W dniu 15.04.2020 o 08:42, Sarosh Arif pisze:
> v2:
> remove double freeing of mbufs
>
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> ---
>   app/test/test_distributor.c | 9 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
> index ba1f81cf8..5e972bb2e 100644
> --- a/app/test/test_distributor.c
> +++ b/app/test/test_distributor.c
> @@ -128,6 +128,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   		printf("Line %d: Error, not all packets flushed. "
>   				"Expected %u, got %u\n",
>   				__LINE__, BURST, total_packet_count());
> +		rte_mempool_put_bulk(p, (void *)bufs, BURST);
>   		return -1;
>   	}
>   
> @@ -153,6 +154,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   			printf("Line %d: Error, not all packets flushed. "
>   					"Expected %u, got %u\n",
>   					__LINE__, BURST, total_packet_count());
> +			rte_mempool_put_bulk(p, (void *)bufs, BURST);
>   			return -1;
>   		}
>   
> @@ -179,6 +181,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   		printf("Line %d: Error, not all packets flushed. "
>   				"Expected %u, got %u\n",
>   				__LINE__, BURST, total_packet_count());
> +		rte_mempool_put_bulk(p, (void *)bufs, BURST);
>   		return -1;
>   	}
>   
> @@ -233,6 +236,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   	if (num_returned != BIG_BATCH) {
>   		printf("line %d: Missing packets, expected %d\n",
>   				__LINE__, num_returned);
> +		rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
>   		return -1;
>   	}
>   
> @@ -247,6 +251,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   
>   		if (j == BIG_BATCH) {
>   			printf("Error: could not find source packet #%u\n", i);
> +			rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
>   			return -1;
>   		}
>   	}
>
> 2.17.1
>
The sanity_test is ok now and does not have any mempool leaks.

What about other tests in this file: Do you plan to work on them also?

-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com


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

* Re: [dpdk-dev] [PATCH v2] app/test/test_distributor.c: prevent memory leakages from the pool
  2020-04-15  6:52     ` Lukasz Wojciechowski
@ 2020-04-15  7:06       ` Sarosh Arif
  2020-04-15  7:08         ` Lukasz Wojciechowski
  0 siblings, 1 reply; 16+ messages in thread
From: Sarosh Arif @ 2020-04-15  7:06 UTC (permalink / raw)
  To: Lukasz Wojciechowski; +Cc: dev

Yes, I plan to work on them when I get time.

On Wed, Apr 15, 2020 at 11:52 AM Lukasz Wojciechowski <
l.wojciechow@partner.samsung.com> wrote:

>
> W dniu 15.04.2020 o 08:42, Sarosh Arif pisze:
> > v2:
> > remove double freeing of mbufs
> >
> > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> > ---
> >   app/test/test_distributor.c | 9 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
> > index ba1f81cf8..5e972bb2e 100644
> > --- a/app/test/test_distributor.c
> > +++ b/app/test/test_distributor.c
> > @@ -128,6 +128,7 @@ sanity_test(struct worker_params *wp, struct
> rte_mempool *p)
> >               printf("Line %d: Error, not all packets flushed. "
> >                               "Expected %u, got %u\n",
> >                               __LINE__, BURST, total_packet_count());
> > +             rte_mempool_put_bulk(p, (void *)bufs, BURST);
> >               return -1;
> >       }
> >
> > @@ -153,6 +154,7 @@ sanity_test(struct worker_params *wp, struct
> rte_mempool *p)
> >                       printf("Line %d: Error, not all packets flushed. "
> >                                       "Expected %u, got %u\n",
> >                                       __LINE__, BURST,
> total_packet_count());
> > +                     rte_mempool_put_bulk(p, (void *)bufs, BURST);
> >                       return -1;
> >               }
> >
> > @@ -179,6 +181,7 @@ sanity_test(struct worker_params *wp, struct
> rte_mempool *p)
> >               printf("Line %d: Error, not all packets flushed. "
> >                               "Expected %u, got %u\n",
> >                               __LINE__, BURST, total_packet_count());
> > +             rte_mempool_put_bulk(p, (void *)bufs, BURST);
> >               return -1;
> >       }
> >
> > @@ -233,6 +236,7 @@ sanity_test(struct worker_params *wp, struct
> rte_mempool *p)
> >       if (num_returned != BIG_BATCH) {
> >               printf("line %d: Missing packets, expected %d\n",
> >                               __LINE__, num_returned);
> > +             rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
> >               return -1;
> >       }
> >
> > @@ -247,6 +251,7 @@ sanity_test(struct worker_params *wp, struct
> rte_mempool *p)
> >
> >               if (j == BIG_BATCH) {
> >                       printf("Error: could not find source packet
> #%u\n", i);
> > +                     rte_mempool_put_bulk(p, (void *)many_bufs,
> BIG_BATCH);
> >                       return -1;
> >               }
> >       }
> >
> > 2.17.1
> >
> The sanity_test is ok now and does not have any mempool leaks.
>
> What about other tests in this file: Do you plan to work on them also?
>
> --
>
> Lukasz Wojciechowski
> Principal Software Engineer
>
> Samsung R&D Institute Poland
> Samsung Electronics
> Office +48 22 377 88 25
> l.wojciechow@partner.samsung.com
>
>

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

* Re: [dpdk-dev] [PATCH v2] app/test/test_distributor.c: prevent memory leakages from the pool
  2020-04-15  7:06       ` Sarosh Arif
@ 2020-04-15  7:08         ` Lukasz Wojciechowski
       [not found]           ` <CABoZmYPV-GXESgjey7-g-j=cNif38O-SZ4L7TfrDTwb6KfDsog@mail.gmail.com>
  0 siblings, 1 reply; 16+ messages in thread
From: Lukasz Wojciechowski @ 2020-04-15  7:08 UTC (permalink / raw)
  To: Sarosh Arif; +Cc: dev


W dniu 15.04.2020 o 09:06, Sarosh Arif pisze:
> Yes, I plan to work on them when I get time.
Great, please add me to CC. I would be glad to review it.
>
> On Wed, Apr 15, 2020 at 11:52 AM Lukasz Wojciechowski 
> <l.wojciechow@partner.samsung.com 
> <mailto:l.wojciechow@partner.samsung.com>> wrote:
>
>
>     W dniu 15.04.2020 o 08:42, Sarosh Arif pisze:
>     > v2:
>     > remove double freeing of mbufs
>     >
>     > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com
>     <mailto:sarosh.arif@emumba.com>>
>     > ---
>     >   app/test/test_distributor.c | 9 +++++++
>     >   1 file changed, 7 insertions(+)
>     >
>     > diff --git a/app/test/test_distributor.c
>     b/app/test/test_distributor.c
>     > index ba1f81cf8..5e972bb2e 100644
>     > --- a/app/test/test_distributor.c
>     > +++ b/app/test/test_distributor.c
>     > @@ -128,6 +128,7 @@ sanity_test(struct worker_params *wp, struct
>     rte_mempool *p)
>     >               printf("Line %d: Error, not all packets flushed. "
>     >                               "Expected %u, got %u\n",
>     >                               __LINE__, BURST,
>     total_packet_count());
>     > +             rte_mempool_put_bulk(p, (void *)bufs, BURST);
>     >               return -1;
>     >       }
>     >
>     > @@ -153,6 +154,7 @@ sanity_test(struct worker_params *wp, struct
>     rte_mempool *p)
>     >                       printf("Line %d: Error, not all packets
>     flushed. "
>     >                                       "Expected %u, got %u\n",
>     >                                       __LINE__, BURST,
>     total_packet_count());
>     > +                     rte_mempool_put_bulk(p, (void *)bufs, BURST);
>     >                       return -1;
>     >               }
>     >
>     > @@ -179,6 +181,7 @@ sanity_test(struct worker_params *wp, struct
>     rte_mempool *p)
>     >               printf("Line %d: Error, not all packets flushed. "
>     >                               "Expected %u, got %u\n",
>     >                               __LINE__, BURST,
>     total_packet_count());
>     > +             rte_mempool_put_bulk(p, (void *)bufs, BURST);
>     >               return -1;
>     >       }
>     >
>     > @@ -233,6 +236,7 @@ sanity_test(struct worker_params *wp, struct
>     rte_mempool *p)
>     >       if (num_returned != BIG_BATCH) {
>     >               printf("line %d: Missing packets, expected %d\n",
>     >                               __LINE__, num_returned);
>     > +             rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
>     >               return -1;
>     >       }
>     >
>     > @@ -247,6 +251,7 @@ sanity_test(struct worker_params *wp, struct
>     rte_mempool *p)
>     >
>     >               if (j == BIG_BATCH) {
>     >                       printf("Error: could not find source
>     packet #%u\n", i);
>     > +                     rte_mempool_put_bulk(p, (void *)many_bufs,
>     BIG_BATCH);
>     >                       return -1;
>     >               }
>     >       }
>     >
>     > 2.17.1
>     >
>     The sanity_test is ok now and does not have any mempool leaks.
>
>     What about other tests in this file: Do you plan to work on them also?
>
>     -- 
>
>     Lukasz Wojciechowski
>     Principal Software Engineer
>
>     Samsung R&D Institute Poland
>     Samsung Electronics
>     Office +48 22 377 88 25
>     l.wojciechow@partner.samsung.com
>     <mailto:l.wojciechow@partner.samsung.com>
>
-- 

Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com


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

* Re: [dpdk-dev] [PATCH v2] app/test/test_distributor.c: prevent memory leakages from the pool
       [not found]             ` <CABoZmYOhpYqV07Q4JguvSwJmAPzfhO-f8fMKUzsMoUdUGU5hjQ@mail.gmail.com>
@ 2020-06-24 10:02               ` Sarosh Arif
  2020-09-01 18:31                 ` Lukasz Wojciechowski
  0 siblings, 1 reply; 16+ messages in thread
From: Sarosh Arif @ 2020-06-24 10:02 UTC (permalink / raw)
  To: Lukasz Wojciechowski, david.hunt; +Cc: dev

Some tests are failing on this patch but I don't think the reason
behind the failure is this patch. Is there a certain way to know that
the problem is in the patch or somewhere else?


On Wed, Jun 24, 2020 at 2:04 PM Sarosh Arif <sarosh.arif@emumba.com> wrote:
>
> Some tests are failing on this patch but I don't think the reason behind the failure is this patch. Is there a certain way to know that the problem is in the patch or somewhere else?
>
> On Wed, Apr 15, 2020 at 12:20 PM Sarosh Arif <sarosh.arif@emumba.com> wrote:
>>
>> Sure, will do that.
>>
>> On Wed, Apr 15, 2020 at 12:08 PM Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> wrote:
>>>
>>>
>>> W dniu 15.04.2020 o 09:06, Sarosh Arif pisze:
>>>
>>> Yes, I plan to work on them when I get time.
>>>
>>> Great, please add me to CC. I would be glad to review it.
>>>
>>>
>>> On Wed, Apr 15, 2020 at 11:52 AM Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> wrote:
>>>>
>>>>
>>>> W dniu 15.04.2020 o 08:42, Sarosh Arif pisze:
>>>> > v2:
>>>> > remove double freeing of mbufs
>>>> >
>>>> > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
>>>> > ---
>>>> >   app/test/test_distributor.c | 9 +++++++
>>>> >   1 file changed, 7 insertions(+)
>>>> >
>>>> > diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
>>>> > index ba1f81cf8..5e972bb2e 100644
>>>> > --- a/app/test/test_distributor.c
>>>> > +++ b/app/test/test_distributor.c
>>>> > @@ -128,6 +128,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>>>> >               printf("Line %d: Error, not all packets flushed. "
>>>> >                               "Expected %u, got %u\n",
>>>> >                               __LINE__, BURST, total_packet_count());
>>>> > +             rte_mempool_put_bulk(p, (void *)bufs, BURST);
>>>> >               return -1;
>>>> >       }
>>>> >
>>>> > @@ -153,6 +154,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>>>> >                       printf("Line %d: Error, not all packets flushed. "
>>>> >                                       "Expected %u, got %u\n",
>>>> >                                       __LINE__, BURST, total_packet_count());
>>>> > +                     rte_mempool_put_bulk(p, (void *)bufs, BURST);
>>>> >                       return -1;
>>>> >               }
>>>> >
>>>> > @@ -179,6 +181,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>>>> >               printf("Line %d: Error, not all packets flushed. "
>>>> >                               "Expected %u, got %u\n",
>>>> >                               __LINE__, BURST, total_packet_count());
>>>> > +             rte_mempool_put_bulk(p, (void *)bufs, BURST);
>>>> >               return -1;
>>>> >       }
>>>> >
>>>> > @@ -233,6 +236,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>>>> >       if (num_returned != BIG_BATCH) {
>>>> >               printf("line %d: Missing packets, expected %d\n",
>>>> >                               __LINE__, num_returned);
>>>> > +             rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
>>>> >               return -1;
>>>> >       }
>>>> >
>>>> > @@ -247,6 +251,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>>>> >
>>>> >               if (j == BIG_BATCH) {
>>>> >                       printf("Error: could not find source packet #%u\n", i);
>>>> > +                     rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
>>>> >                       return -1;
>>>> >               }
>>>> >       }
>>>> >
>>>> > 2.17.1
>>>> >
>>>> The sanity_test is ok now and does not have any mempool leaks.
>>>>
>>>> What about other tests in this file: Do you plan to work on them also?
>>>>
>>>> --
>>>>
>>>> Lukasz Wojciechowski
>>>> Principal Software Engineer
>>>>
>>>> Samsung R&D Institute Poland
>>>> Samsung Electronics
>>>> Office +48 22 377 88 25
>>>> l.wojciechow@partner.samsung.com
>>>>
>>> --
>>>
>>> Lukasz Wojciechowski
>>> Principal Software Engineer
>>>
>>> Samsung R&D Institute Poland
>>> Samsung Electronics
>>> Office +48 22 377 88 25
>>> l.wojciechow@partner.samsung.com
>>>
>>>
>>>
>>>

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

* Re: [dpdk-dev] [PATCH v2] app/test/test_distributor.c: prevent memory leakages from the pool
  2020-06-24 10:02               ` Sarosh Arif
@ 2020-09-01 18:31                 ` Lukasz Wojciechowski
  0 siblings, 0 replies; 16+ messages in thread
From: Lukasz Wojciechowski @ 2020-09-01 18:31 UTC (permalink / raw)
  To: Sarosh Arif, david.hunt; +Cc: dev

Sarosh,

I cherry picked your patch on current main branch and it works and 
builds without problems,

maybe try resubmitting it as v3.

Best regards

Lukasz

W dniu 24.06.2020 o 12:02, Sarosh Arif pisze:
> Some tests are failing on this patch but I don't think the reason
> behind the failure is this patch. Is there a certain way to know that
> the problem is in the patch or somewhere else?
>
>
> On Wed, Jun 24, 2020 at 2:04 PM Sarosh Arif <sarosh.arif@emumba.com> wrote:
>> Some tests are failing on this patch but I don't think the reason behind the failure is this patch. Is there a certain way to know that the problem is in the patch or somewhere else?
>>
>> On Wed, Apr 15, 2020 at 12:20 PM Sarosh Arif <sarosh.arif@emumba.com> wrote:
>>> Sure, will do that.
>>>
>>> On Wed, Apr 15, 2020 at 12:08 PM Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> wrote:
>>>>
>>>> W dniu 15.04.2020 o 09:06, Sarosh Arif pisze:
>>>>
>>>> Yes, I plan to work on them when I get time.
>>>>
>>>> Great, please add me to CC. I would be glad to review it.
>>>>
>>>>
>>>> On Wed, Apr 15, 2020 at 11:52 AM Lukasz Wojciechowski <l.wojciechow@partner.samsung.com> wrote:
>>>>>
>>>>> W dniu 15.04.2020 o 08:42, Sarosh Arif pisze:
>>>>>> v2:
>>>>>> remove double freeing of mbufs
>>>>>>
>>>>>> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
>>>>>> ---
>>>>>>    app/test/test_distributor.c | 9 +++++++
>>>>>>    1 file changed, 7 insertions(+)
>>>>>>
>>>>>> diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
>>>>>> index ba1f81cf8..5e972bb2e 100644
>>>>>> --- a/app/test/test_distributor.c
>>>>>> +++ b/app/test/test_distributor.c
>>>>>> @@ -128,6 +128,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>>>>>>                printf("Line %d: Error, not all packets flushed. "
>>>>>>                                "Expected %u, got %u\n",
>>>>>>                                __LINE__, BURST, total_packet_count());
>>>>>> +             rte_mempool_put_bulk(p, (void *)bufs, BURST);
>>>>>>                return -1;
>>>>>>        }
>>>>>>
>>>>>> @@ -153,6 +154,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>>>>>>                        printf("Line %d: Error, not all packets flushed. "
>>>>>>                                        "Expected %u, got %u\n",
>>>>>>                                        __LINE__, BURST, total_packet_count());
>>>>>> +                     rte_mempool_put_bulk(p, (void *)bufs, BURST);
>>>>>>                        return -1;
>>>>>>                }
>>>>>>
>>>>>> @@ -179,6 +181,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>>>>>>                printf("Line %d: Error, not all packets flushed. "
>>>>>>                                "Expected %u, got %u\n",
>>>>>>                                __LINE__, BURST, total_packet_count());
>>>>>> +             rte_mempool_put_bulk(p, (void *)bufs, BURST);
>>>>>>                return -1;
>>>>>>        }
>>>>>>
>>>>>> @@ -233,6 +236,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>>>>>>        if (num_returned != BIG_BATCH) {
>>>>>>                printf("line %d: Missing packets, expected %d\n",
>>>>>>                                __LINE__, num_returned);
>>>>>> +             rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
>>>>>>                return -1;
>>>>>>        }
>>>>>>
>>>>>> @@ -247,6 +251,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>>>>>>
>>>>>>                if (j == BIG_BATCH) {
>>>>>>                        printf("Error: could not find source packet #%u\n", i);
>>>>>> +                     rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
>>>>>>                        return -1;
>>>>>>                }
>>>>>>        }
>>>>>>
>>>>>> 2.17.1
>>>>>>
>>>>> The sanity_test is ok now and does not have any mempool leaks.
>>>>>
>>>>> What about other tests in this file: Do you plan to work on them also?
>>>>>
>>>>> --
>>>>>
>>>>> Lukasz Wojciechowski
>>>>> Principal Software Engineer
>>>>>
>>>>> Samsung R&D Institute Poland
>>>>> Samsung Electronics
>>>>> Office +48 22 377 88 25
>>>>> l.wojciechow@partner.samsung.com
>>>>>
>>>> --
>>>>
>>>> Lukasz Wojciechowski
>>>> Principal Software Engineer
>>>>
>>>> Samsung R&D Institute Poland
>>>> Samsung Electronics
>>>> Office +48 22 377 88 25
>>>> l.wojciechow@partner.samsung.com
>>>>
>>>>
>>>>
>>>>
-- 
Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com


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

* [dpdk-dev] [v3 PATCH] test_distributor: prevent memory leakages from the pool
  2020-04-15  6:42   ` [dpdk-dev] [PATCH v2] app/test/test_distributor.c: " Sarosh Arif
  2020-04-15  6:52     ` Lukasz Wojciechowski
@ 2020-09-08 10:22     ` Sarosh Arif
  2020-09-16 19:01       ` Lukasz Wojciechowski
                         ` (2 more replies)
  1 sibling, 3 replies; 16+ messages in thread
From: Sarosh Arif @ 2020-09-08 10:22 UTC (permalink / raw)
  To: l.wojciechow, david.hunt; +Cc: dev, Sarosh Arif

rte_mempool_get_bulk is used to get bufs/many_bufs from the pool,
but at some locations when test fails the bufs/many_bufs are
not returned back to the pool.
Due to this, multiple executions of distributor_autotest gives the
following error message: Error getting mbufs from pool.
To resolve this issue rte_mempool_put_bulk is used whenever the test
fails and returns.

Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
---
v2:
remove double freeing of mbufs
v3:
resubmit to run the tests again
---
 app/test/test_distributor.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
index ba1f81cf8..1a893a7d9 100644
--- a/app/test/test_distributor.c
+++ b/app/test/test_distributor.c
@@ -128,6 +128,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 		printf("Line %d: Error, not all packets flushed. "
 				"Expected %u, got %u\n",
 				__LINE__, BURST, total_packet_count());
+		rte_mempool_put_bulk(p, (void *)bufs, BURST);
 		return -1;
 	}
 
@@ -153,6 +154,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 			printf("Line %d: Error, not all packets flushed. "
 					"Expected %u, got %u\n",
 					__LINE__, BURST, total_packet_count());
+			rte_mempool_put_bulk(p, (void *)bufs, BURST);
 			return -1;
 		}
 
@@ -179,6 +181,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 		printf("Line %d: Error, not all packets flushed. "
 				"Expected %u, got %u\n",
 				__LINE__, BURST, total_packet_count());
+		rte_mempool_put_bulk(p, (void *)bufs, BURST);
 		return -1;
 	}
 
@@ -233,6 +236,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 	if (num_returned != BIG_BATCH) {
 		printf("line %d: Missing packets, expected %d\n",
 				__LINE__, num_returned);
+		rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
 		return -1;
 	}
 
@@ -247,6 +251,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
 
 		if (j == BIG_BATCH) {
 			printf("Error: could not find source packet #%u\n", i);
+			rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
 			return -1;
 		}
 	}
-- 
2.25.1


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

* Re: [dpdk-dev] [v3 PATCH] test_distributor: prevent memory leakages from the pool
  2020-09-08 10:22     ` [dpdk-dev] [v3 PATCH] test_distributor: " Sarosh Arif
@ 2020-09-16 19:01       ` Lukasz Wojciechowski
  2020-09-25 14:22       ` David Marchand
  2020-09-25 15:26       ` David Hunt
  2 siblings, 0 replies; 16+ messages in thread
From: Lukasz Wojciechowski @ 2020-09-16 19:01 UTC (permalink / raw)
  To: Sarosh Arif, david.hunt; +Cc: dev, "'Lukasz Wojciechowski'",

W dniu 08.09.2020 o 12:22, Sarosh Arif pisze:
> rte_mempool_get_bulk is used to get bufs/many_bufs from the pool,
> but at some locations when test fails the bufs/many_bufs are
> not returned back to the pool.
> Due to this, multiple executions of distributor_autotest gives the
> following error message: Error getting mbufs from pool.
> To resolve this issue rte_mempool_put_bulk is used whenever the test
> fails and returns.
>
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> ---
> v2:
> remove double freeing of mbufs
> v3:
> resubmit to run the tests again
> ---
>   app/test/test_distributor.c | 5 +++++
>   1 file changed, 5 insertions(+)
>
> diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c
> index ba1f81cf8..1a893a7d9 100644
> --- a/app/test/test_distributor.c
> +++ b/app/test/test_distributor.c
> @@ -128,6 +128,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   		printf("Line %d: Error, not all packets flushed. "
>   				"Expected %u, got %u\n",
>   				__LINE__, BURST, total_packet_count());
> +		rte_mempool_put_bulk(p, (void *)bufs, BURST);
>   		return -1;
>   	}
>   
> @@ -153,6 +154,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   			printf("Line %d: Error, not all packets flushed. "
>   					"Expected %u, got %u\n",
>   					__LINE__, BURST, total_packet_count());
> +			rte_mempool_put_bulk(p, (void *)bufs, BURST);
>   			return -1;
>   		}
>   
> @@ -179,6 +181,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   		printf("Line %d: Error, not all packets flushed. "
>   				"Expected %u, got %u\n",
>   				__LINE__, BURST, total_packet_count());
> +		rte_mempool_put_bulk(p, (void *)bufs, BURST);
>   		return -1;
>   	}
>   
> @@ -233,6 +236,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   	if (num_returned != BIG_BATCH) {
>   		printf("line %d: Missing packets, expected %d\n",
>   				__LINE__, num_returned);
> +		rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
>   		return -1;
>   	}
>   
> @@ -247,6 +251,7 @@ sanity_test(struct worker_params *wp, struct rte_mempool *p)
>   
>   		if (j == BIG_BATCH) {
>   			printf("Error: could not find source packet #%u\n", i);
> +			rte_mempool_put_bulk(p, (void *)many_bufs, BIG_BATCH);
>   			return -1;
>   		}
>   	}
Acked-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>

-- 
Lukasz Wojciechowski
Principal Software Engineer

Samsung R&D Institute Poland
Samsung Electronics
Office +48 22 377 88 25
l.wojciechow@partner.samsung.com


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

* Re: [dpdk-dev] [v3 PATCH] test_distributor: prevent memory leakages from the pool
  2020-09-08 10:22     ` [dpdk-dev] [v3 PATCH] test_distributor: " Sarosh Arif
  2020-09-16 19:01       ` Lukasz Wojciechowski
@ 2020-09-25 14:22       ` David Marchand
  2020-09-25 15:31         ` David Hunt
  2020-09-28  9:55         ` Sarosh Arif
  2020-09-25 15:26       ` David Hunt
  2 siblings, 2 replies; 16+ messages in thread
From: David Marchand @ 2020-09-25 14:22 UTC (permalink / raw)
  To: Sarosh Arif, David Hunt; +Cc: Lukasz Wojciechowski, dev

On Tue, Sep 8, 2020 at 12:22 PM Sarosh Arif <sarosh.arif@emumba.com> wrote:
>
> rte_mempool_get_bulk is used to get bufs/many_bufs from the pool,
> but at some locations when test fails the bufs/many_bufs are
> not returned back to the pool.
> Due to this, multiple executions of distributor_autotest gives the
> following error message: Error getting mbufs from pool.
> To resolve this issue rte_mempool_put_bulk is used whenever the test
> fails and returns.
>
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>

It deserves a Fixes: line.

I can see a ack from Lukasz.
David, review please?


-- 
David Marchand


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

* Re: [dpdk-dev] [v3 PATCH] test_distributor: prevent memory leakages from the pool
  2020-09-08 10:22     ` [dpdk-dev] [v3 PATCH] test_distributor: " Sarosh Arif
  2020-09-16 19:01       ` Lukasz Wojciechowski
  2020-09-25 14:22       ` David Marchand
@ 2020-09-25 15:26       ` David Hunt
  2020-10-19  8:34         ` David Marchand
  2 siblings, 1 reply; 16+ messages in thread
From: David Hunt @ 2020-09-25 15:26 UTC (permalink / raw)
  To: Sarosh Arif, l.wojciechow; +Cc: dev


On 8/9/2020 11:22 AM, Sarosh Arif wrote:
> rte_mempool_get_bulk is used to get bufs/many_bufs from the pool,
> but at some locations when test fails the bufs/many_bufs are
> not returned back to the pool.
> Due to this, multiple executions of distributor_autotest gives the
> following error message: Error getting mbufs from pool.
> To resolve this issue rte_mempool_put_bulk is used whenever the test
> fails and returns.
>
> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> ---
> v2:
> remove double freeing of mbufs
> v3:
> resubmit to run the tests again
> ---
> }

--snip--

Looks good to me. Even though I could not repeat the conditions to cause 
one of these errors, the additons make sense.

In my testing, I did add in several rte_mempool_avail_count() checks to 
see if there were leakages, and all the mempool counts were
stable, but that was due to the fixes in the patch set from Lukasz : 
http://patches.dpdk.org/project/dpdk/list/?series=12442

However, if there are situations where packets are not returned from 
workers, this should clean them up nicely before returning.

Reviewed-by: David Hunt <david.hunt@intel.com>





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

* Re: [dpdk-dev] [v3 PATCH] test_distributor: prevent memory leakages from the pool
  2020-09-25 14:22       ` David Marchand
@ 2020-09-25 15:31         ` David Hunt
  2020-09-28  9:55         ` Sarosh Arif
  1 sibling, 0 replies; 16+ messages in thread
From: David Hunt @ 2020-09-25 15:31 UTC (permalink / raw)
  To: David Marchand, Sarosh Arif; +Cc: Lukasz Wojciechowski, dev

Hi David,

On 25/9/2020 3:22 PM, David Marchand wrote:
> On Tue, Sep 8, 2020 at 12:22 PM Sarosh Arif <sarosh.arif@emumba.com> wrote:
>> rte_mempool_get_bulk is used to get bufs/many_bufs from the pool,
>> but at some locations when test fails the bufs/many_bufs are
>> not returned back to the pool.
>> Due to this, multiple executions of distributor_autotest gives the
>> following error message: Error getting mbufs from pool.
>> To resolve this issue rte_mempool_put_bulk is used whenever the test
>> fails and returns.
>>
>> Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
> It deserves a Fixes: line.
>
> I can see a ack from Lukasz.
> David, review please?

Done. Apoligies, I should have put you on CC:

My review comments have just been sent to the mailing list.

Rgds,
Dave.





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

* Re: [dpdk-dev] [v3 PATCH] test_distributor: prevent memory leakages from the pool
  2020-09-25 14:22       ` David Marchand
  2020-09-25 15:31         ` David Hunt
@ 2020-09-28  9:55         ` Sarosh Arif
  2020-09-28 10:14           ` David Marchand
  1 sibling, 1 reply; 16+ messages in thread
From: Sarosh Arif @ 2020-09-28  9:55 UTC (permalink / raw)
  To: David Marchand; +Cc: David Hunt, Lukasz Wojciechowski, dev

On Fri, Sep 25, 2020 at 7:22 PM David Marchand <david.marchand@redhat.com>
wrote:

> On Tue, Sep 8, 2020 at 12:22 PM Sarosh Arif <sarosh.arif@emumba.com>
> wrote:
> >
> > rte_mempool_get_bulk is used to get bufs/many_bufs from the pool,
> > but at some locations when test fails the bufs/many_bufs are
> > not returned back to the pool.
> > Due to this, multiple executions of distributor_autotest gives the
> > following error message: Error getting mbufs from pool.
> > To resolve this issue rte_mempool_put_bulk is used whenever the test
> > fails and returns.
> >
> > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
>
> It deserves a Fixes: line.
>
Fixes: c3eabff124e6 ("distributor: add unit tests")

Should I submit another version of this patch to add fixes?

>
> I can see a ack from Lukasz.
> David, review please?
>
>
> --
> David Marchand
>
>

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

* Re: [dpdk-dev] [v3 PATCH] test_distributor: prevent memory leakages from the pool
  2020-09-28  9:55         ` Sarosh Arif
@ 2020-09-28 10:14           ` David Marchand
  0 siblings, 0 replies; 16+ messages in thread
From: David Marchand @ 2020-09-28 10:14 UTC (permalink / raw)
  To: Sarosh Arif; +Cc: David Hunt, Lukasz Wojciechowski, dev

On Mon, Sep 28, 2020 at 11:55 AM Sarosh Arif <sarosh.arif@emumba.com> wrote:
> On Fri, Sep 25, 2020 at 7:22 PM David Marchand <david.marchand@redhat.com> wrote:
>> It deserves a Fixes: line.
>
> Fixes: c3eabff124e6 ("distributor: add unit tests")
>
> Should I submit another version of this patch to add fixes?

I was waiting for a review from David H. and this Fixes line lgtm.
I will update when applying (with the other ut tests fixes from
Lukasz), no need for a new revision.

Thanks.


-- 
David Marchand


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

* Re: [dpdk-dev] [v3 PATCH] test_distributor: prevent memory leakages from the pool
  2020-09-25 15:26       ` David Hunt
@ 2020-10-19  8:34         ` David Marchand
  0 siblings, 0 replies; 16+ messages in thread
From: David Marchand @ 2020-10-19  8:34 UTC (permalink / raw)
  To: Sarosh Arif; +Cc: David Hunt, Lukasz Wojciechowski, dev

On Fri, Sep 25, 2020 at 5:27 PM David Hunt <david.hunt@intel.com> wrote:
> On 8/9/2020 11:22 AM, Sarosh Arif wrote:
> > rte_mempool_get_bulk is used to get bufs/many_bufs from the pool,
> > but at some locations when test fails the bufs/many_bufs are
> > not returned back to the pool.
> > Due to this, multiple executions of distributor_autotest gives the
> > following error message: Error getting mbufs from pool.
> > To resolve this issue rte_mempool_put_bulk is used whenever the test
> > fails and returns.
> >

Fixes: c3eabff124e6 ("distributor: add unit tests")
Cc: stable@dpdk.org

> > Signed-off-by: Sarosh Arif <sarosh.arif@emumba.com>
Acked-by: Lukasz Wojciechowski <l.wojciechow@partner.samsung.com>
> Reviewed-by: David Hunt <david.hunt@intel.com>

Applied, thanks Sarosh.


-- 
David Marchand


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

end of thread, other threads:[~2020-10-19  8:34 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20200413092039eucas1p1372014e18d014115634fc8eb23508004@eucas1p1.samsung.com>
2020-04-13  9:19 ` [dpdk-dev] [PATCH] test_distributor.c: prevent memory leakages from the pool Sarosh Arif
2020-04-15  1:15   ` Lukasz Wojciechowski
2020-04-15  6:42   ` [dpdk-dev] [PATCH v2] app/test/test_distributor.c: " Sarosh Arif
2020-04-15  6:52     ` Lukasz Wojciechowski
2020-04-15  7:06       ` Sarosh Arif
2020-04-15  7:08         ` Lukasz Wojciechowski
     [not found]           ` <CABoZmYPV-GXESgjey7-g-j=cNif38O-SZ4L7TfrDTwb6KfDsog@mail.gmail.com>
     [not found]             ` <CABoZmYOhpYqV07Q4JguvSwJmAPzfhO-f8fMKUzsMoUdUGU5hjQ@mail.gmail.com>
2020-06-24 10:02               ` Sarosh Arif
2020-09-01 18:31                 ` Lukasz Wojciechowski
2020-09-08 10:22     ` [dpdk-dev] [v3 PATCH] test_distributor: " Sarosh Arif
2020-09-16 19:01       ` Lukasz Wojciechowski
2020-09-25 14:22       ` David Marchand
2020-09-25 15:31         ` David Hunt
2020-09-28  9:55         ` Sarosh Arif
2020-09-28 10:14           ` David Marchand
2020-09-25 15:26       ` David Hunt
2020-10-19  8:34         ` David Marchand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).