[PATCH 0/6] perf test pmu-events case

[PATCH 0/6] perf test pmu-events case

[John Garry]

This series introduces a basic perf test pmu-events test case.

The test works by verifying correctly created aliases for the HW PMUs in
the host system.

The aliases come from a set of invented test events, which are (mostly)
arch agnostic (even though I copied these aliases from pre-existing x86
and arm64 JSONs).

In the test, core and uncore events are treated slightly differently. For
core test events, all events are matched. However, for uncore test events,
these have "Unit" property, so can only be matched when the corresponding
HW PMU exists in the host sytem.

A test run looks like this on my x86 dev box:

kernel-dev$ tools/perf/perf test -vv 10
Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
10: PMU event aliases                                     :
--- start ---
test child forked, pid 30869
Using CPUID GenuineIntel-6-9E-9
intel_pt default config: tsc,mtc,mtc_period=3,psb_period=3,pt,branch
skipping testing PMU software
testing PMU power: skip
testing PMU cpu: matched event segment_reg_loads.any
testing PMU cpu: matched event dispatch_blocked.any
testing PMU cpu: matched event eist_trans
testing PMU cpu: matched event bp_l1_btb_correct
testing PMU cpu: matched event bp_l2_btb_correct
testing PMU cpu: pass
testing PMU cstate_core: skip
testing PMU uncore_cbox_2: matched event unc_cbo_xsnp_response.miss_eviction
testing PMU uncore_cbox_2: pass
skipping testing PMU breakpoint
testing PMU uncore_cbox_0: matched event unc_cbo_xsnp_response.miss_eviction
testing PMU uncore_cbox_0: pass
skipping testing PMU tracepoint
testing PMU cstate_pkg: skip
testing PMU uncore_arb: skip
testing PMU msr: skip
testing PMU uncore_cbox_3: matched event unc_cbo_xsnp_response.miss_eviction
testing PMU uncore_cbox_3: pass
testing PMU intel_pt: skip
testing PMU uncore_cbox_1: matched event unc_cbo_xsnp_response.miss_eviction
testing PMU uncore_cbox_1: pass
test child finished with 0
---- end ----
PMU event aliases: Ok


Comments welcome.

John Garry (6):
  perf jevents: Fix leak of mapfile memory
  perf jevents: Support test events folder
  perf jevents: Add some test events
  perf pmu: Refactor pmu_add_cpu_aliases()
  perf pmu: Add is_pmu_core()
  perf test: Add pmu-events test

 .../pmu-events/arch/test/test_cpu/branch.json |  12 ++
 .../pmu-events/arch/test/test_cpu/other.json  |  26 +++
 .../pmu-events/arch/test/test_cpu/uncore.json |  21 ++
 tools/perf/pmu-events/jevents.c               |  59 +++++-
 tools/perf/pmu-events/pmu-events.h            |   4 +
 tools/perf/tests/Build                        |   1 +
 tools/perf/tests/builtin-test.c               |   4 +
 tools/perf/tests/pmu-events.c                 | 192 ++++++++++++++++++
 tools/perf/tests/tests.h                      |   1 +
 tools/perf/util/pmu.c                         |  26 ++-
 tools/perf/util/pmu.h                         |   4 +
 11 files changed, 336 insertions(+), 14 deletions(-)
 create mode 100644 tools/perf/pmu-events/arch/test/test_cpu/branch.json
 create mode 100644 tools/perf/pmu-events/arch/test/test_cpu/other.json
 create mode 100644 tools/perf/pmu-events/arch/test/test_cpu/uncore.json
 create mode 100644 tools/perf/tests/pmu-events.c

-- 
2.17.1

[PATCH 1/6] perf jevents: Fix leak of mapfile memory

[John Garry]

The memory for global pointer is never freed during normal program
execution, so let's do that in the main function exit as a good programming
practice.

A stray blank line is also removed.

Reported-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/pmu-events/jevents.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 079c77b6a2fd..27b4da80f751 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -1082,10 +1082,9 @@ static int process_one_file(const char *fpath, const struct stat *sb,
  */
 int main(int argc, char *argv[])
 {
-	int rc;
+	int rc, ret = 0;
 	int maxfds;
 	char ldirname[PATH_MAX];
-
 	const char *arch;
 	const char *output_file;
 	const char *start_dirname;
@@ -1156,7 +1155,8 @@ int main(int argc, char *argv[])
 		/* Make build fail */
 		fclose(eventsfp);
 		free_arch_std_events();
-		return 1;
+		ret = 1;
+		goto out_free_mapfile;
 	} else if (rc) {
 		goto empty_map;
 	}
@@ -1174,14 +1174,17 @@ int main(int argc, char *argv[])
 		/* Make build fail */
 		fclose(eventsfp);
 		free_arch_std_events();
-		return 1;
+		ret = 1;
 	}
 
-	return 0;
+
+	goto out_free_mapfile;
 
 empty_map:
 	fclose(eventsfp);
 	create_empty_mapping(output_file);
 	free_arch_std_events();
-	return 0;
+out_free_mapfile:
+	free(mapfile);
+	return ret;
 }
-- 
2.17.1

[PATCH 2/6] perf jevents: Support test events folder

[John Garry]

With the goal of supporting pmu-events test case, introduce support for a
test events folder.

These test events can be used for testing alias creation for any arch.

When running the pmu-events test case, these test events will be used
as the platform-agnostic events, so aliases can be created per-PMU and
validated against known expected values.

To support the test events, create a new table of events in generated
pmu-events.c, outside the existing pmu_events_map[] table. The table
of test events will not have any matching against a CPUID.

The resultant generated pmu-events.c will now look like the following:

struct pmu_event pme_ampere_emag[] = {
{
	.name = "ldrex_spec",
	.event = "event=0x6c",
	.desc = "Exclusive operation spe...",
	.topic = "intrinsic",
	.long_desc = "Exclusive operation ...",
},
...
};
struct pmu_events_map pmu_events_map[] = {
...
{
	.cpuid = "0x00000000500f0000",
	.version = "v1",
	.type = "core",
	.table = pme_ampere_emag
},
...
{
	.cpuid = 0,
	.version = 0,
	.type = 0,
	.table = 0,
},
};

struct pmu_event pme_test_cpu[] = {
{
	.name = "uncore_hisi_ddrc.flux_wcmd",
	.event = "event=0x2",
	.desc = "DDRC write commands. Unit: hisi_sccl,ddrc ",
	.topic = "uncore",
	.long_desc = "DDRC write commands",
	.pmu = "hisi_sccl,ddrc",
},
{
	.name = "unc_cbo_xsnp_response.miss_eviction",
	.event = "umask=0x81,event=0x22",
	.desc = "Unit: uncore_cbox A cross-core snoop resulted ...",
	.topic = "uncore",
	.long_desc = "A cross-core snoop resulted from L3 ...",
	.pmu = "uncore_cbox",
},
{
	.name = "eist_trans",
	.event = "umask=0x0,period=200000,event=0x3a",
	.desc = "Number of Enhanced Intel SpeedStep(R) ...",
	.topic = "other",
},
{
	.name = 0,
},
};

