DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v1] test/func_reentrancy: free memzones after creating test case
@ 2021-07-28  7:33 Joyce Kong
  2021-07-30 11:27 ` Olivier Matz
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Joyce Kong @ 2021-07-28  7:33 UTC (permalink / raw)
  To: anatoly.burakov, olivier.matz, andrew.rybchenko, yipeng1.wang,
	sameh.gobriel, bruce.richardson, vladimir.medvedkin,
	konstantin.ananyev, honnappa.nagarahalli, ruifeng.wang
  Cc: dev, nd, stable

Function reentrancy test limits maximum number of iterations
simultaneously, however it doesn't free the 'fr_test_once'
memzones after the fact, so introduce freeing 'fr_test_once'
in ring/mempool/hash/fbk/lpm_clean.

Fixes: 104a92bd026f ("app: add reentrancy tests")
Fixes: 995eec619024 ("test: clean up memory for function reentrancy test")
Cc: stable@dpdk.org

Signed-off-by: Joyce Kong <joyce.kong@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
---
 app/test/test_func_reentrancy.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/app/test/test_func_reentrancy.c b/app/test/test_func_reentrancy.c
index 231c99a9eb..e4e9c2cc7c 100644
--- a/app/test/test_func_reentrancy.c
+++ b/app/test/test_func_reentrancy.c
@@ -89,6 +89,10 @@ ring_clean(unsigned int lcore_id)
 	char ring_name[MAX_STRING_SIZE];
 	int i;
 
+	rp = rte_ring_lookup("fr_test_once");
+	if (rp != NULL)
+		rte_ring_free(rp);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(ring_name, sizeof(ring_name),
 				"fr_test_%d_%d", lcore_id, i);
@@ -148,7 +152,10 @@ mempool_clean(unsigned int lcore_id)
 	char mempool_name[MAX_STRING_SIZE];
 	int i;
 
-	/* verify all ring created successful */
+	mp = rte_mempool_lookup("fr_test_once");
+	if (mp != NULL)
+		rte_mempool_free(mp);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(mempool_name, sizeof(mempool_name), "fr_test_%d_%d",
 			 lcore_id, i);
@@ -208,6 +215,10 @@ hash_clean(unsigned lcore_id)
 	struct rte_hash *handle;
 	int i;
 
+	handle = rte_hash_find_existing("fr_test_once");
+	if (handle != NULL)
+		rte_hash_free(handle);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(hash_name, sizeof(hash_name), "fr_test_%d_%d",  lcore_id, i);
 
@@ -272,6 +283,10 @@ fbk_clean(unsigned lcore_id)
 	struct rte_fbk_hash_table *handle;
 	int i;
 
+	handle = rte_fbk_hash_find_existing("fr_test_once");
+	if (handle != NULL)
+		rte_fbk_hash_free(handle);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(fbk_name, sizeof(fbk_name), "fr_test_%d_%d",  lcore_id, i);
 
@@ -338,6 +353,10 @@ lpm_clean(unsigned int lcore_id)
 	struct rte_lpm *lpm;
 	int i;
 
+	lpm = rte_lpm_find_existing("fr_test_once");
+	if (lpm != NULL)
+		rte_lpm_free(lpm);
+
 	for (i = 0; i < MAX_LPM_ITER_TIMES; i++) {
 		snprintf(lpm_name, sizeof(lpm_name), "fr_test_%d_%d",  lcore_id, i);
 
@@ -454,6 +473,9 @@ launch_test(struct test_case *pt_case)
 			pt_case->clean(lcore_id);
 	}
 
+	if (pt_case->clean != NULL)
+		pt_case->clean(rte_get_main_lcore());
+
 	count = rte_atomic32_read(&obj_count);
 	if (count != 1) {
 		printf("%s: common object allocated %d times (should be 1)\n",
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v1] test/func_reentrancy: free memzones after creating test case
  2021-07-28  7:33 [dpdk-dev] [PATCH v1] test/func_reentrancy: free memzones after creating test case Joyce Kong
@ 2021-07-30 11:27 ` Olivier Matz
  2021-07-31  8:42   ` Joyce Kong
  2021-07-31 12:03 ` [dpdk-dev] [PATCH v2] " Joyce Kong
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Olivier Matz @ 2021-07-30 11:27 UTC (permalink / raw)
  To: Joyce Kong
  Cc: anatoly.burakov, andrew.rybchenko, yipeng1.wang, sameh.gobriel,
	bruce.richardson, vladimir.medvedkin, konstantin.ananyev,
	honnappa.nagarahalli, ruifeng.wang, dev, nd, stable

Hi Joyce,

