* [PATCH 1/3] perf script brstackinsn: Fix recovery from LBR/binary mismatch
@ 2019-09-27 23:35 Andi Kleen
2019-09-27 23:35 ` [PATCH 2/3] perf jevents: Fix period for Intel fixed counters Andi Kleen
` (3 more replies)
0 siblings, 4 replies; 7+ messages in thread
From: Andi Kleen @ 2019-09-27 23:35 UTC (permalink / raw)
To: acme; +Cc: jolsa, linux-kernel, Andi Kleen
From: Andi Kleen <ak@linux.intel.com>
When the LBR data and the instructions in a binary do not match the
loop printing instructions could get confused and print a long
stream of bogus <bad> instructions.
The problem was that if the instruction decoder cannot decode an
instruction it ilen wasn't initialized, so the loop going through
the basic block would continue with the previous value.
Harden the code to avoid such problems:
- Make sure ilen is always freshly initialized and is 0 for bad
instructions.
- Do not overrun the code buffer while printing instructions
- Print a warning message if the final jump is not on an
instruction boundary.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
tools/perf/builtin-script.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index e079b34201f2..32b17d51c982 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1061,7 +1061,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
continue;
insn = 0;
- for (off = 0;; off += ilen) {
+ for (off = 0; off < (unsigned)len; off += ilen) {
uint64_t ip = start + off;
printed += ip__fprintf_sym(ip, thread, x.cpumode, x.cpu, &lastsym, attr, fp);
@@ -1072,6 +1072,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
printed += print_srccode(thread, x.cpumode, ip);
break;
} else {
+ ilen = 0;
printed += fprintf(fp, "\t%016" PRIx64 "\t%s\n", ip,
dump_insn(&x, ip, buffer + off, len - off, &ilen));
if (ilen == 0)
@@ -1081,6 +1082,8 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
insn++;
}
}
+ if (off != (unsigned)len)
+ printed += fprintf(fp, "\tmismatch of LBR data and executable\n");
}
/*
@@ -1121,6 +1124,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
goto out;
}
for (off = 0; off <= end - start; off += ilen) {
+ ilen = 0;
printed += fprintf(fp, "\t%016" PRIx64 "\t%s\n", start + off,
dump_insn(&x, start + off, buffer + off, len - off, &ilen));
if (ilen == 0)
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] perf jevents: Fix period for Intel fixed counters
2019-09-27 23:35 [PATCH 1/3] perf script brstackinsn: Fix recovery from LBR/binary mismatch Andi Kleen
@ 2019-09-27 23:35 ` Andi Kleen
2019-09-30 10:50 ` Arnaldo Carvalho de Melo
2019-10-07 14:49 ` [tip: perf/urgent] " tip-bot2 for Andi Kleen
2019-09-27 23:35 ` [PATCH 3/3] perf annotate: Improve handling of corrupted ~/.debug Andi Kleen
` (2 subsequent siblings)
3 siblings, 2 replies; 7+ messages in thread
From: Andi Kleen @ 2019-09-27 23:35 UTC (permalink / raw)
To: acme; +Cc: jolsa, linux-kernel, Andi Kleen
From: Andi Kleen <ak@linux.intel.com>
The Intel fixed counters use a special table to override the JSON
information. During this override the period information from
the JSON file got dropped, which results in inst_retired.any
and similar running with frequency mode instead of a period.
Just specify the expected period in the table.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
tools/perf/pmu-events/jevents.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index d413761621b0..fa85e33762f7 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -449,12 +449,12 @@ static struct fixed {
const char *name;
const char *event;
} fixed[] = {
- { "inst_retired.any", "event=0xc0" },
- { "inst_retired.any_p", "event=0xc0" },
- { "cpu_clk_unhalted.ref", "event=0x0,umask=0x03" },
- { "cpu_clk_unhalted.thread", "event=0x3c" },
- { "cpu_clk_unhalted.core", "event=0x3c" },
- { "cpu_clk_unhalted.thread_any", "event=0x3c,any=1" },
+ { "inst_retired.any", "event=0xc0,period=2000003" },
+ { "inst_retired.any_p", "event=0xc0,period=2000003" },
+ { "cpu_clk_unhalted.ref", "event=0x0,umask=0x03,period=2000003" },
+ { "cpu_clk_unhalted.thread", "event=0x3c,period=2000003" },
+ { "cpu_clk_unhalted.core", "event=0x3c,period=2000003" },
+ { "cpu_clk_unhalted.thread_any", "event=0x3c,any=1,period=2000003" },
{ NULL, NULL},
};
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] perf annotate: Improve handling of corrupted ~/.debug
2019-09-27 23:35 [PATCH 1/3] perf script brstackinsn: Fix recovery from LBR/binary mismatch Andi Kleen
2019-09-27 23:35 ` [PATCH 2/3] perf jevents: Fix period for Intel fixed counters Andi Kleen
@ 2019-09-27 23:35 ` Andi Kleen
2019-09-30 10:50 ` [PATCH 1/3] perf script brstackinsn: Fix recovery from LBR/binary mismatch Arnaldo Carvalho de Melo
2019-10-07 14:49 ` [tip: perf/urgent] " tip-bot2 for Andi Kleen
3 siblings, 0 replies; 7+ messages in thread
From: Andi Kleen @ 2019-09-27 23:35 UTC (permalink / raw)
To: acme; +Cc: jolsa, linux-kernel, Andi Kleen
From: Andi Kleen <ak@linux.intel.com>
Sometimes ~/.debug can get corrupted and contain files that still
have symbol tables, but which objdump cannot handle. Add a fallback
to read the "original" file in such a case. This might be wrong
too if it's different, but in many cases when profiling
on the same host it will work.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
---
tools/perf/util/annotate.c | 27 +++++++++++++++++++++++----
1 file changed, 23 insertions(+), 4 deletions(-)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 1748f528b6e9..cff5f36786fa 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -1638,19 +1638,22 @@ int symbol__strerror_disassemble(struct symbol *sym __maybe_unused, struct map *
return 0;
}
-static int dso__disassemble_filename(struct dso *dso, char *filename, size_t filename_size)
+static int dso__disassemble_filename(struct dso *dso, char *filename, size_t filename_size,
+ bool *build_id)
{
char linkname[PATH_MAX];
char *build_id_filename;
char *build_id_path = NULL;
char *pos;
+ *build_id = false;
if (dso->symtab_type == DSO_BINARY_TYPE__KALLSYMS &&
!dso__is_kcore(dso))
return SYMBOL_ANNOTATE_ERRNO__NO_VMLINUX;
build_id_filename = dso__build_id_filename(dso, NULL, 0, false);
if (build_id_filename) {
+ *build_id = true;
__symbol__join_symfs(filename, filename_size, build_id_filename);
free(build_id_filename);
} else {
@@ -1854,11 +1857,14 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
int lineno = 0;
int nline;
pid_t pid;
- int err = dso__disassemble_filename(dso, symfs_filename, sizeof(symfs_filename));
+ bool build_id;
+ int err = dso__disassemble_filename(dso, symfs_filename, sizeof(symfs_filename),
+ &build_id);
if (err)
return err;
+again:
pr_debug("%s: filename=%s, sym=%s, start=%#" PRIx64 ", end=%#" PRIx64 "\n", __func__,
symfs_filename, sym->name, map->unmap_ip(map, sym->start),
map->unmap_ip(map, sym->end));
@@ -1955,8 +1961,21 @@ static int symbol__disassemble(struct symbol *sym, struct annotate_args *args)
nline++;
}
- if (nline == 0)
- pr_err("No output from %s\n", command);
+ if (nline == 0) {
+ pr_err("No objdump output for %s. Corrupted file?\n", symfs_filename);
+ if (build_id) {
+ /*
+ * It could be that the buildid file is corrupted.
+ * Try again with the "true" file. This might be wrong
+ * too, but it's better than nothing.
+ * We could move the build id file here?
+ */
+ __symbol__join_symfs(symfs_filename, sizeof(symfs_filename),
+ dso->long_name);
+ build_id = false;
+ goto again;
+ }
+ }
/*
* kallsyms does not have symbol sizes so there may a nop at the end.
--
2.21.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] perf script brstackinsn: Fix recovery from LBR/binary mismatch
2019-09-27 23:35 [PATCH 1/3] perf script brstackinsn: Fix recovery from LBR/binary mismatch Andi Kleen
2019-09-27 23:35 ` [PATCH 2/3] perf jevents: Fix period for Intel fixed counters Andi Kleen
2019-09-27 23:35 ` [PATCH 3/3] perf annotate: Improve handling of corrupted ~/.debug Andi Kleen
@ 2019-09-30 10:50 ` Arnaldo Carvalho de Melo
2019-10-07 14:49 ` [tip: perf/urgent] " tip-bot2 for Andi Kleen
3 siblings, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-09-30 10:50 UTC (permalink / raw)
To: Andi Kleen; +Cc: jolsa, linux-kernel, Andi Kleen
Em Fri, Sep 27, 2019 at 04:35:44PM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
>
> When the LBR data and the instructions in a binary do not match the
> loop printing instructions could get confused and print a long
> stream of bogus <bad> instructions.
>
> The problem was that if the instruction decoder cannot decode an
> instruction it ilen wasn't initialized, so the loop going through
> the basic block would continue with the previous value.
>
> Harden the code to avoid such problems:
> - Make sure ilen is always freshly initialized and is 0 for bad
> instructions.
> - Do not overrun the code buffer while printing instructions
> - Print a warning message if the final jump is not on an
> instruction boundary.
Thanks, applied.
- Arnaldo
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
> tools/perf/builtin-script.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
> index e079b34201f2..32b17d51c982 100644
> --- a/tools/perf/builtin-script.c
> +++ b/tools/perf/builtin-script.c
> @@ -1061,7 +1061,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
> continue;
>
> insn = 0;
> - for (off = 0;; off += ilen) {
> + for (off = 0; off < (unsigned)len; off += ilen) {
> uint64_t ip = start + off;
>
> printed += ip__fprintf_sym(ip, thread, x.cpumode, x.cpu, &lastsym, attr, fp);
> @@ -1072,6 +1072,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
> printed += print_srccode(thread, x.cpumode, ip);
> break;
> } else {
> + ilen = 0;
> printed += fprintf(fp, "\t%016" PRIx64 "\t%s\n", ip,
> dump_insn(&x, ip, buffer + off, len - off, &ilen));
> if (ilen == 0)
> @@ -1081,6 +1082,8 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
> insn++;
> }
> }
> + if (off != (unsigned)len)
> + printed += fprintf(fp, "\tmismatch of LBR data and executable\n");
> }
>
> /*
> @@ -1121,6 +1124,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
> goto out;
> }
> for (off = 0; off <= end - start; off += ilen) {
> + ilen = 0;
> printed += fprintf(fp, "\t%016" PRIx64 "\t%s\n", start + off,
> dump_insn(&x, start + off, buffer + off, len - off, &ilen));
> if (ilen == 0)
> --
> 2.21.0
--
- Arnaldo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] perf jevents: Fix period for Intel fixed counters
2019-09-27 23:35 ` [PATCH 2/3] perf jevents: Fix period for Intel fixed counters Andi Kleen
@ 2019-09-30 10:50 ` Arnaldo Carvalho de Melo
2019-10-07 14:49 ` [tip: perf/urgent] " tip-bot2 for Andi Kleen
1 sibling, 0 replies; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-09-30 10:50 UTC (permalink / raw)
To: Andi Kleen; +Cc: jolsa, linux-kernel, Andi Kleen
Em Fri, Sep 27, 2019 at 04:35:45PM -0700, Andi Kleen escreveu:
> From: Andi Kleen <ak@linux.intel.com>
>
> The Intel fixed counters use a special table to override the JSON
> information. During this override the period information from
> the JSON file got dropped, which results in inst_retired.any
> and similar running with frequency mode instead of a period.
> Just specify the expected period in the table.
Thanks, applied.
- Arnaldo
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
> tools/perf/pmu-events/jevents.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index d413761621b0..fa85e33762f7 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -449,12 +449,12 @@ static struct fixed {
> const char *name;
> const char *event;
> } fixed[] = {
> - { "inst_retired.any", "event=0xc0" },
> - { "inst_retired.any_p", "event=0xc0" },
> - { "cpu_clk_unhalted.ref", "event=0x0,umask=0x03" },
> - { "cpu_clk_unhalted.thread", "event=0x3c" },
> - { "cpu_clk_unhalted.core", "event=0x3c" },
> - { "cpu_clk_unhalted.thread_any", "event=0x3c,any=1" },
> + { "inst_retired.any", "event=0xc0,period=2000003" },
> + { "inst_retired.any_p", "event=0xc0,period=2000003" },
> + { "cpu_clk_unhalted.ref", "event=0x0,umask=0x03,period=2000003" },
> + { "cpu_clk_unhalted.thread", "event=0x3c,period=2000003" },
> + { "cpu_clk_unhalted.core", "event=0x3c,period=2000003" },
> + { "cpu_clk_unhalted.thread_any", "event=0x3c,any=1,period=2000003" },
> { NULL, NULL},
> };
>
> --
> 2.21.0
--
- Arnaldo
^ permalink raw reply [flat|nested] 7+ messages in thread
* [tip: perf/urgent] perf jevents: Fix period for Intel fixed counters
2019-09-27 23:35 ` [PATCH 2/3] perf jevents: Fix period for Intel fixed counters Andi Kleen
2019-09-30 10:50 ` Arnaldo Carvalho de Melo
@ 2019-10-07 14:49 ` tip-bot2 for Andi Kleen
1 sibling, 0 replies; 7+ messages in thread
From: tip-bot2 for Andi Kleen @ 2019-10-07 14:49 UTC (permalink / raw)
To: linux-tip-commits
Cc: Andi Kleen, Jiri Olsa, Arnaldo Carvalho de Melo, Ingo Molnar,
Borislav Petkov, linux-kernel
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: 6bdfd9f118bd59cf0f85d3bf4b72b586adea17c1
Gitweb: https://git.kernel.org/tip/6bdfd9f118bd59cf0f85d3bf4b72b586adea17c1
Author: Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 27 Sep 2019 16:35:45 -07:00
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Mon, 30 Sep 2019 17:29:53 -03:00
perf jevents: Fix period for Intel fixed counters
The Intel fixed counters use a special table to override the JSON
information.
During this override the period information from the JSON file got
dropped, which results in inst_retired.any and similar running with
frequency mode instead of a period.
Just specify the expected period in the table.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Link: http://lore.kernel.org/lkml/20190927233546.11533-2-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/pmu-events/jevents.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 9e37287..e283726 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -450,12 +450,12 @@ static struct fixed {
const char *name;
const char *event;
} fixed[] = {
- { "inst_retired.any", "event=0xc0" },
- { "inst_retired.any_p", "event=0xc0" },
- { "cpu_clk_unhalted.ref", "event=0x0,umask=0x03" },
- { "cpu_clk_unhalted.thread", "event=0x3c" },
- { "cpu_clk_unhalted.core", "event=0x3c" },
- { "cpu_clk_unhalted.thread_any", "event=0x3c,any=1" },
+ { "inst_retired.any", "event=0xc0,period=2000003" },
+ { "inst_retired.any_p", "event=0xc0,period=2000003" },
+ { "cpu_clk_unhalted.ref", "event=0x0,umask=0x03,period=2000003" },
+ { "cpu_clk_unhalted.thread", "event=0x3c,period=2000003" },
+ { "cpu_clk_unhalted.core", "event=0x3c,period=2000003" },
+ { "cpu_clk_unhalted.thread_any", "event=0x3c,any=1,period=2000003" },
{ NULL, NULL},
};
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [tip: perf/urgent] perf script brstackinsn: Fix recovery from LBR/binary mismatch
2019-09-27 23:35 [PATCH 1/3] perf script brstackinsn: Fix recovery from LBR/binary mismatch Andi Kleen
` (2 preceding siblings ...)
2019-09-30 10:50 ` [PATCH 1/3] perf script brstackinsn: Fix recovery from LBR/binary mismatch Arnaldo Carvalho de Melo
@ 2019-10-07 14:49 ` tip-bot2 for Andi Kleen
3 siblings, 0 replies; 7+ messages in thread
From: tip-bot2 for Andi Kleen @ 2019-10-07 14:49 UTC (permalink / raw)
To: linux-tip-commits
Cc: Andi Kleen, Jiri Olsa, Arnaldo Carvalho de Melo, Ingo Molnar,
Borislav Petkov, linux-kernel
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: e98df280bc2a499fd41d7f9e2d6733884de69902
Gitweb: https://git.kernel.org/tip/e98df280bc2a499fd41d7f9e2d6733884de69902
Author: Andi Kleen <ak@linux.intel.com>
AuthorDate: Fri, 27 Sep 2019 16:35:44 -07:00
Committer: Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Mon, 30 Sep 2019 17:29:52 -03:00
perf script brstackinsn: Fix recovery from LBR/binary mismatch
When the LBR data and the instructions in a binary do not match the loop
printing instructions could get confused and print a long stream of
bogus <bad> instructions.
The problem was that if the instruction decoder cannot decode an
instruction it ilen wasn't initialized, so the loop going through the
basic block would continue with the previous value.
Harden the code to avoid such problems:
- Make sure ilen is always freshly initialized and is 0 for bad
instructions.
- Do not overrun the code buffer while printing instructions
- Print a warning message if the final jump is not on an instruction
boundary.
Signed-off-by: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Link: http://lore.kernel.org/lkml/20190927233546.11533-1-andi@firstfloor.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/builtin-script.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 286fc70..67be8d3 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1063,7 +1063,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
continue;
insn = 0;
- for (off = 0;; off += ilen) {
+ for (off = 0; off < (unsigned)len; off += ilen) {
uint64_t ip = start + off;
printed += ip__fprintf_sym(ip, thread, x.cpumode, x.cpu, &lastsym, attr, fp);
@@ -1074,6 +1074,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
printed += print_srccode(thread, x.cpumode, ip);
break;
} else {
+ ilen = 0;
printed += fprintf(fp, "\t%016" PRIx64 "\t%s\n", ip,
dump_insn(&x, ip, buffer + off, len - off, &ilen));
if (ilen == 0)
@@ -1083,6 +1084,8 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
insn++;
}
}
+ if (off != (unsigned)len)
+ printed += fprintf(fp, "\tmismatch of LBR data and executable\n");
}
/*
@@ -1123,6 +1126,7 @@ static int perf_sample__fprintf_brstackinsn(struct perf_sample *sample,
goto out;
}
for (off = 0; off <= end - start; off += ilen) {
+ ilen = 0;
printed += fprintf(fp, "\t%016" PRIx64 "\t%s\n", start + off,
dump_insn(&x, start + off, buffer + off, len - off, &ilen));
if (ilen == 0)
^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-07 14:52 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-27 23:35 [PATCH 1/3] perf script brstackinsn: Fix recovery from LBR/binary mismatch Andi Kleen
2019-09-27 23:35 ` [PATCH 2/3] perf jevents: Fix period for Intel fixed counters Andi Kleen
2019-09-30 10:50 ` Arnaldo Carvalho de Melo
2019-10-07 14:49 ` [tip: perf/urgent] " tip-bot2 for Andi Kleen
2019-09-27 23:35 ` [PATCH 3/3] perf annotate: Improve handling of corrupted ~/.debug Andi Kleen
2019-09-30 10:50 ` [PATCH 1/3] perf script brstackinsn: Fix recovery from LBR/binary mismatch Arnaldo Carvalho de Melo
2019-10-07 14:49 ` [tip: perf/urgent] " tip-bot2 for Andi Kleen
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).