struct pmu_events_map pmu_events_map_test = {
	.table = pme_test_cpu,
};

Structure pmu_events_map_test will be used in pmu-events test as the events
to create and validate aliases for.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/pmu-events/jevents.c    | 46 +++++++++++++++++++++++++++++-
 tools/perf/pmu-events/pmu-events.h |  4 +++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 27b4da80f751..21b7fd83c8b8 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -53,6 +53,8 @@
 int verbose;
 char *prog;
 
+const char *start_dirname;
+
 int eprintf(int level, int var, const char *fmt, ...)
 {
 
@@ -764,6 +766,25 @@ static void print_mapping_table_suffix(FILE *outfp)
 	fprintf(outfp, "};\n");
 }
 
+static void print_test_event_tables_prefix(FILE *outfp)
+{
+	fprintf(outfp, "\nstruct pmu_events_map pmu_events_map_test = {\n");
+}
+
+static void print_test_event_tables_suffix(FILE *outfp)
+{
+	fprintf(outfp, "};\n");
+}
+
+static void process_test_event_tables(FILE *outfp)
+{
+	struct test_event_table *test_event_table;
+
+	print_test_event_tables_prefix(outfp);
+	fprintf(outfp, "\t.table = pme_test_cpu,\n");
+	print_test_event_tables_suffix(outfp);
+}
+
 static int process_mapfile(FILE *outfp, char *fpath)
 {
 	int n = 16384;
@@ -868,6 +889,8 @@ static void create_empty_mapping(const char *output_file)
 	fprintf(outfp, "#include \"pmu-events/pmu-events.h\"\n");
 	print_mapping_table_prefix(outfp);
 	print_mapping_table_suffix(outfp);
+	print_test_event_tables_prefix(outfp);
+	print_test_event_tables_suffix(outfp);
 	fclose(outfp);
 }
 
@@ -1085,9 +1108,9 @@ int main(int argc, char *argv[])
 	int rc, ret = 0;
 	int maxfds;
 	char ldirname[PATH_MAX];
+
 	const char *arch;
 	const char *output_file;
-	const char *start_dirname;
 	struct stat stbuf;
 
 	prog = basename(argv[0]);
@@ -1177,6 +1200,27 @@ int main(int argc, char *argv[])
 		ret = 1;
 	}
 
+	/* walk "test" folder, regardless of the arch */
+	sprintf(ldirname, "%s/test", start_dirname);
+
+	rc = nftw(ldirname, process_one_file, maxfds, 0);
+	if (rc && verbose) {
+		pr_info("%s: Error walking file tree %s rc=%d for test\n",
+			prog, ldirname, rc);
+		goto empty_map;
+	} else if (rc < 0) {
+		/* Make build fail */
+		free_arch_std_events();
+		ret = 1;
+		goto out_free_mapfile;
+	} else if (rc) {
+		goto empty_map;
+	}
+
+	if (close_table)
+		print_events_table_suffix(eventsfp);
+
+	process_test_event_tables(eventsfp);
 
 	goto out_free_mapfile;
 
diff --git a/tools/perf/pmu-events/pmu-events.h b/tools/perf/pmu-events/pmu-events.h
index caeb577d36c9..96fc60a481b1 100644
--- a/tools/perf/pmu-events/pmu-events.h
+++ b/tools/perf/pmu-events/pmu-events.h
@@ -36,10 +36,14 @@ struct pmu_events_map {
 	struct pmu_event *table;
 };
 
+struct pmu_test_events {
+	struct pmu_event *table;
+};
 /*
  * Global table mapping each known CPU for the architecture to its
  * table of PMU events.
  */
 extern struct pmu_events_map pmu_events_map[];
+extern struct pmu_events_map pmu_events_map_test;
 
 #endif
-- 
2.17.1

[PATCH 3/6] perf jevents: Add some test events

[John Garry]

Add some test PMU events. The events are randomly chosen from x86 and
arm64 JSONs. The events include CPU and uncore events.

Signed-off-by: John Garry <john.garry@huawei.com>
---
 .../pmu-events/arch/test/test_cpu/branch.json | 12 +++++++++
 .../pmu-events/arch/test/test_cpu/other.json  | 26 +++++++++++++++++++
 .../pmu-events/arch/test/test_cpu/uncore.json | 21 +++++++++++++++
 3 files changed, 59 insertions(+)
 create mode 100644 tools/perf/pmu-events/arch/test/test_cpu/branch.json
 create mode 100644 tools/perf/pmu-events/arch/test/test_cpu/other.json
 create mode 100644 tools/perf/pmu-events/arch/test/test_cpu/uncore.json

diff --git a/tools/perf/pmu-events/arch/test/test_cpu/branch.json b/tools/perf/pmu-events/arch/test/test_cpu/branch.json
new file mode 100644
index 000000000000..93ddfd8053ca
--- /dev/null
+++ b/tools/perf/pmu-events/arch/test/test_cpu/branch.json
@@ -0,0 +1,12 @@
+[
+  {
+    "EventName": "bp_l1_btb_correct",
+    "EventCode": "0x8a",
+    "BriefDescription": "L1 BTB Correction."
+  },
+  {
+    "EventName": "bp_l2_btb_correct",
+    "EventCode": "0x8b",
+    "BriefDescription": "L2 BTB Correction."
+  }
+]
diff --git a/tools/perf/pmu-events/arch/test/test_cpu/other.json b/tools/perf/pmu-events/arch/test/test_cpu/other.json
new file mode 100644
index 000000000000..7d53d7ecd723
--- /dev/null
+++ b/tools/perf/pmu-events/arch/test/test_cpu/other.json
@@ -0,0 +1,26 @@
+[
+    {
+        "EventCode": "0x6",
+        "Counter": "0,1",
+        "UMask": "0x80",
+        "EventName": "SEGMENT_REG_LOADS.ANY",
+        "SampleAfterValue": "200000",
+        "BriefDescription": "Number of segment register loads."
+    },
+    {
+        "EventCode": "0x9",
+        "Counter": "0,1",
+        "UMask": "0x20",
+        "EventName": "DISPATCH_BLOCKED.ANY",
+        "SampleAfterValue": "200000",
+        "BriefDescription": "Memory cluster signals to block micro-op dispatch for any reason"
+    },
+    {
+        "EventCode": "0x3A",
+        "Counter": "0,1",
+        "UMask": "0x0",
+        "EventName": "EIST_TRANS",
+        "SampleAfterValue": "200000",
+        "BriefDescription": "Number of Enhanced Intel SpeedStep(R) Technology (EIST) transitions"
+    }
+]
\ No newline at end of file
diff --git a/tools/perf/pmu-events/arch/test/test_cpu/uncore.json b/tools/perf/pmu-events/arch/test/test_cpu/uncore.json
new file mode 100644
index 000000000000..d0a890cc814d
--- /dev/null
+++ b/tools/perf/pmu-events/arch/test/test_cpu/uncore.json
@@ -0,0 +1,21 @@
+[
+ {
+	    "EventCode": "0x02",
+	    "EventName": "uncore_hisi_ddrc.flux_wcmd",
+	    "BriefDescription": "DDRC write commands",
+	    "PublicDescription": "DDRC write commands",
+	    "Unit": "hisi_sccl,ddrc"
+   },
+   {
+	    "Unit": "CBO",
+	    "EventCode": "0x22",
+	    "UMask": "0x81",
+	    "EventName": "UNC_CBO_XSNP_RESPONSE.MISS_EVICTION",
+	    "BriefDescription": "A cross-core snoop resulted from L3 Eviction which misses in some processor core.",
+	    "PublicDescription": "A cross-core snoop resulted from L3 Eviction which misses in some processor core.",
+	    "Counter": "0,1",
+	    "CounterMask": "0",
+	    "Invert": "0",
+	    "EdgeDetect": "0"
+  }
+]
-- 
2.17.1

