perf stat: Only auto-merge events that are PMU aliases

PATCH In review Arnaldo Carvalho de Melo 5 participants 5 messages Aug 31, 2017 – Sep 5, 2017 perf events

lore

Share & tools

lore

Arnaldo Carvalho de Melo

author maintainer

2017-08-31 18:41 UTC #0 lore raw

PATCH 1/1 perf stat: Only auto-merge events that are PMU aliases

Hi Andi, Jiri,

Please check the patch below, would be glad to have your acks,

- Arnaldo

Peter reported that when he explicitely asked for multiple events with the same name on the command line it got coalesced into just one line, i.e.:

   # perf stat -e cycles -e cycles -e cycles usleep 1

Performance counter stats for 'usleep 1':

         3,269,652      cycles

       0.000884123 seconds time elapsed

And while there is the --no-merges option to disable that auto-merging, this is an blunt change in behaviour for such explicit request, so change the code so that this auto merging is done only when handling the multi PMU aliases with the same name that introduced this coalescing, restoring the previous behaviour for explicit case:

  # perf stat -e cycles -e cycles -e cycles usleep 1

Performance counter stats for 'usleep 1':

         1,472,837      cycles
         1,472,837      cycles
         1,472,837      cycles

       0.001764870 seconds time elapsed

Reported-by Peter Zijlstra <[email protected]>

Cc Adrian Hunter <[email protected]>

Cc Andi Kleen <[email protected]>

Cc Jiri Olsa <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Wang Nan <[email protected]>

Fixes 430daf2dc7af ("perf stat: Collapse identically named events")

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/builtin-stat.c      |  2 +-
 tools/perf/util/evsel.h        |  1 +
 tools/perf/util/parse-events.c | 24 ++++++++++++++++--------
 3 files changed, 18 insertions(+), 9 deletions(-)

tools/perf/builtin-stat.c +1 -1

index 866da7aa54bf..85e992d9215b 100644
@@ -1257,7 +1257,7 @@ static bool collect_data(struct perf_evsel *counter,
 	if (counter->merged_stat)
 		return false;
 	cb(counter, data, true);
-	if (!no_merge)
+	if (!no_merge && counter->auto_merge_stats)
 		collect_all_aliases(counter, cb, data);
 	return true;
 }

tools/perf/util/evsel.h +1 -0

