perf parse-events: Specially handle uncore event alias in small groups

PATCH New [email protected] 2 participants 3 messages May 7, 2018 – May 31, 2018 perf events

lore

Share & tools

lore

[email protected]

author

2018-05-07 21:14 UTC #0 lore raw

PATCH v6 1/1 perf parse-events: Specially handle uncore event alias in small groups

From: Kan Liang <[email protected]>

Perf stat doesn't count the uncore event aliases from the same uncore block in a group, for example:

perf stat -e '{unc_m_cas_count.all,unc_m_clockticks}' -a -I 1000
# time counts unit events

       1.000447342      <not counted>      unc_m_cas_count.all
       1.000447342      <not counted>      unc_m_clockticks
       2.000740654      <not counted>      unc_m_cas_count.all
       2.000740654      <not counted>      unc_m_clockticks

The output is very misleading. It gives a wrong impression that the uncore event doesn't work.

An uncore block could be composed by several PMUs. An uncore event alias is a joint name which means the same event runs on all PMUs of a block. Perf doesn't support mixed events from different PMUs in the same group. It is wrong to put uncore event aliases in a big group.

The right way is to split the big group into multiple small groups which only include the events from the same PMU.
Only uncore event aliases from the same uncore block should be specially handled here. It doesn't make sense to mix the uncore events with other uncore events from different blocks or even core events in a group.

With the patch:
# time counts unit events

     1.001557653            140,833      unc_m_cas_count.all
     1.001557653      1,330,231,332      unc_m_clockticks
     2.002709483             85,007      unc_m_cas_count.all
     2.002709483      1,429,494,563      unc_m_clockticks

Reported-by Andi Kleen <[email protected]>

Acked-by Jiri Olsa <[email protected]>

Signed-off-by Kan Liang <[email protected]>

---

Changes since V5:
- Re-base on perf/urgent

 tools/perf/util/evsel.h        |   1 +
 tools/perf/util/parse-events.c | 130 ++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/parse-events.h |   7 ++-
 tools/perf/util/parse-events.y |   8 +--
 4 files changed, 137 insertions(+), 9 deletions(-)

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

index 92ec009..b13f5f2 100644
@@ -127,6 +127,7 @@ struct perf_evsel {
 	bool			precise_max;
 	bool			ignore_missing_thread;
 	bool			forced_leader;
+	bool			use_uncore_alias;
 	/* parse modifier helper */
 	int			exclude_GH;
 	int			nr_members;

tools/perf/util/parse-events.c +127 -3

177 lines not on this page · raw message

tools/perf/util/parse-events.h +5 -2

index 5015cfd..4473dac 100644
@@ -167,7 +167,9 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
 				void *ptr, char *type, u64 len);
 int parse_events_add_pmu(struct parse_events_state *parse_state,
 			 struct list_head *list, char *name,
-			 struct list_head *head_config, bool auto_merge_stats);
+			 struct list_head *head_config,
+			 bool auto_merge_stats,
+			 bool use_alias);
 
 int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 			       char *str,

@@ -178,7 +180,8 @@ int parse_events_copy_term_list(struct list_head *old,
 
 enum perf_pmu_event_symbol_type
 perf_pmu__parse_check(const char *name);
-void parse_events__set_leader(char *name, struct list_head *list);
+void parse_events__set_leader(char *name, struct list_head *list,
+			      struct parse_events_state *parse_state);
 void parse_events_update_lists(struct list_head *list_event,
 			       struct list_head *list_all);
 void parse_events_evlist_error(struct parse_events_state *parse_state,

tools/perf/util/parse-events.y +4 -4

index 7afeb80..e37608a 100644
@@ -161,7 +161,7 @@ PE_NAME '{' events '}'
 	struct list_head *list = $3;
 
 	inc_group_count(list, _parse_state);
-	parse_events__set_leader($1, list);
+	parse_events__set_leader($1, list, _parse_state);
 	$$ = list;
 }
 |

@@ -170,7 +170,7 @@ PE_NAME '{' events '}'
 	struct list_head *list = $2;
 
 	inc_group_count(list, _parse_state);
-	parse_events__set_leader(NULL, list);
+	parse_events__set_leader(NULL, list, _parse_state);
 	$$ = list;
 }

@@ -232,7 +232,7 @@ PE_NAME opt_event_config
 		YYABORT;
 
 	ALLOC_LIST(list);
-	if (parse_events_add_pmu(_parse_state, list, $1, $2, false)) {
+	if (parse_events_add_pmu(_parse_state, list, $1, $2, false, false)) {
 		struct perf_pmu *pmu = NULL;
 		int ok = 0;
 		char *pattern;

@@ -251,7 +251,7 @@ PE_NAME opt_event_config
 					free(pattern);
 					YYABORT;
 				}
-				if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true))
+				if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true, false))
 					ok++;
 				parse_events_terms__delete(terms);
 			}

Signature · 1 line

2.7.4

tip-bot for Kan Liang

merge bot

2018-05-16 18:05 UTC #1 lore raw

[tip:perf/core] perf parse-events: Handle uncore event aliases in small groups properly

perf parse-events: Handle uncore event aliases in small groups properly