[PATCH 4/6] perf pmu: Refactor pmu_add_cpu_aliases()

[John Garry]

Create pmu_add_cpu_aliases_map() from pmu_add_cpu_aliases(), so the caller
can pass the map; the pmu-events test would use this since there would
be no CPUID matching to a mapfile there.

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

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index 8b99fd312aae..c616a06a34a8 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -21,7 +21,6 @@
 #include "pmu.h"
 #include "parse-events.h"
 #include "header.h"
-#include "pmu-events/pmu-events.h"
 #include "string2.h"
 #include "strbuf.h"
 #include "fncache.h"
@@ -744,16 +743,11 @@ static bool pmu_uncore_alias_match(const char *pmu_name, const char *name)
  * to the current running CPU. Then, add all PMU events from that table
  * as aliases.
  */
-static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
+void pmu_add_cpu_aliases_map(struct list_head *head, struct perf_pmu *pmu,
+			     struct pmu_events_map *map)
 {
 	int i;
-	struct pmu_events_map *map;
 	const char *name = pmu->name;
-
-	map = perf_pmu__find_map(pmu);
-	if (!map)
-		return;
-
 	/*
 	 * Found a matching PMU events table. Create aliases
 	 */
@@ -788,6 +782,17 @@ static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
 	}
 }
 
+static void pmu_add_cpu_aliases(struct list_head *head, struct perf_pmu *pmu)
+{
+	struct pmu_events_map *map;
+
+	map = perf_pmu__find_map(pmu);
+	if (!map)
+		return;
+
+	pmu_add_cpu_aliases_map(head, pmu, map);
+}
+
 struct perf_event_attr * __weak
 perf_pmu__get_default_config(struct perf_pmu *pmu __maybe_unused)
 {
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 6737e3d5d568..0b4a0efae38e 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -7,6 +7,7 @@
 #include <linux/perf_event.h>
 #include <stdbool.h>
 #include "parse-events.h"
+#include "pmu-events/pmu-events.h"
 
 struct perf_evsel_config_term;
 
@@ -97,6 +98,8 @@ int perf_pmu__scan_file(struct perf_pmu *pmu, const char *name, const char *fmt,
 int perf_pmu__test(void);
 
 struct perf_event_attr *perf_pmu__get_default_config(struct perf_pmu *pmu);
+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);
 
-- 
2.17.1

[PATCH 5/6] perf pmu: Add is_pmu_core()

[John Garry]

Add a function to decide whether a PMU is a core PMU.

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

diff --git a/tools/perf/util/pmu.c b/tools/perf/util/pmu.c
index c616a06a34a8..55129d09f19d 100644
--- a/tools/perf/util/pmu.c
+++ b/tools/perf/util/pmu.c
@@ -1400,6 +1400,11 @@ static void wordwrap(char *s, int start, int max, int corr)
 	}
 }
 
