Re: [RFC 2/5] common/mlx5: introduce tracepoints for mlx5 drivers

[Thomas Monjalon <thomas@monjalon.net>]

27/06/2023 11:43, Slava Ovsiienko:
> From: Thomas Monjalon <thomas@monjalon.net>
> > 27/06/2023 10:19, Slava Ovsiienko:
> > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > 27/06/2023 08:15, Slava Ovsiienko:
> > > > > From: Thomas Monjalon <thomas@monjalon.net>
> > > > > > 13/06/2023 18:01, Jerin Jacob:
> > > > > > > On Tue, Jun 13, 2023 at 9:29 PM Slava Ovsiienko
> > > > > > > <viacheslavo@nvidia.com>
> > > > > > wrote:
> > > > > > > > From: Jerin Jacob <jerinjacobk@gmail.com>
> > > > > > > > > On Tue, Jun 13, 2023 at 9:20 PM Slava Ovsiienko
> > > > > > > > > <viacheslavo@nvidia.com>
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi,
> > > > > > > > > >
> > > > > > > > > > <..snip..>
> > > > > > > > > > > >
> > > > > > > > > > > >         mlx5_os_interrupt_handler_create; #
> > > > WINDOWS_NO_EXPORT
> > > > > > > > > > > >         mlx5_os_interrupt_handler_destroy; #
> > > > > > > > > > > > WINDOWS_NO_EXPORT
> > > > > > > > > > > > +
> > > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_entry;
> > > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_exit;
> > > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_wqe;
> > > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_wait;
> > > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_push;
> > > > > > > > > > > > +       __rte_pmd_mlx5_trace_tx_complete;
> > > > > > > > > > >
> > > > > > > > > > > No need to expose these symbols. It is getting removed
> > > > > > > > > > > from rest of
> > > > > > DPDK.
> > > > > > > > > > > Application can do rte_trace_lookup() to get this address.
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > It is not for application, it is for PMD itself, w/o
> > > > > > > > > > exposing the symbols build
> > > > > > > > > failed.
> > > > > > > > >
> > > > > > > > > PMD is implementing this trace endpoints, not consuming
> > > > > > > > > this trace
> > > > > > point.
> > > > > > > > > Right? If so, Why to expose these symbols?
> > > > > > > >
> > > > > > > > As far as understand:
> > > > > > > > The tracepoint routines are defined in dedicated
> > > > > > > > common/mlx5_trace.c
> > > > > > file.
> > > > > > > > The tx_burst in mlx5 is implemented as template in header
> > > > > > > > file, and this template is used in multiple .c files under net/mlx5
> > filder.
> > > > > > > > So, common/mlx5 should expose its symbols to net/mlx5 to
> > > > > > > > allow successful linkage.
> > > > > > >
> > > > > > > OK. I missed the fact the these are in common code and net
> > > > > > > driver is depened on that.
> > > > > > > So changes makes sense.
> > > > > >
> > > > > > It does not make sense to me.
> > > > > > These are tracepoints for the ethdev driver.
> > > > > > Why declaring them in the common library?
> > > > >
> > > > > Just to gather all mlx5 traces in the single file, to see all
> > > > > available tracing
> > > > caps in single view.
> > > >
> > > > Better to not export them.
> > > >
> > > It is not about export, we have analyzing script over the trace data,
> > > and it would be better to see all the tracing routines gathered in one place.
> > I would prefer not to spread trace point definitions over the multiple source
> > files.
> > 
> > You don't need to export trace symbols for using them.
> > It has been discussed already with Jerin that we prefer not exporting trace
> > symbols if it can be avoided. Here it can be avoided.
> 
> Without exporting symbols defined in common/mlx5 we have link error for net/mlx5.
> So, do you insist on having dedicated mlx5_trace.c per /net/mlx5, common/mlx5, etc?

Yes please it looks better to have tracepoints close to their respective functions.