All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] perf arm64 metricgroup support
@ 2021-03-25 10:33 ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

This series contains support to get basic metricgroups working for
arm64 CPUs.

Initial support is added for HiSilicon hip08 platform.

Some sample usage on Huawei D06 board:

 $ ./perf list metric    

List of pre-defined events (to be used in -e): 

Metrics:     

  bp_misp_flush
       [BP misp flush L3 topdown metric]
  branch_mispredicts
       [Branch mispredicts L2 topdown metric]
  core_bound
       [Core bound L2 topdown metric]
  divider
       [Divider L3 topdown metric]
  exe_ports_util
       [EXE ports util L3 topdown metric]
  fetch_bandwidth_bound
       [Fetch bandwidth bound L2 topdown metric]
  fetch_latency_bound
       [Fetch latency bound L2 topdown metric]
  fsu_stall
       [FSU stall L3 topdown metric]
  idle_by_icache_miss

$ sudo ./perf stat -v -M core_bound sleep 1
Using CPUID 0x00000000480fd010
metric expr (exe_stall_cycle - (mem_stall_anyload + armv8_pmuv3_0@event\=0x7005@)) / cpu_cycles for core_bound
found event cpu_cycles
found event armv8_pmuv3_0/event=0x7005/
found event exe_stall_cycle
found event mem_stall_anyload
adding {cpu_cycles -> armv8_pmuv3_0/event=0x7001/
mem_stall_anyload -> armv8_pmuv3_0/event=0x7004/
Control descriptor is not initialized
cpu_cycles: 989433 385050 385050
armv8_pmuv3_0/event=0x7005/: 19207 385050 385050
exe_stall_cycle: 900825 385050 385050
mem_stall_anyload: 253516 385050 385050

Performance counter stats for 'sleep':

989,433      cpu_cycles      #     0.63 core_bound
  19,207      armv8_pmuv3_0/event=0x7005/
 900,825      exe_stall_cycle
 253,516      mem_stall_anyload

       0.000805809 seconds time elapsed

       0.000875000 seconds user
       0.000000000 seconds sys
       
perf stat --topdown is not supported, as this requires the CPU PMU to
expose (alias) events for the TopDown L1 metrics from sysfs, which arm 
does not do. To get that to work, we probably need to make perf use the
pmu-events cpumap to learn about those alias events.

Metric reuse support is added for pmu-events parse metric testcase.
This had been broken on power9 recentlty:
https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/ 

Differences to v1:
- Add pmu_events_map__find() as arm64-specific function
- Fix metric reuse for pmu-events parse metric testcase 

John Garry (6):
  perf metricgroup: Make find_metric() public with name change
  perf test: Handle metric reuse in pmu-events parsing test
  perf pmu: Add pmu_events_map__find()
  perf vendor events arm64: Add Hisi hip08 L1 metrics
  perf vendor events arm64: Add Hisi hip08 L2 metrics
  perf vendor events arm64: Add Hisi hip08 L3 metrics

 tools/perf/arch/arm64/util/Build              |   1 +
 tools/perf/arch/arm64/util/pmu.c              |  25 ++
 .../arch/arm64/hisilicon/hip08/metrics.json   | 233 ++++++++++++++++++
 tools/perf/tests/pmu-events.c                 |  82 +++++-
 tools/perf/util/metricgroup.c                 |  12 +-
 tools/perf/util/metricgroup.h                 |   3 +-
 tools/perf/util/pmu.c                         |   5 +
 tools/perf/util/pmu.h                         |   1 +
 tools/perf/util/s390-sample-raw.c             |   4 +-
 9 files changed, 355 insertions(+), 11 deletions(-)
 create mode 100644 tools/perf/arch/arm64/util/pmu.c
 create mode 100644 tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json

-- 
2.26.2


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

* [PATCH v2 0/6] perf arm64 metricgroup support
@ 2021-03-25 10:33 ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

This series contains support to get basic metricgroups working for
arm64 CPUs.

Initial support is added for HiSilicon hip08 platform.

Some sample usage on Huawei D06 board:

 $ ./perf list metric    

List of pre-defined events (to be used in -e): 

Metrics:     

  bp_misp_flush
       [BP misp flush L3 topdown metric]
  branch_mispredicts
       [Branch mispredicts L2 topdown metric]
  core_bound
       [Core bound L2 topdown metric]
  divider
       [Divider L3 topdown metric]
  exe_ports_util
       [EXE ports util L3 topdown metric]
  fetch_bandwidth_bound
       [Fetch bandwidth bound L2 topdown metric]
  fetch_latency_bound
       [Fetch latency bound L2 topdown metric]
  fsu_stall
       [FSU stall L3 topdown metric]
  idle_by_icache_miss

$ sudo ./perf stat -v -M core_bound sleep 1
Using CPUID 0x00000000480fd010
metric expr (exe_stall_cycle - (mem_stall_anyload + armv8_pmuv3_0@event\=0x7005@)) / cpu_cycles for core_bound
found event cpu_cycles
found event armv8_pmuv3_0/event=0x7005/
found event exe_stall_cycle
found event mem_stall_anyload
adding {cpu_cycles -> armv8_pmuv3_0/event=0x7001/
mem_stall_anyload -> armv8_pmuv3_0/event=0x7004/
Control descriptor is not initialized
cpu_cycles: 989433 385050 385050
armv8_pmuv3_0/event=0x7005/: 19207 385050 385050
exe_stall_cycle: 900825 385050 385050
mem_stall_anyload: 253516 385050 385050

Performance counter stats for 'sleep':

989,433      cpu_cycles      #     0.63 core_bound
  19,207      armv8_pmuv3_0/event=0x7005/
 900,825      exe_stall_cycle
 253,516      mem_stall_anyload

       0.000805809 seconds time elapsed

       0.000875000 seconds user
       0.000000000 seconds sys
       
perf stat --topdown is not supported, as this requires the CPU PMU to
expose (alias) events for the TopDown L1 metrics from sysfs, which arm 
does not do. To get that to work, we probably need to make perf use the
pmu-events cpumap to learn about those alias events.

Metric reuse support is added for pmu-events parse metric testcase.
This had been broken on power9 recentlty:
https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/ 

Differences to v1:
- Add pmu_events_map__find() as arm64-specific function
- Fix metric reuse for pmu-events parse metric testcase 

John Garry (6):
  perf metricgroup: Make find_metric() public with name change
  perf test: Handle metric reuse in pmu-events parsing test
  perf pmu: Add pmu_events_map__find()
  perf vendor events arm64: Add Hisi hip08 L1 metrics
  perf vendor events arm64: Add Hisi hip08 L2 metrics
  perf vendor events arm64: Add Hisi hip08 L3 metrics

 tools/perf/arch/arm64/util/Build              |   1 +
 tools/perf/arch/arm64/util/pmu.c              |  25 ++
 .../arch/arm64/hisilicon/hip08/metrics.json   | 233 ++++++++++++++++++
 tools/perf/tests/pmu-events.c                 |  82 +++++-
 tools/perf/util/metricgroup.c                 |  12 +-
 tools/perf/util/metricgroup.h                 |   3 +-
 tools/perf/util/pmu.c                         |   5 +
 tools/perf/util/pmu.h                         |   1 +
 tools/perf/util/s390-sample-raw.c             |   4 +-
 9 files changed, 355 insertions(+), 11 deletions(-)
 create mode 100644 tools/perf/arch/arm64/util/pmu.c
 create mode 100644 tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json

-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v2 1/6] perf metricgroup: Make find_metric() public with name change
  2021-03-25 10:33 ` John Garry
@ 2021-03-25 10:33   ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

Function find_metric() is required for the metric processing in the
pmu-events testcase, so make it public. Also change the name to include
"metricgroup".

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/util/metricgroup.c | 5 +++--
 tools/perf/util/metricgroup.h | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 6acb44ad439b..71a13406e0bd 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -900,7 +900,8 @@ static int __add_metric(struct list_head *metric_list,
 		    (match_metric(__pe->metric_group, __metric) ||	\
 		     match_metric(__pe->metric_name, __metric)))
 
-static struct pmu_event *find_metric(const char *metric, struct pmu_events_map *map)
+struct pmu_event *metrcgroup_find_metric(const char *metric,
+					 struct pmu_events_map *map)
 {
 	struct pmu_event *pe;
 	int i;
@@ -985,7 +986,7 @@ static int __resolve_metric(struct metric *m,
 			struct expr_id *parent;
 			struct pmu_event *pe;
 
-			pe = find_metric(cur->key, map);
+			pe = metrcgroup_find_metric(cur->key, map);
 			if (!pe)
 				continue;
 
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index ed1b9392e624..1674c6a36d74 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -44,7 +44,8 @@ int metricgroup__parse_groups(const struct option *opt,
 			      bool metric_no_group,
 			      bool metric_no_merge,
 			      struct rblist *metric_events);
-
+struct pmu_event *metrcgroup_find_metric(const char *metric,
+					 struct pmu_events_map *map);
 int metricgroup__parse_groups_test(struct evlist *evlist,
 				   struct pmu_events_map *map,
 				   const char *str,
-- 
2.26.2


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

* [PATCH v2 1/6] perf metricgroup: Make find_metric() public with name change
@ 2021-03-25 10:33   ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

Function find_metric() is required for the metric processing in the
pmu-events testcase, so make it public. Also change the name to include
"metricgroup".

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/util/metricgroup.c | 5 +++--
 tools/perf/util/metricgroup.h | 3 ++-
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 6acb44ad439b..71a13406e0bd 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -900,7 +900,8 @@ static int __add_metric(struct list_head *metric_list,
 		    (match_metric(__pe->metric_group, __metric) ||	\
 		     match_metric(__pe->metric_name, __metric)))
 
-static struct pmu_event *find_metric(const char *metric, struct pmu_events_map *map)
+struct pmu_event *metrcgroup_find_metric(const char *metric,
+					 struct pmu_events_map *map)
 {
 	struct pmu_event *pe;
 	int i;
@@ -985,7 +986,7 @@ static int __resolve_metric(struct metric *m,
 			struct expr_id *parent;
 			struct pmu_event *pe;
 
-			pe = find_metric(cur->key, map);
+			pe = metrcgroup_find_metric(cur->key, map);
 			if (!pe)
 				continue;
 
diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
index ed1b9392e624..1674c6a36d74 100644
--- a/tools/perf/util/metricgroup.h
+++ b/tools/perf/util/metricgroup.h
@@ -44,7 +44,8 @@ int metricgroup__parse_groups(const struct option *opt,
 			      bool metric_no_group,
 			      bool metric_no_merge,
 			      struct rblist *metric_events);
-
+struct pmu_event *metrcgroup_find_metric(const char *metric,
+					 struct pmu_events_map *map);
 int metricgroup__parse_groups_test(struct evlist *evlist,
 				   struct pmu_events_map *map,
 				   const char *str,
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
  2021-03-25 10:33 ` John Garry
@ 2021-03-25 10:33   ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

The pmu-events parsing test does not handle metric reuse at all.

Introduce some simple handling to resolve metrics who reference other
metrics.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/tests/pmu-events.c | 80 +++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index 0ca6a5a53523..20b6bf14f7f7 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -12,6 +12,7 @@
 #include "util/evlist.h"
 #include "util/expr.h"
 #include "util/parse-events.h"
+#include "metricgroup.h"
 
 struct perf_pmu_test_event {
 	/* used for matching against events from generated pmu-events.c */
@@ -471,6 +472,70 @@ static void expr_failure(const char *msg,
 	pr_debug("On expression %s\n", pe->metric_expr);
 }
 
+struct metric {
+	struct list_head list;
+	struct metric_ref metric_ref;
+};
+
+static int resolve_metric_simple(struct expr_parse_ctx *pctx,
+				 struct list_head *compound_list,
+				 struct pmu_events_map *map,
+				 const char *metric_name)
+{
+	struct hashmap_entry *cur, *cur_tmp;
+	struct metric *metric, *tmp;
+	size_t bkt;
+	bool all;
+	int rc;
+
+	do {
+		all = true;
+		hashmap__for_each_entry_safe((&pctx->ids), cur, cur_tmp, bkt) {
+			struct metric_ref *ref;
+			struct pmu_event *pe;
+
+			pe = metrcgroup_find_metric(cur->key, map);
+			if (!pe)
+				continue;
+
+			if (!strcmp(metric_name, (char *)cur->key)) {
+				pr_warning("Recursion detected for metric %s\n", metric_name);
+				rc = -1;
+				goto out_err;
+			}
+
+			all = false;
+
+			/* The metric key itself needs to go out.. */
+			expr__del_id(pctx, cur->key);
+
+			metric = malloc(sizeof(*metric));
+			if (!metric) {
+				rc = -ENOMEM;
+				goto out_err;
+			}
+
+			ref = &metric->metric_ref;
+			ref->metric_name = pe->metric_name;
+			ref->metric_expr = pe->metric_expr;
+			list_add_tail(&metric->list, compound_list);
+
+			rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
+			if (rc)
+				goto out_err;
+		}
+	} while (!all);
+
+	return 0;
+
+out_err:
+	list_for_each_entry_safe(metric, tmp, compound_list, list)
+		free(metric);
+
+	return rc;
+
+}
+
 static int test_parsing(void)
 {
 	struct pmu_events_map *cpus_map = perf_pmu__find_map(NULL);
@@ -488,7 +553,9 @@ static int test_parsing(void)
 			break;
 		j = 0;
 		for (;;) {
+			struct metric *metric, *tmp;
 			struct hashmap_entry *cur;
+			LIST_HEAD(compound_list);
 			size_t bkt;
 
 			pe = &map->table[j++];
@@ -504,6 +571,13 @@ static int test_parsing(void)
 				continue;
 			}
 
+			if (resolve_metric_simple(&ctx, &compound_list, map,
+						  pe->metric_name)) {
+				expr_failure("Could not resolve metrics", map, pe);
+				ret++;
+				goto exit; /* Don't tolerate errors due to severity */
+			}
+
 			/*
 			 * Add all ids with a made up value. The value may
 			 * trigger divide by zero when subtracted and so try to
@@ -519,6 +593,11 @@ static int test_parsing(void)
 					ret++;
 			}
 
+			list_for_each_entry_safe(metric, tmp, &compound_list, list) {
+				expr__add_ref(&ctx, &metric->metric_ref);
+				free(metric);
+			}
+
 			if (expr__parse(&result, &ctx, pe->metric_expr, 0)) {
 				expr_failure("Parse failed", map, pe);
 				ret++;
@@ -527,6 +606,7 @@ static int test_parsing(void)
 		}
 	}
 	/* TODO: fail when not ok */
+exit:
 	return ret == 0 ? TEST_OK : TEST_SKIP;
 }
 
-- 
2.26.2


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

* [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
@ 2021-03-25 10:33   ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

The pmu-events parsing test does not handle metric reuse at all.

Introduce some simple handling to resolve metrics who reference other
metrics.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/tests/pmu-events.c | 80 +++++++++++++++++++++++++++++++++++
 1 file changed, 80 insertions(+)

diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index 0ca6a5a53523..20b6bf14f7f7 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -12,6 +12,7 @@
 #include "util/evlist.h"
 #include "util/expr.h"
 #include "util/parse-events.h"
+#include "metricgroup.h"
 
 struct perf_pmu_test_event {
 	/* used for matching against events from generated pmu-events.c */
@@ -471,6 +472,70 @@ static void expr_failure(const char *msg,
 	pr_debug("On expression %s\n", pe->metric_expr);
 }
 
+struct metric {
+	struct list_head list;
+	struct metric_ref metric_ref;
+};
+
+static int resolve_metric_simple(struct expr_parse_ctx *pctx,
+				 struct list_head *compound_list,
+				 struct pmu_events_map *map,
+				 const char *metric_name)
+{
+	struct hashmap_entry *cur, *cur_tmp;
+	struct metric *metric, *tmp;
+	size_t bkt;
+	bool all;
+	int rc;
+
+	do {
+		all = true;
+		hashmap__for_each_entry_safe((&pctx->ids), cur, cur_tmp, bkt) {
+			struct metric_ref *ref;
+			struct pmu_event *pe;
+
+			pe = metrcgroup_find_metric(cur->key, map);
+			if (!pe)
+				continue;
+
+			if (!strcmp(metric_name, (char *)cur->key)) {
+				pr_warning("Recursion detected for metric %s\n", metric_name);
+				rc = -1;
+				goto out_err;
+			}
+
+			all = false;
+
+			/* The metric key itself needs to go out.. */
+			expr__del_id(pctx, cur->key);
+
+			metric = malloc(sizeof(*metric));
+			if (!metric) {
+				rc = -ENOMEM;
+				goto out_err;
+			}
+
+			ref = &metric->metric_ref;
+			ref->metric_name = pe->metric_name;
+			ref->metric_expr = pe->metric_expr;
+			list_add_tail(&metric->list, compound_list);
+
+			rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
+			if (rc)
+				goto out_err;
+		}
+	} while (!all);
+
+	return 0;
+
+out_err:
+	list_for_each_entry_safe(metric, tmp, compound_list, list)
+		free(metric);
+
+	return rc;
+
+}
+
 static int test_parsing(void)
 {
 	struct pmu_events_map *cpus_map = perf_pmu__find_map(NULL);
@@ -488,7 +553,9 @@ static int test_parsing(void)
 			break;
 		j = 0;
 		for (;;) {
+			struct metric *metric, *tmp;
 			struct hashmap_entry *cur;
+			LIST_HEAD(compound_list);
 			size_t bkt;
 
 			pe = &map->table[j++];
@@ -504,6 +571,13 @@ static int test_parsing(void)
 				continue;
 			}
 
+			if (resolve_metric_simple(&ctx, &compound_list, map,
+						  pe->metric_name)) {
+				expr_failure("Could not resolve metrics", map, pe);
+				ret++;
+				goto exit; /* Don't tolerate errors due to severity */
+			}
+
 			/*
 			 * Add all ids with a made up value. The value may
 			 * trigger divide by zero when subtracted and so try to
@@ -519,6 +593,11 @@ static int test_parsing(void)
 					ret++;
 			}
 
+			list_for_each_entry_safe(metric, tmp, &compound_list, list) {
+				expr__add_ref(&ctx, &metric->metric_ref);
+				free(metric);
+			}
+
 			if (expr__parse(&result, &ctx, pe->metric_expr, 0)) {
 				expr_failure("Parse failed", map, pe);
 				ret++;
@@ -527,6 +606,7 @@ static int test_parsing(void)
 		}
 	}
 	/* TODO: fail when not ok */
+exit:
 	return ret == 0 ? TEST_OK : TEST_SKIP;
 }
 
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v2 3/6] perf pmu: Add pmu_events_map__find()
  2021-03-25 10:33 ` John Garry
@ 2021-03-25 10:33   ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

Add a function to find the common PMU map for the system.

For arm64, a special variant is added. This is because arm64 supports
heterogeneous CPU systems. As such, it cannot be guaranteed that the cpumap
is same for all CPUs. So in case of heterogeneous systems, don't return
a cpumap.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/arch/arm64/util/Build  |  1 +
 tools/perf/arch/arm64/util/pmu.c  | 25 +++++++++++++++++++++++++
 tools/perf/tests/pmu-events.c     |  2 +-
 tools/perf/util/metricgroup.c     |  7 +++----
 tools/perf/util/pmu.c             |  5 +++++
 tools/perf/util/pmu.h             |  1 +
 tools/perf/util/s390-sample-raw.c |  4 +---
 7 files changed, 37 insertions(+), 8 deletions(-)
 create mode 100644 tools/perf/arch/arm64/util/pmu.c

diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
index ead2f2275eee..9fcb4e68add9 100644
--- a/tools/perf/arch/arm64/util/Build
+++ b/tools/perf/arch/arm64/util/Build
@@ -2,6 +2,7 @@ perf-y += header.o
 perf-y += machine.o
 perf-y += perf_regs.o
 perf-y += tsc.o
+perf-y += pmu.o
 perf-y += kvm-stat.o
 perf-$(CONFIG_DWARF)     += dwarf-regs.o
 perf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o
diff --git a/tools/perf/arch/arm64/util/pmu.c b/tools/perf/arch/arm64/util/pmu.c
new file mode 100644
index 000000000000..d3259d61ca75
--- /dev/null
+++ b/tools/perf/arch/arm64/util/pmu.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "../../util/cpumap.h"
+#include "../../util/pmu.h"
+
+struct pmu_events_map *pmu_events_map__find(void)
+{
+	struct perf_pmu *pmu = NULL;
+
+	while ((pmu = perf_pmu__scan(pmu))) {
+		if (!is_pmu_core(pmu->name))
+			continue;
+
+		/*
+		 * The cpumap should cover all CPUs. Otherwise, some CPUs may
+		 * not support some events or have different event IDs.
+		 */
+		if (pmu->cpus->nr != cpu__max_cpu())
+			return NULL;
+
+		return perf_pmu__find_map(pmu);
+	}
+
+	return NULL;
+}
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index 20b6bf14f7f7..9fe2455b355b 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -538,7 +538,7 @@ static int resolve_metric_simple(struct expr_parse_ctx *pctx,
 
 static int test_parsing(void)
 {
-	struct pmu_events_map *cpus_map = perf_pmu__find_map(NULL);
+	struct pmu_events_map *cpus_map = pmu_events_map__find();
 	struct pmu_events_map *map;
 	struct pmu_event *pe;
 	int i, j, k;
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 71a13406e0bd..e6e787e94f91 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -618,7 +618,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)
 {
-	struct pmu_events_map *map = perf_pmu__find_map(NULL);
+	struct pmu_events_map *map = pmu_events_map__find();
 	struct pmu_event *pe;
 	int i;
 	struct rblist groups;
@@ -1254,8 +1254,7 @@ int metricgroup__parse_groups(const struct option *opt,
 			      struct rblist *metric_events)
 {
 	struct evlist *perf_evlist = *(struct evlist **)opt->value;
-	struct pmu_events_map *map = perf_pmu__find_map(NULL);
-
+	struct pmu_events_map *map = pmu_events_map__find();
 
 	return parse_groups(perf_evlist, str, metric_no_group,
 			    metric_no_merge, NULL, metric_events, map);
@@ -1274,7 +1273,7 @@ int metricgroup__parse_groups_test(struct evlist *evlist,
 
 bool metricgroup__has_metric(const char *metric)
 {
-	struct pmu_events_map *map = perf_pmu__find_map(NULL);
+	struct pmu_events_map *map = pmu_events_map__find();
 	struct pmu_event *pe;
 	int i;
 
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 88da5cf6aee8..419ef6c4fbc0 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -717,6 +717,11 @@ struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu)
 	return map;
 }
 
+struct pmu_events_map *__weak pmu_events_map__find(void)
+{
+	return perf_pmu__find_map(NULL);
+}
+
 bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
 {
 	char *tmp = NULL, *tok, *str;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 8164388478c6..012317229488 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -114,6 +114,7 @@ void pmu_add_cpu_aliases_map(struct list_head *head, struct perf_pmu *pmu,
 			     struct pmu_events_map *map);
 
 struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu);
+struct pmu_events_map *pmu_events_map__find(void);
 bool pmu_uncore_alias_match(const char *pmu_name, const char *name);
 void perf_pmu_free_alias(struct perf_pmu_alias *alias);
 
diff --git a/tools/perf/util/s390-sample-raw.c b/tools/perf/util/s390-sample-raw.c
index cfcf8d534d76..08ec3c3ae0ee 100644
--- a/tools/perf/util/s390-sample-raw.c
+++ b/tools/perf/util/s390-sample-raw.c
@@ -160,11 +160,9 @@ static void s390_cpumcfdg_dump(struct perf_sample *sample)
 	const char *color = PERF_COLOR_BLUE;
 	struct cf_ctrset_entry *cep, ce;
 	struct pmu_events_map *map;
-	struct perf_pmu pmu;
 	u64 *p;
 
-	memset(&pmu, 0, sizeof(pmu));
-	map = perf_pmu__find_map(&pmu);
+	map = pmu_events_map__find();
 	while (offset < len) {
 		cep = (struct cf_ctrset_entry *)(buf + offset);
 
-- 
2.26.2


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

* [PATCH v2 3/6] perf pmu: Add pmu_events_map__find()
@ 2021-03-25 10:33   ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

Add a function to find the common PMU map for the system.

For arm64, a special variant is added. This is because arm64 supports
heterogeneous CPU systems. As such, it cannot be guaranteed that the cpumap
is same for all CPUs. So in case of heterogeneous systems, don't return
a cpumap.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/arch/arm64/util/Build  |  1 +
 tools/perf/arch/arm64/util/pmu.c  | 25 +++++++++++++++++++++++++
 tools/perf/tests/pmu-events.c     |  2 +-
 tools/perf/util/metricgroup.c     |  7 +++----
 tools/perf/util/pmu.c             |  5 +++++
 tools/perf/util/pmu.h             |  1 +
 tools/perf/util/s390-sample-raw.c |  4 +---
 7 files changed, 37 insertions(+), 8 deletions(-)
 create mode 100644 tools/perf/arch/arm64/util/pmu.c

diff --git a/tools/perf/arch/arm64/util/Build b/tools/perf/arch/arm64/util/Build
index ead2f2275eee..9fcb4e68add9 100644
--- a/tools/perf/arch/arm64/util/Build
+++ b/tools/perf/arch/arm64/util/Build
@@ -2,6 +2,7 @@ perf-y += header.o
 perf-y += machine.o
 perf-y += perf_regs.o
 perf-y += tsc.o
+perf-y += pmu.o
 perf-y += kvm-stat.o
 perf-$(CONFIG_DWARF)     += dwarf-regs.o
 perf-$(CONFIG_LOCAL_LIBUNWIND) += unwind-libunwind.o
diff --git a/tools/perf/arch/arm64/util/pmu.c b/tools/perf/arch/arm64/util/pmu.c
new file mode 100644
index 000000000000..d3259d61ca75
--- /dev/null
+++ b/tools/perf/arch/arm64/util/pmu.c
@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include "../../util/cpumap.h"
+#include "../../util/pmu.h"
+
+struct pmu_events_map *pmu_events_map__find(void)
+{
+	struct perf_pmu *pmu = NULL;
+
+	while ((pmu = perf_pmu__scan(pmu))) {
+		if (!is_pmu_core(pmu->name))
+			continue;
+
+		/*
+		 * The cpumap should cover all CPUs. Otherwise, some CPUs may
+		 * not support some events or have different event IDs.
+		 */
+		if (pmu->cpus->nr != cpu__max_cpu())
+			return NULL;
+
+		return perf_pmu__find_map(pmu);
+	}
+
+	return NULL;
+}
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
index 20b6bf14f7f7..9fe2455b355b 100644
--- a/tools/perf/tests/pmu-events.c
+++ b/tools/perf/tests/pmu-events.c
@@ -538,7 +538,7 @@ static int resolve_metric_simple(struct expr_parse_ctx *pctx,
 
 static int test_parsing(void)
 {
-	struct pmu_events_map *cpus_map = perf_pmu__find_map(NULL);
+	struct pmu_events_map *cpus_map = pmu_events_map__find();
 	struct pmu_events_map *map;
 	struct pmu_event *pe;
 	int i, j, k;
diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 71a13406e0bd..e6e787e94f91 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -618,7 +618,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)
 {
-	struct pmu_events_map *map = perf_pmu__find_map(NULL);
+	struct pmu_events_map *map = pmu_events_map__find();
 	struct pmu_event *pe;
 	int i;
 	struct rblist groups;
@@ -1254,8 +1254,7 @@ int metricgroup__parse_groups(const struct option *opt,
 			      struct rblist *metric_events)
 {
 	struct evlist *perf_evlist = *(struct evlist **)opt->value;
-	struct pmu_events_map *map = perf_pmu__find_map(NULL);
-
+	struct pmu_events_map *map = pmu_events_map__find();
 
 	return parse_groups(perf_evlist, str, metric_no_group,
 			    metric_no_merge, NULL, metric_events, map);
@@ -1274,7 +1273,7 @@ int metricgroup__parse_groups_test(struct evlist *evlist,
 
 bool metricgroup__has_metric(const char *metric)
 {
-	struct pmu_events_map *map = perf_pmu__find_map(NULL);
+	struct pmu_events_map *map = pmu_events_map__find();
 	struct pmu_event *pe;
 	int i;
 
diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 88da5cf6aee8..419ef6c4fbc0 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -717,6 +717,11 @@ struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu)
 	return map;
 }
 
+struct pmu_events_map *__weak pmu_events_map__find(void)
+{
+	return perf_pmu__find_map(NULL);
+}
+
 bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
 {
 	char *tmp = NULL, *tok, *str;
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 8164388478c6..012317229488 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -114,6 +114,7 @@ void pmu_add_cpu_aliases_map(struct list_head *head, struct perf_pmu *pmu,
 			     struct pmu_events_map *map);
 
 struct pmu_events_map *perf_pmu__find_map(struct perf_pmu *pmu);
+struct pmu_events_map *pmu_events_map__find(void);
 bool pmu_uncore_alias_match(const char *pmu_name, const char *name);
 void perf_pmu_free_alias(struct perf_pmu_alias *alias);
 
diff --git a/tools/perf/util/s390-sample-raw.c b/tools/perf/util/s390-sample-raw.c
index cfcf8d534d76..08ec3c3ae0ee 100644
--- a/tools/perf/util/s390-sample-raw.c
+++ b/tools/perf/util/s390-sample-raw.c
@@ -160,11 +160,9 @@ static void s390_cpumcfdg_dump(struct perf_sample *sample)
 	const char *color = PERF_COLOR_BLUE;
 	struct cf_ctrset_entry *cep, ce;
 	struct pmu_events_map *map;
-	struct perf_pmu pmu;
 	u64 *p;
 
-	memset(&pmu, 0, sizeof(pmu));
-	map = perf_pmu__find_map(&pmu);
+	map = pmu_events_map__find();
 	while (offset < len) {
 		cep = (struct cf_ctrset_entry *)(buf + offset);
 
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v2 4/6] perf vendor events arm64: Add Hisi hip08 L1 metrics
  2021-03-25 10:33 ` John Garry
@ 2021-03-25 10:33   ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

Add L1 metrics. Formula is as consistent as possible with MAN pages
description for these metrics.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 .../arch/arm64/hisilicon/hip08/metrics.json   | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json

diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
new file mode 100644
index 000000000000..dc5ff3051639
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
@@ -0,0 +1,30 @@
+[
+    {
+        "MetricExpr": "FETCH_BUBBLE / (4 * CPU_CYCLES)",
+        "PublicDescription": "Frontend bound L1 topdown metric",
+        "BriefDescription": "Frontend bound L1 topdown metric",
+        "MetricGroup": "TopDownL1",
+        "MetricName": "frontend_bound"
+    },
+    {
+        "MetricExpr": "(INST_SPEC - INST_RETIRED) / (4 * CPU_CYCLES)",
+        "PublicDescription": "Bad Speculation L1 topdown metric",
+        "BriefDescription": "Bad Speculation L1 topdown metric",
+        "MetricGroup": "TopDownL1",
+        "MetricName": "bad_speculation"
+    },
+    {
+        "MetricExpr": "INST_RETIRED / (CPU_CYCLES * 4)",
+        "PublicDescription": "Retiring L1 topdown metric",
+        "BriefDescription": "Retiring L1 topdown metric",
+        "MetricGroup": "TopDownL1",
+        "MetricName": "retiring"
+    },
+    {
+        "MetricExpr": "1 - (frontend_bound + bad_speculation + retiring)",
+        "PublicDescription": "Backend Bound L1 topdown metric",
+        "BriefDescription": "Backend Bound L1 topdown metric",
+        "MetricGroup": "TopDownL1",
+        "MetricName": "backend_bound"
+    },
+]
-- 
2.26.2


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

* [PATCH v2 4/6] perf vendor events arm64: Add Hisi hip08 L1 metrics
@ 2021-03-25 10:33   ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

Add L1 metrics. Formula is as consistent as possible with MAN pages
description for these metrics.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 .../arch/arm64/hisilicon/hip08/metrics.json   | 30 +++++++++++++++++++
 1 file changed, 30 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json

diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
new file mode 100644
index 000000000000..dc5ff3051639
--- /dev/null
+++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
@@ -0,0 +1,30 @@
+[
+    {
+        "MetricExpr": "FETCH_BUBBLE / (4 * CPU_CYCLES)",
+        "PublicDescription": "Frontend bound L1 topdown metric",
+        "BriefDescription": "Frontend bound L1 topdown metric",
+        "MetricGroup": "TopDownL1",
+        "MetricName": "frontend_bound"
+    },
+    {
+        "MetricExpr": "(INST_SPEC - INST_RETIRED) / (4 * CPU_CYCLES)",
+        "PublicDescription": "Bad Speculation L1 topdown metric",
+        "BriefDescription": "Bad Speculation L1 topdown metric",
+        "MetricGroup": "TopDownL1",
+        "MetricName": "bad_speculation"
+    },
+    {
+        "MetricExpr": "INST_RETIRED / (CPU_CYCLES * 4)",
+        "PublicDescription": "Retiring L1 topdown metric",
+        "BriefDescription": "Retiring L1 topdown metric",
+        "MetricGroup": "TopDownL1",
+        "MetricName": "retiring"
+    },
+    {
+        "MetricExpr": "1 - (frontend_bound + bad_speculation + retiring)",
+        "PublicDescription": "Backend Bound L1 topdown metric",
+        "BriefDescription": "Backend Bound L1 topdown metric",
+        "MetricGroup": "TopDownL1",
+        "MetricName": "backend_bound"
+    },
+]
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v2 5/6] perf vendor events arm64: Add Hisi hip08 L2 metrics
  2021-03-25 10:33 ` John Garry
@ 2021-03-25 10:33   ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

Add L2 metrics.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 .../arch/arm64/hisilicon/hip08/metrics.json   | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
index dc5ff3051639..dda898d23c2d 100644
--- a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
+++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
@@ -27,4 +27,46 @@
         "MetricGroup": "TopDownL1",
         "MetricName": "backend_bound"
     },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x201d@ / CPU_CYCLES",
+        "PublicDescription": "Fetch latency bound L2 topdown metric",
+        "BriefDescription": "Fetch latency bound L2 topdown metric",
+        "MetricGroup": "TopDownL2",
+        "MetricName": "fetch_latency_bound"
+    },
+    {
+        "MetricExpr": "frontend_bound - fetch_latency_bound",
+        "PublicDescription": "Fetch bandwidth bound L2 topdown metric",
+        "BriefDescription": "Fetch bandwidth bound L2 topdown metric",
+        "MetricGroup": "TopDownL2",
+        "MetricName": "fetch_bandwidth_bound"
+    },
+    {
+        "MetricExpr": "(bad_speculation * BR_MIS_PRED) / (BR_MIS_PRED + armv8_pmuv3_0@event\\=0x2013@)",
+        "PublicDescription": "Branch mispredicts L2 topdown metric",
+        "BriefDescription": "Branch mispredicts L2 topdown metric",
+        "MetricGroup": "TopDownL2",
+        "MetricName": "branch_mispredicts"
+    },
+    {
+        "MetricExpr": "bad_speculation - branch_mispredicts",
+        "PublicDescription": "Machine clears L2 topdown metric",
+        "BriefDescription": "Machine clears L2 topdown metric",
+        "MetricGroup": "TopDownL2",
+        "MetricName": "machine_clears"
+    },
+    {
+        "MetricExpr": "(EXE_STALL_CYCLE - (MEM_STALL_ANYLOAD + armv8_pmuv3_0@event\\=0x7005@)) / CPU_CYCLES",
+        "PublicDescription": "Core bound L2 topdown metric",
+        "BriefDescription": "Core bound L2 topdown metric",
+        "MetricGroup": "TopDownL2",
+        "MetricName": "core_bound"
+    },
+    {
+        "MetricExpr": "(MEM_STALL_ANYLOAD + armv8_pmuv3_0@event\\=0x7005@) / CPU_CYCLES",
+        "PublicDescription": "Memory bound L2 topdown metric",
+        "BriefDescription": "Memory bound L2 topdown metric",
+        "MetricGroup": "TopDownL2",
+        "MetricName": "memory_bound"
+    },
 ]
-- 
2.26.2


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

* [PATCH v2 5/6] perf vendor events arm64: Add Hisi hip08 L2 metrics
@ 2021-03-25 10:33   ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

Add L2 metrics.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 .../arch/arm64/hisilicon/hip08/metrics.json   | 42 +++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
index dc5ff3051639..dda898d23c2d 100644
--- a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
+++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
@@ -27,4 +27,46 @@
         "MetricGroup": "TopDownL1",
         "MetricName": "backend_bound"
     },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x201d@ / CPU_CYCLES",
+        "PublicDescription": "Fetch latency bound L2 topdown metric",
+        "BriefDescription": "Fetch latency bound L2 topdown metric",
+        "MetricGroup": "TopDownL2",
+        "MetricName": "fetch_latency_bound"
+    },
+    {
+        "MetricExpr": "frontend_bound - fetch_latency_bound",
+        "PublicDescription": "Fetch bandwidth bound L2 topdown metric",
+        "BriefDescription": "Fetch bandwidth bound L2 topdown metric",
+        "MetricGroup": "TopDownL2",
+        "MetricName": "fetch_bandwidth_bound"
+    },
+    {
+        "MetricExpr": "(bad_speculation * BR_MIS_PRED) / (BR_MIS_PRED + armv8_pmuv3_0@event\\=0x2013@)",
+        "PublicDescription": "Branch mispredicts L2 topdown metric",
+        "BriefDescription": "Branch mispredicts L2 topdown metric",
+        "MetricGroup": "TopDownL2",
+        "MetricName": "branch_mispredicts"
+    },
+    {
+        "MetricExpr": "bad_speculation - branch_mispredicts",
+        "PublicDescription": "Machine clears L2 topdown metric",
+        "BriefDescription": "Machine clears L2 topdown metric",
+        "MetricGroup": "TopDownL2",
+        "MetricName": "machine_clears"
+    },
+    {
+        "MetricExpr": "(EXE_STALL_CYCLE - (MEM_STALL_ANYLOAD + armv8_pmuv3_0@event\\=0x7005@)) / CPU_CYCLES",
+        "PublicDescription": "Core bound L2 topdown metric",
+        "BriefDescription": "Core bound L2 topdown metric",
+        "MetricGroup": "TopDownL2",
+        "MetricName": "core_bound"
+    },
+    {
+        "MetricExpr": "(MEM_STALL_ANYLOAD + armv8_pmuv3_0@event\\=0x7005@) / CPU_CYCLES",
+        "PublicDescription": "Memory bound L2 topdown metric",
+        "BriefDescription": "Memory bound L2 topdown metric",
+        "MetricGroup": "TopDownL2",
+        "MetricName": "memory_bound"
+    },
 ]
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* [PATCH v2 6/6] perf vendor events arm64: Add Hisi hip08 L3 metrics
  2021-03-25 10:33 ` John Garry
@ 2021-03-25 10:33   ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

Add L3 metrics.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 .../arch/arm64/hisilicon/hip08/metrics.json   | 161 ++++++++++++++++++
 1 file changed, 161 insertions(+)

diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
index dda898d23c2d..dda8e59149d2 100644
--- a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
+++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
@@ -69,4 +69,165 @@
         "MetricGroup": "TopDownL2",
         "MetricName": "memory_bound"
     },
+    {
+        "MetricExpr": "(((L2I_TLB - L2I_TLB_REFILL) * 15) + (L2I_TLB_REFILL * 100)) / CPU_CYCLES",
+        "PublicDescription": "Idle by itlb miss L3 topdown metric",
+        "BriefDescription": "Idle by itlb miss L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "idle_by_itlb_miss"
+    },
+    {
+        "MetricExpr": "(((L2I_CACHE - L2I_CACHE_REFILL) * 15) + (L2I_CACHE_REFILL * 100)) / CPU_CYCLES",
+        "PublicDescription": "Idle by icache miss L3 topdown metric",
+        "BriefDescription": "Idle by icache miss L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "idle_by_icache_miss"
+    },
+    {
+        "MetricExpr": "(BR_MIS_PRED * 5) / CPU_CYCLES",
+        "PublicDescription": "BP misp flush L3 topdown metric",
+        "BriefDescription": "BP misp flush L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "bp_misp_flush"
+    },
+    {
+        "MetricExpr": "(armv8_pmuv3_0@event\\=0x2013@ * 5) / CPU_CYCLES",
+        "PublicDescription": "OOO flush L3 topdown metric",
+        "BriefDescription": "OOO flush L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "ooo_flush"
+    },
+    {
+        "MetricExpr": "(armv8_pmuv3_0@event\\=0x1001@ * 5) / CPU_CYCLES",
+        "PublicDescription": "Static predictor flush L3 topdown metric",
+        "BriefDescription": "Static predictor flush L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "sp_flush"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x1010@ / BR_MIS_PRED",
+        "PublicDescription": "Indirect branch L3 topdown metric",
+        "BriefDescription": "Indirect branch L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "indirect_branch"
+    },
+    {
+        "MetricExpr": "(armv8_pmuv3_0@event\\=0x1014@ + armv8_pmuv3_0@event\\=0x1018@) / BR_MIS_PRED",
+        "PublicDescription": "Push branch L3 topdown metric",
+        "BriefDescription": "Push branch L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "push_branch"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x100c@ / BR_MIS_PRED",
+        "PublicDescription": "Pop branch L3 topdown metric",
+        "BriefDescription": "Pop branch L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "pop_branch"
+    },
+    {
+        "MetricExpr": "(BR_MIS_PRED - armv8_pmuv3_0@event\\=0x1010@ - armv8_pmuv3_0@event\\=0x1014@ - armv8_pmuv3_0@event\\=0x1018@ - armv8_pmuv3_0@event\\=0x100c@) / BR_MIS_PRED",
+        "PublicDescription": "Other branch L3 topdown metric",
+        "BriefDescription": "Other branch L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "other_branch"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x2012@ / armv8_pmuv3_0@event\\=0x2013@",
+        "PublicDescription": "Nuke flush L3 topdown metric",
+        "BriefDescription": "Nuke flush L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "nuke_flush"
+    },
+    {
+        "MetricExpr": "1 - nuke_flush",
+        "PublicDescription": "Other flush L3 topdown metric",
+        "BriefDescription": "Other flush L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "other_flush"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x2010@ / CPU_CYCLES",
+        "PublicDescription": "Sync stall L3 topdown metric",
+        "BriefDescription": "Sync stall L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "sync_stall"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x2004@ / CPU_CYCLES",
+        "PublicDescription": "Rob stall L3 topdown metric",
+        "BriefDescription": "Rob stall L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "rob_stall"
+    },
+    {
+        "MetricExpr": "(armv8_pmuv3_0@event\\=0x2006@ + armv8_pmuv3_0@event\\=0x2007@ + armv8_pmuv3_0@event\\=0x2008@) / CPU_CYCLES",
+        "PublicDescription": "Ptag stall L3 topdown metric",
+        "BriefDescription": "Ptag stall L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "ptag_stall"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x201e@ / CPU_CYCLES",
+        "PublicDescription": "SaveOpQ stall L3 topdown metric",
+        "BriefDescription": "SaveOpQ stall L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "saveopq_stall"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x2005@ / CPU_CYCLES",
+        "PublicDescription": "PC buffer stall L3 topdown metric",
+        "BriefDescription": "PC buffer stall L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "pc_buffer_stall"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x7002@ / CPU_CYCLES",
+        "PublicDescription": "Divider L3 topdown metric",
+        "BriefDescription": "Divider L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "divider"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x7003@ / CPU_CYCLES",
+        "PublicDescription": "FSU stall L3 topdown metric",
+        "BriefDescription": "FSU stall L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "fsu_stall"
+    },
+    {
+        "MetricExpr": "core_bound - divider - fsu_stall",
+        "PublicDescription": "EXE ports util L3 topdown metric",
+        "BriefDescription": "EXE ports util L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "exe_ports_util"
+    },
+    {
+        "MetricExpr": "(MEM_STALL_ANYLOAD - MEM_STALL_L1MISS) / CPU_CYCLES",
+        "PublicDescription": "L1 bound L3 topdown metric",
+        "BriefDescription": "L1 bound L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "l1_bound"
+    },
+    {
+        "MetricExpr": "(MEM_STALL_L1MISS - MEM_STALL_L2MISS) / CPU_CYCLES",
+        "PublicDescription": "L2 bound L3 topdown metric",
+        "BriefDescription": "L2 bound L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "l2_bound"
+    },
+    {
+        "MetricExpr": "MEM_STALL_L2MISS / CPU_CYCLES",
+        "PublicDescription": "Mem bound L3 topdown metric",
+        "BriefDescription": "Mem bound L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "mem_bound"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x7005@ / CPU_CYCLES",
+        "PublicDescription": "Store bound L3 topdown metric",
+        "BriefDescription": "Store bound L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "store_bound"
+    },
 ]