+bool is_pmu_core(const char *name)
+{
+	return !strcmp(name, "cpu") || is_arm_pmu_core(name);
+}
+
 void print_pmu_events(const char *event_glob, bool name_only, bool quiet_flag,
 			bool long_desc, bool details_flag, bool deprecated)
 {
diff --git a/tools/perf/util/pmu.h b/tools/perf/util/pmu.h
index 0b4a0efae38e..b756946ae78d 100644
--- a/tools/perf/util/pmu.h
+++ b/tools/perf/util/pmu.h
@@ -88,6 +88,7 @@ int perf_pmu__format_parse(char *dir, struct list_head *head);
 
 struct perf_pmu *perf_pmu__scan(struct perf_pmu *pmu);
 
+bool is_pmu_core(const char *name);
 void print_pmu_events(const char *event_glob, bool name_only, bool quiet,
 		      bool long_desc, bool details_flag,
 		      bool deprecated);
-- 
2.17.1

[PATCH 6/6] perf test: Add pmu-events test

[John Garry]

Add a pmu-events test.

This test will scan all PMUs in the system, and run a PMU event aliasing
test for each CPU or uncore PMU.

For known aliases added in pmu-events/arch/test, we need to add an entry
in test_cpu_aliases[] or test_uncore_aliases[].

A sample run is as follows for x86:

Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
10: PMU event aliases                                     :
--- start ---
test child forked, pid 30869
Using CPUID GenuineIntel-6-9E-9
intel_pt default config: tsc,mtc,mtc_period=3,psb_period=3,pt,branch
skipping testing PMU software
testing PMU power: skip
testing PMU cpu: matched event segment_reg_loads.any
testing PMU cpu: matched event dispatch_blocked.any
testing PMU cpu: matched event eist_trans
testing PMU cpu: matched event bp_l1_btb_correct
testing PMU cpu: matched event bp_l2_btb_correct
testing PMU cpu: pass
testing PMU cstate_core: skip
testing PMU uncore_cbox_2: matched event unc_cbo_xsnp_response.miss_eviction
testing PMU uncore_cbox_2: pass
skipping testing PMU breakpoint
testing PMU uncore_cbox_0: matched event unc_cbo_xsnp_response.miss_eviction
testing PMU uncore_cbox_0: pass
skipping testing PMU tracepoint
testing PMU cstate_pkg: skip
testing PMU uncore_arb: skip
testing PMU msr: skip
testing PMU uncore_cbox_3: matched event unc_cbo_xsnp_response.miss_eviction
testing PMU uncore_cbox_3: pass
testing PMU intel_pt: skip
testing PMU uncore_cbox_1: matched event unc_cbo_xsnp_response.miss_eviction
testing PMU uncore_cbox_1: pass
test child finished with 0
---- end ----
PMU event aliases: Ok


Signed-off-by: John Garry <john.garry@huawei.com>
---
 tools/perf/tests/Build          |   1 +
 tools/perf/tests/builtin-test.c |   4 +
 tools/perf/tests/pmu-events.c   | 192 ++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |   1 +
 4 files changed, 198 insertions(+)
 create mode 100644 tools/perf/tests/pmu-events.c

diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index 1692529639b0..b3d1bf13ca07 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -14,6 +14,7 @@ perf-y += evsel-roundtrip-name.o
 perf-y += evsel-tp-sched.o
 perf-y += fdarray.o
 perf-y += pmu.o
+perf-y += pmu-events.o
 perf-y += hists_common.o
 perf-y += hists_link.o
 perf-y += hists_filter.o
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 5f05db75cdd8..e8f56740d14a 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -72,6 +72,10 @@ static struct test generic_tests[] = {
 		.desc = "Parse perf pmu format",
 		.func = test__pmu,
 	},
+	{
+		.desc = "PMU event aliases",
+		.func = test__pmu_event_aliases,
+	},
 	{
 		.desc = "DSO data read",
 		.func = test__dso_data,
diff --git a/tools/perf/tests/pmu-events.c b/tools/perf/tests/pmu-events.c
new file mode 100644
index 000000000000..176b80666b5e
--- /dev/null
+++ b/tools/perf/tests/pmu-events.c
@@ -0,0 +1,192 @@
+// SPDX-License-Identifier: GPL-2.0
+#include "parse-events.h"
+#include "pmu.h"
+#include "tests.h"
+#include <errno.h>
+#include <stdio.h>
+#include <linux/kernel.h>
+
+#include <linux/zalloc.h>
+
+#include "debug.h"
+#include "../pmu-events/pmu-events.h"
+
+static struct perf_pmu_alias test_cpu_aliases[] = {
+	{
+		.name = (char *)"segment_reg_loads.any",
+		.str = (char *)"umask=0x80,(null)=0x30d40,event=0x6",
+		.desc = (char *)"Number of segment register loads",
+		.topic = (char *)"other",
+
+	},
+	{
+		.name = (char *)"dispatch_blocked.any",
+		.str = (char *)"umask=0x20,(null)=0x30d40,event=0x9",
+		.desc = (char *)"Memory cluster signals to block micro-op dispatch for any reason",
+		.topic = (char *)"other",
+
+	},
+	{
+		.name = (char *)"eist_trans",
+		.str = (char *)"umask=0,(null)=0x30d40,event=0x3a",
+		.desc = (char *)"Number of Enhanced Intel SpeedStep(R) Technology (EIST) transitions",
+		.topic = (char *)"other",
+
+	},
+	{
+		.name = (char *)"bp_l1_btb_correct",
+		.str = (char *)"event=0x8a",
+		.desc = (char *)"L1 BTB Correction",
+		.topic = (char *)"branch",
+
+	},
+	{
+		.name = (char *)"bp_l2_btb_correct",
+		.str = (char *)"event=0x8b",
+		.desc = (char *)"L2 BTB Correction",
+		.topic = (char *)"branch",
+
+	},
+	{ /* sentinel */
+	}
+};
+
+static struct perf_pmu_alias test_uncore_aliases[] = {
+	{
+		.name = (char *)"uncore_hisi_ddrc.flux_wcmd",
+		.str = (char *)"event=0x2",
+		.desc = (char *)"DDRC write commands. Unit: hisi_sccl,ddrc ",
+		.topic = (char *)"uncore",
+
+	},
+	{
+		.name = (char *)"unc_cbo_xsnp_response.miss_eviction",
+		.str = (char *)"umask=0x81,event=0x22",
+		.desc = (char *)"Unit: uncore_cbox A cross-core snoop resulted from L3 Eviction which misses in some processor core",
+		.long_desc = (char *)"A cross-core snoop resulted from L3 Eviction which misses in some processor core",
+		.topic = (char *)"uncore",
+
+	},
+	{ /* sentinel */
+	}
+
+};
+
+static bool is_same(char *reference, char *test)
+{
+	if (!test)
+		return false;
+
+	return !strcmp(reference, test);
+}
+
+static struct perf_pmu_alias *find_alias(char *pmu_name, char *alias_name)
+{
+	struct perf_pmu_alias *alias;
+
+	if (is_pmu_core(pmu_name))
+		alias = &test_cpu_aliases[0];
+	else
+		alias = &test_uncore_aliases[0];
+
+	for (; alias->name; alias++)
+		if (!strcmp(alias_name, alias->name))
+			return alias;
+
+	return NULL;
+}
+
+static int __test__pmu_event_aliases(char *pmu_name, int *count)
+{
+	struct perf_pmu_alias *alias;
+	struct perf_pmu *pmu;
+	LIST_HEAD(aliases);
+	int res = 0;
+
+	pmu = zalloc(sizeof(*pmu));
+	if (!pmu)
+		return -1;
+
+	pmu->name = pmu_name;
+
+	pmu_add_cpu_aliases_map(&aliases, pmu, &pmu_events_map_test);
+
+	list_for_each_entry(alias, &aliases, list) {
+		struct perf_pmu_alias *test_alias;
+
+		test_alias = find_alias(pmu_name, alias->name);
+
+		if (!test_alias) {
+			pr_debug2("no alias for PMU %s\n", pmu_name);
+			res = -1;
+			break;
+		}
+
+		if (test_alias->desc &&
+		    !is_same(test_alias->desc, alias->desc)) {
+			pr_debug2("mismatched desc for PMU %s, %s vs %s\n",
+				  pmu_name, test_alias->desc, alias->desc);
+			res = -1;
+			break;
+		}
+
+		if (test_alias->long_desc &&
+		    !is_same(test_alias->long_desc, alias->long_desc)) {
+			pr_debug2("mismatched long_desc for PMU %s, %s vs %s\n",
+				  pmu_name, test_alias->long_desc,
+				  alias->long_desc);
+			res = -1;
+			break;
+		}
+
+		if (test_alias->str && !is_same(test_alias->str, alias->str)) {
+			pr_debug2("mismatched str for PMU %s, %s vs %s\n",
+				  pmu_name, test_alias->str, alias->str);
+			res = -1;
+			break;
+		}
+
+		if (test_alias->topic &&
+		    !is_same(test_alias->topic, alias->topic)) {
+			pr_debug2("mismatched topic for PMU %s, %s vs %s\n",
+				  pmu_name, test_alias->topic, alias->topic);
+			res = -1;
+			break;
+		}
+
+		(*count)++;
+
+		pr_debug2("testing PMU %s: matched event %s\n",
+			  pmu_name, test_alias->name);
+	}
+
+	free(pmu);
+	return res;
+}
+
+int test__pmu_event_aliases(struct test *test __maybe_unused,
+			    int subtest __maybe_unused)
+{
+	struct perf_pmu *pmu = NULL;
+
+	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
+		int count = 0;
+
+		if (list_empty(&pmu->format)) {
+			pr_debug2("skipping testing PMU %s\n", pmu->name);
+			continue;
+		}
+
+		if (__test__pmu_event_aliases(pmu->name, &count)) {
+			pr_debug("testing PMU %s: failed\n", pmu->name);
+			return -1;
+		}
+
+		if (count == 0)
+			pr_debug("testing PMU %s: skip\n", pmu->name);
+		else
+			pr_debug("testing PMU %s: pass\n", pmu->name);
+	}
+
+	return 0;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 9a160fef47c9..a7ca3bb1c9cd 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -49,6 +49,7 @@ int test__perf_evsel__roundtrip_name_test(struct test *test, int subtest);
 int test__perf_evsel__tp_sched_test(struct test *test, int subtest);
 int test__syscall_openat_tp_fields(struct test *test, int subtest);
 int test__pmu(struct test *test, int subtest);
+int test__pmu_event_aliases(struct test *test, int subtest);
 int test__attr(struct test *test, int subtest);
 int test__dso_data(struct test *test, int subtest);
 int test__dso_data_cache(struct test *test, int subtest);
-- 
2.17.1

Re: [PATCH 1/6] perf jevents: Fix leak of mapfile memory

[Arnaldo Carvalho de Melo]

Em Thu, Mar 05, 2020 at 07:08:01PM +0800, John Garry escreveu:
> The memory for global pointer is never freed during normal program
> execution, so let's do that in the main function exit as a good programming
> practice.
> 
> A stray blank line is also removed.

Thanks, applied to perf/urgent.

- Arnaldo
 
> Reported-by: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: John Garry <john.garry@huawei.com>
> ---
>  tools/perf/pmu-events/jevents.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
> index 079c77b6a2fd..27b4da80f751 100644
> --- a/tools/perf/pmu-events/jevents.c
> +++ b/tools/perf/pmu-events/jevents.c
> @@ -1082,10 +1082,9 @@ static int process_one_file(const char *fpath, const struct stat *sb,
>   */
>  int main(int argc, char *argv[])
>  {
> -	int rc;
> +	int rc, ret = 0;
>  	int maxfds;
>  	char ldirname[PATH_MAX];
> -
>  	const char *arch;
>  	const char *output_file;
>  	const char *start_dirname;
> @@ -1156,7 +1155,8 @@ int main(int argc, char *argv[])
>  		/* Make build fail */
>  		fclose(eventsfp);
>  		free_arch_std_events();
> -		return 1;
> +		ret = 1;
> +		goto out_free_mapfile;
>  	} else if (rc) {
>  		goto empty_map;
>  	}
> @@ -1174,14 +1174,17 @@ int main(int argc, char *argv[])
>  		/* Make build fail */
>  		fclose(eventsfp);
>  		free_arch_std_events();
> -		return 1;
> +		ret = 1;
>  	}
>  
> -	return 0;
> +
> +	goto out_free_mapfile;
>  
>  empty_map:
>  	fclose(eventsfp);
>  	create_empty_mapping(output_file);
>  	free_arch_std_events();
> -	return 0;
> +out_free_mapfile:
> +	free(mapfile);
> +	return ret;
>  }
> -- 
> 2.17.1
> 

-- 

- Arnaldo

[tip: perf/urgent] perf jevents: Fix leak of mapfile memory

[tip-bot2 for John Garry]

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

Commit-ID:     3f5777fbaf04c58d940526a22a2e0c813c837936
Gitweb:        https://git.kernel.org/tip/3f5777fbaf04c58d940526a22a2e0c813c837936
Author:        John Garry <john.garry@huawei.com>
AuthorDate:    Thu, 05 Mar 2020 19:08:01 +08:00
Committer:     Arnaldo Carvalho de Melo <acme@redhat.com>
CommitterDate: Fri, 06 Mar 2020 08:30:47 -03:00

perf jevents: Fix leak of mapfile memory

The memory for global pointer is never freed during normal program
execution, so let's do that in the main function exit as a good
programming practice.

A stray blank line is also removed.

Reported-by: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: John Garry <john.garry@huawei.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
Cc: James Clark <james.clark@arm.com>
Cc: Joakim Zhang <qiangqing.zhang@nxp.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Will Deacon <will@kernel.org>
Cc: linuxarm@huawei.com
Link: http://lore.kernel.org/lkml/1583406486-154841-2-git-send-email-john.garry@huawei.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/pmu-events/jevents.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/tools/perf/pmu-events/jevents.c b/tools/perf/pmu-events/jevents.c
index 079c77b..27b4da8 100644
--- a/tools/perf/pmu-events/jevents.c
+++ b/tools/perf/pmu-events/jevents.c
@@ -1082,10 +1082,9 @@ static int process_one_file(const char *fpath, const struct stat *sb,
  */
 int main(int argc, char *argv[])
 {
-	int rc;
+	int rc, ret = 0;
 	int maxfds;
 	char ldirname[PATH_MAX];
-
 	const char *arch;
 	const char *output_file;
 	const char *start_dirname;
@@ -1156,7 +1155,8 @@ int main(int argc, char *argv[])
 		/* Make build fail */
 		fclose(eventsfp);
 		free_arch_std_events();
-		return 1;
+		ret = 1;
+		goto out_free_mapfile;
 	} else if (rc) {
 		goto empty_map;
 	}
@@ -1174,14 +1174,17 @@ int main(int argc, char *argv[])
 		/* Make build fail */
 		fclose(eventsfp);
 		free_arch_std_events();
-		return 1;
+		ret = 1;
 	}
 
-	return 0;
+
+	goto out_free_mapfile;
 
 empty_map:
 	fclose(eventsfp);
 	create_empty_mapping(output_file);
 	free_arch_std_events();
-	return 0;
+out_free_mapfile:
+	free(mapfile);
+	return ret;
 }