Commit-ID:  3cdc5c2cb924acb43a93241e75c61570eab71614
Gitweb:     https://git.kernel.org/tip/3cdc5c2cb924acb43a93241e75c61570eab71614
Author:     Kan Liang <[email protected]>
AuthorDate: Mon, 7 May 2018 14:13:43 -0700
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 16 May 2018 10:01:54 -0300

perf parse-events: Handle uncore event aliases in small groups properly

Perf stat doesn't count the uncore event aliases from the same uncore block in a group, for example:

perf stat -e '{unc_m_cas_count.all,unc_m_clockticks}' -a -I 1000
# time counts unit events

       1.000447342      <not counted>      unc_m_cas_count.all
       1.000447342      <not counted>      unc_m_clockticks
       2.000740654      <not counted>      unc_m_cas_count.all
       2.000740654      <not counted>      unc_m_clockticks

The output is very misleading. It gives a wrong impression that the uncore event doesn't work.

The right way is to split the big group into multiple small groups which only include the events from the same PMU.

Only uncore event aliases from the same uncore block should be specially handled here. It doesn't make sense to mix the uncore events with other uncore events from different blocks or even core events in a group.

With the patch:
# time counts unit events

     1.001557653            140,833      unc_m_cas_count.all
     1.001557653      1,330,231,332      unc_m_clockticks
     2.002709483             85,007      unc_m_cas_count.all
     2.002709483      1,429,494,563      unc_m_clockticks

Reported-by Andi Kleen <[email protected]>

Signed-off-by Kan Liang <[email protected]>

Acked-by Jiri Olsa <[email protected]>

Cc Agustin Vega-Frias <[email protected]>

Cc Ganapatrao Kulkarni <[email protected]>

Cc Jin Yao <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Shaokun Zhang <[email protected]>

Cc Will Deacon <[email protected]>

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

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

---
 tools/perf/util/evsel.h        |   1 +
 tools/perf/util/parse-events.c | 130 ++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/parse-events.h |   7 ++-
 tools/perf/util/parse-events.y |   8 +--
 4 files changed, 137 insertions(+), 9 deletions(-)

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

index 92ec009a292d..b13f5f234c8f 100644
@@ -127,6 +127,7 @@ struct perf_evsel {
 	bool			precise_max;
 	bool			ignore_missing_thread;
 	bool			forced_leader;
+	bool			use_uncore_alias;
 	/* parse modifier helper */
 	int			exclude_GH;
 	int			nr_members;

tools/perf/util/parse-events.c +127 -3

177 lines not on this page · raw message

tools/perf/util/parse-events.h +5 -2

index 5015cfd58277..4473dac27aee 100644
@@ -167,7 +167,9 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
 				void *ptr, char *type, u64 len);
 int parse_events_add_pmu(struct parse_events_state *parse_state,
 			 struct list_head *list, char *name,
-			 struct list_head *head_config, bool auto_merge_stats);
+			 struct list_head *head_config,
+			 bool auto_merge_stats,
+			 bool use_alias);
 
 int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 			       char *str,

@@ -178,7 +180,8 @@ int parse_events_copy_term_list(struct list_head *old,
 
 enum perf_pmu_event_symbol_type
 perf_pmu__parse_check(const char *name);
-void parse_events__set_leader(char *name, struct list_head *list);
+void parse_events__set_leader(char *name, struct list_head *list,
+			      struct parse_events_state *parse_state);
 void parse_events_update_lists(struct list_head *list_event,
 			       struct list_head *list_all);
 void parse_events_evlist_error(struct parse_events_state *parse_state,

tools/perf/util/parse-events.y +4 -4

index 7afeb80cc39e..e37608a87dba 100644
@@ -161,7 +161,7 @@ PE_NAME '{' events '}'
 	struct list_head *list = $3;
 
 	inc_group_count(list, _parse_state);
-	parse_events__set_leader($1, list);
+	parse_events__set_leader($1, list, _parse_state);
 	$$ = list;
 }
 |

@@ -170,7 +170,7 @@ PE_NAME '{' events '}'
 	struct list_head *list = $2;
 
 	inc_group_count(list, _parse_state);
-	parse_events__set_leader(NULL, list);
+	parse_events__set_leader(NULL, list, _parse_state);
 	$$ = list;
 }

@@ -232,7 +232,7 @@ PE_NAME opt_event_config
 		YYABORT;
 
 	ALLOC_LIST(list);
-	if (parse_events_add_pmu(_parse_state, list, $1, $2, false)) {
+	if (parse_events_add_pmu(_parse_state, list, $1, $2, false, false)) {
 		struct perf_pmu *pmu = NULL;
 		int ok = 0;
 		char *pattern;

@@ -251,7 +251,7 @@ PE_NAME opt_event_config
 					free(pattern);
 					YYABORT;
 				}
-				if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true))
+				if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true, false))
 					ok++;
 				parse_events_terms__delete(terms);
 			}

tip-bot for Kan Liang

merge bot

2018-05-31 10:43 UTC #2 lore raw

[tip:perf/urgent] perf parse-events: Handle uncore event aliases in small groups properly