index 351d3b2d8887..dd2c4b5112a5 100644
@@ -131,6 +131,7 @@ struct perf_evsel {
 	bool			cmdline_group_boundary;
 	struct list_head	config_terms;
 	int			bpf_fd;
+	bool			auto_merge_stats;
 	bool			merged_stat;
 	const char *		metric_expr;
 	const char *		metric_name;

tools/perf/util/parse-events.c +16 -8

index f44aeba51d1f..f6257fb4f08c 100644
@@ -310,7 +310,7 @@ static struct perf_evsel *
 __add_event(struct list_head *list, int *idx,
 	    struct perf_event_attr *attr,
 	    char *name, struct cpu_map *cpus,
-	    struct list_head *config_terms)
+	    struct list_head *config_terms, bool auto_merge_stats)
 {
 	struct perf_evsel *evsel;

@@ -324,6 +324,7 @@ __add_event(struct list_head *list, int *idx,
 	evsel->cpus        = cpu_map__get(cpus);
 	evsel->own_cpus    = cpu_map__get(cpus);
 	evsel->system_wide = !!cpus;
+	evsel->auto_merge_stats = auto_merge_stats;
 
 	if (name)
 		evsel->name = strdup(name);

@@ -339,7 +340,7 @@ static int add_event(struct list_head *list, int *idx,
 		     struct perf_event_attr *attr, char *name,
 		     struct list_head *config_terms)
 {
-	return __add_event(list, idx, attr, name, NULL, config_terms) ? 0 : -ENOMEM;
+	return __add_event(list, idx, attr, name, NULL, config_terms, false) ? 0 : -ENOMEM;
 }
 
 static int parse_aliases(char *str, const char *names[][PERF_EVSEL__MAX_ALIASES], int size)

@@ -1209,9 +1210,9 @@ int parse_events_add_numeric(struct parse_events_state *parse_state,
 			 get_config_name(head_config), &config_terms);
 }
 
-int parse_events_add_pmu(struct parse_events_state *parse_state,
+static int __parse_events_add_pmu(struct parse_events_state *parse_state,
 			 struct list_head *list, char *name,
-			 struct list_head *head_config)
+			 struct list_head *head_config, bool auto_merge_stats)
 {
 	struct perf_event_attr attr;
 	struct perf_pmu_info info;

@@ -1232,7 +1233,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 
 	if (!head_config) {
 		attr.type = pmu->type;
-		evsel = __add_event(list, &parse_state->idx, &attr, NULL, pmu->cpus, NULL);
+		evsel = __add_event(list, &parse_state->idx, &attr, NULL, pmu->cpus, NULL, auto_merge_stats);
 		return evsel ? 0 : -ENOMEM;
 	}

@@ -1254,7 +1255,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 
 	evsel = __add_event(list, &parse_state->idx, &attr,
 			    get_config_name(head_config), pmu->cpus,
-			    &config_terms);
+			    &config_terms, auto_merge_stats);
 	if (evsel) {
 		evsel->unit = info.unit;
 		evsel->scale = info.scale;

@@ -1267,6 +1268,13 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 	return evsel ? 0 : -ENOMEM;
 }
 
+int parse_events_add_pmu(struct parse_events_state *parse_state,
+			 struct list_head *list, char *name,
+			 struct list_head *head_config)
+{
+	return __parse_events_add_pmu(parse_state, list, name, head_config, false);
+}
+
 int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 			       char *str, struct list_head **listp)
 {

@@ -1296,8 +1304,8 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 					return -1;
 				list_add_tail(&term->list, head);
 
-				if (!parse_events_add_pmu(parse_state, list,
-						  pmu->name, head)) {
+				if (!__parse_events_add_pmu(parse_state, list,
+							    pmu->name, head, true)) {
 					pr_debug("%s -> %s/%s/\n", str,
 						 pmu->name, alias->str);
 					ok++;

Signature · 1 line

2.13.5

Andi Kleen

Acked-by on v1 1/1

2017-08-31 19:02 UTC #1 lore raw

Acked-by Andi Kleen <[email protected]>

Peter Zijlstra

maintainer

2017-09-01 11:20 UTC #2 lore raw

In reply to Arnaldo Carvalho de Melo

Quoted 38 lines · Arnaldo Carvalho de Melo

Hi Andi, Jiri,
Please check the patch below, would be glad to have your acks,
- Arnaldo
Peter reported that when he explicitely asked for multiple events with the same name on the command line it got coalesced into just one line, i.e.:
   # perf stat -e cycles -e cycles -e cycles usleep 1
   Performance counter stats for 'usleep 1':

         3,269,652      cycles

       0.000884123 seconds time elapsed
#
And while there is the --no-merges option to disable that auto-merging, this is an blunt change in behaviour for such explicit request, so change the code so that this auto merging is done only when handling the multi PMU aliases with the same name that introduced this coalescing, restoring the previous behaviour for explicit case:
  # perf stat -e cycles -e cycles -e cycles usleep 1
   Performance counter stats for 'usleep 1':

         1,472,837      cycles
         1,472,837      cycles
         1,472,837      cycles

       0.001764870 seconds time elapsed
#

Quoted 10 lines

Reported-by Peter Zijlstra <[email protected]>
Cc Adrian Hunter <[email protected]>
Cc Andi Kleen <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Wang Nan <[email protected]>
Fixes 430daf2dc7af ("perf stat: Collapse identically named events")
Link http://lkml.kernel.org/n/[email protected] open here
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

Thanks Arnaldo!

Jiri Olsa

Acked-by on v1 1/1

2017-09-01 16:12 UTC #3 lore raw

In reply to Arnaldo Carvalho de Melo

Quoted 10 lines · Arnaldo Carvalho de Melo

Hi Andi, Jiri,
Please check the patch below, would be glad to have your acks,
- Arnaldo
Peter reported that when he explicitely asked for multiple events with the same name on the command line it got coalesced into just one line, i.e.:

Quoted 7 lines

   # perf stat -e cycles -e cycles -e cycles usleep 1

   Performance counter stats for 'usleep 1':

         3,269,652      cycles

       0.000884123 seconds time elapsed

looks good

Acked-by Jiri Olsa <[email protected]>

thanks,
jirka

tip-bot for Arnaldo Carvalho de Melo

merge bot

2017-09-05 05:26 UTC #4 lore raw

In reply to Arnaldo Carvalho de Melo

perf stat: Only auto-merge events that are PMU aliases

Commit-ID:  63ce8449bc1081711eef1add68909e9bd758de62
Gitweb:     http://git.kernel.org/tip/63ce8449bc1081711eef1add68909e9bd758de62
Author:     Arnaldo Carvalho de Melo <[email protected]>
AuthorDate: Thu, 31 Aug 2017 15:32:18 -0300
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 1 Sep 2017 14:48:59 -0300

perf stat: Only auto-merge events that are PMU aliases

Peter reported that when he explicitely asked for multiple events with the same name on the command line it got coalesced into just one line, i.e.:

   # perf stat -e cycles -e cycles -e cycles usleep 1

Performance counter stats for 'usleep 1':

         3,269,652      cycles

       0.000884123 seconds time elapsed

And while there is the --no-merges option to disable that auto-merging, this is a blunt change in behaviour for such explicit request, so change the code so that this auto merging is done only when handling the multi PMU aliases with the same name that introduced this coalescing, restoring the previous behaviour for the explicit case:

  # perf stat -e cycles -e cycles -e cycles usleep 1

Performance counter stats for 'usleep 1':

         1,472,837      cycles
         1,472,837      cycles
         1,472,837      cycles

       0.001764870 seconds time elapsed

Reported-by Peter Zijlstra <[email protected]>

Acked-by Andi Kleen <[email protected]>

Acked-by Jiri Olsa <[email protected]>

Cc Adrian Hunter <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Wang Nan <[email protected]>

Fixes 430daf2dc7af ("perf stat: Collapse identically named events")

Link http://lkml.kernel.org/r/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/builtin-stat.c      |  2 +-
 tools/perf/util/evsel.h        |  1 +
 tools/perf/util/parse-events.c | 24 ++++++++++++++++--------
 3 files changed, 18 insertions(+), 9 deletions(-)

tools/perf/builtin-stat.c +1 -1

index 866da7a..85e992d 100644
@@ -1257,7 +1257,7 @@ static bool collect_data(struct perf_evsel *counter,
 	if (counter->merged_stat)
 		return false;
 	cb(counter, data, true);
-	if (!no_merge)
+	if (!no_merge && counter->auto_merge_stats)
 		collect_all_aliases(counter, cb, data);
 	return true;
 }

tools/perf/util/evsel.h +1 -0

index 351d3b2..dd2c4b5 100644
@@ -131,6 +131,7 @@ struct perf_evsel {
 	bool			cmdline_group_boundary;
 	struct list_head	config_terms;
 	int			bpf_fd;
+	bool			auto_merge_stats;
 	bool			merged_stat;
 	const char *		metric_expr;
 	const char *		metric_name;

tools/perf/util/parse-events.c +16 -8

index f44aeba..f6257fb 100644
@@ -310,7 +310,7 @@ static struct perf_evsel *
 __add_event(struct list_head *list, int *idx,
 	    struct perf_event_attr *attr,
 	    char *name, struct cpu_map *cpus,
-	    struct list_head *config_terms)
+	    struct list_head *config_terms, bool auto_merge_stats)
 {
 	struct perf_evsel *evsel;

@@ -324,6 +324,7 @@ __add_event(struct list_head *list, int *idx,
 	evsel->cpus        = cpu_map__get(cpus);
 	evsel->own_cpus    = cpu_map__get(cpus);
 	evsel->system_wide = !!cpus;
+	evsel->auto_merge_stats = auto_merge_stats;
 
 	if (name)
 		evsel->name = strdup(name);

@@ -339,7 +340,7 @@ static int add_event(struct list_head *list, int *idx,
 		     struct perf_event_attr *attr, char *name,
 		     struct list_head *config_terms)
 {
-	return __add_event(list, idx, attr, name, NULL, config_terms) ? 0 : -ENOMEM;
+	return __add_event(list, idx, attr, name, NULL, config_terms, false) ? 0 : -ENOMEM;
 }
 
 static int parse_aliases(char *str, const char *names[][PERF_EVSEL__MAX_ALIASES], int size)

@@ -1209,9 +1210,9 @@ int parse_events_add_numeric(struct parse_events_state *parse_state,
 			 get_config_name(head_config), &config_terms);
 }
 
-int parse_events_add_pmu(struct parse_events_state *parse_state,
+static int __parse_events_add_pmu(struct parse_events_state *parse_state,
 			 struct list_head *list, char *name,
-			 struct list_head *head_config)
+			 struct list_head *head_config, bool auto_merge_stats)
 {
 	struct perf_event_attr attr;
 	struct perf_pmu_info info;

@@ -1232,7 +1233,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 
 	if (!head_config) {
 		attr.type = pmu->type;
-		evsel = __add_event(list, &parse_state->idx, &attr, NULL, pmu->cpus, NULL);
+		evsel = __add_event(list, &parse_state->idx, &attr, NULL, pmu->cpus, NULL, auto_merge_stats);
 		return evsel ? 0 : -ENOMEM;
 	}

@@ -1254,7 +1255,7 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 
 	evsel = __add_event(list, &parse_state->idx, &attr,
 			    get_config_name(head_config), pmu->cpus,
-			    &config_terms);
+			    &config_terms, auto_merge_stats);
 	if (evsel) {
 		evsel->unit = info.unit;
 		evsel->scale = info.scale;

@@ -1267,6 +1268,13 @@ int parse_events_add_pmu(struct parse_events_state *parse_state,
 	return evsel ? 0 : -ENOMEM;
 }
 
+int parse_events_add_pmu(struct parse_events_state *parse_state,
+			 struct list_head *list, char *name,
+			 struct list_head *head_config)
+{
+	return __parse_events_add_pmu(parse_state, list, name, head_config, false);
+}
+
 int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 			       char *str, struct list_head **listp)
 {

@@ -1296,8 +1304,8 @@ int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 					return -1;
 				list_add_tail(&term->list, head);
 
-				if (!parse_events_add_pmu(parse_state, list,
-						  pmu->name, head)) {
+				if (!__parse_events_add_pmu(parse_state, list,
+							    pmu->name, head, true)) {
 					pr_debug("%s -> %s/%s/\n", str,
 						 pmu->name, alias->str);
 					ok++;