On Wed, Jul 28, 2021 at 02:33:22AM -0500, Joyce Kong wrote:
> Function reentrancy test limits maximum number of iterations
> simultaneously, however it doesn't free the 'fr_test_once'
> memzones after the fact, so introduce freeing 'fr_test_once'
> in ring/mempool/hash/fbk/lpm_clean.
> 
> Fixes: 104a92bd026f ("app: add reentrancy tests")
> Fixes: 995eec619024 ("test: clean up memory for function reentrancy test")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> ---
>  app/test/test_func_reentrancy.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test/test_func_reentrancy.c b/app/test/test_func_reentrancy.c
> index 231c99a9eb..e4e9c2cc7c 100644
> --- a/app/test/test_func_reentrancy.c
> +++ b/app/test/test_func_reentrancy.c
> @@ -89,6 +89,10 @@ ring_clean(unsigned int lcore_id)
>  	char ring_name[MAX_STRING_SIZE];
>  	int i;
>  
> +	rp = rte_ring_lookup("fr_test_once");
> +	if (rp != NULL)
> +		rte_ring_free(rp);
> +
>  	for (i = 0; i < MAX_ITER_MULTI; i++) {
>  		snprintf(ring_name, sizeof(ring_name),
>  				"fr_test_%d_%d", lcore_id, i);
> @@ -148,7 +152,10 @@ mempool_clean(unsigned int lcore_id)
>  	char mempool_name[MAX_STRING_SIZE];
>  	int i;
>  
> -	/* verify all ring created successful */
> +	mp = rte_mempool_lookup("fr_test_once");
> +	if (mp != NULL)
> +		rte_mempool_free(mp);
> +
>  	for (i = 0; i < MAX_ITER_MULTI; i++) {
>  		snprintf(mempool_name, sizeof(mempool_name), "fr_test_%d_%d",
>  			 lcore_id, i);
> @@ -208,6 +215,10 @@ hash_clean(unsigned lcore_id)
>  	struct rte_hash *handle;
>  	int i;
>  
> +	handle = rte_hash_find_existing("fr_test_once");
> +	if (handle != NULL)
> +		rte_hash_free(handle);
> +
>  	for (i = 0; i < MAX_ITER_MULTI; i++) {
>  		snprintf(hash_name, sizeof(hash_name), "fr_test_%d_%d",  lcore_id, i);
>  
> @@ -272,6 +283,10 @@ fbk_clean(unsigned lcore_id)
>  	struct rte_fbk_hash_table *handle;
>  	int i;
>  
> +	handle = rte_fbk_hash_find_existing("fr_test_once");
> +	if (handle != NULL)
> +		rte_fbk_hash_free(handle);
> +
>  	for (i = 0; i < MAX_ITER_MULTI; i++) {
>  		snprintf(fbk_name, sizeof(fbk_name), "fr_test_%d_%d",  lcore_id, i);
>  
> @@ -338,6 +353,10 @@ lpm_clean(unsigned int lcore_id)
>  	struct rte_lpm *lpm;
>  	int i;
>  
> +	lpm = rte_lpm_find_existing("fr_test_once");
> +	if (lpm != NULL)
> +		rte_lpm_free(lpm);
> +
>  	for (i = 0; i < MAX_LPM_ITER_TIMES; i++) {
>  		snprintf(lpm_name, sizeof(lpm_name), "fr_test_%d_%d",  lcore_id, i);
>  
> @@ -454,6 +473,9 @@ launch_test(struct test_case *pt_case)
>  			pt_case->clean(lcore_id);
>  	}
>  
> +	if (pt_case->clean != NULL)
> +		pt_case->clean(rte_get_main_lcore());
> +

Is it the same issue? It looks it adds the missing frees for the main thread
(not only "fr_test_once"). I don't think it requires another patch, but a word
could be added about it in the commit log.


>  	count = rte_atomic32_read(&obj_count);
>  	if (count != 1) {
>  		printf("%s: common object allocated %d times (should be 1)\n",
> -- 
> 2.17.1
> 

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

* Re: [dpdk-dev] [PATCH v1] test/func_reentrancy: free memzones after creating test case
  2021-07-30 11:27 ` Olivier Matz
@ 2021-07-31  8:42   ` Joyce Kong
  0 siblings, 0 replies; 12+ messages in thread
From: Joyce Kong @ 2021-07-31  8:42 UTC (permalink / raw)
  To: Olivier Matz
  Cc: anatoly.burakov, andrew.rybchenko, yipeng1.wang, sameh.gobriel,
	bruce.richardson, vladimir.medvedkin, konstantin.ananyev,
	Honnappa Nagarahalli, Ruifeng Wang, dev, nd, stable



> -----Original Message-----
> From: Olivier Matz <olivier.matz@6wind.com>
> Sent: Friday, July 30, 2021 7:27 PM
> To: Joyce Kong <Joyce.Kong@arm.com>
> Cc: anatoly.burakov@intel.com; andrew.rybchenko@oktetlabs.ru;
> yipeng1.wang@intel.com; sameh.gobriel@intel.com;
> bruce.richardson@intel.com; vladimir.medvedkin@intel.com;
> konstantin.ananyev@intel.com; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; dev@dpdk.org; nd <nd@arm.com>;
> stable@dpdk.org
> Subject: Re: [PATCH v1] test/func_reentrancy: free memzones after creating
> test case
> 
> Hi Joyce,
> 
> On Wed, Jul 28, 2021 at 02:33:22AM -0500, Joyce Kong wrote:
> > Function reentrancy test limits maximum number of iterations
> > simultaneously, however it doesn't free the 'fr_test_once'
> > memzones after the fact, so introduce freeing 'fr_test_once'
> > in ring/mempool/hash/fbk/lpm_clean.
> >
> > Fixes: 104a92bd026f ("app: add reentrancy tests")
> > Fixes: 995eec619024 ("test: clean up memory for function reentrancy
> > test")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> > ---
> >  app/test/test_func_reentrancy.c | 24 +++++++++++++++++++++++-
> >  1 file changed, 23 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test/test_func_reentrancy.c
> > b/app/test/test_func_reentrancy.c index 231c99a9eb..e4e9c2cc7c 100644
> > --- a/app/test/test_func_reentrancy.c
> > +++ b/app/test/test_func_reentrancy.c
> > @@ -89,6 +89,10 @@ ring_clean(unsigned int lcore_id)
> >  	char ring_name[MAX_STRING_SIZE];
> >  	int i;
> >
> > +	rp = rte_ring_lookup("fr_test_once");
> > +	if (rp != NULL)
> > +		rte_ring_free(rp);
> > +
> >  	for (i = 0; i < MAX_ITER_MULTI; i++) {
> >  		snprintf(ring_name, sizeof(ring_name),
> >  				"fr_test_%d_%d", lcore_id, i);
> > @@ -148,7 +152,10 @@ mempool_clean(unsigned int lcore_id)
> >  	char mempool_name[MAX_STRING_SIZE];
> >  	int i;
> >
> > -	/* verify all ring created successful */
> > +	mp = rte_mempool_lookup("fr_test_once");
> > +	if (mp != NULL)
> > +		rte_mempool_free(mp);
> > +
> >  	for (i = 0; i < MAX_ITER_MULTI; i++) {
> >  		snprintf(mempool_name, sizeof(mempool_name),
> "fr_test_%d_%d",
> >  			 lcore_id, i);
> > @@ -208,6 +215,10 @@ hash_clean(unsigned lcore_id)
> >  	struct rte_hash *handle;
> >  	int i;
> >
> > +	handle = rte_hash_find_existing("fr_test_once");
> > +	if (handle != NULL)
> > +		rte_hash_free(handle);
> > +
> >  	for (i = 0; i < MAX_ITER_MULTI; i++) {
> >  		snprintf(hash_name, sizeof(hash_name), "fr_test_%d_%d",
> lcore_id,
> > i);
> >
> > @@ -272,6 +283,10 @@ fbk_clean(unsigned lcore_id)
> >  	struct rte_fbk_hash_table *handle;
> >  	int i;
> >
> > +	handle = rte_fbk_hash_find_existing("fr_test_once");
> > +	if (handle != NULL)
> > +		rte_fbk_hash_free(handle);
> > +
> >  	for (i = 0; i < MAX_ITER_MULTI; i++) {
> >  		snprintf(fbk_name, sizeof(fbk_name), "fr_test_%d_%d",
> lcore_id,
> > i);
> >
> > @@ -338,6 +353,10 @@ lpm_clean(unsigned int lcore_id)
> >  	struct rte_lpm *lpm;
> >  	int i;
> >
> > +	lpm = rte_lpm_find_existing("fr_test_once");
> > +	if (lpm != NULL)
> > +		rte_lpm_free(lpm);
> > +
> >  	for (i = 0; i < MAX_LPM_ITER_TIMES; i++) {
> >  		snprintf(lpm_name, sizeof(lpm_name), "fr_test_%d_%d",
> lcore_id,
> > i);
> >
> > @@ -454,6 +473,9 @@ launch_test(struct test_case *pt_case)
> >  			pt_case->clean(lcore_id);
> >  	}
> >
> > +	if (pt_case->clean != NULL)
> > +		pt_case->clean(rte_get_main_lcore());
> > +
> 
> Is it the same issue? It looks it adds the missing frees for the main thread (not
> only "fr_test_once"). I don't think it requires another patch, but a word could
> be added about it in the commit log.
> 
Thanks, Olivier, will send another version to add this in the commit log.

> 
> >  	count = rte_atomic32_read(&obj_count);
> >  	if (count != 1) {
> >  		printf("%s: common object allocated %d times (should be
> 1)\n",
> > --
> > 2.17.1
> >

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

* [dpdk-dev] [PATCH v2] test/func_reentrancy: free memzones after creating test case
  2021-07-28  7:33 [dpdk-dev] [PATCH v1] test/func_reentrancy: free memzones after creating test case Joyce Kong
  2021-07-30 11:27 ` Olivier Matz
@ 2021-07-31 12:03 ` Joyce Kong
  2021-08-17  8:17   ` David Marchand
  2021-08-20  8:12 ` [dpdk-dev] [PATCH v3] test/func_reentrancy: free memzones after creating test Joyce Kong
  2021-08-23  2:57 ` [dpdk-dev] [PATCH v4] " Joyce Kong
  3 siblings, 1 reply; 12+ messages in thread
From: Joyce Kong @ 2021-07-31 12:03 UTC (permalink / raw)
  To: anatoly.burakov, olivier.matz, andrew.rybchenko, yipeng1.wang,
	sameh.gobriel, bruce.richardson, vladimir.medvedkin,
	konstantin.ananyev, honnappa.nagarahalli, ruifeng.wang
  Cc: dev, nd, stable

Function reentrancy test limits maximum number of iterations
simultaneously, however it doesn't free the 'fr_test_once'
memzones after the fact, so introduce freeing 'fr_test_once'
in ring/mempool/hash/fbk/lpm_clean.

Meanwhile, add the missing free for test case on main thread.

Fixes: 104a92bd026f ("app: add reentrancy tests")
Fixes: 995eec619024 ("test: clean up memory for function reentrancy test")
Cc: stable@dpdk.org

Signed-off-by: Joyce Kong <joyce.kong@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
---
 app/test/test_func_reentrancy.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/app/test/test_func_reentrancy.c b/app/test/test_func_reentrancy.c
index 231c99a9eb..e4e9c2cc7c 100644
--- a/app/test/test_func_reentrancy.c
+++ b/app/test/test_func_reentrancy.c
@@ -89,6 +89,10 @@ ring_clean(unsigned int lcore_id)
 	char ring_name[MAX_STRING_SIZE];
 	int i;
 
+	rp = rte_ring_lookup("fr_test_once");
+	if (rp != NULL)
+		rte_ring_free(rp);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(ring_name, sizeof(ring_name),
 				"fr_test_%d_%d", lcore_id, i);
@@ -148,7 +152,10 @@ mempool_clean(unsigned int lcore_id)
 	char mempool_name[MAX_STRING_SIZE];
 	int i;
 
-	/* verify all ring created successful */
+	mp = rte_mempool_lookup("fr_test_once");
+	if (mp != NULL)
+		rte_mempool_free(mp);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(mempool_name, sizeof(mempool_name), "fr_test_%d_%d",
 			 lcore_id, i);
@@ -208,6 +215,10 @@ hash_clean(unsigned lcore_id)
 	struct rte_hash *handle;
 	int i;
 
+	handle = rte_hash_find_existing("fr_test_once");
+	if (handle != NULL)
+		rte_hash_free(handle);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(hash_name, sizeof(hash_name), "fr_test_%d_%d",  lcore_id, i);
 
@@ -272,6 +283,10 @@ fbk_clean(unsigned lcore_id)
 	struct rte_fbk_hash_table *handle;
 	int i;
 
+	handle = rte_fbk_hash_find_existing("fr_test_once");
+	if (handle != NULL)
+		rte_fbk_hash_free(handle);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(fbk_name, sizeof(fbk_name), "fr_test_%d_%d",  lcore_id, i);
 
@@ -338,6 +353,10 @@ lpm_clean(unsigned int lcore_id)
 	struct rte_lpm *lpm;
 	int i;
 
+	lpm = rte_lpm_find_existing("fr_test_once");
+	if (lpm != NULL)
+		rte_lpm_free(lpm);
+
 	for (i = 0; i < MAX_LPM_ITER_TIMES; i++) {
 		snprintf(lpm_name, sizeof(lpm_name), "fr_test_%d_%d",  lcore_id, i);
 
@@ -454,6 +473,9 @@ launch_test(struct test_case *pt_case)
 			pt_case->clean(lcore_id);
 	}
 
+	if (pt_case->clean != NULL)
+		pt_case->clean(rte_get_main_lcore());
+
 	count = rte_atomic32_read(&obj_count);
 	if (count != 1) {
 		printf("%s: common object allocated %d times (should be 1)\n",
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v2] test/func_reentrancy: free memzones after creating test case
  2021-07-31 12:03 ` [dpdk-dev] [PATCH v2] " Joyce Kong
@ 2021-08-17  8:17   ` David Marchand
  2021-08-20  7:57     ` Joyce Kong
  0 siblings, 1 reply; 12+ messages in thread
From: David Marchand @ 2021-08-17  8:17 UTC (permalink / raw)
  To: Joyce Kong
  Cc: Burakov, Anatoly, Olivier Matz, Andrew Rybchenko, Wang, Yipeng1,
	Gobriel, Sameh, Bruce Richardson, Vladimir Medvedkin, Ananyev,
	Konstantin, Honnappa Nagarahalli,
	Ruifeng Wang (Arm Technology China),
	dev, nd, dpdk stable, Aaron Conole

On Sat, Jul 31, 2021 at 2:04 PM Joyce Kong <joyce.kong@arm.com> wrote:
>
> Function reentrancy test limits maximum number of iterations
> simultaneously, however it doesn't free the 'fr_test_once'
> memzones after the fact, so introduce freeing 'fr_test_once'
> in ring/mempool/hash/fbk/lpm_clean.
>
> Meanwhile, add the missing free for test case on main thread.
>
> Fixes: 104a92bd026f ("app: add reentrancy tests")
> Fixes: 995eec619024 ("test: clean up memory for function reentrancy test")
> Cc: stable@dpdk.org
>
> Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>

This patch actually breaks the test (we are lucky, the failure happens
often ;)).

28/94 DPDK:fast-tests / func_reentrancy_autotest       FAIL
0.22s (exit status 255 or signal 127 SIGinvalid)

--- command ---
16:13:45 DPDK_TEST='func_reentrancy_autotest'
/home-local/jenkins-local/jenkins-agent/workspace/Generic-Unit-Test-DPDK/dpdk/build/app/test/dpdk-test
-l 0-15 --file-prefix=func_reentrancy_autotest
--- stdout ---
RTE>>func_reentrancy_autotest
Func-ReEnt CASE 0: eal init once PASS
ring create/lookup: common object allocated 2 times (should be 1)
Func-ReEnt CASE 1: ring create/lookup FAIL
Test Failed
RTE>>
--- stderr ---


I guess, this is what happens:

main lcore                          worker lcore 1              ...
worker lcore X
                                    enters ring_create_lookup()

enters ring_create_lookup()
rte_eal_wait_lcore(worker lcore 1);
                                    leaves ring_create_lookup()
ring_clean(worker lcore 1);

leaves ring_create_lookup()

There is no synchronisation point for the main lcore to know the
worker lcores are finished invoking the func callback.
With this patch, the "common" object is freed by the main lcore
*potentially* before some workers start trying to create it.
And we end up with multiple workers successfully creating this object,
hence the obj_count being incremented.


-- 
David Marchand


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

* Re: [dpdk-dev] [PATCH v2] test/func_reentrancy: free memzones after creating test case
  2021-08-17  8:17   ` David Marchand
@ 2021-08-20  7:57     ` Joyce Kong
  2021-08-20  8:52       ` David Marchand
  0 siblings, 1 reply; 12+ messages in thread
From: Joyce Kong @ 2021-08-20  7:57 UTC (permalink / raw)
  To: David Marchand
  Cc: Burakov, Anatoly, Olivier Matz, Andrew Rybchenko, Wang, Yipeng1,
	Gobriel, Sameh, Bruce Richardson, Vladimir Medvedkin, Ananyev,
	Konstantin, Honnappa Nagarahalli, Ruifeng Wang, dev, nd,
	dpdk stable, Aaron Conole

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, August 17, 2021 4:17 PM
> To: Joyce Kong <Joyce.Kong@arm.com>
> Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; Olivier Matz
> <olivier.matz@6wind.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Wang, Yipeng1
> <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>;
> Bruce Richardson <bruce.richardson@intel.com>; Vladimir Medvedkin
> <vladimir.medvedkin@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Honnappa Nagarahalli
> <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> <Ruifeng.Wang@arm.com>; dev <dev@dpdk.org>; nd <nd@arm.com>; dpdk
> stable <stable@dpdk.org>; Aaron Conole <aconole@redhat.com>
> Subject: Re: [dpdk-dev] [PATCH v2] test/func_reentrancy: free memzones
> after creating test case
> 
> On Sat, Jul 31, 2021 at 2:04 PM Joyce Kong <joyce.kong@arm.com> wrote:
> >
> > Function reentrancy test limits maximum number of iterations
> > simultaneously, however it doesn't free the 'fr_test_once'
> > memzones after the fact, so introduce freeing 'fr_test_once'
> > in ring/mempool/hash/fbk/lpm_clean.
> >
> > Meanwhile, add the missing free for test case on main thread.
> >
> > Fixes: 104a92bd026f ("app: add reentrancy tests")
> > Fixes: 995eec619024 ("test: clean up memory for function reentrancy
> > test")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> 
> This patch actually breaks the test (we are lucky, the failure happens often ;)).
> 
> 28/94 DPDK:fast-tests / func_reentrancy_autotest       FAIL
> 0.22s (exit status 255 or signal 127 SIGinvalid)
> 
> --- command ---
> 16:13:45 DPDK_TEST='func_reentrancy_autotest'
> /home-local/jenkins-local/jenkins-agent/workspace/Generic-Unit-Test-
> DPDK/dpdk/build/app/test/dpdk-test
> -l 0-15 --file-prefix=func_reentrancy_autotest
> --- stdout ---
> RTE>>func_reentrancy_autotest
> Func-ReEnt CASE 0: eal init once PASS
> ring create/lookup: common object allocated 2 times (should be 1) Func-
> ReEnt CASE 1: ring create/lookup FAIL Test Failed
> RTE>>
> --- stderr ---
> 
> 
> I guess, this is what happens:
> 
> main lcore                          worker lcore 1              ...
> worker lcore X
>                                     enters ring_create_lookup()
> 
> enters ring_create_lookup()
> rte_eal_wait_lcore(worker lcore 1);
>                                     leaves ring_create_lookup() ring_clean(worker lcore 1);
> 
> leaves ring_create_lookup()
> 
> There is no synchronisation point for the main lcore to know the worker
> lcores are finished invoking the func callback.
> With this patch, the "common" object is freed by the main lcore
> *potentially* before some workers start trying to create it.
> And we end up with multiple workers successfully creating this object, hence
> the obj_count being incremented.
> 
> 
> --
> David Marchand

I think add rte_eal_mp_wait_lcore() like below can ensure the lcores to free objects
after all func callback finished.
Shall do the change in next version.

	RTE_LCORE_FOREACH_WORKER(lcore_id) {
		if (cores == 1)
			break;
		cores--;
		rte_eal_remote_launch(pt_case->func, pt_case->arg, lcore_id);
	}
	rte_atomic32_set(&synchro, 1);
	if (pt_case->func(pt_case->arg) < 0)
		ret = -1;

+	rte_eal_mp_wait_lcore();

	cores = cores_save;
	RTE_LCORE_FOREACH_WORKER(lcore_id) {
		if (cores == 1)
			break;
		cores--;
-		if (rte_eal_wait_lcore(lcore_id) < 0)
-			ret = -1;
		if (pt_case->clean != NULL)
			pt_case->clean(lcore_id);
	}
	

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

* [dpdk-dev] [PATCH v3] test/func_reentrancy: free memzones after creating test
  2021-07-28  7:33 [dpdk-dev] [PATCH v1] test/func_reentrancy: free memzones after creating test case Joyce Kong
  2021-07-30 11:27 ` Olivier Matz
  2021-07-31 12:03 ` [dpdk-dev] [PATCH v2] " Joyce Kong
@ 2021-08-20  8:12 ` Joyce Kong
  2021-08-23  2:57 ` [dpdk-dev] [PATCH v4] " Joyce Kong
  3 siblings, 0 replies; 12+ messages in thread
From: Joyce Kong @ 2021-08-20  8:12 UTC (permalink / raw)
  To: david.marchand, anatoly.burakov, olivier.matz, andrew.rybchenko,
	yipeng1.wang, sameh.gobriel, bruce.richardson,
	vladimir.medvedkin, konstantin.ananyev, honnappa.nagarahalli,
	ruifeng.wang
  Cc: dev, nd, stable

Function reentrancy test limits maximum number of iterations
simultaneously, however it doesn't free the 'fr_test_once'
memzones after the fact, so introduce freeing 'fr_test_once'
in ring/mempool/hash/fbk/lpm_clean.

Meanwhile, add the missing free for test case on main thread.

Fixes: 104a92bd026f ("app: add reentrancy tests")
Fixes: 995eec619024 ("test: clean up memory for function reentrancy test")
Cc: stable@dpdk.org

Signed-off-by: Joyce Kong <joyce.kong@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
---
 app/test/test_func_reentrancy.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/app/test/test_func_reentrancy.c b/app/test/test_func_reentrancy.c
index 231c99a9eb..f53268ac08 100644
--- a/app/test/test_func_reentrancy.c
+++ b/app/test/test_func_reentrancy.c
@@ -89,6 +89,10 @@ ring_clean(unsigned int lcore_id)
 	char ring_name[MAX_STRING_SIZE];
 	int i;
 
+	rp = rte_ring_lookup("fr_test_once");
+	if (rp != NULL)
+		rte_ring_free(rp);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(ring_name, sizeof(ring_name),
 				"fr_test_%d_%d", lcore_id, i);
@@ -148,7 +152,10 @@ mempool_clean(unsigned int lcore_id)
 	char mempool_name[MAX_STRING_SIZE];
 	int i;
 
-	/* verify all ring created successful */
+	mp = rte_mempool_lookup("fr_test_once");
+	if (mp != NULL)
+		rte_mempool_free(mp);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(mempool_name, sizeof(mempool_name), "fr_test_%d_%d",
 			 lcore_id, i);
@@ -208,6 +215,10 @@ hash_clean(unsigned lcore_id)
 	struct rte_hash *handle;
 	int i;
 
+	handle = rte_hash_find_existing("fr_test_once");
+	if (handle != NULL)
+		rte_hash_free(handle);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(hash_name, sizeof(hash_name), "fr_test_%d_%d",  lcore_id, i);
 
@@ -272,6 +283,10 @@ fbk_clean(unsigned lcore_id)
 	struct rte_fbk_hash_table *handle;
 	int i;
 
+	handle = rte_fbk_hash_find_existing("fr_test_once");
+	if (handle != NULL)
+		rte_fbk_hash_free(handle);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(fbk_name, sizeof(fbk_name), "fr_test_%d_%d",  lcore_id, i);
 
@@ -338,6 +353,10 @@ lpm_clean(unsigned int lcore_id)
 	struct rte_lpm *lpm;
 	int i;
 
+	lpm = rte_lpm_find_existing("fr_test_once");
+	if (lpm != NULL)
+		rte_lpm_free(lpm);
+
 	for (i = 0; i < MAX_LPM_ITER_TIMES; i++) {
 		snprintf(lpm_name, sizeof(lpm_name), "fr_test_%d_%d",  lcore_id, i);
 
@@ -441,19 +460,21 @@ launch_test(struct test_case *pt_case)
 
 	if (pt_case->func(pt_case->arg) < 0)
 		ret = -1;
+	rte_eal_mp_wait_lcore();
 
 	cores = cores_save;
 	RTE_LCORE_FOREACH_WORKER(lcore_id) {
 		if (cores == 1)
 			break;
 		cores--;
-		if (rte_eal_wait_lcore(lcore_id) < 0)
-			ret = -1;
 
 		if (pt_case->clean != NULL)
 			pt_case->clean(lcore_id);
 	}
 
+	if (pt_case->clean != NULL)
+		pt_case->clean(rte_get_main_lcore());
+
 	count = rte_atomic32_read(&obj_count);
 	if (count != 1) {
 		printf("%s: common object allocated %d times (should be 1)\n",
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v2] test/func_reentrancy: free memzones after creating test case
  2021-08-20  7:57     ` Joyce Kong
@ 2021-08-20  8:52       ` David Marchand
  2021-08-23  2:25         ` Joyce Kong
  0 siblings, 1 reply; 12+ messages in thread
From: David Marchand @ 2021-08-20  8:52 UTC (permalink / raw)
  To: Joyce Kong
  Cc: Burakov, Anatoly, Olivier Matz, Andrew Rybchenko, Wang, Yipeng1,
	Gobriel, Sameh, Bruce Richardson, Vladimir Medvedkin, Ananyev,
	Konstantin, Honnappa Nagarahalli, Ruifeng Wang, dev, nd,
	dpdk stable, Aaron Conole

On Fri, Aug 20, 2021 at 9:57 AM Joyce Kong <Joyce.Kong@arm.com> wrote:
>
> > -----Original Message-----
> > From: David Marchand <david.marchand@redhat.com>
> > Sent: Tuesday, August 17, 2021 4:17 PM
> > To: Joyce Kong <Joyce.Kong@arm.com>
> > Cc: Burakov, Anatoly <anatoly.burakov@intel.com>; Olivier Matz
> > <olivier.matz@6wind.com>; Andrew Rybchenko
> > <andrew.rybchenko@oktetlabs.ru>; Wang, Yipeng1
> > <yipeng1.wang@intel.com>; Gobriel, Sameh <sameh.gobriel@intel.com>;
> > Bruce Richardson <bruce.richardson@intel.com>; Vladimir Medvedkin
> > <vladimir.medvedkin@intel.com>; Ananyev, Konstantin
> > <konstantin.ananyev@intel.com>; Honnappa Nagarahalli
> > <Honnappa.Nagarahalli@arm.com>; Ruifeng Wang
> > <Ruifeng.Wang@arm.com>; dev <dev@dpdk.org>; nd <nd@arm.com>; dpdk
> > stable <stable@dpdk.org>; Aaron Conole <aconole@redhat.com>
> > Subject: Re: [dpdk-dev] [PATCH v2] test/func_reentrancy: free memzones
> > after creating test case
> >
> > On Sat, Jul 31, 2021 at 2:04 PM Joyce Kong <joyce.kong@arm.com> wrote:
> > >
> > > Function reentrancy test limits maximum number of iterations
> > > simultaneously, however it doesn't free the 'fr_test_once'
> > > memzones after the fact, so introduce freeing 'fr_test_once'
> > > in ring/mempool/hash/fbk/lpm_clean.
> > >
> > > Meanwhile, add the missing free for test case on main thread.
> > >
> > > Fixes: 104a92bd026f ("app: add reentrancy tests")
> > > Fixes: 995eec619024 ("test: clean up memory for function reentrancy
> > > test")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> >
> > This patch actually breaks the test (we are lucky, the failure happens often ;)).
> >
> > 28/94 DPDK:fast-tests / func_reentrancy_autotest       FAIL
> > 0.22s (exit status 255 or signal 127 SIGinvalid)
> >
> > --- command ---
> > 16:13:45 DPDK_TEST='func_reentrancy_autotest'
> > /home-local/jenkins-local/jenkins-agent/workspace/Generic-Unit-Test-
> > DPDK/dpdk/build/app/test/dpdk-test
> > -l 0-15 --file-prefix=func_reentrancy_autotest
> > --- stdout ---
> > RTE>>func_reentrancy_autotest
> > Func-ReEnt CASE 0: eal init once PASS
> > ring create/lookup: common object allocated 2 times (should be 1) Func-
> > ReEnt CASE 1: ring create/lookup FAIL Test Failed
> > RTE>>
> > --- stderr ---
> >
> >
> > I guess, this is what happens:
> >
> > main lcore                          worker lcore 1              ...
> > worker lcore X
> >                                     enters ring_create_lookup()
> >
> > enters ring_create_lookup()
> > rte_eal_wait_lcore(worker lcore 1);
> >                                     leaves ring_create_lookup() ring_clean(worker lcore 1);
> >
> > leaves ring_create_lookup()
> >
> > There is no synchronisation point for the main lcore to know the worker
> > lcores are finished invoking the func callback.
> > With this patch, the "common" object is freed by the main lcore
> > *potentially* before some workers start trying to create it.
> > And we end up with multiple workers successfully creating this object, hence
> > the obj_count being incremented.
> >
> >
> > --
> > David Marchand
>
> I think add rte_eal_mp_wait_lcore() like below can ensure the lcores to free objects
> after all func callback finished.
> Shall do the change in next version.
>
>         RTE_LCORE_FOREACH_WORKER(lcore_id) {
>                 if (cores == 1)
>                         break;
>                 cores--;
>                 rte_eal_remote_launch(pt_case->func, pt_case->arg, lcore_id);
>         }
>         rte_atomic32_set(&synchro, 1);
>         if (pt_case->func(pt_case->arg) < 0)
>                 ret = -1;
>
> +       rte_eal_mp_wait_lcore();
>
>         cores = cores_save;
>         RTE_LCORE_FOREACH_WORKER(lcore_id) {
>                 if (cores == 1)
>                         break;
>                 cores--;
> -               if (rte_eal_wait_lcore(lcore_id) < 0)
> -                       ret = -1;
>                 if (pt_case->clean != NULL)
>                         pt_case->clean(lcore_id);
>         }

Using mp_wait_lcore, the test can't tell if a lcore returned an error
after executing the passed callback.

An alternative is to split the current loop to first have the per
lcore rte_eal_wait_lcore() calls + ret code check, and then a second
loop calls the clean() callback.


-- 
David Marchand


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

* Re: [dpdk-dev] [PATCH v2] test/func_reentrancy: free memzones after creating test case
  2021-08-20  8:52       ` David Marchand
@ 2021-08-23  2:25         ` Joyce Kong
  0 siblings, 0 replies; 12+ messages in thread
From: Joyce Kong @ 2021-08-23  2:25 UTC (permalink / raw)
  To: David Marchand
  Cc: Burakov, Anatoly, Olivier Matz, Andrew Rybchenko, Wang, Yipeng1,
	Gobriel, Sameh, Bruce Richardson, Vladimir Medvedkin, Ananyev,
	Konstantin, Honnappa Nagarahalli, Ruifeng Wang, dev, nd,
	dpdk stable, Aaron Conole

<snip>

> > > Subject: Re: [dpdk-dev] [PATCH v2] test/func_reentrancy: free
> > > memzones after creating test case
> > >
> > > On Sat, Jul 31, 2021 at 2:04 PM Joyce Kong <joyce.kong@arm.com> wrote:
> > > >
> > > > Function reentrancy test limits maximum number of iterations
> > > > simultaneously, however it doesn't free the 'fr_test_once'
> > > > memzones after the fact, so introduce freeing 'fr_test_once'
> > > > in ring/mempool/hash/fbk/lpm_clean.
> > > >
> > > > Meanwhile, add the missing free for test case on main thread.
> > > >
> > > > Fixes: 104a92bd026f ("app: add reentrancy tests")
> > > > Fixes: 995eec619024 ("test: clean up memory for function
> > > > reentrancy
> > > > test")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> > > > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > > > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> > >
> > > This patch actually breaks the test (we are lucky, the failure happens
> often ;)).
> > >
> > > 28/94 DPDK:fast-tests / func_reentrancy_autotest       FAIL
> > > 0.22s (exit status 255 or signal 127 SIGinvalid)
> > >
> > > --- command ---
> > > 16:13:45 DPDK_TEST='func_reentrancy_autotest'
> > > /home-local/jenkins-local/jenkins-agent/workspace/Generic-Unit-Test-
> > > DPDK/dpdk/build/app/test/dpdk-test
> > > -l 0-15 --file-prefix=func_reentrancy_autotest
> > > --- stdout ---
> > > RTE>>func_reentrancy_autotest
> > > Func-ReEnt CASE 0: eal init once PASS ring create/lookup: common
> > > object allocated 2 times (should be 1) Func- ReEnt CASE 1: ring
> > > create/lookup FAIL Test Failed
> > > RTE>>
> > > --- stderr ---
> > >
> > >
> > > I guess, this is what happens:
> > >
> > > main lcore                          worker lcore 1              ...
> > > worker lcore X
> > >                                     enters ring_create_lookup()
> > >
> > > enters ring_create_lookup()
> > > rte_eal_wait_lcore(worker lcore 1);
> > >                                     leaves ring_create_lookup()
> > > ring_clean(worker lcore 1);
> > >
> > > leaves ring_create_lookup()
> > >
> > > There is no synchronisation point for the main lcore to know the
> > > worker lcores are finished invoking the func callback.
> > > With this patch, the "common" object is freed by the main lcore
> > > *potentially* before some workers start trying to create it.
> > > And we end up with multiple workers successfully creating this
> > > object, hence the obj_count being incremented.
> > >
> > >
> > > --
> > > David Marchand
> >
> > I think add rte_eal_mp_wait_lcore() like below can ensure the lcores
> > to free objects after all func callback finished.
> > Shall do the change in next version.
> >
> >         RTE_LCORE_FOREACH_WORKER(lcore_id) {
> >                 if (cores == 1)
> >                         break;
> >                 cores--;
> >                 rte_eal_remote_launch(pt_case->func, pt_case->arg, lcore_id);
> >         }
> >         rte_atomic32_set(&synchro, 1);
> >         if (pt_case->func(pt_case->arg) < 0)
> >                 ret = -1;
> >
> > +       rte_eal_mp_wait_lcore();
> >
> >         cores = cores_save;
> >         RTE_LCORE_FOREACH_WORKER(lcore_id) {
> >                 if (cores == 1)
> >                         break;
> >                 cores--;
> > -               if (rte_eal_wait_lcore(lcore_id) < 0)
> > -                       ret = -1;
> >                 if (pt_case->clean != NULL)
> >                         pt_case->clean(lcore_id);
> >         }
> 
> Using mp_wait_lcore, the test can't tell if a lcore returned an error after
> executing the passed callback.
> 
> An alternative is to split the current loop to first have the per lcore
> rte_eal_wait_lcore() calls + ret code check, and then a second loop calls the
> clean() callback.
> 
> 
> --
> David Marchand

Hi, David, thanks for your suggestion, will send a new version with this modification.

--
Joyce 


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

* [dpdk-dev] [PATCH v4] test/func_reentrancy: free memzones after creating test
  2021-07-28  7:33 [dpdk-dev] [PATCH v1] test/func_reentrancy: free memzones after creating test case Joyce Kong
                   ` (2 preceding siblings ...)
  2021-08-20  8:12 ` [dpdk-dev] [PATCH v3] test/func_reentrancy: free memzones after creating test Joyce Kong
@ 2021-08-23  2:57 ` Joyce Kong
  2021-09-16 15:04   ` Olivier Matz
  3 siblings, 1 reply; 12+ messages in thread
From: Joyce Kong @ 2021-08-23  2:57 UTC (permalink / raw)
  To: david.marchand, anatoly.burakov, olivier.matz, andrew.rybchenko,
	yipeng1.wang, sameh.gobriel, bruce.richardson,
	vladimir.medvedkin, konstantin.ananyev, honnappa.nagarahalli,
	ruifeng.wang
  Cc: dev, nd, stable

Function reentrancy test limits maximum number of iterations
simultaneously, however it doesn't free the 'fr_test_once'
memzones after the fact, so introduce freeing 'fr_test_once'
in ring/mempool/hash/fbk/lpm_clean.

Meanwhile, add the missing free for test case on main thread.

Fixes: 104a92bd026f ("app: add reentrancy tests")
Fixes: 995eec619024 ("test: clean up memory for function reentrancy test")
Cc: stable@dpdk.org

Signed-off-by: Joyce Kong <joyce.kong@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
---
v3/v4:
  Add the synchronization between the main lcore and worker
  lcores to ensure lcores to free "common object" after all
  func callback finished. <David Marchand>

v2:
  Add the discription about adding missing frees for the
  main thread in the commit log. <Olivier Matz>

v1:
  The initial version.
---
 app/test/test_func_reentrancy.c | 30 +++++++++++++++++++++++-------
 1 file changed, 23 insertions(+), 7 deletions(-)

diff --git a/app/test/test_func_reentrancy.c b/app/test/test_func_reentrancy.c
index 231c99a9eb..6bb46c6130 100644
--- a/app/test/test_func_reentrancy.c
+++ b/app/test/test_func_reentrancy.c
@@ -89,6 +89,10 @@ ring_clean(unsigned int lcore_id)
 	char ring_name[MAX_STRING_SIZE];
 	int i;
 
+	rp = rte_ring_lookup("fr_test_once");
+	if (rp != NULL)
+		rte_ring_free(rp);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(ring_name, sizeof(ring_name),
 				"fr_test_%d_%d", lcore_id, i);
@@ -148,7 +152,10 @@ mempool_clean(unsigned int lcore_id)
 	char mempool_name[MAX_STRING_SIZE];
 	int i;
 
-	/* verify all ring created successful */
+	mp = rte_mempool_lookup("fr_test_once");
+	if (mp != NULL)
+		rte_mempool_free(mp);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(mempool_name, sizeof(mempool_name), "fr_test_%d_%d",
 			 lcore_id, i);
@@ -208,6 +215,10 @@ hash_clean(unsigned lcore_id)
 	struct rte_hash *handle;
 	int i;
 
+	handle = rte_hash_find_existing("fr_test_once");
+	if (handle != NULL)
+		rte_hash_free(handle);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(hash_name, sizeof(hash_name), "fr_test_%d_%d",  lcore_id, i);
 
@@ -272,6 +283,10 @@ fbk_clean(unsigned lcore_id)
 	struct rte_fbk_hash_table *handle;
 	int i;
 
+	handle = rte_fbk_hash_find_existing("fr_test_once");
+	if (handle != NULL)
+		rte_fbk_hash_free(handle);
+
 	for (i = 0; i < MAX_ITER_MULTI; i++) {
 		snprintf(fbk_name, sizeof(fbk_name), "fr_test_%d_%d",  lcore_id, i);
 
@@ -338,6 +353,10 @@ lpm_clean(unsigned int lcore_id)
 	struct rte_lpm *lpm;
 	int i;
 
+	lpm = rte_lpm_find_existing("fr_test_once");
+	if (lpm != NULL)
+		rte_lpm_free(lpm);
+
 	for (i = 0; i < MAX_LPM_ITER_TIMES; i++) {
 		snprintf(lpm_name, sizeof(lpm_name), "fr_test_%d_%d",  lcore_id, i);
 
@@ -420,8 +439,7 @@ launch_test(struct test_case *pt_case)
 {
 	int ret = 0;
 	unsigned lcore_id;
-	unsigned cores_save = rte_lcore_count();
-	unsigned cores = RTE_MIN(cores_save, MAX_LCORES);
+	unsigned cores = RTE_MIN(rte_lcore_count(), MAX_LCORES);
 	unsigned count;
 
 	if (pt_case->func == NULL)
@@ -442,14 +460,12 @@ launch_test(struct test_case *pt_case)
 	if (pt_case->func(pt_case->arg) < 0)
 		ret = -1;
 
-	cores = cores_save;
 	RTE_LCORE_FOREACH_WORKER(lcore_id) {
-		if (cores == 1)
-			break;
-		cores--;
 		if (rte_eal_wait_lcore(lcore_id) < 0)
 			ret = -1;
+	}
 
+	RTE_LCORE_FOREACH(lcore_id) {
 		if (pt_case->clean != NULL)
 			pt_case->clean(lcore_id);
 	}
-- 
2.17.1


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

* Re: [dpdk-dev] [PATCH v4] test/func_reentrancy: free memzones after creating test
  2021-08-23  2:57 ` [dpdk-dev] [PATCH v4] " Joyce Kong
@ 2021-09-16 15:04   ` Olivier Matz
  2021-09-16 15:09     ` David Marchand
  0 siblings, 1 reply; 12+ messages in thread
From: Olivier Matz @ 2021-09-16 15:04 UTC (permalink / raw)
  To: Joyce Kong
  Cc: david.marchand, anatoly.burakov, andrew.rybchenko, yipeng1.wang,
	sameh.gobriel, bruce.richardson, vladimir.medvedkin,
	konstantin.ananyev, honnappa.nagarahalli, ruifeng.wang, dev, nd,
	stable

On Sun, Aug 22, 2021 at 09:57:40PM -0500, Joyce Kong wrote:
> Function reentrancy test limits maximum number of iterations
> simultaneously, however it doesn't free the 'fr_test_once'
> memzones after the fact, so introduce freeing 'fr_test_once'
> in ring/mempool/hash/fbk/lpm_clean.
> 
> Meanwhile, add the missing free for test case on main thread.
> 
> Fixes: 104a92bd026f ("app: add reentrancy tests")
> Fixes: 995eec619024 ("test: clean up memory for function reentrancy test")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Feifei Wang <feifei.wang2@arm.com>

Reviewed-by: Olivier Matz <olivier.matz@6wind.com>

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

* Re: [dpdk-dev] [PATCH v4] test/func_reentrancy: free memzones after creating test
  2021-09-16 15:04   ` Olivier Matz
@ 2021-09-16 15:09     ` David Marchand
  0 siblings, 0 replies; 12+ messages in thread
From: David Marchand @ 2021-09-16 15:09 UTC (permalink / raw)
  To: Joyce Kong
  Cc: Olivier Matz, Burakov, Anatoly, Andrew Rybchenko, Wang, Yipeng1,
	Gobriel, Sameh, Bruce Richardson, Vladimir Medvedkin, Ananyev,
	Konstantin, Honnappa Nagarahalli,
	Ruifeng Wang (Arm Technology China),
	dev, nd, dpdk stable, Aaron Conole

On Thu, Sep 16, 2021 at 5:04 PM Olivier Matz <olivier.matz@6wind.com> wrote:
>
> On Sun, Aug 22, 2021 at 09:57:40PM -0500, Joyce Kong wrote:
> > Function reentrancy test limits maximum number of iterations
> > simultaneously, however it doesn't free the 'fr_test_once'
> > memzones after the fact, so introduce freeing 'fr_test_once'
> > in ring/mempool/hash/fbk/lpm_clean.
> >
> > Meanwhile, add the missing free for test case on main thread.
> >
> > Fixes: 104a92bd026f ("app: add reentrancy tests")
> > Fixes: 995eec619024 ("test: clean up memory for function reentrancy test")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Joyce Kong <joyce.kong@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Feifei Wang <feifei.wang2@arm.com>
> Reviewed-by: Olivier Matz <olivier.matz@6wind.com>
Acked-by: David Marchand <david.marchand@redhat.com>

Applied, thanks.


-- 
David Marchand


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

end of thread, other threads:[~2021-09-16 15:09 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-28  7:33 [dpdk-dev] [PATCH v1] test/func_reentrancy: free memzones after creating test case Joyce Kong
2021-07-30 11:27 ` Olivier Matz
2021-07-31  8:42   ` Joyce Kong
2021-07-31 12:03 ` [dpdk-dev] [PATCH v2] " Joyce Kong
2021-08-17  8:17   ` David Marchand
2021-08-20  7:57     ` Joyce Kong
2021-08-20  8:52       ` David Marchand
2021-08-23  2:25         ` Joyce Kong
2021-08-20  8:12 ` [dpdk-dev] [PATCH v3] test/func_reentrancy: free memzones after creating test Joyce Kong
2021-08-23  2:57 ` [dpdk-dev] [PATCH v4] " Joyce Kong
2021-09-16 15:04   ` Olivier Matz
2021-09-16 15:09     ` 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).