Re: [PATCH 6/6] perf test: Add pmu-events test

[Jiri Olsa]

On Thu, Mar 05, 2020 at 07:08:06PM +0800, John Garry wrote:
> Add a pmu-events test.
> 
> This test will scan all PMUs in the system, and run a PMU event aliasing
> test for each CPU or uncore PMU.
> 
> For known aliases added in pmu-events/arch/test, we need to add an entry
> in test_cpu_aliases[] or test_uncore_aliases[].

heya, awesome! ;-)

> 
> A sample run is as follows for x86:
> 
> Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
> 10: PMU event aliases                                     :
> --- start ---
> test child forked, pid 30869
> Using CPUID GenuineIntel-6-9E-9
> intel_pt default config: tsc,mtc,mtc_period=3,psb_period=3,pt,branch
> skipping testing PMU software
> testing PMU power: skip
> testing PMU cpu: matched event segment_reg_loads.any
> testing PMU cpu: matched event dispatch_blocked.any
> testing PMU cpu: matched event eist_trans
> testing PMU cpu: matched event bp_l1_btb_correct
> testing PMU cpu: matched event bp_l2_btb_correct
> testing PMU cpu: pass
> testing PMU cstate_core: skip
> testing PMU uncore_cbox_2: matched event unc_cbo_xsnp_response.miss_eviction
> testing PMU uncore_cbox_2: pass
> skipping testing PMU breakpoint
> testing PMU uncore_cbox_0: matched event unc_cbo_xsnp_response.miss_eviction
> testing PMU uncore_cbox_0: pass
> skipping testing PMU tracepoint
> testing PMU cstate_pkg: skip
> testing PMU uncore_arb: skip
> testing PMU msr: skip
> testing PMU uncore_cbox_3: matched event unc_cbo_xsnp_response.miss_eviction
> testing PMU uncore_cbox_3: pass
> testing PMU intel_pt: skip
> testing PMU uncore_cbox_1: matched event unc_cbo_xsnp_response.miss_eviction
> testing PMU uncore_cbox_1: pass
> test child finished with 0
> ---- end ----
> PMU event aliases: Ok

