* [PATCH] perf env: Normalize aarch64.* to arm64 in normalize_arch()
@ 2021-07-23 1:49 Li Huafei
2021-07-23 19:23 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 4+ messages in thread
From: Li Huafei @ 2021-07-23 1:49 UTC (permalink / raw)
To: jolsa, acme
Cc: peterz, mark.rutland, mingo, alexander.shishkin, namhyung,
mliska, irogers, dzhu, rickyman7, yao.jin, linux-perf-users,
linux-kernel, zhangjinhao2
On my aarch64 big endian machine, the perf annotate does not work.
# perf annotate
Percent | Source code & Disassembly of [kernel.kallsyms] for cycles (253 samples, percent: local period)
--------------------------------------------------------------------------------------------------------------
Percent | Source code & Disassembly of [kernel.kallsyms] for cycles (1 samples, percent: local period)
------------------------------------------------------------------------------------------------------------
Percent | Source code & Disassembly of [kernel.kallsyms] for cycles (47 samples, percent: local period)
-------------------------------------------------------------------------------------------------------------
...
This is because the arch_find() function uses the normalized architecture
name provided by normalize_arch(), and my machine's architecture name
aarch64_be is not normalized to arm64. Like other architectures such as
arm and powerpc, we can fuzzy match the architecture names associated with
aarch64.* and normalize them.
Fixes: c34df25b40c2 ("perf annotate: Add symbol__annotate function")
Signed-off-by: Li Huafei <lihuafei1@huawei.com>
---
tools/perf/util/annotate.c | 4 +++-
tools/perf/util/env.c | 2 +-
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index aa04a3655236..cb280de3369f 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -2192,8 +2192,10 @@ int symbol__annotate(struct map_symbol *ms, struct evsel *evsel,
return errno;
args.arch = arch = arch__find(arch_name);
- if (arch == NULL)
+ if (arch == NULL) {
+ pr_err("%s: unsupported arch %s\n", __func__, arch_name);
return ENOTSUP;
+ }
if (parch)
*parch = arch;
diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c
index cec2e6cad8aa..a91da1e9b201 100644
--- a/tools/perf/util/env.c
+++ b/tools/perf/util/env.c
@@ -349,7 +349,7 @@ static const char *normalize_arch(char *arch)
return "x86";
if (!strcmp(arch, "sun4u") || !strncmp(arch, "sparc", 5))
return "sparc";
- if (!strcmp(arch, "aarch64") || !strcmp(arch, "arm64"))
+ if (!strncmp(arch, "aarch64", 7) || !strcmp(arch, "arm64"))
return "arm64";
if (!strncmp(arch, "arm", 3) || !strcmp(arch, "sa110"))
return "arm";
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] perf env: Normalize aarch64.* to arm64 in normalize_arch() 2021-07-23 1:49 [PATCH] perf env: Normalize aarch64.* to arm64 in normalize_arch() Li Huafei @ 2021-07-23 19:23 ` Arnaldo Carvalho de Melo 2021-07-26 10:04 ` James Clark 0 siblings, 1 reply; 4+ messages in thread From: Arnaldo Carvalho de Melo @ 2021-07-23 19:23 UTC (permalink / raw) To: Li Huafei Cc: jolsa, peterz, mark.rutland, mingo, alexander.shishkin, namhyung, mliska, irogers, dzhu, rickyman7, yao.jin, linux-perf-users, linux-kernel, zhangjinhao2 Em Fri, Jul 23, 2021 at 09:49:44AM +0800, Li Huafei escreveu: > On my aarch64 big endian machine, the perf annotate does not work. > > # perf annotate > Percent | Source code & Disassembly of [kernel.kallsyms] for cycles (253 samples, percent: local period) > -------------------------------------------------------------------------------------------------------------- > Percent | Source code & Disassembly of [kernel.kallsyms] for cycles (1 samples, percent: local period) > ------------------------------------------------------------------------------------------------------------ > Percent | Source code & Disassembly of [kernel.kallsyms] for cycles (47 samples, percent: local period) > ------------------------------------------------------------------------------------------------------------- > ... > > This is because the arch_find() function uses the normalized architecture > name provided by normalize_arch(), and my machine's architecture name > aarch64_be is not normalized to arm64. Like other architectures such as > arm and powerpc, we can fuzzy match the architecture names associated with > aarch64.* and normalize them. This looks ok modulo fixing the problem and adding that extra pr_err() in a single patch, please split this into two. Also I fail to see why c34df25b40c2 introduced this problem :-\ Can some ARM person ack/review this, please? - Arnaldo > Fixes: c34df25b40c2 ("perf annotate: Add symbol__annotate function") > Signed-off-by: Li Huafei <lihuafei1@huawei.com> > --- > tools/perf/util/annotate.c | 4 +++- > tools/perf/util/env.c | 2 +- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index aa04a3655236..cb280de3369f 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -2192,8 +2192,10 @@ int symbol__annotate(struct map_symbol *ms, struct evsel *evsel, > return errno; > > args.arch = arch = arch__find(arch_name); > - if (arch == NULL) > + if (arch == NULL) { > + pr_err("%s: unsupported arch %s\n", __func__, arch_name); > return ENOTSUP; > + } > > if (parch) > *parch = arch; > diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c > index cec2e6cad8aa..a91da1e9b201 100644 > --- a/tools/perf/util/env.c > +++ b/tools/perf/util/env.c > @@ -349,7 +349,7 @@ static const char *normalize_arch(char *arch) > return "x86"; > if (!strcmp(arch, "sun4u") || !strncmp(arch, "sparc", 5)) > return "sparc"; > - if (!strcmp(arch, "aarch64") || !strcmp(arch, "arm64")) > + if (!strncmp(arch, "aarch64", 7) || !strcmp(arch, "arm64")) > return "arm64"; > if (!strncmp(arch, "arm", 3) || !strcmp(arch, "sa110")) > return "arm"; > -- > 2.17.1 > -- - Arnaldo ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf env: Normalize aarch64.* to arm64 in normalize_arch() 2021-07-23 19:23 ` Arnaldo Carvalho de Melo @ 2021-07-26 10:04 ` James Clark 2021-07-26 13:04 ` Li Huafei 0 siblings, 1 reply; 4+ messages in thread From: James Clark @ 2021-07-26 10:04 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Li Huafei Cc: jolsa, peterz, mark.rutland, mingo, alexander.shishkin, namhyung, mliska, irogers, dzhu, rickyman7, yao.jin, linux-perf-users, linux-kernel, zhangjinhao2 On 23/07/2021 20:23, Arnaldo Carvalho de Melo wrote: > Em Fri, Jul 23, 2021 at 09:49:44AM +0800, Li Huafei escreveu: >> On my aarch64 big endian machine, the perf annotate does not work. >> >> # perf annotate >> Percent | Source code & Disassembly of [kernel.kallsyms] for cycles (253 samples, percent: local period) >> -------------------------------------------------------------------------------------------------------------- >> Percent | Source code & Disassembly of [kernel.kallsyms] for cycles (1 samples, percent: local period) >> ------------------------------------------------------------------------------------------------------------ >> Percent | Source code & Disassembly of [kernel.kallsyms] for cycles (47 samples, percent: local period) >> ------------------------------------------------------------------------------------------------------------- >> ... >> >> This is because the arch_find() function uses the normalized architecture >> name provided by normalize_arch(), and my machine's architecture name >> aarch64_be is not normalized to arm64. Like other architectures such as >> arm and powerpc, we can fuzzy match the architecture names associated with >> aarch64.* and normalize them. > > This looks ok modulo fixing the problem and adding that extra pr_err() > in a single patch, please split this into two. > > Also I fail to see why c34df25b40c2 introduced this problem :-\ I checked the parent commit of c34df25b40c2 and set the architecture to "aarch64_be" but it doesn't work either, so I also don't see any regressions. > > Can some ARM person ack/review this, please? > > - Arnaldo > >> Fixes: c34df25b40c2 ("perf annotate: Add symbol__annotate function") >> Signed-off-by: Li Huafei <lihuafei1@huawei.com> >> --- >> tools/perf/util/annotate.c | 4 +++- >> tools/perf/util/env.c | 2 +- >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >> index aa04a3655236..cb280de3369f 100644 >> --- a/tools/perf/util/annotate.c >> +++ b/tools/perf/util/annotate.c >> @@ -2192,8 +2192,10 @@ int symbol__annotate(struct map_symbol *ms, struct evsel *evsel, >> return errno; >> >> args.arch = arch = arch__find(arch_name); >> - if (arch == NULL) >> + if (arch == NULL) { >> + pr_err("%s: unsupported arch %s\n", __func__, arch_name); >> return ENOTSUP; >> + } >> >> if (parch) >> *parch = arch; >> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c >> index cec2e6cad8aa..a91da1e9b201 100644 >> --- a/tools/perf/util/env.c >> +++ b/tools/perf/util/env.c >> @@ -349,7 +349,7 @@ static const char *normalize_arch(char *arch) >> return "x86"; >> if (!strcmp(arch, "sun4u") || !strncmp(arch, "sparc", 5)) >> return "sparc"; >> - if (!strcmp(arch, "aarch64") || !strcmp(arch, "arm64")) >> + if (!strncmp(arch, "aarch64", 7) || !strcmp(arch, "arm64")) This seems ok to me, but a quick google shows some references to "arm64_be". I don't know if this could ever get into perf, but most of the other ones search for prefixes, so it probably doesn't hurt to do strcmp(arch, "arm64", 5) as well. Thanks James >> return "arm64"; >> if (!strncmp(arch, "arm", 3) || !strcmp(arch, "sa110")) >> return "arm"; >> -- >> 2.17.1 >> > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] perf env: Normalize aarch64.* to arm64 in normalize_arch() 2021-07-26 10:04 ` James Clark @ 2021-07-26 13:04 ` Li Huafei 0 siblings, 0 replies; 4+ messages in thread From: Li Huafei @ 2021-07-26 13:04 UTC (permalink / raw) To: James Clark, Arnaldo Carvalho de Melo Cc: jolsa, peterz, mark.rutland, mingo, alexander.shishkin, namhyung, mliska, irogers, dzhu, rickyman7, yao.jin, linux-perf-users, linux-kernel, zhangjinhao2 On 2021/7/26 18:04, James Clark wrote: > > > On 23/07/2021 20:23, Arnaldo Carvalho de Melo wrote: >> Em Fri, Jul 23, 2021 at 09:49:44AM +0800, Li Huafei escreveu: >>> On my aarch64 big endian machine, the perf annotate does not work. >>> >>> # perf annotate >>> Percent | Source code & Disassembly of [kernel.kallsyms] for cycles (253 samples, percent: local period) >>> -------------------------------------------------------------------------------------------------------------- >>> Percent | Source code & Disassembly of [kernel.kallsyms] for cycles (1 samples, percent: local period) >>> ------------------------------------------------------------------------------------------------------------ >>> Percent | Source code & Disassembly of [kernel.kallsyms] for cycles (47 samples, percent: local period) >>> ------------------------------------------------------------------------------------------------------------- >>> ... >>> >>> This is because the arch_find() function uses the normalized architecture >>> name provided by normalize_arch(), and my machine's architecture name >>> aarch64_be is not normalized to arm64. Like other architectures such as >>> arm and powerpc, we can fuzzy match the architecture names associated with >>> aarch64.* and normalize them. >> >> This looks ok modulo fixing the problem and adding that extra pr_err() >> in a single patch, please split this into two. OK, I've sent the patch set for v2, which puts pr_err() in a single patch: https://lore.kernel.org/patchwork/patch/1467154/ >> >> Also I fail to see why c34df25b40c2 introduced this problem :-\ > > I checked the parent commit of c34df25b40c2 and set the architecture to "aarch64_be" > but it doesn't work either, so I also don't see any regressions. > Sorry, this may be my mistake. It should be that perf annotate adds support for aarch64_be, not that there is a bug in the existing code. In v2, I've removed the "Fixes" tag. >> >> Can some ARM person ack/review this, please? >> >> - Arnaldo >> >>> Fixes: c34df25b40c2 ("perf annotate: Add symbol__annotate function") >>> Signed-off-by: Li Huafei <lihuafei1@huawei.com> >>> --- >>> tools/perf/util/annotate.c | 4 +++- >>> tools/perf/util/env.c | 2 +- >>> 2 files changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c >>> index aa04a3655236..cb280de3369f 100644 >>> --- a/tools/perf/util/annotate.c >>> +++ b/tools/perf/util/annotate.c >>> @@ -2192,8 +2192,10 @@ int symbol__annotate(struct map_symbol *ms, struct evsel *evsel, >>> return errno; >>> >>> args.arch = arch = arch__find(arch_name); >>> - if (arch == NULL) >>> + if (arch == NULL) { >>> + pr_err("%s: unsupported arch %s\n", __func__, arch_name); >>> return ENOTSUP; >>> + } >>> >>> if (parch) >>> *parch = arch; >>> diff --git a/tools/perf/util/env.c b/tools/perf/util/env.c >>> index cec2e6cad8aa..a91da1e9b201 100644 >>> --- a/tools/perf/util/env.c >>> +++ b/tools/perf/util/env.c >>> @@ -349,7 +349,7 @@ static const char *normalize_arch(char *arch) >>> return "x86"; >>> if (!strcmp(arch, "sun4u") || !strncmp(arch, "sparc", 5)) >>> return "sparc"; >>> - if (!strcmp(arch, "aarch64") || !strcmp(arch, "arm64")) >>> + if (!strncmp(arch, "aarch64", 7) || !strcmp(arch, "arm64")) > > This seems ok to me, but a quick google shows some references to "arm64_be". > I don't know if this could ever get into perf, but most of the other ones > search for prefixes, so it probably doesn't hurt to do strcmp(arch, "arm64", 5) > as well. In v2, I also added a normalization of "arm64_be". After this patch, perf annotate seems to work fine on my aarch64_be machine. And I checked other source files using perf_env__arch(): - arch/common.c - builtin-trace.c - util/sample-raw.c - util/thread-stack.c - util/unwind-libunwind.c Looks like it's OK. Thank you Arnaldo and James for the review. Huafei > > Thanks > James > >>> return "arm64"; >>> if (!strncmp(arch, "arm", 3) || !strcmp(arch, "sa110")) >>> return "arm"; >>> -- >>> 2.17.1 >>> >> > . > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-26 13:04 UTC | newest] Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-23 1:49 [PATCH] perf env: Normalize aarch64.* to arm64 in normalize_arch() Li Huafei 2021-07-23 19:23 ` Arnaldo Carvalho de Melo 2021-07-26 10:04 ` James Clark 2021-07-26 13:04 ` Li Huafei
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).