-- 
2.26.2


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

* [PATCH v2 6/6] perf vendor events arm64: Add Hisi hip08 L3 metrics
@ 2021-03-25 10:33   ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-25 10:33 UTC (permalink / raw)
  To: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun, pc, John Garry

Add L3 metrics.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 .../arch/arm64/hisilicon/hip08/metrics.json   | 161 ++++++++++++++++++
 1 file changed, 161 insertions(+)

diff --git a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
index dda898d23c2d..dda8e59149d2 100644
--- a/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
+++ b/tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
@@ -69,4 +69,165 @@
         "MetricGroup": "TopDownL2",
         "MetricName": "memory_bound"
     },
+    {
+        "MetricExpr": "(((L2I_TLB - L2I_TLB_REFILL) * 15) + (L2I_TLB_REFILL * 100)) / CPU_CYCLES",
+        "PublicDescription": "Idle by itlb miss L3 topdown metric",
+        "BriefDescription": "Idle by itlb miss L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "idle_by_itlb_miss"
+    },
+    {
+        "MetricExpr": "(((L2I_CACHE - L2I_CACHE_REFILL) * 15) + (L2I_CACHE_REFILL * 100)) / CPU_CYCLES",
+        "PublicDescription": "Idle by icache miss L3 topdown metric",
+        "BriefDescription": "Idle by icache miss L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "idle_by_icache_miss"
+    },
+    {
+        "MetricExpr": "(BR_MIS_PRED * 5) / CPU_CYCLES",
+        "PublicDescription": "BP misp flush L3 topdown metric",
+        "BriefDescription": "BP misp flush L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "bp_misp_flush"
+    },
+    {
+        "MetricExpr": "(armv8_pmuv3_0@event\\=0x2013@ * 5) / CPU_CYCLES",
+        "PublicDescription": "OOO flush L3 topdown metric",
+        "BriefDescription": "OOO flush L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "ooo_flush"
+    },
+    {
+        "MetricExpr": "(armv8_pmuv3_0@event\\=0x1001@ * 5) / CPU_CYCLES",
+        "PublicDescription": "Static predictor flush L3 topdown metric",
+        "BriefDescription": "Static predictor flush L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "sp_flush"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x1010@ / BR_MIS_PRED",
+        "PublicDescription": "Indirect branch L3 topdown metric",
+        "BriefDescription": "Indirect branch L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "indirect_branch"
+    },
+    {
+        "MetricExpr": "(armv8_pmuv3_0@event\\=0x1014@ + armv8_pmuv3_0@event\\=0x1018@) / BR_MIS_PRED",
+        "PublicDescription": "Push branch L3 topdown metric",
+        "BriefDescription": "Push branch L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "push_branch"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x100c@ / BR_MIS_PRED",
+        "PublicDescription": "Pop branch L3 topdown metric",
+        "BriefDescription": "Pop branch L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "pop_branch"
+    },
+    {
+        "MetricExpr": "(BR_MIS_PRED - armv8_pmuv3_0@event\\=0x1010@ - armv8_pmuv3_0@event\\=0x1014@ - armv8_pmuv3_0@event\\=0x1018@ - armv8_pmuv3_0@event\\=0x100c@) / BR_MIS_PRED",
+        "PublicDescription": "Other branch L3 topdown metric",
+        "BriefDescription": "Other branch L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "other_branch"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x2012@ / armv8_pmuv3_0@event\\=0x2013@",
+        "PublicDescription": "Nuke flush L3 topdown metric",
+        "BriefDescription": "Nuke flush L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "nuke_flush"
+    },
+    {
+        "MetricExpr": "1 - nuke_flush",
+        "PublicDescription": "Other flush L3 topdown metric",
+        "BriefDescription": "Other flush L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "other_flush"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x2010@ / CPU_CYCLES",
+        "PublicDescription": "Sync stall L3 topdown metric",
+        "BriefDescription": "Sync stall L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "sync_stall"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x2004@ / CPU_CYCLES",
+        "PublicDescription": "Rob stall L3 topdown metric",
+        "BriefDescription": "Rob stall L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "rob_stall"
+    },
+    {
+        "MetricExpr": "(armv8_pmuv3_0@event\\=0x2006@ + armv8_pmuv3_0@event\\=0x2007@ + armv8_pmuv3_0@event\\=0x2008@) / CPU_CYCLES",
+        "PublicDescription": "Ptag stall L3 topdown metric",
+        "BriefDescription": "Ptag stall L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "ptag_stall"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x201e@ / CPU_CYCLES",
+        "PublicDescription": "SaveOpQ stall L3 topdown metric",
+        "BriefDescription": "SaveOpQ stall L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "saveopq_stall"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x2005@ / CPU_CYCLES",
+        "PublicDescription": "PC buffer stall L3 topdown metric",
+        "BriefDescription": "PC buffer stall L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "pc_buffer_stall"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x7002@ / CPU_CYCLES",
+        "PublicDescription": "Divider L3 topdown metric",
+        "BriefDescription": "Divider L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "divider"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x7003@ / CPU_CYCLES",
+        "PublicDescription": "FSU stall L3 topdown metric",
+        "BriefDescription": "FSU stall L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "fsu_stall"
+    },
+    {
+        "MetricExpr": "core_bound - divider - fsu_stall",
+        "PublicDescription": "EXE ports util L3 topdown metric",
+        "BriefDescription": "EXE ports util L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "exe_ports_util"
+    },
+    {
+        "MetricExpr": "(MEM_STALL_ANYLOAD - MEM_STALL_L1MISS) / CPU_CYCLES",
+        "PublicDescription": "L1 bound L3 topdown metric",
+        "BriefDescription": "L1 bound L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "l1_bound"
+    },
+    {
+        "MetricExpr": "(MEM_STALL_L1MISS - MEM_STALL_L2MISS) / CPU_CYCLES",
+        "PublicDescription": "L2 bound L3 topdown metric",
+        "BriefDescription": "L2 bound L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "l2_bound"
+    },
+    {
+        "MetricExpr": "MEM_STALL_L2MISS / CPU_CYCLES",
+        "PublicDescription": "Mem bound L3 topdown metric",
+        "BriefDescription": "Mem bound L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "mem_bound"
+    },
+    {
+        "MetricExpr": "armv8_pmuv3_0@event\\=0x7005@ / CPU_CYCLES",
+        "PublicDescription": "Store bound L3 topdown metric",
+        "BriefDescription": "Store bound L3 topdown metric",
+        "MetricGroup": "TopDownL3",
+        "MetricName": "store_bound"
+    },
 ]
