Re: [PATCH 3/4] SUNRPC: Replace dprintk() call site in xs_data_ready

Re: [PATCH 3/4] SUNRPC: Replace dprintk() call site in xs_data_ready

[Steven Rostedt]

[ Added the user space perf folks ]

On Fri, 22 Jul 2022 18:45:30 +0000
Chuck Lever III <chuck.lever@oracle.com> wrote:

> >> +TRACE_EVENT(xs_data_ready,
> >> +  TP_PROTO(
> >> +  const struct rpc_xprt *xprt
> >> +  ),
> >> +
> >> +  TP_ARGS(xprt),
> >> +
> >> +  TP_STRUCT__entry(
> >> +  __sockaddr(addr, xprt->addrlen)
> >> +  ),
> >> +
> >> +  TP_fast_assign(
> >> +  __assign_sockaddr(addr, &xprt->addr, xprt->addrlen);
> >> +  ),
> >> +
> >> +  TP_printk("peer=%pISpc", __get_sockaddr(addr))  
> > 
> > NACK. Please resolve and store the string up front instead of storing
> > the sockaddr. Most versions of perf can't resolve those kernel-specific
> > %p printks and just end up barfing on them.  
> 
> Interesting. We added get_sockaddr() to avoid this issue in
> trace-cmd. Sounds like perf needs to be fixed up too, or
> maybe this is another case of having an old libtraceevent?
> 
> Meanwhile, I can revert this back to the old way of handling
> presentation addresses.
> 

Hmm, I thought that perf now uses the external libtraceevent.

Perhaps it hasn't been updated to the latest release that has the ability
to parse this.

Maybe just install

  git://git.kernel.org/pub/scm/libs/libtrace/libtraceevent.git

?

-- Steve

Re: [PATCH 3/4] SUNRPC: Replace dprintk() call site in xs_data_ready

[Arnaldo Carvalho de Melo]

Em Fri, Jul 22, 2022 at 04:22:12PM -0400, Steven Rostedt escreveu:
> [ Added the user space perf folks ]
> 
> On Fri, 22 Jul 2022 18:45:30 +0000
> Chuck Lever III <chuck.lever@oracle.com> wrote:
> 
> > >> +TRACE_EVENT(xs_data_ready,
> > >> +  TP_PROTO(
> > >> +  const struct rpc_xprt *xprt
> > >> +  ),
> > >> +
> > >> +  TP_ARGS(xprt),
> > >> +
> > >> +  TP_STRUCT__entry(
> > >> +  __sockaddr(addr, xprt->addrlen)
> > >> +  ),
> > >> +
> > >> +  TP_fast_assign(
> > >> +  __assign_sockaddr(addr, &xprt->addr, xprt->addrlen);
> > >> +  ),
> > >> +
> > >> +  TP_printk("peer=%pISpc", __get_sockaddr(addr))  
> > > 
> > > NACK. Please resolve and store the string up front instead of storing
> > > the sockaddr. Most versions of perf can't resolve those kernel-specific
> > > %p printks and just end up barfing on them.  
> > 
> > Interesting. We added get_sockaddr() to avoid this issue in
> > trace-cmd. Sounds like perf needs to be fixed up too, or
> > maybe this is another case of having an old libtraceevent?
> > 
> > Meanwhile, I can revert this back to the old way of handling
> > presentation addresses.
> > 
> 
> Hmm, I thought that perf now uses the external libtraceevent.
> 
> Perhaps it hasn't been updated to the latest release that has the ability
> to parse this.
> 
> Maybe just install
> 
>   git://git.kernel.org/pub/scm/libs/libtrace/libtraceevent.git

To use it one has to use:

  make -C tools/perf LIBTRACEEVENT_DYNAMIC=1

Then we get it linked with libtraceevent-devel:

$ ldd ~/bin/perf | grep traceevent
	libtraceevent.so.1 => /lib64/libtraceevent.so.1 (0x00007faa50f93000)
$

Perhaps it'd be better to check if libtracevent-devel is installed and
use it, falling back to tools/lib/traceevent/ and then adding a warning
that the in-tree codebase is being used?

- Arnaldo

Re: [PATCH 3/4] SUNRPC: Replace dprintk() call site in xs_data_ready

[Steven Rostedt]

On Mon, 25 Jul 2022 12:49:20 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Perhaps it'd be better to check if libtracevent-devel is installed and
> use it, falling back to tools/lib/traceevent/ and then adding a warning
> that the in-tree codebase is being used?

Yeah, this is the way trace-cmd went. For a few releases, it would just
warn that it couldn't find the system libraries, and then fall back to the
internals, and then it finally just removed the internals and failed with a
message stating where to get the necessary libraries.

-- Steve