Re: [PATCH 3/3] perf-probe: Do not show the skipped events

From: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
To: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	stable@vger.kernel.org
Subject: Re: [PATCH 3/3] perf-probe: Do not show the skipped events
Date: Fri, 24 Apr 2020 10:08:03 -0300	[thread overview]
Message-ID: <20200424130803.GL19437@kernel.org> (raw)
In-Reply-To: <20200424083305.6bff9456650308ab7a4ab750@kernel.org>

Em Fri, Apr 24, 2020 at 08:33:05AM +0900, Masami Hiramatsu escreveu:
> On Thu, 23 Apr 2020 11:01:39 -0300
> Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> 
> > Em Thu, Apr 23, 2020 at 08:01:22PM +0900, Masami Hiramatsu escreveu:
> > > When a probe point is expanded to several places (like inlined) and
> > > if some of them are skipped because of blacklisted or __init function,
> > > those trace_events has no event name. It must be skipped while showing
> > > results.
> > > 
> > > Without this fix, you can see "(null):(null)" on the list,
> > > ===========
> > 
> > Ok, you broke the patch in two, I think its better to combine both, ok?
> 
> No, if an inlined function is embedded in blacklisted areas, it also
> shows same "(null):(null)" without [2/3].
> 
> Reordering the patches is OK, but this is still an independent fix.

Ok, so I'll try reordering, so that we don't see it in the cset log for
the other fix.

Thanks for the clarification,

- Arnaldo

> Thank you,
> 
> > 
> > - Arnaldo
> > 
> > >   # ./perf probe request_resource
> > >   reserve_setup is out of .text, skip it.
> > >   Added new events:
> > >     (null):(null)        (on request_resource)
> > >     probe:request_resource (on request_resource)
> > > 
> > >   You can now use it in all perf tools, such as:
> > > 
> > >   	perf record -e probe:request_resource -aR sleep 1
> > > 
> > > ===========
> > > 
> > > With this fix, it is ignored.
> > > ===========
> > >   # ./perf probe request_resource
> > >   reserve_setup is out of .text, skip it.
> > >   Added new events:
> > >     probe:request_resource (on request_resource)
> > > 
> > >   You can now use it in all perf tools, such as:
> > > 
> > >   	perf record -e probe:request_resource -aR sleep 1
> > > 
> > > ===========
> > > 
> > > Fixes: 5a51fcd1f30c ("perf probe: Skip kernel symbols which is out of .text")
> > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > > Cc: stable@vger.kernel.org
> > > ---
> > >  tools/perf/builtin-probe.c |    3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> > > index 70548df2abb9..6b1507566770 100644
> > > --- a/tools/perf/builtin-probe.c
> > > +++ b/tools/perf/builtin-probe.c
> > > @@ -364,6 +364,9 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
> > >  
> > >  		for (k = 0; k < pev->ntevs; k++) {
> > >  			struct probe_trace_event *tev = &pev->tevs[k];
> > > +			/* Skipped events have no event name */
> > > +			if (!tev->event)
> > > +				continue;
> > >  
> > >  			/* We use tev's name for showing new events */
> > >  			show_perf_probe_event(tev->group, tev->event, pev,
> > > 
> > 
> > -- 
> > 
> > - Arnaldo
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@kernel.org>

-- 

- Arnaldo