-- 
2.26.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re:  [PATCH v2 0/6] perf arm64 metricgroup support
  2021-03-25 10:33 ` John Garry
@ 2021-03-25 20:39   ` Paul A. Clarke
  -1 siblings, 0 replies; 52+ messages in thread
From: Paul A. Clarke @ 2021-03-25 20:39 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun

On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
> Metric reuse support is added for pmu-events parse metric testcase.
> This had been broken on power9 recentlty:
> https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/

Much better.  Before:
--
$ perf test -v 10 2>&1 | grep -i error | wc -l
112
--
After:
--
$ perf test -v 10 2>&1 | grep -i error | wc -l
17
--

And these seem like different types of issues:
--
$ perf test -v 10 2>&1 | grep -i error
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
--

(Added Kajol Jain to CC)

PC

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

* Re:  [PATCH v2 0/6] perf arm64 metricgroup support
@ 2021-03-25 20:39   ` Paul A. Clarke
  0 siblings, 0 replies; 52+ messages in thread
From: Paul A. Clarke @ 2021-03-25 20:39 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun

On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
> Metric reuse support is added for pmu-events parse metric testcase.
> This had been broken on power9 recentlty:
> https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/

Much better.  Before:
--
$ perf test -v 10 2>&1 | grep -i error | wc -l
112
--
After:
--
$ perf test -v 10 2>&1 | grep -i error | wc -l
17
--

And these seem like different types of issues:
--
$ perf test -v 10 2>&1 | grep -i error
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
--

(Added Kajol Jain to CC)

PC

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 0/6] perf arm64 metricgroup support
  2021-03-25 20:39   ` Paul A. Clarke
@ 2021-03-26 10:57     ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-26 10:57 UTC (permalink / raw)
  To: Paul A. Clarke
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun

On 25/03/2021 20:39, Paul A. Clarke wrote:
> On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
>> Metric reuse support is added for pmu-events parse metric testcase.
>> This had been broken on power9 recentlty:
>> https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/
> 
> Much better.  Before:
> --
> $ perf test -v 10 2>&1 | grep -i error | wc -l
> 112
> --
> After:
> --
> $ perf test -v 10 2>&1 | grep -i error | wc -l
> 17
> --
> 
> And these seem like different types of issues:
> --
> $ perf test -v 10 2>&1 | grep -i error
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> --
> 

This looks suspicious.

Firstly, does /sys/bus/event_source/devices/nest_mcs01_imc (or others, 
above) exist on your system? I guess not.

Secondly, checking Documentation/powerpc/imc.rst, we have examples of:
nest_mcs01/PM_MCS01_64B_R...

So is the PMU name correct in the metric file for nest_mcs01_imc? 
Looking at the kernel driver file, arch/powerpc/perf/imc-pmu.c, it seems 
to be correct. Not sure.

Thanks,
John



> (Added Kajol Jain to CC)
> 



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

* Re: [PATCH v2 0/6] perf arm64 metricgroup support
@ 2021-03-26 10:57     ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-03-26 10:57 UTC (permalink / raw)
  To: Paul A. Clarke
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun

On 25/03/2021 20:39, Paul A. Clarke wrote:
> On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
>> Metric reuse support is added for pmu-events parse metric testcase.
>> This had been broken on power9 recentlty:
>> https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/
> 
> Much better.  Before:
> --
> $ perf test -v 10 2>&1 | grep -i error | wc -l
> 112
> --
> After:
> --
> $ perf test -v 10 2>&1 | grep -i error | wc -l
> 17
> --
> 
> And these seem like different types of issues:
> --
> $ perf test -v 10 2>&1 | grep -i error
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> --
> 

This looks suspicious.

Firstly, does /sys/bus/event_source/devices/nest_mcs01_imc (or others, 
above) exist on your system? I guess not.

Secondly, checking Documentation/powerpc/imc.rst, we have examples of:
nest_mcs01/PM_MCS01_64B_R...

So is the PMU name correct in the metric file for nest_mcs01_imc? 
Looking at the kernel driver file, arch/powerpc/perf/imc-pmu.c, it seems 
to be correct. Not sure.

Thanks,
John



> (Added Kajol Jain to CC)
> 



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [PATCH v2 0/6] perf arm64 metricgroup support
  2021-03-26 10:57     ` John Garry
@ 2021-03-26 13:13       ` Paul A. Clarke
  -1 siblings, 0 replies; 52+ messages in thread
