| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 3 Apr 2019 21:48:23 +0000
From: Song Liu <songliubraving@...com>
To: Arnaldo Carvalho de Melo <arnaldo.melo@...il.com>
CC: Jiri Olsa <jolsa@...hat.com>,
Adrian Hunter <adrian.hunter@...el.com>,
Andi Kleen <andi@...stfloor.org>,
"jolsa@...nel.org" <jolsa@...nel.org>,
"namhyung@...nel.org" <namhyung@...nel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-perf-users@...r.kernel.org" <linux-perf-users@...r.kernel.org>,
Andi Kleen <ak@...ux.intel.com>
Subject: Re: [BUG] perf: intel_pt won't display kernel function
> On Apr 3, 2019, at 11:59 AM, Song Liu <songliubraving@...com> wrote:
>
>
>
>> On Apr 3, 2019, at 11:55 AM, Song Liu <songliubraving@...com> wrote:
>>
>>
>>
>>> On Apr 3, 2019, at 11:50 AM, Arnaldo Carvalho de Melo <arnaldo.melo@...il.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@...il.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@...com>
>>>>
>>>> Thanks,
>>>> Song
>>>>
>>>>
>>>>> commit 011318ccc2024ba03e96c32a06f74ca5d6ab5503
>>>>> Author: Arnaldo Carvalho de Melo <acme@...hat.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@...el.com>
>>>>> Cc: Alexei Starovoitov <ast@...nel.org>
>>>>> Cc: Andrii Nakryiko <andrii.nakryiko@...il.com>
>>>>> Cc: Daniel Borkmann <daniel@...earbox.net>
>>>>> Cc: Jiri Olsa <jolsa@...nel.org>
>>>>> Cc: Martin KaFai Lau <kafai@...com>
>>>>> Cc: Namhyung Kim <namhyung@...nel.org>
>>>>> Cc: Peter Zijlstra <peterz@...radead.org>
>>>>> Cc: Song Liu <songliubraving@...com>
>>>>> Cc: Yonghong Song <yhs@...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@...hat.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
Yet another version, with commit message and a little more logic:
>From 8c842b52905e747487447a014cef8c98d0db94ef Mon Sep 17 00:00:00 2001
From: Song Liu <songliubraving@...com>
Date: Wed, 3 Apr 2019 14:24:02 -0700
Subject: [PATCH] perf util: check maps for bpf programs
As reported by Jiri Olsa in:
"[BUG] perf: intel_pt won't display kernel function"
https://lore.kernel.org/lkml/20190403143738.GB32001@krava
Recent changes to support PERF_RECORD_KSYMBOL and PERF_RECORD_BPF_EVENT
broke --kallsyms option. This is because it broke test __map__is_kmodule.
This patch fixes this by adding check for bpf program, so that these maps
are not mistaken as kernel modules.
Reported-by: Jiri Olsa <jolsa@...nel.org>
Cc: Adrian Hunter <adrian.hunter@...el.com>
Cc: Alexei Starovoitov <ast@...nel.org>
Cc: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Daniel Borkmann <daniel@...earbox.net>
Cc: Martin KaFai Lau <kafai@...com>
Cc: Namhyung Kim <namhyung@...nel.org>
Cc: Peter Zijlstra <peterz@...radead.org>
Cc: Song Liu <songliubraving@...com>
Cc: Yonghong Song <yhs@...com>
Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
Fixes: 76193a94522f ("perf, bpf: Introduce PERF_RECORD_KSYMBOL")
Link: https://lore.kernel.org/lkml/20190403145353.GE32553@kernel.org
Signed-off-by: Song Liu <songliubraving@...com>
---
tools/perf/util/map.c | 16 ++++++++++++++++
tools/perf/util/map.h | 4 +++-
2 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index e32628cd20a7..28d484ef74ae 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -261,6 +261,22 @@ 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;
+
+ if (map->dso->binary_type == DSO_BINARY_TYPE__BPF_PROG_INFO)
+ return true;
+
+ /*
+ * If PERF_RECORD_BPF_EVENT is not included, the dso will not have
+ * type of DSO_BINARY_TYPE__BPF_PROG_INFO. In such cases, we can
+ * guess the type based on name.
+ */
+ 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 a/tools/perf/util/map.h b/tools/perf/util/map.h
index 0e20749f2c55..01079f1f4375 100644
--- a/tools/perf/util/map.h
+++ b/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);
--
2.17.1
Powered by blists - more mailing lists