2024-03-15 07:23:41

by Adrian Hunter

Subject: [PATCH 1/2] perf script: Show also errors for --insn-trace option

The trace could be misleading if trace errors are not taken into
account, so display them also by adding the itrace "e" option.

Note --call-trace and --call-ret-trace already add the itrace "e"
option.

Fixes: b585ebdb5912 ("perf script: Add --insn-trace for instruction decoding")
Cc: [email protected]
Signed-off-by: Adrian Hunter <[email protected]>
---
tools/perf/builtin-script.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 37088cc0ff1b..2e7148d667bd 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -3806,7 +3806,7 @@ static int parse_insn_trace(const struct option *opt __maybe_unused,
if (ret < 0)
return ret;

- itrace_parse_synth_opts(opt, "i0ns", 0);
+ itrace_parse_synth_opts(opt, "i0nse", 0);
symbol_conf.nanosecs = true;
return 0;
}
--
2.34.1

2024-03-15 07:23:51

by Adrian Hunter

Subject: [PATCH 2/2] perf auxtrace: Fix multiple use of --itrace option

If the --itrace option is used more than once, the options are
combined, but "i" and "y" (sub-)options can be corrupted because
itrace_do_parse_synth_opts() incorrectly overwrites the period type and
period with default values.

For example, with:

--itrace=i0ns --itrace=e

The processing of "--itrace=e", resets the "i" period from 0 nanoseconds
to the default 100 microseconds.

Fix by performing the default setting of period type and period only if
"i" or "y" are present in the currently processed --itrace value.

Fixes: f6986c95af84 ("perf session: Add instruction tracing options")
Cc: [email protected]
Signed-off-by: Adrian Hunter <[email protected]>
---
tools/perf/util/auxtrace.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c
index 3684e6009b63..ef314a5797e3 100644
--- a/tools/perf/util/auxtrace.c
+++ b/tools/perf/util/auxtrace.c
@@ -1466,6 +1466,7 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
char *endptr;
bool period_type_set = false;
bool period_set = false;
+ bool iy = false;

synth_opts->set = true;

@@ -1484,6 +1485,7 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
switch (*p++) {
case 'i':
case 'y':
+ iy = true;
if (p[-1] == 'y')
synth_opts->cycles = true;
else
@@ -1649,7 +1651,7 @@ int itrace_do_parse_synth_opts(struct itrace_synth_opts *synth_opts,
}
}
out:
- if (synth_opts->instructions || synth_opts->cycles) {
+ if (iy) {
if (!period_type_set)
synth_opts->period_type =
PERF_ITRACE_DEFAULT_PERIOD_TYPE;
--
2.34.1

2024-03-15 19:11:56

by Andi Kleen

Subject: Re: [PATCH 1/2] perf script: Show also errors for --insn-trace option

Reviewed-by: Andi Kleen <[email protected]>

2024-03-18 21:20:33

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH 1/2] perf script: Show also errors for --insn-trace option

On Fri, Mar 15, 2024 at 09:13:33AM +0200, Adrian Hunter wrote:
> The trace could be misleading if trace errors are not taken into
> account, so display them also by adding the itrace "e" option.
>
> Note --call-trace and --call-ret-trace already add the itrace "e"
> option.
>
> Fixes: b585ebdb5912 ("perf script: Add --insn-trace for instruction decoding")
> Cc: [email protected]

Thanks, applied both patches to perf-tools-next.

- Arnaldo