[2/8] trace: simplify trace point registration
Checks
Commit Message
RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
app/test/test_trace_register.c | 12 +-
doc/guides/prog_guide/trace_lib.rst | 12 +-
lib/librte_cryptodev/cryptodev_trace_points.c | 84 +++----
.../common/eal_common_trace_points.c | 164 ++++++--------
lib/librte_eal/include/rte_eal_trace.h | 122 +++++------
lib/librte_eal/include/rte_trace_point.h | 14 +-
.../include/rte_trace_point_register.h | 6 +-
lib/librte_ethdev/ethdev_trace_points.c | 44 ++--
lib/librte_eventdev/eventdev_trace_points.c | 205 +++++++-----------
lib/librte_mempool/mempool_trace_points.c | 124 ++++-------
10 files changed, 309 insertions(+), 478 deletions(-)
Comments
On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
>
> RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
Initially, I thought of doing the same. But, later I realized that
this largely grows the number of constructors been called.
I had concerns about the boot time of the application and/or loading
the shared library, that the reason why spitting
as two so that constructor registers a burst of traces like rte_log.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> app/test/test_trace_register.c | 12 +-
> doc/guides/prog_guide/trace_lib.rst | 12 +-
> lib/librte_cryptodev/cryptodev_trace_points.c | 84 +++----
> .../common/eal_common_trace_points.c | 164 ++++++--------
> lib/librte_eal/include/rte_eal_trace.h | 122 +++++------
> lib/librte_eal/include/rte_trace_point.h | 14 +-
> .../include/rte_trace_point_register.h | 6 +-
> lib/librte_ethdev/ethdev_trace_points.c | 44 ++--
> lib/librte_eventdev/eventdev_trace_points.c | 205 +++++++-----------
> lib/librte_mempool/mempool_trace_points.c | 124 ++++-------
> 10 files changed, 309 insertions(+), 478 deletions(-)
04/05/2020 04:46, Jerin Jacob:
> On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> >
> > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
>
>
> Initially, I thought of doing the same. But, later I realized that
> this largely grows the number of constructors been called.
> I had concerns about the boot time of the application and/or loading
> the shared library, that the reason why spitting
> as two so that constructor registers a burst of traces like rte_log.
I don't understand the concern.
How adding more constructors is affecting the library load time?
Do you have any number?
On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> >
> > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
>
>
> Initially, I thought of doing the same. But, later I realized that
> this largely grows the number of constructors been called.
> I had concerns about the boot time of the application and/or loading
> the shared library, that the reason why spitting
> as two so that constructor registers a burst of traces like rte_log.
I am a bit skeptical.
In terms of cycles and looking at __rte_trace_point_register() (which
calls malloc), the cost of calling multiple constructors instead of
one is negligible.
On Mon, May 4, 2020 at 7:34 PM David Marchand <david.marchand@redhat.com> wrote:
>
> On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >
> > On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> > >
> > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
> >
> >
> > Initially, I thought of doing the same. But, later I realized that
> > this largely grows the number of constructors been called.
> > I had concerns about the boot time of the application and/or loading
> > the shared library, that the reason why spitting
> > as two so that constructor registers a burst of traces like rte_log.
>
> I am a bit skeptical.
> In terms of cycles and looking at __rte_trace_point_register() (which
> calls malloc), the cost of calling multiple constructors instead of
> one is negligible.
We will have a lot tracepoints latter, each one translates to the
constructor may not be a good
improvement. The scope is limited only to register function so IMO it
is okay to have split
just like rte_log. I don't see any reason why it has to be a different
than rte_log.
One of the thought process is, we probably remove the constructor
scheme to all other with DPDK
and replace it with a more register scheme. In such a case, we can
skip calling the constructor all tother
when trace is disabled.
>
>
> --
> David Marchand
>
On Mon, May 4, 2020 at 4:39 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Mon, May 4, 2020 at 7:34 PM David Marchand <david.marchand@redhat.com> wrote:
> >
> > On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > >
> > > On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> > > >
> > > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > > > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
> > >
> > >
> > > Initially, I thought of doing the same. But, later I realized that
> > > this largely grows the number of constructors been called.
> > > I had concerns about the boot time of the application and/or loading
> > > the shared library, that the reason why spitting
> > > as two so that constructor registers a burst of traces like rte_log.
> >
> > I am a bit skeptical.
> > In terms of cycles and looking at __rte_trace_point_register() (which
> > calls malloc), the cost of calling multiple constructors instead of
> > one is negligible.
>
> We will have a lot tracepoints latter, each one translates to the
> constructor may not be a good
> improvement. The scope is limited only to register function so IMO it
> is okay to have split
> just like rte_log. I don't see any reason why it has to be a different
> than rte_log.
What is similar to rte_log?
There is neither RTE_LOG_REGISTER macro, nor two-steps declaration of
dynamic logtypes.
>
> One of the thought process is, we probably remove the constructor
> scheme to all other with DPDK
> and replace it with a more register scheme. In such a case, we can
> skip calling the constructor all tother
> when trace is disabled.
Sorry, but I have a hard time understanding your point.
Are you talking about application boot time?
On Mon, May 4, 2020 at 10:38 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Mon, May 4, 2020 at 4:39 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >
> > On Mon, May 4, 2020 at 7:34 PM David Marchand <david.marchand@redhat.com> wrote:
> > >
> > > On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > >
> > > > On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> > > > >
> > > > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > > > > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
> > > >
> > > >
> > > > Initially, I thought of doing the same. But, later I realized that
> > > > this largely grows the number of constructors been called.
> > > > I had concerns about the boot time of the application and/or loading
> > > > the shared library, that the reason why spitting
> > > > as two so that constructor registers a burst of traces like rte_log.
> > >
> > > I am a bit skeptical.
> > > In terms of cycles and looking at __rte_trace_point_register() (which
> > > calls malloc), the cost of calling multiple constructors instead of
> > > one is negligible.
> >
> > We will have a lot tracepoints latter, each one translates to the
> > constructor may not be a good
> > improvement. The scope is limited only to register function so IMO it
> > is okay to have split
> > just like rte_log. I don't see any reason why it has to be a different
> > than rte_log.
>
> What is similar to rte_log?
> There is neither RTE_LOG_REGISTER macro, nor two-steps declaration of
> dynamic logtypes.
Here is an example of rte_log registration. Which has _one_
constructor and N number of
rte_log_register() underneath.
RTE_INIT(otx2_log_init);
static void
otx2_log_init(void)
{
otx2_logtype_base = rte_log_register("pmd.octeontx2.base");
if (otx2_logtype_base >= 0)
rte_log_set_level(otx2_logtype_base, RTE_LOG_NOTICE);
otx2_logtype_mbox = rte_log_register("pmd.octeontx2.mbox");
if (otx2_logtype_mbox >= 0)
rte_log_set_level(otx2_logtype_mbox, RTE_LOG_NOTICE);
otx2_logtype_npa = rte_log_register("pmd.mempool.octeontx2");
if (otx2_logtype_npa >= 0)
rte_log_set_level(otx2_logtype_npa, RTE_LOG_NOTICE);
otx2_logtype_nix = rte_log_register("pmd.net.octeontx2");
if (otx2_logtype_nix >= 0)
rte_log_set_level(otx2_logtype_nix, RTE_LOG_NOTICE);
otx2_logtype_npc = rte_log_register("pmd.net.octeontx2.flow");
if (otx2_logtype_npc >= 0)
rte_log_set_level(otx2_logtype_npc, RTE_LOG_NOTICE);
otx2_logtype_tm = rte_log_register("pmd.net.octeontx2.tm");
if (otx2_logtype_tm >= 0)
rte_log_set_level(otx2_logtype_tm, RTE_LOG_NOTICE);
otx2_logtype_sso = rte_log_register("pmd.event.octeontx2");
if (otx2_logtype_sso >= 0)
rte_log_set_level(otx2_logtype_sso, RTE_LOG_NOTICE);
otx2_logtype_tim = rte_log_register("pmd.event.octeontx2.timer");
if (otx2_logtype_tim >= 0)
rte_log_set_level(otx2_logtype_tim, RTE_LOG_NOTICE);
otx2_logtype_dpi = rte_log_register("pmd.raw.octeontx2.dpi");
if (otx2_logtype_dpi >= 0)
rte_log_set_level(otx2_logtype_dpi, RTE_LOG_NOTICE);
otx2_logtype_ep = rte_log_register("pmd.raw.octeontx2.ep");
if (otx2_logtype_ep >= 0)
rte_log_set_level(otx2_logtype_ep, RTE_LOG_NOTICE);
}
>
>
> >
> > One of the thought process is, we probably remove the constructor
> > scheme to all other with DPDK
> > and replace it with a more register scheme. In such a case, we can
> > skip calling the constructor all tother
> > when trace is disabled.
>
> Sorry, but I have a hard time understanding your point.
> Are you talking about application boot time?
Yes. The optimization of application boottime time in case of static
binary and/or shared library(.so) load time.
>
>
> --
> David Marchand
>
On Mon, May 4, 2020 at 7:19 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Mon, May 4, 2020 at 10:38 PM David Marchand
> <david.marchand@redhat.com> wrote:
> >
> > On Mon, May 4, 2020 at 4:39 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > >
> > > On Mon, May 4, 2020 at 7:34 PM David Marchand <david.marchand@redhat.com> wrote:
> > > >
> > > > On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > >
> > > > > On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> > > > > >
> > > > > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > > > > > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
> > > > >
> > > > >
> > > > > Initially, I thought of doing the same. But, later I realized that
> > > > > this largely grows the number of constructors been called.
> > > > > I had concerns about the boot time of the application and/or loading
> > > > > the shared library, that the reason why spitting
> > > > > as two so that constructor registers a burst of traces like rte_log.
> > > >
> > > > I am a bit skeptical.
> > > > In terms of cycles and looking at __rte_trace_point_register() (which
> > > > calls malloc), the cost of calling multiple constructors instead of
> > > > one is negligible.
> > >
> > > We will have a lot tracepoints latter, each one translates to the
> > > constructor may not be a good
> > > improvement. The scope is limited only to register function so IMO it
> > > is okay to have split
> > > just like rte_log. I don't see any reason why it has to be a different
> > > than rte_log.
> >
> > What is similar to rte_log?
> > There is neither RTE_LOG_REGISTER macro, nor two-steps declaration of
> > dynamic logtypes.
>
>
> Here is an example of rte_log registration. Which has _one_
> constructor and N number of
> rte_log_register() underneath.
rte_log is one thing, rte_trace is already different.
There is _no macro_ in rte_log for registration.
The reason being in that a rte_log logtype is a simple integer without
any special declaration requiring a macro.
For tracepoints, we have a special two steps thing: the tracepoint
handle must be derived from the tracepoint name.
Then this handle must be registered.
What I proposed is to make life easier for developers that want to add
tracepoints and I suppose you noticed patch 1 of this series.
> > > One of the thought process is, we probably remove the constructor
> > > scheme to all other with DPDK
> > > and replace it with a more register scheme. In such a case, we can
> > > skip calling the constructor all tother
> > > when trace is disabled.
> >
> > Sorry, but I have a hard time understanding your point.
> > Are you talking about application boot time?
>
> Yes. The optimization of application boottime time in case of static
> binary and/or shared library(.so) load time.
As Thomas mentioned, do you have numbers?
On Mon, May 4, 2020 at 11:10 PM David Marchand
<david.marchand@redhat.com> wrote:
>
> On Mon, May 4, 2020 at 7:19 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >
> > On Mon, May 4, 2020 at 10:38 PM David Marchand
> > <david.marchand@redhat.com> wrote:
> > >
> > > On Mon, May 4, 2020 at 4:39 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > >
> > > > On Mon, May 4, 2020 at 7:34 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > >
> > > > > On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > >
> > > > > > On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > >
> > > > > > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > > > > > > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
> > > > > >
> > > > > >
> > > > > > Initially, I thought of doing the same. But, later I realized that
> > > > > > this largely grows the number of constructors been called.
> > > > > > I had concerns about the boot time of the application and/or loading
> > > > > > the shared library, that the reason why spitting
> > > > > > as two so that constructor registers a burst of traces like rte_log.
> > > > >
> > > > > I am a bit skeptical.
> > > > > In terms of cycles and looking at __rte_trace_point_register() (which
> > > > > calls malloc), the cost of calling multiple constructors instead of
> > > > > one is negligible.
> > > >
> > > > We will have a lot tracepoints latter, each one translates to the
> > > > constructor may not be a good
> > > > improvement. The scope is limited only to register function so IMO it
> > > > is okay to have split
> > > > just like rte_log. I don't see any reason why it has to be a different
> > > > than rte_log.
> > >
> > > What is similar to rte_log?
> > > There is neither RTE_LOG_REGISTER macro, nor two-steps declaration of
> > > dynamic logtypes.
> >
> >
> > Here is an example of rte_log registration. Which has _one_
> > constructor and N number of
> > rte_log_register() underneath.
>
> rte_log is one thing, rte_trace is already different.
>
> There is _no macro_ in rte_log for registration.
> The reason being in that a rte_log logtype is a simple integer without
> any special declaration requiring a macro.
I just wrapped in macro for convincing, but it has the same semantics.
global variable and API/macro to register.
>
> For tracepoints, we have a special two steps thing: the tracepoint
> handle must be derived from the tracepoint name.
> Then this handle must be registered.
> What I proposed is to make life easier for developers that want to add
> tracepoints and I suppose you noticed patch 1 of this series.
To reduce the constructors. I don't want trace libraries to add lot of
constructors.
I don't think it simplifies a lot as the scope of only for registration.
>
>
> > > > One of the thought process is, we probably remove the constructor
> > > > scheme to all other with DPDK
> > > > and replace it with a more register scheme. In such a case, we can
> > > > skip calling the constructor all tother
> > > > when trace is disabled.
> > >
> > > Sorry, but I have a hard time understanding your point.
> > > Are you talking about application boot time?
> >
> > Yes. The optimization of application boottime time in case of static
> > binary and/or shared library(.so) load time.
>
> As Thomas mentioned, do you have numbers?
No. But I know, it is obvious that current code is better in terms of
boot time than the proposed one.
I would like to not add a lot of constructor for trace as the FIRST
module in DPDK.
>
>
> --
> David Marchand
>
04/05/2020 19:54, Jerin Jacob:
> On Mon, May 4, 2020 at 11:10 PM David Marchand
> > On Mon, May 4, 2020 at 7:19 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > On Mon, May 4, 2020 at 10:38 PM David Marchand
> > > > On Mon, May 4, 2020 at 4:39 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > On Mon, May 4, 2020 at 7:34 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > >
> > > > > > > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > > > > > > > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
> > > > > > >
> > > > > > >
> > > > > > > Initially, I thought of doing the same. But, later I realized that
> > > > > > > this largely grows the number of constructors been called.
> > > > > > > I had concerns about the boot time of the application and/or loading
> > > > > > > the shared library, that the reason why spitting
> > > > > > > as two so that constructor registers a burst of traces like rte_log.
> > > > > >
> > > > > > I am a bit skeptical.
> > > > > > In terms of cycles and looking at __rte_trace_point_register() (which
> > > > > > calls malloc), the cost of calling multiple constructors instead of
> > > > > > one is negligible.
> > > > >
> > > > > We will have a lot tracepoints latter, each one translates to the
> > > > > constructor may not be a good
> > > > > improvement. The scope is limited only to register function so IMO it
> > > > > is okay to have split
> > > > > just like rte_log. I don't see any reason why it has to be a different
> > > > > than rte_log.
> > > >
> > > > What is similar to rte_log?
> > > > There is neither RTE_LOG_REGISTER macro, nor two-steps declaration of
> > > > dynamic logtypes.
> > >
> > >
> > > Here is an example of rte_log registration. Which has _one_
> > > constructor and N number of
> > > rte_log_register() underneath.
> >
> > rte_log is one thing, rte_trace is already different.
> >
> > There is _no macro_ in rte_log for registration.
> > The reason being in that a rte_log logtype is a simple integer without
> > any special declaration requiring a macro.
>
> I just wrapped in macro for convincing, but it has the same semantics.
> global variable and API/macro to register.
>
>
> >
> > For tracepoints, we have a special two steps thing: the tracepoint
> > handle must be derived from the tracepoint name.
> > Then this handle must be registered.
> > What I proposed is to make life easier for developers that want to add
> > tracepoints and I suppose you noticed patch 1 of this series.
>
> To reduce the constructors. I don't want trace libraries to add lot of
> constructors.
> I don't think it simplifies a lot as the scope of only for registration.
>
>
> >
> >
> > > > > One of the thought process is, we probably remove the constructor
> > > > > scheme to all other with DPDK
> > > > > and replace it with a more register scheme. In such a case, we can
> > > > > skip calling the constructor all tother
> > > > > when trace is disabled.
> > > >
> > > > Sorry, but I have a hard time understanding your point.
> > > > Are you talking about application boot time?
> > >
> > > Yes. The optimization of application boottime time in case of static
> > > binary and/or shared library(.so) load time.
> >
> > As Thomas mentioned, do you have numbers?
>
> No. But I know, it is obvious that current code is better in terms of
> boot time than the proposed one.
> I would like to not add a lot of constructor for trace as the FIRST
> module in DPDK.
No, it is not obvious.
The version from David looks simpler to use and to understand.
Without any number, I consider usability (and maintenance) wins.
On Tue, May 5, 2020 at 3:01 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 04/05/2020 19:54, Jerin Jacob:
> > On Mon, May 4, 2020 at 11:10 PM David Marchand
> > > On Mon, May 4, 2020 at 7:19 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > On Mon, May 4, 2020 at 10:38 PM David Marchand
> > > > > On Mon, May 4, 2020 at 4:39 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > On Mon, May 4, 2020 at 7:34 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > >
> > > > > > > > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > > > > > > > > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
> > > > > > > >
> > > > > > > >
> > > > > > > > Initially, I thought of doing the same. But, later I realized that
> > > > > > > > this largely grows the number of constructors been called.
> > > > > > > > I had concerns about the boot time of the application and/or loading
> > > > > > > > the shared library, that the reason why spitting
> > > > > > > > as two so that constructor registers a burst of traces like rte_log.
> > > > > > >
> > > > > > > I am a bit skeptical.
> > > > > > > In terms of cycles and looking at __rte_trace_point_register() (which
> > > > > > > calls malloc), the cost of calling multiple constructors instead of
> > > > > > > one is negligible.
> > > > > >
> > > > > > We will have a lot tracepoints latter, each one translates to the
> > > > > > constructor may not be a good
> > > > > > improvement. The scope is limited only to register function so IMO it
> > > > > > is okay to have split
> > > > > > just like rte_log. I don't see any reason why it has to be a different
> > > > > > than rte_log.
> > > > >
> > > > > What is similar to rte_log?
> > > > > There is neither RTE_LOG_REGISTER macro, nor two-steps declaration of
> > > > > dynamic logtypes.
> > > >
> > > >
> > > > Here is an example of rte_log registration. Which has _one_
> > > > constructor and N number of
> > > > rte_log_register() underneath.
> > >
> > > rte_log is one thing, rte_trace is already different.
> > >
> > > There is _no macro_ in rte_log for registration.
> > > The reason being in that a rte_log logtype is a simple integer without
> > > any special declaration requiring a macro.
> >
> > I just wrapped in macro for convincing, but it has the same semantics.
> > global variable and API/macro to register.
> >
> >
> > >
> > > For tracepoints, we have a special two steps thing: the tracepoint
> > > handle must be derived from the tracepoint name.
> > > Then this handle must be registered.
> > > What I proposed is to make life easier for developers that want to add
> > > tracepoints and I suppose you noticed patch 1 of this series.
> >
> > To reduce the constructors. I don't want trace libraries to add lot of
> > constructors.
> > I don't think it simplifies a lot as the scope of only for registration.
> >
> >
> > >
> > >
> > > > > > One of the thought process is, we probably remove the constructor
> > > > > > scheme to all other with DPDK
> > > > > > and replace it with a more register scheme. In such a case, we can
> > > > > > skip calling the constructor all tother
> > > > > > when trace is disabled.
> > > > >
> > > > > Sorry, but I have a hard time understanding your point.
> > > > > Are you talking about application boot time?
> > > >
> > > > Yes. The optimization of application boottime time in case of static
> > > > binary and/or shared library(.so) load time.
> > >
> > > As Thomas mentioned, do you have numbers?
> >
> > No. But I know, it is obvious that current code is better in terms of
> > boot time than the proposed one.
> > I would like to not add a lot of constructor for trace as the FIRST
> > module in DPDK.
>
> No, it is not obvious.
> The version from David looks simpler to use and to understand.
> Without any number, I consider usability (and maintenance) wins.
Thanks for the feedback.
As the trace maintainer, I would like not to explode constructor usage
for trace library.
My reasoning, We could do trace registration without this constructor scheme.
If you think, it is better usability, lets add an option for rte_log
for the constructor
scheme. Analyze the impact wrt boot time and cross-platform pov as the log
has a lot of entries to test. If the usage makes sense then it should make sense
for rte_log too. I would like to NOT have trace to be the first
library to explode
with the constructor scheme. I suggest removing this specific patch from RC2 and
revisit later.
>
>
05/05/2020 05:43, Jerin Jacob:
> On Tue, May 5, 2020 at 3:01 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > 04/05/2020 19:54, Jerin Jacob:
> > > On Mon, May 4, 2020 at 11:10 PM David Marchand
> > > > On Mon, May 4, 2020 at 7:19 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > On Mon, May 4, 2020 at 10:38 PM David Marchand
> > > > > > On Mon, May 4, 2020 at 4:39 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > On Mon, May 4, 2020 at 7:34 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > > >
> > > > > > > > > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > > > > > > > > > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Initially, I thought of doing the same. But, later I realized that
> > > > > > > > > this largely grows the number of constructors been called.
> > > > > > > > > I had concerns about the boot time of the application and/or loading
> > > > > > > > > the shared library, that the reason why spitting
> > > > > > > > > as two so that constructor registers a burst of traces like rte_log.
> > > > > > > >
> > > > > > > > I am a bit skeptical.
> > > > > > > > In terms of cycles and looking at __rte_trace_point_register() (which
> > > > > > > > calls malloc), the cost of calling multiple constructors instead of
> > > > > > > > one is negligible.
> > > > > > >
> > > > > > > We will have a lot tracepoints latter, each one translates to the
> > > > > > > constructor may not be a good
> > > > > > > improvement. The scope is limited only to register function so IMO it
> > > > > > > is okay to have split
> > > > > > > just like rte_log. I don't see any reason why it has to be a different
> > > > > > > than rte_log.
> > > > > >
> > > > > > What is similar to rte_log?
> > > > > > There is neither RTE_LOG_REGISTER macro, nor two-steps declaration of
> > > > > > dynamic logtypes.
> > > > >
> > > > >
> > > > > Here is an example of rte_log registration. Which has _one_
> > > > > constructor and N number of
> > > > > rte_log_register() underneath.
> > > >
> > > > rte_log is one thing, rte_trace is already different.
> > > >
> > > > There is _no macro_ in rte_log for registration.
> > > > The reason being in that a rte_log logtype is a simple integer without
> > > > any special declaration requiring a macro.
> > >
> > > I just wrapped in macro for convincing, but it has the same semantics.
> > > global variable and API/macro to register.
> > >
> > >
> > > >
> > > > For tracepoints, we have a special two steps thing: the tracepoint
> > > > handle must be derived from the tracepoint name.
> > > > Then this handle must be registered.
> > > > What I proposed is to make life easier for developers that want to add
> > > > tracepoints and I suppose you noticed patch 1 of this series.
> > >
> > > To reduce the constructors. I don't want trace libraries to add lot of
> > > constructors.
> > > I don't think it simplifies a lot as the scope of only for registration.
> > >
> > >
> > > >
> > > >
> > > > > > > One of the thought process is, we probably remove the constructor
> > > > > > > scheme to all other with DPDK
> > > > > > > and replace it with a more register scheme. In such a case, we can
> > > > > > > skip calling the constructor all tother
> > > > > > > when trace is disabled.
> > > > > >
> > > > > > Sorry, but I have a hard time understanding your point.
> > > > > > Are you talking about application boot time?
> > > > >
> > > > > Yes. The optimization of application boottime time in case of static
> > > > > binary and/or shared library(.so) load time.
> > > >
> > > > As Thomas mentioned, do you have numbers?
> > >
> > > No. But I know, it is obvious that current code is better in terms of
> > > boot time than the proposed one.
> > > I would like to not add a lot of constructor for trace as the FIRST
> > > module in DPDK.
> >
> > No, it is not obvious.
> > The version from David looks simpler to use and to understand.
> > Without any number, I consider usability (and maintenance) wins.
>
> Thanks for the feedback.
>
> As the trace maintainer, I would like not to explode constructor usage
> for trace library.
> My reasoning, We could do trace registration without this constructor scheme.
???
> If you think, it is better usability, lets add an option for rte_log
> for the constructor scheme.
It makes non-sense.
rte_log requires only one function call per log type.
rte_trace requires 3 macros calls per trace type:
RTE_TRACE_POINT_REGISTER, RTE_TRACE_POINT_DEFINE, RTE_TRACE_POINT_ARGS
This patch is unifying the first 2 macro calls to make usage simpler,
and ease rte_trace adoption.
Note: the other usability weirdness is mandating declaring each trace
function with a magic double underscore prefix which appears nowhere else.
> Analyze the impact wrt boot time and cross-platform pov as the log
> has a lot of entries to test. If the usage makes sense then it should make sense
> for rte_log too. I would like to NOT have trace to be the first
> library to explode
> with the constructor scheme. I suggest removing this specific patch from RC2 and
> revisit later.
You still did not give any argument against increasing the number
of constructors.
On Tue, May 5, 2020 at 12:31 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 05/05/2020 05:43, Jerin Jacob:
> > On Tue, May 5, 2020 at 3:01 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > 04/05/2020 19:54, Jerin Jacob:
> > > > On Mon, May 4, 2020 at 11:10 PM David Marchand
> > > > > On Mon, May 4, 2020 at 7:19 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > On Mon, May 4, 2020 at 10:38 PM David Marchand
> > > > > > > On Mon, May 4, 2020 at 4:39 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > On Mon, May 4, 2020 at 7:34 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > > On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > > On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > > > >
> > > > > > > > > > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > > > > > > > > > > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Initially, I thought of doing the same. But, later I realized that
> > > > > > > > > > this largely grows the number of constructors been called.
> > > > > > > > > > I had concerns about the boot time of the application and/or loading
> > > > > > > > > > the shared library, that the reason why spitting
> > > > > > > > > > as two so that constructor registers a burst of traces like rte_log.
> > > > > > > > >
> > > > > > > > > I am a bit skeptical.
> > > > > > > > > In terms of cycles and looking at __rte_trace_point_register() (which
> > > > > > > > > calls malloc), the cost of calling multiple constructors instead of
> > > > > > > > > one is negligible.
> > > > > > > >
> > > > > > > > We will have a lot tracepoints latter, each one translates to the
> > > > > > > > constructor may not be a good
> > > > > > > > improvement. The scope is limited only to register function so IMO it
> > > > > > > > is okay to have split
> > > > > > > > just like rte_log. I don't see any reason why it has to be a different
> > > > > > > > than rte_log.
> > > > > > >
> > > > > > > What is similar to rte_log?
> > > > > > > There is neither RTE_LOG_REGISTER macro, nor two-steps declaration of
> > > > > > > dynamic logtypes.
> > > > > >
> > > > > >
> > > > > > Here is an example of rte_log registration. Which has _one_
> > > > > > constructor and N number of
> > > > > > rte_log_register() underneath.
> > > > >
> > > > > rte_log is one thing, rte_trace is already different.
> > > > >
> > > > > There is _no macro_ in rte_log for registration.
> > > > > The reason being in that a rte_log logtype is a simple integer without
> > > > > any special declaration requiring a macro.
> > > >
> > > > I just wrapped in macro for convincing, but it has the same semantics.
> > > > global variable and API/macro to register.
> > > >
> > > >
> > > > >
> > > > > For tracepoints, we have a special two steps thing: the tracepoint
> > > > > handle must be derived from the tracepoint name.
> > > > > Then this handle must be registered.
> > > > > What I proposed is to make life easier for developers that want to add
> > > > > tracepoints and I suppose you noticed patch 1 of this series.
> > > >
> > > > To reduce the constructors. I don't want trace libraries to add lot of
> > > > constructors.
> > > > I don't think it simplifies a lot as the scope of only for registration.
> > > >
> > > >
> > > > >
> > > > >
> > > > > > > > One of the thought process is, we probably remove the constructor
> > > > > > > > scheme to all other with DPDK
> > > > > > > > and replace it with a more register scheme. In such a case, we can
> > > > > > > > skip calling the constructor all tother
> > > > > > > > when trace is disabled.
> > > > > > >
> > > > > > > Sorry, but I have a hard time understanding your point.
> > > > > > > Are you talking about application boot time?
> > > > > >
> > > > > > Yes. The optimization of application boottime time in case of static
> > > > > > binary and/or shared library(.so) load time.
> > > > >
> > > > > As Thomas mentioned, do you have numbers?
> > > >
> > > > No. But I know, it is obvious that current code is better in terms of
> > > > boot time than the proposed one.
> > > > I would like to not add a lot of constructor for trace as the FIRST
> > > > module in DPDK.
> > >
> > > No, it is not obvious.
> > > The version from David looks simpler to use and to understand.
> > > Without any number, I consider usability (and maintenance) wins.
> >
> > Thanks for the feedback.
> >
> > As the trace maintainer, I would like not to explode constructor usage
> > for trace library.
> > My reasoning, We could do trace registration without this constructor scheme.
> ???
We don't need this patch to make trace to work.
>
>
> > If you think, it is better usability, lets add an option for rte_log
> > for the constructor scheme.
>
> It makes non-sense.
> rte_log requires only one function call per log type.
Here is the example of the log registration:
global variable:
int otx2_logtype_base;
int otx2_logtype_mbox;
int otx2_logtype_npa;
RTE_INIT(otx2_log_init);
static void
otx2_log_init(void)
{
otx2_logtype_base = rte_log_register("pmd.octeontx2.base");
otx2_logtype_mbox = rte_log_register("pmd.octeontx2.mbox");
otx2_logtype_npa = rte_log_register("pmd.mempool.octeontx2");
}
What the proposed patch here.
# Making N constructors from one
# Grouping global variable and register function under a single Marco
and making it as N constructors.
Why can we do the same logic for rte_log?
> rte_trace requires 3 macros calls per trace type:
> RTE_TRACE_POINT_REGISTER, RTE_TRACE_POINT_DEFINE, RTE_TRACE_POINT_ARGS
> This patch is unifying the first 2 macro calls to make usage simpler,
> and ease rte_trace adoption.
RTE_TRACE_POINT_ARGS is NOP and for the syntax.
It is similar to rte_log. rte_log don't have RTE_TRACE_POINT_REGISTER instead
it is creating global variable see, "int otx2_logtype_base;
>
> Note: the other usability weirdness is mandating declaring each trace
> function with a magic double underscore prefix which appears nowhere else.
>
>
> > Analyze the impact wrt boot time and cross-platform pov as the log
> > has a lot of entries to test. If the usage makes sense then it should make sense
> > for rte_log too. I would like to NOT have trace to be the first
> > library to explode
> > with the constructor scheme. I suggest removing this specific patch from RC2 and
> > revisit later.
>
> You still did not give any argument against increasing the number
> of constructors.
If you are proposing the new scheme, you have to prove the overhead
with a significant number of constructors
and why it has differed from existing scheme of things. That's is the
norm in opensource.
>
>
05/05/2020 09:17, Jerin Jacob:
> On Tue, May 5, 2020 at 12:31 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > 05/05/2020 05:43, Jerin Jacob:
> > > On Tue, May 5, 2020 at 3:01 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > 04/05/2020 19:54, Jerin Jacob:
> > > > > On Mon, May 4, 2020 at 11:10 PM David Marchand
> > > > > > On Mon, May 4, 2020 at 7:19 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > On Mon, May 4, 2020 at 10:38 PM David Marchand
> > > > > > > > On Mon, May 4, 2020 at 4:39 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > On Mon, May 4, 2020 at 7:34 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > > > On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > > > On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > > > > > > > > > > > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Initially, I thought of doing the same. But, later I realized that
> > > > > > > > > > > this largely grows the number of constructors been called.
> > > > > > > > > > > I had concerns about the boot time of the application and/or loading
> > > > > > > > > > > the shared library, that the reason why spitting
> > > > > > > > > > > as two so that constructor registers a burst of traces like rte_log.
> > > > > > > > > >
> > > > > > > > > > I am a bit skeptical.
> > > > > > > > > > In terms of cycles and looking at __rte_trace_point_register() (which
> > > > > > > > > > calls malloc), the cost of calling multiple constructors instead of
> > > > > > > > > > one is negligible.
> > > > > > > > >
> > > > > > > > > We will have a lot tracepoints latter, each one translates to the
> > > > > > > > > constructor may not be a good
> > > > > > > > > improvement. The scope is limited only to register function so IMO it
> > > > > > > > > is okay to have split
> > > > > > > > > just like rte_log. I don't see any reason why it has to be a different
> > > > > > > > > than rte_log.
> > > > > > > >
> > > > > > > > What is similar to rte_log?
> > > > > > > > There is neither RTE_LOG_REGISTER macro, nor two-steps declaration of
> > > > > > > > dynamic logtypes.
> > > > > > >
> > > > > > >
> > > > > > > Here is an example of rte_log registration. Which has _one_
> > > > > > > constructor and N number of
> > > > > > > rte_log_register() underneath.
> > > > > >
> > > > > > rte_log is one thing, rte_trace is already different.
> > > > > >
> > > > > > There is _no macro_ in rte_log for registration.
> > > > > > The reason being in that a rte_log logtype is a simple integer without
> > > > > > any special declaration requiring a macro.
> > > > >
> > > > > I just wrapped in macro for convincing, but it has the same semantics.
> > > > > global variable and API/macro to register.
> > > > >
> > > > >
> > > > > >
> > > > > > For tracepoints, we have a special two steps thing: the tracepoint
> > > > > > handle must be derived from the tracepoint name.
> > > > > > Then this handle must be registered.
> > > > > > What I proposed is to make life easier for developers that want to add
> > > > > > tracepoints and I suppose you noticed patch 1 of this series.
> > > > >
> > > > > To reduce the constructors. I don't want trace libraries to add lot of
> > > > > constructors.
> > > > > I don't think it simplifies a lot as the scope of only for registration.
> > > > >
> > > > >
> > > > > >
> > > > > >
> > > > > > > > > One of the thought process is, we probably remove the constructor
> > > > > > > > > scheme to all other with DPDK
> > > > > > > > > and replace it with a more register scheme. In such a case, we can
> > > > > > > > > skip calling the constructor all tother
> > > > > > > > > when trace is disabled.
> > > > > > > >
> > > > > > > > Sorry, but I have a hard time understanding your point.
> > > > > > > > Are you talking about application boot time?
> > > > > > >
> > > > > > > Yes. The optimization of application boottime time in case of static
> > > > > > > binary and/or shared library(.so) load time.
> > > > > >
> > > > > > As Thomas mentioned, do you have numbers?
> > > > >
> > > > > No. But I know, it is obvious that current code is better in terms of
> > > > > boot time than the proposed one.
> > > > > I would like to not add a lot of constructor for trace as the FIRST
> > > > > module in DPDK.
> > > >
> > > > No, it is not obvious.
> > > > The version from David looks simpler to use and to understand.
> > > > Without any number, I consider usability (and maintenance) wins.
> > >
> > > Thanks for the feedback.
> > >
> > > As the trace maintainer, I would like not to explode constructor usage
> > > for trace library.
> > > My reasoning, We could do trace registration without this constructor scheme.
> > ???
>
> We don't need this patch to make trace to work.
>
> >
> >
> > > If you think, it is better usability, lets add an option for rte_log
> > > for the constructor scheme.
> >
> > It makes non-sense.
> > rte_log requires only one function call per log type.
>
> Here is the example of the log registration:
>
> global variable:
> int otx2_logtype_base;
> int otx2_logtype_mbox;
> int otx2_logtype_npa;
>
> RTE_INIT(otx2_log_init);
> static void
> otx2_log_init(void)
> {
> otx2_logtype_base = rte_log_register("pmd.octeontx2.base");
> otx2_logtype_mbox = rte_log_register("pmd.octeontx2.mbox");
> otx2_logtype_npa = rte_log_register("pmd.mempool.octeontx2");
> }
>
> What the proposed patch here.
> # Making N constructors from one
> # Grouping global variable and register function under a single Marco
> and making it as N constructors.
> Why can we do the same logic for rte_log?
rte_log is simple, there is nothing to simplify.
This comparison makes no sense.
> > rte_trace requires 3 macros calls per trace type:
> > RTE_TRACE_POINT_REGISTER, RTE_TRACE_POINT_DEFINE, RTE_TRACE_POINT_ARGS
> > This patch is unifying the first 2 macro calls to make usage simpler,
> > and ease rte_trace adoption.
>
> RTE_TRACE_POINT_ARGS is NOP and for the syntax.
> It is similar to rte_log. rte_log don't have RTE_TRACE_POINT_REGISTER instead
> it is creating global variable see, "int otx2_logtype_base;
>
> >
> > Note: the other usability weirdness is mandating declaring each trace
> > function with a magic double underscore prefix which appears nowhere else.
> >
> >
> > > Analyze the impact wrt boot time and cross-platform pov as the log
> > > has a lot of entries to test. If the usage makes sense then it should make sense
> > > for rte_log too. I would like to NOT have trace to be the first
> > > library to explode
> > > with the constructor scheme. I suggest removing this specific patch from RC2 and
> > > revisit later.
> >
> > You still did not give any argument against increasing the number
> > of constructors.
>
> If you are proposing the new scheme, you have to prove the overhead
> with a significant number of constructors
> and why it has differed from existing scheme of things. That's is the
> norm in opensource.
I say there is no overhead.
The target is to simplify the usage and I prove it:
1 call replacing 2 calls.
On Tue, May 5, 2020 at 12:55 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 05/05/2020 09:17, Jerin Jacob:
> > On Tue, May 5, 2020 at 12:31 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > 05/05/2020 05:43, Jerin Jacob:
> > > > On Tue, May 5, 2020 at 3:01 AM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > > 04/05/2020 19:54, Jerin Jacob:
> > > > > > On Mon, May 4, 2020 at 11:10 PM David Marchand
> > > > > > > On Mon, May 4, 2020 at 7:19 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > On Mon, May 4, 2020 at 10:38 PM David Marchand
> > > > > > > > > On Mon, May 4, 2020 at 4:39 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > > On Mon, May 4, 2020 at 7:34 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > > > > On Mon, May 4, 2020 at 4:47 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > > > > On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> > > > > > > > > > > > > Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > Initially, I thought of doing the same. But, later I realized that
> > > > > > > > > > > > this largely grows the number of constructors been called.
> > > > > > > > > > > > I had concerns about the boot time of the application and/or loading
> > > > > > > > > > > > the shared library, that the reason why spitting
> > > > > > > > > > > > as two so that constructor registers a burst of traces like rte_log.
> > > > > > > > > > >
> > > > > > > > > > > I am a bit skeptical.
> > > > > > > > > > > In terms of cycles and looking at __rte_trace_point_register() (which
> > > > > > > > > > > calls malloc), the cost of calling multiple constructors instead of
> > > > > > > > > > > one is negligible.
> > > > > > > > > >
> > > > > > > > > > We will have a lot tracepoints latter, each one translates to the
> > > > > > > > > > constructor may not be a good
> > > > > > > > > > improvement. The scope is limited only to register function so IMO it
> > > > > > > > > > is okay to have split
> > > > > > > > > > just like rte_log. I don't see any reason why it has to be a different
> > > > > > > > > > than rte_log.
> > > > > > > > >
> > > > > > > > > What is similar to rte_log?
> > > > > > > > > There is neither RTE_LOG_REGISTER macro, nor two-steps declaration of
> > > > > > > > > dynamic logtypes.
> > > > > > > >
> > > > > > > >
> > > > > > > > Here is an example of rte_log registration. Which has _one_
> > > > > > > > constructor and N number of
> > > > > > > > rte_log_register() underneath.
> > > > > > >
> > > > > > > rte_log is one thing, rte_trace is already different.
> > > > > > >
> > > > > > > There is _no macro_ in rte_log for registration.
> > > > > > > The reason being in that a rte_log logtype is a simple integer without
> > > > > > > any special declaration requiring a macro.
> > > > > >
> > > > > > I just wrapped in macro for convincing, but it has the same semantics.
> > > > > > global variable and API/macro to register.
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > For tracepoints, we have a special two steps thing: the tracepoint
> > > > > > > handle must be derived from the tracepoint name.
> > > > > > > Then this handle must be registered.
> > > > > > > What I proposed is to make life easier for developers that want to add
> > > > > > > tracepoints and I suppose you noticed patch 1 of this series.
> > > > > >
> > > > > > To reduce the constructors. I don't want trace libraries to add lot of
> > > > > > constructors.
> > > > > > I don't think it simplifies a lot as the scope of only for registration.
> > > > > >
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > > > > One of the thought process is, we probably remove the constructor
> > > > > > > > > > scheme to all other with DPDK
> > > > > > > > > > and replace it with a more register scheme. In such a case, we can
> > > > > > > > > > skip calling the constructor all tother
> > > > > > > > > > when trace is disabled.
> > > > > > > > >
> > > > > > > > > Sorry, but I have a hard time understanding your point.
> > > > > > > > > Are you talking about application boot time?
> > > > > > > >
> > > > > > > > Yes. The optimization of application boottime time in case of static
> > > > > > > > binary and/or shared library(.so) load time.
> > > > > > >
> > > > > > > As Thomas mentioned, do you have numbers?
> > > > > >
> > > > > > No. But I know, it is obvious that current code is better in terms of
> > > > > > boot time than the proposed one.
> > > > > > I would like to not add a lot of constructor for trace as the FIRST
> > > > > > module in DPDK.
> > > > >
> > > > > No, it is not obvious.
> > > > > The version from David looks simpler to use and to understand.
> > > > > Without any number, I consider usability (and maintenance) wins.
> > > >
> > > > Thanks for the feedback.
> > > >
> > > > As the trace maintainer, I would like not to explode constructor usage
> > > > for trace library.
> > > > My reasoning, We could do trace registration without this constructor scheme.
> > > ???
> >
> > We don't need this patch to make trace to work.
> >
> > >
> > >
> > > > If you think, it is better usability, lets add an option for rte_log
> > > > for the constructor scheme.
> > >
> > > It makes non-sense.
> > > rte_log requires only one function call per log type.
> >
> > Here is the example of the log registration:
> >
> > global variable:
> > int otx2_logtype_base;
> > int otx2_logtype_mbox;
> > int otx2_logtype_npa;
> >
> > RTE_INIT(otx2_log_init);
> > static void
> > otx2_log_init(void)
> > {
> > otx2_logtype_base = rte_log_register("pmd.octeontx2.base");
> > otx2_logtype_mbox = rte_log_register("pmd.octeontx2.mbox");
> > otx2_logtype_npa = rte_log_register("pmd.mempool.octeontx2");
> > }
> >
> > What the proposed patch here.
> > # Making N constructors from one
> > # Grouping global variable and register function under a single Marco
> > and making it as N constructors.
> > Why can we do the same logic for rte_log?
>
> rte_log is simple, there is nothing to simplify.
Why not make, rte_log_register() and the global variable under a macro?
That's something done by the proposed patch.
>
> This comparison makes no sense.
>
>
> > > rte_trace requires 3 macros calls per trace type:
> > > RTE_TRACE_POINT_REGISTER, RTE_TRACE_POINT_DEFINE, RTE_TRACE_POINT_ARGS
> > > This patch is unifying the first 2 macro calls to make usage simpler,
> > > and ease rte_trace adoption.
> >
> > RTE_TRACE_POINT_ARGS is NOP and for the syntax.
> > It is similar to rte_log. rte_log don't have RTE_TRACE_POINT_REGISTER instead
> > it is creating global variable see, "int otx2_logtype_base;
> >
> > >
> > > Note: the other usability weirdness is mandating declaring each trace
> > > function with a magic double underscore prefix which appears nowhere else.
> > >
> > >
> > > > Analyze the impact wrt boot time and cross-platform pov as the log
> > > > has a lot of entries to test. If the usage makes sense then it should make sense
> > > > for rte_log too. I would like to NOT have trace to be the first
> > > > library to explode
> > > > with the constructor scheme. I suggest removing this specific patch from RC2 and
> > > > revisit later.
> > >
> > > You still did not give any argument against increasing the number
> > > of constructors.
> >
> > If you are proposing the new scheme, you have to prove the overhead
> > with a significant number of constructors
> > and why it has differed from existing scheme of things. That's is the
> > norm in opensource.
>
> I say there is no overhead.
Please share the data.
> The target is to simplify the usage and I prove it:
> 1 call replacing 2 calls.
That we can the same scheme with rte_log as well.
>
>
>
On Tue, May 5, 2020 at 9:33 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > What the proposed patch here.
> > > # Making N constructors from one
> > > # Grouping global variable and register function under a single Marco
> > > and making it as N constructors.
> > > Why can we do the same logic for rte_log?
> >
> > rte_log is simple, there is nothing to simplify.
>
> Why not make, rte_log_register() and the global variable under a macro?
> That's something done by the proposed patch.
At the moment, there is not much that would go into such a macro, but
I had started to do some cleanups on logtype registration.
I did not finish because the question of the default log level is
still unclear (and I did not take the time).
Having the logtype definition as part of the macro would be fine to me.
https://patchwork.dpdk.org/patch/57743/
> > > > rte_trace requires 3 macros calls per trace type:
> > > > RTE_TRACE_POINT_REGISTER, RTE_TRACE_POINT_DEFINE, RTE_TRACE_POINT_ARGS
> > > > This patch is unifying the first 2 macro calls to make usage simpler,
> > > > and ease rte_trace adoption.
> > >
> > > RTE_TRACE_POINT_ARGS is NOP and for the syntax.
> > > It is similar to rte_log. rte_log don't have RTE_TRACE_POINT_REGISTER instead
> > > it is creating global variable see, "int otx2_logtype_base;
> > >
> > > >
> > > > Note: the other usability weirdness is mandating declaring each trace
> > > > function with a magic double underscore prefix which appears nowhere else.
> > > >
> > > >
> > > > > Analyze the impact wrt boot time and cross-platform pov as the log
> > > > > has a lot of entries to test. If the usage makes sense then it should make sense
> > > > > for rte_log too. I would like to NOT have trace to be the first
> > > > > library to explode
> > > > > with the constructor scheme. I suggest removing this specific patch from RC2 and
> > > > > revisit later.
> > > >
> > > > You still did not give any argument against increasing the number
> > > > of constructors.
> > >
> > > If you are proposing the new scheme, you have to prove the overhead
> > > with a significant number of constructors
> > > and why it has differed from existing scheme of things. That's is the
> > > norm in opensource.
> >
> > I say there is no overhead.
>
> Please share the data.
Measured time between first rte_trace_point_register and last one with
a simple patch:
diff --git a/lib/librte_eal/common/eal_common_trace.c
b/lib/librte_eal/common/eal_common_trace.c
index 875553d7e..95618314b 100644
--- a/lib/librte_eal/common/eal_common_trace.c
+++ b/lib/librte_eal/common/eal_common_trace.c
@@ -8,6 +8,7 @@
#include <regex.h>
#include <rte_common.h>
+#include <rte_cycles.h>
#include <rte_errno.h>
#include <rte_lcore.h>
#include <rte_per_lcore.h>
@@ -23,6 +24,9 @@ static RTE_DEFINE_PER_LCORE(int, ctf_count);
static struct trace_point_head tp_list = STAILQ_HEAD_INITIALIZER(tp_list);
static struct trace trace = { .args = STAILQ_HEAD_INITIALIZER(trace.args), };
+uint64_t first_register;
+uint64_t last_register;
+
struct trace *
trace_obj_get(void)
{
@@ -43,6 +47,8 @@ eal_trace_init(void)
/* Trace memory should start with 8B aligned for natural alignment */
RTE_BUILD_BUG_ON((offsetof(struct __rte_trace_header, mem) % 8) != 0);
+ trace_err("delta=%"PRIu64, last_register - first_register);
+
/* One of the trace point registration failed */
if (trace.register_errno) {
rte_errno = trace.register_errno;
@@ -425,6 +431,9 @@ __rte_trace_point_register(rte_trace_point_t
*handle, const char *name,
goto fail;
}
+ if (first_register == 0)
+ first_register = rte_get_tsc_cycles();
+
/* Check the size of the trace point object */
RTE_PER_LCORE(trace_point_sz) = 0;
RTE_PER_LCORE(ctf_count) = 0;
@@ -486,6 +495,8 @@ __rte_trace_point_register(rte_trace_point_t
*handle, const char *name,
STAILQ_INSERT_TAIL(&tp_list, tp, next);
__atomic_thread_fence(__ATOMIC_RELEASE);
+ last_register = rte_get_tsc_cycles();
+
/* All Good !!! */
return 0;
free:
I started testpmd 100 times for static and shared gcc builds
(test-meson-builds.sh) on a system with a 2.6GHz xeon.
v20.05-rc1-13-g08dd97130 (before patch):
static: count=100, min=580812, max=1482326, avg=1764932
shared: count=100, min=554648, max=1344163, avg=1704638
v20.05-rc1-14-g44250f392 (after patch):
static: count=100, min=668273, max=1530330, avg=1682548
shared: count=100, min=554634, max=1330264, avg=1672398
On Tue, May 5, 2020 at 1:53 PM David Marchand <david.marchand@redhat.com> wrote:
>
> On Tue, May 5, 2020 at 9:33 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > What the proposed patch here.
> > > > # Making N constructors from one
> > > > # Grouping global variable and register function under a single Marco
> > > > and making it as N constructors.
> > > > Why can we do the same logic for rte_log?
> > >
> > > rte_log is simple, there is nothing to simplify.
> >
> > Why not make, rte_log_register() and the global variable under a macro?
> > That's something done by the proposed patch.
>
> At the moment, there is not much that would go into such a macro, but
> I had started to do some cleanups on logtype registration.
> I did not finish because the question of the default log level is
> still unclear (and I did not take the time).
>
> Having the logtype definition as part of the macro would be fine to me.
> https://patchwork.dpdk.org/patch/57743/
+ Olivier (To get the feedback from rte_log PoV).
The patchwork one is a bit different, IMO, Following is the mapping of
this patch to rte_log one.
Are we OK with the below semantics?
diff --git a/drivers/common/octeontx2/otx2_common.c
b/drivers/common/octeontx2/otx2_common.c
index 1a257cf07..4d391a7e0 100644
--- a/drivers/common/octeontx2/otx2_common.c
+++ b/drivers/common/octeontx2/otx2_common.c
@@ -169,89 +169,13 @@ int otx2_npa_lf_obj_ref(void)
return cnt ? 0 : -EINVAL;
}
-/**
- * @internal
- */
-int otx2_logtype_base;
-/**
- * @internal
- */
-int otx2_logtype_mbox;
-/**
- * @internal
- */
-int otx2_logtype_npa;
-/**
- * @internal
- */
-int otx2_logtype_nix;
-/**
- * @internal
- */
-int otx2_logtype_npc;
-/**
- * @internal
- */
-int otx2_logtype_tm;
-/**
- * @internal
- */
-int otx2_logtype_sso;
-/**
- * @internal
- */
-int otx2_logtype_tim;
-/**
- * @internal
- */
-int otx2_logtype_dpi;
-/**
- * @internal
- */
-int otx2_logtype_ep;
-
-RTE_INIT(otx2_log_init);
-static void
-otx2_log_init(void)
-{
- otx2_logtype_base = rte_log_register("pmd.octeontx2.base");
- if (otx2_logtype_base >= 0)
- rte_log_set_level(otx2_logtype_base, RTE_LOG_NOTICE);
-
- otx2_logtype_mbox = rte_log_register("pmd.octeontx2.mbox");
- if (otx2_logtype_mbox >= 0)
- rte_log_set_level(otx2_logtype_mbox, RTE_LOG_NOTICE);
-
- otx2_logtype_npa = rte_log_register("pmd.mempool.octeontx2");
- if (otx2_logtype_npa >= 0)
- rte_log_set_level(otx2_logtype_npa, RTE_LOG_NOTICE);
-
- otx2_logtype_nix = rte_log_register("pmd.net.octeontx2");
- if (otx2_logtype_nix >= 0)
- rte_log_set_level(otx2_logtype_nix, RTE_LOG_NOTICE);
-
- otx2_logtype_npc = rte_log_register("pmd.net.octeontx2.flow");
- if (otx2_logtype_npc >= 0)
- rte_log_set_level(otx2_logtype_npc, RTE_LOG_NOTICE);
-
- otx2_logtype_tm = rte_log_register("pmd.net.octeontx2.tm");
- if (otx2_logtype_tm >= 0)
- rte_log_set_level(otx2_logtype_tm, RTE_LOG_NOTICE);
-
- otx2_logtype_sso = rte_log_register("pmd.event.octeontx2");
- if (otx2_logtype_sso >= 0)
- rte_log_set_level(otx2_logtype_sso, RTE_LOG_NOTICE);
-
- otx2_logtype_tim = rte_log_register("pmd.event.octeontx2.timer");
- if (otx2_logtype_tim >= 0)
- rte_log_set_level(otx2_logtype_tim, RTE_LOG_NOTICE);
-
- otx2_logtype_dpi = rte_log_register("pmd.raw.octeontx2.dpi");
- if (otx2_logtype_dpi >= 0)
- rte_log_set_level(otx2_logtype_dpi, RTE_LOG_NOTICE);
-
- otx2_logtype_ep = rte_log_register("pmd.raw.octeontx2.ep");
- if (otx2_logtype_ep >= 0)
- rte_log_set_level(otx2_logtype_ep, RTE_LOG_NOTICE);
-
-}
+RTE_LOG_REGISTER(otx2_logtype_base, pmd.octeontx2.base, NOTICE);
+RTE_LOG_REGISTER(otx2_logtype_mbox, pmd.octeontx2.mbox, NOTICE);
+RTE_LOG_REGISTER(otx2_logtype_npa, pmd.mempool.octeontx2, NOTICE);
+RTE_LOG_REGISTER(otx2_logtype_nix, pmd.net.octeontx2, NOTICE);
+RTE_LOG_REGISTER(otx2_logtype_npc, pmd.net.octeontx2.flow, NOTICE);
+RTE_LOG_REGISTER(otx2_logtype_tm, pmd.net.octeontx2.tm, NOTICE);
+RTE_LOG_REGISTER(otx2_logtype_sso, pmd.event.octeontx2, NOTICE);
+RTE_LOG_REGISTER(otx2_logtype_tim, pmd.event.octeontx2.timer, NOTICE);
+RTE_LOG_REGISTER(otx2_logtype_dpi, pmd.raw.octeontx2.dpi, NOTICE);
+RTE_LOG_REGISTER(otx2_logtype_ep, pmd.raw.octeontx2.ep, NOTICE);
diff --git a/lib/librte_eal/include/rte_log.h b/lib/librte_eal/include/rte_log.h
index 1789ede56..22fc3802f 100644
--- a/lib/librte_eal/include/rte_log.h
+++ b/lib/librte_eal/include/rte_log.h
@@ -376,6 +376,15 @@ int rte_vlog(uint32_t level, uint32_t logtype,
const char *format, va_list ap)
RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) : \
0)
+#define RTE_LOG_REGISTER(type, name, level) \
+int type; \
+RTE_INIT(__##type) \
+{ \
+ type = rte_log_register(RTE_STR(name)); \
+ if (type >= 0) \
+ rte_log_set_level(type, RTE_LOG_##level); \
+}
+
#ifdef __cplusplus
}
#endif
>
>
> > > > > rte_trace requires 3 macros calls per trace type:
> > > > > RTE_TRACE_POINT_REGISTER, RTE_TRACE_POINT_DEFINE, RTE_TRACE_POINT_ARGS
> > > > > This patch is unifying the first 2 macro calls to make usage simpler,
> > > > > and ease rte_trace adoption.
> > > >
> > > > RTE_TRACE_POINT_ARGS is NOP and for the syntax.
> > > > It is similar to rte_log. rte_log don't have RTE_TRACE_POINT_REGISTER instead
> > > > it is creating global variable see, "int otx2_logtype_base;
> > > >
> > > > >
> > > > > Note: the other usability weirdness is mandating declaring each trace
> > > > > function with a magic double underscore prefix which appears nowhere else.
> > > > >
> > > > >
> > > > > > Analyze the impact wrt boot time and cross-platform pov as the log
> > > > > > has a lot of entries to test. If the usage makes sense then it should make sense
> > > > > > for rte_log too. I would like to NOT have trace to be the first
> > > > > > library to explode
> > > > > > with the constructor scheme. I suggest removing this specific patch from RC2 and
> > > > > > revisit later.
> > > > >
> > > > > You still did not give any argument against increasing the number
> > > > > of constructors.
> > > >
> > > > If you are proposing the new scheme, you have to prove the overhead
> > > > with a significant number of constructors
> > > > and why it has differed from existing scheme of things. That's is the
> > > > norm in opensource.
> > >
> > > I say there is no overhead.
> >
> > Please share the data.
>
> Measured time between first rte_trace_point_register and last one with
> a simple patch:
I will try to reproduce this, once we finalize on the above synergy
with rte_log.
>
> diff --git a/lib/librte_eal/common/eal_common_trace.c
> b/lib/librte_eal/common/eal_common_trace.c
> index 875553d7e..95618314b 100644
> --- a/lib/librte_eal/common/eal_common_trace.c
> +++ b/lib/librte_eal/common/eal_common_trace.c
> @@ -8,6 +8,7 @@
> #include <regex.h>
>
> #include <rte_common.h>
> +#include <rte_cycles.h>
> #include <rte_errno.h>
> #include <rte_lcore.h>
> #include <rte_per_lcore.h>
> @@ -23,6 +24,9 @@ static RTE_DEFINE_PER_LCORE(int, ctf_count);
> static struct trace_point_head tp_list = STAILQ_HEAD_INITIALIZER(tp_list);
> static struct trace trace = { .args = STAILQ_HEAD_INITIALIZER(trace.args), };
>
> +uint64_t first_register;
> +uint64_t last_register;
> +
> struct trace *
> trace_obj_get(void)
> {
> @@ -43,6 +47,8 @@ eal_trace_init(void)
> /* Trace memory should start with 8B aligned for natural alignment */
> RTE_BUILD_BUG_ON((offsetof(struct __rte_trace_header, mem) % 8) != 0);
>
> + trace_err("delta=%"PRIu64, last_register - first_register);
> +
> /* One of the trace point registration failed */
> if (trace.register_errno) {
> rte_errno = trace.register_errno;
> @@ -425,6 +431,9 @@ __rte_trace_point_register(rte_trace_point_t
> *handle, const char *name,
> goto fail;
> }
>
> + if (first_register == 0)
> + first_register = rte_get_tsc_cycles();
> +
> /* Check the size of the trace point object */
> RTE_PER_LCORE(trace_point_sz) = 0;
> RTE_PER_LCORE(ctf_count) = 0;
> @@ -486,6 +495,8 @@ __rte_trace_point_register(rte_trace_point_t
> *handle, const char *name,
> STAILQ_INSERT_TAIL(&tp_list, tp, next);
> __atomic_thread_fence(__ATOMIC_RELEASE);
>
> + last_register = rte_get_tsc_cycles();
> +
> /* All Good !!! */
> return 0;
> free:
>
>
> I started testpmd 100 times for static and shared gcc builds
> (test-meson-builds.sh) on a system with a 2.6GHz xeon.
>
> v20.05-rc1-13-g08dd97130 (before patch):
> static: count=100, min=580812, max=1482326, avg=1764932
> shared: count=100, min=554648, max=1344163, avg=1704638
>
> v20.05-rc1-14-g44250f392 (after patch):
> static: count=100, min=668273, max=1530330, avg=1682548
> shared: count=100, min=554634, max=1330264, avg=1672398
>
>
>
> --
> David Marchand
>
05/05/2020 12:12, Jerin Jacob:
> On Tue, May 5, 2020 at 1:53 PM David Marchand <david.marchand@redhat.com> wrote:
> > On Tue, May 5, 2020 at 9:33 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > What the proposed patch here.
> > > > > # Making N constructors from one
> > > > > # Grouping global variable and register function under a single Marco
> > > > > and making it as N constructors.
> > > > > Why can we do the same logic for rte_log?
> > > >
> > > > rte_log is simple, there is nothing to simplify.
> > >
> > > Why not make, rte_log_register() and the global variable under a macro?
> > > That's something done by the proposed patch.
> >
> > At the moment, there is not much that would go into such a macro, but
> > I had started to do some cleanups on logtype registration.
> > I did not finish because the question of the default log level is
> > still unclear (and I did not take the time).
> >
> > Having the logtype definition as part of the macro would be fine to me.
> > https://patchwork.dpdk.org/patch/57743/
>
> + Olivier (To get the feedback from rte_log PoV).
>
> The patchwork one is a bit different, IMO, Following is the mapping of
> this patch to rte_log one.
>
> Are we OK with the below semantics?
>
>
> diff --git a/drivers/common/octeontx2/otx2_common.c
> b/drivers/common/octeontx2/otx2_common.c
> index 1a257cf07..4d391a7e0 100644
> --- a/drivers/common/octeontx2/otx2_common.c
> +++ b/drivers/common/octeontx2/otx2_common.c
> @@ -169,89 +169,13 @@ int otx2_npa_lf_obj_ref(void)
> return cnt ? 0 : -EINVAL;
> }
> -/**
> - * @internal
> - */
> -int otx2_logtype_base;
> -/**
> - * @internal
> - */
> -int otx2_logtype_mbox;
> -/**
> - * @internal
> - */
> -int otx2_logtype_npa;
> -/**
> - * @internal
> - */
> -int otx2_logtype_nix;
> -/**
> - * @internal
> - */
> -int otx2_logtype_npc;
> -/**
> - * @internal
> - */
> -int otx2_logtype_tm;
> -/**
> - * @internal
> - */
> -int otx2_logtype_sso;
> -/**
> - * @internal
> - */
> -int otx2_logtype_tim;
> -/**
> - * @internal
> - */
> -int otx2_logtype_dpi;
> -/**
> - * @internal
> - */
> -int otx2_logtype_ep;
> -
> -RTE_INIT(otx2_log_init);
> -static void
> -otx2_log_init(void)
> -{
> - otx2_logtype_base = rte_log_register("pmd.octeontx2.base");
> - if (otx2_logtype_base >= 0)
> - rte_log_set_level(otx2_logtype_base, RTE_LOG_NOTICE);
> -
> - otx2_logtype_mbox = rte_log_register("pmd.octeontx2.mbox");
> - if (otx2_logtype_mbox >= 0)
> - rte_log_set_level(otx2_logtype_mbox, RTE_LOG_NOTICE);
> -
> - otx2_logtype_npa = rte_log_register("pmd.mempool.octeontx2");
> - if (otx2_logtype_npa >= 0)
> - rte_log_set_level(otx2_logtype_npa, RTE_LOG_NOTICE);
> -
> - otx2_logtype_nix = rte_log_register("pmd.net.octeontx2");
> - if (otx2_logtype_nix >= 0)
> - rte_log_set_level(otx2_logtype_nix, RTE_LOG_NOTICE);
> -
> - otx2_logtype_npc = rte_log_register("pmd.net.octeontx2.flow");
> - if (otx2_logtype_npc >= 0)
> - rte_log_set_level(otx2_logtype_npc, RTE_LOG_NOTICE);
> -
> - otx2_logtype_tm = rte_log_register("pmd.net.octeontx2.tm");
> - if (otx2_logtype_tm >= 0)
> - rte_log_set_level(otx2_logtype_tm, RTE_LOG_NOTICE);
> -
> - otx2_logtype_sso = rte_log_register("pmd.event.octeontx2");
> - if (otx2_logtype_sso >= 0)
> - rte_log_set_level(otx2_logtype_sso, RTE_LOG_NOTICE);
> -
> - otx2_logtype_tim = rte_log_register("pmd.event.octeontx2.timer");
> - if (otx2_logtype_tim >= 0)
> - rte_log_set_level(otx2_logtype_tim, RTE_LOG_NOTICE);
> -
> - otx2_logtype_dpi = rte_log_register("pmd.raw.octeontx2.dpi");
> - if (otx2_logtype_dpi >= 0)
> - rte_log_set_level(otx2_logtype_dpi, RTE_LOG_NOTICE);
> -
> - otx2_logtype_ep = rte_log_register("pmd.raw.octeontx2.ep");
> - if (otx2_logtype_ep >= 0)
> - rte_log_set_level(otx2_logtype_ep, RTE_LOG_NOTICE);
> -
> -}
> +RTE_LOG_REGISTER(otx2_logtype_base, pmd.octeontx2.base, NOTICE);
> +RTE_LOG_REGISTER(otx2_logtype_mbox, pmd.octeontx2.mbox, NOTICE);
> +RTE_LOG_REGISTER(otx2_logtype_npa, pmd.mempool.octeontx2, NOTICE);
> +RTE_LOG_REGISTER(otx2_logtype_nix, pmd.net.octeontx2, NOTICE);
> +RTE_LOG_REGISTER(otx2_logtype_npc, pmd.net.octeontx2.flow, NOTICE);
> +RTE_LOG_REGISTER(otx2_logtype_tm, pmd.net.octeontx2.tm, NOTICE);
> +RTE_LOG_REGISTER(otx2_logtype_sso, pmd.event.octeontx2, NOTICE);
> +RTE_LOG_REGISTER(otx2_logtype_tim, pmd.event.octeontx2.timer, NOTICE);
> +RTE_LOG_REGISTER(otx2_logtype_dpi, pmd.raw.octeontx2.dpi, NOTICE);
> +RTE_LOG_REGISTER(otx2_logtype_ep, pmd.raw.octeontx2.ep, NOTICE);
>
> diff --git a/lib/librte_eal/include/rte_log.h b/lib/librte_eal/include/rte_log.h
> index 1789ede56..22fc3802f 100644
> --- a/lib/librte_eal/include/rte_log.h
> +++ b/lib/librte_eal/include/rte_log.h
> @@ -376,6 +376,15 @@ int rte_vlog(uint32_t level, uint32_t logtype,
> const char *format, va_list ap)
> RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) : \
> 0)
>
> +#define RTE_LOG_REGISTER(type, name, level) \
> +int type; \
> +RTE_INIT(__##type) \
> +{ \
> + type = rte_log_register(RTE_STR(name)); \
> + if (type >= 0) \
> + rte_log_set_level(type, RTE_LOG_##level); \
> +}
> +
Yes I agree, we could do that.
And now I better understand what you mean comparing rte_trace and rte_log.
> > > > > > rte_trace requires 3 macros calls per trace type:
> > > > > > RTE_TRACE_POINT_REGISTER, RTE_TRACE_POINT_DEFINE, RTE_TRACE_POINT_ARGS
> > > > > > This patch is unifying the first 2 macro calls to make usage simpler,
> > > > > > and ease rte_trace adoption.
> > > > >
> > > > > RTE_TRACE_POINT_ARGS is NOP and for the syntax.
> > > > > It is similar to rte_log. rte_log don't have RTE_TRACE_POINT_REGISTER instead
> > > > > it is creating global variable see, "int otx2_logtype_base;
> > > > >
> > > > > >
> > > > > > Note: the other usability weirdness is mandating declaring each trace
> > > > > > function with a magic double underscore prefix which appears nowhere else.
> > > > > >
> > > > > >
> > > > > > > Analyze the impact wrt boot time and cross-platform pov as the log
> > > > > > > has a lot of entries to test. If the usage makes sense then it should make sense
> > > > > > > for rte_log too. I would like to NOT have trace to be the first
> > > > > > > library to explode
> > > > > > > with the constructor scheme. I suggest removing this specific patch from RC2 and
> > > > > > > revisit later.
> > > > > >
> > > > > > You still did not give any argument against increasing the number
> > > > > > of constructors.
> > > > >
> > > > > If you are proposing the new scheme, you have to prove the overhead
> > > > > with a significant number of constructors
> > > > > and why it has differed from existing scheme of things. That's is the
> > > > > norm in opensource.
> > > >
> > > > I say there is no overhead.
> > >
> > > Please share the data.
> >
> > Measured time between first rte_trace_point_register and last one with
> > a simple patch:
>
> I will try to reproduce this, once we finalize on the above synergy
> with rte_log.
>
>
> >
> > diff --git a/lib/librte_eal/common/eal_common_trace.c
> > b/lib/librte_eal/common/eal_common_trace.c
> > index 875553d7e..95618314b 100644
> > --- a/lib/librte_eal/common/eal_common_trace.c
> > +++ b/lib/librte_eal/common/eal_common_trace.c
> > @@ -8,6 +8,7 @@
> > #include <regex.h>
> >
> > #include <rte_common.h>
> > +#include <rte_cycles.h>
> > #include <rte_errno.h>
> > #include <rte_lcore.h>
> > #include <rte_per_lcore.h>
> > @@ -23,6 +24,9 @@ static RTE_DEFINE_PER_LCORE(int, ctf_count);
> > static struct trace_point_head tp_list = STAILQ_HEAD_INITIALIZER(tp_list);
> > static struct trace trace = { .args = STAILQ_HEAD_INITIALIZER(trace.args), };
> >
> > +uint64_t first_register;
> > +uint64_t last_register;
> > +
> > struct trace *
> > trace_obj_get(void)
> > {
> > @@ -43,6 +47,8 @@ eal_trace_init(void)
> > /* Trace memory should start with 8B aligned for natural alignment */
> > RTE_BUILD_BUG_ON((offsetof(struct __rte_trace_header, mem) % 8) != 0);
> >
> > + trace_err("delta=%"PRIu64, last_register - first_register);
> > +
> > /* One of the trace point registration failed */
> > if (trace.register_errno) {
> > rte_errno = trace.register_errno;
> > @@ -425,6 +431,9 @@ __rte_trace_point_register(rte_trace_point_t
> > *handle, const char *name,
> > goto fail;
> > }
> >
> > + if (first_register == 0)
> > + first_register = rte_get_tsc_cycles();
> > +
> > /* Check the size of the trace point object */
> > RTE_PER_LCORE(trace_point_sz) = 0;
> > RTE_PER_LCORE(ctf_count) = 0;
> > @@ -486,6 +495,8 @@ __rte_trace_point_register(rte_trace_point_t
> > *handle, const char *name,
> > STAILQ_INSERT_TAIL(&tp_list, tp, next);
> > __atomic_thread_fence(__ATOMIC_RELEASE);
> >
> > + last_register = rte_get_tsc_cycles();
> > +
> > /* All Good !!! */
> > return 0;
> > free:
> >
> >
> > I started testpmd 100 times for static and shared gcc builds
> > (test-meson-builds.sh) on a system with a 2.6GHz xeon.
> >
> > v20.05-rc1-13-g08dd97130 (before patch):
> > static: count=100, min=580812, max=1482326, avg=1764932
> > shared: count=100, min=554648, max=1344163, avg=1704638
> >
> > v20.05-rc1-14-g44250f392 (after patch):
> > static: count=100, min=668273, max=1530330, avg=1682548
> > shared: count=100, min=554634, max=1330264, avg=1672398
On Tue, May 5, 2020 at 3:56 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 05/05/2020 12:12, Jerin Jacob:
> > On Tue, May 5, 2020 at 1:53 PM David Marchand <david.marchand@redhat.com> wrote:
> > > On Tue, May 5, 2020 at 9:33 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > What the proposed patch here.
> > > > > > # Making N constructors from one
> > > > > > # Grouping global variable and register function under a single Marco
> > > > > > and making it as N constructors.
> > > > > > Why can we do the same logic for rte_log?
> > > > >
> > > > > rte_log is simple, there is nothing to simplify.
> > > >
> > > > Why not make, rte_log_register() and the global variable under a macro?
> > > > That's something done by the proposed patch.
> > >
> > > At the moment, there is not much that would go into such a macro, but
> > > I had started to do some cleanups on logtype registration.
> > > I did not finish because the question of the default log level is
> > > still unclear (and I did not take the time).
> > >
> > > Having the logtype definition as part of the macro would be fine to me.
> > > https://patchwork.dpdk.org/patch/57743/
> >
> > + Olivier (To get the feedback from rte_log PoV).
> >
> > The patchwork one is a bit different, IMO, Following is the mapping of
> > this patch to rte_log one.
> >
> > Are we OK with the below semantics?
> >
> >
> > diff --git a/drivers/common/octeontx2/otx2_common.c
> > b/drivers/common/octeontx2/otx2_common.c
> > index 1a257cf07..4d391a7e0 100644
> > --- a/drivers/common/octeontx2/otx2_common.c
> > +++ b/drivers/common/octeontx2/otx2_common.c
> > @@ -169,89 +169,13 @@ int otx2_npa_lf_obj_ref(void)
> > return cnt ? 0 : -EINVAL;
> > }
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_base;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_mbox;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_npa;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_nix;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_npc;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_tm;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_sso;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_tim;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_dpi;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_ep;
> > -
> > -RTE_INIT(otx2_log_init);
> > -static void
> > -otx2_log_init(void)
> > -{
> > - otx2_logtype_base = rte_log_register("pmd.octeontx2.base");
> > - if (otx2_logtype_base >= 0)
> > - rte_log_set_level(otx2_logtype_base, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_mbox = rte_log_register("pmd.octeontx2.mbox");
> > - if (otx2_logtype_mbox >= 0)
> > - rte_log_set_level(otx2_logtype_mbox, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_npa = rte_log_register("pmd.mempool.octeontx2");
> > - if (otx2_logtype_npa >= 0)
> > - rte_log_set_level(otx2_logtype_npa, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_nix = rte_log_register("pmd.net.octeontx2");
> > - if (otx2_logtype_nix >= 0)
> > - rte_log_set_level(otx2_logtype_nix, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_npc = rte_log_register("pmd.net.octeontx2.flow");
> > - if (otx2_logtype_npc >= 0)
> > - rte_log_set_level(otx2_logtype_npc, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_tm = rte_log_register("pmd.net.octeontx2.tm");
> > - if (otx2_logtype_tm >= 0)
> > - rte_log_set_level(otx2_logtype_tm, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_sso = rte_log_register("pmd.event.octeontx2");
> > - if (otx2_logtype_sso >= 0)
> > - rte_log_set_level(otx2_logtype_sso, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_tim = rte_log_register("pmd.event.octeontx2.timer");
> > - if (otx2_logtype_tim >= 0)
> > - rte_log_set_level(otx2_logtype_tim, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_dpi = rte_log_register("pmd.raw.octeontx2.dpi");
> > - if (otx2_logtype_dpi >= 0)
> > - rte_log_set_level(otx2_logtype_dpi, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_ep = rte_log_register("pmd.raw.octeontx2.ep");
> > - if (otx2_logtype_ep >= 0)
> > - rte_log_set_level(otx2_logtype_ep, RTE_LOG_NOTICE);
> > -
> > -}
> > +RTE_LOG_REGISTER(otx2_logtype_base, pmd.octeontx2.base, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_mbox, pmd.octeontx2.mbox, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_npa, pmd.mempool.octeontx2, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_nix, pmd.net.octeontx2, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_npc, pmd.net.octeontx2.flow, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_tm, pmd.net.octeontx2.tm, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_sso, pmd.event.octeontx2, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_tim, pmd.event.octeontx2.timer, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_dpi, pmd.raw.octeontx2.dpi, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_ep, pmd.raw.octeontx2.ep, NOTICE);
> >
> > diff --git a/lib/librte_eal/include/rte_log.h b/lib/librte_eal/include/rte_log.h
> > index 1789ede56..22fc3802f 100644
> > --- a/lib/librte_eal/include/rte_log.h
> > +++ b/lib/librte_eal/include/rte_log.h
> > @@ -376,6 +376,15 @@ int rte_vlog(uint32_t level, uint32_t logtype,
> > const char *format, va_list ap)
> > RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) : \
> > 0)
> >
> > +#define RTE_LOG_REGISTER(type, name, level) \
> > +int type; \
> > +RTE_INIT(__##type) \
> > +{ \
> > + type = rte_log_register(RTE_STR(name)); \
> > + if (type >= 0) \
> > + rte_log_set_level(type, RTE_LOG_##level); \
> > +}
> > +
>
>
> Yes I agree, we could do that.
> And now I better understand what you mean comparing rte_trace and rte_log.
OK.
Let Olivier share his view, I was/is under the impression that, The
reason for not have this silly Marco to
don't explode constructor usage in dpdk
If we are OK this scheme then lets first clean up rte_log registration.
>
>
>
> > > > > > > rte_trace requires 3 macros calls per trace type:
> > > > > > > RTE_TRACE_POINT_REGISTER, RTE_TRACE_POINT_DEFINE, RTE_TRACE_POINT_ARGS
> > > > > > > This patch is unifying the first 2 macro calls to make usage simpler,
> > > > > > > and ease rte_trace adoption.
> > > > > >
> > > > > > RTE_TRACE_POINT_ARGS is NOP and for the syntax.
> > > > > > It is similar to rte_log. rte_log don't have RTE_TRACE_POINT_REGISTER instead
> > > > > > it is creating global variable see, "int otx2_logtype_base;
> > > > > >
> > > > > > >
> > > > > > > Note: the other usability weirdness is mandating declaring each trace
> > > > > > > function with a magic double underscore prefix which appears nowhere else.
> > > > > > >
> > > > > > >
> > > > > > > > Analyze the impact wrt boot time and cross-platform pov as the log
> > > > > > > > has a lot of entries to test. If the usage makes sense then it should make sense
> > > > > > > > for rte_log too. I would like to NOT have trace to be the first
> > > > > > > > library to explode
> > > > > > > > with the constructor scheme. I suggest removing this specific patch from RC2 and
> > > > > > > > revisit later.
> > > > > > >
> > > > > > > You still did not give any argument against increasing the number
> > > > > > > of constructors.
> > > > > >
> > > > > > If you are proposing the new scheme, you have to prove the overhead
> > > > > > with a significant number of constructors
> > > > > > and why it has differed from existing scheme of things. That's is the
> > > > > > norm in opensource.
> > > > >
> > > > > I say there is no overhead.
> > > >
> > > > Please share the data.
> > >
> > > Measured time between first rte_trace_point_register and last one with
> > > a simple patch:
> >
> > I will try to reproduce this, once we finalize on the above synergy
> > with rte_log.
> >
> >
> > >
> > > diff --git a/lib/librte_eal/common/eal_common_trace.c
> > > b/lib/librte_eal/common/eal_common_trace.c
> > > index 875553d7e..95618314b 100644
> > > --- a/lib/librte_eal/common/eal_common_trace.c
> > > +++ b/lib/librte_eal/common/eal_common_trace.c
> > > @@ -8,6 +8,7 @@
> > > #include <regex.h>
> > >
> > > #include <rte_common.h>
> > > +#include <rte_cycles.h>
> > > #include <rte_errno.h>
> > > #include <rte_lcore.h>
> > > #include <rte_per_lcore.h>
> > > @@ -23,6 +24,9 @@ static RTE_DEFINE_PER_LCORE(int, ctf_count);
> > > static struct trace_point_head tp_list = STAILQ_HEAD_INITIALIZER(tp_list);
> > > static struct trace trace = { .args = STAILQ_HEAD_INITIALIZER(trace.args), };
> > >
> > > +uint64_t first_register;
> > > +uint64_t last_register;
> > > +
> > > struct trace *
> > > trace_obj_get(void)
> > > {
> > > @@ -43,6 +47,8 @@ eal_trace_init(void)
> > > /* Trace memory should start with 8B aligned for natural alignment */
> > > RTE_BUILD_BUG_ON((offsetof(struct __rte_trace_header, mem) % 8) != 0);
> > >
> > > + trace_err("delta=%"PRIu64, last_register - first_register);
> > > +
> > > /* One of the trace point registration failed */
> > > if (trace.register_errno) {
> > > rte_errno = trace.register_errno;
> > > @@ -425,6 +431,9 @@ __rte_trace_point_register(rte_trace_point_t
> > > *handle, const char *name,
> > > goto fail;
> > > }
> > >
> > > + if (first_register == 0)
> > > + first_register = rte_get_tsc_cycles();
> > > +
> > > /* Check the size of the trace point object */
> > > RTE_PER_LCORE(trace_point_sz) = 0;
> > > RTE_PER_LCORE(ctf_count) = 0;
> > > @@ -486,6 +495,8 @@ __rte_trace_point_register(rte_trace_point_t
> > > *handle, const char *name,
> > > STAILQ_INSERT_TAIL(&tp_list, tp, next);
> > > __atomic_thread_fence(__ATOMIC_RELEASE);
> > >
> > > + last_register = rte_get_tsc_cycles();
> > > +
> > > /* All Good !!! */
> > > return 0;
> > > free:
> > >
> > >
> > > I started testpmd 100 times for static and shared gcc builds
> > > (test-meson-builds.sh) on a system with a 2.6GHz xeon.
> > >
> > > v20.05-rc1-13-g08dd97130 (before patch):
> > > static: count=100, min=580812, max=1482326, avg=1764932
> > > shared: count=100, min=554648, max=1344163, avg=1704638
> > >
> > > v20.05-rc1-14-g44250f392 (after patch):
> > > static: count=100, min=668273, max=1530330, avg=1682548
> > > shared: count=100, min=554634, max=1330264, avg=1672398
>
>
>
>
On Tue, May 5, 2020 at 12:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > Please share the data.
> >
> > Measured time between first rte_trace_point_register and last one with
> > a simple patch:
>
> I will try to reproduce this, once we finalize on the above synergy
> with rte_log.
I took the time to provide measure but you won't take the time to look at this.
Really nice.
Hi,
On Tue, May 05, 2020 at 04:16:02PM +0530, Jerin Jacob wrote:
> On Tue, May 5, 2020 at 3:56 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > 05/05/2020 12:12, Jerin Jacob:
> > > On Tue, May 5, 2020 at 1:53 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > On Tue, May 5, 2020 at 9:33 AM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > What the proposed patch here.
> > > > > > > # Making N constructors from one
> > > > > > > # Grouping global variable and register function under a single Marco
> > > > > > > and making it as N constructors.
> > > > > > > Why can we do the same logic for rte_log?
> > > > > >
> > > > > > rte_log is simple, there is nothing to simplify.
> > > > >
> > > > > Why not make, rte_log_register() and the global variable under a macro?
> > > > > That's something done by the proposed patch.
> > > >
> > > > At the moment, there is not much that would go into such a macro, but
> > > > I had started to do some cleanups on logtype registration.
> > > > I did not finish because the question of the default log level is
> > > > still unclear (and I did not take the time).
> > > >
> > > > Having the logtype definition as part of the macro would be fine to me.
> > > > https://patchwork.dpdk.org/patch/57743/
> > >
> > > + Olivier (To get the feedback from rte_log PoV).
> > >
> > > The patchwork one is a bit different, IMO, Following is the mapping of
> > > this patch to rte_log one.
> > >
> > > Are we OK with the below semantics?
> > >
> > >
> > > diff --git a/drivers/common/octeontx2/otx2_common.c
> > > b/drivers/common/octeontx2/otx2_common.c
> > > index 1a257cf07..4d391a7e0 100644
> > > --- a/drivers/common/octeontx2/otx2_common.c
> > > +++ b/drivers/common/octeontx2/otx2_common.c
> > > @@ -169,89 +169,13 @@ int otx2_npa_lf_obj_ref(void)
> > > return cnt ? 0 : -EINVAL;
> > > }
> > > -/**
> > > - * @internal
> > > - */
> > > -int otx2_logtype_base;
> > > -/**
> > > - * @internal
> > > - */
> > > -int otx2_logtype_mbox;
> > > -/**
> > > - * @internal
> > > - */
> > > -int otx2_logtype_npa;
> > > -/**
> > > - * @internal
> > > - */
> > > -int otx2_logtype_nix;
> > > -/**
> > > - * @internal
> > > - */
> > > -int otx2_logtype_npc;
> > > -/**
> > > - * @internal
> > > - */
> > > -int otx2_logtype_tm;
> > > -/**
> > > - * @internal
> > > - */
> > > -int otx2_logtype_sso;
> > > -/**
> > > - * @internal
> > > - */
> > > -int otx2_logtype_tim;
> > > -/**
> > > - * @internal
> > > - */
> > > -int otx2_logtype_dpi;
> > > -/**
> > > - * @internal
> > > - */
> > > -int otx2_logtype_ep;
> > > -
> > > -RTE_INIT(otx2_log_init);
> > > -static void
> > > -otx2_log_init(void)
> > > -{
> > > - otx2_logtype_base = rte_log_register("pmd.octeontx2.base");
> > > - if (otx2_logtype_base >= 0)
> > > - rte_log_set_level(otx2_logtype_base, RTE_LOG_NOTICE);
> > > -
> > > - otx2_logtype_mbox = rte_log_register("pmd.octeontx2.mbox");
> > > - if (otx2_logtype_mbox >= 0)
> > > - rte_log_set_level(otx2_logtype_mbox, RTE_LOG_NOTICE);
> > > -
> > > - otx2_logtype_npa = rte_log_register("pmd.mempool.octeontx2");
> > > - if (otx2_logtype_npa >= 0)
> > > - rte_log_set_level(otx2_logtype_npa, RTE_LOG_NOTICE);
> > > -
> > > - otx2_logtype_nix = rte_log_register("pmd.net.octeontx2");
> > > - if (otx2_logtype_nix >= 0)
> > > - rte_log_set_level(otx2_logtype_nix, RTE_LOG_NOTICE);
> > > -
> > > - otx2_logtype_npc = rte_log_register("pmd.net.octeontx2.flow");
> > > - if (otx2_logtype_npc >= 0)
> > > - rte_log_set_level(otx2_logtype_npc, RTE_LOG_NOTICE);
> > > -
> > > - otx2_logtype_tm = rte_log_register("pmd.net.octeontx2.tm");
> > > - if (otx2_logtype_tm >= 0)
> > > - rte_log_set_level(otx2_logtype_tm, RTE_LOG_NOTICE);
> > > -
> > > - otx2_logtype_sso = rte_log_register("pmd.event.octeontx2");
> > > - if (otx2_logtype_sso >= 0)
> > > - rte_log_set_level(otx2_logtype_sso, RTE_LOG_NOTICE);
> > > -
> > > - otx2_logtype_tim = rte_log_register("pmd.event.octeontx2.timer");
> > > - if (otx2_logtype_tim >= 0)
> > > - rte_log_set_level(otx2_logtype_tim, RTE_LOG_NOTICE);
> > > -
> > > - otx2_logtype_dpi = rte_log_register("pmd.raw.octeontx2.dpi");
> > > - if (otx2_logtype_dpi >= 0)
> > > - rte_log_set_level(otx2_logtype_dpi, RTE_LOG_NOTICE);
> > > -
> > > - otx2_logtype_ep = rte_log_register("pmd.raw.octeontx2.ep");
> > > - if (otx2_logtype_ep >= 0)
> > > - rte_log_set_level(otx2_logtype_ep, RTE_LOG_NOTICE);
> > > -
> > > -}
> > > +RTE_LOG_REGISTER(otx2_logtype_base, pmd.octeontx2.base, NOTICE);
> > > +RTE_LOG_REGISTER(otx2_logtype_mbox, pmd.octeontx2.mbox, NOTICE);
> > > +RTE_LOG_REGISTER(otx2_logtype_npa, pmd.mempool.octeontx2, NOTICE);
> > > +RTE_LOG_REGISTER(otx2_logtype_nix, pmd.net.octeontx2, NOTICE);
> > > +RTE_LOG_REGISTER(otx2_logtype_npc, pmd.net.octeontx2.flow, NOTICE);
> > > +RTE_LOG_REGISTER(otx2_logtype_tm, pmd.net.octeontx2.tm, NOTICE);
> > > +RTE_LOG_REGISTER(otx2_logtype_sso, pmd.event.octeontx2, NOTICE);
> > > +RTE_LOG_REGISTER(otx2_logtype_tim, pmd.event.octeontx2.timer, NOTICE);
> > > +RTE_LOG_REGISTER(otx2_logtype_dpi, pmd.raw.octeontx2.dpi, NOTICE);
> > > +RTE_LOG_REGISTER(otx2_logtype_ep, pmd.raw.octeontx2.ep, NOTICE);
> > >
> > > diff --git a/lib/librte_eal/include/rte_log.h b/lib/librte_eal/include/rte_log.h
> > > index 1789ede56..22fc3802f 100644
> > > --- a/lib/librte_eal/include/rte_log.h
> > > +++ b/lib/librte_eal/include/rte_log.h
> > > @@ -376,6 +376,15 @@ int rte_vlog(uint32_t level, uint32_t logtype,
> > > const char *format, va_list ap)
> > > RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) : \
> > > 0)
> > >
> > > +#define RTE_LOG_REGISTER(type, name, level) \
> > > +int type; \
> > > +RTE_INIT(__##type) \
> > > +{ \
> > > + type = rte_log_register(RTE_STR(name)); \
> > > + if (type >= 0) \
> > > + rte_log_set_level(type, RTE_LOG_##level); \
> > > +}
> > > +
> >
> >
> > Yes I agree, we could do that.
> > And now I better understand what you mean comparing rte_trace and rte_log.
>
> OK.
>
> Let Olivier share his view, I was/is under the impression that, The
> reason for not have this silly Marco to
> don't explode constructor usage in dpdk
>
> If we are OK this scheme then lets first clean up rte_log registration.
Honnestly, I had no particular idea in mind about constructor number
when I added the rte_log_register() API. To me, it was quite simple:
just call a register function when you need a new log type. Now, as it's
mostly (always?) done at init time, I'm fine with the the principle of
having a macro to register new logs, given we also keep the previous
API.
To get back on the topic of the thread (RTE_TRACE), I think a simpler
API (one macro) is better. Since it's a new API, it makes sense to make
it as good as possible for the first version.
And by the way, thank you for this nice work.
Regards,
Olivier
>
>
> >
> >
> >
> > > > > > > > rte_trace requires 3 macros calls per trace type:
> > > > > > > > RTE_TRACE_POINT_REGISTER, RTE_TRACE_POINT_DEFINE, RTE_TRACE_POINT_ARGS
> > > > > > > > This patch is unifying the first 2 macro calls to make usage simpler,
> > > > > > > > and ease rte_trace adoption.
> > > > > > >
> > > > > > > RTE_TRACE_POINT_ARGS is NOP and for the syntax.
> > > > > > > It is similar to rte_log. rte_log don't have RTE_TRACE_POINT_REGISTER instead
> > > > > > > it is creating global variable see, "int otx2_logtype_base;
> > > > > > >
> > > > > > > >
> > > > > > > > Note: the other usability weirdness is mandating declaring each trace
> > > > > > > > function with a magic double underscore prefix which appears nowhere else.
> > > > > > > >
> > > > > > > >
> > > > > > > > > Analyze the impact wrt boot time and cross-platform pov as the log
> > > > > > > > > has a lot of entries to test. If the usage makes sense then it should make sense
> > > > > > > > > for rte_log too. I would like to NOT have trace to be the first
> > > > > > > > > library to explode
> > > > > > > > > with the constructor scheme. I suggest removing this specific patch from RC2 and
> > > > > > > > > revisit later.
> > > > > > > >
> > > > > > > > You still did not give any argument against increasing the number
> > > > > > > > of constructors.
> > > > > > >
> > > > > > > If you are proposing the new scheme, you have to prove the overhead
> > > > > > > with a significant number of constructors
> > > > > > > and why it has differed from existing scheme of things. That's is the
> > > > > > > norm in opensource.
> > > > > >
> > > > > > I say there is no overhead.
> > > > >
> > > > > Please share the data.
> > > >
> > > > Measured time between first rte_trace_point_register and last one with
> > > > a simple patch:
> > >
> > > I will try to reproduce this, once we finalize on the above synergy
> > > with rte_log.
> > >
> > >
> > > >
> > > > diff --git a/lib/librte_eal/common/eal_common_trace.c
> > > > b/lib/librte_eal/common/eal_common_trace.c
> > > > index 875553d7e..95618314b 100644
> > > > --- a/lib/librte_eal/common/eal_common_trace.c
> > > > +++ b/lib/librte_eal/common/eal_common_trace.c
> > > > @@ -8,6 +8,7 @@
> > > > #include <regex.h>
> > > >
> > > > #include <rte_common.h>
> > > > +#include <rte_cycles.h>
> > > > #include <rte_errno.h>
> > > > #include <rte_lcore.h>
> > > > #include <rte_per_lcore.h>
> > > > @@ -23,6 +24,9 @@ static RTE_DEFINE_PER_LCORE(int, ctf_count);
> > > > static struct trace_point_head tp_list = STAILQ_HEAD_INITIALIZER(tp_list);
> > > > static struct trace trace = { .args = STAILQ_HEAD_INITIALIZER(trace.args), };
> > > >
> > > > +uint64_t first_register;
> > > > +uint64_t last_register;
> > > > +
> > > > struct trace *
> > > > trace_obj_get(void)
> > > > {
> > > > @@ -43,6 +47,8 @@ eal_trace_init(void)
> > > > /* Trace memory should start with 8B aligned for natural alignment */
> > > > RTE_BUILD_BUG_ON((offsetof(struct __rte_trace_header, mem) % 8) != 0);
> > > >
> > > > + trace_err("delta=%"PRIu64, last_register - first_register);
> > > > +
> > > > /* One of the trace point registration failed */
> > > > if (trace.register_errno) {
> > > > rte_errno = trace.register_errno;
> > > > @@ -425,6 +431,9 @@ __rte_trace_point_register(rte_trace_point_t
> > > > *handle, const char *name,
> > > > goto fail;
> > > > }
> > > >
> > > > + if (first_register == 0)
> > > > + first_register = rte_get_tsc_cycles();
> > > > +
> > > > /* Check the size of the trace point object */
> > > > RTE_PER_LCORE(trace_point_sz) = 0;
> > > > RTE_PER_LCORE(ctf_count) = 0;
> > > > @@ -486,6 +495,8 @@ __rte_trace_point_register(rte_trace_point_t
> > > > *handle, const char *name,
> > > > STAILQ_INSERT_TAIL(&tp_list, tp, next);
> > > > __atomic_thread_fence(__ATOMIC_RELEASE);
> > > >
> > > > + last_register = rte_get_tsc_cycles();
> > > > +
> > > > /* All Good !!! */
> > > > return 0;
> > > > free:
> > > >
> > > >
> > > > I started testpmd 100 times for static and shared gcc builds
> > > > (test-meson-builds.sh) on a system with a 2.6GHz xeon.
> > > >
> > > > v20.05-rc1-13-g08dd97130 (before patch):
> > > > static: count=100, min=580812, max=1482326, avg=1764932
> > > > shared: count=100, min=554648, max=1344163, avg=1704638
> > > >
> > > > v20.05-rc1-14-g44250f392 (after patch):
> > > > static: count=100, min=668273, max=1530330, avg=1682548
> > > > shared: count=100, min=554634, max=1330264, avg=1672398
> >
> >
> >
> >
On Tue, May 5, 2020 at 5:06 PM David Marchand <david.marchand@redhat.com> wrote:
>
> On Tue, May 5, 2020 at 12:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > Please share the data.
> > >
> > > Measured time between first rte_trace_point_register and last one with
> > > a simple patch:
> >
> > I will try to reproduce this, once we finalize on the above synergy
> > with rte_log.
>
> I took the time to provide measure but you won't take the time to look at this.
I will spend time on this. I would like to test with a shared library
also and more tracepoints.
I was looking for an agreement on using the constructor for rte_log as
well(Just make sure the direction is correct).
Next steps:
- I will analyze the come back on this overhead on this thread.
- Olivier is OK for changing the RTE_LOG_REGSISTER macro, So we could
clean the log as well.
> Really nice.
>
>
> --
> David Marchand
>
On Tue, May 5, 2020 at 5:56 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Tue, May 5, 2020 at 5:06 PM David Marchand <david.marchand@redhat.com> wrote:
> >
> > On Tue, May 5, 2020 at 12:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > Please share the data.
> > > >
> > > > Measured time between first rte_trace_point_register and last one with
> > > > a simple patch:
> > >
> > > I will try to reproduce this, once we finalize on the above synergy
> > > with rte_log.
> >
> > I took the time to provide measure but you won't take the time to look at this.
>
> I will spend time on this. I would like to test with a shared library
> also and more tracepoints.
> I was looking for an agreement on using the constructor for rte_log as
> well(Just make sure the direction is correct).
>
> Next steps:
> - I will analyze the come back on this overhead on this thread.
I have added 500 constructors for testing the overhead with the shared
build and static build.
My results inline with your results aka negligible overhead.
David,
Do you have plan for similar RTE_LOG_REGISTER as mentioned earlier?
I would like to have rte_log and rte_trace semantics similar to registration.
If you are not planning to submit the rte_log patch then I can send
one for RC2 cleanup.
> - Olivier is OK for changing the RTE_LOG_REGSISTER macro, So we could
> clean the log as well.
>
>
> > Really nice.
>
>
>
> >
> >
> > --
> > David Marchand
> >
On Tue, May 5, 2020 at 5:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Tue, May 5, 2020 at 5:56 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >
> > On Tue, May 5, 2020 at 5:06 PM David Marchand <david.marchand@redhat.com> wrote:
> > >
> > > On Tue, May 5, 2020 at 12:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > Please share the data.
> > > > >
> > > > > Measured time between first rte_trace_point_register and last one with
> > > > > a simple patch:
> > > >
> > > > I will try to reproduce this, once we finalize on the above synergy
> > > > with rte_log.
> > >
> > > I took the time to provide measure but you won't take the time to look at this.
> >
> > I will spend time on this. I would like to test with a shared library
> > also and more tracepoints.
> > I was looking for an agreement on using the constructor for rte_log as
> > well(Just make sure the direction is correct).
> >
> > Next steps:
> > - I will analyze the come back on this overhead on this thread.
>
> I have added 500 constructors for testing the overhead with the shared
> build and static build.
> My results inline with your results aka negligible overhead.
>
> David,
> Do you have plan for similar RTE_LOG_REGISTER as mentioned earlier?
> I would like to have rte_log and rte_trace semantics similar to registration.
> If you are not planning to submit the rte_log patch then I can send
> one for RC2 cleanup.
It won't be possible for me.
Relying on the current rte_log_register is buggy with shared builds,
as drivers are calling rte_log_register, then impose a default level
without caring about what the user passed.
So if we introduce a RTE_LOG_REGISTER macro now at least this must be fixed too.
What I wanted to do:
- merge rte_log_register_and_pick_level() (experimental) into
rte_log_register, doing this should be fine from my pov,
- reconsider the relevance of a fallback logtype when registration fails,
- shoot the default level per component thing: levels meaning is
fragmented across the drivers/libraries because of it, but this will
open a big box of stuff,
On Tue, May 5, 2020 at 9:58 PM David Marchand <david.marchand@redhat.com> wrote:
>
> On Tue, May 5, 2020 at 5:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> >
> > On Tue, May 5, 2020 at 5:56 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > >
> > > On Tue, May 5, 2020 at 5:06 PM David Marchand <david.marchand@redhat.com> wrote:
> > > >
> > > > On Tue, May 5, 2020 at 12:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > Please share the data.
> > > > > >
> > > > > > Measured time between first rte_trace_point_register and last one with
> > > > > > a simple patch:
> > > > >
> > > > > I will try to reproduce this, once we finalize on the above synergy
> > > > > with rte_log.
> > > >
> > > > I took the time to provide measure but you won't take the time to look at this.
> > >
> > > I will spend time on this. I would like to test with a shared library
> > > also and more tracepoints.
> > > I was looking for an agreement on using the constructor for rte_log as
> > > well(Just make sure the direction is correct).
> > >
> > > Next steps:
> > > - I will analyze the come back on this overhead on this thread.
> >
> > I have added 500 constructors for testing the overhead with the shared
> > build and static build.
> > My results inline with your results aka negligible overhead.
> >
> > David,
> > Do you have plan for similar RTE_LOG_REGISTER as mentioned earlier?
> > I would like to have rte_log and rte_trace semantics similar to registration.
> > If you are not planning to submit the rte_log patch then I can send
> > one for RC2 cleanup.
>
> It won't be possible for me.
I can do that if we agree on the specifics.
>
> Relying on the current rte_log_register is buggy with shared builds,
> as drivers are calling rte_log_register, then impose a default level
> without caring about what the user passed.
> So if we introduce a RTE_LOG_REGISTER macro now at least this must be fixed too.
>
> What I wanted to do:
> - merge rte_log_register_and_pick_level() (experimental) into
> rte_log_register, doing this should be fine from my pov,
> - reconsider the relevance of a fallback logtype when registration fails,
> - shoot the default level per component thing: levels meaning is
> fragmented across the drivers/libraries because of it, but this will
> open a big box of stuff,
This you are referring to internal implementation improvement. Right?
I was referring to remove the current clutter[1]
If we stick the following as the interface. Then you can do other
improvements when you get time
that won't change the consumer code or interference part.
#define RTE_LOG_REGISTER(type, name, level)
[1]
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_base;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_mbox;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_npa;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_nix;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_npc;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_tm;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_sso;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_tim;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_dpi;
> > -/**
> > - * @internal
> > - */
> > -int otx2_logtype_ep;
> > -
> > -RTE_INIT(otx2_log_init);
> > -static void
> > -otx2_log_init(void)
> > -{
> > - otx2_logtype_base = rte_log_register("pmd.octeontx2.base");
> > - if (otx2_logtype_base >= 0)
> > - rte_log_set_level(otx2_logtype_base, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_mbox = rte_log_register("pmd.octeontx2.mbox");
> > - if (otx2_logtype_mbox >= 0)
> > - rte_log_set_level(otx2_logtype_mbox, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_npa = rte_log_register("pmd.mempool.octeontx2");
> > - if (otx2_logtype_npa >= 0)
> > - rte_log_set_level(otx2_logtype_npa, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_nix = rte_log_register("pmd.net.octeontx2");
> > - if (otx2_logtype_nix >= 0)
> > - rte_log_set_level(otx2_logtype_nix, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_npc = rte_log_register("pmd.net.octeontx2.flow");
> > - if (otx2_logtype_npc >= 0)
> > - rte_log_set_level(otx2_logtype_npc, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_tm = rte_log_register("pmd.net.octeontx2.tm");
> > - if (otx2_logtype_tm >= 0)
> > - rte_log_set_level(otx2_logtype_tm, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_sso = rte_log_register("pmd.event.octeontx2");
> > - if (otx2_logtype_sso >= 0)
> > - rte_log_set_level(otx2_logtype_sso, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_tim = rte_log_register("pmd.event.octeontx2.timer");
> > - if (otx2_logtype_tim >= 0)
> > - rte_log_set_level(otx2_logtype_tim, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_dpi = rte_log_register("pmd.raw.octeontx2.dpi");
> > - if (otx2_logtype_dpi >= 0)
> > - rte_log_set_level(otx2_logtype_dpi, RTE_LOG_NOTICE);
> > -
> > - otx2_logtype_ep = rte_log_register("pmd.raw.octeontx2.ep");
> > - if (otx2_logtype_ep >= 0)
> > - rte_log_set_level(otx2_logtype_ep, RTE_LOG_NOTICE);
> > -
> > -}
> > +RTE_LOG_REGISTER(otx2_logtype_base, pmd.octeontx2.base, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_mbox, pmd.octeontx2.mbox, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_npa, pmd.mempool.octeontx2, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_nix, pmd.net.octeontx2, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_npc, pmd.net.octeontx2.flow, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_tm, pmd.net.octeontx2.tm, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_sso, pmd.event.octeontx2, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_tim, pmd.event.octeontx2.timer, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_dpi, pmd.raw.octeontx2.dpi, NOTICE);
> > +RTE_LOG_REGISTER(otx2_logtype_ep, pmd.raw.octeontx2.ep, NOTICE);
> >
> > diff --git a/lib/librte_eal/include/rte_log.h b/lib/librte_eal/include/rte_log.h
> > index 1789ede56..22fc3802f 100644
> > --- a/lib/librte_eal/include/rte_log.h
> > +++ b/lib/librte_eal/include/rte_log.h
> > @@ -376,6 +376,15 @@ int rte_vlog(uint32_t level, uint32_t logtype,
> > const char *format, va_list ap)
> > RTE_LOGTYPE_ ## t, # t ": " __VA_ARGS__) : \
> > 0)
> >
> > +#define RTE_LOG_REGISTER(type, name, level) \
> > +int type; \
> > +RTE_INIT(__##type) \
> > +{ \
> > + type = rte_log_register(RTE_STR(name)); \
> > + if (type >= 0) \
> > + rte_log_set_level(type, RTE_LOG_##level); \
> > +}
> > +
>
>
>
>
> --
> David Marchand
>
05/05/2020 18:46, Jerin Jacob:
> On Tue, May 5, 2020 at 9:58 PM David Marchand <david.marchand@redhat.com> wrote:
> > On Tue, May 5, 2020 at 5:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > On Tue, May 5, 2020 at 5:56 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > On Tue, May 5, 2020 at 5:06 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > On Tue, May 5, 2020 at 12:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > Please share the data.
> > > > > > >
> > > > > > > Measured time between first rte_trace_point_register and last one with
> > > > > > > a simple patch:
> > > > > >
> > > > > > I will try to reproduce this, once we finalize on the above synergy
> > > > > > with rte_log.
> > > > >
> > > > > I took the time to provide measure but you won't take the time to look at this.
> > > >
> > > > I will spend time on this. I would like to test with a shared library
> > > > also and more tracepoints.
> > > > I was looking for an agreement on using the constructor for rte_log as
> > > > well(Just make sure the direction is correct).
> > > >
> > > > Next steps:
> > > > - I will analyze the come back on this overhead on this thread.
> > >
> > > I have added 500 constructors for testing the overhead with the shared
> > > build and static build.
> > > My results inline with your results aka negligible overhead.
> > >
> > > David,
> > > Do you have plan for similar RTE_LOG_REGISTER as mentioned earlier?
> > > I would like to have rte_log and rte_trace semantics similar to registration.
> > > If you are not planning to submit the rte_log patch then I can send
> > > one for RC2 cleanup.
> >
> > It won't be possible for me.
>
> I can do that if we agree on the specifics.
>
>
> >
> > Relying on the current rte_log_register is buggy with shared builds,
> > as drivers are calling rte_log_register, then impose a default level
> > without caring about what the user passed.
> > So if we introduce a RTE_LOG_REGISTER macro now at least this must be fixed too.
> >
> > What I wanted to do:
> > - merge rte_log_register_and_pick_level() (experimental) into
> > rte_log_register, doing this should be fine from my pov,
> > - reconsider the relevance of a fallback logtype when registration fails,
> > - shoot the default level per component thing: levels meaning is
> > fragmented across the drivers/libraries because of it, but this will
> > open a big box of stuff,
>
> This you are referring to internal implementation improvement. Right?
> I was referring to remove the current clutter[1]
> If we stick the following as the interface. Then you can do other
> improvements when you get time
> that won't change the consumer code or interference part.
>
> #define RTE_LOG_REGISTER(type, name, level)
This discussion is interesting but out of scope for rte_trace.
I am also interested in rte_log registration cleanup,
but I know it is too much work for the last weeks of 20.05.
As Olivier said about rte_trace,
"Since it's a new API, it makes sense to make
it as good as possible for the first version."
So please let's conclude on this rte_trace patch for 20.05-rc2,
and commit to fix rte_log registration in the first days of 20.08.
On Tue, May 5, 2020 at 10:28 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 05/05/2020 18:46, Jerin Jacob:
> > On Tue, May 5, 2020 at 9:58 PM David Marchand <david.marchand@redhat.com> wrote:
> > > On Tue, May 5, 2020 at 5:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > On Tue, May 5, 2020 at 5:56 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > On Tue, May 5, 2020 at 5:06 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > On Tue, May 5, 2020 at 12:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > Please share the data.
> > > > > > > >
> > > > > > > > Measured time between first rte_trace_point_register and last one with
> > > > > > > > a simple patch:
> > > > > > >
> > > > > > > I will try to reproduce this, once we finalize on the above synergy
> > > > > > > with rte_log.
> > > > > >
> > > > > > I took the time to provide measure but you won't take the time to look at this.
> > > > >
> > > > > I will spend time on this. I would like to test with a shared library
> > > > > also and more tracepoints.
> > > > > I was looking for an agreement on using the constructor for rte_log as
> > > > > well(Just make sure the direction is correct).
> > > > >
> > > > > Next steps:
> > > > > - I will analyze the come back on this overhead on this thread.
> > > >
> > > > I have added 500 constructors for testing the overhead with the shared
> > > > build and static build.
> > > > My results inline with your results aka negligible overhead.
> > > >
> > > > David,
> > > > Do you have plan for similar RTE_LOG_REGISTER as mentioned earlier?
> > > > I would like to have rte_log and rte_trace semantics similar to registration.
> > > > If you are not planning to submit the rte_log patch then I can send
> > > > one for RC2 cleanup.
> > >
> > > It won't be possible for me.
> >
> > I can do that if we agree on the specifics.
> >
> >
> > >
> > > Relying on the current rte_log_register is buggy with shared builds,
> > > as drivers are calling rte_log_register, then impose a default level
> > > without caring about what the user passed.
> > > So if we introduce a RTE_LOG_REGISTER macro now at least this must be fixed too.
> > >
> > > What I wanted to do:
> > > - merge rte_log_register_and_pick_level() (experimental) into
> > > rte_log_register, doing this should be fine from my pov,
> > > - reconsider the relevance of a fallback logtype when registration fails,
> > > - shoot the default level per component thing: levels meaning is
> > > fragmented across the drivers/libraries because of it, but this will
> > > open a big box of stuff,
> >
> > This you are referring to internal implementation improvement. Right?
> > I was referring to remove the current clutter[1]
> > If we stick the following as the interface. Then you can do other
> > improvements when you get time
> > that won't change the consumer code or interference part.
> >
> > #define RTE_LOG_REGISTER(type, name, level)
>
> This discussion is interesting but out of scope for rte_trace.
> I am also interested in rte_log registration cleanup,
> but I know it is too much work for the last weeks of 20.05.
>
> As Olivier said about rte_trace,
> "Since it's a new API, it makes sense to make
> it as good as possible for the first version."
>
> So please let's conclude on this rte_trace patch for 20.05-rc2,
> and commit to fix rte_log registration in the first days of 20.08.
Why not hold the trace registration patch 2/8 and apply rest for RC2.
Once we have synergy between the registration scheme between rte_log
and rte_trace
apply the patch for RC2.
>
>
>
On Tue, May 5, 2020 at 10:38 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
>
> On Tue, May 5, 2020 at 10:28 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > 05/05/2020 18:46, Jerin Jacob:
> > > On Tue, May 5, 2020 at 9:58 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > On Tue, May 5, 2020 at 5:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > On Tue, May 5, 2020 at 5:56 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > On Tue, May 5, 2020 at 5:06 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > On Tue, May 5, 2020 at 12:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > > Please share the data.
> > > > > > > > >
> > > > > > > > > Measured time between first rte_trace_point_register and last one with
> > > > > > > > > a simple patch:
> > > > > > > >
> > > > > > > > I will try to reproduce this, once we finalize on the above synergy
> > > > > > > > with rte_log.
> > > > > > >
> > > > > > > I took the time to provide measure but you won't take the time to look at this.
> > > > > >
> > > > > > I will spend time on this. I would like to test with a shared library
> > > > > > also and more tracepoints.
> > > > > > I was looking for an agreement on using the constructor for rte_log as
> > > > > > well(Just make sure the direction is correct).
> > > > > >
> > > > > > Next steps:
> > > > > > - I will analyze the come back on this overhead on this thread.
> > > > >
> > > > > I have added 500 constructors for testing the overhead with the shared
> > > > > build and static build.
> > > > > My results inline with your results aka negligible overhead.
> > > > >
> > > > > David,
> > > > > Do you have plan for similar RTE_LOG_REGISTER as mentioned earlier?
> > > > > I would like to have rte_log and rte_trace semantics similar to registration.
> > > > > If you are not planning to submit the rte_log patch then I can send
> > > > > one for RC2 cleanup.
> > > >
> > > > It won't be possible for me.
> > >
> > > I can do that if we agree on the specifics.
> > >
> > >
> > > >
> > > > Relying on the current rte_log_register is buggy with shared builds,
> > > > as drivers are calling rte_log_register, then impose a default level
> > > > without caring about what the user passed.
> > > > So if we introduce a RTE_LOG_REGISTER macro now at least this must be fixed too.
> > > >
> > > > What I wanted to do:
> > > > - merge rte_log_register_and_pick_level() (experimental) into
> > > > rte_log_register, doing this should be fine from my pov,
> > > > - reconsider the relevance of a fallback logtype when registration fails,
> > > > - shoot the default level per component thing: levels meaning is
> > > > fragmented across the drivers/libraries because of it, but this will
> > > > open a big box of stuff,
> > >
> > > This you are referring to internal implementation improvement. Right?
> > > I was referring to remove the current clutter[1]
> > > If we stick the following as the interface. Then you can do other
> > > improvements when you get time
> > > that won't change the consumer code or interference part.
> > >
> > > #define RTE_LOG_REGISTER(type, name, level)
> >
> > This discussion is interesting but out of scope for rte_trace.
> > I am also interested in rte_log registration cleanup,
> > but I know it is too much work for the last weeks of 20.05.
> >
> > As Olivier said about rte_trace,
> > "Since it's a new API, it makes sense to make
> > it as good as possible for the first version."
> >
> > So please let's conclude on this rte_trace patch for 20.05-rc2,
> > and commit to fix rte_log registration in the first days of 20.08.
>
> Why not hold the trace registration patch 2/8 and apply rest for RC2.
> Once we have synergy between the registration scheme between rte_log
> and rte_trace
> apply the patch for RC2.
I meant, Once we have synergy between the registration scheme between
rte_log and rte_trace
apply the patch for _20.08_?
>
>
> >
> >
> >
05/05/2020 19:09, Jerin Jacob:
> On Tue, May 5, 2020 at 10:38 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > On Tue, May 5, 2020 at 10:28 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > 05/05/2020 18:46, Jerin Jacob:
> > > > On Tue, May 5, 2020 at 9:58 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > On Tue, May 5, 2020 at 5:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > On Tue, May 5, 2020 at 5:56 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > On Tue, May 5, 2020 at 5:06 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > On Tue, May 5, 2020 at 12:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > > > Please share the data.
> > > > > > > > > >
> > > > > > > > > > Measured time between first rte_trace_point_register and last one with
> > > > > > > > > > a simple patch:
> > > > > > > > >
> > > > > > > > > I will try to reproduce this, once we finalize on the above synergy
> > > > > > > > > with rte_log.
> > > > > > > >
> > > > > > > > I took the time to provide measure but you won't take the time to look at this.
> > > > > > >
> > > > > > > I will spend time on this. I would like to test with a shared library
> > > > > > > also and more tracepoints.
> > > > > > > I was looking for an agreement on using the constructor for rte_log as
> > > > > > > well(Just make sure the direction is correct).
> > > > > > >
> > > > > > > Next steps:
> > > > > > > - I will analyze the come back on this overhead on this thread.
> > > > > >
> > > > > > I have added 500 constructors for testing the overhead with the shared
> > > > > > build and static build.
> > > > > > My results inline with your results aka negligible overhead.
> > > > > >
> > > > > > David,
> > > > > > Do you have plan for similar RTE_LOG_REGISTER as mentioned earlier?
> > > > > > I would like to have rte_log and rte_trace semantics similar to registration.
> > > > > > If you are not planning to submit the rte_log patch then I can send
> > > > > > one for RC2 cleanup.
> > > > >
> > > > > It won't be possible for me.
> > > >
> > > > I can do that if we agree on the specifics.
> > > >
> > > >
> > > > >
> > > > > Relying on the current rte_log_register is buggy with shared builds,
> > > > > as drivers are calling rte_log_register, then impose a default level
> > > > > without caring about what the user passed.
> > > > > So if we introduce a RTE_LOG_REGISTER macro now at least this must be fixed too.
> > > > >
> > > > > What I wanted to do:
> > > > > - merge rte_log_register_and_pick_level() (experimental) into
> > > > > rte_log_register, doing this should be fine from my pov,
> > > > > - reconsider the relevance of a fallback logtype when registration fails,
> > > > > - shoot the default level per component thing: levels meaning is
> > > > > fragmented across the drivers/libraries because of it, but this will
> > > > > open a big box of stuff,
> > > >
> > > > This you are referring to internal implementation improvement. Right?
> > > > I was referring to remove the current clutter[1]
> > > > If we stick the following as the interface. Then you can do other
> > > > improvements when you get time
> > > > that won't change the consumer code or interference part.
> > > >
> > > > #define RTE_LOG_REGISTER(type, name, level)
> > >
> > > This discussion is interesting but out of scope for rte_trace.
> > > I am also interested in rte_log registration cleanup,
> > > but I know it is too much work for the last weeks of 20.05.
> > >
> > > As Olivier said about rte_trace,
> > > "Since it's a new API, it makes sense to make
> > > it as good as possible for the first version."
> > >
> > > So please let's conclude on this rte_trace patch for 20.05-rc2,
> > > and commit to fix rte_log registration in the first days of 20.08.
> >
> > Why not hold the trace registration patch 2/8 and apply rest for RC2.
> > Once we have synergy between the registration scheme between rte_log
> > and rte_trace
> > apply the patch for RC2.
>
> I meant, Once we have synergy between the registration scheme between
> rte_log and rte_trace
> apply the patch for _20.08_?
Because of what I wrote above:
As Olivier said about rte_trace,
"Since it's a new API, it makes sense to make
it as good as possible for the first version."
The intent is to show an API as simple as possible
in order to have a maximum of developers integrating it,
and getting more interesting feedbacks.
In other words, we want to make your work shine for prime time.
On Tue, May 5, 2020 at 10:50 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 05/05/2020 19:09, Jerin Jacob:
> > On Tue, May 5, 2020 at 10:38 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > On Tue, May 5, 2020 at 10:28 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > 05/05/2020 18:46, Jerin Jacob:
> > > > > On Tue, May 5, 2020 at 9:58 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > On Tue, May 5, 2020 at 5:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > On Tue, May 5, 2020 at 5:56 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > On Tue, May 5, 2020 at 5:06 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > > On Tue, May 5, 2020 at 12:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > > > > Please share the data.
> > > > > > > > > > >
> > > > > > > > > > > Measured time between first rte_trace_point_register and last one with
> > > > > > > > > > > a simple patch:
> > > > > > > > > >
> > > > > > > > > > I will try to reproduce this, once we finalize on the above synergy
> > > > > > > > > > with rte_log.
> > > > > > > > >
> > > > > > > > > I took the time to provide measure but you won't take the time to look at this.
> > > > > > > >
> > > > > > > > I will spend time on this. I would like to test with a shared library
> > > > > > > > also and more tracepoints.
> > > > > > > > I was looking for an agreement on using the constructor for rte_log as
> > > > > > > > well(Just make sure the direction is correct).
> > > > > > > >
> > > > > > > > Next steps:
> > > > > > > > - I will analyze the come back on this overhead on this thread.
> > > > > > >
> > > > > > > I have added 500 constructors for testing the overhead with the shared
> > > > > > > build and static build.
> > > > > > > My results inline with your results aka negligible overhead.
> > > > > > >
> > > > > > > David,
> > > > > > > Do you have plan for similar RTE_LOG_REGISTER as mentioned earlier?
> > > > > > > I would like to have rte_log and rte_trace semantics similar to registration.
> > > > > > > If you are not planning to submit the rte_log patch then I can send
> > > > > > > one for RC2 cleanup.
> > > > > >
> > > > > > It won't be possible for me.
> > > > >
> > > > > I can do that if we agree on the specifics.
> > > > >
> > > > >
> > > > > >
> > > > > > Relying on the current rte_log_register is buggy with shared builds,
> > > > > > as drivers are calling rte_log_register, then impose a default level
> > > > > > without caring about what the user passed.
> > > > > > So if we introduce a RTE_LOG_REGISTER macro now at least this must be fixed too.
> > > > > >
> > > > > > What I wanted to do:
> > > > > > - merge rte_log_register_and_pick_level() (experimental) into
> > > > > > rte_log_register, doing this should be fine from my pov,
> > > > > > - reconsider the relevance of a fallback logtype when registration fails,
> > > > > > - shoot the default level per component thing: levels meaning is
> > > > > > fragmented across the drivers/libraries because of it, but this will
> > > > > > open a big box of stuff,
> > > > >
> > > > > This you are referring to internal implementation improvement. Right?
> > > > > I was referring to remove the current clutter[1]
> > > > > If we stick the following as the interface. Then you can do other
> > > > > improvements when you get time
> > > > > that won't change the consumer code or interference part.
> > > > >
> > > > > #define RTE_LOG_REGISTER(type, name, level)
> > > >
> > > > This discussion is interesting but out of scope for rte_trace.
> > > > I am also interested in rte_log registration cleanup,
> > > > but I know it is too much work for the last weeks of 20.05.
> > > >
> > > > As Olivier said about rte_trace,
> > > > "Since it's a new API, it makes sense to make
> > > > it as good as possible for the first version."
> > > >
> > > > So please let's conclude on this rte_trace patch for 20.05-rc2,
> > > > and commit to fix rte_log registration in the first days of 20.08.
> > >
> > > Why not hold the trace registration patch 2/8 and apply rest for RC2.
> > > Once we have synergy between the registration scheme between rte_log
> > > and rte_trace
> > > apply the patch for RC2.
> >
> > I meant, Once we have synergy between the registration scheme between
> > rte_log and rte_trace
> > apply the patch for _20.08_?
>
> Because of what I wrote above:
> As Olivier said about rte_trace,
> "Since it's a new API, it makes sense to make
> it as good as possible for the first version."
>
> The intent is to show an API as simple as possible
> in order to have a maximum of developers integrating it,
> and getting more interesting feedbacks.
>
> In other words, we want to make your work shine for prime time.
I like that, If it is not shining just because of 2/8 not applying now
then I fine with that.
Anyway, it is an experimental API, There is still room to change and
nothing is set and stone.
For me, the synergy between log/trace interface important as trace
needs to replace rte_log.
>
>
05/05/2020 19:28, Jerin Jacob:
> On Tue, May 5, 2020 at 10:50 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > 05/05/2020 19:09, Jerin Jacob:
> > > On Tue, May 5, 2020 at 10:38 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > On Tue, May 5, 2020 at 10:28 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > > 05/05/2020 18:46, Jerin Jacob:
> > > > > > On Tue, May 5, 2020 at 9:58 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > On Tue, May 5, 2020 at 5:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > On Tue, May 5, 2020 at 5:56 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > On Tue, May 5, 2020 at 5:06 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > > > On Tue, May 5, 2020 at 12:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > > > > > Please share the data.
> > > > > > > > > > > >
> > > > > > > > > > > > Measured time between first rte_trace_point_register and last one with
> > > > > > > > > > > > a simple patch:
> > > > > > > > > > >
> > > > > > > > > > > I will try to reproduce this, once we finalize on the above synergy
> > > > > > > > > > > with rte_log.
> > > > > > > > > >
> > > > > > > > > > I took the time to provide measure but you won't take the time to look at this.
> > > > > > > > >
> > > > > > > > > I will spend time on this. I would like to test with a shared library
> > > > > > > > > also and more tracepoints.
> > > > > > > > > I was looking for an agreement on using the constructor for rte_log as
> > > > > > > > > well(Just make sure the direction is correct).
> > > > > > > > >
> > > > > > > > > Next steps:
> > > > > > > > > - I will analyze the come back on this overhead on this thread.
> > > > > > > >
> > > > > > > > I have added 500 constructors for testing the overhead with the shared
> > > > > > > > build and static build.
> > > > > > > > My results inline with your results aka negligible overhead.
> > > > > > > >
> > > > > > > > David,
> > > > > > > > Do you have plan for similar RTE_LOG_REGISTER as mentioned earlier?
> > > > > > > > I would like to have rte_log and rte_trace semantics similar to registration.
> > > > > > > > If you are not planning to submit the rte_log patch then I can send
> > > > > > > > one for RC2 cleanup.
> > > > > > >
> > > > > > > It won't be possible for me.
> > > > > >
> > > > > > I can do that if we agree on the specifics.
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > Relying on the current rte_log_register is buggy with shared builds,
> > > > > > > as drivers are calling rte_log_register, then impose a default level
> > > > > > > without caring about what the user passed.
> > > > > > > So if we introduce a RTE_LOG_REGISTER macro now at least this must be fixed too.
> > > > > > >
> > > > > > > What I wanted to do:
> > > > > > > - merge rte_log_register_and_pick_level() (experimental) into
> > > > > > > rte_log_register, doing this should be fine from my pov,
> > > > > > > - reconsider the relevance of a fallback logtype when registration fails,
> > > > > > > - shoot the default level per component thing: levels meaning is
> > > > > > > fragmented across the drivers/libraries because of it, but this will
> > > > > > > open a big box of stuff,
> > > > > >
> > > > > > This you are referring to internal implementation improvement. Right?
> > > > > > I was referring to remove the current clutter[1]
> > > > > > If we stick the following as the interface. Then you can do other
> > > > > > improvements when you get time
> > > > > > that won't change the consumer code or interference part.
> > > > > >
> > > > > > #define RTE_LOG_REGISTER(type, name, level)
> > > > >
> > > > > This discussion is interesting but out of scope for rte_trace.
> > > > > I am also interested in rte_log registration cleanup,
> > > > > but I know it is too much work for the last weeks of 20.05.
> > > > >
> > > > > As Olivier said about rte_trace,
> > > > > "Since it's a new API, it makes sense to make
> > > > > it as good as possible for the first version."
> > > > >
> > > > > So please let's conclude on this rte_trace patch for 20.05-rc2,
> > > > > and commit to fix rte_log registration in the first days of 20.08.
> > > >
> > > > Why not hold the trace registration patch 2/8 and apply rest for RC2.
> > > > Once we have synergy between the registration scheme between rte_log
> > > > and rte_trace
> > > > apply the patch for RC2.
> > >
> > > I meant, Once we have synergy between the registration scheme between
> > > rte_log and rte_trace
> > > apply the patch for _20.08_?
> >
> > Because of what I wrote above:
> > As Olivier said about rte_trace,
> > "Since it's a new API, it makes sense to make
> > it as good as possible for the first version."
> >
> > The intent is to show an API as simple as possible
> > in order to have a maximum of developers integrating it,
> > and getting more interesting feedbacks.
> >
> > In other words, we want to make your work shine for prime time.
>
> I like that, If it is not shining just because of 2/8 not applying now
> then I fine with that.
> Anyway, it is an experimental API, There is still room to change and
> nothing is set and stone.
> For me, the synergy between log/trace interface important as trace
> needs to replace rte_log.
Now that I better understand what rte_trace (and tracing in general) is,
I believe rte_log cannot be replaced.
I think we can write logs in traces, as a log option, but it should be
just one possible output among others.
I think everybody agree to use one constructor per log type and
per trace type. We are ready to do this change for rte_trace first.
This is your call to accept it or not, even if don't understand
why you would like both to be done at the exact same time.
On Wed, May 6, 2020 at 1:40 AM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> 05/05/2020 19:28, Jerin Jacob:
> > On Tue, May 5, 2020 at 10:50 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > 05/05/2020 19:09, Jerin Jacob:
> > > > On Tue, May 5, 2020 at 10:38 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > On Tue, May 5, 2020 at 10:28 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> > > > > > 05/05/2020 18:46, Jerin Jacob:
> > > > > > > On Tue, May 5, 2020 at 9:58 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > On Tue, May 5, 2020 at 5:25 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > On Tue, May 5, 2020 at 5:56 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > > On Tue, May 5, 2020 at 5:06 PM David Marchand <david.marchand@redhat.com> wrote:
> > > > > > > > > > > On Tue, May 5, 2020 at 12:13 PM Jerin Jacob <jerinjacobk@gmail.com> wrote:
> > > > > > > > > > > > > > Please share the data.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Measured time between first rte_trace_point_register and last one with
> > > > > > > > > > > > > a simple patch:
> > > > > > > > > > > >
> > > > > > > > > > > > I will try to reproduce this, once we finalize on the above synergy
> > > > > > > > > > > > with rte_log.
> > > > > > > > > > >
> > > > > > > > > > > I took the time to provide measure but you won't take the time to look at this.
> > > > > > > > > >
> > > > > > > > > > I will spend time on this. I would like to test with a shared library
> > > > > > > > > > also and more tracepoints.
> > > > > > > > > > I was looking for an agreement on using the constructor for rte_log as
> > > > > > > > > > well(Just make sure the direction is correct).
> > > > > > > > > >
> > > > > > > > > > Next steps:
> > > > > > > > > > - I will analyze the come back on this overhead on this thread.
> > > > > > > > >
> > > > > > > > > I have added 500 constructors for testing the overhead with the shared
> > > > > > > > > build and static build.
> > > > > > > > > My results inline with your results aka negligible overhead.
> > > > > > > > >
> > > > > > > > > David,
> > > > > > > > > Do you have plan for similar RTE_LOG_REGISTER as mentioned earlier?
> > > > > > > > > I would like to have rte_log and rte_trace semantics similar to registration.
> > > > > > > > > If you are not planning to submit the rte_log patch then I can send
> > > > > > > > > one for RC2 cleanup.
> > > > > > > >
> > > > > > > > It won't be possible for me.
> > > > > > >
> > > > > > > I can do that if we agree on the specifics.
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > Relying on the current rte_log_register is buggy with shared builds,
> > > > > > > > as drivers are calling rte_log_register, then impose a default level
> > > > > > > > without caring about what the user passed.
> > > > > > > > So if we introduce a RTE_LOG_REGISTER macro now at least this must be fixed too.
> > > > > > > >
> > > > > > > > What I wanted to do:
> > > > > > > > - merge rte_log_register_and_pick_level() (experimental) into
> > > > > > > > rte_log_register, doing this should be fine from my pov,
> > > > > > > > - reconsider the relevance of a fallback logtype when registration fails,
> > > > > > > > - shoot the default level per component thing: levels meaning is
> > > > > > > > fragmented across the drivers/libraries because of it, but this will
> > > > > > > > open a big box of stuff,
> > > > > > >
> > > > > > > This you are referring to internal implementation improvement. Right?
> > > > > > > I was referring to remove the current clutter[1]
> > > > > > > If we stick the following as the interface. Then you can do other
> > > > > > > improvements when you get time
> > > > > > > that won't change the consumer code or interference part.
> > > > > > >
> > > > > > > #define RTE_LOG_REGISTER(type, name, level)
> > > > > >
> > > > > > This discussion is interesting but out of scope for rte_trace.
> > > > > > I am also interested in rte_log registration cleanup,
> > > > > > but I know it is too much work for the last weeks of 20.05.
> > > > > >
> > > > > > As Olivier said about rte_trace,
> > > > > > "Since it's a new API, it makes sense to make
> > > > > > it as good as possible for the first version."
> > > > > >
> > > > > > So please let's conclude on this rte_trace patch for 20.05-rc2,
> > > > > > and commit to fix rte_log registration in the first days of 20.08.
> > > > >
> > > > > Why not hold the trace registration patch 2/8 and apply rest for RC2.
> > > > > Once we have synergy between the registration scheme between rte_log
> > > > > and rte_trace
> > > > > apply the patch for RC2.
> > > >
> > > > I meant, Once we have synergy between the registration scheme between
> > > > rte_log and rte_trace
> > > > apply the patch for _20.08_?
> > >
> > > Because of what I wrote above:
> > > As Olivier said about rte_trace,
> > > "Since it's a new API, it makes sense to make
> > > it as good as possible for the first version."
> > >
> > > The intent is to show an API as simple as possible
> > > in order to have a maximum of developers integrating it,
> > > and getting more interesting feedbacks.
> > >
> > > In other words, we want to make your work shine for prime time.
> >
> > I like that, If it is not shining just because of 2/8 not applying now
> > then I fine with that.
> > Anyway, it is an experimental API, There is still room to change and
> > nothing is set and stone.
> > For me, the synergy between log/trace interface important as trace
> > needs to replace rte_log.
>
> Now that I better understand what rte_trace (and tracing in general) is,
> I believe rte_log cannot be replaced.
> I think we can write logs in traces, as a log option, but it should be
> just one possible output among others.
IMO, log function can be implemented with trace. Not another way around.
Functionality-wise we can replace/redirect logs are traces.
At least at the registration point, semantically and syntax wise it
can be similar.
>
> I think everybody agree to use one constructor per log type and
> per trace type.
> We are ready to do this change for rte_trace first.
If we consider the constructor per log/trace is an improvement, I
would like to do that first in rte_log
and it has more consumers.
And I am willing to send a patch for the following change across
rte_log consumers.
http://mails.dpdk.org/archives/dev/2020-May/166468.html
I created the rte_trace registration mechanism similar to rte_log with
community feedback on
alignment and rte_trace and rte_log. I would like to maintain that.
> This is your call to accept it or not, even if don't understand
> why you would like both to be done at the exact same time.
>
>
On Mon, May 4, 2020 at 2:02 AM David Marchand <david.marchand@redhat.com> wrote:
>
> RTE_TRACE_POINT_DEFINE and RTE_TRACE_POINT_REGISTER must come in pairs.
> Merge them and let RTE_TRACE_POINT_REGISTER handle the constructor part.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> app/test/test_trace_register.c | 12 +-
> doc/guides/prog_guide/trace_lib.rst | 12 +-
> lib/librte_cryptodev/cryptodev_trace_points.c | 84 +++----
> .../common/eal_common_trace_points.c | 164 ++++++--------
> lib/librte_eal/include/rte_eal_trace.h | 122 +++++------
> lib/librte_eal/include/rte_trace_point.h | 14 +-
> .../include/rte_trace_point_register.h | 6 +-
> lib/librte_ethdev/ethdev_trace_points.c | 44 ++--
> lib/librte_eventdev/eventdev_trace_points.c | 205 +++++++-----------
> lib/librte_mempool/mempool_trace_points.c | 124 ++++-------
> 10 files changed, 309 insertions(+), 478 deletions(-)
This needs to be rebased to ToT. Please merge to RC1 if possible.
Acked-by: Jerin Jacob <jerinj@marvell.com>
@@ -5,13 +5,5 @@
#include "test_trace.h"
-/* Define trace points */
-RTE_TRACE_POINT_DEFINE(app_dpdk_test_tp);
-RTE_TRACE_POINT_DEFINE(app_dpdk_test_fp);
-
-RTE_INIT(register_valid_trace_points)
-{
- RTE_TRACE_POINT_REGISTER(app_dpdk_test_tp, app.dpdk.test.tp);
- RTE_TRACE_POINT_REGISTER(app_dpdk_test_fp, app.dpdk.test.fp);
-}
-
+RTE_TRACE_POINT_REGISTER(app_dpdk_test_tp, app.dpdk.test.tp)
+RTE_TRACE_POINT_REGISTER(app_dpdk_test_fp, app.dpdk.test.fp)
@@ -101,12 +101,7 @@ Register the tracepoint
#include <my_tracepoint_provider.h>
- RTE_TRACE_POINT_DEFINE(app_trace_string);
-
- RTE_INIT(app_trace_init)
- {
- RTE_TRACE_POINT_REGISTER(app_trace_string, app.trace.string);
- }
+ RTE_TRACE_POINT_REGISTER(app_trace_string, app.trace.string)
The above code snippet registers the ``app_trace_string`` tracepoint to
trace library. Here, the ``my_tracepoint_provider.h`` is the header file
@@ -119,9 +114,6 @@ There is no requirement for the tracepoint function and its name to be similar.
However, it is recommended to have a similar name for a better naming
convention.
-The user must register the tracepoint before the ``rte_eal_init`` invocation.
-The user can use the ``RTE_INIT`` construction scheme to achieve this.
-
.. note::
The ``RTE_TRACE_POINT_REGISTER_SELECT`` must be defined before including the
@@ -129,7 +121,7 @@ The user can use the ``RTE_INIT`` construction scheme to achieve this.
.. note::
- The ``RTE_TRACE_POINT_DEFINE`` defines the placeholder for the
+ The ``RTE_TRACE_POINT_REGISTER`` defines the placeholder for the
``rte_trace_point_t`` tracepoint object. The user must export a
``__<trace_function_name>`` symbol in the library ``.map`` file for this
tracepoint to be used out of the library, in shared builds.
@@ -6,70 +6,50 @@
#include "rte_cryptodev_trace.h"
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_configure);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_start);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_stop);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_close);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_queue_pair_setup);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_sym_session_pool_create);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_sym_session_create);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_asym_session_create);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_sym_session_free);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_asym_session_free);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_sym_session_init);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_asym_session_init);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_sym_session_clear);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_asym_session_clear);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_enqueue_burst);
-RTE_TRACE_POINT_DEFINE(rte_cryptodev_trace_dequeue_burst);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_configure,
+ lib.cryptodev.configure)
-RTE_INIT(cryptodev_trace_init)
-{
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_configure,
- lib.cryptodev.configure);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_start,
+ lib.cryptodev.start)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_start,
- lib.cryptodev.start);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_stop,
+ lib.cryptodev.stop)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_stop,
- lib.cryptodev.stop);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_close,
+ lib.cryptodev.close)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_close,
- lib.cryptodev.close);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_queue_pair_setup,
+ lib.cryptodev.queue.pair.setup)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_queue_pair_setup,
- lib.cryptodev.queue.pair.setup);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_pool_create,
+ lib.cryptodev.sym.pool.create)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_pool_create,
- lib.cryptodev.sym.pool.create);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_create,
+ lib.cryptodev.sym.create)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_create,
- lib.cryptodev.sym.create);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_create,
+ lib.cryptodev.asym.create)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_create,
- lib.cryptodev.asym.create);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_free,
+ lib.cryptodev.sym.free)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_free,
- lib.cryptodev.sym.free);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_free,
+ lib.cryptodev.asym.free)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_free,
- lib.cryptodev.asym.free);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_init,
+ lib.cryptodev.sym.init)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_init,
- lib.cryptodev.sym.init);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_init,
+ lib.cryptodev.asym.init)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_init,
- lib.cryptodev.asym.init);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_clear,
+ lib.cryptodev.sym.clear)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_sym_session_clear,
- lib.cryptodev.sym.clear);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_clear,
+ lib.cryptodev.asym.clear)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_asym_session_clear,
- lib.cryptodev.asym.clear);
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_enqueue_burst,
+ lib.cryptodev.enq.burst)
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_enqueue_burst,
- lib.cryptodev.enq.burst);
-
- RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_dequeue_burst,
- lib.cryptodev.deq.burst);
-}
+RTE_TRACE_POINT_REGISTER(rte_cryptodev_trace_dequeue_burst,
+ lib.cryptodev.deq.burst)
@@ -6,110 +6,70 @@
#include <rte_eal_trace.h>
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_void);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_u64);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_u32);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_u16);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_u8);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_i64);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_i32);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_i16);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_i8);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_int);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_long);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_float);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_double);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_ptr);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_str);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_generic_func);
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_void,
+ lib.eal.generic.void)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_u64,
+ lib.eal.generic.u64)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_u32,
+ lib.eal.generic.u32)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_u16,
+ lib.eal.generic.u16)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_u8,
+ lib.eal.generic.u8)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_i64,
+ lib.eal.generic.i64)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_i32,
+ lib.eal.generic.i32)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_i16,
+ lib.eal.generic.i16)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_i8,
+ lib.eal.generic.i8)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_int,
+ lib.eal.generic.int)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_long,
+ lib.eal.generic.long)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_float,
+ lib.eal.generic.float)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_double,
+ lib.eal.generic.double)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_ptr,
+ lib.eal.generic.ptr)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_str,
+ lib.eal.generic.string)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_func,
+ lib.eal.generic.func)
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_alarm_set);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_alarm_cancel);
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_alarm_set,
+ lib.eal.alarm.set)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_alarm_cancel,
+ lib.eal.alarm.cancel)
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_mem_zmalloc);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_mem_malloc);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_mem_realloc);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_mem_free);
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_mem_zmalloc,
+ lib.eal.mem.zmalloc)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_mem_malloc,
+ lib.eal.mem.malloc)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_mem_realloc,
+ lib.eal.mem.realloc)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_mem_free,
+ lib.eal.mem.free)
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_memzone_reserve);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_memzone_lookup);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_memzone_free);
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_memzone_reserve,
+ lib.eal.memzone.reserve)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_memzone_lookup,
+ lib.eal.memzone.lookup)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_memzone_free,
+ lib.eal.memzone.free)
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_thread_remote_launch);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_thread_lcore_ready);
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_remote_launch,
+ lib.eal.thread.remote.launch)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_lcore_ready,
+ lib.eal.thread.lcore.ready)
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_intr_callback_register);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_intr_callback_unregister);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_intr_enable);
-RTE_TRACE_POINT_DEFINE(rte_eal_trace_intr_disable);
-
-RTE_INIT(eal_trace_init)
-{
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_void,
- lib.eal.generic.void);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_u64,
- lib.eal.generic.u64);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_u32,
- lib.eal.generic.u32);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_u16,
- lib.eal.generic.u16);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_u8,
- lib.eal.generic.u8);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_i64,
- lib.eal.generic.i64);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_i32,
- lib.eal.generic.i32);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_i16,
- lib.eal.generic.i16);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_i8,
- lib.eal.generic.i8);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_int,
- lib.eal.generic.int);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_long,
- lib.eal.generic.long);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_float,
- lib.eal.generic.float);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_double,
- lib.eal.generic.double);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_ptr,
- lib.eal.generic.ptr);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_str,
- lib.eal.generic.string);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_generic_func,
- lib.eal.generic.func);
-
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_alarm_set,
- lib.eal.alarm.set);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_alarm_cancel,
- lib.eal.alarm.cancel);
-
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_mem_zmalloc,
- lib.eal.mem.zmalloc);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_mem_malloc,
- lib.eal.mem.malloc);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_mem_realloc,
- lib.eal.mem.realloc);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_mem_free,
- lib.eal.mem.free);
-
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_memzone_reserve,
- lib.eal.memzone.reserve);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_memzone_lookup,
- lib.eal.memzone.lookup);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_memzone_free,
- lib.eal.memzone.free);
-
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_remote_launch,
- lib.eal.thread.remote.launch);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_thread_lcore_ready,
- lib.eal.thread.lcore.ready);
-
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_intr_callback_register,
- lib.eal.intr.register);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_intr_callback_unregister,
- lib.eal.intr.unregister);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_intr_enable,
- lib.eal.intr.enable);
- RTE_TRACE_POINT_REGISTER(rte_eal_trace_intr_disable,
- lib.eal.intr.disable);
-}
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_intr_callback_register,
+ lib.eal.intr.register)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_intr_callback_unregister,
+ lib.eal.intr.unregister)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_intr_enable,
+ lib.eal.intr.enable)
+RTE_TRACE_POINT_REGISTER(rte_eal_trace_intr_disable,
+ lib.eal.intr.disable)
@@ -19,6 +19,26 @@ extern "C" {
#include <rte_interrupts.h>
#include <rte_trace_point.h>
+/* Alarm */
+RTE_TRACE_POINT(
+ rte_eal_trace_alarm_set,
+ RTE_TRACE_POINT_ARGS(uint64_t us, rte_eal_alarm_callback cb_fn,
+ void *cb_arg, int rc),
+ rte_trace_point_emit_u64(us);
+ rte_trace_point_emit_ptr(cb_fn);
+ rte_trace_point_emit_ptr(cb_arg);
+ rte_trace_point_emit_int(rc);
+)
+
+RTE_TRACE_POINT(
+ rte_eal_trace_alarm_cancel,
+ RTE_TRACE_POINT_ARGS(rte_eal_alarm_callback cb_fn, void *cb_arg,
+ int count),
+ rte_trace_point_emit_ptr(cb_fn);
+ rte_trace_point_emit_ptr(cb_arg);
+ rte_trace_point_emit_int(count);
+)
+
/* Generic */
RTE_TRACE_POINT(
rte_eal_trace_generic_void,
@@ -117,24 +137,52 @@ RTE_TRACE_POINT(
#define RTE_EAL_TRACE_GENERIC_FUNC rte_eal_trace_generic_func(__func__)
-/* Alarm */
+/* Interrupt */
RTE_TRACE_POINT(
- rte_eal_trace_alarm_set,
- RTE_TRACE_POINT_ARGS(uint64_t us, rte_eal_alarm_callback cb_fn,
- void *cb_arg, int rc),
- rte_trace_point_emit_u64(us);
- rte_trace_point_emit_ptr(cb_fn);
- rte_trace_point_emit_ptr(cb_arg);
+ rte_eal_trace_intr_callback_register,
+ RTE_TRACE_POINT_ARGS(const struct rte_intr_handle *handle,
+ rte_intr_callback_fn cb, void *cb_arg, int rc),
rte_trace_point_emit_int(rc);
+ rte_trace_point_emit_int(handle->vfio_dev_fd);
+ rte_trace_point_emit_int(handle->fd);
+ rte_trace_point_emit_int(handle->type);
+ rte_trace_point_emit_u32(handle->max_intr);
+ rte_trace_point_emit_u32(handle->nb_efd);
+ rte_trace_point_emit_ptr(cb);
+ rte_trace_point_emit_ptr(cb_arg);
)
-
RTE_TRACE_POINT(
- rte_eal_trace_alarm_cancel,
- RTE_TRACE_POINT_ARGS(rte_eal_alarm_callback cb_fn, void *cb_arg,
- int count),
- rte_trace_point_emit_ptr(cb_fn);
+ rte_eal_trace_intr_callback_unregister,
+ RTE_TRACE_POINT_ARGS(const struct rte_intr_handle *handle,
+ rte_intr_callback_fn cb, void *cb_arg, int rc),
+ rte_trace_point_emit_int(rc);
+ rte_trace_point_emit_int(handle->vfio_dev_fd);
+ rte_trace_point_emit_int(handle->fd);
+ rte_trace_point_emit_int(handle->type);
+ rte_trace_point_emit_u32(handle->max_intr);
+ rte_trace_point_emit_u32(handle->nb_efd);
+ rte_trace_point_emit_ptr(cb);
rte_trace_point_emit_ptr(cb_arg);
- rte_trace_point_emit_int(count);
+)
+RTE_TRACE_POINT(
+ rte_eal_trace_intr_enable,
+ RTE_TRACE_POINT_ARGS(const struct rte_intr_handle *handle, int rc),
+ rte_trace_point_emit_int(rc);
+ rte_trace_point_emit_int(handle->vfio_dev_fd);
+ rte_trace_point_emit_int(handle->fd);
+ rte_trace_point_emit_int(handle->type);
+ rte_trace_point_emit_u32(handle->max_intr);
+ rte_trace_point_emit_u32(handle->nb_efd);
+)
+RTE_TRACE_POINT(
+ rte_eal_trace_intr_disable,
+ RTE_TRACE_POINT_ARGS(const struct rte_intr_handle *handle, int rc),
+ rte_trace_point_emit_int(rc);
+ rte_trace_point_emit_int(handle->vfio_dev_fd);
+ rte_trace_point_emit_int(handle->fd);
+ rte_trace_point_emit_int(handle->type);
+ rte_trace_point_emit_u32(handle->max_intr);
+ rte_trace_point_emit_u32(handle->nb_efd);
)
/* Memory */
@@ -223,54 +271,6 @@ RTE_TRACE_POINT(
rte_trace_point_emit_string(cpuset);
)
-/* Interrupt */
-RTE_TRACE_POINT(
- rte_eal_trace_intr_callback_register,
- RTE_TRACE_POINT_ARGS(const struct rte_intr_handle *handle,
- rte_intr_callback_fn cb, void *cb_arg, int rc),
- rte_trace_point_emit_int(rc);
- rte_trace_point_emit_int(handle->vfio_dev_fd);
- rte_trace_point_emit_int(handle->fd);
- rte_trace_point_emit_int(handle->type);
- rte_trace_point_emit_u32(handle->max_intr);
- rte_trace_point_emit_u32(handle->nb_efd);
- rte_trace_point_emit_ptr(cb);
- rte_trace_point_emit_ptr(cb_arg);
-)
-RTE_TRACE_POINT(
- rte_eal_trace_intr_callback_unregister,
- RTE_TRACE_POINT_ARGS(const struct rte_intr_handle *handle,
- rte_intr_callback_fn cb, void *cb_arg, int rc),
- rte_trace_point_emit_int(rc);
- rte_trace_point_emit_int(handle->vfio_dev_fd);
- rte_trace_point_emit_int(handle->fd);
- rte_trace_point_emit_int(handle->type);
- rte_trace_point_emit_u32(handle->max_intr);
- rte_trace_point_emit_u32(handle->nb_efd);
- rte_trace_point_emit_ptr(cb);
- rte_trace_point_emit_ptr(cb_arg);
-)
-RTE_TRACE_POINT(
- rte_eal_trace_intr_enable,
- RTE_TRACE_POINT_ARGS(const struct rte_intr_handle *handle, int rc),
- rte_trace_point_emit_int(rc);
- rte_trace_point_emit_int(handle->vfio_dev_fd);
- rte_trace_point_emit_int(handle->fd);
- rte_trace_point_emit_int(handle->type);
- rte_trace_point_emit_u32(handle->max_intr);
- rte_trace_point_emit_u32(handle->nb_efd);
-)
-RTE_TRACE_POINT(
- rte_eal_trace_intr_disable,
- RTE_TRACE_POINT_ARGS(const struct rte_intr_handle *handle, int rc),
- rte_trace_point_emit_int(rc);
- rte_trace_point_emit_int(handle->vfio_dev_fd);
- rte_trace_point_emit_int(handle->fd);
- rte_trace_point_emit_int(handle->type);
- rte_trace_point_emit_u32(handle->max_intr);
- rte_trace_point_emit_u32(handle->nb_efd);
-)
-
#ifdef __cplusplus
}
#endif
@@ -29,10 +29,6 @@ extern "C" {
/** The tracepoint object. */
typedef uint64_t rte_trace_point_t;
-/** Macro to define the tracepoint. */
-#define RTE_TRACE_POINT_DEFINE(tp) \
-rte_trace_point_t __attribute__((section("__rte_trace_point"))) __##tp
-
/**
* Macro to define the tracepoint arguments in RTE_TRACE_POINT macro.
@@ -64,7 +60,7 @@ _tp _args \
*
* @param tp
* Tracepoint object. Before using the tracepoint, an application needs to
- * define the tracepoint using RTE_TRACE_POINT_DEFINE macro.
+ * define the tracepoint using RTE_TRACE_POINT_REGISTER macro.
* @param args
* C function style input arguments to define the arguments to tracepoint
* function.
@@ -72,7 +68,7 @@ _tp _args \
* Define the payload of trace function. The payload will be formed using
* rte_trace_point_emit_* macros. Use ";" delimiter between two payloads.
*
- * @see RTE_TRACE_POINT_ARGS, RTE_TRACE_POINT_DEFINE, rte_trace_point_emit_*
+ * @see RTE_TRACE_POINT_ARGS, RTE_TRACE_POINT_REGISTER, rte_trace_point_emit_*
*/
#define RTE_TRACE_POINT(tp, args, ...) \
__RTE_TRACE_POINT(generic, tp, args, __VA_ARGS__)
@@ -85,7 +81,7 @@ _tp _args \
*
* @param tp
* Tracepoint object. Before using the tracepoint, an application needs to
- * define the tracepoint using RTE_TRACE_POINT_DEFINE macro.
+ * define the tracepoint using RTE_TRACE_POINT_REGISTER macro.
* @param args
* C function style input arguments to define the arguments to tracepoint.
* function.
@@ -115,7 +111,7 @@ _tp _args \
* Register a tracepoint.
*
* @param trace
- * The tracepoint object created using RTE_TRACE_POINT_DEFINE.
+ * The tracepoint object created using RTE_TRACE_POINT_REGISTER.
* @param name
* The name of the tracepoint object.
* @return
@@ -262,7 +258,7 @@ void __rte_trace_point_emit_field(size_t sz, const char *field,
* Use RTE_TRACE_POINT_REGISTER macro for tracepoint registration.
*
* @param trace
- * The tracepoint object created using RTE_TRACE_POINT_DEFINE.
+ * The tracepoint object created using RTE_TRACE_POINT_REGISTER.
* @param name
* The name of the tracepoint object.
* @param register_fn
@@ -14,8 +14,12 @@
RTE_DECLARE_PER_LCORE(volatile int, trace_point_sz);
#define RTE_TRACE_POINT_REGISTER(trace, name) \
+rte_trace_point_t __attribute__((section("__rte_trace_point"))) __##trace; \
+RTE_INIT(trace##_init) \
+{ \
__rte_trace_point_register(&__##trace, RTE_STR(name), \
- (void (*)(void)) trace)
+ (void (*)(void)) trace); \
+}
#define __rte_trace_point_emit_header_generic(t) \
RTE_PER_LCORE(trace_point_sz) = __RTE_TRACE_EVENT_HEADER_SZ
@@ -6,38 +6,26 @@
#include <rte_ethdev_trace.h>
-RTE_TRACE_POINT_DEFINE(rte_ethdev_trace_configure);
-RTE_TRACE_POINT_DEFINE(rte_ethdev_trace_rxq_setup);
-RTE_TRACE_POINT_DEFINE(rte_ethdev_trace_txq_setup);
-RTE_TRACE_POINT_DEFINE(rte_ethdev_trace_start);
-RTE_TRACE_POINT_DEFINE(rte_ethdev_trace_stop);
-RTE_TRACE_POINT_DEFINE(rte_ethdev_trace_close);
-RTE_TRACE_POINT_DEFINE(rte_ethdev_trace_rx_burst);
-RTE_TRACE_POINT_DEFINE(rte_ethdev_trace_tx_burst);
+RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_configure,
+ lib.ethdev.configure)
-RTE_INIT(ethdev_trace_init)
-{
- RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_configure,
- lib.ethdev.configure);
+RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_rxq_setup,
+ lib.ethdev.rxq.setup)
- RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_rxq_setup,
- lib.ethdev.rxq.setup);
+RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_txq_setup,
+ lib.ethdev.txq.setup)
- RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_txq_setup,
- lib.ethdev.txq.setup);
+RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_start,
+ lib.ethdev.start)
- RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_start,
- lib.ethdev.start);
+RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_stop,
+ lib.ethdev.stop)
- RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_stop,
- lib.ethdev.stop);
+RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_close,
+ lib.ethdev.close)
- RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_close,
- lib.ethdev.close);
+RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_rx_burst,
+ lib.ethdev.rx.burst)
- RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_rx_burst,
- lib.ethdev.rx.burst);
-
- RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_tx_burst,
- lib.ethdev.tx.burst);
-}
+RTE_TRACE_POINT_REGISTER(rte_ethdev_trace_tx_burst,
+ lib.ethdev.tx.burst)
@@ -7,167 +7,114 @@
#include "rte_eventdev_trace.h"
/* Eventdev trace points */
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_configure);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_queue_setup);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_port_setup);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_port_link);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_port_unlink);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_start);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_stop);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_close);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_enq_burst);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_deq_burst);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_configure,
+ lib.eventdev.configure)
-/* Eventdev Rx adapter trace points */
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_rx_adapter_create);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_rx_adapter_free);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_rx_adapter_queue_add);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_rx_adapter_queue_del);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_rx_adapter_start);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_rx_adapter_stop);
-
-/* Eventdev Tx adapter trace points */
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_tx_adapter_create);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_tx_adapter_free);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_tx_adapter_queue_add);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_tx_adapter_queue_del);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_tx_adapter_start);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_tx_adapter_stop);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_eth_tx_adapter_enqueue);
-
-/* Eventdev Timer adapter trace points */
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_timer_adapter_create);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_timer_adapter_start);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_timer_adapter_stop);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_timer_adapter_free);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_timer_arm_burst);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_timer_arm_tmo_tick_burst);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_timer_cancel_burst);
-
-/* Eventdev Crypto adapter trace points */
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_crypto_adapter_create);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_crypto_adapter_free);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_crypto_adapter_queue_pair_add);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_crypto_adapter_queue_pair_del);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_crypto_adapter_start);
-RTE_TRACE_POINT_DEFINE(rte_eventdev_trace_crypto_adapter_stop);
-
-RTE_INIT(eventdev_trace_init)
-{
- /* Eventdev trace points */
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_configure,
- lib.eventdev.configure);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_queue_setup,
+ lib.eventdev.queue.setup)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_queue_setup,
- lib.eventdev.queue.setup);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_port_setup,
+ lib.eventdev.port.setup)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_port_setup,
- lib.eventdev.port.setup);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_port_link,
+ lib.eventdev.port.link)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_port_link,
- lib.eventdev.port.link);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_port_unlink,
+ lib.eventdev.port.unlink)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_port_unlink,
- lib.eventdev.port.unlink);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_start,
+ lib.eventdev.start)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_start,
- lib.eventdev.start);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_stop,
+ lib.eventdev.stop)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_stop,
- lib.eventdev.stop);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_close,
+ lib.eventdev.close)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_close,
- lib.eventdev.close);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_enq_burst,
+ lib.eventdev.enq.burst)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_enq_burst,
- lib.eventdev.enq.burst);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_deq_burst,
+ lib.eventdev.deq.burst)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_deq_burst,
- lib.eventdev.deq.burst);
-
-
- /* Eventdev Rx adapter trace points */
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_create,
- lib.eventdev.rx.adapter.create);
-
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_free,
- lib.eventdev.rx.adapter.free);
+/* Eventdev Rx adapter trace points */
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_create,
+ lib.eventdev.rx.adapter.create)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_queue_add,
- lib.eventdev.rx.adapter.queue.add);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_free,
+ lib.eventdev.rx.adapter.free)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_queue_del,
- lib.eventdev.rx.adapter.queue.del);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_queue_add,
+ lib.eventdev.rx.adapter.queue.add)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_start,
- lib.eventdev.rx.adapter.start);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_queue_del,
+ lib.eventdev.rx.adapter.queue.del)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_stop,
- lib.eventdev.rx.adapter.stop);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_start,
+ lib.eventdev.rx.adapter.start)
- /* Eventdev Tx adapter trace points */
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_create,
- lib.eventdev.tx.adapter.create);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_rx_adapter_stop,
+ lib.eventdev.rx.adapter.stop)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_free,
- lib.eventdev.tx.adapter.free);
+/* Eventdev Tx adapter trace points */
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_create,
+ lib.eventdev.tx.adapter.create)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_queue_add,
- lib.eventdev.tx.adapter.queue.add);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_free,
+ lib.eventdev.tx.adapter.free)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_queue_del,
- lib.eventdev.tx.adapter.queue.del);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_queue_add,
+ lib.eventdev.tx.adapter.queue.add)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_start,
- lib.eventdev.tx.adapter.start);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_queue_del,
+ lib.eventdev.tx.adapter.queue.del)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_stop,
- lib.eventdev.tx.adapter.stop);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_start,
+ lib.eventdev.tx.adapter.start)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_enqueue,
- lib.eventdev.tx.adapter.enq);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_stop,
+ lib.eventdev.tx.adapter.stop)
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_eth_tx_adapter_enqueue,
+ lib.eventdev.tx.adapter.enq)
- /* Eventdev Timer adapter trace points */
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_adapter_create,
- lib.eventdev.timer.create);
+/* Eventdev Timer adapter trace points */
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_adapter_create,
+ lib.eventdev.timer.create)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_adapter_start,
- lib.eventdev.timer.start);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_adapter_start,
+ lib.eventdev.timer.start)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_adapter_stop,
- lib.eventdev.timer.stop);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_adapter_stop,
+ lib.eventdev.timer.stop)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_adapter_free,
- lib.eventdev.timer.free);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_adapter_free,
+ lib.eventdev.timer.free)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_arm_burst,
- lib.eventdev.timer.burst);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_arm_burst,
+ lib.eventdev.timer.burst)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_arm_tmo_tick_burst,
- lib.eventdev.timer.tick.burst);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_arm_tmo_tick_burst,
+ lib.eventdev.timer.tick.burst)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_cancel_burst,
- lib.eventdev.timer.cancel);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_timer_cancel_burst,
+ lib.eventdev.timer.cancel)
- /* Eventdev Crypto adapter trace points */
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_crypto_adapter_create,
- lib.eventdev.crypto.create);
+/* Eventdev Crypto adapter trace points */
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_crypto_adapter_create,
+ lib.eventdev.crypto.create)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_crypto_adapter_free,
- lib.eventdev.crypto.free);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_crypto_adapter_free,
+ lib.eventdev.crypto.free)
- RTE_TRACE_POINT_REGISTER(
- rte_eventdev_trace_crypto_adapter_queue_pair_add,
- lib.eventdev.crypto.queue.add);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_crypto_adapter_queue_pair_add,
+ lib.eventdev.crypto.queue.add)
- RTE_TRACE_POINT_REGISTER(
- rte_eventdev_trace_crypto_adapter_queue_pair_del,
- lib.eventdev.crypto.queue.del);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_crypto_adapter_queue_pair_del,
+ lib.eventdev.crypto.queue.del)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_crypto_adapter_start,
- lib.eventdev.crypto.start);
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_crypto_adapter_start,
+ lib.eventdev.crypto.start)
- RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_crypto_adapter_stop,
- lib.eventdev.crypto.stop);
-}
+RTE_TRACE_POINT_REGISTER(rte_eventdev_trace_crypto_adapter_stop,
+ lib.eventdev.crypto.stop)
@@ -6,102 +6,74 @@
#include "rte_mempool_trace.h"
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_ops_dequeue_bulk);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_ops_dequeue_contig_blocks);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_ops_enqueue_bulk);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_generic_put);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_put_bulk);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_generic_get);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_get_bulk);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_get_contig_blocks);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_create);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_create_empty);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_free);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_populate_iova);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_populate_virt);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_populate_default);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_populate_anon);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_cache_create);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_cache_free);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_default_cache);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_get_page_size);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_cache_flush);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_ops_populate);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_ops_alloc);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_ops_free);
-RTE_TRACE_POINT_DEFINE(rte_mempool_trace_set_ops_byname);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_ops_dequeue_bulk,
+ lib.mempool.ops.deq.bulk)
-RTE_INIT(mempool_trace_init)
-{
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_ops_dequeue_bulk,
- lib.mempool.ops.deq.bulk);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_ops_dequeue_contig_blocks,
+ lib.mempool.ops.deq.contig)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_ops_dequeue_contig_blocks,
- lib.mempool.ops.deq.contig);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_ops_enqueue_bulk,
+ lib.mempool.ops.enq.bulk)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_ops_enqueue_bulk,
- lib.mempool.ops.enq.bulk);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_generic_put,
+ lib.mempool.generic.put)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_generic_put,
- lib.mempool.generic.put);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_put_bulk,
+ lib.mempool.put.bulk)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_put_bulk,
- lib.mempool.put.bulk);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_generic_get,
+ lib.mempool.generic.get)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_generic_get,
- lib.mempool.generic.get);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_get_bulk,
+ lib.mempool.get.bulk)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_get_bulk,
- lib.mempool.get.bulk);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_get_contig_blocks,
+ lib.mempool.get.blocks)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_get_contig_blocks,
- lib.mempool.get.blocks);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_create,
+ lib.mempool.create)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_create,
- lib.mempool.create);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_create_empty,
+ lib.mempool.create.empty)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_create_empty,
- lib.mempool.create.empty);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_free,
+ lib.mempool.free)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_free,
- lib.mempool.free);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_populate_iova,
+ lib.mempool.populate.iova)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_populate_iova,
- lib.mempool.populate.iova);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_populate_virt,
+ lib.mempool.populate.virt)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_populate_virt,
- lib.mempool.populate.virt);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_populate_default,
+ lib.mempool.populate.default)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_populate_default,
- lib.mempool.populate.default);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_populate_anon,
+ lib.mempool.populate.anon)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_populate_anon,
- lib.mempool.populate.anon);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_cache_create,
+ lib.mempool.cache_create)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_cache_create,
- lib.mempool.cache_create);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_cache_free,
+ lib.mempool.cache.free)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_cache_free,
- lib.mempool.cache.free);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_default_cache,
+ lib.mempool.default.cache)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_default_cache,
- lib.mempool.default.cache);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_get_page_size,
+ lib.mempool.get.page.size)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_get_page_size,
- lib.mempool.get.page.size);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_cache_flush,
+ lib.mempool.cache.flush)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_cache_flush,
- lib.mempool.cache.flush);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_ops_populate,
+ lib.mempool.ops.populate)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_ops_populate,
- lib.mempool.ops.populate);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_ops_alloc,
+ lib.mempool.ops.alloc)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_ops_alloc,
- lib.mempool.ops.alloc);
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_ops_free,
+ lib.mempool.ops.free)
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_ops_free,
- lib.mempool.ops.free);
-
- RTE_TRACE_POINT_REGISTER(rte_mempool_trace_set_ops_byname,
- lib.mempool.set.ops.byname);
-}
+RTE_TRACE_POINT_REGISTER(rte_mempool_trace_set_ops_byname,
+ lib.mempool.set.ops.byname)