Messages in this thread Patch in this message | | | Subject | Re: [PATCH v5 6/9] perf, tools, script: Make itrace script default to all calls | From | Adrian Hunter <> | Date | Tue, 18 Sep 2018 16:24:55 +0300 |
| |
On 18/09/18 15:32, Andi Kleen wrote: > From: Andi Kleen <ak@linux.intel.com> > > By default perf script for itrace outputs sampled instructions or > branches. In my experience this is confusing to users because it's > hard to correlate with real program behavior. The sampling makes > sense for tools like report that actually sample to reduce > the run time, but run time is normally not a problem for perf script. > It's better to give an accurate representation of the program flow. > > Default perf script to output all calls for itrace. That's a much saner > default. The old behavior can be still requested with > perf script --itrace=ibxwpe100000 > > v2: Fix ETM build failure > v3: Really fix ETM build failure (Kim Phillips) > Signed-off-by: Andi Kleen <ak@linux.intel.com> > --- > tools/perf/Documentation/itrace.txt | 7 ++++--- > tools/perf/builtin-script.c | 5 ++++- > tools/perf/util/auxtrace.c | 17 ++++++++++++----- > tools/perf/util/auxtrace.h | 5 ++++- > tools/perf/util/cs-etm.c | 3 ++- > tools/perf/util/intel-bts.c | 3 ++- > tools/perf/util/intel-pt.c | 3 ++- > 7 files changed, 30 insertions(+), 13 deletions(-) > > diff --git a/tools/perf/Documentation/itrace.txt b/tools/perf/Documentation/itrace.txt > index a3abe04c779d..c2182cbabde3 100644 > --- a/tools/perf/Documentation/itrace.txt > +++ b/tools/perf/Documentation/itrace.txt > @@ -11,10 +11,11 @@ > l synthesize last branch entries (use with i or x) > s skip initial number of events > > - The default is all events i.e. the same as --itrace=ibxwpe > + The default is all events i.e. the same as --itrace=ibxwpe, > + except for perf script where it is --itrace=ce > > - In addition, the period (default 100000) for instructions events > - can be specified in units of: > + In addition, the period (default 100000, except for perf script where it is 1) > + for instructions events can be specified in units of: > > i instructions > t ticks > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c > index 57df6f7cff28..d3a463766e2d 100644 > --- a/tools/perf/builtin-script.c > +++ b/tools/perf/builtin-script.c > @@ -3112,7 +3112,10 @@ int cmd_script(int argc, const char **argv) > char *rec_script_path = NULL; > char *rep_script_path = NULL; > struct perf_session *session; > - struct itrace_synth_opts itrace_synth_opts = { .set = false, }; > + struct itrace_synth_opts itrace_synth_opts = { > + .set = false, > + .default_no_sample = true, > + }; > char *script_path = NULL; > const char **__argv; > int i, j, err = 0; > diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c > index db1511359c5e..2363a41f0e81 100644 > --- a/tools/perf/util/auxtrace.c > +++ b/tools/perf/util/auxtrace.c > @@ -964,16 +964,23 @@ s64 perf_event__process_auxtrace(struct perf_tool *tool, > #define PERF_ITRACE_DEFAULT_LAST_BRANCH_SZ 64 > #define PERF_ITRACE_MAX_LAST_BRANCH_SZ 1024 > > -void itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts) > +void itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts, > + bool no_sample)
Rather than pass no_sample, what about passing the whole of the tool's options i.e.
diff --git a/tools/perf/util/auxtrace.c b/tools/perf/util/auxtrace.c index db1511359c5e..d4db61626a5f 100644 --- a/tools/perf/util/auxtrace.c +++ b/tools/perf/util/auxtrace.c @@ -964,8 +964,14 @@ s64 perf_event__process_auxtrace(struct perf_tool *tool, #define PERF_ITRACE_DEFAULT_LAST_BRANCH_SZ 64 #define PERF_ITRACE_MAX_LAST_BRANCH_SZ 1024 -void itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts) +bool itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts, + const struct itrace_synth_opts *tool_synth_opts) { + if (tool_synth_opts && tool_synth_opts->set) { + *synth_opts = *tool_synth_opts; + return false; + } + synth_opts->instructions = true; synth_opts->branches = true; synth_opts->transactions = true; @@ -977,6 +983,8 @@ void itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts) synth_opts->callchain_sz = PERF_ITRACE_DEFAULT_CALLCHAIN_SZ; synth_opts->last_branch_sz = PERF_ITRACE_DEFAULT_LAST_BRANCH_SZ; synth_opts->initial_skip = 0; + + return true; } /* @@ -1001,7 +1009,7 @@ int itrace_parse_synth_opts(const struct option *opt, const char *str, } if (!str) { - itrace_synth_opts__set_default(synth_opts); + itrace_synth_opts__set_default(synth_opts, NULL); return 0; } diff --git a/tools/perf/util/auxtrace.h b/tools/perf/util/auxtrace.h index a516a891d7ea..172c92a8c596 100644 --- a/tools/perf/util/auxtrace.h +++ b/tools/perf/util/auxtrace.h @@ -528,7 +528,8 @@ int perf_event__process_auxtrace_error(struct perf_tool *tool, struct perf_session *session); int itrace_parse_synth_opts(const struct option *opt, const char *str, int unset); -void itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts); +bool itrace_synth_opts__set_default(struct itrace_synth_opts *synth_opts, + const struct itrace_synth_opts *tool_synth_opts); size_t perf_event__fprintf_auxtrace_error(union perf_event *event, FILE *fp); void perf_session__auxtrace_error_inc(struct perf_session *session, diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c index 2ae640257fdb..45b9a2b9f473 100644 --- a/tools/perf/util/cs-etm.c +++ b/tools/perf/util/cs-etm.c @@ -1429,12 +1429,9 @@ int cs_etm__process_auxtrace_info(union perf_event *event, return 0; } - if (session->itrace_synth_opts && session->itrace_synth_opts->set) { - etm->synth_opts = *session->itrace_synth_opts; - } else { - itrace_synth_opts__set_default(&etm->synth_opts); + if (itrace_synth_opts__set_default(&etm->synth_opts, + session->itrace_synth_opts)) etm->synth_opts.callchain = false; - } err = cs_etm__synth_events(etm, session); if (err) diff --git a/tools/perf/util/intel-bts.c b/tools/perf/util/intel-bts.c index 7f0c83b6332b..2d96f4743789 100644 --- a/tools/perf/util/intel-bts.c +++ b/tools/perf/util/intel-bts.c @@ -907,12 +907,9 @@ int intel_bts_process_auxtrace_info(union perf_event *event, if (dump_trace) return 0; - if (session->itrace_synth_opts && session->itrace_synth_opts->set) { - bts->synth_opts = *session->itrace_synth_opts; - } else { - itrace_synth_opts__set_default(&bts->synth_opts); - if (session->itrace_synth_opts) - bts->synth_opts.thread_stack = + if (itrace_synth_opts__set_default(&bts->synth_opts, + session->itrace_synth_opts)) { + bts->synth_opts.thread_stack = session->itrace_synth_opts->thread_stack; } diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c index aec68908d604..b6ac8ac2f4c3 100644 --- a/tools/perf/util/intel-pt.c +++ b/tools/perf/util/intel-pt.c @@ -2551,10 +2551,8 @@ int intel_pt_process_auxtrace_info(union perf_event *event, goto err_delete_thread; } - if (session->itrace_synth_opts && session->itrace_synth_opts->set) { - pt->synth_opts = *session->itrace_synth_opts; - } else { - itrace_synth_opts__set_default(&pt->synth_opts); + if (itrace_synth_opts__set_default(&pt->synth_opts, + session->itrace_synth_opts)) { if (use_browser != -1) { pt->synth_opts.branches = false; pt->synth_opts.callchain = true;
| |