In reply to [email protected]

perf parse-events: Handle uncore event aliases in small groups properly

Commit-ID:  369b230806a9f3ff691466d54795e5e3ff3f8f5c
Gitweb:     https://git.kernel.org/tip/369b230806a9f3ff691466d54795e5e3ff3f8f5c
Author:     Kan Liang <[email protected]>
AuthorDate: Mon, 7 May 2018 14:13:43 -0700
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 30 May 2018 10:40:44 -0300

perf parse-events: Handle uncore event aliases in small groups properly

Perf stat doesn't count the uncore event aliases from the same uncore block in a group, for example:

perf stat -e '{unc_m_cas_count.all,unc_m_clockticks}' -a -I 1000
# time counts unit events

       1.000447342      <not counted>      unc_m_cas_count.all
       1.000447342      <not counted>      unc_m_clockticks
       2.000740654      <not counted>      unc_m_cas_count.all
       2.000740654      <not counted>      unc_m_clockticks

The output is very misleading. It gives a wrong impression that the uncore event doesn't work.

The right way is to split the big group into multiple small groups which only include the events from the same PMU.

With the patch:
# time counts unit events

     1.001557653            140,833      unc_m_cas_count.all
     1.001557653      1,330,231,332      unc_m_clockticks
     2.002709483             85,007      unc_m_cas_count.all
     2.002709483      1,429,494,563      unc_m_clockticks

Reported-by Andi Kleen <[email protected]>

Signed-off-by Kan Liang <[email protected]>

Acked-by Jiri Olsa <[email protected]>

Cc Agustin Vega-Frias <[email protected]>

Cc Ganapatrao Kulkarni <[email protected]>

Cc Jin Yao <[email protected]>

Cc Namhyung Kim <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Shaokun Zhang <[email protected]>

Cc Will Deacon <[email protected]>

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

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

---
 tools/perf/util/evsel.h        |   1 +
 tools/perf/util/parse-events.c | 130 ++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/parse-events.h |   7 ++-
 tools/perf/util/parse-events.y |   8 +--
 4 files changed, 137 insertions(+), 9 deletions(-)

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

index 92ec009a292d..b13f5f234c8f 100644
@@ -127,6 +127,7 @@ struct perf_evsel {
 	bool			precise_max;
 	bool			ignore_missing_thread;
 	bool			forced_leader;
+	bool			use_uncore_alias;
 	/* parse modifier helper */
 	int			exclude_GH;
 	int			nr_members;

tools/perf/util/parse-events.c +127 -3

177 lines not on this page · raw message

tools/perf/util/parse-events.h +5 -2

index 5015cfd58277..4473dac27aee 100644
@@ -167,7 +167,9 @@ int parse_events_add_breakpoint(struct list_head *list, int *idx,
 				void *ptr, char *type, u64 len);
 int parse_events_add_pmu(struct parse_events_state *parse_state,
 			 struct list_head *list, char *name,
-			 struct list_head *head_config, bool auto_merge_stats);
+			 struct list_head *head_config,
+			 bool auto_merge_stats,
+			 bool use_alias);
 
 int parse_events_multi_pmu_add(struct parse_events_state *parse_state,
 			       char *str,

@@ -178,7 +180,8 @@ int parse_events_copy_term_list(struct list_head *old,
 
 enum perf_pmu_event_symbol_type
 perf_pmu__parse_check(const char *name);
-void parse_events__set_leader(char *name, struct list_head *list);
+void parse_events__set_leader(char *name, struct list_head *list,
+			      struct parse_events_state *parse_state);
 void parse_events_update_lists(struct list_head *list_event,
 			       struct list_head *list_all);
 void parse_events_evlist_error(struct parse_events_state *parse_state,

tools/perf/util/parse-events.y +4 -4

index 7afeb80cc39e..e37608a87dba 100644
@@ -161,7 +161,7 @@ PE_NAME '{' events '}'
 	struct list_head *list = $3;
 
 	inc_group_count(list, _parse_state);
-	parse_events__set_leader($1, list);
+	parse_events__set_leader($1, list, _parse_state);
 	$$ = list;
 }
 |

@@ -170,7 +170,7 @@ PE_NAME '{' events '}'
 	struct list_head *list = $2;
 
 	inc_group_count(list, _parse_state);
-	parse_events__set_leader(NULL, list);
+	parse_events__set_leader(NULL, list, _parse_state);
 	$$ = list;
 }

@@ -232,7 +232,7 @@ PE_NAME opt_event_config
 		YYABORT;
 
 	ALLOC_LIST(list);
-	if (parse_events_add_pmu(_parse_state, list, $1, $2, false)) {
+	if (parse_events_add_pmu(_parse_state, list, $1, $2, false, false)) {
 		struct perf_pmu *pmu = NULL;
 		int ok = 0;
 		char *pattern;

@@ -251,7 +251,7 @@ PE_NAME opt_event_config
 					free(pattern);
 					YYABORT;
 				}
-				if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true))
+				if (!parse_events_add_pmu(_parse_state, list, pmu->name, terms, true, false))
 					ok++;
 				parse_events_terms__delete(terms);
 			}