From: Paul A. Clarke @ 2021-03-26 13:13 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun

On Fri, Mar 26, 2021 at 10:57:40AM +0000, John Garry wrote:
> On 25/03/2021 20:39, Paul A. Clarke wrote:
> > On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
> > > Metric reuse support is added for pmu-events parse metric testcase.
> > > This had been broken on power9 recentlty:
> > > https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/
> > 
> > Much better.  Before:
> > --
> > $ perf test -v 10 2>&1 | grep -i error | wc -l
> > 112
> > --
> > After:
> > --
> > $ perf test -v 10 2>&1 | grep -i error | wc -l
> > 17
> > --
> > 
> > And these seem like different types of issues:
> > --
> > $ perf test -v 10 2>&1 | grep -i error
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > --
> > 
> 
> This looks suspicious.
> 
> Firstly, does /sys/bus/event_source/devices/nest_mcs01_imc (or others,
> above) exist on your system? I guess not.
> 
> Secondly, checking Documentation/powerpc/imc.rst, we have examples of:
> nest_mcs01/PM_MCS01_64B_R...
> 
> So is the PMU name correct in the metric file for nest_mcs01_imc? Looking at
> the kernel driver file, arch/powerpc/perf/imc-pmu.c, it seems to be correct.
> Not sure.

This may be caused by me testing a new perf on an older kernel. Let me retest.

PC

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

* RE: [PATCH v2 0/6] perf arm64 metricgroup support
@ 2021-03-26 13:13       ` Paul A. Clarke
  0 siblings, 0 replies; 52+ messages in thread
From: Paul A. Clarke @ 2021-03-26 13:13 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun

On Fri, Mar 26, 2021 at 10:57:40AM +0000, John Garry wrote:
> On 25/03/2021 20:39, Paul A. Clarke wrote:
> > On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
> > > Metric reuse support is added for pmu-events parse metric testcase.
> > > This had been broken on power9 recentlty:
> > > https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/
> > 
> > Much better.  Before:
> > --
> > $ perf test -v 10 2>&1 | grep -i error | wc -l
> > 112
> > --
> > After:
> > --
> > $ perf test -v 10 2>&1 | grep -i error | wc -l
> > 17
> > --
> > 
> > And these seem like different types of issues:
> > --
> > $ perf test -v 10 2>&1 | grep -i error
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > --
> > 
> 
> This looks suspicious.
> 
> Firstly, does /sys/bus/event_source/devices/nest_mcs01_imc (or others,
> above) exist on your system? I guess not.
> 
> Secondly, checking Documentation/powerpc/imc.rst, we have examples of:
> nest_mcs01/PM_MCS01_64B_R...
> 
> So is the PMU name correct in the metric file for nest_mcs01_imc? Looking at
> the kernel driver file, arch/powerpc/perf/imc-pmu.c, it seems to be correct.
> Not sure.

This may be caused by me testing a new perf on an older kernel. Let me retest.

PC

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [PATCH v2 0/6] perf arm64 metricgroup support
  2021-03-26 10:57     ` John Garry
@ 2021-03-29 21:07       ` Paul A. Clarke
  -1 siblings, 0 replies; 52+ messages in thread
From: Paul A. Clarke @ 2021-03-29 21:07 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun

On Fri, Mar 26, 2021 at 10:57:40AM +0000, John Garry wrote:
> On 25/03/2021 20:39, Paul A. Clarke wrote:
> > On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
> > > Metric reuse support is added for pmu-events parse metric testcase.
> > > This had been broken on power9 recentlty:
> > > https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/
> > 
> > Much better.  Before:
> > --
> > $ perf test -v 10 2>&1 | grep -i error | wc -l
> > 112
> > --
> > After:
> > --
> > $ perf test -v 10 2>&1 | grep -i error | wc -l
> > 17
> > --
> > 
> > And these seem like different types of issues:
> > --
> > $ perf test -v 10 2>&1 | grep -i error
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > --
> > 
> 
> This looks suspicious.
> 
> Firstly, does /sys/bus/event_source/devices/nest_mcs01_imc (or others,
> above) exist on your system? I guess not.
> 
> Secondly, checking Documentation/powerpc/imc.rst, we have examples of:
> nest_mcs01/PM_MCS01_64B_R...
> 
> So is the PMU name correct in the metric file for nest_mcs01_imc? Looking at
> the kernel driver file, arch/powerpc/perf/imc-pmu.c, it seems to be correct.
> Not sure.

I ran with a newer kernel, and the above errors disappeared, replaced with
about 10 of:
--
Error string 'Cannot find PMU `hv_24x7'. Missing kernel support?' help '(null)'
--

...but I was running without a hypervisor, so I tried the same kernel on a
PowerVM-virtualized system and the "hv_24x7" messages went away, but the
"nest" messages returned.  This may all be expected behavior... I confess
I haven't followed these new perf capabilities closely.

It's extremely likely that none of these errors has anything to do with your
changes. :-)

PC

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

* RE: [PATCH v2 0/6] perf arm64 metricgroup support
@ 2021-03-29 21:07       ` Paul A. Clarke
  0 siblings, 0 replies; 52+ messages in thread
From: Paul A. Clarke @ 2021-03-29 21:07 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kjain, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun

On Fri, Mar 26, 2021 at 10:57:40AM +0000, John Garry wrote:
> On 25/03/2021 20:39, Paul A. Clarke wrote:
> > On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
> > > Metric reuse support is added for pmu-events parse metric testcase.
> > > This had been broken on power9 recentlty:
> > > https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/
> > 
> > Much better.  Before:
> > --
> > $ perf test -v 10 2>&1 | grep -i error | wc -l
> > 112
> > --
> > After:
> > --
> > $ perf test -v 10 2>&1 | grep -i error | wc -l
> > 17
> > --
> > 
> > And these seem like different types of issues:
> > --
> > $ perf test -v 10 2>&1 | grep -i error
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > --
> > 
> 
> This looks suspicious.
> 
> Firstly, does /sys/bus/event_source/devices/nest_mcs01_imc (or others,
> above) exist on your system? I guess not.
> 
> Secondly, checking Documentation/powerpc/imc.rst, we have examples of:
> nest_mcs01/PM_MCS01_64B_R...
> 
> So is the PMU name correct in the metric file for nest_mcs01_imc? Looking at
> the kernel driver file, arch/powerpc/perf/imc-pmu.c, it seems to be correct.
> Not sure.

I ran with a newer kernel, and the above errors disappeared, replaced with
about 10 of:
--
Error string 'Cannot find PMU `hv_24x7'. Missing kernel support?' help '(null)'
--

...but I was running without a hypervisor, so I tried the same kernel on a
PowerVM-virtualized system and the "hv_24x7" messages went away, but the
"nest" messages returned.  This may all be expected behavior... I confess
I haven't followed these new perf capabilities closely.

It's extremely likely that none of these errors has anything to do with your
changes. :-)

PC

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 0/6] perf arm64 metricgroup support
  2021-03-29 21:07       ` Paul A. Clarke
@ 2021-03-30  6:41         ` kajoljain
  -1 siblings, 0 replies; 52+ messages in thread
From: kajoljain @ 2021-03-30  6:41 UTC (permalink / raw)
  To: Paul A. Clarke, John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun



On 3/30/21 2:37 AM, Paul A. Clarke wrote:
> On Fri, Mar 26, 2021 at 10:57:40AM +0000, John Garry wrote:
>> On 25/03/2021 20:39, Paul A. Clarke wrote:
>>> On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
>>>> Metric reuse support is added for pmu-events parse metric testcase.
>>>> This had been broken on power9 recentlty:
>>>> https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/
>>>
>>> Much better.  Before:
>>> --
>>> $ perf test -v 10 2>&1 | grep -i error | wc -l
>>> 112
>>> --
>>> After:
>>> --
>>> $ perf test -v 10 2>&1 | grep -i error | wc -l
>>> 17
>>> --
>>>
>>> And these seem like different types of issues:
>>> --
>>> $ perf test -v 10 2>&1 | grep -i error
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> --
>>>
>>
>> This looks suspicious.
>>
>> Firstly, does /sys/bus/event_source/devices/nest_mcs01_imc (or others,
>> above) exist on your system? I guess not.
>>
>> Secondly, checking Documentation/powerpc/imc.rst, we have examples of:
>> nest_mcs01/PM_MCS01_64B_R...
>>
>> So is the PMU name correct in the metric file for nest_mcs01_imc? Looking at
>> the kernel driver file, arch/powerpc/perf/imc-pmu.c, it seems to be correct.
>> Not sure.
> 
> I ran with a newer kernel, and the above errors disappeared, replaced with
> about 10 of:
> --
> Error string 'Cannot find PMU `hv_24x7'. Missing kernel support?' help '(null)'
> --
> 
> ...but I was running without a hypervisor, so I tried the same kernel on a
> PowerVM-virtualized system and the "hv_24x7" messages went away, but the
> "nest" messages returned.  This may all be expected behavior... I confess
> I haven't followed these new perf capabilities closely.
> 

Hi Paul/John,
   This is something expected. For nest-imc we need bare-metal system and for
hv-24x7 we need VM environment. Since you are checking this test in VM machine,
there nest events are not supported and hence we are getting this error.

Thanks,
Kajol Jain

> It's extremely likely that none of these errors has anything to do with your
> changes. :-
> 
> PC
> 

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

* Re: [PATCH v2 0/6] perf arm64 metricgroup support
@ 2021-03-30  6:41         ` kajoljain
  0 siblings, 0 replies; 52+ messages in thread
From: kajoljain @ 2021-03-30  6:41 UTC (permalink / raw)
  To: Paul A. Clarke, John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun



On 3/30/21 2:37 AM, Paul A. Clarke wrote:
> On Fri, Mar 26, 2021 at 10:57:40AM +0000, John Garry wrote:
>> On 25/03/2021 20:39, Paul A. Clarke wrote:
>>> On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
>>>> Metric reuse support is added for pmu-events parse metric testcase.
>>>> This had been broken on power9 recentlty:
>>>> https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/
>>>
>>> Much better.  Before:
>>> --
>>> $ perf test -v 10 2>&1 | grep -i error | wc -l
>>> 112
>>> --
>>> After:
>>> --
>>> $ perf test -v 10 2>&1 | grep -i error | wc -l
>>> 17
>>> --
>>>
>>> And these seem like different types of issues:
>>> --
>>> $ perf test -v 10 2>&1 | grep -i error
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>> --
>>>
>>
>> This looks suspicious.
>>
>> Firstly, does /sys/bus/event_source/devices/nest_mcs01_imc (or others,
>> above) exist on your system? I guess not.
>>
>> Secondly, checking Documentation/powerpc/imc.rst, we have examples of:
>> nest_mcs01/PM_MCS01_64B_R...
>>
>> So is the PMU name correct in the metric file for nest_mcs01_imc? Looking at
>> the kernel driver file, arch/powerpc/perf/imc-pmu.c, it seems to be correct.
>> Not sure.
> 
> I ran with a newer kernel, and the above errors disappeared, replaced with
> about 10 of:
> --
> Error string 'Cannot find PMU `hv_24x7'. Missing kernel support?' help '(null)'
> --
> 
> ...but I was running without a hypervisor, so I tried the same kernel on a
> PowerVM-virtualized system and the "hv_24x7" messages went away, but the
> "nest" messages returned.  This may all be expected behavior... I confess
> I haven't followed these new perf capabilities closely.
> 

Hi Paul/John,
   This is something expected. For nest-imc we need bare-metal system and for
hv-24x7 we need VM environment. Since you are checking this test in VM machine,
there nest events are not supported and hence we are getting this error.

Thanks,
Kajol Jain

> It's extremely likely that none of these errors has anything to do with your
> changes. :-
> 
> PC
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
  2021-03-25 10:33   ` John Garry
@ 2021-04-01 13:49     ` Jiri Olsa
  -1 siblings, 0 replies; 52+ messages in thread
From: Jiri Olsa @ 2021-04-01 13:49 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On Thu, Mar 25, 2021 at 06:33:14PM +0800, John Garry wrote:

SNIP

> +struct metric {
> +	struct list_head list;
> +	struct metric_ref metric_ref;
> +};
> +
> +static int resolve_metric_simple(struct expr_parse_ctx *pctx,
> +				 struct list_head *compound_list,
> +				 struct pmu_events_map *map,
> +				 const char *metric_name)
> +{
> +	struct hashmap_entry *cur, *cur_tmp;
> +	struct metric *metric, *tmp;
> +	size_t bkt;
> +	bool all;
> +	int rc;
> +
> +	do {
> +		all = true;
> +		hashmap__for_each_entry_safe((&pctx->ids), cur, cur_tmp, bkt) {
> +			struct metric_ref *ref;
> +			struct pmu_event *pe;
> +
> +			pe = metrcgroup_find_metric(cur->key, map);
> +			if (!pe)
> +				continue;
> +
> +			if (!strcmp(metric_name, (char *)cur->key)) {
> +				pr_warning("Recursion detected for metric %s\n", metric_name);
> +				rc = -1;
> +				goto out_err;
> +			}
> +
> +			all = false;
> +
> +			/* The metric key itself needs to go out.. */
> +			expr__del_id(pctx, cur->key);
> +
> +			metric = malloc(sizeof(*metric));
> +			if (!metric) {
> +				rc = -ENOMEM;
> +				goto out_err;
> +			}
> +
> +			ref = &metric->metric_ref;
> +			ref->metric_name = pe->metric_name;
> +			ref->metric_expr = pe->metric_expr;
> +			list_add_tail(&metric->list, compound_list);
> +
> +			rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);

so this might add new items to pctx->ids, I think you need
to restart the iteration as we do it in __resolve_metric
otherwise you could miss some new keys

jirka

> +			if (rc)
> +				goto out_err;
> +		}
> +	} while (!all);
> +
> +	return 0;
> +
> +out_err:
> +	list_for_each_entry_safe(metric, tmp, compound_list, list)
> +		free(metric);
> +
> +	return rc;
> +
> +}

SNIP


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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
@ 2021-04-01 13:49     ` Jiri Olsa
  0 siblings, 0 replies; 52+ messages in thread
From: Jiri Olsa @ 2021-04-01 13:49 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On Thu, Mar 25, 2021 at 06:33:14PM +0800, John Garry wrote:

SNIP

> +struct metric {
> +	struct list_head list;
> +	struct metric_ref metric_ref;
> +};
> +
> +static int resolve_metric_simple(struct expr_parse_ctx *pctx,
> +				 struct list_head *compound_list,
> +				 struct pmu_events_map *map,
> +				 const char *metric_name)
> +{
> +	struct hashmap_entry *cur, *cur_tmp;
> +	struct metric *metric, *tmp;
> +	size_t bkt;
> +	bool all;
> +	int rc;
> +
> +	do {
> +		all = true;
> +		hashmap__for_each_entry_safe((&pctx->ids), cur, cur_tmp, bkt) {
> +			struct metric_ref *ref;
> +			struct pmu_event *pe;
> +
> +			pe = metrcgroup_find_metric(cur->key, map);
> +			if (!pe)
> +				continue;
> +
> +			if (!strcmp(metric_name, (char *)cur->key)) {
> +				pr_warning("Recursion detected for metric %s\n", metric_name);
> +				rc = -1;
> +				goto out_err;
> +			}
> +
> +			all = false;
> +
> +			/* The metric key itself needs to go out.. */
> +			expr__del_id(pctx, cur->key);
> +
> +			metric = malloc(sizeof(*metric));
> +			if (!metric) {
> +				rc = -ENOMEM;
> +				goto out_err;
> +			}
> +
> +			ref = &metric->metric_ref;
> +			ref->metric_name = pe->metric_name;
> +			ref->metric_expr = pe->metric_expr;
> +			list_add_tail(&metric->list, compound_list);
> +
> +			rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);

so this might add new items to pctx->ids, I think you need
to restart the iteration as we do it in __resolve_metric
otherwise you could miss some new keys

jirka

> +			if (rc)
> +				goto out_err;
> +		}
> +	} while (!all);
> +
> +	return 0;
> +
> +out_err:
> +	list_for_each_entry_safe(metric, tmp, compound_list, list)
> +		free(metric);
> +
> +	return rc;
> +
> +}

SNIP


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 1/6] perf metricgroup: Make find_metric() public with name change
  2021-03-25 10:33   ` John Garry
@ 2021-04-01 23:16     ` Ian Rogers
  -1 siblings, 0 replies; 52+ messages in thread
From: Ian Rogers @ 2021-04-01 23:16 UTC (permalink / raw)
  To: John Garry
  Cc: Will Deacon, Mathieu Poirier, Leo Yan, Peter Zijlstra,
	Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, linuxarm, kajoljain,
	Liang, Kan, LKML, Linux ARM, zhangshaokun, Paul Clarke

On Thu, Mar 25, 2021 at 3:38 AM John Garry <john.garry@huawei.com> wrote:
>
> Function find_metric() is required for the metric processing in the
> pmu-events testcase, so make it public. Also change the name to include
> "metricgroup".

Would it make more sense as "pmu_events_map__find_metric" ?

Thanks,
Ian

> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  tools/perf/util/metricgroup.c | 5 +++--
>  tools/perf/util/metricgroup.h | 3 ++-
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 6acb44ad439b..71a13406e0bd 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -900,7 +900,8 @@ static int __add_metric(struct list_head *metric_list,
>                     (match_metric(__pe->metric_group, __metric) ||      \
>                      match_metric(__pe->metric_name, __metric)))
>
> -static struct pmu_event *find_metric(const char *metric, struct pmu_events_map *map)
> +struct pmu_event *metrcgroup_find_metric(const char *metric,
> +                                        struct pmu_events_map *map)
>  {
>         struct pmu_event *pe;
>         int i;
> @@ -985,7 +986,7 @@ static int __resolve_metric(struct metric *m,
>                         struct expr_id *parent;
>                         struct pmu_event *pe;
>
> -                       pe = find_metric(cur->key, map);
> +                       pe = metrcgroup_find_metric(cur->key, map);
>                         if (!pe)
>                                 continue;
>
> diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
> index ed1b9392e624..1674c6a36d74 100644
> --- a/tools/perf/util/metricgroup.h
> +++ b/tools/perf/util/metricgroup.h
> @@ -44,7 +44,8 @@ int metricgroup__parse_groups(const struct option *opt,
>                               bool metric_no_group,
>                               bool metric_no_merge,
>                               struct rblist *metric_events);
> -
> +struct pmu_event *metrcgroup_find_metric(const char *metric,
> +                                        struct pmu_events_map *map);
>  int metricgroup__parse_groups_test(struct evlist *evlist,
>                                    struct pmu_events_map *map,
>                                    const char *str,
> --
> 2.26.2
>

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

* Re: [PATCH v2 1/6] perf metricgroup: Make find_metric() public with name change
@ 2021-04-01 23:16     ` Ian Rogers
  0 siblings, 0 replies; 52+ messages in thread
