LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] perf script: Fix ip display when type != attr->type
@ 2021-09-11 13:30 Adrian Hunter
  2021-09-13 15:13 ` Liang, Kan
  0 siblings, 1 reply; 3+ messages in thread
From: Adrian Hunter @ 2021-09-11 13:30 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, Jin Yao, Kan Liang, linux-kernel

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));
+		evsel = find_first_output_type(session->evlist, j);
 
 		/*
 		 * even if fields is set to 0 (ie., show nothing) event must
-- 
2.17.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf script: Fix ip display when type != attr->type
  2021-09-11 13:30 [PATCH] perf script: Fix ip display when type != attr->type Adrian Hunter
@ 2021-09-13 15:13 ` Liang, Kan
  2021-09-13 19:55   ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 3+ messages in thread
From: Liang, Kan @ 2021-09-13 15:13 UTC (permalink / raw)
  To: Adrian Hunter, Arnaldo Carvalho de Melo; +Cc: Jiri Olsa, Jin Yao, linux-kernel



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
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] perf script: Fix ip display when type != attr->type
  2021-09-13 15:13 ` Liang, Kan
@ 2021-09-13 19:55   ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 3+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-09-13 19:55 UTC (permalink / raw)
  To: Liang, Kan; +Cc: Adrian Hunter, Jiri Olsa, Jin Yao, linux-kernel

Em Mon, Sep 13, 2021 at 11:13:56AM -0400, Liang, Kan escreveu:
> 
> 
> 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, applied.

- Arnaldo


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-09-13 19:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-11 13:30 [PATCH] perf script: Fix ip display when type != attr->type Adrian Hunter
2021-09-13 15:13 ` Liang, Kan
2021-09-13 19:55   ` Arnaldo Carvalho de Melo

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).