diff mbox series

[v8,06/10] common/mlx5: avoid using class constructor priority

Message ID 20200723200910.376581-7-parav@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Raslan Darawsheh
Headers
Series Improve mlx5 PMD driver framework for multiple classes |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail apply issues

Commit Message

Parav Pandit July 23, 2020, 8:09 p.m. UTC 
  mlx5_common is shared library between mlx5 net, VDPA and regex PMD.
It is better to use common initialization helper instead of using
RTE_INIT_CLASS priority.

Signed-off-by: Parav Pandit <parav@mellanox.com>
---
 drivers/common/mlx5/mlx5_common.c               | 13 +++++++++++--
 drivers/common/mlx5/mlx5_common.h               |  3 +++
 drivers/common/mlx5/rte_common_mlx5_version.map |  1 +
 drivers/net/mlx5/mlx5.c                         |  1 +
 drivers/regex/mlx5/mlx5_regex.c                 |  1 +
 drivers/vdpa/mlx5/mlx5_vdpa.c                   |  1 +
 6 files changed, 18 insertions(+), 2 deletions(-)

Comments

David Marchand July 24, 2020, 1:45 p.m. UTC | #1 
On Thu, Jul 23, 2020 at 10:10 PM Parav Pandit <parav@mellanox.com> wrote:
>
> mlx5_common is shared library between mlx5 net, VDPA and regex PMD.
> It is better to use common initialization helper instead of using
> RTE_INIT_CLASS priority.

RTE_INIT_CLASS does not exist, I suppose you meant RTE_PRIORITY_CLASS.

Suggested-by: David Marchand <david.marchand@redhat.com>