From: Ian Rogers @ 2021-04-01 23:16 UTC (permalink / raw)
  To: John Garry
  Cc: Will Deacon, Mathieu Poirier, Leo Yan, Peter Zijlstra,
	Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, linuxarm, kajoljain,
	Liang, Kan, LKML, Linux ARM, zhangshaokun, Paul Clarke

On Thu, Mar 25, 2021 at 3:38 AM John Garry <john.garry@huawei.com> wrote:
>
> Function find_metric() is required for the metric processing in the
> pmu-events testcase, so make it public. Also change the name to include
> "metricgroup".

Would it make more sense as "pmu_events_map__find_metric" ?

Thanks,
Ian

> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  tools/perf/util/metricgroup.c | 5 +++--
>  tools/perf/util/metricgroup.h | 3 ++-
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index 6acb44ad439b..71a13406e0bd 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -900,7 +900,8 @@ static int __add_metric(struct list_head *metric_list,
>                     (match_metric(__pe->metric_group, __metric) ||      \
>                      match_metric(__pe->metric_name, __metric)))
>
> -static struct pmu_event *find_metric(const char *metric, struct pmu_events_map *map)
> +struct pmu_event *metrcgroup_find_metric(const char *metric,
> +                                        struct pmu_events_map *map)
>  {
>         struct pmu_event *pe;
>         int i;
> @@ -985,7 +986,7 @@ static int __resolve_metric(struct metric *m,
>                         struct expr_id *parent;
>                         struct pmu_event *pe;
>
> -                       pe = find_metric(cur->key, map);
> +                       pe = metrcgroup_find_metric(cur->key, map);
>                         if (!pe)
>                                 continue;
>
> diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
> index ed1b9392e624..1674c6a36d74 100644
> --- a/tools/perf/util/metricgroup.h
> +++ b/tools/perf/util/metricgroup.h
> @@ -44,7 +44,8 @@ int metricgroup__parse_groups(const struct option *opt,
>                               bool metric_no_group,
>                               bool metric_no_merge,
>                               struct rblist *metric_events);
> -
> +struct pmu_event *metrcgroup_find_metric(const char *metric,
> +                                        struct pmu_events_map *map);
>  int metricgroup__parse_groups_test(struct evlist *evlist,
>                                    struct pmu_events_map *map,
>                                    const char *str,
> --
> 2.26.2
>

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 1/6] perf metricgroup: Make find_metric() public with name change
  2021-04-01 23:16     ` Ian Rogers
@ 2021-04-06  9:54       ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-04-06  9:54 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Will Deacon, Mathieu Poirier, Leo Yan, Peter Zijlstra,
	Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, linuxarm, kajoljain,
	Liang, Kan, LKML, Linux ARM, zhangshaokun, Paul Clarke

On 02/04/2021 00:16, Ian Rogers wrote:
> On Thu, Mar 25, 2021 at 3:38 AM John Garry <john.garry@huawei.com> wrote:
>>
>> Function find_metric() is required for the metric processing in the
>> pmu-events testcase, so make it public. Also change the name to include
>> "metricgroup".
> 
> Would it make more sense as "pmu_events_map__find_metric" ?
> 

So all functions apart from one in metricgroup.h are named 
metricgroup__XXX, so I was trying to keep this style - apart from the 
double-underscore (which can be remedied).

Personally I don't think pmu_events_map__find_metric name fits with that 
convention.

Thanks,
John

> Thanks,
> Ian
> 
>> Signed-off-by: John Garry <john.garry@huawei.com>
>> ---
>>   tools/perf/util/metricgroup.c | 5 +++--
>>   tools/perf/util/metricgroup.h | 3 ++-
>>   2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
>> index 6acb44ad439b..71a13406e0bd 100644
>> --- a/tools/perf/util/metricgroup.c
>> +++ b/tools/perf/util/metricgroup.c
>> @@ -900,7 +900,8 @@ static int __add_metric(struct list_head *metric_list,
>>                      (match_metric(__pe->metric_group, __metric) ||      \
>>                       match_metric(__pe->metric_name, __metric)))
>>
>> -static struct pmu_event *find_metric(const char *metric, struct pmu_events_map *map)
>> +struct pmu_event *metrcgroup_find_metric(const char *metric,
>> +                                        struct pmu_events_map *map)
>>   {
>>          struct pmu_event *pe;
>>          int i;
>> @@ -985,7 +986,7 @@ static int __resolve_metric(struct metric *m,
>>                          struct expr_id *parent;
>>                          struct pmu_event *pe;
>>
>> -                       pe = find_metric(cur->key, map);
>> +                       pe = metrcgroup_find_metric(cur->key, map);
>>                          if (!pe)
>>                                  continue;
>>
>> diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
>> index ed1b9392e624..1674c6a36d74 100644
>> --- a/tools/perf/util/metricgroup.h
>> +++ b/tools/perf/util/metricgroup.h
>> @@ -44,7 +44,8 @@ int metricgroup__parse_groups(const struct option *opt,
>>                                bool metric_no_group,
>>                                bool metric_no_merge,
>>                                struct rblist *metric_events);
>> -
>> +struct pmu_event *metrcgroup_find_metric(const char *metric,
>> +                                        struct pmu_events_map *map);
>>   int metricgroup__parse_groups_test(struct evlist *evlist,
>>                                     struct pmu_events_map *map,
>>                                     const char *str,
>> --
>> 2.26.2
>>
> .
> 


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

* Re: [PATCH v2 1/6] perf metricgroup: Make find_metric() public with name change
@ 2021-04-06  9:54       ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-04-06  9:54 UTC (permalink / raw)
  To: Ian Rogers
  Cc: Will Deacon, Mathieu Poirier, Leo Yan, Peter Zijlstra,
	Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, linuxarm, kajoljain,
	Liang, Kan, LKML, Linux ARM, zhangshaokun, Paul Clarke

On 02/04/2021 00:16, Ian Rogers wrote:
> On Thu, Mar 25, 2021 at 3:38 AM John Garry <john.garry@huawei.com> wrote:
>>
>> Function find_metric() is required for the metric processing in the
>> pmu-events testcase, so make it public. Also change the name to include
>> "metricgroup".
> 
> Would it make more sense as "pmu_events_map__find_metric" ?
> 

So all functions apart from one in metricgroup.h are named 
metricgroup__XXX, so I was trying to keep this style - apart from the 
double-underscore (which can be remedied).

Personally I don't think pmu_events_map__find_metric name fits with that 
convention.

Thanks,
John

> Thanks,
> Ian
> 
>> Signed-off-by: John Garry <john.garry@huawei.com>
>> ---
>>   tools/perf/util/metricgroup.c | 5 +++--
>>   tools/perf/util/metricgroup.h | 3 ++-
>>   2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
>> index 6acb44ad439b..71a13406e0bd 100644
>> --- a/tools/perf/util/metricgroup.c
>> +++ b/tools/perf/util/metricgroup.c
>> @@ -900,7 +900,8 @@ static int __add_metric(struct list_head *metric_list,
>>                      (match_metric(__pe->metric_group, __metric) ||      \
>>                       match_metric(__pe->metric_name, __metric)))
>>
>> -static struct pmu_event *find_metric(const char *metric, struct pmu_events_map *map)
>> +struct pmu_event *metrcgroup_find_metric(const char *metric,
>> +                                        struct pmu_events_map *map)
>>   {
>>          struct pmu_event *pe;
>>          int i;
>> @@ -985,7 +986,7 @@ static int __resolve_metric(struct metric *m,
>>                          struct expr_id *parent;
>>                          struct pmu_event *pe;
>>
>> -                       pe = find_metric(cur->key, map);
>> +                       pe = metrcgroup_find_metric(cur->key, map);
>>                          if (!pe)
>>                                  continue;
>>
>> diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
>> index ed1b9392e624..1674c6a36d74 100644
>> --- a/tools/perf/util/metricgroup.h
>> +++ b/tools/perf/util/metricgroup.h
>> @@ -44,7 +44,8 @@ int metricgroup__parse_groups(const struct option *opt,
>>                                bool metric_no_group,
>>                                bool metric_no_merge,
>>                                struct rblist *metric_events);
>> -
>> +struct pmu_event *metrcgroup_find_metric(const char *metric,
>> +                                        struct pmu_events_map *map);
>>   int metricgroup__parse_groups_test(struct evlist *evlist,
>>                                     struct pmu_events_map *map,
>>                                     const char *str,
>> --
>> 2.26.2
>>
> .
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
  2021-04-01 13:49     ` Jiri Olsa
@ 2021-04-06 11:00       ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-04-06 11:00 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On 01/04/2021 14:49, Jiri Olsa wrote:
> On Thu, Mar 25, 2021 at 06:33:14PM +0800, John Garry wrote:
> 
> SNIP
> 
>> +struct metric {
>> +	struct list_head list;
>> +	struct metric_ref metric_ref;
>> +};
>> +
>> +static int resolve_metric_simple(struct expr_parse_ctx *pctx,
>> +				 struct list_head *compound_list,
>> +				 struct pmu_events_map *map,
>> +				 const char *metric_name)
>> +{
>> +	struct hashmap_entry *cur, *cur_tmp;
>> +	struct metric *metric, *tmp;
>> +	size_t bkt;
>> +	bool all;
>> +	int rc;
>> +
>> +	do {
>> +		all = true;
>> +		hashmap__for_each_entry_safe((&pctx->ids), cur, cur_tmp, bkt) {
>> +			struct metric_ref *ref;
>> +			struct pmu_event *pe;
>> +
>> +			pe = metrcgroup_find_metric(cur->key, map);

*

>> +			if (!pe)
>> +				continue;
>> +
>> +			if (!strcmp(metric_name, (char *)cur->key)) {
>> +				pr_warning("Recursion detected for metric %s\n", metric_name);
>> +				rc = -1;
>> +				goto out_err;
>> +			}
>> +
>> +			all = false;
>> +
>> +			/* The metric key itself needs to go out.. */
>> +			expr__del_id(pctx, cur->key);
>> +
>> +			metric = malloc(sizeof(*metric));
>> +			if (!metric) {
>> +				rc = -ENOMEM;
>> +				goto out_err;
>> +			}
>> +
>> +			ref = &metric->metric_ref;
>> +			ref->metric_name = pe->metric_name;
>> +			ref->metric_expr = pe->metric_expr;
>> +			list_add_tail(&metric->list, compound_list);
>> +
>> +			rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> 

Hi Jirka,

> so this might add new items to pctx->ids, I think you need
> to restart the iteration as we do it in __resolve_metric
> otherwise you could miss some new keys

I thought that I was doing this. Indeed, this code is very much like 
__resolve_metric() ;)

So expr__find_other() may add a new item to pctx->ids, and we always 
iterate again, and try to lookup any pmu_events, *, above. If none 
exist, then we have broken down pctx into primitive events aliases and 
unresolvable metrics, and stop iterating. And then unresolvable metrics 
would be found in check_parse_cpu().

As an example, we can deal with metric test1, below, which references 2x 
other metrics:

     {
         "MetricExpr": "IDQ_UOPS_NOT_DELIVERED.CORE / (4 * (( ( 
CPU_CLK_UNHALTED.THREAD / 2 ) * ( 1 + CPU_CLK_UNHALTED.ONE_THREAD_ACTIVE 
/ CPU_CLK_UNHALTED.REF_XCLK ) )))",
       "MetricName": "Frontend_Bound",
     },
     {
         "MetricExpr": "( UOPS_ISSUED.ANY - UOPS_RETIRED.RETIRE_SLOTS + 
4 * INT_MISC.RECOVERY_CYCLES ) / (4 * cycles)",
         "MetricName": "Bad_Speculation",
     },
     {
         "MetricExpr": "Bad_Speculation + Frontend_Bound",
         "MetricName": "test1",
     },

Does that satisfy your concern, or have I missed something?

Thanks,
John

> 
> jirka
> 
>> +			if (rc)
>> +				goto out_err;
>> +		}
>> +	} while (!all);
>> +
>> +	return 0;
>> +
>> +out_err:
>> +	list_for_each_entry_safe(metric, tmp, compound_list, list)
>> +		free(metric);
>> +
>> +	return rc;
>> +
>> +}
> 
> SNIP
> 
> .
> 


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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
@ 2021-04-06 11:00       ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-04-06 11:00 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On 01/04/2021 14:49, Jiri Olsa wrote:
> On Thu, Mar 25, 2021 at 06:33:14PM +0800, John Garry wrote:
> 
> SNIP
> 
>> +struct metric {
>> +	struct list_head list;
>> +	struct metric_ref metric_ref;
>> +};
>> +
>> +static int resolve_metric_simple(struct expr_parse_ctx *pctx,
>> +				 struct list_head *compound_list,
>> +				 struct pmu_events_map *map,
>> +				 const char *metric_name)
>> +{
>> +	struct hashmap_entry *cur, *cur_tmp;
>> +	struct metric *metric, *tmp;
>> +	size_t bkt;
>> +	bool all;
>> +	int rc;
>> +
>> +	do {
>> +		all = true;
>> +		hashmap__for_each_entry_safe((&pctx->ids), cur, cur_tmp, bkt) {
>> +			struct metric_ref *ref;
>> +			struct pmu_event *pe;
>> +
>> +			pe = metrcgroup_find_metric(cur->key, map);

*

>> +			if (!pe)
>> +				continue;
>> +
>> +			if (!strcmp(metric_name, (char *)cur->key)) {
>> +				pr_warning("Recursion detected for metric %s\n", metric_name);
>> +				rc = -1;
>> +				goto out_err;
>> +			}
>> +
>> +			all = false;
>> +
>> +			/* The metric key itself needs to go out.. */
>> +			expr__del_id(pctx, cur->key);
>> +
>> +			metric = malloc(sizeof(*metric));
>> +			if (!metric) {
>> +				rc = -ENOMEM;
>> +				goto out_err;
>> +			}
>> +
>> +			ref = &metric->metric_ref;
>> +			ref->metric_name = pe->metric_name;
>> +			ref->metric_expr = pe->metric_expr;
>> +			list_add_tail(&metric->list, compound_list);
>> +
>> +			rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> 

Hi Jirka,

> so this might add new items to pctx->ids, I think you need
> to restart the iteration as we do it in __resolve_metric
> otherwise you could miss some new keys

I thought that I was doing this. Indeed, this code is very much like 
__resolve_metric() ;)

So expr__find_other() may add a new item to pctx->ids, and we always 
iterate again, and try to lookup any pmu_events, *, above. If none 
exist, then we have broken down pctx into primitive events aliases and 
unresolvable metrics, and stop iterating. And then unresolvable metrics 
would be found in check_parse_cpu().

As an example, we can deal with metric test1, below, which references 2x 
other metrics:

     {
         "MetricExpr": "IDQ_UOPS_NOT_DELIVERED.CORE / (4 * (( ( 
CPU_CLK_UNHALTED.THREAD / 2 ) * ( 1 + CPU_CLK_UNHALTED.ONE_THREAD_ACTIVE 
/ CPU_CLK_UNHALTED.REF_XCLK ) )))",
       "MetricName": "Frontend_Bound",
     },
     {
         "MetricExpr": "( UOPS_ISSUED.ANY - UOPS_RETIRED.RETIRE_SLOTS + 
4 * INT_MISC.RECOVERY_CYCLES ) / (4 * cycles)",
         "MetricName": "Bad_Speculation",
     },
     {
         "MetricExpr": "Bad_Speculation + Frontend_Bound",
         "MetricName": "test1",
     },

Does that satisfy your concern, or have I missed something?

Thanks,
John

> 
> jirka
> 
>> +			if (rc)
>> +				goto out_err;
>> +		}
>> +	} while (!all);
>> +
>> +	return 0;
>> +
>> +out_err:
>> +	list_for_each_entry_safe(metric, tmp, compound_list, list)
>> +		free(metric);
>> +
>> +	return rc;
>> +
>> +}
> 
> SNIP
> 
> .
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 0/6] perf arm64 metricgroup support
  2021-03-30  6:41         ` kajoljain
@ 2021-04-06 11:02           ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-04-06 11:02 UTC (permalink / raw)
  To: kajoljain, Paul A. Clarke
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun

On 30/03/2021 07:41, kajoljain wrote:
> 
> 
> On 3/30/21 2:37 AM, Paul A. Clarke wrote:
>> On Fri, Mar 26, 2021 at 10:57:40AM +0000, John Garry wrote:
>>> On 25/03/2021 20:39, Paul A. Clarke wrote:
>>>> On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
>>>>> Metric reuse support is added for pmu-events parse metric testcase.
>>>>> This had been broken on power9 recentlty:
>>>>> https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/
>>>>
>>>> Much better.  Before:
>>>> --
>>>> $ perf test -v 10 2>&1 | grep -i error | wc -l
>>>> 112
>>>> --
>>>> After:
>>>> --
>>>> $ perf test -v 10 2>&1 | grep -i error | wc -l
>>>> 17
>>>> --
>>>>
>>>> And these seem like different types of issues:
>>>> --
>>>> $ perf test -v 10 2>&1 | grep -i error
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> --
>>>>
>>>
>>> This looks suspicious.
>>>
>>> Firstly, does /sys/bus/event_source/devices/nest_mcs01_imc (or others,
>>> above) exist on your system? I guess not.
>>>
>>> Secondly, checking Documentation/powerpc/imc.rst, we have examples of:
>>> nest_mcs01/PM_MCS01_64B_R...
>>>
>>> So is the PMU name correct in the metric file for nest_mcs01_imc? Looking at
>>> the kernel driver file, arch/powerpc/perf/imc-pmu.c, it seems to be correct.
>>> Not sure.
>>
>> I ran with a newer kernel, and the above errors disappeared, replaced with
>> about 10 of:
>> --
>> Error string 'Cannot find PMU `hv_24x7'. Missing kernel support?' help '(null)'
>> --
>>
>> ...but I was running without a hypervisor, so I tried the same kernel on a
>> PowerVM-virtualized system and the "hv_24x7" messages went away, but the
>> "nest" messages returned.  This may all be expected behavior... I confess
>> I haven't followed these new perf capabilities closely.
>>
> 
> Hi Paul/John,
>     This is something expected. For nest-imc we need bare-metal system and for
> hv-24x7 we need VM environment. Since you are checking this test in VM machine,
> there nest events are not supported and hence we are getting this error.
> 
> Thanks,
> Kajol Jain

