linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] perf list: Display hybrid pmu events with cpu type
@ 2021-09-03  2:52 Jin Yao
  2021-09-09 22:37 ` Ian Rogers
  2021-10-19 21:14 ` Arnaldo Carvalho de Melo
  0 siblings, 2 replies; 7+ messages in thread
From: Jin Yao @ 2021-09-03  2:52 UTC (permalink / raw)
  To: acme, jolsa, peterz, mingo, alexander.shishkin
  Cc: Linux-kernel, linux-perf-users, ak, kan.liang, yao.jin, Jin Yao

Add a new option '--cputype' to perf-list to display core-only pmu events
or atom-only pmu events.

Each hybrid pmu event has been assigned with a pmu name, this patch
compares the pmu name before listing the result.

For example,

perf list --cputype atom
...
cache:
  core_reject_l2q.any
       [Counts the number of request that were not accepted into the L2Q because the L2Q is FULL. Unit: cpu_atom]
...

The "Unit: cpu_atom" is displayed in the brief description section
to indicate this is an atom event.

Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
---
v2:
 - Rebase to perf/core branch.

 tools/perf/Documentation/perf-list.txt |  4 +++
 tools/perf/builtin-list.c              | 42 ++++++++++++++++++--------
 tools/perf/util/metricgroup.c          |  7 ++++-
 tools/perf/util/metricgroup.h          |  2 +-
 tools/perf/util/parse-events.c         |  8 +++--
 tools/perf/util/parse-events.h         |  3 +-
 tools/perf/util/pmu.c                  | 29 +++++++++++++++---
 tools/perf/util/pmu.h                  |  2 +-
 8 files changed, 73 insertions(+), 24 deletions(-)

diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
index 4c7db1da8fcc..4dc8d0af19df 100644
--- a/tools/perf/Documentation/perf-list.txt
+++ b/tools/perf/Documentation/perf-list.txt
@@ -39,6 +39,10 @@ any extra expressions computed by perf stat.
 --deprecated::
 Print deprecated events. By default the deprecated events are hidden.
 
+--cputype::
+Print events applying cpu with this type for hybrid platform
+(e.g. --cputype core or --cputype atom)
+
 [[EVENT_MODIFIERS]]
 EVENT MODIFIERS
 ---------------
diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
index 10ab5e40a34f..468958154ed9 100644
--- a/tools/perf/builtin-list.c
+++ b/tools/perf/builtin-list.c
@@ -12,6 +12,7 @@
 
 #include "util/parse-events.h"
 #include "util/pmu.h"
+#include "util/pmu-hybrid.h"
 #include "util/debug.h"
 #include "util/metricgroup.h"
 #include <subcmd/pager.h>
@@ -20,13 +21,15 @@
 
 static bool desc_flag = true;
 static bool details_flag;
+static const char *hybrid_type;
 
 int cmd_list(int argc, const char **argv)
 {
-	int i;
+	int i, ret = 0;
 	bool raw_dump = false;
 	bool long_desc_flag = false;
 	bool deprecated = false;
+	char *pmu_name = NULL;
 	struct option list_options[] = {
 		OPT_BOOLEAN(0, "raw-dump", &raw_dump, "Dump raw events"),
 		OPT_BOOLEAN('d', "desc", &desc_flag,
@@ -37,6 +40,9 @@ int cmd_list(int argc, const char **argv)
 			    "Print information on the perf event names and expressions used internally by events."),
 		OPT_BOOLEAN(0, "deprecated", &deprecated,
 			    "Print deprecated events."),
+		OPT_STRING(0, "cputype", &hybrid_type, "hybrid cpu type",
+			   "Print events applying cpu with this type for hybrid platform "
+			   "(e.g. core or atom)"),
 		OPT_INCR(0, "debug", &verbose,
 			     "Enable debugging output"),
 		OPT_END()
@@ -56,10 +62,16 @@ int cmd_list(int argc, const char **argv)
 	if (!raw_dump && pager_in_use())
 		printf("\nList of pre-defined events (to be used in -e):\n\n");
 
+	if (hybrid_type) {
+		pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_type);
+		if (!pmu_name)
+			pr_warning("WARNING: hybrid cputype is not supported!\n");
+	}
+
 	if (argc == 0) {
 		print_events(NULL, raw_dump, !desc_flag, long_desc_flag,
-				details_flag, deprecated);
-		return 0;
+				details_flag, deprecated, pmu_name);
+		goto out;
 	}
 
 	for (i = 0; i < argc; ++i) {
@@ -82,25 +94,27 @@ int cmd_list(int argc, const char **argv)
 		else if (strcmp(argv[i], "pmu") == 0)
 			print_pmu_events(NULL, raw_dump, !desc_flag,
 						long_desc_flag, details_flag,
-						deprecated);
+						deprecated, pmu_name);
 		else if (strcmp(argv[i], "sdt") == 0)
 			print_sdt_events(NULL, NULL, raw_dump);
 		else if (strcmp(argv[i], "metric") == 0 || strcmp(argv[i], "metrics") == 0)