> Signed-off-by: Parav Pandit <parav@mellanox.com>
> ---
>  drivers/common/mlx5/mlx5_common.c               | 13 +++++++++++--
>  drivers/common/mlx5/mlx5_common.h               |  3 +++
>  drivers/common/mlx5/rte_common_mlx5_version.map |  1 +
>  drivers/net/mlx5/mlx5.c                         |  1 +
>  drivers/regex/mlx5/mlx5_regex.c                 |  1 +
>  drivers/vdpa/mlx5/mlx5_vdpa.c                   |  1 +
>  6 files changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/common/mlx5/mlx5_common.c b/drivers/common/mlx5/mlx5_common.c
> index 1a67a1b16..2b336bb2d 100644
> --- a/drivers/common/mlx5/mlx5_common.c
> +++ b/drivers/common/mlx5/mlx5_common.c
> @@ -86,12 +86,21 @@ RTE_INIT_PRIO(mlx5_log_init, LOG)
>                 rte_log_set_level(mlx5_common_logtype, RTE_LOG_NOTICE);
>  }
>
> +static bool mlx5_common_initialized;
> +
>  /**
> - * Initialization routine for run-time dependency on glue library.
> + * One time innitialization routine for run-time dependency on glue library
> + * for multiple PMDs. Each mlx5 PMD that depends on mlx5_common module,
> + * must invoke in its constructor.
>   */
> -RTE_INIT_PRIO(mlx5_glue_init, CLASS)
> +void
> +mlx5_common_init(void)
>  {
> +       if (mlx5_common_initialized)
> +               return;
> +
>         mlx5_glue_constructor();
> +       mlx5_common_initialized = true;
>  }
>
>  /**
> diff --git a/drivers/common/mlx5/mlx5_common.h b/drivers/common/mlx5/mlx5_common.h
> index a811eb6c9..ebe4e9ced 100644
> --- a/drivers/common/mlx5/mlx5_common.h
> +++ b/drivers/common/mlx5/mlx5_common.h
> @@ -260,4 +260,7 @@ int32_t mlx5_release_dbr(struct mlx5_dbr_page_list *head, uint32_t umem_id,
>                          uint64_t offset);
>  extern uint8_t haswell_broadwell_cpu;
>
> +__rte_internal
> +void mlx5_common_init(void);
> +
>  #endif /* RTE_PMD_MLX5_COMMON_H_ */
> diff --git a/drivers/common/mlx5/rte_common_mlx5_version.map b/drivers/common/mlx5/rte_common_mlx5_version.map
> index 132a0695f..65f25252a 100644
> --- a/drivers/common/mlx5/rte_common_mlx5_version.map
> +++ b/drivers/common/mlx5/rte_common_mlx5_version.map
> @@ -3,6 +3,7 @@ INTERNAL {
>
>         mlx5_class_get;
>
> +       mlx5_common_init;
>         mlx5_common_verbs_reg_mr;
>         mlx5_common_verbs_dereg_mr;
>
> diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
> index 647ada339..037703d2e 100644
> --- a/drivers/net/mlx5/mlx5.c
> +++ b/drivers/net/mlx5/mlx5.c
> @@ -2111,6 +2111,7 @@ RTE_LOG_REGISTER(mlx5_logtype, pmd.net.mlx5, NOTICE)
>   */
>  RTE_INIT(rte_mlx5_pmd_init)
>  {
> +       mlx5_common_init();
>         /* Build the static tables for Verbs conversion. */
>         mlx5_set_ptype_table();
>         mlx5_set_cksum_table();
> diff --git a/drivers/regex/mlx5/mlx5_regex.c b/drivers/regex/mlx5/mlx5_regex.c
> index 36ae9f809..4e0367052 100644
> --- a/drivers/regex/mlx5/mlx5_regex.c
> +++ b/drivers/regex/mlx5/mlx5_regex.c
> @@ -258,6 +258,7 @@ static struct rte_pci_driver mlx5_regex_driver = {
>
>  RTE_INIT(rte_mlx5_regex_init)
>  {
> +       mlx5_common_init();
>         if (mlx5_glue)
>                 rte_pci_register(&mlx5_regex_driver);
>  }
> diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.c b/drivers/vdpa/mlx5/mlx5_vdpa.c
> index 67e77b11a..85dbcf956 100644
> --- a/drivers/vdpa/mlx5/mlx5_vdpa.c
> +++ b/drivers/vdpa/mlx5/mlx5_vdpa.c
> @@ -846,6 +846,7 @@ RTE_LOG_REGISTER(mlx5_vdpa_logtype, pmd.vdpa.mlx5, NOTICE)
>   */
>  RTE_INIT(rte_mlx5_vdpa_init)
>  {
> +       mlx5_common_init();
>         if (mlx5_glue)
>                 rte_pci_register(&mlx5_vdpa_driver);
>  }
> --
> 2.25.4
>

Reviewed-by: David Marchand <david.marchand@redhat.com>
Parav Pandit July 24, 2020, 1:47 p.m. UTC | #2 
> From: David Marchand <david.marchand@redhat.com>
> Sent: Friday, July 24, 2020 7:15 PM
> To: Parav Pandit <parav@mellanox.com>
> Cc: dev <dev@dpdk.org>; Gaetan Rivet <grive@u256.net>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; Thomas Monjalon <thomas@monjalon.net>;
> Raslan Darawsheh <rasland@mellanox.com>; Ori Kam
> <orika@mellanox.com>; Matan Azrad <matan@mellanox.com>; Joyce Kong
> <joyce.kong@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v8 06/10] common/mlx5: avoid using class
> constructor priority
> 
> On Thu, Jul 23, 2020 at 10:10 PM Parav Pandit <parav@mellanox.com> wrote:
> >
> > mlx5_common is shared library between mlx5 net, VDPA and regex PMD.
> > It is better to use common initialization helper instead of using
> > RTE_INIT_CLASS priority.
> 
> RTE_INIT_CLASS does not exist, I suppose you meant RTE_PRIORITY_CLASS.
> 
You are right.
Correcting it and adding below tags.

> Suggested-by: David Marchand <david.marchand@redhat.com>
> 
> > Signed-off-by: Parav Pandit <parav@mellanox.com>
> > ---
> >  drivers/common/mlx5/mlx5_common.c               | 13 +++++++++++--
> >  drivers/common/mlx5/mlx5_common.h               |  3 +++
> >  drivers/common/mlx5/rte_common_mlx5_version.map |  1 +
> >  drivers/net/mlx5/mlx5.c                         |  1 +
> >  drivers/regex/mlx5/mlx5_regex.c                 |  1 +
> >  drivers/vdpa/mlx5/mlx5_vdpa.c                   |  1 +
> >  6 files changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/common/mlx5/mlx5_common.c
> > b/drivers/common/mlx5/mlx5_common.c
> > index 1a67a1b16..2b336bb2d 100644
> > --- a/drivers/common/mlx5/mlx5_common.c
> > +++ b/drivers/common/mlx5/mlx5_common.c
> > @@ -86,12 +86,21 @@ RTE_INIT_PRIO(mlx5_log_init, LOG)
> >                 rte_log_set_level(mlx5_common_logtype,
> > RTE_LOG_NOTICE);  }
> >
> > +static bool mlx5_common_initialized;
> > +
> >  /**
> > - * Initialization routine for run-time dependency on glue library.
> > + * One time innitialization routine for run-time dependency on glue
> > + library
> > + * for multiple PMDs. Each mlx5 PMD that depends on mlx5_common
> > + module,
> > + * must invoke in its constructor.
> >   */
> > -RTE_INIT_PRIO(mlx5_glue_init, CLASS)
> > +void
> > +mlx5_common_init(void)
> >  {
> > +       if (mlx5_common_initialized)
> > +               return;
> > +
> >         mlx5_glue_constructor();
> > +       mlx5_common_initialized = true;
> >  }
> >
> >  /**
> > diff --git a/drivers/common/mlx5/mlx5_common.h
> > b/drivers/common/mlx5/mlx5_common.h
> > index a811eb6c9..ebe4e9ced 100644
> > --- a/drivers/common/mlx5/mlx5_common.h
> > +++ b/drivers/common/mlx5/mlx5_common.h
> > @@ -260,4 +260,7 @@ int32_t mlx5_release_dbr(struct
> mlx5_dbr_page_list *head, uint32_t umem_id,
> >                          uint64_t offset);  extern uint8_t
> > haswell_broadwell_cpu;
> >
> > +__rte_internal
> > +void mlx5_common_init(void);
> > +
> >  #endif /* RTE_PMD_MLX5_COMMON_H_ */
> > diff --git a/drivers/common/mlx5/rte_common_mlx5_version.map
> > b/drivers/common/mlx5/rte_common_mlx5_version.map
> > index 132a0695f..65f25252a 100644
> > --- a/drivers/common/mlx5/rte_common_mlx5_version.map
> > +++ b/drivers/common/mlx5/rte_common_mlx5_version.map
> > @@ -3,6 +3,7 @@ INTERNAL {
> >
> >         mlx5_class_get;
> >
> > +       mlx5_common_init;
> >         mlx5_common_verbs_reg_mr;
> >         mlx5_common_verbs_dereg_mr;
> >
> > diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c index
> > 647ada339..037703d2e 100644
> > --- a/drivers/net/mlx5/mlx5.c
> > +++ b/drivers/net/mlx5/mlx5.c
> > @@ -2111,6 +2111,7 @@ RTE_LOG_REGISTER(mlx5_logtype,
> pmd.net.mlx5, NOTICE)
> >   */
> >  RTE_INIT(rte_mlx5_pmd_init)
> >  {
> > +       mlx5_common_init();
> >         /* Build the static tables for Verbs conversion. */
> >         mlx5_set_ptype_table();
> >         mlx5_set_cksum_table();
> > diff --git a/drivers/regex/mlx5/mlx5_regex.c
> > b/drivers/regex/mlx5/mlx5_regex.c index 36ae9f809..4e0367052 100644
> > --- a/drivers/regex/mlx5/mlx5_regex.c
> > +++ b/drivers/regex/mlx5/mlx5_regex.c
> > @@ -258,6 +258,7 @@ static struct rte_pci_driver mlx5_regex_driver = {
> >
> >  RTE_INIT(rte_mlx5_regex_init)
> >  {
> > +       mlx5_common_init();
> >         if (mlx5_glue)
> >                 rte_pci_register(&mlx5_regex_driver);
> >  }
> > diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.c
> > b/drivers/vdpa/mlx5/mlx5_vdpa.c index 67e77b11a..85dbcf956 100644
> > --- a/drivers/vdpa/mlx5/mlx5_vdpa.c
> > +++ b/drivers/vdpa/mlx5/mlx5_vdpa.c
> > @@ -846,6 +846,7 @@ RTE_LOG_REGISTER(mlx5_vdpa_logtype,
> pmd.vdpa.mlx5, NOTICE)
> >   */
> >  RTE_INIT(rte_mlx5_vdpa_init)
> >  {
> > +       mlx5_common_init();
> >         if (mlx5_glue)
> >                 rte_pci_register(&mlx5_vdpa_driver);
> >  }
> > --
> > 2.25.4
> >
> 
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> 
> 
> --
> David Marchand
diff mbox series

Patch

diff --git a/drivers/common/mlx5/mlx5_common.c b/drivers/common/mlx5/mlx5_common.c
index 1a67a1b16..2b336bb2d 100644
--- a/drivers/common/mlx5/mlx5_common.c
+++ b/drivers/common/mlx5/mlx5_common.c
@@ -86,12 +86,21 @@  RTE_INIT_PRIO(mlx5_log_init, LOG)
 		rte_log_set_level(mlx5_common_logtype, RTE_LOG_NOTICE);
 }
 
+static bool mlx5_common_initialized;
+
 /**
- * Initialization routine for run-time dependency on glue library.
+ * One time innitialization routine for run-time dependency on glue library
+ * for multiple PMDs. Each mlx5 PMD that depends on mlx5_common module,
+ * must invoke in its constructor.
  */
-RTE_INIT_PRIO(mlx5_glue_init, CLASS)
+void
+mlx5_common_init(void)
 {
+	if (mlx5_common_initialized)
+		return;
+
 	mlx5_glue_constructor();
+	mlx5_common_initialized = true;
 }
 
 /**
diff --git a/drivers/common/mlx5/mlx5_common.h b/drivers/common/mlx5/mlx5_common.h
index a811eb6c9..ebe4e9ced 100644
--- a/drivers/common/mlx5/mlx5_common.h
+++ b/drivers/common/mlx5/mlx5_common.h
@@ -260,4 +260,7 @@  int32_t mlx5_release_dbr(struct mlx5_dbr_page_list *head, uint32_t umem_id,
 			 uint64_t offset);
 extern uint8_t haswell_broadwell_cpu;
 
+__rte_internal
+void mlx5_common_init(void);
+
 #endif /* RTE_PMD_MLX5_COMMON_H_ */
diff --git a/drivers/common/mlx5/rte_common_mlx5_version.map b/drivers/common/mlx5/rte_common_mlx5_version.map
index 132a0695f..65f25252a 100644
--- a/drivers/common/mlx5/rte_common_mlx5_version.map
+++ b/drivers/common/mlx5/rte_common_mlx5_version.map
@@ -3,6 +3,7 @@  INTERNAL {
 
 	mlx5_class_get;
 
+	mlx5_common_init;
 	mlx5_common_verbs_reg_mr;
 	mlx5_common_verbs_dereg_mr;
 
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 647ada339..037703d2e 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -2111,6 +2111,7 @@  RTE_LOG_REGISTER(mlx5_logtype, pmd.net.mlx5, NOTICE)
  */
 RTE_INIT(rte_mlx5_pmd_init)
 {
+	mlx5_common_init();
 	/* Build the static tables for Verbs conversion. */
 	mlx5_set_ptype_table();
 	mlx5_set_cksum_table();
diff --git a/drivers/regex/mlx5/mlx5_regex.c b/drivers/regex/mlx5/mlx5_regex.c
index 36ae9f809..4e0367052 100644
--- a/drivers/regex/mlx5/mlx5_regex.c
+++ b/drivers/regex/mlx5/mlx5_regex.c
@@ -258,6 +258,7 @@  static struct rte_pci_driver mlx5_regex_driver = {
 
 RTE_INIT(rte_mlx5_regex_init)
 {
+	mlx5_common_init();
 	if (mlx5_glue)
 		rte_pci_register(&mlx5_regex_driver);
 }
diff --git a/drivers/vdpa/mlx5/mlx5_vdpa.c b/drivers/vdpa/mlx5/mlx5_vdpa.c
index 67e77b11a..85dbcf956 100644
--- a/drivers/vdpa/mlx5/mlx5_vdpa.c
+++ b/drivers/vdpa/mlx5/mlx5_vdpa.c
@@ -846,6 +846,7 @@  RTE_LOG_REGISTER(mlx5_vdpa_logtype, pmd.vdpa.mlx5, NOTICE)
  */
 RTE_INIT(rte_mlx5_vdpa_init)
 {
+	mlx5_common_init();
 	if (mlx5_glue)
 		rte_pci_register(&mlx5_vdpa_driver);
 }