[dpdk-dev,v2,01/18] eal: introduce dtor macros

Message ID ec844df869b0bef9a4b64d07adae14ee35cbfa6a.1521652453.git.gaetan.rivet@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

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

Commit Message

Gaëtan Rivet March 21, 2018, 5:15 p.m. UTC
  Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/include/rte_common.h | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
  

Comments

Neil Horman March 22, 2018, 11:35 a.m. UTC | #1
On Wed, Mar 21, 2018 at 06:15:22PM +0100, Gaetan Rivet wrote:
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  lib/librte_eal/common/include/rte_common.h | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> index c7803e41c..500fc3adb 100644
> --- a/lib/librte_eal/common/include/rte_common.h
> +++ b/lib/librte_eal/common/include/rte_common.h
> @@ -105,6 +105,29 @@ static void __attribute__((constructor, used)) func(void)
>  static void __attribute__((constructor(prio), used)) func(void)
>  
>  /**
> + * Run after main() with high priority.
> + *
> + * The destructor will be run *before* prioritized destructors.
> + *
> + * @param func
> + *   Destructor function name.
> + */
> +#define RTE_FINI(func) \
> +static void __attribute__((destructor, used)) func(void)
> +
> +/**
> + * Run after main() with low priority.
> + *
> + * @param func
> + *   Destructor function name.
> + * @param prio
> + *   Priority number must be above 100.
> + *   Lowest number is the last to run.
> + */
> +#define RTE_FINI_PRIO(func, prio) \
> +static void __attribute__((destructor(prio), used)) func(void)
> +
> +/**
If you need to require that priority be above 100, you probably want to create a
BUILD_BUG type macro to enforce that.

Neil

>   * Force a function to be inlined
>   */
>  #define __rte_always_inline inline __attribute__((always_inline))
> -- 
> 2.11.0
> 
>
  
Neil Horman March 22, 2018, 1:51 p.m. UTC | #2
On Wed, Mar 21, 2018 at 06:15:22PM +0100, Gaetan Rivet wrote:
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  lib/librte_eal/common/include/rte_common.h | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> index c7803e41c..500fc3adb 100644
> --- a/lib/librte_eal/common/include/rte_common.h
> +++ b/lib/librte_eal/common/include/rte_common.h
> @@ -105,6 +105,29 @@ static void __attribute__((constructor, used)) func(void)
>  static void __attribute__((constructor(prio), used)) func(void)
>  
>  /**
> + * Run after main() with high priority.
> + *
> + * The destructor will be run *before* prioritized destructors.
> + *
> + * @param func
> + *   Destructor function name.
> + */
> +#define RTE_FINI(func) \
> +static void __attribute__((destructor, used)) func(void)
> +
> +/**
> + * Run after main() with low priority.
> + *
> + * @param func
> + *   Destructor function name.
> + * @param prio
> + *   Priority number must be above 100.
> + *   Lowest number is the last to run.
> + */
> +#define RTE_FINI_PRIO(func, prio) \
> +static void __attribute__((destructor(prio), used)) func(void)
> +
> +/**
Additionally, it might be nice to further abstract the destructor priority to
fixed levels so that people aren't always trying to guess what the right magic
number should be.  I.e. create several destructor macros of the form:
RTE_FINI_<NAME>

Where name is a meaningfull term like FINAL, PMD, EARLY, or some such set that
implies a priority value encoded within the macro definition.  That would also
eliminate the need to create a BUILD BUG macro if the priority was specified to
be too low

Neil

>   * Force a function to be inlined
>   */
>  #define __rte_always_inline inline __attribute__((always_inline))
> -- 
> 2.11.0
> 
>
  
