[v2,1/2] metrics: new API to deinitialise metrics library

Message ID 1551434828-3518-1-git-send-email-hkalra@marvell.com (mailing list archive)
State Changes Requested, archived
Delegated to: Thomas Monjalon
Headers
Series [v2,1/2] metrics: new API to deinitialise metrics library |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/intel-Performance-Testing success Performance Testing PASS

Commit Message

Harman Kalra March 1, 2019, 10:07 a.m. UTC
  Once the library usage is over, it must be deinitialized which
will free the shared memory reserved during initialization.

Fixes: observed an issue while running 'metrics_autotest'
continuously without quiting. For the first run 'metrics_autotest'
passes all test cases but second run onwards first test case
fails because metrics library is already initialized during
first run.
Cc: stable@dpdk.org

Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
v2:
* Adding stable@dpdk.org into cc as this patch falls between bug fix
and new feature.

 doc/guides/prog_guide/metrics_lib.rst      | 14 ++++++++++++++
 lib/librte_metrics/rte_metrics.c           | 20 ++++++++++++++++++++
 lib/librte_metrics/rte_metrics.h           | 17 +++++++++++++++++
 lib/librte_metrics/rte_metrics_version.map |  6 ++++++
 4 files changed, 57 insertions(+)
  

Comments

Pattan, Reshma June 27, 2019, 10:59 a.m. UTC | #1
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Harman Kalra
> Sent: Friday, March 1, 2019 10:08 AM
> To: Horton, Remy <remy.horton@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Harman Kalra <hkalra@marvell.com>
> Subject: [dpdk-dev] [PATCH v2 1/2] metrics: new API to deinitialise metrics
> library
> 
> Once the library usage is over, it must be deinitialized which will free the
> shared memory reserved during initialization.
> 
> Fixes: observed an issue while running 'metrics_autotest'
> continuously without quiting. For the first run 'metrics_autotest'
> passes all test cases but second run onwards first test case fails because
> metrics library is already initialized during first run.
> Cc: stable@dpdk.org
> 
> Signed-off-by: Harman Kalra <hkalra@marvell.com>
> ---
> v2:
> * Adding stable@dpdk.org into cc as this patch falls between bug fix and
> new feature.
> 
>  doc/guides/prog_guide/metrics_lib.rst      | 14 ++++++++++++++
>  lib/librte_metrics/rte_metrics.c           | 20 ++++++++++++++++++++
>  lib/librte_metrics/rte_metrics.h           | 17 +++++++++++++++++
>  lib/librte_metrics/rte_metrics_version.map |  6 ++++++
>  4 files changed, 57 insertions(+)
> 
> diff --git a/doc/guides/prog_guide/metrics_lib.rst
> b/doc/guides/prog_guide/metrics_lib.rst
> index e68e4e743..08e107df3 100644
> --- a/doc/guides/prog_guide/metrics_lib.rst
> +++ b/doc/guides/prog_guide/metrics_lib.rst
> @@ -154,6 +154,20 @@ print out all metrics for a given port:
>      }
> 
> 
> +Deinitialising the library
> +------------------------

1)
The underline should be same length as heading. Below is the error from "make doc-guides-html"

sphinx processing guides-html...
metrics_lib.rst:158: WARNING: Title underline too short.

Deinitialising the library
------------------------
metrics_lib.rst:158: WARNING: Title underline too short.


2) Need to fix below
./devtools/check-git-log.sh
Wrong tag:
        Fixes: observed an issue while running 'metrics_autotest'
Wrong 'Fixes' reference:
        Fixes: observed an issue while running 'metrics_autotest'

If you want to add Fixes line it should be of below form
Fixes: <commitd> ("commit heading")
Ex: Fixes: d7a0da3c0043 ("mempool/octeontx2: add fast path mempool ops")


Otherwise, 

Tested-by : Reshma Pattan <reshma.pattan@intel.com>
Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
Acked-by: Reshma Pattan <reshma.pattan@intel.com>

Please keep these tags for next versions,  otherwise it can easily miss from being applied.

Thanks,
Reshma
  
