linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] tools/perf/metricgroup: Fix printing event names of metric group with multiple events
@ 2019-11-20  8:40 Kajol Jain
  2019-12-04  6:55 ` Ravi Bangoria
  2019-12-17 11:31 ` [tip: perf/urgent] perf metricgroup: " tip-bot2 for Kajol Jain
  0 siblings, 2 replies; 6+ messages in thread
From: Kajol Jain @ 2019-11-20  8:40 UTC (permalink / raw)
  To: acme
  Cc: linux-kernel, linux-perf-users, kjain, Alexander Shishkin,
	Andi Kleen, Jiri Olsa, Kan Liang, Peter Zijlstra, Jin Yao,
	Madhavan Srinivasan, Anju T Sudhakar, Ravi Bangoria

Commit f01642e4912b ("perf metricgroup: Support multiple
events for metricgroup") introduced support for multiple events
in a metric group. But with the current upstream, metric events
names are not printed properly

In power9 platform:
command:# ./perf stat --metric-only -M translation -C 0 -I 1000 sleep 2
     1.000208486
     2.000368863
     2.001400558

Similarly in skylake platform:
command:./perf stat --metric-only -M Power -I 1000
     1.000579994
     2.002189493

With current upstream version, issue is with event name comparison
logic in find_evsel_group(). Current logic is to compare events
belonging to a metric group to the events in perf_evlist.
Since the break statement is missing in the loop used for comparison
between metric group and perf_evlist events, the loop continues to
execute even after getting a pattern match, and end up in discarding
the matches.
Incase of single metric event belongs to metric group, its working fine,
because in case of single event once it compare all events it reaches to
end of perf_evlist.

Example for single metric event in power9 platform
command:# ./perf stat --metric-only  -M branches_per_inst -I 1000 sleep 1
     1.000094653                  0.2
     1.001337059                  0.0

Patch fixes the issue by making sure once we found all events
belongs to that metric event matched in find_evsel_group(), we
successfully break from that loop by adding corresponding condition.

With this patch:
In power9 platform:

command:# ./perf stat --metric-only -M translation -C 0 -I 1000 sleep 2
result:#           time derat_4k_miss_rate_percent  derat_4k_miss_ratio
     derat_miss_ratio derat_64k_miss_rate_percent derat_64k_miss_ratio
         dslb_miss_rate_percent islb_miss_rate_percent
     1.000135672                         0.0                  0.3
                  1.0                          0.0                  0.2
                 0.0                     0.0
     2.000380617                         0.0                  0.0
                                              0.0                  0.0
                0.0                     0.0

command:# ./perf stat --metric-only -M Power -I 1000

Similarly in skylake platform:
result:#           time    Turbo_Utilization    C3_Core_Residency
            C6_Core_Residency    C7_Core_Residency     C2_Pkg_Residency
             C3_Pkg_Residency     C6_Pkg_Residency     C7_Pkg_Residency
     1.000563580                  0.3                  0.0
                  2.6                44.2                 21.9
                  0.0                  0.0                  0.0
     2.002235027                  0.4                  0.0
                  2.7           43.0                 20.7
                  0.0                  0.0               0.0

Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
---
 tools/perf/util/metricgroup.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index a7c0424dbda3..940a6e7a6854 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -103,8 +103,11 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 		if (!strcmp(ev->name, ids[i])) {
 			if (!metric_events[i])
 				metric_events[i] = ev;
+			i++;
+			if (i == idnum)
+				break;
 		} else {
-			if (++i == idnum) {
+			if (i + 1 == idnum) {
 				/* Discard the whole match and start again */
 				i = 0;
 				memset(metric_events, 0,
@@ -124,7 +127,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 		}
 	}
 
-	if (i != idnum - 1) {
+	if (i != idnum) {
 		/* Not whole match */
 		return NULL;
 	}
-- 
2.21.0


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

* Re: [PATCH] tools/perf/metricgroup: Fix printing event names of metric group with multiple events
  2019-11-20  8:40 [PATCH] tools/perf/metricgroup: Fix printing event names of metric group with multiple events Kajol Jain
@ 2019-12-04  6:55 ` Ravi Bangoria
  2019-12-11 13:45   ` Arnaldo Carvalho de Melo
  2019-12-17 11:31 ` [tip: perf/urgent] perf metricgroup: " tip-bot2 for Kajol Jain
  1 sibling, 1 reply; 6+ messages in thread
From: Ravi Bangoria @ 2019-12-04  6:55 UTC (permalink / raw)
  To: Kajol Jain, acme, Andi Kleen, Jin Yao
  Cc: linux-kernel, linux-perf-users, Alexander Shishkin, Jiri Olsa,
	Kan Liang, Peter Zijlstra, Madhavan Srinivasan, Anju T Sudhakar,
	Ravi Bangoria



On 11/20/19 2:10 PM, Kajol Jain wrote:
> Commit f01642e4912b ("perf metricgroup: Support multiple
> events for metricgroup") introduced support for multiple events
> in a metric group. But with the current upstream, metric events
> names are not printed properly
> 
> In power9 platform:
> command:# ./perf stat --metric-only -M translation -C 0 -I 1000 sleep 2
>       1.000208486
>       2.000368863
>       2.001400558
> 
> Similarly in skylake platform:
> command:./perf stat --metric-only -M Power -I 1000
>       1.000579994
>       2.002189493
> 
> With current upstream version, issue is with event name comparison
> logic in find_evsel_group(). Current logic is to compare events
> belonging to a metric group to the events in perf_evlist.
> Since the break statement is missing in the loop used for comparison
> between metric group and perf_evlist events, the loop continues to
> execute even after getting a pattern match, and end up in discarding
> the matches.
> Incase of single metric event belongs to metric group, its working fine,
> because in case of single event once it compare all events it reaches to
> end of perf_evlist.
> 
> Example for single metric event in power9 platform
> command:# ./perf stat --metric-only  -M branches_per_inst -I 1000 sleep 1
>       1.000094653                  0.2
>       1.001337059                  0.0
> 
> Patch fixes the issue by making sure once we found all events
> belongs to that metric event matched in find_evsel_group(), we
> successfully break from that loop by adding corresponding condition.
> 
> With this patch:
> In power9 platform:
> 
> command:# ./perf stat --metric-only -M translation -C 0 -I 1000 sleep 2
> result:#           time derat_4k_miss_rate_percent  derat_4k_miss_ratio
>       derat_miss_ratio derat_64k_miss_rate_percent derat_64k_miss_ratio
>           dslb_miss_rate_percent islb_miss_rate_percent
>       1.000135672                         0.0                  0.3
>                    1.0                          0.0                  0.2
>                   0.0                     0.0
>       2.000380617                         0.0                  0.0
>                                                0.0                  0.0
>                  0.0                     0.0
> 
> command:# ./perf stat --metric-only -M Power -I 1000
> 
> Similarly in skylake platform:
> result:#           time    Turbo_Utilization    C3_Core_Residency
>              C6_Core_Residency    C7_Core_Residency     C2_Pkg_Residency
>               C3_Pkg_Residency     C6_Pkg_Residency     C7_Pkg_Residency
>       1.000563580                  0.3                  0.0
>                    2.6                44.2                 21.9
>                    0.0                  0.0                  0.0
>       2.002235027                  0.4                  0.0
>                    2.7           43.0                 20.7
>                    0.0                  0.0               0.0
> 
> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Cc: Andi Kleen <ak@linux.intel.com>
> Cc: Jiri Olsa <jolsa@kernel.org>
> Cc: Kan Liang <kan.liang@linux.intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Jin Yao <yao.jin@linux.intel.com>
> Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>

Fixes: f01642e4912b ("perf metricgroup: Support multiple events for metricgroup")
Reviewed-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>

But while looking at the patch, I found that, commit f01642e4912b
has (again) screwed up logic for metric with overlapping events.

   $ sudo ./perf stat -M UPI,IPC sleep 1

    Performance counter stats for 'sleep 1':

            948,650      uops_retired.retire_slots
            866,182      inst_retired.any          #      0.7 IPC
            866,182      inst_retired.any
          1,175,671      cpu_clk_unhalted.thread

This also needs to be fixed.

Ravi


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

* Re: [PATCH] tools/perf/metricgroup: Fix printing event names of metric group with multiple events
  2019-12-04  6:55 ` Ravi Bangoria
@ 2019-12-11 13:45   ` Arnaldo Carvalho de Melo
  2019-12-11 20:38     ` Andi Kleen
  2019-12-12  5:54     ` kajoljain
  0 siblings, 2 replies; 6+ messages in thread
From: Arnaldo Carvalho de Melo @ 2019-12-11 13:45 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: Kajol Jain, Andi Kleen, Jin Yao, linux-kernel, linux-perf-users,
	Alexander Shishkin, Jiri Olsa, Kan Liang, Peter Zijlstra,
	Madhavan Srinivasan, Anju T Sudhakar

Em Wed, Dec 04, 2019 at 12:25:22PM +0530, Ravi Bangoria escreveu:
> 
> 
> On 11/20/19 2:10 PM, Kajol Jain wrote:
> > Commit f01642e4912b ("perf metricgroup: Support multiple
> > events for metricgroup") introduced support for multiple events
> > in a metric group. But with the current upstream, metric events
> > names are not printed properly
> > 
> > In power9 platform:
> > command:# ./perf stat --metric-only -M translation -C 0 -I 1000 sleep 2
> >       1.000208486
> >       2.000368863
> >       2.001400558
> > 
> > Similarly in skylake platform:
> > command:./perf stat --metric-only -M Power -I 1000
> >       1.000579994
> >       2.002189493
> > 
> > With current upstream version, issue is with event name comparison
> > logic in find_evsel_group(). Current logic is to compare events
> > belonging to a metric group to the events in perf_evlist.
> > Since the break statement is missing in the loop used for comparison
> > between metric group and perf_evlist events, the loop continues to
> > execute even after getting a pattern match, and end up in discarding
> > the matches.
> > Incase of single metric event belongs to metric group, its working fine,
> > because in case of single event once it compare all events it reaches to
> > end of perf_evlist.
> > 
> > Example for single metric event in power9 platform
> > command:# ./perf stat --metric-only  -M branches_per_inst -I 1000 sleep 1
> >       1.000094653                  0.2
> >       1.001337059                  0.0
> > 
> > Patch fixes the issue by making sure once we found all events
> > belongs to that metric event matched in find_evsel_group(), we
> > successfully break from that loop by adding corresponding condition.
> > 
> > With this patch:
> > In power9 platform:
> > 
> > command:# ./perf stat --metric-only -M translation -C 0 -I 1000 sleep 2
> > result:#           time derat_4k_miss_rate_percent  derat_4k_miss_ratio
> >       derat_miss_ratio derat_64k_miss_rate_percent derat_64k_miss_ratio
> >           dslb_miss_rate_percent islb_miss_rate_percent
> >       1.000135672                         0.0                  0.3
> >                    1.0                          0.0                  0.2
> >                   0.0                     0.0
> >       2.000380617                         0.0                  0.0
> >                                                0.0                  0.0
> >                  0.0                     0.0
> > 
> > command:# ./perf stat --metric-only -M Power -I 1000
> > 
> > Similarly in skylake platform:
> > result:#           time    Turbo_Utilization    C3_Core_Residency
> >              C6_Core_Residency    C7_Core_Residency     C2_Pkg_Residency
> >               C3_Pkg_Residency     C6_Pkg_Residency     C7_Pkg_Residency
> >       1.000563580                  0.3                  0.0
> >                    2.6                44.2                 21.9
> >                    0.0                  0.0                  0.0
> >       2.002235027                  0.4                  0.0
> >                    2.7           43.0                 20.7
> >                    0.0                  0.0               0.0
> > 
> > Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
> > Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> > Cc: Andi Kleen <ak@linux.intel.com>
> > Cc: Jiri Olsa <jolsa@kernel.org>
> > Cc: Kan Liang <kan.liang@linux.intel.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > Cc: Jin Yao <yao.jin@linux.intel.com>
> > Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
> > Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
> > Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> 
> Fixes: f01642e4912b ("perf metricgroup: Support multiple events for metricgroup")
> Reviewed-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>

Tested on a Skylake machine and applied.

> But while looking at the patch, I found that, commit f01642e4912b
> has (again) screwed up logic for metric with overlapping events.

Is someone looking into this?
 
>   $ sudo ./perf stat -M UPI,IPC sleep 1
> 
>    Performance counter stats for 'sleep 1':
> 
>            948,650      uops_retired.retire_slots
>            866,182      inst_retired.any          #      0.7 IPC
>            866,182      inst_retired.any
>          1,175,671      cpu_clk_unhalted.thread
> 
> This also needs to be fixed.
> 
> Ravi

-- 

- Arnaldo

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

* Re: [PATCH] tools/perf/metricgroup: Fix printing event names of metric group with multiple events
  2019-12-11 13:45   ` Arnaldo Carvalho de Melo
@ 2019-12-11 20:38     ` Andi Kleen
  2019-12-12  5:54     ` kajoljain
  1 sibling, 0 replies; 6+ messages in thread
From: Andi Kleen @ 2019-12-11 20:38 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ravi Bangoria, Kajol Jain, Jin Yao, linux-kernel,
	linux-perf-users, Alexander Shishkin, Jiri Olsa, Kan Liang,
	Peter Zijlstra, Madhavan Srinivasan, Anju T Sudhakar

> > But while looking at the patch, I found that, commit f01642e4912b
> > has (again) screwed up logic for metric with overlapping events.
> 
> Is someone looking into this?

Looks like we really need some proper regression tests for this.

-Andi

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

* Re: [PATCH] tools/perf/metricgroup: Fix printing event names of metric group with multiple events
  2019-12-11 13:45   ` Arnaldo Carvalho de Melo
  2019-12-11 20:38     ` Andi Kleen
@ 2019-12-12  5:54     ` kajoljain
  1 sibling, 0 replies; 6+ messages in thread
From: kajoljain @ 2019-12-12  5:54 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Ravi Bangoria
  Cc: Andi Kleen, Jin Yao, linux-kernel, linux-perf-users,
	Alexander Shishkin, Jiri Olsa, Kan Liang, Peter Zijlstra,
	Madhavan Srinivasan, Anju T Sudhakar


On 12/11/19 7:15 PM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Dec 04, 2019 at 12:25:22PM +0530, Ravi Bangoria escreveu:
>>
>> On 11/20/19 2:10 PM, Kajol Jain wrote:
>>> Commit f01642e4912b ("perf metricgroup: Support multiple
>>> events for metricgroup") introduced support for multiple events
>>> in a metric group. But with the current upstream, metric events
>>> names are not printed properly
>>>
>>> In power9 platform:
>>> command:# ./perf stat --metric-only -M translation -C 0 -I 1000 sleep 2
>>>        1.000208486
>>>        2.000368863
>>>        2.001400558
>>>
>>> Similarly in skylake platform:
>>> command:./perf stat --metric-only -M Power -I 1000
>>>        1.000579994
>>>        2.002189493
>>>
>>> With current upstream version, issue is with event name comparison
>>> logic in find_evsel_group(). Current logic is to compare events
>>> belonging to a metric group to the events in perf_evlist.
>>> Since the break statement is missing in the loop used for comparison
>>> between metric group and perf_evlist events, the loop continues to
>>> execute even after getting a pattern match, and end up in discarding
>>> the matches.
>>> Incase of single metric event belongs to metric group, its working fine,
>>> because in case of single event once it compare all events it reaches to
>>> end of perf_evlist.
>>>
>>> Example for single metric event in power9 platform
>>> command:# ./perf stat --metric-only  -M branches_per_inst -I 1000 sleep 1
>>>        1.000094653                  0.2
>>>        1.001337059                  0.0
>>>
>>> Patch fixes the issue by making sure once we found all events
>>> belongs to that metric event matched in find_evsel_group(), we
>>> successfully break from that loop by adding corresponding condition.
>>>
>>> With this patch:
>>> In power9 platform:
>>>
>>> command:# ./perf stat --metric-only -M translation -C 0 -I 1000 sleep 2
>>> result:#           time derat_4k_miss_rate_percent  derat_4k_miss_ratio
>>>        derat_miss_ratio derat_64k_miss_rate_percent derat_64k_miss_ratio
>>>            dslb_miss_rate_percent islb_miss_rate_percent
>>>        1.000135672                         0.0                  0.3
>>>                     1.0                          0.0                  0.2
>>>                    0.0                     0.0
>>>        2.000380617                         0.0                  0.0
>>>                                                 0.0                  0.0
>>>                   0.0                     0.0
>>>
>>> command:# ./perf stat --metric-only -M Power -I 1000
>>>
>>> Similarly in skylake platform:
>>> result:#           time    Turbo_Utilization    C3_Core_Residency
>>>               C6_Core_Residency    C7_Core_Residency     C2_Pkg_Residency
>>>                C3_Pkg_Residency     C6_Pkg_Residency     C7_Pkg_Residency
>>>        1.000563580                  0.3                  0.0
>>>                     2.6                44.2                 21.9
>>>                     0.0                  0.0                  0.0
>>>        2.002235027                  0.4                  0.0
>>>                     2.7           43.0                 20.7
>>>                     0.0                  0.0               0.0
>>>
>>> Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
>>> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
>>> Cc: Andi Kleen <ak@linux.intel.com>
>>> Cc: Jiri Olsa <jolsa@kernel.org>
>>> Cc: Kan Liang <kan.liang@linux.intel.com>
>>> Cc: Peter Zijlstra <peterz@infradead.org>
>>> Cc: Jin Yao <yao.jin@linux.intel.com>
>>> Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
>>> Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
>>> Cc: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
>> Fixes: f01642e4912b ("perf metricgroup: Support multiple events for metricgroup")
>> Reviewed-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> Tested on a Skylake machine and applied.
>
>> But while looking at the patch, I found that, commit f01642e4912b
>> has (again) screwed up logic for metric with overlapping events.
> Is someone looking into this?


Hi Arnaldo,

             I am looking  into this issue and will post the fix soon.

Kajol

>   
>>    $ sudo ./perf stat -M UPI,IPC sleep 1
>>
>>     Performance counter stats for 'sleep 1':
>>
>>             948,650      uops_retired.retire_slots
>>             866,182      inst_retired.any          #      0.7 IPC
>>             866,182      inst_retired.any
>>           1,175,671      cpu_clk_unhalted.thread
>>
>> This also needs to be fixed.
>>
>> Ravi


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

* [tip: perf/urgent] perf metricgroup: Fix printing event names of metric group with multiple events
  2019-11-20  8:40 [PATCH] tools/perf/metricgroup: Fix printing event names of metric group with multiple events Kajol Jain
  2019-12-04  6:55 ` Ravi Bangoria
@ 2019-12-17 11:31 ` tip-bot2 for Kajol Jain
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot2 for Kajol Jain @ 2019-12-17 11:31 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Kajol Jain, Ravi Bangoria, Arnaldo Carvalho de Melo,
	Alexander Shishkin, Andi Kleen, Anju T Sudhakar, Jin Yao,
	Jiri Olsa, Kan Liang, Madhavan Srinivasan, Peter Zijlstra, x86,
	LKML

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     eb573e746b9d4f0921dcb2449be3df41dae3caea
Gitweb:        https://git.kernel.org/tip/eb573e746b9d4f0921dcb2449be3df41dae3caea
Author:        Kajol Jain <kjain@linux.ibm.com>
AuthorDate:    Wed, 20 Nov 2019 14:10:59 +05:30
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Wed, 11 Dec 2019 12:28:14 -03:00

perf metricgroup: Fix printing event names of metric group with multiple events

Commit f01642e4912b ("perf metricgroup: Support multiple events for
metricgroup") introduced support for multiple events in a metric group.
But with the current upstream, metric events names are not printed
properly

In power9 platform:

command:# ./perf stat --metric-only -M translation -C 0 -I 1000 sleep 2
     1.000208486
     2.000368863
     2.001400558

Similarly in skylake platform:

command:./perf stat --metric-only -M Power -I 1000
     1.000579994
     2.002189493

With current upstream version, issue is with event name comparison logic
in find_evsel_group(). Current logic is to compare events belonging to a
metric group to the events in perf_evlist.  Since the break statement is
missing in the loop used for comparison between metric group and
perf_evlist events, the loop continues to execute even after getting a
pattern match, and end up in discarding the matches.

Incase of single metric event belongs to metric group, its working fine,
because in case of single event once it compare all events it reaches to
end of perf_evlist.

Example for single metric event in power9 platform:

command:# ./perf stat --metric-only  -M branches_per_inst -I 1000 sleep 1
     1.000094653                  0.2
     1.001337059                  0.0

This patch fixes the issue by making sure once we found all events
belongs to that metric event matched in find_evsel_group(), we
successfully break from that loop by adding corresponding condition.

With this patch:
In power9 platform:

command:# ./perf stat --metric-only -M translation -C 0 -I 1000 sleep 2
result:#
            time  derat_4k_miss_rate_percent  derat_4k_miss_ratio derat_miss_ratio derat_64k_miss_rate_percent  derat_64k_miss_ratio dslb_miss_rate_percent islb_miss_rate_percent
     1.000135672                         0.0                  0.3              1.0                         0.0                   0.2                    0.0                    0.0
     2.000380617                         0.0                  0.0              0.0                         0.0                   0.0                    0.0                    0.0

command:# ./perf stat --metric-only -M Power -I 1000

Similarly in skylake platform:
result:#
            time    Turbo_Utilization    C3_Core_Residency  C6_Core_Residency  C7_Core_Residency    C2_Pkg_Residency  C3_Pkg_Residency     C6_Pkg_Residency   C7_Pkg_Residency
     1.000563580                  0.3                  0.0                2.6               44.2                21.9               0.0                  0.0               0.0
     2.002235027                  0.4                  0.0                2.7               43.0                20.7               0.0                  0.0               0.0

Committer testing:

  Before:

  [root@seventh ~]# perf stat --metric-only -M Power -I 1000
  #           time
       1.000383223
       2.001168182
       3.001968545
       4.002741200
       5.003442022
  ^C     5.777687244

  [root@seventh ~]#

  After the patch:

  [root@seventh ~]# perf stat --metric-only -M Power -I 1000
  #           time    Turbo_Utilization    C3_Core_Residency    C6_Core_Residency    C7_Core_Residency     C2_Pkg_Residency     C3_Pkg_Residency     C6_Pkg_Residency     C7_Pkg_Residency
       1.000406577                  0.4                  0.1                  1.4                 97.0                  0.0                  0.0                  0.0                  0.0
       2.001481572                  0.3                  0.0                  0.6                 97.9                  0.0                  0.0                  0.0                  0.0
       3.002332585                  0.2                  0.0                  1.0                 97.5                  0.0                  0.0                  0.0                  0.0
       4.003196624                  0.2                  0.0                  0.3                 98.6                  0.0                  0.0                  0.0                  0.0
       5.004063851                  0.3                  0.0                  0.7                 97.7                  0.0                  0.0                  0.0                  0.0
  ^C     5.471260276                  0.2                  0.0                  0.5                 49.3                  0.0                  0.0                  0.0                  0.0

  [root@seventh ~]#
  [root@seventh ~]# dmesg | grep -i skylake
  [    0.187807] Performance Events: PEBS fmt3+, Skylake events, 32-deep LBR, full-width counters, Intel PMU driver.
  [root@seventh ~]#

Fixes: f01642e4912b ("perf metricgroup: Support multiple events for metricgroup")
Signed-off-by: Kajol Jain <kjain@linux.ibm.com>
Reviewed-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
Tested-by: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: Anju T Sudhakar <anju@linux.vnet.ibm.com>
Cc: Jin Yao <yao.jin@linux.intel.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Madhavan Srinivasan <maddy@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lore.kernel.org/lkml/20191120084059.24458-1-kjain@linux.ibm.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/metricgroup.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 6a4d350..02aee94 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -103,8 +103,11 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 		if (!strcmp(ev->name, ids[i])) {
 			if (!metric_events[i])
 				metric_events[i] = ev;
+			i++;
+			if (i == idnum)
+				break;
 		} else {
-			if (++i == idnum) {
+			if (i + 1 == idnum) {
 				/* Discard the whole match and start again */
 				i = 0;
 				memset(metric_events, 0,
@@ -124,7 +127,7 @@ static struct evsel *find_evsel_group(struct evlist *perf_evlist,
 		}
 	}
 
-	if (i != idnum - 1) {
+	if (i != idnum) {
 		/* Not whole match */
 		return NULL;
 	}

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

end of thread, other threads:[~2019-12-17 11:32 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-20  8:40 [PATCH] tools/perf/metricgroup: Fix printing event names of metric group with multiple events Kajol Jain
2019-12-04  6:55 ` Ravi Bangoria
2019-12-11 13:45   ` Arnaldo Carvalho de Melo
2019-12-11 20:38     ` Andi Kleen
2019-12-12  5:54     ` kajoljain
2019-12-17 11:31 ` [tip: perf/urgent] perf metricgroup: " tip-bot2 for Kajol Jain

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