Cool, so I hope that tested-by or similar can be provided :) [obviously 
pending any changes that come from reviews]

Thanks

> 
>> It's extremely likely that none of these errors has anything to do with your
>> changes. :-
>>
>> PC
>>
> .
> 


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

* Re: [PATCH v2 0/6] perf arm64 metricgroup support
@ 2021-04-06 11:02           ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-04-06 11:02 UTC (permalink / raw)
  To: kajoljain, Paul A. Clarke
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun

On 30/03/2021 07:41, kajoljain wrote:
> 
> 
> On 3/30/21 2:37 AM, Paul A. Clarke wrote:
>> On Fri, Mar 26, 2021 at 10:57:40AM +0000, John Garry wrote:
>>> On 25/03/2021 20:39, Paul A. Clarke wrote:
>>>> On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
>>>>> Metric reuse support is added for pmu-events parse metric testcase.
>>>>> This had been broken on power9 recentlty:
>>>>> https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/
>>>>
>>>> Much better.  Before:
>>>> --
>>>> $ perf test -v 10 2>&1 | grep -i error | wc -l
>>>> 112
>>>> --
>>>> After:
>>>> --
>>>> $ perf test -v 10 2>&1 | grep -i error | wc -l
>>>> 17
>>>> --
>>>>
>>>> And these seem like different types of issues:
>>>> --
>>>> $ perf test -v 10 2>&1 | grep -i error
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
>>>> --
>>>>
>>>
>>> This looks suspicious.
>>>
>>> Firstly, does /sys/bus/event_source/devices/nest_mcs01_imc (or others,
>>> above) exist on your system? I guess not.
>>>
>>> Secondly, checking Documentation/powerpc/imc.rst, we have examples of:
>>> nest_mcs01/PM_MCS01_64B_R...
>>>
>>> So is the PMU name correct in the metric file for nest_mcs01_imc? Looking at
>>> the kernel driver file, arch/powerpc/perf/imc-pmu.c, it seems to be correct.
>>> Not sure.
>>
>> I ran with a newer kernel, and the above errors disappeared, replaced with
>> about 10 of:
>> --
>> Error string 'Cannot find PMU `hv_24x7'. Missing kernel support?' help '(null)'
>> --
>>
>> ...but I was running without a hypervisor, so I tried the same kernel on a
>> PowerVM-virtualized system and the "hv_24x7" messages went away, but the
>> "nest" messages returned.  This may all be expected behavior... I confess
>> I haven't followed these new perf capabilities closely.
>>
> 
> Hi Paul/John,
>     This is something expected. For nest-imc we need bare-metal system and for
> hv-24x7 we need VM environment. Since you are checking this test in VM machine,
> there nest events are not supported and hence we are getting this error.
> 
> Thanks,
> Kajol Jain

Cool, so I hope that tested-by or similar can be provided :) [obviously 
pending any changes that come from reviews]

Thanks

> 
>> It's extremely likely that none of these errors has anything to do with your
>> changes. :-
>>
>> PC
>>
> .
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
  2021-04-06 11:00       ` John Garry
@ 2021-04-06 12:17         ` Jiri Olsa
  -1 siblings, 0 replies; 52+ messages in thread
From: Jiri Olsa @ 2021-04-06 12:17 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On Tue, Apr 06, 2021 at 12:00:27PM +0100, John Garry wrote:
> On 01/04/2021 14:49, Jiri Olsa wrote:
> > On Thu, Mar 25, 2021 at 06:33:14PM +0800, John Garry wrote:
> > 
> > SNIP
> > 
> > > +struct metric {
> > > +	struct list_head list;
> > > +	struct metric_ref metric_ref;
> > > +};
> > > +
> > > +static int resolve_metric_simple(struct expr_parse_ctx *pctx,
> > > +				 struct list_head *compound_list,
> > > +				 struct pmu_events_map *map,
> > > +				 const char *metric_name)
> > > +{
> > > +	struct hashmap_entry *cur, *cur_tmp;
> > > +	struct metric *metric, *tmp;
> > > +	size_t bkt;
> > > +	bool all;
> > > +	int rc;
> > > +
> > > +	do {
> > > +		all = true;
> > > +		hashmap__for_each_entry_safe((&pctx->ids), cur, cur_tmp, bkt) {
> > > +			struct metric_ref *ref;
> > > +			struct pmu_event *pe;
> > > +
> > > +			pe = metrcgroup_find_metric(cur->key, map);
> 
> *
> 
> > > +			if (!pe)
> > > +				continue;
> > > +
> > > +			if (!strcmp(metric_name, (char *)cur->key)) {
> > > +				pr_warning("Recursion detected for metric %s\n", metric_name);
> > > +				rc = -1;
> > > +				goto out_err;
> > > +			}
> > > +
> > > +			all = false;
> > > +
> > > +			/* The metric key itself needs to go out.. */
> > > +			expr__del_id(pctx, cur->key);
> > > +
> > > +			metric = malloc(sizeof(*metric));
> > > +			if (!metric) {
> > > +				rc = -ENOMEM;
> > > +				goto out_err;
> > > +			}
> > > +
> > > +			ref = &metric->metric_ref;
> > > +			ref->metric_name = pe->metric_name;
> > > +			ref->metric_expr = pe->metric_expr;
> > > +			list_add_tail(&metric->list, compound_list);
> > > +
> > > +			rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> > 
> 
> Hi Jirka,
> 
> > so this might add new items to pctx->ids, I think you need
> > to restart the iteration as we do it in __resolve_metric
> > otherwise you could miss some new keys
> 
> I thought that I was doing this. Indeed, this code is very much like
> __resolve_metric() ;)
> 
> So expr__find_other() may add a new item to pctx->ids, and we always iterate
> again, and try to lookup any pmu_events, *, above. If none exist, then we

hm, I don't see that.. so, what you do is:

	hashmap__for_each_entry_safe((&pctx->ids) ....) {

		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
	}

and what I think we need to do is:

	hashmap__for_each_entry_safe((&pctx->ids) ....) {

		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);

		break;	
	}

each time you resolve another metric, you need to restart
the pctx->ids iteration, because there will be new items,
and we are in the middle of it

jirka


> have broken down pctx into primitive events aliases and unresolvable
> metrics, and stop iterating. And then unresolvable metrics would be found in
> check_parse_cpu().
> 
> As an example, we can deal with metric test1, below, which references 2x
> other metrics:
> 
>     {
>         "MetricExpr": "IDQ_UOPS_NOT_DELIVERED.CORE / (4 * (( (
> CPU_CLK_UNHALTED.THREAD / 2 ) * ( 1 + CPU_CLK_UNHALTED.ONE_THREAD_ACTIVE /
> CPU_CLK_UNHALTED.REF_XCLK ) )))",
>       "MetricName": "Frontend_Bound",
>     },
>     {
>         "MetricExpr": "( UOPS_ISSUED.ANY - UOPS_RETIRED.RETIRE_SLOTS + 4 *
> INT_MISC.RECOVERY_CYCLES ) / (4 * cycles)",
>         "MetricName": "Bad_Speculation",
>     },
>     {
>         "MetricExpr": "Bad_Speculation + Frontend_Bound",
>         "MetricName": "test1",
>     },
> 
> Does that satisfy your concern, or have I missed something?
> 
> Thanks,
> John
> 
> > 
> > jirka
> > 
> > > +			if (rc)
> > > +				goto out_err;
> > > +		}
> > > +	} while (!all);
> > > +
> > > +	return 0;
> > > +
> > > +out_err:
> > > +	list_for_each_entry_safe(metric, tmp, compound_list, list)
> > > +		free(metric);
> > > +
> > > +	return rc;
> > > +
> > > +}
> > 
> > SNIP
> > 
> > .
> > 
> 


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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
@ 2021-04-06 12:17         ` Jiri Olsa
  0 siblings, 0 replies; 52+ messages in thread
From: Jiri Olsa @ 2021-04-06 12:17 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On Tue, Apr 06, 2021 at 12:00:27PM +0100, John Garry wrote:
> On 01/04/2021 14:49, Jiri Olsa wrote:
> > On Thu, Mar 25, 2021 at 06:33:14PM +0800, John Garry wrote:
> > 
> > SNIP
> > 
> > > +struct metric {
> > > +	struct list_head list;
> > > +	struct metric_ref metric_ref;
> > > +};
> > > +
> > > +static int resolve_metric_simple(struct expr_parse_ctx *pctx,
> > > +				 struct list_head *compound_list,
> > > +				 struct pmu_events_map *map,
> > > +				 const char *metric_name)
> > > +{
> > > +	struct hashmap_entry *cur, *cur_tmp;
> > > +	struct metric *metric, *tmp;
> > > +	size_t bkt;
> > > +	bool all;
> > > +	int rc;
> > > +
> > > +	do {
> > > +		all = true;
> > > +		hashmap__for_each_entry_safe((&pctx->ids), cur, cur_tmp, bkt) {
> > > +			struct metric_ref *ref;
> > > +			struct pmu_event *pe;
> > > +
> > > +			pe = metrcgroup_find_metric(cur->key, map);
> 
> *
> 
> > > +			if (!pe)
> > > +				continue;
> > > +
> > > +			if (!strcmp(metric_name, (char *)cur->key)) {
> > > +				pr_warning("Recursion detected for metric %s\n", metric_name);
> > > +				rc = -1;
> > > +				goto out_err;
> > > +			}
> > > +
> > > +			all = false;
> > > +
> > > +			/* The metric key itself needs to go out.. */
> > > +			expr__del_id(pctx, cur->key);
> > > +
> > > +			metric = malloc(sizeof(*metric));
> > > +			if (!metric) {
> > > +				rc = -ENOMEM;
> > > +				goto out_err;
> > > +			}
> > > +
> > > +			ref = &metric->metric_ref;
> > > +			ref->metric_name = pe->metric_name;
> > > +			ref->metric_expr = pe->metric_expr;
> > > +			list_add_tail(&metric->list, compound_list);
> > > +
> > > +			rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> > 
> 
> Hi Jirka,
> 
> > so this might add new items to pctx->ids, I think you need
> > to restart the iteration as we do it in __resolve_metric
> > otherwise you could miss some new keys
> 
> I thought that I was doing this. Indeed, this code is very much like
> __resolve_metric() ;)
> 
> So expr__find_other() may add a new item to pctx->ids, and we always iterate
> again, and try to lookup any pmu_events, *, above. If none exist, then we

hm, I don't see that.. so, what you do is:

	hashmap__for_each_entry_safe((&pctx->ids) ....) {

		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
	}

and what I think we need to do is:

	hashmap__for_each_entry_safe((&pctx->ids) ....) {

		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);

		break;	
	}

each time you resolve another metric, you need to restart
the pctx->ids iteration, because there will be new items,
and we are in the middle of it

jirka


> have broken down pctx into primitive events aliases and unresolvable
> metrics, and stop iterating. And then unresolvable metrics would be found in
> check_parse_cpu().
> 
> As an example, we can deal with metric test1, below, which references 2x
> other metrics:
> 
>     {
>         "MetricExpr": "IDQ_UOPS_NOT_DELIVERED.CORE / (4 * (( (
> CPU_CLK_UNHALTED.THREAD / 2 ) * ( 1 + CPU_CLK_UNHALTED.ONE_THREAD_ACTIVE /
> CPU_CLK_UNHALTED.REF_XCLK ) )))",
>       "MetricName": "Frontend_Bound",
>     },
>     {
>         "MetricExpr": "( UOPS_ISSUED.ANY - UOPS_RETIRED.RETIRE_SLOTS + 4 *
> INT_MISC.RECOVERY_CYCLES ) / (4 * cycles)",
>         "MetricName": "Bad_Speculation",
>     },
>     {
>         "MetricExpr": "Bad_Speculation + Frontend_Bound",
>         "MetricName": "test1",
>     },
> 
> Does that satisfy your concern, or have I missed something?
> 
> Thanks,
> John
> 
> > 
> > jirka
> > 
> > > +			if (rc)
> > > +				goto out_err;
> > > +		}
> > > +	} while (!all);
> > > +
> > > +	return 0;
> > > +
> > > +out_err:
> > > +	list_for_each_entry_safe(metric, tmp, compound_list, list)
> > > +		free(metric);
> > > +
> > > +	return rc;
> > > +
> > > +}
> > 
> > SNIP
> > 
> > .
> > 
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* RE: [PATCH v2 0/6] perf arm64 metricgroup support
  2021-04-06 11:02           ` John Garry
@ 2021-04-06 12:18             ` Paul A. Clarke
  -1 siblings, 0 replies; 52+ messages in thread
From: Paul A. Clarke @ 2021-04-06 12:18 UTC (permalink / raw)
  To: John Garry
  Cc: kajoljain, will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun

On Tue, Apr 06, 2021 at 12:02:36PM +0100, John Garry wrote:
> On 30/03/2021 07:41, kajoljain wrote:
> > On 3/30/21 2:37 AM, Paul A. Clarke wrote:
> > > On Fri, Mar 26, 2021 at 10:57:40AM +0000, John Garry wrote:
> > > > On 25/03/2021 20:39, Paul A. Clarke wrote:
> > > > > On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
> > > > > > Metric reuse support is added for pmu-events parse metric testcase.
> > > > > > This had been broken on power9 recentlty:
> > > > > > https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/
> > > > > 
> > > > > Much better.  Before:
> > > > > --
> > > > > $ perf test -v 10 2>&1 | grep -i error | wc -l
> > > > > 112
> > > > > --
> > > > > After:
> > > > > --
> > > > > $ perf test -v 10 2>&1 | grep -i error | wc -l
> > > > > 17
> > > > > --
> > > > > 
> > > > > And these seem like different types of issues:
> > > > > --
> > > > > $ perf test -v 10 2>&1 | grep -i error
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > --
> > > > > 
> > > > 
> > > > This looks suspicious.
> > > > 
> > > > Firstly, does /sys/bus/event_source/devices/nest_mcs01_imc (or others,
> > > > above) exist on your system? I guess not.
> > > > 
> > > > Secondly, checking Documentation/powerpc/imc.rst, we have examples of:
> > > > nest_mcs01/PM_MCS01_64B_R...
> > > > 
> > > > So is the PMU name correct in the metric file for nest_mcs01_imc? Looking at
> > > > the kernel driver file, arch/powerpc/perf/imc-pmu.c, it seems to be correct.
> > > > Not sure.
> > > 
> > > I ran with a newer kernel, and the above errors disappeared, replaced with
> > > about 10 of:
> > > --
> > > Error string 'Cannot find PMU `hv_24x7'. Missing kernel support?' help '(null)'
> > > --
> > > 
> > > ...but I was running without a hypervisor, so I tried the same kernel on a
> > > PowerVM-virtualized system and the "hv_24x7" messages went away, but the
> > > "nest" messages returned.  This may all be expected behavior... I confess
> > > I haven't followed these new perf capabilities closely.
> > > 
> > 
> > Hi Paul/John,
> >     This is something expected. For nest-imc we need bare-metal system and for
> > hv-24x7 we need VM environment. Since you are checking this test in VM machine,
> > there nest events are not supported and hence we are getting this error.
> > 
> > Thanks,
> > Kajol Jain
> 
> Cool, so I hope that tested-by or similar can be provided :) [obviously
> pending any changes that come from reviews]

Certainly!

Tested-by: Paul A. Clarke <pc@us.ibm.com>

PC

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

* RE: [PATCH v2 0/6] perf arm64 metricgroup support
@ 2021-04-06 12:18             ` Paul A. Clarke
  0 siblings, 0 replies; 52+ messages in thread
From: Paul A. Clarke @ 2021-04-06 12:18 UTC (permalink / raw)
  To: John Garry
  Cc: kajoljain, will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers,
	linuxarm, kan.liang, linux-kernel, linux-arm-kernel,
	zhangshaokun

On Tue, Apr 06, 2021 at 12:02:36PM +0100, John Garry wrote:
> On 30/03/2021 07:41, kajoljain wrote:
> > On 3/30/21 2:37 AM, Paul A. Clarke wrote:
> > > On Fri, Mar 26, 2021 at 10:57:40AM +0000, John Garry wrote:
> > > > On 25/03/2021 20:39, Paul A. Clarke wrote:
> > > > > On Thu, Mar 25, 2021 at 06:33:12PM +0800, John Garry wrote:
> > > > > > Metric reuse support is added for pmu-events parse metric testcase.
> > > > > > This had been broken on power9 recentlty:
> > > > > > https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/
> > > > > 
> > > > > Much better.  Before:
> > > > > --
> > > > > $ perf test -v 10 2>&1 | grep -i error | wc -l
> > > > > 112
> > > > > --
> > > > > After:
> > > > > --
> > > > > $ perf test -v 10 2>&1 | grep -i error | wc -l
> > > > > 17
> > > > > --
> > > > > 
> > > > > And these seem like different types of issues:
> > > > > --
> > > > > $ perf test -v 10 2>&1 | grep -i error
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_powerbus0_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs01_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > Error string 'Cannot find PMU `nest_mcs23_imc'. Missing kernel support?' help '(null)'
> > > > > --
> > > > > 
> > > > 
> > > > This looks suspicious.
> > > > 
> > > > Firstly, does /sys/bus/event_source/devices/nest_mcs01_imc (or others,
> > > > above) exist on your system? I guess not.
> > > > 
> > > > Secondly, checking Documentation/powerpc/imc.rst, we have examples of:
> > > > nest_mcs01/PM_MCS01_64B_R...
> > > > 
> > > > So is the PMU name correct in the metric file for nest_mcs01_imc? Looking at
> > > > the kernel driver file, arch/powerpc/perf/imc-pmu.c, it seems to be correct.
> > > > Not sure.
> > > 
> > > I ran with a newer kernel, and the above errors disappeared, replaced with
> > > about 10 of:
> > > --
> > > Error string 'Cannot find PMU `hv_24x7'. Missing kernel support?' help '(null)'
> > > --
> > > 
> > > ...but I was running without a hypervisor, so I tried the same kernel on a
> > > PowerVM-virtualized system and the "hv_24x7" messages went away, but the
> > > "nest" messages returned.  This may all be expected behavior... I confess
> > > I haven't followed these new perf capabilities closely.
> > > 
> > 
> > Hi Paul/John,
> >     This is something expected. For nest-imc we need bare-metal system and for
> > hv-24x7 we need VM environment. Since you are checking this test in VM machine,
> > there nest events are not supported and hence we are getting this error.
> > 
> > Thanks,
> > Kajol Jain
> 
> Cool, so I hope that tested-by or similar can be provided :) [obviously
> pending any changes that come from reviews]