Gaëtan Rivet March 22, 2018, 3:56 p.m. UTC | #3
On Thu, Mar 22, 2018 at 09:51:14AM -0400, Neil Horman wrote:
> On Wed, Mar 21, 2018 at 06:15:22PM +0100, Gaetan Rivet wrote:
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> >  lib/librte_eal/common/include/rte_common.h | 23 +++++++++++++++++++++++
> >  1 file changed, 23 insertions(+)
> > 
> > diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> > index c7803e41c..500fc3adb 100644
> > --- a/lib/librte_eal/common/include/rte_common.h
> > +++ b/lib/librte_eal/common/include/rte_common.h
> > @@ -105,6 +105,29 @@ static void __attribute__((constructor, used)) func(void)
> >  static void __attribute__((constructor(prio), used)) func(void)
> >  
> >  /**
> > + * Run after main() with high priority.
> > + *
> > + * The destructor will be run *before* prioritized destructors.
> > + *
> > + * @param func
> > + *   Destructor function name.
> > + */
> > +#define RTE_FINI(func) \
> > +static void __attribute__((destructor, used)) func(void)
> > +
> > +/**
> > + * Run after main() with low priority.
> > + *
> > + * @param func
> > + *   Destructor function name.
> > + * @param prio
> > + *   Priority number must be above 100.
> > + *   Lowest number is the last to run.
> > + */
> > +#define RTE_FINI_PRIO(func, prio) \
> > +static void __attribute__((destructor(prio), used)) func(void)
> > +
> > +/**
> Additionally, it might be nice to further abstract the destructor priority to
> fixed levels so that people aren't always trying to guess what the right magic
> number should be.  I.e. create several destructor macros of the form:
> RTE_FINI_<NAME>
> 
> Where name is a meaningfull term like FINAL, PMD, EARLY, or some such set that
> implies a priority value encoded within the macro definition.  That would also
> eliminate the need to create a BUILD BUG macro if the priority was specified to
> be too low
> 
> Neil
> 

Good idea, and I agree that we need helpers on this.
Not sure about _FINAL and _EARLY however.

I will propose a patch as a response to this mail, let me know if that's
what you had in mind.
  
Neil Horman March 23, 2018, 12:38 a.m. UTC | #4
On Thu, Mar 22, 2018 at 04:56:43PM +0100, Gaëtan Rivet wrote:
> On Thu, Mar 22, 2018 at 09:51:14AM -0400, Neil Horman wrote:
> > On Wed, Mar 21, 2018 at 06:15:22PM +0100, Gaetan Rivet wrote:
> > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > ---
> > >  lib/librte_eal/common/include/rte_common.h | 23 +++++++++++++++++++++++
> > >  1 file changed, 23 insertions(+)
> > > 
> > > diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> > > index c7803e41c..500fc3adb 100644
> > > --- a/lib/librte_eal/common/include/rte_common.h
> > > +++ b/lib/librte_eal/common/include/rte_common.h
> > > @@ -105,6 +105,29 @@ static void __attribute__((constructor, used)) func(void)
> > >  static void __attribute__((constructor(prio), used)) func(void)
> > >  
> > >  /**
> > > + * Run after main() with high priority.
> > > + *
> > > + * The destructor will be run *before* prioritized destructors.
> > > + *
> > > + * @param func
> > > + *   Destructor function name.
> > > + */
> > > +#define RTE_FINI(func) \
> > > +static void __attribute__((destructor, used)) func(void)
> > > +
> > > +/**
> > > + * Run after main() with low priority.
> > > + *
> > > + * @param func
> > > + *   Destructor function name.
> > > + * @param prio
> > > + *   Priority number must be above 100.
> > > + *   Lowest number is the last to run.
> > > + */
> > > +#define RTE_FINI_PRIO(func, prio) \
> > > +static void __attribute__((destructor(prio), used)) func(void)
> > > +
> > > +/**
> > Additionally, it might be nice to further abstract the destructor priority to
> > fixed levels so that people aren't always trying to guess what the right magic
> > number should be.  I.e. create several destructor macros of the form:
> > RTE_FINI_<NAME>
> > 
> > Where name is a meaningfull term like FINAL, PMD, EARLY, or some such set that
> > implies a priority value encoded within the macro definition.  That would also
> > eliminate the need to create a BUILD BUG macro if the priority was specified to
> > be too low
> > 
> > Neil
> > 
> 
> Good idea, and I agree that we need helpers on this.
> Not sure about _FINAL and _EARLY however.
> 
Yeah, I don't like those either, but I couldn't think of any other priority
suffixes off the top of my head

FWIW, the kernel creates these macros to register initcalls:
#define pure_initcall(fn)               __define_initcall(fn, 0)
#define core_initcall(fn)               __define_initcall(fn, 1)
#define core_initcall_sync(fn)          __define_initcall(fn, 1s)
#define postcore_initcall(fn)           __define_initcall(fn, 2)
#define postcore_initcall_sync(fn)      __define_initcall(fn, 2s)
#define arch_initcall(fn)               __define_initcall(fn, 3)
#define arch_initcall_sync(fn)          __define_initcall(fn, 3s)
#define subsys_initcall(fn)             __define_initcall(fn, 4)
#define subsys_initcall_sync(fn)        __define_initcall(fn, 4s)
#define fs_initcall(fn)                 __define_initcall(fn, 5)
#define fs_initcall_sync(fn)            __define_initcall(fn, 5s)
#define rootfs_initcall(fn)             __define_initcall(fn, rootfs)
#define device_initcall(fn)             __define_initcall(fn, 6)
#define device_initcall_sync(fn)        __define_initcall(fn, 6s)
#define late_initcall(fn)               __define_initcall(fn, 7)
#define late_initcall_sync(fn)          __define_initcall(fn, 7s)

Its not an exact match, but you could probably borrow from that somewhat.

> I will propose a patch as a response to this mail, let me know if that's
> what you had in mind.
That was my hope yes, thanks!
Neil

> 
> -- 
> Gaëtan Rivet
> 6WIND
>
  

Patch

diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
index c7803e41c..500fc3adb 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -105,6 +105,29 @@  static void __attribute__((constructor, used)) func(void)
 static void __attribute__((constructor(prio), used)) func(void)
 
 /**
+ * Run after main() with high priority.
+ *
+ * The destructor will be run *before* prioritized destructors.
+ *
+ * @param func
+ *   Destructor function name.
+ */
+#define RTE_FINI(func) \
+static void __attribute__((destructor, used)) func(void)
+
+/**
+ * Run after main() with low priority.
+ *
+ * @param func
+ *   Destructor function name.
+ * @param prio
+ *   Priority number must be above 100.
+ *   Lowest number is the last to run.
+ */
+#define RTE_FINI_PRIO(func, prio) \
+static void __attribute__((destructor(prio), used)) func(void)
+
+/**
  * Force a function to be inlined
  */
 #define __rte_always_inline inline __attribute__((always_inline))