Pattan, Reshma June 27, 2019, 11:03 a.m. UTC | #2
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Harman Kalra
> Sent: Friday, March 1, 2019 10:08 AM
> To: Horton, Remy <remy.horton@intel.com>; Burakov, Anatoly
> <anatoly.burakov@intel.com>; Kovacevic, Marko
> <marko.kovacevic@intel.com>; Mcnamara, John
> <john.mcnamara@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org; Harman Kalra <hkalra@marvell.com>
> Subject: [dpdk-dev] [PATCH v2 1/2] metrics: new API to deinitialise metrics
> library
> 
> Once the library usage is over, it must be deinitialized which will free the
> shared memory reserved during initialization.
> 
> Fixes: observed an issue while running 'metrics_autotest'

 Need to fix this
./devtools/check-git-log.sh
Wrong tag:
        Fixes: observed an issue while running 'metrics_autotest'
Wrong 'Fixes' reference:
        Fixes: observed an issue while running 'metrics_autotest'

No need of of Fixes line, so just correct the sentence,
 
Tested-by : Reshma Pattan <reshma.pattan@intel.com>
Reviewed-by: Reshma Pattan <reshma.pattan@intel.com>
Acked-by: Reshma Pattan <reshma.pattan@intel.com>

Please keep these tags for next versions.

Thanks,
Reshma
  

Patch

diff --git a/doc/guides/prog_guide/metrics_lib.rst b/doc/guides/prog_guide/metrics_lib.rst
index e68e4e743..08e107df3 100644
--- a/doc/guides/prog_guide/metrics_lib.rst
+++ b/doc/guides/prog_guide/metrics_lib.rst
@@ -154,6 +154,20 @@  print out all metrics for a given port:
     }
 
 
+Deinitialising the library
+------------------------
+
+Once the library usage is done, it must be deinitialized by calling
+``rte_metrics_deinit()`` which will free the shared memory reserved
+during initialization.
+
+.. code-block:: c
+
+    err = rte_metrics_deinit(void);
+
+If the return value is negative, it means deinitialization failed.
+This function **must** be called from a primary process.
+
 Bit-rate statistics library
 ---------------------------
 
diff --git a/lib/librte_metrics/rte_metrics.c b/lib/librte_metrics/rte_metrics.c
index 99a96b651..0c816a1fc 100644
--- a/lib/librte_metrics/rte_metrics.c
+++ b/lib/librte_metrics/rte_metrics.c
@@ -76,6 +76,26 @@  rte_metrics_init(int socket_id)
 	rte_spinlock_init(&stats->lock);
 }
 
+int __rte_experimental
+rte_metrics_deinit(void)
+{
+	struct rte_metrics_data_s *stats;
+	const struct rte_memzone *memzone;
+
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+		return -EINVAL;
+
+	memzone = rte_memzone_lookup(RTE_METRICS_MEMZONE_NAME);
+	if (memzone == NULL)
+		return -EIO;
+
+	stats = memzone->addr;
+	memset(stats, 0, sizeof(struct rte_metrics_data_s));
+
+	return rte_memzone_free(memzone);
+
+}
+
 int
 rte_metrics_reg_name(const char *name)
 {
diff --git a/lib/librte_metrics/rte_metrics.h b/lib/librte_metrics/rte_metrics.h
index 67a60fadd..0957a94b6 100644
--- a/lib/librte_metrics/rte_metrics.h
+++ b/lib/librte_metrics/rte_metrics.h
@@ -24,6 +24,7 @@ 
 #define _RTE_METRICS_H_
 
 #include <stdint.h>
+#include <rte_compat.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -80,6 +81,22 @@  struct rte_metric_value {
  */
 void rte_metrics_init(int socket_id);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Deinitialize metric module. This function must be called from
+ * a primary process after all the metrics usage is over, to
+ *  release the shared memory.
+ *
+ * @return
+ *  -EINVAL - invalid parameter.
+ *  -EIO: Error, unable to access metrics shared memory
+ *    (rte_metrics_init() not called)
+ *  0 - success
+ */
+int __rte_experimental rte_metrics_deinit(void);
+
 /**
  * Register a metric, making it available as a reporting parameter.
  *
diff --git a/lib/librte_metrics/rte_metrics_version.map b/lib/librte_metrics/rte_metrics_version.map
index 4c5234cd1..6ac99a44a 100644
--- a/lib/librte_metrics/rte_metrics_version.map
+++ b/lib/librte_metrics/rte_metrics_version.map
@@ -11,3 +11,9 @@  DPDK_17.05 {
 
 	local: *;
 };
+
+EXPERIMENTAL {
+	global:
+
+	rte_metrics_deinit;
+};