-			metricgroup__print(true, false, NULL, raw_dump, details_flag);
+			metricgroup__print(true, false, NULL, raw_dump, details_flag, pmu_name);
 		else if (strcmp(argv[i], "metricgroup") == 0 || strcmp(argv[i], "metricgroups") == 0)
-			metricgroup__print(false, true, NULL, raw_dump, details_flag);
+			metricgroup__print(false, true, NULL, raw_dump, details_flag, pmu_name);
 		else if ((sep = strchr(argv[i], ':')) != NULL) {
 			int sep_idx;
 
 			sep_idx = sep - argv[i];
 			s = strdup(argv[i]);
-			if (s == NULL)
-				return -1;
+			if (s == NULL) {
+				ret = -1;
+				goto out;
+			}
 
 			s[sep_idx] = '\0';
 			print_tracepoint_events(s, s + sep_idx + 1, raw_dump);
 			print_sdt_events(s, s + sep_idx + 1, raw_dump);
-			metricgroup__print(true, true, s, raw_dump, details_flag);
+			metricgroup__print(true, true, s, raw_dump, details_flag, pmu_name);
 			free(s);
 		} else {
 			if (asprintf(&s, "*%s*", argv[i]) < 0) {
@@ -116,12 +130,16 @@ int cmd_list(int argc, const char **argv)
 			print_pmu_events(s, raw_dump, !desc_flag,
 						long_desc_flag,
 						details_flag,
-						deprecated);
+						deprecated,
+						pmu_name);
 			print_tracepoint_events(NULL, s, raw_dump);
 			print_sdt_events(NULL, s, raw_dump);
-			metricgroup__print(true, true, s, raw_dump, details_flag);
+			metricgroup__print(true, true, s, raw_dump, details_flag, pmu_name);
 			free(s);
 		}
 	}
-	return 0;
+
+out:
+	free(pmu_name);
+	return ret;
 }
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 99d047c5ead0..ad2587079689 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -11,6 +11,7 @@
 #include "evsel.h"
 #include "strbuf.h"
 #include "pmu.h"
+#include "pmu-hybrid.h"
 #include "expr.h"
 #include "rblist.h"
 #include <string.h>
@@ -616,7 +617,7 @@ static int metricgroup__print_sys_event_iter(struct pmu_event *pe, void *data)
 }
 
 void metricgroup__print(bool metrics, bool metricgroups, char *filter,
-			bool raw, bool details)
+			bool raw, bool details, const char *pmu_name)
 {
 	struct pmu_events_map *map = pmu_events_map__find();
 	struct pmu_event *pe;
@@ -642,6 +643,10 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
 			break;
 		if (!pe->metric_expr)
 			continue;
+		if (pmu_name && perf_pmu__is_hybrid(pe->pmu) &&
+		    strcmp(pmu_name, pe->pmu)) {
+			continue;
+		}
 		if (metricgroup__print_pmu_event(pe, metricgroups, filter,
 						 raw, details, &groups,
 						 metriclist) < 0)
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index cc4a92492a61..9deee6691f2e 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -53,7 +53,7 @@ int metricgroup__parse_groups_test(struct evlist *evlist,
 				   struct rblist *metric_events);
 
 void metricgroup__print(bool metrics, bool groups, char *filter,
-			bool raw, bool details);
+			bool raw, bool details, const char *pmu_name);
 bool metricgroup__has_metric(const char *metric);
 int arch_get_runtimeparam(struct pmu_event *pe __maybe_unused);
 void metricgroup__rblist_exit(struct rblist *metric_events);
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index e5eae23cfceb..f36e748ad715 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -2995,7 +2995,8 @@ void print_symbol_events(const char *event_glob, unsigned type,
  * Print the help text for the event symbols:
  */
 void print_events(const char *event_glob, bool name_only, bool quiet_flag,
-			bool long_desc, bool details_flag, bool deprecated)
+			bool long_desc, bool details_flag, bool deprecated,
+			const char *pmu_name)
 {
 	print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
 			    event_symbols_hw, PERF_COUNT_HW_MAX, name_only);
@@ -3007,7 +3008,7 @@ void print_events(const char *event_glob, bool name_only, bool quiet_flag,
 	print_hwcache_events(event_glob, name_only);
 
 	print_pmu_events(event_glob, name_only, quiet_flag, long_desc,
-			details_flag, deprecated);
+			details_flag, deprecated, pmu_name);
 
 	if (event_glob != NULL)
 		return;
