All of 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 related	[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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.