SNIP

> +int test__pmu_event_aliases(struct test *test __maybe_unused,
> +			    int subtest __maybe_unused)
> +{
> +	struct perf_pmu *pmu = NULL;
> +
> +	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> +		int count = 0;

I don't follow the pmu iteration in here.. I'd expect
we create 'test' pmu and check that all the aliasses
are in place as we expect them.. why do we match them
to existing events?

or as I'm thinking about that now, would it be enough
to check pme_test_cpu array to have string that we
expect?

thanks for doing this,
jirka

Re: [PATCH 6/6] perf test: Add pmu-events test

[John Garry]

>>
>> A sample run is as follows for x86:
>>
>> Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
>> 10: PMU event aliases                                     :
>> --- start ---
>> test child forked, pid 30869
>> Using CPUID GenuineIntel-6-9E-9
>> intel_pt default config: tsc,mtc,mtc_period=3,psb_period=3,pt,branch
>> skipping testing PMU software
>> testing PMU power: skip
>> testing PMU cpu: matched event segment_reg_loads.any
>> testing PMU cpu: matched event dispatch_blocked.any
>> testing PMU cpu: matched event eist_trans
>> testing PMU cpu: matched event bp_l1_btb_correct
>> testing PMU cpu: matched event bp_l2_btb_correct
>> testing PMU cpu: pass
>> testing PMU cstate_core: skip
>> testing PMU uncore_cbox_2: matched event unc_cbo_xsnp_response.miss_eviction
>> testing PMU uncore_cbox_2: pass
>> skipping testing PMU breakpoint
>> testing PMU uncore_cbox_0: matched event unc_cbo_xsnp_response.miss_eviction
>> testing PMU uncore_cbox_0: pass
>> skipping testing PMU tracepoint
>> testing PMU cstate_pkg: skip
>> testing PMU uncore_arb: skip
>> testing PMU msr: skip
>> testing PMU uncore_cbox_3: matched event unc_cbo_xsnp_response.miss_eviction
>> testing PMU uncore_cbox_3: pass
>> testing PMU intel_pt: skip
>> testing PMU uncore_cbox_1: matched event unc_cbo_xsnp_response.miss_eviction
>> testing PMU uncore_cbox_1: pass
>> test child finished with 0
>> ---- end ----
>> PMU event aliases: Ok
> 
> SNIP
> 
>> +int test__pmu_event_aliases(struct test *test __maybe_unused,
>> +			    int subtest __maybe_unused)
>> +{
>> +	struct perf_pmu *pmu = NULL;
>> +
>> +	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
>> +		int count = 0;
> 

Hi jirka,

> I don't follow the pmu iteration in here.. I'd expect
> we create 'test' pmu 

well, that's what we do next in the call to __test__pmu_event_aliases():

static int __test__pmu_event_aliases(char *pmu_name, int *count)
{
	struct perf_pmu_alias *alias;
	struct perf_pmu *pmu;
	LIST_HEAD(aliases);
	int res = 0;

	pmu = zalloc(sizeof(*pmu));
	if (!pmu)
		return -1;

	pmu->name = pmu_name;

	pmu_add_cpu_aliases_map(&aliases, pmu, &pmu_events_map_test);


Here we clone the HW PMU, create the aliases, and verify them against a 
known, expected list.

and check that all the aliasses
> are in place as we expect them.. why do we match them
> to existing events?

The events in test_cpu_aliases[] or test_uncore_aliases[] are checked 
against the events from pmu-events/arch/test/test_cpu/*.json

> 
> or as I'm thinking about that now, would it be enough
> to check pme_test_cpu array to have string that we
> expect?

Right, I might change this.

So currently we iterate the PMU aliases to ensure that we have a 
matching event in pme_test_cpu[]. It may be better to iterate the events 
in pme_test_cpu[] to ensure that we have an alias.

The problem here is uncore PMUs. They have the "Unit" field, which is 
used for matching the PMU. So we cannot ensure test events from 
uncore.json will always have an event alias created per PMU. But maybe I 
could use pmu_uncore_alias_match() to check if the test event matches in 
this case.

Thanks,
John

Re: [PATCH 6/6] perf test: Add pmu-events test

[Jiri Olsa]

On Mon, Mar 09, 2020 at 10:12:15AM +0000, John Garry wrote:
> > > 
> > > A sample run is as follows for x86:
> > > 
> > > Couldn't bump rlimit(MEMLOCK), failures may take place when creating BPF maps, etc
> > > 10: PMU event aliases                                     :
> > > --- start ---
> > > test child forked, pid 30869
> > > Using CPUID GenuineIntel-6-9E-9
> > > intel_pt default config: tsc,mtc,mtc_period=3,psb_period=3,pt,branch
> > > skipping testing PMU software
> > > testing PMU power: skip
> > > testing PMU cpu: matched event segment_reg_loads.any
> > > testing PMU cpu: matched event dispatch_blocked.any
> > > testing PMU cpu: matched event eist_trans
> > > testing PMU cpu: matched event bp_l1_btb_correct
> > > testing PMU cpu: matched event bp_l2_btb_correct
> > > testing PMU cpu: pass
> > > testing PMU cstate_core: skip
> > > testing PMU uncore_cbox_2: matched event unc_cbo_xsnp_response.miss_eviction
> > > testing PMU uncore_cbox_2: pass
> > > skipping testing PMU breakpoint
> > > testing PMU uncore_cbox_0: matched event unc_cbo_xsnp_response.miss_eviction
> > > testing PMU uncore_cbox_0: pass
> > > skipping testing PMU tracepoint
> > > testing PMU cstate_pkg: skip
> > > testing PMU uncore_arb: skip
> > > testing PMU msr: skip
> > > testing PMU uncore_cbox_3: matched event unc_cbo_xsnp_response.miss_eviction
> > > testing PMU uncore_cbox_3: pass
> > > testing PMU intel_pt: skip
> > > testing PMU uncore_cbox_1: matched event unc_cbo_xsnp_response.miss_eviction
> > > testing PMU uncore_cbox_1: pass
> > > test child finished with 0
> > > ---- end ----
> > > PMU event aliases: Ok
> > 
> > SNIP
> > 
> > > +int test__pmu_event_aliases(struct test *test __maybe_unused,
> > > +			    int subtest __maybe_unused)
> > > +{
> > > +	struct perf_pmu *pmu = NULL;
> > > +
> > > +	while ((pmu = perf_pmu__scan(pmu)) != NULL) {
> > > +		int count = 0;
> > 
> 
> Hi jirka,
> 
> > I don't follow the pmu iteration in here.. I'd expect
> > we create 'test' pmu
> 
> well, that's what we do next in the call to __test__pmu_event_aliases():
> 
> static int __test__pmu_event_aliases(char *pmu_name, int *count)
> {
> 	struct perf_pmu_alias *alias;
> 	struct perf_pmu *pmu;
> 	LIST_HEAD(aliases);
> 	int res = 0;
> 
> 	pmu = zalloc(sizeof(*pmu));
> 	if (!pmu)
> 		return -1;
> 
> 	pmu->name = pmu_name;
> 
> 	pmu_add_cpu_aliases_map(&aliases, pmu, &pmu_events_map_test);
> 
> 
> Here we clone the HW PMU, create the aliases, and verify them against a
> known, expected list.
> 
> and check that all the aliasses
> > are in place as we expect them.. why do we match them
> > to existing events?
> 
> The events in test_cpu_aliases[] or test_uncore_aliases[] are checked
> against the events from pmu-events/arch/test/test_cpu/*.json

I don't understand the benefit of this.. so IIUC:

  - jevents will go through arch/test and populate pmu-events/pmu-events.c
    with:
       struct pmu_event pme_test_cpu[] ...
       struct pmu_events_map pmu_events_map_test ...

  - so we actualy have the parsed json events in C structs and we can go
    through them and check it contains fields with strings that we expect

  - you go through all detected pmus and check if the tests events we
    generated are matching some of the events from these pmus,
    and that's where I'm lost ;-) why?

> 
> > 
> > or as I'm thinking about that now, would it be enough
> > to check pme_test_cpu array to have string that we
> > expect?
> 
> Right, I might change this.
> 
> So currently we iterate the PMU aliases to ensure that we have a matching
> event in pme_test_cpu[]. It may be better to iterate the events in
> pme_test_cpu[] to ensure that we have an alias.

that's what I described above.. I dont understand the connection/value
of this tests

> 
> The problem here is uncore PMUs. They have the "Unit" field, which is used
> for matching the PMU. So we cannot ensure test events from uncore.json will
> always have an event alias created per PMU. But maybe I could use
> pmu_uncore_alias_match() to check if the test event matches in this case.

hum I guess I don't follow all the details.. but some more explanation
of the test would be great

jirka

Re: [PATCH 6/6] perf test: Add pmu-events test

[John Garry]

>>
>> The events in test_cpu_aliases[] or test_uncore_aliases[] are checked
>> against the events from pmu-events/arch/test/test_cpu/*.json
> 

Hi Jirka,

> I don't understand the benefit of this.. so IIUC:
> 
>    - jevents will go through arch/test and populate pmu-events/pmu-events.c
>      with:
>         struct pmu_event pme_test_cpu[] ...
>         struct pmu_events_map pmu_events_map_test ...

Right. And the idea is that pme_test_cpu[] can be used as generic set of 
events for testing on any arch/cpuid. (note: I'll just ignore uncore 
events for the moment)

> 
>    - so we actualy have the parsed json events in C structs and we can go
>      through them and check it contains fields with strings that we expect

No, we use pme_test_cpu[] to generate the event aliases for a PMU, and 
verify that the aliases are as expected.

> 
>    - you go through all detected pmus and check if the tests events we
>      generated are matching some of the events from these pmus,

Not exactly.

>      and that's where I'm lost ;-) why?

So consider the "cpu" HW PMU. During normal operation, we create the 
event aliases for this PMU in pmu_lookup()->pmu_add_cpu_aliases(). This 
step looks up a map of cpu events for that CPUID, and then creates the 
event aliases for that PMU from that map.

I want the test to recreate this and verify that the events from the 
test JSONs will have event aliases created properly.

So in the test when we scan the PMUs and find "cpu" HW PMU, we create a 
test PMU with the same name, create the event aliases from 
pme_test_cpu[] for that test PMU, and then verify that the event aliases 
created are as expected. Then the test PMU is deleted.

So overall the test covers:
a. jevents code to generate the struct pmu_event []
b. util/pmu.c code to create the event aliases for a given PMU

Note: the test does not (yet) cover matching of events declared in the 
HW PMU sysfs folder. I'm talking about these, for example:

$ ls /sys/bus/event_source/devices/cpu/events/
branch-instructions  cache-references  el-abort     el-start 
ref-cycles     ...

> 
>>
>>>
>>> or as I'm thinking about that now, would it be enough
>>> to check pme_test_cpu array to have string that we
>>> expect?
>>
>> Right, I might change this.
>>
>> So currently we iterate the PMU aliases to ensure that we have a matching
>> event in pme_test_cpu[]. It may be better to iterate the events in
>> pme_test_cpu[] to ensure that we have an alias.
> 
> that's what I described above.. I dont understand the connection/value
> of this tests
> 
>>
>> The problem here is uncore PMUs. They have the "Unit" field, which is used
>> for matching the PMU. So we cannot ensure test events from uncore.json will
>> always have an event alias created per PMU. But maybe I could use
>> pmu_uncore_alias_match() to check if the test event matches in this case.
> 
> hum I guess I don't follow all the details.. but some more explanation
> of the test would be great

Let's just concentrate on core PMU ATM :)

Thanks,
John

Re: [PATCH 6/6] perf test: Add pmu-events test

[Jiri Olsa]

On Mon, Mar 09, 2020 at 04:20:52PM +0000, John Garry wrote:
> > > 
> > > The events in test_cpu_aliases[] or test_uncore_aliases[] are checked
> > > against the events from pmu-events/arch/test/test_cpu/*.json
> > 
> 
> Hi Jirka,
> 
> > I don't understand the benefit of this.. so IIUC:
> > 
> >    - jevents will go through arch/test and populate pmu-events/pmu-events.c
> >      with:
> >         struct pmu_event pme_test_cpu[] ...
> >         struct pmu_events_map pmu_events_map_test ...
> 
> Right. And the idea is that pme_test_cpu[] can be used as generic set of
> events for testing on any arch/cpuid. (note: I'll just ignore uncore events
> for the moment)
> 
> > 
> >    - so we actualy have the parsed json events in C structs and we can go
> >      through them and check it contains fields with strings that we expect
> 
> No, we use pme_test_cpu[] to generate the event aliases for a PMU, and
> verify that the aliases are as expected.
> 
> > 
> >    - you go through all detected pmus and check if the tests events we
> >      generated are matching some of the events from these pmus,
> 
> Not exactly.
> 
> >      and that's where I'm lost ;-) why?
> 
> So consider the "cpu" HW PMU. During normal operation, we create the event
> aliases for this PMU in pmu_lookup()->pmu_add_cpu_aliases(). This step looks
> up a map of cpu events for that CPUID, and then creates the event aliases
> for that PMU from that map.
> 
> I want the test to recreate this and verify that the events from the test
> JSONs will have event aliases created properly.

aah ok, my first objective was to have some way to test pmu-events
changes we plan to do and their affect to generated pmu-event.c

you want to test the code paths after that.. perfect

> 
> So in the test when we scan the PMUs and find "cpu" HW PMU, we create a test
> PMU with the same name, create the event aliases from pme_test_cpu[] for
> that test PMU, and then verify that the event aliases created are as
> expected. Then the test PMU is deleted.
> 
> So overall the test covers:
> a. jevents code to generate the struct pmu_event []
> b. util/pmu.c code to create the event aliases for a given PMU
> 
> Note: the test does not (yet) cover matching of events declared in the HW
> PMU sysfs folder. I'm talking about these, for example:

ok

> 
> $ ls /sys/bus/event_source/devices/cpu/events/
> branch-instructions  cache-references  el-abort     el-start ref-cycles
> ...
> 
> > 
> > > 
> > > > 
> > > > or as I'm thinking about that now, would it be enough
> > > > to check pme_test_cpu array to have string that we
> > > > expect?
> > > 
> > > Right, I might change this.
> > > 
> > > So currently we iterate the PMU aliases to ensure that we have a matching
> > > event in pme_test_cpu[]. It may be better to iterate the events in
> > > pme_test_cpu[] to ensure that we have an alias.
> > 
> > that's what I described above.. I dont understand the connection/value
> > of this tests
> > 
> > > 
> > > The problem here is uncore PMUs. They have the "Unit" field, which is used
> > > for matching the PMU. So we cannot ensure test events from uncore.json will
> > > always have an event alias created per PMU. But maybe I could use
> > > pmu_uncore_alias_match() to check if the test event matches in this case.
> > 
> > hum I guess I don't follow all the details.. but some more explanation
> > of the test would be great
> 
> Let's just concentrate on core PMU ATM :)

ok, thanks,
jirka

Re: [PATCH 6/6] perf test: Add pmu-events test

[John Garry]

Hi jirka,

>>>
>>>     - so we actualy have the parsed json events in C structs and we can go
>>>       through them and check it contains fields with strings that we expect
>>
>> No, we use pme_test_cpu[] to generate the event aliases for a PMU, and
>> verify that the aliases are as expected.
>>
>>>
>>>     - you go through all detected pmus and check if the tests events we
>>>       generated are matching some of the events from these pmus,
>>
>> Not exactly.
>>
>>>       and that's where I'm lost ;-) why?
>>
>> So consider the "cpu" HW PMU. During normal operation, we create the event
>> aliases for this PMU in pmu_lookup()->pmu_add_cpu_aliases(). This step looks
>> up a map of cpu events for that CPUID, and then creates the event aliases
>> for that PMU from that map.
>>
>> I want the test to recreate this and verify that the events from the test
>> JSONs will have event aliases created properly.
> 
> aah ok, my first objective was to have some way to test pmu-events
> changes we plan to do and their affect to generated pmu-event.c

Since the format of the test events table is slightly different to 
regular event tables, I wasn't sure on the value there. But I could add it.

For that, I could modify how we generate pmu-events.c as to not have a 
separate special test table, but add the test events as a special entry 
in pmu_events_map[], with some fake test cpuid. That could be better if 
we want to verify the generated test event table.

> 
> you want to test the code paths after that.. perfect
> 
>>
>> So in the test when we scan the PMUs and find "cpu" HW PMU, we create a test
>> PMU with the same name, create the event aliases from pme_test_cpu[] for
>> that test PMU, and then verify that the event aliases created are as
>> expected. Then the test PMU is deleted.
>>
>> So overall the test covers:
>> a. jevents code to generate the struct pmu_event []
>> b. util/pmu.c code to create the event aliases for a given PMU
>>
>> Note: the test does not (yet) cover matching of events declared in the HW
>> PMU sysfs folder. I'm talking about these, for example:
> 
> ok
> 

I'm going to look at what I can do here. I just worry it will make 
things more arch specific and make the test brittle.

>>
>> $ ls /sys/bus/event_source/devices/cpu/events/
>> branch-instructions  cache-references  el-abort     el-start ref-cycles
>> ...
>>
>>>
>>>>
>>>>>
>>>>> or as I'm thinking about that now, would it be enough
>>>>> to check pme_test_cpu array to have string that we
>>>>> expect?
>>>>
>>>> Right, I might change this.
>>>>
>>>> So currently we iterate the PMU aliases to ensure that we have a matching
>>>> event in pme_test_cpu[]. It may be better to iterate the events in
>>>> pme_test_cpu[] to ensure that we have an alias.
>>>
>>> that's what I described above.. I dont understand the connection/value
>>> of this tests
>>>
>>>>
>>>> The problem here is uncore PMUs. They have the "Unit" field, which is used
>>>> for matching the PMU. So we cannot ensure test events from uncore.json will
>>>> always have an event alias created per PMU. But maybe I could use
>>>> pmu_uncore_alias_match() to check if the test event matches in this case.
>>>
>>> hum I guess I don't follow all the details.. but some more explanation
>>> of the test would be great
>>
>> Let's just concentrate on core PMU ATM :)

So just going back to the uncore events now, since they have the "Unit" 
property to match to specific uncore PMUs, we need to test them slightly 
differently to core PMUs. But I have improved the testing for that now.

Thanks,
John