@@ -3033,7 +3034,8 @@ void print_events(const char *event_glob, bool name_only, bool quiet_flag,
 
 	print_sdt_events(NULL, NULL, name_only);
 
-	metricgroup__print(true, true, NULL, name_only, details_flag);
+	metricgroup__print(true, true, NULL, name_only, details_flag,
+			   pmu_name);
 
 	print_libpfm_events(name_only, long_desc);
 }
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index bf6e41aa9b6a..ce0c910163d1 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -219,7 +219,8 @@ void parse_events_evlist_error(struct parse_events_state *parse_state,
 			       int idx, const char *str);
 
 void print_events(const char *event_glob, bool name_only, bool quiet,
-		  bool long_desc, bool details_flag, bool deprecated);
+		  bool long_desc, bool details_flag, bool deprecated,
+		  const char *pmu_name);
 
 struct event_symbol {
 	const char	*symbol;
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 6cdbee8a12e7..77204c5af29c 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1577,6 +1577,7 @@ static int cmp_sevent(const void *a, const void *b)
 {
 	const struct sevent *as = a;
 	const struct sevent *bs = b;
+	int ret;
 
 	/* Put extra events last */
 	if (!!as->desc != !!bs->desc)
@@ -1592,7 +1593,13 @@ static int cmp_sevent(const void *a, const void *b)
 	if (as->is_cpu != bs->is_cpu)
 		return bs->is_cpu - as->is_cpu;
 
-	return strcmp(as->name, bs->name);
+	ret = strcmp(as->name, bs->name);
+	if (!ret) {
+		if (as->pmu && bs->pmu)
+			return strcmp(as->pmu, bs->pmu);
+	}
+
+	return ret;
 }
 
 static void wordwrap(char *s, int start, int max, int corr)
@@ -1622,7 +1629,8 @@ bool is_pmu_core(const char *name)
 }
 
 void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
