[v5,6/6] eal/alarm: introduce alarm fini routine

Message ID 20211022204934.132186-7-hkalra@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: David Marchand
Headers
Series make rte_intr_handle internal |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/github-robot: build success github build: passed
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-x86_64-compile-testing fail Testing issues
ci/iol-x86_64-unit-testing success Testing PASS

Commit Message

Harman Kalra Oct. 22, 2021, 8:49 p.m. UTC
  Implementing alarm cleanup routine, where the memory allocated
for interrupt instance can be freed.

Signed-off-by: Harman Kalra <hkalra@marvell.com>
---
 lib/eal/common/eal_private.h | 11 +++++++++++
 lib/eal/freebsd/eal.c        |  1 +
 lib/eal/freebsd/eal_alarm.c  |  7 +++++++
 lib/eal/linux/eal.c          |  1 +
 lib/eal/linux/eal_alarm.c    |  7 +++++++
 5 files changed, 27 insertions(+)
  

Comments

Dmitry Kozlyuk Oct. 22, 2021, 11:33 p.m. UTC | #1
2021-10-23 02:19 (UTC+0530), Harman Kalra:
> Implementing alarm cleanup routine, where the memory allocated
> for interrupt instance can be freed.
> 
> Signed-off-by: Harman Kalra <hkalra@marvell.com>
> ---
>  lib/eal/common/eal_private.h | 11 +++++++++++
>  lib/eal/freebsd/eal.c        |  1 +
>  lib/eal/freebsd/eal_alarm.c  |  7 +++++++
>  lib/eal/linux/eal.c          |  1 +
>  lib/eal/linux/eal_alarm.c    |  7 +++++++
>  5 files changed, 27 insertions(+)
> 
> diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
> index 86dab1f057..7fb9bc1324 100644
> --- a/lib/eal/common/eal_private.h
> +++ b/lib/eal/common/eal_private.h
> @@ -163,6 +163,17 @@ int rte_eal_intr_init(void);
>   */
>  int rte_eal_alarm_init(void);
>  
> +/**
> + * Init alarm mechanism. This is to allow a callback be called after
> + * specific time.
> + *
> + * This function is private to EAL.
> + *
> + * @return
> + *  0 on success, negative on error
> + */
> +void rte_eal_alarm_fini(void);
> +

The description should say the opposite.

>  /**
>   * Function is to check if the kernel module(like, vfio, vfio_iommu_type1,
>   * etc.) loaded.
> diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
> index 56a60f13e9..535ea687ca 100644
> --- a/lib/eal/freebsd/eal.c
> +++ b/lib/eal/freebsd/eal.c
> @@ -977,6 +977,7 @@ rte_eal_cleanup(void)
>  	rte_eal_memory_detach();
>  	rte_trace_save();
>  	eal_trace_fini();
> +	rte_eal_alarm_fini();

Alarms are initialized after tracing, so they should be finalized after.

>  	eal_cleanup_config(internal_conf);
>  	return 0;
>  }
> diff --git a/lib/eal/freebsd/eal_alarm.c b/lib/eal/freebsd/eal_alarm.c
> index 1f2ea58175..cf706f609f 100644
> --- a/lib/eal/freebsd/eal_alarm.c
> +++ b/lib/eal/freebsd/eal_alarm.c
> @@ -46,6 +46,13 @@ static rte_spinlock_t alarm_list_lk = RTE_SPINLOCK_INITIALIZER;
>  static struct rte_intr_handle *intr_handle;
>  static void eal_alarm_callback(void *arg);
>  
> +void
> +rte_eal_alarm_fini(void)
> +{
> +	if (intr_handle)
> +		rte_intr_instance_free(intr_handle);
> +}
> +
>  int
>  rte_eal_alarm_init(void)
>  {
> diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
> index 0d0fc66668..806158f297 100644
> --- a/lib/eal/linux/eal.c
> +++ b/lib/eal/linux/eal.c
> @@ -1370,6 +1370,7 @@ rte_eal_cleanup(void)
>  	rte_eal_memory_detach();
>  	rte_trace_save();
>  	eal_trace_fini();
> +	rte_eal_alarm_fini();

Likewise.

>  	eal_cleanup_config(internal_conf);
>  	return 0;
>  }
> diff --git a/lib/eal/linux/eal_alarm.c b/lib/eal/linux/eal_alarm.c
> index 3d4307686c..c3a3c943a8 100644
> --- a/lib/eal/linux/eal_alarm.c
> +++ b/lib/eal/linux/eal_alarm.c
> @@ -58,6 +58,13 @@ static struct rte_intr_handle *intr_handle;
>  static int handler_registered = 0;
>  static void eal_alarm_callback(void *arg);
>  
> +void
> +rte_eal_alarm_fini(void)
> +{
> +	if (intr_handle)
> +		rte_intr_instance_free(intr_handle);
> +}
> +
>  int
>  rte_eal_alarm_init(void)
>  {

That being fixed,
Acked-by: Dmitry Kozlyuk <dmitry.kozliuk@gmail.com>
  
Dmitry Kozlyuk Oct. 22, 2021, 11:37 p.m. UTC | #2
2021-10-23 02:33 (UTC+0300), Dmitry Kozlyuk:
> 2021-10-23 02:19 (UTC+0530), Harman Kalra:
> [...]
> >  /**
> >   * Function is to check if the kernel module(like, vfio, vfio_iommu_type1,
> >   * etc.) loaded.
> > diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
> > index 56a60f13e9..535ea687ca 100644
> > --- a/lib/eal/freebsd/eal.c
> > +++ b/lib/eal/freebsd/eal.c
> > @@ -977,6 +977,7 @@ rte_eal_cleanup(void)
> >  	rte_eal_memory_detach();
> >  	rte_trace_save();
> >  	eal_trace_fini();
> > +	rte_eal_alarm_fini();  
> 
> Alarms are initialized after tracing, so they should be finalized after.

"...finalized before", sorry.
  

Patch

diff --git a/lib/eal/common/eal_private.h b/lib/eal/common/eal_private.h
index 86dab1f057..7fb9bc1324 100644
--- a/lib/eal/common/eal_private.h
+++ b/lib/eal/common/eal_private.h
@@ -163,6 +163,17 @@  int rte_eal_intr_init(void);
  */
 int rte_eal_alarm_init(void);
 
