Re: [BUG] perf: intel_pt won't display kernel function

[Song Liu <songliubraving@fb.com>]

> On Apr 3, 2019, at 11:55 AM, Song Liu <songliubraving@fb.com> wrote:
> 
> 
> 
>> On Apr 3, 2019, at 11:50 AM, Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
>> 
>> Em Wed, Apr 03, 2019 at 04:27:56PM +0000, Song Liu escreveu:
>>> 
>>> 
>>>> On Apr 3, 2019, at 8:15 AM, Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
>>>> 
>>>> Em Wed, Apr 03, 2019 at 11:53:53AM -0300, Arnaldo Carvalho de Melo escreveu:
>>>>> Em Wed, Apr 03, 2019 at 04:37:38PM +0200, Jiri Olsa escreveu:
>>>>>> hi,
>>>>>> perf script --call-trace stop working for me recently,
>>>>>> and displays only user space functions
>>>>>> 
>>>>>> I bisected that to:
>>>>>> 7b612e291a5a perf tools: Synthesize PERF_RECORD_* for loaded BPF programs
>>>>>> 
>>>>>> data from following comands will display user space functions only:
>>>>>> # perf-with-kcore record pt -e intel_pt// -- ls
>>>>>> # perf-with-kcore script pt --call-trace
>>>>>> 
>>>>>> when I disable the bpf synthesizing (patch below), kernel functions are back
>>>>>> 
>>>>>> I guess the new events mess up with intel_pt decoder somehow
>>>>> 
>>>>> I.e. I'm adding the patch below to my perf/urgent branch.
>>>> 
>>>> Song, that is what I have, can I have your Acked-by and please consider
>>>> taking a look at the bug Jiri reported,
>>>> 
>>>> Thanks,
>>>> 
>>>> - Arnaldo
>>> 
>>> Current logic with --no-bpf-event is to generate PERF_RECORD_KSYMBOL, but not
>>> PERF_RECORD_BPF_EVENT:
>> 
>> I see... The opts is done later, after querying the kernel for existing
>> BPF programs so that at least the KSYMBOL ones can be generated. So I'll
>> keep it as is, no need for this patch.
>> 
>>> https://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.com_linux_v5.1-2Drc3_source_tools_perf_util_bpf-2Devent.c-23L254&d=DwIBAg&c=5VD0RTtNlTh3ycd41b3MUw&r=dR8692q0_uaizy0jkrBJQM5k2hfm4CiFxYT8KaysFrg&m=8ITYgIrqFBIy1uQFQPHrsX5kBX5RlmhwcknKYJa2eGs&s=MdB2zA9JstwC93qKEonsfTQeQYiw-yeOLbPxm3QMRo8&e=
>>> 
>>> I will look into the intel_pt problem. 
>> 
>> Ok.
>> 
>>> In the meanwhile, let's fix it for now. 
>> 
>> I'll wait a bit.
> 
> I guess something like this might be a better fix? (Sorry for missing 
> commit message):
> 
> diff --git i/tools/perf/util/map.c w/tools/perf/util/map.c
> index e32628cd20a7..741430a35dca 100644
> --- i/tools/perf/util/map.c
> +++ w/tools/perf/util/map.c
> @@ -261,6 +261,12 @@ bool __map__is_extra_kernel_map(const struct map *map)
>        return kmap && kmap->name[0];
> }
> 
> +bool __map__is_bpf_prog(const struct map *map)
> +{
> +       const char *name = map->dso->short_name;
> +       return name && (strstr(name, "bpf_prog_") == name);
> +}
> +
> bool map__has_symbols(const struct map *map)
> {
>        return dso__has_symbols(map->dso);
> diff --git i/tools/perf/util/map.h w/tools/perf/util/map.h
> index 0e20749f2c55..01079f1f4375 100644
> --- i/tools/perf/util/map.h
> +++ w/tools/perf/util/map.h
> @@ -159,10 +159,12 @@ int map__set_kallsyms_ref_reloc_sym(struct map *map, const char *symbol_name,
> 
> bool __map__is_kernel(const struct map *map);
> bool __map__is_extra_kernel_map(const struct map *map);
> +bool __map__is_bpf_prog(const struct map *map);
> 
> static inline bool __map__is_kmodule(const struct map *map)
> {
> -       return !__map__is_kernel(map) && !__map__is_extra_kernel_map(map);
> +       return !__map__is_kernel(map) && !__map__is_extra_kernel_map(map) &&
> +               !__map__is_bpf_prog(map);
> }
> 
> bool map__has_symbols(const struct map *map);
> 
> Thanks,
> Song

Actually, it should be something like this:

diff --git i/tools/perf/util/map.c w/tools/perf/util/map.c
index e32628cd20a7..31cd23612529 100644
--- i/tools/perf/util/map.c
+++ w/tools/perf/util/map.c
@@ -261,6 +261,11 @@ bool __map__is_extra_kernel_map(const struct map *map)
        return kmap && kmap->name[0];
 }

+bool __map__is_bpf_prog(const struct map *map)
+{
+       return map->dso->binary_type == DSO_BINARY_TYPE__BPF_PROG_INFO;
+}
+
 bool map__has_symbols(const struct map *map)
 {
        return dso__has_symbols(map->dso);
diff --git i/tools/perf/util/map.h w/tools/perf/util/map.h
index 0e20749f2c55..01079f1f4375 100644
--- i/tools/perf/util/map.h
+++ w/tools/perf/util/map.h
@@ -159,10 +159,12 @@ int map__set_kallsyms_ref_reloc_sym(struct map *map, const char *symbol_name,

 bool __map__is_kernel(const struct map *map);
 bool __map__is_extra_kernel_map(const struct map *map);
+bool __map__is_bpf_prog(const struct map *map);

 static inline bool __map__is_kmodule(const struct map *map)
 {
-       return !__map__is_kernel(map) && !__map__is_extra_kernel_map(map);
+       return !__map__is_kernel(map) && !__map__is_extra_kernel_map(map) &&
+               !__map__is_bpf_prog(map);
 }

 bool map__has_symbols(const struct map *map);


> 
>> 
>>> Acked-by: Song Liu <songliubraving@fb.com>
>>> 
>>> Thanks, 
>>> Song
>>> 
>>> 
>>>> commit 011318ccc2024ba03e96c32a06f74ca5d6ab5503
>>>> Author: Arnaldo Carvalho de Melo <acme@redhat.com>
>>>> Date:   Wed Apr 3 12:05:15 2019 -0300
>>>> 
>>>>  perf record: Do not synthesize BPF records when --no-bpf-event is used
>>>> 
>>>>  By default we synthesize and ask the kernel for BPF events, having a
>>>>  --no-bpf-event option to disable that, which can be useful, for
>>>>  instance, if there are still bugs in that code, which seems to be the
>>>>  case as reported by Jiri Olsa in:
>>>> 
>>>>    "[BUG] perf: intel_pt won't display kernel function"
>>>>    https://lore.kernel.org/lkml/20190403143738.GB32001@krava
>>>> 
>>>>  So add the check for record_opts.no_bpf_event when considering
>>>>  synthesizing BPF events for pre-existing BPF programs in 'perf record'.
>>>> 
>>>>  The reported bug needs further analysis and is a separate matter.
>>>> 
>>>>  Cc: Adrian Hunter <adrian.hunter@intel.com>
>>>>  Cc: Alexei Starovoitov <ast@kernel.org>
>>>>  Cc: Andrii Nakryiko <andrii.nakryiko@gmail.com>
>>>>  Cc: Daniel Borkmann <daniel@iogearbox.net>
>>>>  Cc: Jiri Olsa <jolsa@kernel.org>
>>>>  Cc: Martin KaFai Lau <kafai@fb.com>
>>>>  Cc: Namhyung Kim <namhyung@kernel.org>
>>>>  Cc: Peter Zijlstra <peterz@infradead.org>
>>>>  Cc: Song Liu <songliubraving@fb.com>
>>>>  Cc: Yonghong Song <yhs@fb.com>
>>>>  Fixes: 7b612e291a5a ("perf tools: Synthesize PERF_RECORD_* for loaded BPF programs")
>>>>  Link: https://lore.kernel.org/lkml/20190403145353.GE32553@kernel.org
>>>>  Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
>>>> 
>>>> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
>>>> index 4e2d953d4bc5..17d772f192ad 100644
>>>> --- a/tools/perf/builtin-record.c
>>>> +++ b/tools/perf/builtin-record.c
>>>> @@ -1114,10 +1114,11 @@ static int record__synthesize(struct record *rec, bool tail)
>>>> 		return err;
>>>> 	}
>>>> 
>>>> -	err = perf_event__synthesize_bpf_events(session, process_synthesized_event,
>>>> -						machine, opts);
>>>> -	if (err < 0)
>>>> -		pr_warning("Couldn't synthesize bpf events.\n");
>>>> +	if (!opts->no_bpf_event) {
>>>> +		err = perf_event__synthesize_bpf_events(session, process_synthesized_event, machine, opts);
>>>> +		if (err < 0)
>>>> +			pr_warning("Couldn't synthesize bpf events.\n");
>>>> +	}
>>>> 
>>>> 	err = __machine__synthesize_threads(machine, tool, &opts->target, rec->evlist->threads,
>>>> 					    process_synthesized_event, opts->sample_address,
>> 
>> -- 
>> 
>> - Arnaldo