diff mbox

[dpdk-dev,v5,02/21] eal: list acceptable init priorities

Message ID 3c46c7947bec6462e07a6c33e39f680c8d9e688b.1523404469.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 April 11, 2018, 12:04 a.m. UTC 
  Build a central list to quickly see each used priorities for
constructors, allowing to verify that they are both above 100 and in the
proper order.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_eal/common/eal_common_log.c     | 2 +-
 lib/librte_eal/common/include/rte_bus.h    | 2 +-
 lib/librte_eal/common/include/rte_common.h | 8 +++++++-
 3 files changed, 9 insertions(+), 3 deletions(-)

Comments

Neil Horman April 12, 2018, 11:28 a.m. UTC | #1 
On Wed, Apr 11, 2018 at 02:04:03AM +0200, Gaetan Rivet wrote:
> Build a central list to quickly see each used priorities for
> constructors, allowing to verify that they are both above 100 and in the
> proper order.
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> Acked-by: Neil Horman <nhorman@tuxdriver.com>
> Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
>  lib/librte_eal/common/eal_common_log.c     | 2 +-
>  lib/librte_eal/common/include/rte_bus.h    | 2 +-
>  lib/librte_eal/common/include/rte_common.h | 8 +++++++-
>  3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> index a27192620..36b9d6e08 100644
> --- a/lib/librte_eal/common/eal_common_log.c
> +++ b/lib/librte_eal/common/eal_common_log.c
> @@ -260,7 +260,7 @@ static const struct logtype logtype_strings[] = {
>  };
>  
>  /* Logging should be first initializer (before drivers and bus) */
> -RTE_INIT_PRIO(rte_log_init, 101);
> +RTE_INIT_PRIO(rte_log_init, LOG);
>  static void
>  rte_log_init(void)
>  {
> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> index 6fb08341a..eb9eded4e 100644
> --- a/lib/librte_eal/common/include/rte_bus.h
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -325,7 +325,7 @@ enum rte_iova_mode rte_bus_get_iommu_class(void);
>   * The constructor has higher priority than PMD constructors.
>   */
>  #define RTE_REGISTER_BUS(nm, bus) \
> -RTE_INIT_PRIO(businitfn_ ##nm, 110); \
> +RTE_INIT_PRIO(businitfn_ ##nm, BUS); \
>  static void businitfn_ ##nm(void) \
>  {\
>  	(bus).name = RTE_STR(nm);\
> diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> index 6c5bc5a76..8f04518f7 100644
> --- a/lib/librte_eal/common/include/rte_common.h
> +++ b/lib/librte_eal/common/include/rte_common.h
> @@ -81,6 +81,12 @@ typedef uint16_t unaligned_uint16_t;
>   */
>  #define RTE_SET_USED(x) (void)(x)
>  
> +#define RTE_PRIORITY_LOG 101
> +#define RTE_PRIORITY_BUS 110
> +
> +#define RTE_PRIO(prio) \
> +	RTE_PRIORITY_ ## prio
> +
>  /**
>   * Run function before main() with low priority.
>   *
> @@ -102,7 +108,7 @@ static void __attribute__((constructor, used)) func(void)
>   *   Lowest number is the first to run.
>   */
>  #define RTE_INIT_PRIO(func, prio) \
> -static void __attribute__((constructor(prio), used)) func(void)
> +static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)
>  
It just occured to me, that perhaps you should add a RTE_PRORITY_LAST priority,
and redefine RTE_INIT to RTE_INIT_PRIO(func, RTE_PRIORITY_LAST) for clarity.  I
presume that constructors with no explicit priority run last, but the gcc
manual doesn't explicitly say that.  It would be a heck of a bug to track down
if somehow unprioritized constructors ran early.

Neil

>  /**
>   * Force a function to be inlined
> -- 
> 2.11.0
> 
>
Gaëtan Rivet April 12, 2018, 9:57 p.m. UTC | #2 
Hello Neil,

On Thu, Apr 12, 2018 at 07:28:26AM -0400, Neil Horman wrote:
> On Wed, Apr 11, 2018 at 02:04:03AM +0200, Gaetan Rivet wrote:
> > Build a central list to quickly see each used priorities for
> > constructors, allowing to verify that they are both above 100 and in the
> > proper order.
> > 
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> > ---
> >  lib/librte_eal/common/eal_common_log.c     | 2 +-
> >  lib/librte_eal/common/include/rte_bus.h    | 2 +-
> >  lib/librte_eal/common/include/rte_common.h | 8 +++++++-
> >  3 files changed, 9 insertions(+), 3 deletions(-)
> > 
> > diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> > index a27192620..36b9d6e08 100644
> > --- a/lib/librte_eal/common/eal_common_log.c
> > +++ b/lib/librte_eal/common/eal_common_log.c
> > @@ -260,7 +260,7 @@ static const struct logtype logtype_strings[] = {
> >  };
> >  
> >  /* Logging should be first initializer (before drivers and bus) */
> > -RTE_INIT_PRIO(rte_log_init, 101);
> > +RTE_INIT_PRIO(rte_log_init, LOG);
> >  static void
> >  rte_log_init(void)
> >  {
> > diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> > index 6fb08341a..eb9eded4e 100644
> > --- a/lib/librte_eal/common/include/rte_bus.h
> > +++ b/lib/librte_eal/common/include/rte_bus.h
> > @@ -325,7 +325,7 @@ enum rte_iova_mode rte_bus_get_iommu_class(void);
> >   * The constructor has higher priority than PMD constructors.
> >   */
> >  #define RTE_REGISTER_BUS(nm, bus) \
> > -RTE_INIT_PRIO(businitfn_ ##nm, 110); \
> > +RTE_INIT_PRIO(businitfn_ ##nm, BUS); \
> >  static void businitfn_ ##nm(void) \
> >  {\
> >  	(bus).name = RTE_STR(nm);\
> > diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> > index 6c5bc5a76..8f04518f7 100644
> > --- a/lib/librte_eal/common/include/rte_common.h
> > +++ b/lib/librte_eal/common/include/rte_common.h
> > @@ -81,6 +81,12 @@ typedef uint16_t unaligned_uint16_t;
> >   */
> >  #define RTE_SET_USED(x) (void)(x)
> >  
> > +#define RTE_PRIORITY_LOG 101
> > +#define RTE_PRIORITY_BUS 110
> > +
> > +#define RTE_PRIO(prio) \
> > +	RTE_PRIORITY_ ## prio
> > +
> >  /**
> >   * Run function before main() with low priority.
> >   *
> > @@ -102,7 +108,7 @@ static void __attribute__((constructor, used)) func(void)
> >   *   Lowest number is the first to run.
> >   */
> >  #define RTE_INIT_PRIO(func, prio) \
> > -static void __attribute__((constructor(prio), used)) func(void)
> > +static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)
> >  
> It just occured to me, that perhaps you should add a RTE_PRORITY_LAST priority,
> and redefine RTE_INIT to RTE_INIT_PRIO(func, RTE_PRIORITY_LAST) for clarity.  I
> presume that constructors with no explicit priority run last, but the gcc
> manual doesn't explicitly say that.  It would be a heck of a bug to track down
> if somehow unprioritized constructors ran early.
> 
> Neil
> 

While certainly poorly documented, the behavior is well-defined. I don't see
a situation where the bug you describe could arise.

Adding RTE_PRIORITY_LAST is pretty harmless, but I'm not sure it's
justified to add it. If you still think it is useful, I will do it.

I'd be curious to hear if anyone has had issues of this kind.
Neil Horman April 13, 2018, 11:42 a.m. UTC | #3 
On Thu, Apr 12, 2018 at 11:57:47PM +0200, Gaëtan Rivet wrote:
> Hello Neil,
> 
> On Thu, Apr 12, 2018 at 07:28:26AM -0400, Neil Horman wrote:
> > On Wed, Apr 11, 2018 at 02:04:03AM +0200, Gaetan Rivet wrote:
> > > Build a central list to quickly see each used priorities for
> > > constructors, allowing to verify that they are both above 100 and in the
> > > proper order.
> > > 
> > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > > Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> > > ---
> > >  lib/librte_eal/common/eal_common_log.c     | 2 +-
> > >  lib/librte_eal/common/include/rte_bus.h    | 2 +-
> > >  lib/librte_eal/common/include/rte_common.h | 8 +++++++-
> > >  3 files changed, 9 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> > > index a27192620..36b9d6e08 100644
> > > --- a/lib/librte_eal/common/eal_common_log.c
> > > +++ b/lib/librte_eal/common/eal_common_log.c
> > > @@ -260,7 +260,7 @@ static const struct logtype logtype_strings[] = {
> > >  };
> > >  
> > >  /* Logging should be first initializer (before drivers and bus) */
> > > -RTE_INIT_PRIO(rte_log_init, 101);
> > > +RTE_INIT_PRIO(rte_log_init, LOG);
> > >  static void
> > >  rte_log_init(void)
> > >  {
> > > diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> > > index 6fb08341a..eb9eded4e 100644
> > > --- a/lib/librte_eal/common/include/rte_bus.h
> > > +++ b/lib/librte_eal/common/include/rte_bus.h
> > > @@ -325,7 +325,7 @@ enum rte_iova_mode rte_bus_get_iommu_class(void);
> > >   * The constructor has higher priority than PMD constructors.
> > >   */
> > >  #define RTE_REGISTER_BUS(nm, bus) \
> > > -RTE_INIT_PRIO(businitfn_ ##nm, 110); \
> > > +RTE_INIT_PRIO(businitfn_ ##nm, BUS); \
> > >  static void businitfn_ ##nm(void) \
> > >  {\
> > >  	(bus).name = RTE_STR(nm);\
> > > diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> > > index 6c5bc5a76..8f04518f7 100644
> > > --- a/lib/librte_eal/common/include/rte_common.h
> > > +++ b/lib/librte_eal/common/include/rte_common.h
> > > @@ -81,6 +81,12 @@ typedef uint16_t unaligned_uint16_t;
> > >   */
> > >  #define RTE_SET_USED(x) (void)(x)
> > >  
> > > +#define RTE_PRIORITY_LOG 101
> > > +#define RTE_PRIORITY_BUS 110
> > > +
> > > +#define RTE_PRIO(prio) \
> > > +	RTE_PRIORITY_ ## prio
> > > +
> > >  /**
> > >   * Run function before main() with low priority.
> > >   *
> > > @@ -102,7 +108,7 @@ static void __attribute__((constructor, used)) func(void)
> > >   *   Lowest number is the first to run.
> > >   */
> > >  #define RTE_INIT_PRIO(func, prio) \
> > > -static void __attribute__((constructor(prio), used)) func(void)
> > > +static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)
> > >  
> > It just occured to me, that perhaps you should add a RTE_PRORITY_LAST priority,
> > and redefine RTE_INIT to RTE_INIT_PRIO(func, RTE_PRIORITY_LAST) for clarity.  I
> > presume that constructors with no explicit priority run last, but the gcc
> > manual doesn't explicitly say that.  It would be a heck of a bug to track down
> > if somehow unprioritized constructors ran early.
> > 
> > Neil
> > 
> 
> While certainly poorly documented, the behavior is well-defined. I don't see
> a situation where the bug you describe could arise.
> 
> Adding RTE_PRIORITY_LAST is pretty harmless, but I'm not sure it's
> justified to add it. If you still think it is useful, I will do it.
> 
It was more just a way to unify the macros is all, probably not important.

> I'd be curious to hear if anyone has had issues of this kind.
> 
I've not had any, but I was suprised to see that the gcc manual didn't
explicitly call out the implied priority of unprioritized constructors

Neil

> -- 
> Gaëtan Rivet
> 6WIND
>
Shreyansh Jain April 13, 2018, 12:52 p.m. UTC | #4 
On Friday 13 April 2018 05:12 PM, Neil Horman wrote:
> On Thu, Apr 12, 2018 at 11:57:47PM +0200, Gaëtan Rivet wrote:
>> Hello Neil,
>>
>> On Thu, Apr 12, 2018 at 07:28:26AM -0400, Neil Horman wrote:
>>> On Wed, Apr 11, 2018 at 02:04:03AM +0200, Gaetan Rivet wrote:
>>>> Build a central list to quickly see each used priorities for
>>>> constructors, allowing to verify that they are both above 100 and in the
>>>> proper order.
>>>>
>>>> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
>>>> Acked-by: Neil Horman <nhorman@tuxdriver.com>
>>>> Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>>> ---
>>>>   lib/librte_eal/common/eal_common_log.c     | 2 +-
>>>>   lib/librte_eal/common/include/rte_bus.h    | 2 +-
>>>>   lib/librte_eal/common/include/rte_common.h | 8 +++++++-
>>>>   3 files changed, 9 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
>>>> index a27192620..36b9d6e08 100644
>>>> --- a/lib/librte_eal/common/eal_common_log.c
>>>> +++ b/lib/librte_eal/common/eal_common_log.c
>>>> @@ -260,7 +260,7 @@ static const struct logtype logtype_strings[] = {
>>>>   };
>>>>   
>>>>   /* Logging should be first initializer (before drivers and bus) */
>>>> -RTE_INIT_PRIO(rte_log_init, 101);
>>>> +RTE_INIT_PRIO(rte_log_init, LOG);
>>>>   static void
>>>>   rte_log_init(void)
>>>>   {
>>>> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
>>>> index 6fb08341a..eb9eded4e 100644
>>>> --- a/lib/librte_eal/common/include/rte_bus.h
>>>> +++ b/lib/librte_eal/common/include/rte_bus.h
>>>> @@ -325,7 +325,7 @@ enum rte_iova_mode rte_bus_get_iommu_class(void);
>>>>    * The constructor has higher priority than PMD constructors.
>>>>    */
>>>>   #define RTE_REGISTER_BUS(nm, bus) \
>>>> -RTE_INIT_PRIO(businitfn_ ##nm, 110); \
>>>> +RTE_INIT_PRIO(businitfn_ ##nm, BUS); \
>>>>   static void businitfn_ ##nm(void) \
>>>>   {\
>>>>   	(bus).name = RTE_STR(nm);\
>>>> diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
>>>> index 6c5bc5a76..8f04518f7 100644
>>>> --- a/lib/librte_eal/common/include/rte_common.h
>>>> +++ b/lib/librte_eal/common/include/rte_common.h
>>>> @@ -81,6 +81,12 @@ typedef uint16_t unaligned_uint16_t;
>>>>    */
>>>>   #define RTE_SET_USED(x) (void)(x)
>>>>   
>>>> +#define RTE_PRIORITY_LOG 101
>>>> +#define RTE_PRIORITY_BUS 110
>>>> +
>>>> +#define RTE_PRIO(prio) \
>>>> +	RTE_PRIORITY_ ## prio
>>>> +
>>>>   /**
>>>>    * Run function before main() with low priority.
>>>>    *
>>>> @@ -102,7 +108,7 @@ static void __attribute__((constructor, used)) func(void)
>>>>    *   Lowest number is the first to run.
>>>>    */
>>>>   #define RTE_INIT_PRIO(func, prio) \
>>>> -static void __attribute__((constructor(prio), used)) func(void)
>>>> +static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)
>>>>   
>>> It just occured to me, that perhaps you should add a RTE_PRORITY_LAST priority,
>>> and redefine RTE_INIT to RTE_INIT_PRIO(func, RTE_PRIORITY_LAST) for clarity.  I
>>> presume that constructors with no explicit priority run last, but the gcc
>>> manual doesn't explicitly say that.  It would be a heck of a bug to track down
>>> if somehow unprioritized constructors ran early.
>>>
>>> Neil
>>>
>>
>> While certainly poorly documented, the behavior is well-defined. I don't see
>> a situation where the bug you describe could arise.
>>
>> Adding RTE_PRIORITY_LAST is pretty harmless, but I'm not sure it's
>> justified to add it. If you still think it is useful, I will do it.
>>
> It was more just a way to unify the macros is all, probably not important.
> 
>> I'd be curious to hear if anyone has had issues of this kind.
>>
> I've not had any, but I was suprised to see that the gcc manual didn't
> explicitly call out the implied priority of unprioritized constructors

I (tried to) looked into the gcc code base. It seems that when priority 
is not defined, DEFAULT_INIT_PRIORITY 65536, is used.

--->8--- gcc/collect2.c ---
   /* Extract init_p number from ctor/dtor name.  */
   pri = atoi (name + pos);
   return pri ? pri : DEFAULT_INIT_PRIORITY;
--->8---

Though, I couldn't find any documentation for this fact - and, I can 
never be confident about gcc code.

I found one of the ARM compiler (clang) does has a policy for using 
non-specified priority as lower than specified priority. [1]

[1] 
https://developer.arm.com/docs/dui0774/latest/compiler-specific-function-variable-and-type-attributes/__attribute__constructorpriority-function-attribute

A specified value for RTE_PRIORITY_LAST is not a bad option - it would 
help in keeping the priorities bound without relying on the unknown of 
priority for unspecified constructors.

> 
> Neil
> 
>> -- 
>> Gaëtan Rivet
>> 6WIND
>>
Gaëtan Rivet April 13, 2018, 12:55 p.m. UTC | #5 
Hi Shreyansh,

On Fri, Apr 13, 2018 at 06:22:43PM +0530, Shreyansh Jain wrote:
> On Friday 13 April 2018 05:12 PM, Neil Horman wrote:
> > On Thu, Apr 12, 2018 at 11:57:47PM +0200, Gaëtan Rivet wrote:
> > > Hello Neil,
> > > 
> > > On Thu, Apr 12, 2018 at 07:28:26AM -0400, Neil Horman wrote:
> > > > On Wed, Apr 11, 2018 at 02:04:03AM +0200, Gaetan Rivet wrote:
> > > > > Build a central list to quickly see each used priorities for
> > > > > constructors, allowing to verify that they are both above 100 and in the
> > > > > proper order.
> > > > > 
> > > > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > > > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > > > > Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> > > > > ---
> > > > >   lib/librte_eal/common/eal_common_log.c     | 2 +-
> > > > >   lib/librte_eal/common/include/rte_bus.h    | 2 +-
> > > > >   lib/librte_eal/common/include/rte_common.h | 8 +++++++-
> > > > >   3 files changed, 9 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> > > > > index a27192620..36b9d6e08 100644
> > > > > --- a/lib/librte_eal/common/eal_common_log.c
> > > > > +++ b/lib/librte_eal/common/eal_common_log.c
> > > > > @@ -260,7 +260,7 @@ static const struct logtype logtype_strings[] = {
> > > > >   };
> > > > >   /* Logging should be first initializer (before drivers and bus) */
> > > > > -RTE_INIT_PRIO(rte_log_init, 101);
> > > > > +RTE_INIT_PRIO(rte_log_init, LOG);
> > > > >   static void
> > > > >   rte_log_init(void)
> > > > >   {
> > > > > diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> > > > > index 6fb08341a..eb9eded4e 100644
> > > > > --- a/lib/librte_eal/common/include/rte_bus.h
> > > > > +++ b/lib/librte_eal/common/include/rte_bus.h
> > > > > @@ -325,7 +325,7 @@ enum rte_iova_mode rte_bus_get_iommu_class(void);
> > > > >    * The constructor has higher priority than PMD constructors.
> > > > >    */
> > > > >   #define RTE_REGISTER_BUS(nm, bus) \
> > > > > -RTE_INIT_PRIO(businitfn_ ##nm, 110); \
> > > > > +RTE_INIT_PRIO(businitfn_ ##nm, BUS); \
> > > > >   static void businitfn_ ##nm(void) \
> > > > >   {\
> > > > >   	(bus).name = RTE_STR(nm);\
> > > > > diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> > > > > index 6c5bc5a76..8f04518f7 100644
> > > > > --- a/lib/librte_eal/common/include/rte_common.h
> > > > > +++ b/lib/librte_eal/common/include/rte_common.h
> > > > > @@ -81,6 +81,12 @@ typedef uint16_t unaligned_uint16_t;
> > > > >    */
> > > > >   #define RTE_SET_USED(x) (void)(x)
> > > > > +#define RTE_PRIORITY_LOG 101
> > > > > +#define RTE_PRIORITY_BUS 110
> > > > > +
> > > > > +#define RTE_PRIO(prio) \
> > > > > +	RTE_PRIORITY_ ## prio
> > > > > +
> > > > >   /**
> > > > >    * Run function before main() with low priority.
> > > > >    *
> > > > > @@ -102,7 +108,7 @@ static void __attribute__((constructor, used)) func(void)
> > > > >    *   Lowest number is the first to run.
> > > > >    */
> > > > >   #define RTE_INIT_PRIO(func, prio) \
> > > > > -static void __attribute__((constructor(prio), used)) func(void)
> > > > > +static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)
> > > > It just occured to me, that perhaps you should add a RTE_PRORITY_LAST priority,
> > > > and redefine RTE_INIT to RTE_INIT_PRIO(func, RTE_PRIORITY_LAST) for clarity.  I
> > > > presume that constructors with no explicit priority run last, but the gcc
> > > > manual doesn't explicitly say that.  It would be a heck of a bug to track down
> > > > if somehow unprioritized constructors ran early.
> > > > 
> > > > Neil
> > > > 
> > > 
> > > While certainly poorly documented, the behavior is well-defined. I don't see
> > > a situation where the bug you describe could arise.
> > > 
> > > Adding RTE_PRIORITY_LAST is pretty harmless, but I'm not sure it's
> > > justified to add it. If you still think it is useful, I will do it.
> > > 
> > It was more just a way to unify the macros is all, probably not important.
> > 
> > > I'd be curious to hear if anyone has had issues of this kind.
> > > 
> > I've not had any, but I was suprised to see that the gcc manual didn't
> > explicitly call out the implied priority of unprioritized constructors
> 
> I (tried to) looked into the gcc code base. It seems that when priority is
> not defined, DEFAULT_INIT_PRIORITY 65536, is used.
> 
> --->8--- gcc/collect2.c ---
>   /* Extract init_p number from ctor/dtor name.  */
>   pri = atoi (name + pos);
>   return pri ? pri : DEFAULT_INIT_PRIORITY;
> --->8---
> 
> Though, I couldn't find any documentation for this fact - and, I can never
> be confident about gcc code.
> 
> I found one of the ARM compiler (clang) does has a policy for using
> non-specified priority as lower than specified priority. [1]
> 
> [1] https://developer.arm.com/docs/dui0774/latest/compiler-specific-function-variable-and-type-attributes/__attribute__constructorpriority-function-attribute
> 
> A specified value for RTE_PRIORITY_LAST is not a bad option - it would help
> in keeping the priorities bound without relying on the unknown of priority
> for unspecified constructors.

This is interesting, thanks for looking up the GCC code.
Ok, unless someone has a strong reason not to, I will add
RTE_PRIORITY_LAST. Not really convinced about it but not
opposed enough either :) .

Regards,
Neil Horman April 14, 2018, 6:45 p.m. UTC | #6 
On Fri, Apr 13, 2018 at 02:55:11PM +0200, Gaëtan Rivet wrote:
> Hi Shreyansh,
> 
> On Fri, Apr 13, 2018 at 06:22:43PM +0530, Shreyansh Jain wrote:
> > On Friday 13 April 2018 05:12 PM, Neil Horman wrote:
> > > On Thu, Apr 12, 2018 at 11:57:47PM +0200, Gaëtan Rivet wrote:
> > > > Hello Neil,
> > > > 
> > > > On Thu, Apr 12, 2018 at 07:28:26AM -0400, Neil Horman wrote:
> > > > > On Wed, Apr 11, 2018 at 02:04:03AM +0200, Gaetan Rivet wrote:
> > > > > > Build a central list to quickly see each used priorities for
> > > > > > constructors, allowing to verify that they are both above 100 and in the
> > > > > > proper order.
> > > > > > 
> > > > > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > > > > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > > > > > Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> > > > > > ---
> > > > > >   lib/librte_eal/common/eal_common_log.c     | 2 +-
> > > > > >   lib/librte_eal/common/include/rte_bus.h    | 2 +-
> > > > > >   lib/librte_eal/common/include/rte_common.h | 8 +++++++-
> > > > > >   3 files changed, 9 insertions(+), 3 deletions(-)
> > > > > > 
> > > > > > diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> > > > > > index a27192620..36b9d6e08 100644
> > > > > > --- a/lib/librte_eal/common/eal_common_log.c
> > > > > > +++ b/lib/librte_eal/common/eal_common_log.c
> > > > > > @@ -260,7 +260,7 @@ static const struct logtype logtype_strings[] = {
> > > > > >   };
> > > > > >   /* Logging should be first initializer (before drivers and bus) */
> > > > > > -RTE_INIT_PRIO(rte_log_init, 101);
> > > > > > +RTE_INIT_PRIO(rte_log_init, LOG);
> > > > > >   static void
> > > > > >   rte_log_init(void)
> > > > > >   {
> > > > > > diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> > > > > > index 6fb08341a..eb9eded4e 100644
> > > > > > --- a/lib/librte_eal/common/include/rte_bus.h
> > > > > > +++ b/lib/librte_eal/common/include/rte_bus.h
> > > > > > @@ -325,7 +325,7 @@ enum rte_iova_mode rte_bus_get_iommu_class(void);
> > > > > >    * The constructor has higher priority than PMD constructors.
> > > > > >    */
> > > > > >   #define RTE_REGISTER_BUS(nm, bus) \
> > > > > > -RTE_INIT_PRIO(businitfn_ ##nm, 110); \
> > > > > > +RTE_INIT_PRIO(businitfn_ ##nm, BUS); \
> > > > > >   static void businitfn_ ##nm(void) \
> > > > > >   {\
> > > > > >   	(bus).name = RTE_STR(nm);\
> > > > > > diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> > > > > > index 6c5bc5a76..8f04518f7 100644
> > > > > > --- a/lib/librte_eal/common/include/rte_common.h
> > > > > > +++ b/lib/librte_eal/common/include/rte_common.h
> > > > > > @@ -81,6 +81,12 @@ typedef uint16_t unaligned_uint16_t;
> > > > > >    */
> > > > > >   #define RTE_SET_USED(x) (void)(x)
> > > > > > +#define RTE_PRIORITY_LOG 101
> > > > > > +#define RTE_PRIORITY_BUS 110
> > > > > > +
> > > > > > +#define RTE_PRIO(prio) \
> > > > > > +	RTE_PRIORITY_ ## prio
> > > > > > +
> > > > > >   /**
> > > > > >    * Run function before main() with low priority.
> > > > > >    *
> > > > > > @@ -102,7 +108,7 @@ static void __attribute__((constructor, used)) func(void)
> > > > > >    *   Lowest number is the first to run.
> > > > > >    */
> > > > > >   #define RTE_INIT_PRIO(func, prio) \
> > > > > > -static void __attribute__((constructor(prio), used)) func(void)
> > > > > > +static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)
> > > > > It just occured to me, that perhaps you should add a RTE_PRORITY_LAST priority,
> > > > > and redefine RTE_INIT to RTE_INIT_PRIO(func, RTE_PRIORITY_LAST) for clarity.  I
> > > > > presume that constructors with no explicit priority run last, but the gcc
> > > > > manual doesn't explicitly say that.  It would be a heck of a bug to track down
> > > > > if somehow unprioritized constructors ran early.
> > > > > 
> > > > > Neil
> > > > > 
> > > > 
> > > > While certainly poorly documented, the behavior is well-defined. I don't see
> > > > a situation where the bug you describe could arise.
> > > > 
> > > > Adding RTE_PRIORITY_LAST is pretty harmless, but I'm not sure it's
> > > > justified to add it. If you still think it is useful, I will do it.
> > > > 
> > > It was more just a way to unify the macros is all, probably not important.
> > > 
> > > > I'd be curious to hear if anyone has had issues of this kind.
> > > > 
> > > I've not had any, but I was suprised to see that the gcc manual didn't
> > > explicitly call out the implied priority of unprioritized constructors
> > 
> > I (tried to) looked into the gcc code base. It seems that when priority is
> > not defined, DEFAULT_INIT_PRIORITY 65536, is used.
> > 
> > --->8--- gcc/collect2.c ---
> >   /* Extract init_p number from ctor/dtor name.  */
> >   pri = atoi (name + pos);
> >   return pri ? pri : DEFAULT_INIT_PRIORITY;
> > --->8---
> > 
> > Though, I couldn't find any documentation for this fact - and, I can never
> > be confident about gcc code.
> > 
> > I found one of the ARM compiler (clang) does has a policy for using
> > non-specified priority as lower than specified priority. [1]
> > 
> > [1] https://developer.arm.com/docs/dui0774/latest/compiler-specific-function-variable-and-type-attributes/__attribute__constructorpriority-function-attribute
> > 
> > A specified value for RTE_PRIORITY_LAST is not a bad option - it would help
> > in keeping the priorities bound without relying on the unknown of priority
> > for unspecified constructors.
> 
> This is interesting, thanks for looking up the GCC code.
> Ok, unless someone has a strong reason not to, I will add
> RTE_PRIORITY_LAST. Not really convinced about it but not
> opposed enough either :) .
> 
I concur.  It sounds like gcc is safe, but clangs priority scheme makes me want
our priorities to be explicit.

Neil

> Regards,
> -- 
> Gaëtan Rivet
> 6WIND
>
Gaëtan Rivet April 15, 2018, 3:13 p.m. UTC | #7 
Hello Neil,

On Sat, Apr 14, 2018 at 02:45:45PM -0400, Neil Horman wrote:
> On Fri, Apr 13, 2018 at 02:55:11PM +0200, Gaëtan Rivet wrote:
> > Hi Shreyansh,
> > 
> > On Fri, Apr 13, 2018 at 06:22:43PM +0530, Shreyansh Jain wrote:
> > > On Friday 13 April 2018 05:12 PM, Neil Horman wrote:
> > > > On Thu, Apr 12, 2018 at 11:57:47PM +0200, Gaëtan Rivet wrote:
> > > > > Hello Neil,
> > > > > 
> > > > > On Thu, Apr 12, 2018 at 07:28:26AM -0400, Neil Horman wrote:
> > > > > > On Wed, Apr 11, 2018 at 02:04:03AM +0200, Gaetan Rivet wrote:
> > > > > > > Build a central list to quickly see each used priorities for
> > > > > > > constructors, allowing to verify that they are both above 100 and in the
> > > > > > > proper order.
> > > > > > > 
> > > > > > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > > > > > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > > > > > > Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> > > > > > > ---
> > > > > > >   lib/librte_eal/common/eal_common_log.c     | 2 +-
> > > > > > >   lib/librte_eal/common/include/rte_bus.h    | 2 +-
> > > > > > >   lib/librte_eal/common/include/rte_common.h | 8 +++++++-
> > > > > > >   3 files changed, 9 insertions(+), 3 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> > > > > > > index a27192620..36b9d6e08 100644
> > > > > > > --- a/lib/librte_eal/common/eal_common_log.c
> > > > > > > +++ b/lib/librte_eal/common/eal_common_log.c
> > > > > > > @@ -260,7 +260,7 @@ static const struct logtype logtype_strings[] = {
> > > > > > >   };
> > > > > > >   /* Logging should be first initializer (before drivers and bus) */
> > > > > > > -RTE_INIT_PRIO(rte_log_init, 101);
> > > > > > > +RTE_INIT_PRIO(rte_log_init, LOG);
> > > > > > >   static void
> > > > > > >   rte_log_init(void)
> > > > > > >   {
> > > > > > > diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> > > > > > > index 6fb08341a..eb9eded4e 100644
> > > > > > > --- a/lib/librte_eal/common/include/rte_bus.h
> > > > > > > +++ b/lib/librte_eal/common/include/rte_bus.h
> > > > > > > @@ -325,7 +325,7 @@ enum rte_iova_mode rte_bus_get_iommu_class(void);
> > > > > > >    * The constructor has higher priority than PMD constructors.
> > > > > > >    */
> > > > > > >   #define RTE_REGISTER_BUS(nm, bus) \
> > > > > > > -RTE_INIT_PRIO(businitfn_ ##nm, 110); \
> > > > > > > +RTE_INIT_PRIO(businitfn_ ##nm, BUS); \
> > > > > > >   static void businitfn_ ##nm(void) \
> > > > > > >   {\
> > > > > > >   	(bus).name = RTE_STR(nm);\
> > > > > > > diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> > > > > > > index 6c5bc5a76..8f04518f7 100644
> > > > > > > --- a/lib/librte_eal/common/include/rte_common.h
> > > > > > > +++ b/lib/librte_eal/common/include/rte_common.h
> > > > > > > @@ -81,6 +81,12 @@ typedef uint16_t unaligned_uint16_t;
> > > > > > >    */
> > > > > > >   #define RTE_SET_USED(x) (void)(x)
> > > > > > > +#define RTE_PRIORITY_LOG 101
> > > > > > > +#define RTE_PRIORITY_BUS 110
> > > > > > > +
> > > > > > > +#define RTE_PRIO(prio) \
> > > > > > > +	RTE_PRIORITY_ ## prio
> > > > > > > +
> > > > > > >   /**
> > > > > > >    * Run function before main() with low priority.
> > > > > > >    *
> > > > > > > @@ -102,7 +108,7 @@ static void __attribute__((constructor, used)) func(void)
> > > > > > >    *   Lowest number is the first to run.
> > > > > > >    */
> > > > > > >   #define RTE_INIT_PRIO(func, prio) \
> > > > > > > -static void __attribute__((constructor(prio), used)) func(void)
> > > > > > > +static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)
> > > > > > It just occured to me, that perhaps you should add a RTE_PRORITY_LAST priority,
> > > > > > and redefine RTE_INIT to RTE_INIT_PRIO(func, RTE_PRIORITY_LAST) for clarity.  I
> > > > > > presume that constructors with no explicit priority run last, but the gcc
> > > > > > manual doesn't explicitly say that.  It would be a heck of a bug to track down
> > > > > > if somehow unprioritized constructors ran early.
> > > > > > 
> > > > > > Neil
> > > > > > 
> > > > > 
> > > > > While certainly poorly documented, the behavior is well-defined. I don't see
> > > > > a situation where the bug you describe could arise.
> > > > > 
> > > > > Adding RTE_PRIORITY_LAST is pretty harmless, but I'm not sure it's
> > > > > justified to add it. If you still think it is useful, I will do it.
> > > > > 
> > > > It was more just a way to unify the macros is all, probably not important.
> > > > 
> > > > > I'd be curious to hear if anyone has had issues of this kind.
> > > > > 
> > > > I've not had any, but I was suprised to see that the gcc manual didn't
> > > > explicitly call out the implied priority of unprioritized constructors
> > > 
> > > I (tried to) looked into the gcc code base. It seems that when priority is
> > > not defined, DEFAULT_INIT_PRIORITY 65536, is used.
> > > 
> > > --->8--- gcc/collect2.c ---
> > >   /* Extract init_p number from ctor/dtor name.  */
> > >   pri = atoi (name + pos);
> > >   return pri ? pri : DEFAULT_INIT_PRIORITY;
> > > --->8---
> > > 
> > > Though, I couldn't find any documentation for this fact - and, I can never
> > > be confident about gcc code.
> > > 
> > > I found one of the ARM compiler (clang) does has a policy for using
> > > non-specified priority as lower than specified priority. [1]
> > > 
> > > [1] https://developer.arm.com/docs/dui0774/latest/compiler-specific-function-variable-and-type-attributes/__attribute__constructorpriority-function-attribute
> > > 
> > > A specified value for RTE_PRIORITY_LAST is not a bad option - it would help
> > > in keeping the priorities bound without relying on the unknown of priority
> > > for unspecified constructors.
> > 
> > This is interesting, thanks for looking up the GCC code.
> > Ok, unless someone has a strong reason not to, I will add
> > RTE_PRIORITY_LAST. Not really convinced about it but not
> > opposed enough either :) .
> > 
> I concur.  It sounds like gcc is safe, but clangs priority scheme makes me want
> our priorities to be explicit.

Unless I've misunderstood, clang documentation describes the same
behavior as GCC?

Anyway, I've sent a new series with RTE_PRIORITY_LAST:

https://dpdk.org/dev/patchwork/patch/38126/
Neil Horman April 16, 2018, 11:31 a.m. UTC | #8 
On Sun, Apr 15, 2018 at 05:13:13PM +0200, Gaëtan Rivet wrote:
> Hello Neil,
> 
> On Sat, Apr 14, 2018 at 02:45:45PM -0400, Neil Horman wrote:
> > On Fri, Apr 13, 2018 at 02:55:11PM +0200, Gaëtan Rivet wrote:
> > > Hi Shreyansh,
> > > 
> > > On Fri, Apr 13, 2018 at 06:22:43PM +0530, Shreyansh Jain wrote:
> > > > On Friday 13 April 2018 05:12 PM, Neil Horman wrote:
> > > > > On Thu, Apr 12, 2018 at 11:57:47PM +0200, Gaëtan Rivet wrote:
> > > > > > Hello Neil,
> > > > > > 
> > > > > > On Thu, Apr 12, 2018 at 07:28:26AM -0400, Neil Horman wrote:
> > > > > > > On Wed, Apr 11, 2018 at 02:04:03AM +0200, Gaetan Rivet wrote:
> > > > > > > > Build a central list to quickly see each used priorities for
> > > > > > > > constructors, allowing to verify that they are both above 100 and in the
> > > > > > > > proper order.
> > > > > > > > 
> > > > > > > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > > > > > > Acked-by: Neil Horman <nhorman@tuxdriver.com>
> > > > > > > > Acked-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> > > > > > > > ---
> > > > > > > >   lib/librte_eal/common/eal_common_log.c     | 2 +-
> > > > > > > >   lib/librte_eal/common/include/rte_bus.h    | 2 +-
> > > > > > > >   lib/librte_eal/common/include/rte_common.h | 8 +++++++-
> > > > > > > >   3 files changed, 9 insertions(+), 3 deletions(-)
> > > > > > > > 
> > > > > > > > diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
> > > > > > > > index a27192620..36b9d6e08 100644
> > > > > > > > --- a/lib/librte_eal/common/eal_common_log.c
> > > > > > > > +++ b/lib/librte_eal/common/eal_common_log.c
> > > > > > > > @@ -260,7 +260,7 @@ static const struct logtype logtype_strings[] = {
> > > > > > > >   };
> > > > > > > >   /* Logging should be first initializer (before drivers and bus) */
> > > > > > > > -RTE_INIT_PRIO(rte_log_init, 101);
> > > > > > > > +RTE_INIT_PRIO(rte_log_init, LOG);
> > > > > > > >   static void
> > > > > > > >   rte_log_init(void)
> > > > > > > >   {
> > > > > > > > diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> > > > > > > > index 6fb08341a..eb9eded4e 100644
> > > > > > > > --- a/lib/librte_eal/common/include/rte_bus.h
> > > > > > > > +++ b/lib/librte_eal/common/include/rte_bus.h
> > > > > > > > @@ -325,7 +325,7 @@ enum rte_iova_mode rte_bus_get_iommu_class(void);
> > > > > > > >    * The constructor has higher priority than PMD constructors.
> > > > > > > >    */
> > > > > > > >   #define RTE_REGISTER_BUS(nm, bus) \
> > > > > > > > -RTE_INIT_PRIO(businitfn_ ##nm, 110); \
> > > > > > > > +RTE_INIT_PRIO(businitfn_ ##nm, BUS); \
> > > > > > > >   static void businitfn_ ##nm(void) \
> > > > > > > >   {\
> > > > > > > >   	(bus).name = RTE_STR(nm);\
> > > > > > > > diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
> > > > > > > > index 6c5bc5a76..8f04518f7 100644
> > > > > > > > --- a/lib/librte_eal/common/include/rte_common.h
> > > > > > > > +++ b/lib/librte_eal/common/include/rte_common.h
> > > > > > > > @@ -81,6 +81,12 @@ typedef uint16_t unaligned_uint16_t;
> > > > > > > >    */
> > > > > > > >   #define RTE_SET_USED(x) (void)(x)
> > > > > > > > +#define RTE_PRIORITY_LOG 101
> > > > > > > > +#define RTE_PRIORITY_BUS 110
> > > > > > > > +
> > > > > > > > +#define RTE_PRIO(prio) \
> > > > > > > > +	RTE_PRIORITY_ ## prio
> > > > > > > > +
> > > > > > > >   /**
> > > > > > > >    * Run function before main() with low priority.
> > > > > > > >    *
> > > > > > > > @@ -102,7 +108,7 @@ static void __attribute__((constructor, used)) func(void)
> > > > > > > >    *   Lowest number is the first to run.
> > > > > > > >    */
> > > > > > > >   #define RTE_INIT_PRIO(func, prio) \
> > > > > > > > -static void __attribute__((constructor(prio), used)) func(void)
> > > > > > > > +static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)
> > > > > > > It just occured to me, that perhaps you should add a RTE_PRORITY_LAST priority,
> > > > > > > and redefine RTE_INIT to RTE_INIT_PRIO(func, RTE_PRIORITY_LAST) for clarity.  I
> > > > > > > presume that constructors with no explicit priority run last, but the gcc
> > > > > > > manual doesn't explicitly say that.  It would be a heck of a bug to track down
> > > > > > > if somehow unprioritized constructors ran early.
> > > > > > > 
> > > > > > > Neil
> > > > > > > 
> > > > > > 
> > > > > > While certainly poorly documented, the behavior is well-defined. I don't see
> > > > > > a situation where the bug you describe could arise.
> > > > > > 
> > > > > > Adding RTE_PRIORITY_LAST is pretty harmless, but I'm not sure it's
> > > > > > justified to add it. If you still think it is useful, I will do it.
> > > > > > 
> > > > > It was more just a way to unify the macros is all, probably not important.
> > > > > 
> > > > > > I'd be curious to hear if anyone has had issues of this kind.
> > > > > > 
> > > > > I've not had any, but I was suprised to see that the gcc manual didn't
> > > > > explicitly call out the implied priority of unprioritized constructors
> > > > 
> > > > I (tried to) looked into the gcc code base. It seems that when priority is
> > > > not defined, DEFAULT_INIT_PRIORITY 65536, is used.
> > > > 
> > > > --->8--- gcc/collect2.c ---
> > > >   /* Extract init_p number from ctor/dtor name.  */
> > > >   pri = atoi (name + pos);
> > > >   return pri ? pri : DEFAULT_INIT_PRIORITY;
> > > > --->8---
> > > > 
> > > > Though, I couldn't find any documentation for this fact - and, I can never
> > > > be confident about gcc code.
> > > > 
> > > > I found one of the ARM compiler (clang) does has a policy for using
> > > > non-specified priority as lower than specified priority. [1]
> > > > 
> > > > [1] https://developer.arm.com/docs/dui0774/latest/compiler-specific-function-variable-and-type-attributes/__attribute__constructorpriority-function-attribute
> > > > 
> > > > A specified value for RTE_PRIORITY_LAST is not a bad option - it would help
> > > > in keeping the priorities bound without relying on the unknown of priority
> > > > for unspecified constructors.
> > > 
> > > This is interesting, thanks for looking up the GCC code.
> > > Ok, unless someone has a strong reason not to, I will add
> > > RTE_PRIORITY_LAST. Not really convinced about it but not
> > > opposed enough either :) .
> > > 
> > I concur.  It sounds like gcc is safe, but clangs priority scheme makes me want
> > our priorities to be explicit.
> 
> Unless I've misunderstood, clang documentation describes the same
> behavior as GCC?
> 
hmm, from the description above, Shreyansh indicates that the default priority
in GCC is 65535, which effectively means "run last".  And in clang, the
documentation above says regardless of value, unprioritized constructors always
run after prioritized constructors.

So, yeah, you're right, the behavior is the same.  Though the fact that it took
me two emails and typing it out to confirm that to myself, probably suggests
spelling it out is best :)

> Anyway, I've sent a new series with RTE_PRIORITY_LAST:
> 
> https://dpdk.org/dev/patchwork/patch/38126/
> 
I think that makes the most sense.  Thanks!

Neil

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

Patch

diff --git a/lib/librte_eal/common/eal_common_log.c b/lib/librte_eal/common/eal_common_log.c
index a27192620..36b9d6e08 100644
--- a/lib/librte_eal/common/eal_common_log.c
+++ b/lib/librte_eal/common/eal_common_log.c
@@ -260,7 +260,7 @@  static const struct logtype logtype_strings[] = {
 };
 
 /* Logging should be first initializer (before drivers and bus) */
-RTE_INIT_PRIO(rte_log_init, 101);
+RTE_INIT_PRIO(rte_log_init, LOG);
 static void
 rte_log_init(void)
 {
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 6fb08341a..eb9eded4e 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -325,7 +325,7 @@  enum rte_iova_mode rte_bus_get_iommu_class(void);
  * The constructor has higher priority than PMD constructors.
  */
 #define RTE_REGISTER_BUS(nm, bus) \
-RTE_INIT_PRIO(businitfn_ ##nm, 110); \
+RTE_INIT_PRIO(businitfn_ ##nm, BUS); \
 static void businitfn_ ##nm(void) \
 {\
 	(bus).name = RTE_STR(nm);\
diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
index 6c5bc5a76..8f04518f7 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -81,6 +81,12 @@  typedef uint16_t unaligned_uint16_t;
  */
 #define RTE_SET_USED(x) (void)(x)
 
+#define RTE_PRIORITY_LOG 101
+#define RTE_PRIORITY_BUS 110
+
+#define RTE_PRIO(prio) \
+	RTE_PRIORITY_ ## prio
+
 /**
  * Run function before main() with low priority.
  *
@@ -102,7 +108,7 @@  static void __attribute__((constructor, used)) func(void)
  *   Lowest number is the first to run.
  */
 #define RTE_INIT_PRIO(func, prio) \
-static void __attribute__((constructor(prio), used)) func(void)
+static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)
 
 /**
  * Force a function to be inlined