+/**
+ * Init alarm mechanism. This is to allow a callback be called after
+ * specific time.
+ *
+ * This function is private to EAL.
+ *
+ * @return
+ *  0 on success, negative on error
+ */
+void rte_eal_alarm_fini(void);
+
 /**
  * Function is to check if the kernel module(like, vfio, vfio_iommu_type1,
  * etc.) loaded.
diff --git a/lib/eal/freebsd/eal.c b/lib/eal/freebsd/eal.c
index 56a60f13e9..535ea687ca 100644
--- a/lib/eal/freebsd/eal.c
+++ b/lib/eal/freebsd/eal.c
@@ -977,6 +977,7 @@  rte_eal_cleanup(void)
 	rte_eal_memory_detach();
 	rte_trace_save();
 	eal_trace_fini();
+	rte_eal_alarm_fini();
 	eal_cleanup_config(internal_conf);
 	return 0;
 }
diff --git a/lib/eal/freebsd/eal_alarm.c b/lib/eal/freebsd/eal_alarm.c
index 1f2ea58175..cf706f609f 100644
--- a/lib/eal/freebsd/eal_alarm.c
+++ b/lib/eal/freebsd/eal_alarm.c
@@ -46,6 +46,13 @@  static rte_spinlock_t alarm_list_lk = RTE_SPINLOCK_INITIALIZER;
 static struct rte_intr_handle *intr_handle;
 static void eal_alarm_callback(void *arg);
 
+void
+rte_eal_alarm_fini(void)
+{
+	if (intr_handle)
+		rte_intr_instance_free(intr_handle);
+}
+
 int
 rte_eal_alarm_init(void)
 {
diff --git a/lib/eal/linux/eal.c b/lib/eal/linux/eal.c
index 0d0fc66668..806158f297 100644
--- a/lib/eal/linux/eal.c
+++ b/lib/eal/linux/eal.c
@@ -1370,6 +1370,7 @@  rte_eal_cleanup(void)
 	rte_eal_memory_detach();
 	rte_trace_save();
 	eal_trace_fini();
+	rte_eal_alarm_fini();
 	eal_cleanup_config(internal_conf);
 	return 0;
 }
diff --git a/lib/eal/linux/eal_alarm.c b/lib/eal/linux/eal_alarm.c
index 3d4307686c..c3a3c943a8 100644
--- a/lib/eal/linux/eal_alarm.c
+++ b/lib/eal/linux/eal_alarm.c
@@ -58,6 +58,13 @@  static struct rte_intr_handle *intr_handle;
 static int handler_registered = 0;
 static void eal_alarm_callback(void *arg);
 
+void
+rte_eal_alarm_fini(void)
+{
+	if (intr_handle)
+		rte_intr_instance_free(intr_handle);
+}
+
 int
 rte_eal_alarm_init(void)
 {