Messages in this thread | | | Subject | Re: [PATCH] perf script: Fix ip display when type != attr->type | From | "Liang, Kan" <> | Date | Mon, 13 Sep 2021 11:13:56 -0400 |
| |
On 9/11/2021 9:30 AM, Adrian Hunter wrote: > set_print_ip_opts() was not being called when type != attr->type > because there is not a one-to-one relationship between output types > and attr->type. That resulted in ip not printing. > > The attr_type() function is removed, and the match of attr->type to > output type is corrected. > > Example on ADL using taskset to select an atom cpu: > > # perf record -e cpu_atom/cpu-cycles/ taskset 0x1000 uname > Linux > [ perf record: Woken up 1 times to write data ] > [ perf record: Captured and wrote 0.003 MB perf.data (7 samples) ] > > Before: > > # perf script | head > taskset 428 [-01] 10394.179041: 1 cpu_atom/cpu-cycles/: > taskset 428 [-01] 10394.179043: 1 cpu_atom/cpu-cycles/: > taskset 428 [-01] 10394.179044: 11 cpu_atom/cpu-cycles/: > taskset 428 [-01] 10394.179045: 407 cpu_atom/cpu-cycles/: > taskset 428 [-01] 10394.179046: 16789 cpu_atom/cpu-cycles/: > taskset 428 [-01] 10394.179052: 676300 cpu_atom/cpu-cycles/: > uname 428 [-01] 10394.179278: 4079859 cpu_atom/cpu-cycles/: > > After: > > # perf script | head > taskset 428 10394.179041: 1 cpu_atom/cpu-cycles/: ffffffff95a0bb97 __intel_pmu_enable_all.constprop.48+0x47 ([kernel.kallsyms]) > taskset 428 10394.179043: 1 cpu_atom/cpu-cycles/: ffffffff95a0bb97 __intel_pmu_enable_all.constprop.48+0x47 ([kernel.kallsyms]) > taskset 428 10394.179044: 11 cpu_atom/cpu-cycles/: ffffffff95a0bb97 __intel_pmu_enable_all.constprop.48+0x47 ([kernel.kallsyms]) > taskset 428 10394.179045: 407 cpu_atom/cpu-cycles/: ffffffff95a0bb97 __intel_pmu_enable_all.constprop.48+0x47 ([kernel.kallsyms]) > taskset 428 10394.179046: 16789 cpu_atom/cpu-cycles/: ffffffff95a0bb97 __intel_pmu_enable_all.constprop.48+0x47 ([kernel.kallsyms]) > taskset 428 10394.179052: 676300 cpu_atom/cpu-cycles/: 7f829ef73800 cfree+0x0 (/lib/libc-2.32.so) > uname 428 10394.179278: 4079859 cpu_atom/cpu-cycles/: ffffffff95bae912 vma_interval_tree_remove+0x1f2 ([kernel.kallsyms]) > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > --- > tools/perf/builtin-script.c | 24 +++++++++++++----------- > 1 file changed, 13 insertions(+), 11 deletions(-) > > diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c > index 0e824f7d8b19..6211d0b84b7a 100644 > --- a/tools/perf/builtin-script.c > +++ b/tools/perf/builtin-script.c > @@ -368,16 +368,6 @@ static inline int output_type(unsigned int type) > return OUTPUT_TYPE_OTHER; > } > > -static inline unsigned int attr_type(unsigned int type) > -{ > - switch (type) { > - case OUTPUT_TYPE_SYNTH: > - return PERF_TYPE_SYNTH; > - default: > - return type; > - } > -} > - > static bool output_set_by_user(void) > { > int j; > @@ -556,6 +546,18 @@ static void set_print_ip_opts(struct perf_event_attr *attr) > output[type].print_ip_opts |= EVSEL__PRINT_SRCLINE; > } > > +static struct evsel *find_first_output_type(struct evlist *evlist, > + unsigned int type) > +{ > + struct evsel *evsel; > + > + evlist__for_each_entry(evlist, evsel) { > + if (output_type(evsel->core.attr.type) == (int)type) > + return evsel; > + } > + return NULL; > +} > + > /* > * verify all user requested events exist and the samples > * have the expected data > @@ -567,7 +569,7 @@ static int perf_session__check_output_opt(struct perf_session *session) > struct evsel *evsel; > > for (j = 0; j < OUTPUT_TYPE_MAX; ++j) { > - evsel = perf_session__find_first_evtype(session, attr_type(j));
I think the only consumer of perf_session__find_first_evtype() will only be in session.c. Seems we can further clean up the function and make it static. Other than that, the patch looks good to me.
Reviewed-by: Kan Liang <kan.liang@linux.intel.com>
Thanks, Kan > + evsel = find_first_output_type(session->evlist, j); > > /* > * even if fields is set to 0 (ie., show nothing) event must >
| |