-			bool long_desc, bool details_flag, bool deprecated)
+			bool long_desc, bool details_flag, bool deprecated,
+			const char *pmu_name)
 {
 	struct perf_pmu *pmu;
 	struct perf_pmu_alias *alias;
@@ -1648,10 +1656,16 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
 	pmu = NULL;
 	j = 0;
 	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		if (pmu_name && perf_pmu__is_hybrid(pmu->name) &&
+		    strcmp(pmu_name, pmu->name)) {
+			continue;
+		}
+
 		list_for_each_entry(alias, &pmu->aliases, list) {
 			char *name = alias->desc ? alias->name :
 				format_alias(buf, sizeof(buf), pmu, alias);
-			bool is_cpu = is_pmu_core(pmu->name);
+			bool is_cpu = is_pmu_core(pmu->name) ||
+				      perf_pmu__is_hybrid(pmu->name);
 
 			if (alias->deprecated && !deprecated)
 				continue;
@@ -1699,8 +1713,13 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
 	qsort(aliases, len, sizeof(struct sevent), cmp_sevent);
 	for (j = 0; j < len; j++) {
 		/* Skip duplicates */
-		if (j > 0 && !strcmp(aliases[j].name, aliases[j - 1].name))
-			continue;
+		if (j > 0 && !strcmp(aliases[j].name, aliases[j - 1].name)) {
+			if (!aliases[j].pmu || !aliases[j - 1].pmu ||
+			    !strcmp(aliases[j].pmu, aliases[j - 1].pmu)) {
+				continue;
+			}
+		}
+
 		if (name_only) {
 			printf("%s ", aliases[j].name);
 			continue;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 033e8211c025..91fc0de892f5 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -108,7 +108,7 @@ struct perf_pmu *perf_pmu__scan(struct perf_pmu *pmu);
 bool is_pmu_core(const char *name);
 void print_pmu_events(const char *event_glob, bool name_only, bool quiet,
 		      bool long_desc, bool details_flag,
-		      bool deprecated);
+		      bool deprecated, const char *pmu_name);
 bool pmu_have_event(const char *pname, const char *name);
 
 int perf_pmu__scan_file(struct perf_pmu *pmu, const char *name, const char *fmt, ...) __scanf(3, 4);
-- 
2.27.0


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

* Re: [PATCH v2] perf list: Display hybrid pmu events with cpu type
  2021-09-03  2:52 [PATCH v2] perf list: Display hybrid pmu events with cpu type Jin Yao
@ 2021-09-09 22:37 ` Ian Rogers
  2021-09-09 22:56   ` Andi Kleen
  2021-10-19 21:14 ` Arnaldo Carvalho de Melo
  1 sibling, 1 reply; 7+ messages in thread
From: Ian Rogers @ 2021-09-09 22:37 UTC (permalink / raw)
  To: Jin Yao
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, linux-kernel,
	linux-perf-users, ak, kan.liang, yao.jin

On Thu, Sep 2, 2021 at 7:54 PM Jin Yao <yao.jin@linux.intel.com> wrote:
>
> Add a new option '--cputype' to perf-list to display core-only pmu events
> or atom-only pmu events.
>
> Each hybrid pmu event has been assigned with a pmu name, this patch
> compares the pmu name before listing the result.

Would this work more broadly for any PMU type? If so perhaps pmu
rather than cputype is a more appropriate option name?

Thanks,
Ian

> For example,
>
> perf list --cputype atom
> ...
> cache:
>   core_reject_l2q.any
>        [Counts the number of request that were not accepted into the L2Q because the L2Q is FULL. Unit: cpu_atom]
> ...
>
> The "Unit: cpu_atom" is displayed in the brief description section
> to indicate this is an atom event.
>
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
> v2:
>  - Rebase to perf/core branch.
>
>  tools/perf/Documentation/perf-list.txt |  4 +++
>  tools/perf/builtin-list.c              | 42 ++++++++++++++++++--------
>  tools/perf/util/metricgroup.c          |  7 ++++-
>  tools/perf/util/metricgroup.h          |  2 +-
>  tools/perf/util/parse-events.c         |  8 +++--
>  tools/perf/util/parse-events.h         |  3 +-
>  tools/perf/util/pmu.c                  | 29 +++++++++++++++---
>  tools/perf/util/pmu.h                  |  2 +-
>  8 files changed, 73 insertions(+), 24 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-list.txt b/tools/perf/Documentation/perf-list.txt
> index 4c7db1da8fcc..4dc8d0af19df 100644
> --- a/tools/perf/Documentation/perf-list.txt
> +++ b/tools/perf/Documentation/perf-list.txt
> @@ -39,6 +39,10 @@ any extra expressions computed by perf stat.
>  --deprecated::
>  Print deprecated events. By default the deprecated events are hidden.
>
> +--cputype::
> +Print events applying cpu with this type for hybrid platform
> +(e.g. --cputype core or --cputype atom)
> +
>  [[EVENT_MODIFIERS]]
>  EVENT MODIFIERS
>  ---------------
> diff --git a/tools/perf/builtin-list.c b/tools/perf/builtin-list.c
> index 10ab5e40a34f..468958154ed9 100644
> --- a/tools/perf/builtin-list.c
> +++ b/tools/perf/builtin-list.c
> @@ -12,6 +12,7 @@
>
>  #include "util/parse-events.h"
>  #include "util/pmu.h"
> +#include "util/pmu-hybrid.h"
>  #include "util/debug.h"
>  #include "util/metricgroup.h"
>  #include <subcmd/pager.h>
> @@ -20,13 +21,15 @@
>
>  static bool desc_flag = true;
>  static bool details_flag;
> +static const char *hybrid_type;
>
>  int cmd_list(int argc, const char **argv)
>  {
> -       int i;
> +       int i, ret = 0;
>         bool raw_dump = false;
>         bool long_desc_flag = false;
>         bool deprecated = false;
> +       char *pmu_name = NULL;
>         struct option list_options[] = {
>                 OPT_BOOLEAN(0, "raw-dump", &raw_dump, "Dump raw events"),
>                 OPT_BOOLEAN('d', "desc", &desc_flag,
> @@ -37,6 +40,9 @@ int cmd_list(int argc, const char **argv)
>                             "Print information on the perf event names and expressions used internally by events."),
>                 OPT_BOOLEAN(0, "deprecated", &deprecated,
>                             "Print deprecated events."),
> +               OPT_STRING(0, "cputype", &hybrid_type, "hybrid cpu type",
> +                          "Print events applying cpu with this type for hybrid platform "
> +                          "(e.g. core or atom)"),
>                 OPT_INCR(0, "debug", &verbose,
>                              "Enable debugging output"),
>                 OPT_END()
> @@ -56,10 +62,16 @@ int cmd_list(int argc, const char **argv)
>         if (!raw_dump && pager_in_use())
>                 printf("\nList of pre-defined events (to be used in -e):\n\n");
>
> +       if (hybrid_type) {
> +               pmu_name = perf_pmu__hybrid_type_to_pmu(hybrid_type);
> +               if (!pmu_name)
> +                       pr_warning("WARNING: hybrid cputype is not supported!\n");
> +       }
> +
>         if (argc == 0) {
>                 print_events(NULL, raw_dump, !desc_flag, long_desc_flag,
> -                               details_flag, deprecated);
> -               return 0;
> +                               details_flag, deprecated, pmu_name);
> +               goto out;
>         }
>
>         for (i = 0; i < argc; ++i) {
> @@ -82,25 +94,27 @@ int cmd_list(int argc, const char **argv)
>                 else if (strcmp(argv[i], "pmu") == 0)
>                         print_pmu_events(NULL, raw_dump, !desc_flag,
>                                                 long_desc_flag, details_flag,
> -                                               deprecated);
> +                                               deprecated, pmu_name);
>                 else if (strcmp(argv[i], "sdt") == 0)
>                         print_sdt_events(NULL, NULL, raw_dump);
>                 else if (strcmp(argv[i], "metric") == 0 || strcmp(argv[i], "metrics") == 0)
> -                       metricgroup__print(true, false, NULL, raw_dump, details_flag);
> +                       metricgroup__print(true, false, NULL, raw_dump, details_flag, pmu_name);
>                 else if (strcmp(argv[i], "metricgroup") == 0 || strcmp(argv[i], "metricgroups") == 0)
> -                       metricgroup__print(false, true, NULL, raw_dump, details_flag);
> +                       metricgroup__print(false, true, NULL, raw_dump, details_flag, pmu_name);
>                 else if ((sep = strchr(argv[i], ':')) != NULL) {
>                         int sep_idx;
>
>                         sep_idx = sep - argv[i];
>                         s = strdup(argv[i]);
> -                       if (s == NULL)
> -                               return -1;
> +                       if (s == NULL) {
> +                               ret = -1;
> +                               goto out;
> +                       }
>
>                         s[sep_idx] = '\0';
>                         print_tracepoint_events(s, s + sep_idx + 1, raw_dump);
>                         print_sdt_events(s, s + sep_idx + 1, raw_dump);
> -                       metricgroup__print(true, true, s, raw_dump, details_flag);
> +                       metricgroup__print(true, true, s, raw_dump, details_flag, pmu_name);
>                         free(s);
>                 } else {
>                         if (asprintf(&s, "*%s*", argv[i]) < 0) {
> @@ -116,12 +130,16 @@ int cmd_list(int argc, const char **argv)
>                         print_pmu_events(s, raw_dump, !desc_flag,
>                                                 long_desc_flag,
>                                                 details_flag,
> -                                               deprecated);
> +                                               deprecated,
> +                                               pmu_name);
>                         print_tracepoint_events(NULL, s, raw_dump);
>                         print_sdt_events(NULL, s, raw_dump);
> -                       metricgroup__print(true, true, s, raw_dump, details_flag);
> +                       metricgroup__print(true, true, s, raw_dump, details_flag, pmu_name);
>                         free(s);
>                 }
>         }
> -       return 0;
> +
> +out:
> +       free(pmu_name);
> +       return ret;
>  }
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 99d047c5ead0..ad2587079689 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -11,6 +11,7 @@
>  #include "evsel.h"
>  #include "strbuf.h"
>  #include "pmu.h"
> +#include "pmu-hybrid.h"
>  #include "expr.h"
>  #include "rblist.h"
>  #include <string.h>
> @@ -616,7 +617,7 @@ static int metricgroup__print_sys_event_iter(struct pmu_event *pe, void *data)
>  }
>
>  void metricgroup__print(bool metrics, bool metricgroups, char *filter,
> -                       bool raw, bool details)
> +                       bool raw, bool details, const char *pmu_name)
>  {
>         struct pmu_events_map *map = pmu_events_map__find();
>         struct pmu_event *pe;
> @@ -642,6 +643,10 @@ void metricgroup__print(bool metrics, bool metricgroups, char *filter,
>                         break;
>                 if (!pe->metric_expr)
>                         continue;
> +               if (pmu_name && perf_pmu__is_hybrid(pe->pmu) &&
> +                   strcmp(pmu_name, pe->pmu)) {
> +                       continue;
> +               }
>                 if (metricgroup__print_pmu_event(pe, metricgroups, filter,
>                                                  raw, details, &groups,
>                                                  metriclist) < 0)
> diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
> index cc4a92492a61..9deee6691f2e 100644
> --- a/tools/perf/util/metricgroup.h
> +++ b/tools/perf/util/metricgroup.h
> @@ -53,7 +53,7 @@ int metricgroup__parse_groups_test(struct evlist *evlist,
>                                    struct rblist *metric_events);
>
>  void metricgroup__print(bool metrics, bool groups, char *filter,
> -                       bool raw, bool details);
> +                       bool raw, bool details, const char *pmu_name);
>  bool metricgroup__has_metric(const char *metric);
>  int arch_get_runtimeparam(struct pmu_event *pe __maybe_unused);
>  void metricgroup__rblist_exit(struct rblist *metric_events);
> diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
> index e5eae23cfceb..f36e748ad715 100644
> --- a/tools/perf/util/parse-events.c
> +++ b/tools/perf/util/parse-events.c
> @@ -2995,7 +2995,8 @@ void print_symbol_events(const char *event_glob, unsigned type,
>   * Print the help text for the event symbols:
>   */
>  void print_events(const char *event_glob, bool name_only, bool quiet_flag,
> -                       bool long_desc, bool details_flag, bool deprecated)
> +                       bool long_desc, bool details_flag, bool deprecated,
> +                       const char *pmu_name)
>  {
>         print_symbol_events(event_glob, PERF_TYPE_HARDWARE,
>                             event_symbols_hw, PERF_COUNT_HW_MAX, name_only);
> @@ -3007,7 +3008,7 @@ void print_events(const char *event_glob, bool name_only, bool quiet_flag,
>         print_hwcache_events(event_glob, name_only);
>
>         print_pmu_events(event_glob, name_only, quiet_flag, long_desc,
> -                       details_flag, deprecated);
> +                       details_flag, deprecated, pmu_name);
>
>         if (event_glob != NULL)
>                 return;
> @@ -3033,7 +3034,8 @@ void print_events(const char *event_glob, bool name_only, bool quiet_flag,
>
>         print_sdt_events(NULL, NULL, name_only);
>
> -       metricgroup__print(true, true, NULL, name_only, details_flag);
> +       metricgroup__print(true, true, NULL, name_only, details_flag,
> +                          pmu_name);
>
>         print_libpfm_events(name_only, long_desc);
>  }
> diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
> index bf6e41aa9b6a..ce0c910163d1 100644
> --- a/tools/perf/util/parse-events.h
> +++ b/tools/perf/util/parse-events.h
> @@ -219,7 +219,8 @@ void parse_events_evlist_error(struct parse_events_state *parse_state,
>                                int idx, const char *str);
>
>  void print_events(const char *event_glob, bool name_only, bool quiet,
> -                 bool long_desc, bool details_flag, bool deprecated);
> +                 bool long_desc, bool details_flag, bool deprecated,
> +                 const char *pmu_name);
>
>  struct event_symbol {
>         const char      *symbol;
> diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
> index 6cdbee8a12e7..77204c5af29c 100644
> --- a/tools/perf/util/pmu.c
> +++ b/tools/perf/util/pmu.c
> @@ -1577,6 +1577,7 @@ static int cmp_sevent(const void *a, const void *b)
>  {
>         const struct sevent *as = a;
>         const struct sevent *bs = b;
> +       int ret;
>
>         /* Put extra events last */
>         if (!!as->desc != !!bs->desc)
> @@ -1592,7 +1593,13 @@ static int cmp_sevent(const void *a, const void *b)
>         if (as->is_cpu != bs->is_cpu)
>                 return bs->is_cpu - as->is_cpu;
>
> -       return strcmp(as->name, bs->name);
> +       ret = strcmp(as->name, bs->name);
> +       if (!ret) {
> +               if (as->pmu && bs->pmu)
> +                       return strcmp(as->pmu, bs->pmu);
> +       }
> +
> +       return ret;
>  }
>
>  static void wordwrap(char *s, int start, int max, int corr)
> @@ -1622,7 +1629,8 @@ bool is_pmu_core(const char *name)
>  }
>
>  void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
> -                       bool long_desc, bool details_flag, bool deprecated)
> +                       bool long_desc, bool details_flag, bool deprecated,
> +                       const char *pmu_name)
>  {
>         struct perf_pmu *pmu;
>         struct perf_pmu_alias *alias;
> @@ -1648,10 +1656,16 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
>         pmu = NULL;
>         j = 0;
>         while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> +               if (pmu_name && perf_pmu__is_hybrid(pmu->name) &&
> +                   strcmp(pmu_name, pmu->name)) {
> +                       continue;
> +               }
> +
>                 list_for_each_entry(alias, &pmu->aliases, list) {
>                         char *name = alias->desc ? alias->name :
>                                 format_alias(buf, sizeof(buf), pmu, alias);
> -                       bool is_cpu = is_pmu_core(pmu->name);
> +                       bool is_cpu = is_pmu_core(pmu->name) ||
> +                                     perf_pmu__is_hybrid(pmu->name);
>
>                         if (alias->deprecated && !deprecated)
>                                 continue;
> @@ -1699,8 +1713,13 @@ void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
>         qsort(aliases, len, sizeof(struct sevent), cmp_sevent);
>         for (j = 0; j < len; j++) {
>                 /* Skip duplicates */
> -               if (j > 0 && !strcmp(aliases[j].name, aliases[j - 1].name))
> -                       continue;
> +               if (j > 0 && !strcmp(aliases[j].name, aliases[j - 1].name)) {
> +                       if (!aliases[j].pmu || !aliases[j - 1].pmu ||
> +                           !strcmp(aliases[j].pmu, aliases[j - 1].pmu)) {
> +                               continue;
> +                       }
> +               }
> +
>                 if (name_only) {
>                         printf("%s ", aliases[j].name);
>                         continue;
> diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
> index 033e8211c025..91fc0de892f5 100644
> --- a/tools/perf/util/pmu.h
> +++ b/tools/perf/util/pmu.h
> @@ -108,7 +108,7 @@ struct perf_pmu *perf_pmu__scan(struct perf_pmu *pmu);
>  bool is_pmu_core(const char *name);
>  void print_pmu_events(const char *event_glob, bool name_only, bool quiet,
>                       bool long_desc, bool details_flag,
> -                     bool deprecated);
> +                     bool deprecated, const char *pmu_name);
>  bool pmu_have_event(const char *pname, const char *name);
>
>  int perf_pmu__scan_file(struct perf_pmu *pmu, const char *name, const char *fmt, ...) __scanf(3, 4);
> --
> 2.27.0
>

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

* Re: [PATCH v2] perf list: Display hybrid pmu events with cpu type
  2021-09-09 22:37 ` Ian Rogers
@ 2021-09-09 22:56   ` Andi Kleen
  2021-09-10  0:39     ` Jin, Yao
  0 siblings, 1 reply; 7+ messages in thread
From: Andi Kleen @ 2021-09-09 22:56 UTC (permalink / raw)
  To: Ian Rogers, Jin Yao
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, linux-kernel,
	linux-perf-users, kan.liang, yao.jin


On 9/9/2021 3:37 PM, Ian Rogers wrote:
> On Thu, Sep 2, 2021 at 7:54 PM Jin Yao <yao.jin@linux.intel.com> wrote:
>> Add a new option '--cputype' to perf-list to display core-only pmu events
>> or atom-only pmu events.
>>
>> Each hybrid pmu event has been assigned with a pmu name, this patch
>> compares the pmu name before listing the result.
> Would this work more broadly for any PMU type? If so perhaps pmu
> rather than cputype is a more appropriate option name?

It's not just the cpu pmu, because it still lists the uncore events, 
which makes sense.

If you want to match the pmu it probably would make sense to match it in 
the default matching for non option arguments in perf list. But that 
would be a different patch.

-Andi



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

* Re: [PATCH v2] perf list: Display hybrid pmu events with cpu type
  2021-09-09 22:56   ` Andi Kleen
@ 2021-09-10  0:39     ` Jin, Yao
  0 siblings, 0 replies; 7+ messages in thread
From: Jin, Yao @ 2021-09-10  0:39 UTC (permalink / raw)
  To: Andi Kleen, Ian Rogers
  Cc: acme, jolsa, peterz, mingo, alexander.shishkin, linux-kernel,
	linux-perf-users, kan.liang, yao.jin



On 9/10/2021 6:56 AM, Andi Kleen wrote:
> 
> On 9/9/2021 3:37 PM, Ian Rogers wrote:
>> On Thu, Sep 2, 2021 at 7:54 PM Jin Yao <yao.jin@linux.intel.com> wrote:
>>> Add a new option '--cputype' to perf-list to display core-only pmu events
>>> or atom-only pmu events.
>>>
>>> Each hybrid pmu event has been assigned with a pmu name, this patch
>>> compares the pmu name before listing the result.
>> Would this work more broadly for any PMU type? If so perhaps pmu
>> rather than cputype is a more appropriate option name?
> 
> It's not just the cpu pmu, because it still lists the uncore events, which makes sense.
> 
> If you want to match the pmu it probably would make sense to match it in the default matching for 
> non option arguments in perf list. But that would be a different patch.
> 
> -Andi
> 
> 

Yes, agree with Andi. Besides for cpu pmu events, it also lists the software events and uncore 
events. So probably cputype is an appropriate option name.

Thanks
Jin Yao


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

* Re: [PATCH v2] perf list: Display hybrid pmu events with cpu type
  2021-09-03  2:52 [PATCH v2] perf list: Display hybrid pmu events with cpu type Jin Yao
  2021-09-09 22:37 ` Ian Rogers
@ 2021-10-19 21:14 ` Arnaldo Carvalho de Melo
  2021-12-15 16:18   ` John Garry
  1 sibling, 1 reply; 7+ messages in thread
From: Arnaldo Carvalho de Melo @ 2021-10-19 21:14 UTC (permalink / raw)
  To: Jin Yao
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
	linux-perf-users, ak, kan.liang, yao.jin

Em Fri, Sep 03, 2021 at 10:52:39AM +0800, Jin Yao escreveu:
> Add a new option '--cputype' to perf-list to display core-only pmu events
> or atom-only pmu events.
> 
> Each hybrid pmu event has been assigned with a pmu name, this patch
> compares the pmu name before listing the result.
> 
> For example,
> 
> perf list --cputype atom
> ...
> cache:
>   core_reject_l2q.any
>        [Counts the number of request that were not accepted into the L2Q because the L2Q is FULL. Unit: cpu_atom]
> ...
> 
> The "Unit: cpu_atom" is displayed in the brief description section
> to indicate this is an atom event.

Thanks, applied.

- Arnaldo


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

* Re: [PATCH v2] perf list: Display hybrid pmu events with cpu type
  2021-10-19 21:14 ` Arnaldo Carvalho de Melo
@ 2021-12-15 16:18   ` John Garry
  2021-12-15 17:34     ` John Garry
  0 siblings, 1 reply; 7+ messages in thread
From: John Garry @ 2021-12-15 16:18 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo, Jin Yao
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
	linux-perf-users, ak, kan.liang, yao.jin

On 19/10/2021 22:14, Arnaldo Carvalho de Melo wrote:
> Em Fri, Sep 03, 2021 at 10:52:39AM +0800, Jin Yao escreveu:
>> Add a new option '--cputype' to perf-list to display core-only pmu events
>> or atom-only pmu events.
>>
>> Each hybrid pmu event has been assigned with a pmu name, this patch
>> compares the pmu name before listing the result.
>>
>> For example,
>>
>> perf list --cputype atom
>> ...
>> cache:
>>    core_reject_l2q.any
>>         [Counts the number of request that were not accepted into the L2Q because the L2Q is FULL. Unit: cpu_atom]
>> ...
>>
>> The "Unit: cpu_atom" is displayed in the brief description section
>> to indicate this is an atom event.
> Thanks, applied.

It seems that this buggers "perf list" for uncore events on my arm64 
platform.

Before:

./perf list "uncore ddrc"
uncore ddrc:
   act_cmd
  [DDRC active commands. Unit: hisi_sccl,ddrc]
   flux_rcmd
  [DDRC read commands. Unit: hisi_sccl,ddrc]
   flux_rd
  [DDRC total read operations. Unit: hisi_sccl,ddrc]
   flux_wcmd
  [DDRC write commands. Unit: hisi_sccl,ddrc]
   flux_wr
  [DDRC total write operations. Unit: hisi_sccl,ddrc]
   pre_cmd
  [DDRC precharge commands. Unit: hisi_sccl,ddrc]
   rnk_chg
  [DDRC rank commands. Unit: hisi_sccl,ddrc]
   rw_chg
  [DDRC read and write changes. Unit: hisi_sccl,ddrc]


After:

./perf list "uncore ddrc"

uncore ddrc:
   act_cmd
  [DDRC active commands. Unit: hisi_sccl,ddrc]
   act_cmd
  [DDRC active commands. Unit: hisi_sccl,ddrc]
   act_cmd
  [DDRC active commands. Unit: hisi_sccl,ddrc]
   act_cmd
  [DDRC active commands. Unit: hisi_sccl,ddrc]
   act_cmd
  [DDRC active commands. Unit: hisi_sccl,ddrc]
   act_cmd
  [DDRC active commands. Unit: hisi_sccl,ddrc]
   act_cmd
  [DDRC active commands. Unit: hisi_sccl,ddrc]
   act_cmd
  [DDRC active commands. Unit: hisi_sccl,ddrc]
   act_cmd
  [DDRC active commands. Unit: hisi_sccl,dd

Notice how the events are repeated.

And then gets broken even worse later some point before v5.16-rc6 such 
that aliasing gets broken altogether.

./perf list  | grep hisi_sccl | grep act_cmd|  wc -l
16

Good should be 0.

I'll have a look. Obviously we need a test case to stop such breakages.

Thanks,
John

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

* Re: [PATCH v2] perf list: Display hybrid pmu events with cpu type
  2021-12-15 16:18   ` John Garry
@ 2021-12-15 17:34     ` John Garry
  0 siblings, 0 replies; 7+ messages in thread
From: John Garry @ 2021-12-15 17:34 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: jolsa, peterz, mingo, alexander.shishkin, Linux-kernel,
	linux-perf-users, ak, kan.liang, irogers

On 15/12/2021 16:18, John Garry wrote:

- yao.jin@linux.intel.com, yao.jin@intel.com

Both these author addresses bounce for me :(

And it's not just my arm64 platform which is damaged, but also my x86 
broadwell machine - uncore aliasing for perf list is broken

Before snippet:
unc_cbo_cache_lookup.any_es
[Unit: uncore_cbox L3 Lookup any request that access cache and found 
line in E or S-state]
unc_cbo_cache_lookup.any_i
[Unit: uncore_cbox L3 Lookup any request that access cache and found 
line in I-state]

After snippet:
unc_cbo_cache_lookup.any_es
[Unit: uncore_cbox L3 Lookup any request that access cache and found 
line in E or S-state]
unc_cbo_cache_lookup.any_es
[Unit: uncore_cbox L3 Lookup any request that access cache and found 
line in E or S-state]
unc_cbo_cache_lookup.any_i
[Unit: uncore_cbox L3 Lookup any request that access cache and found 
line in I-state]
unc_cbo_cache_lookup.any_i
[Unit: uncore_cbox L3 Lookup any request that access cache and found 
line in I-state]

Notice how the events are repeated (twice, for each cbox PMU) after, 
when they should not be.

This seems to be the broken code added in print_pmu_events():

 > qsort(aliases, len, sizeof(struct sevent), cmp_sevent);
 > for (j = 0; j < len; j++) {
 > /* Skip duplicates */
 > - if (j > 0 && !strcmp(aliases[j].name, aliases[j - 1].name))
 > - continue;
 > + if (j > 0 && !strcmp(aliases[j].name, aliases[j - 1].name)) {
 > + if (!aliases[j].pmu || !aliases[j - 1].pmu ||
 > + !strcmp(aliases[j].pmu, aliases[j - 1].pmu)) {
 > + continue;
 > + }
 > + }

Anyone an idea on the !strcmp(aliases[j].pmu, aliases[j - 1].pmu) check 
or how to fix it?

Thanks,
John

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

end of thread, other threads:[~2021-12-15 17:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-03  2:52 [PATCH v2] perf list: Display hybrid pmu events with cpu type Jin Yao
2021-09-09 22:37 ` Ian Rogers
2021-09-09 22:56   ` Andi Kleen
2021-09-10  0:39     ` Jin, Yao
2021-10-19 21:14 ` Arnaldo Carvalho de Melo
2021-12-15 16:18   ` John Garry
2021-12-15 17:34     ` John Garry

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