[1/2] lib/metrics: fix to reset the init flag

Message ID 20200513103629.8436-1-hemant.agrawal@nxp.com
State New
Headers show
Series
  • [1/2] lib/metrics: fix to reset the init flag
Related show

Commit Message

Hemant Agrawal May 13, 2020, 10:36 a.m.
metrics_initialized shall be reset in deinit function
This is currently causing issue in running
metrics_autotest mulutiple times

Fixes: 07c1b6925b65 ("telemetry: invert dependency on metrics library")

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>

---
 lib/librte_metrics/rte_metrics.c | 2 ++
 1 file changed, 2 insertions(+)

-- 
2.17.1

Comments

David Marchand May 19, 2020, 9:31 a.m. | #1
On Wed, May 13, 2020 at 12:39 PM Hemant Agrawal <hemant.agrawal@nxp.com> wrote:
>

> metrics_initialized shall be reset in deinit function

> This is currently causing issue in running

> metrics_autotest mulutiple times

>

> Fixes: 07c1b6925b65 ("telemetry: invert dependency on metrics library")

>

> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>

> ---

>  lib/librte_metrics/rte_metrics.c | 2 ++

>  1 file changed, 2 insertions(+)

>

> diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c

> index e07670219..f570cf226 100644

> --- a/lib/librte_metrics/rte_metrics.c

> +++ b/lib/librte_metrics/rte_metrics.c

> @@ -96,6 +96,8 @@ rte_metrics_deinit(void)

>         stats = memzone->addr;

>         memset(stats, 0, sizeof(struct rte_metrics_data_s));

>

> +       metrics_initialized = 0;

> +

>         return rte_memzone_free(memzone);


Should this flag be reset only if rte_memzone_free succeeds?


-- 
David Marchand
Hemant Agrawal May 19, 2020, 9:50 a.m. | #2
> On Wed, May 13, 2020 at 12:39 PM Hemant Agrawal

> <hemant.agrawal@nxp.com> wrote:

> >

> > metrics_initialized shall be reset in deinit function This is

> > currently causing issue in running metrics_autotest mulutiple times

> >

> > Fixes: 07c1b6925b65 ("telemetry: invert dependency on metrics

> > library")

> >

> > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>

> > ---

> >  lib/librte_metrics/rte_metrics.c | 2 ++

> >  1 file changed, 2 insertions(+)

> >

> > diff --git a/lib/librte_metrics/rte_metrics.c

> > b/lib/librte_metrics/rte_metrics.c

> > index e07670219..f570cf226 100644

> > --- a/lib/librte_metrics/rte_metrics.c

> > +++ b/lib/librte_metrics/rte_metrics.c

> > @@ -96,6 +96,8 @@ rte_metrics_deinit(void)

> >         stats = memzone->addr;

> >         memset(stats, 0, sizeof(struct rte_metrics_data_s));

> >

> > +       metrics_initialized = 0;

> > +

> >         return rte_memzone_free(memzone);

> 

> Should this flag be reset only if rte_memzone_free succeeds?

> 

[Hemant]  I thought about it but I did not do it for following reasons.
1. If the memzone is not freed,  It will not be initialized next time due to following check in init routine.
 	memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
	if (memzone != NULL)
		return;
2. Most applications have very weak error handling in de-init/cleanup parts. 

Having said that. It can be changed to do it only on the success of rte_memzone_free
> 

> --

> David Marchand

Patch

diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
index e07670219..f570cf226 100644
--- a/lib/librte_metrics/rte_metrics.c
+++ b/lib/librte_metrics/rte_metrics.c
@@ -96,6 +96,8 @@  rte_metrics_deinit(void)
 	stats = memzone->addr;
 	memset(stats, 0, sizeof(struct rte_metrics_data_s));
 
+	metrics_initialized = 0;
+
 	return rte_memzone_free(memzone);
 
 }