Certainly!

Tested-by: Paul A. Clarke <pc@us.ibm.com>

PC

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
  2021-04-06 12:17         ` Jiri Olsa
@ 2021-04-06 12:43           ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-04-06 12:43 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On 06/04/2021 13:17, Jiri Olsa wrote:
>>>> +			ref = &metric->metric_ref;
>>>> +			ref->metric_name = pe->metric_name;
>>>> +			ref->metric_expr = pe->metric_expr;
>>>> +			list_add_tail(&metric->list, compound_list);
>>>> +
>>>> +			rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
>> Hi Jirka,
>>
>>> so this might add new items to pctx->ids, I think you need
>>> to restart the iteration as we do it in __resolve_metric
>>> otherwise you could miss some new keys
>> I thought that I was doing this. Indeed, this code is very much like
>> __resolve_metric();)
>>
>> So expr__find_other() may add a new item to pctx->ids, and we always iterate
>> again, and try to lookup any pmu_events, *, above. If none exist, then we
> hm, I don't see that.. so, what you do is:
> 
> 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
> 
> 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> 	}
> 
> and what I think we need to do is:
> 
> 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
> 
> 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> 
> 		break;	
> 	}
> 
> each time you resolve another metric, you need to restart
> the pctx->ids iteration, because there will be new items,
> and we are in the middle of it

Sure, but we will restart anyway.

Regardless of this, I don't think what I am doing is safe, i.e. adding 
new items in the middle of the iter, so I will change in the way you 
suggest.

Thanks,
John

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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
@ 2021-04-06 12:43           ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-04-06 12:43 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On 06/04/2021 13:17, Jiri Olsa wrote:
>>>> +			ref = &metric->metric_ref;
>>>> +			ref->metric_name = pe->metric_name;
>>>> +			ref->metric_expr = pe->metric_expr;
>>>> +			list_add_tail(&metric->list, compound_list);
>>>> +
>>>> +			rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
>> Hi Jirka,
>>
>>> so this might add new items to pctx->ids, I think you need
>>> to restart the iteration as we do it in __resolve_metric
>>> otherwise you could miss some new keys
>> I thought that I was doing this. Indeed, this code is very much like
>> __resolve_metric();)
>>
>> So expr__find_other() may add a new item to pctx->ids, and we always iterate
>> again, and try to lookup any pmu_events, *, above. If none exist, then we
> hm, I don't see that.. so, what you do is:
> 
> 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
> 
> 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> 	}
> 
> and what I think we need to do is:
> 
> 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
> 
> 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> 
> 		break;	
> 	}
> 
> each time you resolve another metric, you need to restart
> the pctx->ids iteration, because there will be new items,
> and we are in the middle of it

Sure, but we will restart anyway.

Regardless of this, I don't think what I am doing is safe, i.e. adding 
new items in the middle of the iter, so I will change in the way you 
suggest.

Thanks,
John

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
  2021-04-06 12:43           ` John Garry
@ 2021-04-06 12:55             ` Jiri Olsa
  -1 siblings, 0 replies; 52+ messages in thread
From: Jiri Olsa @ 2021-04-06 12:55 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On Tue, Apr 06, 2021 at 01:43:09PM +0100, John Garry wrote:
> On 06/04/2021 13:17, Jiri Olsa wrote:
> > > > > +			ref = &metric->metric_ref;
> > > > > +			ref->metric_name = pe->metric_name;
> > > > > +			ref->metric_expr = pe->metric_expr;
> > > > > +			list_add_tail(&metric->list, compound_list);
> > > > > +
> > > > > +			rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> > > Hi Jirka,
> > > 
> > > > so this might add new items to pctx->ids, I think you need
> > > > to restart the iteration as we do it in __resolve_metric
> > > > otherwise you could miss some new keys
> > > I thought that I was doing this. Indeed, this code is very much like
> > > __resolve_metric();)
> > > 
> > > So expr__find_other() may add a new item to pctx->ids, and we always iterate
> > > again, and try to lookup any pmu_events, *, above. If none exist, then we
> > hm, I don't see that.. so, what you do is:
> > 
> > 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
> > 
> > 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> > 	}
> > 
> > and what I think we need to do is:
> > 
> > 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
> > 
> > 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> > 
> > 		break;	
> > 	}
> > 
> > each time you resolve another metric, you need to restart
> > the pctx->ids iteration, because there will be new items,
> > and we are in the middle of it
> 
> Sure, but we will restart anyway.

hum, where? you call expr__find_other and continue to next
pctx->ids item

> 
> Regardless of this, I don't think what I am doing is safe, i.e. adding new
> items in the middle of the iter, so I will change in the way you suggest.

it'll always add items in the middle of the iteration

jirka

> 
> Thanks,
> John
> 


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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
@ 2021-04-06 12:55             ` Jiri Olsa
  0 siblings, 0 replies; 52+ messages in thread
From: Jiri Olsa @ 2021-04-06 12:55 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On Tue, Apr 06, 2021 at 01:43:09PM +0100, John Garry wrote:
> On 06/04/2021 13:17, Jiri Olsa wrote:
> > > > > +			ref = &metric->metric_ref;
> > > > > +			ref->metric_name = pe->metric_name;
> > > > > +			ref->metric_expr = pe->metric_expr;
> > > > > +			list_add_tail(&metric->list, compound_list);
> > > > > +
> > > > > +			rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> > > Hi Jirka,
> > > 
> > > > so this might add new items to pctx->ids, I think you need
> > > > to restart the iteration as we do it in __resolve_metric
> > > > otherwise you could miss some new keys
> > > I thought that I was doing this. Indeed, this code is very much like
> > > __resolve_metric();)
> > > 
> > > So expr__find_other() may add a new item to pctx->ids, and we always iterate
> > > again, and try to lookup any pmu_events, *, above. If none exist, then we
> > hm, I don't see that.. so, what you do is:
> > 
> > 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
> > 
> > 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> > 	}
> > 
> > and what I think we need to do is:
> > 
> > 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
> > 
> > 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> > 
> > 		break;	
> > 	}
> > 
> > each time you resolve another metric, you need to restart
> > the pctx->ids iteration, because there will be new items,
> > and we are in the middle of it
> 
> Sure, but we will restart anyway.

hum, where? you call expr__find_other and continue to next
pctx->ids item

> 
> Regardless of this, I don't think what I am doing is safe, i.e. adding new
> items in the middle of the iter, so I will change in the way you suggest.

it'll always add items in the middle of the iteration

jirka

> 
> Thanks,
> John
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
  2021-04-06 12:55             ` Jiri Olsa
@ 2021-04-06 13:21               ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-04-06 13:21 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On 06/04/2021 13:55, Jiri Olsa wrote:
>>>> So expr__find_other() may add a new item to pctx->ids, and we always iterate
>>>> again, and try to lookup any pmu_events, *, above. If none exist, then we
>>> hm, I don't see that.. so, what you do is:
>>>
>>> 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
>>>
>>> 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
>>> 	}
>>>
>>> and what I think we need to do is:
>>>
>>> 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
>>>
>>> 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
>>>
>>> 		break;	
>>> 	}
>>>
>>> each time you resolve another metric, you need to restart
>>> the pctx->ids iteration, because there will be new items,
>>> and we are in the middle of it
>> Sure, but we will restart anyway.
> hum, where? you call expr__find_other and continue to next
> pctx->ids item

We have:

resolve_metric_simple()
{
	bool all;

	do {
		all = true;

		hashmap__for_each_entry_safe(&pctx->ids, ...) {

			pe = metricgroup_find_metric(cur->key, map);
			if (!pe)
				continue;

			...
			all = false;

			expr_del_id(pctx, cur->key);

			...
			rc = expr__find_other(pe->metric_expr, pctx);
			if (rc)
				goto out_err;
		}

	} while (!all);

}

So once we evaluate a pmu_event in pctx->ids in @pe, @all is set false, 
and we would loop again in the do-while loop, regardless of what 
expr__find_other() does (apart from erroring), and so call 
hashmap__for_each_entry_safe(&pctx->ids, ) again.

This is really what is done in __resolve_metric() - indeed, I would use 
that function directly, but it looks hard to extract that from 
metricgroup.c .

Thanks,
John

> 
>> Regardless of this, I don't think what I am doing is safe, i.e. adding new
>> items in the middle of the iter, so I will change in the way you suggest.
> it'll always add items in the middle of the iteration


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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
@ 2021-04-06 13:21               ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-04-06 13:21 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On 06/04/2021 13:55, Jiri Olsa wrote:
>>>> So expr__find_other() may add a new item to pctx->ids, and we always iterate
>>>> again, and try to lookup any pmu_events, *, above. If none exist, then we
>>> hm, I don't see that.. so, what you do is:
>>>
>>> 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
>>>
>>> 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
>>> 	}
>>>
>>> and what I think we need to do is:
>>>
>>> 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
>>>
>>> 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
>>>
>>> 		break;	
>>> 	}
>>>
>>> each time you resolve another metric, you need to restart
>>> the pctx->ids iteration, because there will be new items,
>>> and we are in the middle of it
>> Sure, but we will restart anyway.
> hum, where? you call expr__find_other and continue to next
> pctx->ids item

We have:

resolve_metric_simple()
{
	bool all;

	do {
		all = true;

		hashmap__for_each_entry_safe(&pctx->ids, ...) {

			pe = metricgroup_find_metric(cur->key, map);
			if (!pe)
				continue;

			...
			all = false;

			expr_del_id(pctx, cur->key);

			...
			rc = expr__find_other(pe->metric_expr, pctx);
			if (rc)
				goto out_err;
		}

	} while (!all);

}

So once we evaluate a pmu_event in pctx->ids in @pe, @all is set false, 
and we would loop again in the do-while loop, regardless of what 
expr__find_other() does (apart from erroring), and so call 
hashmap__for_each_entry_safe(&pctx->ids, ) again.

This is really what is done in __resolve_metric() - indeed, I would use 
that function directly, but it looks hard to extract that from 
metricgroup.c .

Thanks,
John

> 
>> Regardless of this, I don't think what I am doing is safe, i.e. adding new
>> items in the middle of the iter, so I will change in the way you suggest.
> it'll always add items in the middle of the iteration


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
  2021-04-06 13:21               ` John Garry
@ 2021-04-06 13:34                 ` Jiri Olsa
  -1 siblings, 0 replies; 52+ messages in thread
From: Jiri Olsa @ 2021-04-06 13:34 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On Tue, Apr 06, 2021 at 02:21:11PM +0100, John Garry wrote:
> On 06/04/2021 13:55, Jiri Olsa wrote:
> > > > > So expr__find_other() may add a new item to pctx->ids, and we always iterate
> > > > > again, and try to lookup any pmu_events, *, above. If none exist, then we
> > > > hm, I don't see that.. so, what you do is:
> > > > 
> > > > 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
> > > > 
> > > > 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> > > > 	}
> > > > 
> > > > and what I think we need to do is:
> > > > 
> > > > 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
> > > > 
> > > > 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> > > > 
> > > > 		break;	
> > > > 	}
> > > > 
> > > > each time you resolve another metric, you need to restart
> > > > the pctx->ids iteration, because there will be new items,
> > > > and we are in the middle of it
> > > Sure, but we will restart anyway.
> > hum, where? you call expr__find_other and continue to next
> > pctx->ids item
> 
> We have:
> 
> resolve_metric_simple()
> {
> 	bool all;
> 
> 	do {
> 		all = true;
> 
> 		hashmap__for_each_entry_safe(&pctx->ids, ...) {
> 
> 			pe = metricgroup_find_metric(cur->key, map);
> 			if (!pe)
> 				continue;
> 
> 			...
> 			all = false;
> 
> 			expr_del_id(pctx, cur->key);
> 
> 			...
> 			rc = expr__find_other(pe->metric_expr, pctx);
> 			if (rc)
> 				goto out_err;
> 		}
> 
> 	} while (!all);
> 
> }
> 
> So once we evaluate a pmu_event in pctx->ids in @pe, @all is set false, and
> we would loop again in the do-while loop, regardless of what
> expr__find_other() does (apart from erroring), and so call
> hashmap__for_each_entry_safe(&pctx->ids, ) again.

ah ok, so it finishes the hash iteration first and
then restarts it.. ok, I missed that, then it's fine

> 
> This is really what is done in __resolve_metric() - indeed, I would use that
> function directly, but it looks hard to extract that from metricgroup.c .

yea, it's another world ;-) it's better to keep it separated

thanks,
jirka

> 
> Thanks,
> John
> 
> > 
> > > Regardless of this, I don't think what I am doing is safe, i.e. adding new
> > > items in the middle of the iter, so I will change in the way you suggest.
> > it'll always add items in the middle of the iteration
> 


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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
@ 2021-04-06 13:34                 ` Jiri Olsa
  0 siblings, 0 replies; 52+ messages in thread
From: Jiri Olsa @ 2021-04-06 13:34 UTC (permalink / raw)
  To: John Garry
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On Tue, Apr 06, 2021 at 02:21:11PM +0100, John Garry wrote:
> On 06/04/2021 13:55, Jiri Olsa wrote:
> > > > > So expr__find_other() may add a new item to pctx->ids, and we always iterate
> > > > > again, and try to lookup any pmu_events, *, above. If none exist, then we
> > > > hm, I don't see that.. so, what you do is:
> > > > 
> > > > 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
> > > > 
> > > > 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> > > > 	}
> > > > 
> > > > and what I think we need to do is:
> > > > 
> > > > 	hashmap__for_each_entry_safe((&pctx->ids) ....) {
> > > > 
> > > > 		rc = expr__find_other(pe->metric_expr, NULL, pctx, 0);
> > > > 
> > > > 		break;	
> > > > 	}
> > > > 
> > > > each time you resolve another metric, you need to restart
> > > > the pctx->ids iteration, because there will be new items,
> > > > and we are in the middle of it
> > > Sure, but we will restart anyway.
> > hum, where? you call expr__find_other and continue to next
> > pctx->ids item
> 
> We have:
> 
> resolve_metric_simple()
> {
> 	bool all;
> 
> 	do {
> 		all = true;
> 
> 		hashmap__for_each_entry_safe(&pctx->ids, ...) {
> 
> 			pe = metricgroup_find_metric(cur->key, map);
> 			if (!pe)
> 				continue;
> 
> 			...
> 			all = false;
> 
> 			expr_del_id(pctx, cur->key);
> 
> 			...
> 			rc = expr__find_other(pe->metric_expr, pctx);
> 			if (rc)
> 				goto out_err;
> 		}
> 
> 	} while (!all);
> 
> }
> 
> So once we evaluate a pmu_event in pctx->ids in @pe, @all is set false, and
> we would loop again in the do-while loop, regardless of what
> expr__find_other() does (apart from erroring), and so call
> hashmap__for_each_entry_safe(&pctx->ids, ) again.

ah ok, so it finishes the hash iteration first and
then restarts it.. ok, I missed that, then it's fine

> 
> This is really what is done in __resolve_metric() - indeed, I would use that
> function directly, but it looks hard to extract that from metricgroup.c .

yea, it's another world ;-) it's better to keep it separated

thanks,
jirka

> 
> Thanks,
> John
> 
> > 
> > > Regardless of this, I don't think what I am doing is safe, i.e. adding new
> > > items in the middle of the iter, so I will change in the way you suggest.
> > it'll always add items in the middle of the iteration
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
  2021-04-06 13:34                 ` Jiri Olsa
@ 2021-04-06 13:38                   ` John Garry
  -1 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-04-06 13:38 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On 06/04/2021 14:34, Jiri Olsa wrote:
>>
>> }
>>
>> So once we evaluate a pmu_event in pctx->ids in @pe, @all is set false, and
>> we would loop again in the do-while loop, regardless of what
>> expr__find_other() does (apart from erroring), and so call
>> hashmap__for_each_entry_safe(&pctx->ids, ) again.
> ah ok, so it finishes the hash iteration first and
> then restarts it.. ok, I missed that, then it's fine
>  >> This is really what is done in __resolve_metric() - indeed, I would 
use that
>> function directly, but it looks hard to extract that from metricgroup.c .
> yea, it's another world;-)  it's better to keep it separated

ok, so I'll still add the break statement, as suggested.

I'll also wait to see if Ian or you have a strong feeling about the 
function naming in patch 1/6.

Thanks,
John


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

* Re: [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test
@ 2021-04-06 13:38                   ` John Garry
  0 siblings, 0 replies; 52+ messages in thread
From: John Garry @ 2021-04-06 13:38 UTC (permalink / raw)
  To: Jiri Olsa
  Cc: will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, namhyung, irogers, linuxarm,
	kjain, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun,
	pc

On 06/04/2021 14:34, Jiri Olsa wrote:
>>
>> }
>>
>> So once we evaluate a pmu_event in pctx->ids in @pe, @all is set false, and
>> we would loop again in the do-while loop, regardless of what
>> expr__find_other() does (apart from erroring), and so call
>> hashmap__for_each_entry_safe(&pctx->ids, ) again.
> ah ok, so it finishes the hash iteration first and
> then restarts it.. ok, I missed that, then it's fine
>  >> This is really what is done in __resolve_metric() - indeed, I would 
use that
>> function directly, but it looks hard to extract that from metricgroup.c .
> yea, it's another world;-)  it's better to keep it separated

ok, so I'll still add the break statement, as suggested.

I'll also wait to see if Ian or you have a strong feeling about the 
function naming in patch 1/6.

Thanks,
John


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 1/6] perf metricgroup: Make find_metric() public with name change
  2021-04-06  9:54       ` John Garry
@ 2021-04-07  5:39         ` kajoljain
  -1 siblings, 0 replies; 52+ messages in thread
From: kajoljain @ 2021-04-07  5:39 UTC (permalink / raw)
  To: John Garry, Ian Rogers
  Cc: Will Deacon, Mathieu Poirier, Leo Yan, Peter Zijlstra,
	Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, linuxarm, Liang,
	Kan, LKML, Linux ARM, zhangshaokun, Paul Clarke



On 4/6/21 3:24 PM, John Garry wrote:
> On 02/04/2021 00:16, Ian Rogers wrote:
>> On Thu, Mar 25, 2021 at 3:38 AM John Garry <john.garry@huawei.com> wrote:
>>>
>>> Function find_metric() is required for the metric processing in the
>>> pmu-events testcase, so make it public. Also change the name to include
>>> "metricgroup".
>>
>> Would it make more sense as "pmu_events_map__find_metric" ?
>>
> 
> So all functions apart from one in metricgroup.h are named metricgroup__XXX, so I was trying to keep this style - apart from the double-underscore (which can be remedied).
> 
> Personally I don't think pmu_events_map__find_metric name fits with that convention.

I agree, most of the functions in metricgroup.c named as metricgroup__xxx. May be something like metricgroup__find_metric will be better.

Thanks,
Kajol Jain

> 
> Thanks,
> John
> 
>> Thanks,
>> Ian
>>
>>> Signed-off-by: John Garry <john.garry@huawei.com>
>>> ---
>>>   tools/perf/util/metricgroup.c | 5 +++--
>>>   tools/perf/util/metricgroup.h | 3 ++-
>>>   2 files changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
>>> index 6acb44ad439b..71a13406e0bd 100644
>>> --- a/tools/perf/util/metricgroup.c
>>> +++ b/tools/perf/util/metricgroup.c
>>> @@ -900,7 +900,8 @@ static int __add_metric(struct list_head *metric_list,
>>>                      (match_metric(__pe->metric_group, __metric) ||      \
>>>                       match_metric(__pe->metric_name, __metric)))
>>>
>>> -static struct pmu_event *find_metric(const char *metric, struct pmu_events_map *map)
>>> +struct pmu_event *metrcgroup_find_metric(const char *metric,
>>> +                                        struct pmu_events_map *map)
>>>   {
>>>          struct pmu_event *pe;
>>>          int i;
>>> @@ -985,7 +986,7 @@ static int __resolve_metric(struct metric *m,
>>>                          struct expr_id *parent;
>>>                          struct pmu_event *pe;
>>>
>>> -                       pe = find_metric(cur->key, map);
>>> +                       pe = metrcgroup_find_metric(cur->key, map);
>>>                          if (!pe)
>>>                                  continue;
>>>
>>> diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
>>> index ed1b9392e624..1674c6a36d74 100644
>>> --- a/tools/perf/util/metricgroup.h
>>> +++ b/tools/perf/util/metricgroup.h
>>> @@ -44,7 +44,8 @@ int metricgroup__parse_groups(const struct option *opt,
>>>                                bool metric_no_group,
>>>                                bool metric_no_merge,
>>>                                struct rblist *metric_events);
>>> -
>>> +struct pmu_event *metrcgroup_find_metric(const char *metric,
>>> +                                        struct pmu_events_map *map);
>>>   int metricgroup__parse_groups_test(struct evlist *evlist,
>>>                                     struct pmu_events_map *map,
>>>                                     const char *str,
>>> -- 
>>> 2.26.2
>>>
>> .
>>
> 

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

* Re: [PATCH v2 1/6] perf metricgroup: Make find_metric() public with name change
@ 2021-04-07  5:39         ` kajoljain
  0 siblings, 0 replies; 52+ messages in thread
From: kajoljain @ 2021-04-07  5:39 UTC (permalink / raw)
  To: John Garry, Ian Rogers
  Cc: Will Deacon, Mathieu Poirier, Leo Yan, Peter Zijlstra,
	Ingo Molnar, Arnaldo Carvalho de Melo, Mark Rutland,
	Alexander Shishkin, Jiri Olsa, Namhyung Kim, linuxarm, Liang,
	Kan, LKML, Linux ARM, zhangshaokun, Paul Clarke



On 4/6/21 3:24 PM, John Garry wrote:
> On 02/04/2021 00:16, Ian Rogers wrote:
>> On Thu, Mar 25, 2021 at 3:38 AM John Garry <john.garry@huawei.com> wrote:
>>>
>>> Function find_metric() is required for the metric processing in the
>>> pmu-events testcase, so make it public. Also change the name to include
>>> "metricgroup".
>>
>> Would it make more sense as "pmu_events_map__find_metric" ?
>>
> 
> So all functions apart from one in metricgroup.h are named metricgroup__XXX, so I was trying to keep this style - apart from the double-underscore (which can be remedied).
> 
> Personally I don't think pmu_events_map__find_metric name fits with that convention.

I agree, most of the functions in metricgroup.c named as metricgroup__xxx. May be something like metricgroup__find_metric will be better.

Thanks,
Kajol Jain

> 
> Thanks,
> John
> 
>> Thanks,
>> Ian
>>
>>> Signed-off-by: John Garry <john.garry@huawei.com>
>>> ---
>>>   tools/perf/util/metricgroup.c | 5 +++--
>>>   tools/perf/util/metricgroup.h | 3 ++-
>>>   2 files changed, 5 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
>>> index 6acb44ad439b..71a13406e0bd 100644
>>> --- a/tools/perf/util/metricgroup.c
>>> +++ b/tools/perf/util/metricgroup.c
>>> @@ -900,7 +900,8 @@ static int __add_metric(struct list_head *metric_list,
>>>                      (match_metric(__pe->metric_group, __metric) ||      \
>>>                       match_metric(__pe->metric_name, __metric)))
>>>
>>> -static struct pmu_event *find_metric(const char *metric, struct pmu_events_map *map)
>>> +struct pmu_event *metrcgroup_find_metric(const char *metric,
>>> +                                        struct pmu_events_map *map)
>>>   {
>>>          struct pmu_event *pe;
>>>          int i;
>>> @@ -985,7 +986,7 @@ static int __resolve_metric(struct metric *m,
>>>                          struct expr_id *parent;
>>>                          struct pmu_event *pe;
>>>
>>> -                       pe = find_metric(cur->key, map);
>>> +                       pe = metrcgroup_find_metric(cur->key, map);
>>>                          if (!pe)
>>>                                  continue;
>>>
>>> diff --git a/tools/perf/util/metricgroup.h b/tools/perf/util/metricgroup.h
>>> index ed1b9392e624..1674c6a36d74 100644
>>> --- a/tools/perf/util/metricgroup.h
>>> +++ b/tools/perf/util/metricgroup.h
>>> @@ -44,7 +44,8 @@ int metricgroup__parse_groups(const struct option *opt,
>>>                                bool metric_no_group,
>>>                                bool metric_no_merge,
>>>                                struct rblist *metric_events);
>>> -
>>> +struct pmu_event *metrcgroup_find_metric(const char *metric,
>>> +                                        struct pmu_events_map *map);
>>>   int metricgroup__parse_groups_test(struct evlist *evlist,
>>>                                     struct pmu_events_map *map,
>>>                                     const char *str,
>>> -- 
>>> 2.26.2
>>>
>> .
>>
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

* Re: [PATCH v2 0/6] perf arm64 metricgroup support
  2021-03-25 10:33 ` John Garry
@ 2021-04-07  6:03   ` kajoljain
  -1 siblings, 0 replies; 52+ messages in thread
From: kajoljain @ 2021-04-07  6:03 UTC (permalink / raw)
  To: John Garry, will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun, pc



On 3/25/21 4:03 PM, John Garry wrote:
> This series contains support to get basic metricgroups working for
> arm64 CPUs.
> 
> Initial support is added for HiSilicon hip08 platform.
> 
> Some sample usage on Huawei D06 board:
> 
>  $ ./perf list metric    
> 
> List of pre-defined events (to be used in -e): 
> 
> Metrics:     
> 
>   bp_misp_flush
>        [BP misp flush L3 topdown metric]
>   branch_mispredicts
>        [Branch mispredicts L2 topdown metric]
>   core_bound
>        [Core bound L2 topdown metric]
>   divider
>        [Divider L3 topdown metric]
>   exe_ports_util
>        [EXE ports util L3 topdown metric]
>   fetch_bandwidth_bound
>        [Fetch bandwidth bound L2 topdown metric]
>   fetch_latency_bound
>        [Fetch latency bound L2 topdown metric]
>   fsu_stall
>        [FSU stall L3 topdown metric]
>   idle_by_icache_miss
> 
> $ sudo ./perf stat -v -M core_bound sleep 1
> Using CPUID 0x00000000480fd010
> metric expr (exe_stall_cycle - (mem_stall_anyload + armv8_pmuv3_0@event\=0x7005@)) / cpu_cycles for core_bound
> found event cpu_cycles
> found event armv8_pmuv3_0/event=0x7005/
> found event exe_stall_cycle
> found event mem_stall_anyload
> adding {cpu_cycles -> armv8_pmuv3_0/event=0x7001/
> mem_stall_anyload -> armv8_pmuv3_0/event=0x7004/
> Control descriptor is not initialized
> cpu_cycles: 989433 385050 385050
> armv8_pmuv3_0/event=0x7005/: 19207 385050 385050
> exe_stall_cycle: 900825 385050 385050
> mem_stall_anyload: 253516 385050 385050
> 
> Performance counter stats for 'sleep':
> 
> 989,433      cpu_cycles      #     0.63 core_bound
>   19,207      armv8_pmuv3_0/event=0x7005/
>  900,825      exe_stall_cycle
>  253,516      mem_stall_anyload
> 
>        0.000805809 seconds time elapsed
> 
>        0.000875000 seconds user
>        0.000000000 seconds sys
>        
> perf stat --topdown is not supported, as this requires the CPU PMU to
> expose (alias) events for the TopDown L1 metrics from sysfs, which arm 
> does not do. To get that to work, we probably need to make perf use the
> pmu-events cpumap to learn about those alias events.
> 
> Metric reuse support is added for pmu-events parse metric testcase.
> This had been broken on power9 recentlty:
> https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/ 

Patch set looks good to me.

Reviewed-By: Kajol Jain<kjain@linux.ibm.com>

Thanks,
Kajol jain

> 
> Differences to v1:
> - Add pmu_events_map__find() as arm64-specific function
> - Fix metric reuse for pmu-events parse metric testcase 
> 
> John Garry (6):
>   perf metricgroup: Make find_metric() public with name change
>   perf test: Handle metric reuse in pmu-events parsing test
>   perf pmu: Add pmu_events_map__find()
>   perf vendor events arm64: Add Hisi hip08 L1 metrics
>   perf vendor events arm64: Add Hisi hip08 L2 metrics
>   perf vendor events arm64: Add Hisi hip08 L3 metrics
> 
>  tools/perf/arch/arm64/util/Build              |   1 +
>  tools/perf/arch/arm64/util/pmu.c              |  25 ++
>  .../arch/arm64/hisilicon/hip08/metrics.json   | 233 ++++++++++++++++++
>  tools/perf/tests/pmu-events.c                 |  82 +++++-
>  tools/perf/util/metricgroup.c                 |  12 +-
>  tools/perf/util/metricgroup.h                 |   3 +-
>  tools/perf/util/pmu.c                         |   5 +
>  tools/perf/util/pmu.h                         |   1 +
>  tools/perf/util/s390-sample-raw.c             |   4 +-
>  9 files changed, 355 insertions(+), 11 deletions(-)
>  create mode 100644 tools/perf/arch/arm64/util/pmu.c
>  create mode 100644 tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
> 

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

* Re: [PATCH v2 0/6] perf arm64 metricgroup support
@ 2021-04-07  6:03   ` kajoljain
  0 siblings, 0 replies; 52+ messages in thread
From: kajoljain @ 2021-04-07  6:03 UTC (permalink / raw)
  To: John Garry, will, mathieu.poirier, leo.yan, peterz, mingo, acme,
	mark.rutland, alexander.shishkin, jolsa, namhyung, irogers
  Cc: linuxarm, kan.liang, linux-kernel, linux-arm-kernel, zhangshaokun, pc



On 3/25/21 4:03 PM, John Garry wrote:
> This series contains support to get basic metricgroups working for
> arm64 CPUs.
> 
> Initial support is added for HiSilicon hip08 platform.
> 
> Some sample usage on Huawei D06 board:
> 
>  $ ./perf list metric    
> 
> List of pre-defined events (to be used in -e): 
> 
> Metrics:     
> 
>   bp_misp_flush
>        [BP misp flush L3 topdown metric]
>   branch_mispredicts
>        [Branch mispredicts L2 topdown metric]
>   core_bound
>        [Core bound L2 topdown metric]
>   divider
>        [Divider L3 topdown metric]
>   exe_ports_util
>        [EXE ports util L3 topdown metric]
>   fetch_bandwidth_bound
>        [Fetch bandwidth bound L2 topdown metric]
>   fetch_latency_bound
>        [Fetch latency bound L2 topdown metric]
>   fsu_stall
>        [FSU stall L3 topdown metric]
>   idle_by_icache_miss
> 
> $ sudo ./perf stat -v -M core_bound sleep 1
> Using CPUID 0x00000000480fd010
> metric expr (exe_stall_cycle - (mem_stall_anyload + armv8_pmuv3_0@event\=0x7005@)) / cpu_cycles for core_bound
> found event cpu_cycles
> found event armv8_pmuv3_0/event=0x7005/
> found event exe_stall_cycle
> found event mem_stall_anyload
> adding {cpu_cycles -> armv8_pmuv3_0/event=0x7001/
> mem_stall_anyload -> armv8_pmuv3_0/event=0x7004/
> Control descriptor is not initialized
> cpu_cycles: 989433 385050 385050
> armv8_pmuv3_0/event=0x7005/: 19207 385050 385050
> exe_stall_cycle: 900825 385050 385050
> mem_stall_anyload: 253516 385050 385050
> 
> Performance counter stats for 'sleep':
> 
> 989,433      cpu_cycles      #     0.63 core_bound
>   19,207      armv8_pmuv3_0/event=0x7005/
>  900,825      exe_stall_cycle
>  253,516      mem_stall_anyload
> 
>        0.000805809 seconds time elapsed
> 
>        0.000875000 seconds user
>        0.000000000 seconds sys
>        
> perf stat --topdown is not supported, as this requires the CPU PMU to
> expose (alias) events for the TopDown L1 metrics from sysfs, which arm 
> does not do. To get that to work, we probably need to make perf use the
> pmu-events cpumap to learn about those alias events.
> 
> Metric reuse support is added for pmu-events parse metric testcase.
> This had been broken on power9 recentlty:
> https://lore.kernel.org/lkml/20210324015418.GC8931@li-24c3614c-2adc-11b2-a85c-85f334518bdb.ibm.com/ 

Patch set looks good to me.

Reviewed-By: Kajol Jain<kjain@linux.ibm.com>

Thanks,
Kajol jain

> 
> Differences to v1:
> - Add pmu_events_map__find() as arm64-specific function
> - Fix metric reuse for pmu-events parse metric testcase 
> 
> John Garry (6):
>   perf metricgroup: Make find_metric() public with name change
>   perf test: Handle metric reuse in pmu-events parsing test
>   perf pmu: Add pmu_events_map__find()
>   perf vendor events arm64: Add Hisi hip08 L1 metrics
>   perf vendor events arm64: Add Hisi hip08 L2 metrics
>   perf vendor events arm64: Add Hisi hip08 L3 metrics
> 
>  tools/perf/arch/arm64/util/Build              |   1 +
>  tools/perf/arch/arm64/util/pmu.c              |  25 ++
>  .../arch/arm64/hisilicon/hip08/metrics.json   | 233 ++++++++++++++++++
>  tools/perf/tests/pmu-events.c                 |  82 +++++-
>  tools/perf/util/metricgroup.c                 |  12 +-
>  tools/perf/util/metricgroup.h                 |   3 +-
>  tools/perf/util/pmu.c                         |   5 +
>  tools/perf/util/pmu.h                         |   1 +
>  tools/perf/util/s390-sample-raw.c             |   4 +-
>  9 files changed, 355 insertions(+), 11 deletions(-)
>  create mode 100644 tools/perf/arch/arm64/util/pmu.c
>  create mode 100644 tools/perf/pmu-events/arch/arm64/hisilicon/hip08/metrics.json
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

end of thread, other threads:[~2021-04-07  6:06 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-25 10:33 [PATCH v2 0/6] perf arm64 metricgroup support John Garry
2021-03-25 10:33 ` John Garry
2021-03-25 10:33 ` [PATCH v2 1/6] perf metricgroup: Make find_metric() public with name change John Garry
2021-03-25 10:33   ` John Garry
2021-04-01 23:16   ` Ian Rogers
2021-04-01 23:16     ` Ian Rogers
2021-04-06  9:54     ` John Garry
2021-04-06  9:54       ` John Garry
2021-04-07  5:39       ` kajoljain
2021-04-07  5:39         ` kajoljain
2021-03-25 10:33 ` [PATCH v2 2/6] perf test: Handle metric reuse in pmu-events parsing test John Garry
2021-03-25 10:33   ` John Garry
2021-04-01 13:49   ` Jiri Olsa
2021-04-01 13:49     ` Jiri Olsa
2021-04-06 11:00     ` John Garry
2021-04-06 11:00       ` John Garry
2021-04-06 12:17       ` Jiri Olsa
2021-04-06 12:17         ` Jiri Olsa
2021-04-06 12:43         ` John Garry
2021-04-06 12:43           ` John Garry
2021-04-06 12:55           ` Jiri Olsa
2021-04-06 12:55             ` Jiri Olsa
2021-04-06 13:21             ` John Garry
2021-04-06 13:21               ` John Garry
2021-04-06 13:34               ` Jiri Olsa
2021-04-06 13:34                 ` Jiri Olsa
2021-04-06 13:38                 ` John Garry
2021-04-06 13:38                   ` John Garry
2021-03-25 10:33 ` [PATCH v2 3/6] perf pmu: Add pmu_events_map__find() John Garry
2021-03-25 10:33   ` John Garry
2021-03-25 10:33 ` [PATCH v2 4/6] perf vendor events arm64: Add Hisi hip08 L1 metrics John Garry
2021-03-25 10:33   ` John Garry
2021-03-25 10:33 ` [PATCH v2 5/6] perf vendor events arm64: Add Hisi hip08 L2 metrics John Garry
2021-03-25 10:33   ` John Garry
2021-03-25 10:33 ` [PATCH v2 6/6] perf vendor events arm64: Add Hisi hip08 L3 metrics John Garry
2021-03-25 10:33   ` John Garry
2021-03-25 20:39 ` [PATCH v2 0/6] perf arm64 metricgroup support Paul A. Clarke
2021-03-25 20:39   ` Paul A. Clarke
2021-03-26 10:57   ` John Garry
2021-03-26 10:57     ` John Garry
2021-03-26 13:13     ` Paul A. Clarke
2021-03-26 13:13       ` Paul A. Clarke
2021-03-29 21:07     ` Paul A. Clarke
2021-03-29 21:07       ` Paul A. Clarke
2021-03-30  6:41       ` kajoljain
2021-03-30  6:41         ` kajoljain
2021-04-06 11:02         ` John Garry
2021-04-06 11:02           ` John Garry
2021-04-06 12:18           ` Paul A. Clarke
2021-04-06 12:18             ` Paul A. Clarke
2021-04-07  6:03 ` kajoljain
2021-04-07  6:03   ` kajoljain

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.