[PATCH 0/3] perf tests: Add arch tests

[PATCH 0/3] perf tests: Add arch tests

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

To encourage developers to contribute arch-specific tests without
feeling guilty about messing up the generic perf tests, this patch
series introduces "arch tests" and then adds a test for the Intel CQM
perf code which was used to trigger a WARN_ON_ONCE(), fixed in commit
2c534c0da0a6 ("perf/x86/intel/cqm: Return cached counter value from
IRQ context").

The concept of additional lists of tests, like the arch tests, can be
taken further in the future, for instance Arnaldo has expressed
interest in adding tests that require root permissions, etc.

The patch series is based on tip/perf/core.

The original patch that inspired this series can be found here,

  https://lkml.kernel.org/r/1437561539-27939-1-git-send-email-matt@codeblueprint.co.uk

Matt Fleming (3):
  perf tests: Introduce iterator function for tests
  perf tests: Add arch tests
  perf tests: Add Intel CQM test

 tools/perf/arch/x86/Build                    |   2 +-
 tools/perf/arch/x86/include/arch-tests.h     |  18 +++
 tools/perf/arch/x86/tests/Build              |   9 +-
 tools/perf/arch/x86/tests/arch-tests.c       |  28 +++++
 tools/perf/arch/x86/tests/dwarf-unwind.c     |   1 +
 tools/perf/arch/x86/tests/intel-cqm.c        | 124 +++++++++++++++++++
 tools/perf/arch/x86/tests/perf-time-to-tsc.c | 163 ++++++++++++++++++++++++
 tools/perf/arch/x86/tests/rdpmc.c            | 174 ++++++++++++++++++++++++++
 tools/perf/tests/Build                       |   3 -
 tools/perf/tests/builtin-test.c              |  76 ++++++------
 tools/perf/tests/dwarf-unwind.c              |   4 +
 tools/perf/tests/perf-time-to-tsc.c          | 162 ------------------------
 tools/perf/tests/rdpmc.c                     | 177 ---------------------------
 tools/perf/tests/tests.h                     |   9 +-
 14 files changed, 562 insertions(+), 388 deletions(-)
 create mode 100644 tools/perf/arch/x86/include/arch-tests.h
 create mode 100644 tools/perf/arch/x86/tests/arch-tests.c
 create mode 100644 tools/perf/arch/x86/tests/intel-cqm.c
 create mode 100644 tools/perf/arch/x86/tests/perf-time-to-tsc.c
 create mode 100644 tools/perf/arch/x86/tests/rdpmc.c
 delete mode 100644 tools/perf/tests/perf-time-to-tsc.c
 delete mode 100644 tools/perf/tests/rdpmc.c

-- 
2.1.0

[PATCH 1/3] perf tests: Introduce iterator function for tests

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

In preparation for introducing more arrays of tests, e.g. "arch tests"
(architecture-specific tests), abstract the code to iterate over the
list of tests into a helper function.

This way, code that uses a 'struct test' doesn't need to worry about
how the tests are grouped together and changes to the list of tests
doesn't require changes to the code using it.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 tools/perf/tests/builtin-test.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 136cd934be66..8cf0601d1662 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -183,7 +183,7 @@ static struct test {
 	},
 };
 
-static bool perf_test__matches(int curr, int argc, const char *argv[])
+static bool perf_test__matches(struct test *test, int curr, int argc, const char *argv[])
 {
 	int i;
 
@@ -200,7 +200,7 @@ static bool perf_test__matches(int curr, int argc, const char *argv[])
 			continue;
 		}
 
-		if (strstr(tests[curr].desc, argv[i]))
+		if (strstr(test->desc, argv[i]))
 			return true;
 	}
 
@@ -237,27 +237,28 @@ static int run_test(struct test *test)
 	return err;
 }
 
+#define for_each_test(t)	 for (t = &tests[0]; t->func; t++)
+
 static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 {
+	struct test *t;
 	int i = 0;
 	int width = 0;
 
-	while (tests[i].func) {
-		int len = strlen(tests[i].desc);
+	for_each_test(t) {
+		int len = strlen(t->desc);
 
 		if (width < len)
 			width = len;
-		++i;
 	}
 
-	i = 0;
-	while (tests[i].func) {
+	for_each_test(t) {
 		int curr = i++, err;
 
-		if (!perf_test__matches(curr, argc, argv))
+		if (!perf_test__matches(t, curr, argc, argv))
 			continue;
 
-		pr_info("%2d: %-*s:", i, width, tests[curr].desc);
+		pr_info("%2d: %-*s:", i, width, t->desc);
 
 		if (intlist__find(skiplist, i)) {
 			color_fprintf(stderr, PERF_COLOR_YELLOW, " Skip (user override)\n");
@@ -265,8 +266,8 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 		}
 
 		pr_debug("\n--- start ---\n");
-		err = run_test(&tests[curr]);
-		pr_debug("---- end ----\n%s:", tests[curr].desc);
+		err = run_test(t);
+		pr_debug("---- end ----\n%s:", t->desc);
 
 		switch (err) {
 		case TEST_OK:
@@ -287,15 +288,14 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 
 static int perf_test__list(int argc, const char **argv)
 {
+	struct test *t;
 	int i = 0;
 
-	while (tests[i].func) {
-		int curr = i++;
-
-		if (argc > 1 && !strstr(tests[curr].desc, argv[1]))
+	for_each_test(t) {
+		if (argc > 1 && !strstr(t->desc, argv[1]))
 			continue;
 
-		pr_info("%2d: %s\n", i, tests[curr].desc);
+		pr_info("%2d: %s\n", ++i, t->desc);
 	}
 
 	return 0;
-- 
2.1.0

[PATCH 2/3] perf tests: Add arch tests

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

Tests that only make sense for some architectures currently live in
the same place as the generic tests. Move out the x86-specific tests
into tools/perf/arch/x86/tests and define an 'arch_tests' array, which
is the list of tests that only apply to the build architecture.

The main idea is to encourage developers to add arch tests to build
out perf's test coverage, without dumping everything in
tools/perf/tests.

We can also now begin to get rid of some of the #ifdef code that is
present in the generic perf tests.

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 tools/perf/arch/x86/Build                    |   2 +-
 tools/perf/arch/x86/include/arch-tests.h     |  18 +++
 tools/perf/arch/x86/tests/Build              |   8 +-
 tools/perf/arch/x86/tests/arch-tests.c       |  24 ++++
 tools/perf/arch/x86/tests/dwarf-unwind.c     |   1 +
 tools/perf/arch/x86/tests/perf-time-to-tsc.c | 163 ++++++++++++++++++++++++
 tools/perf/arch/x86/tests/rdpmc.c            | 174 ++++++++++++++++++++++++++
 tools/perf/tests/Build                       |   3 -
 tools/perf/tests/builtin-test.c              |  46 ++++---
 tools/perf/tests/dwarf-unwind.c              |   4 +
 tools/perf/tests/perf-time-to-tsc.c          | 162 ------------------------
 tools/perf/tests/rdpmc.c                     | 177 ---------------------------
 tools/perf/tests/tests.h                     |   9 +-
 13 files changed, 418 insertions(+), 373 deletions(-)
 create mode 100644 tools/perf/arch/x86/include/arch-tests.h
 create mode 100644 tools/perf/arch/x86/tests/arch-tests.c
 create mode 100644 tools/perf/arch/x86/tests/perf-time-to-tsc.c
 create mode 100644 tools/perf/arch/x86/tests/rdpmc.c
 delete mode 100644 tools/perf/tests/perf-time-to-tsc.c
 delete mode 100644 tools/perf/tests/rdpmc.c

diff --git a/tools/perf/arch/x86/Build b/tools/perf/arch/x86/Build
index 41bf61da476a..db52fa22d3a1 100644
--- a/tools/perf/arch/x86/Build
+++ b/tools/perf/arch/x86/Build
@@ -1,2 +1,2 @@
 libperf-y += util/
-libperf-$(CONFIG_DWARF_UNWIND) += tests/
+libperf-y += tests/
diff --git a/tools/perf/arch/x86/include/arch-tests.h b/tools/perf/arch/x86/include/arch-tests.h
new file mode 100644
index 000000000000..9e505c5f6947
--- /dev/null
+++ b/tools/perf/arch/x86/include/arch-tests.h
@@ -0,0 +1,18 @@
+#ifndef ARCH_TESTS_H
+#define ARCH_TESTS_H
+
+/* Tests */
+int test__rdpmc(void);
+int test__perf_time_to_tsc(void);
+int test__intel_cqm_count_nmi_context(void);
+
+#ifdef HAVE_DWARF_UNWIND_SUPPORT
+struct thread;
+struct perf_sample;
+int test__arch_unwind_sample(struct perf_sample *sample,
+			     struct thread *thread);
+#endif
+
+extern struct test arch_tests[];
+
+#endif
diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
index b30eff9bcc83..1381525dae06 100644
--- a/tools/perf/arch/x86/tests/Build
+++ b/tools/perf/arch/x86/tests/Build
@@ -1,2 +1,6 @@
-libperf-y += regs_load.o
-libperf-y += dwarf-unwind.o
+libperf-$(CONFIG_DWARF_UNWIND) += regs_load.o
+libperf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
+
+libperf-y += arch-tests.o
+libperf-y += rdpmc.o
+libperf-y += perf-time-to-tsc.o
diff --git a/tools/perf/arch/x86/tests/arch-tests.c b/tools/perf/arch/x86/tests/arch-tests.c
new file mode 100644
index 000000000000..0004fe48dc7e
--- /dev/null
+++ b/tools/perf/arch/x86/tests/arch-tests.c
@@ -0,0 +1,24 @@
+#include <string.h>
+#include "tests/tests.h"
+#include "arch-tests.h"
+
+struct test arch_tests[] = {
+	{
+		.desc = "x86 rdpmc test",
+		.func = test__rdpmc,
+	},
+	{
+		.desc = "Test converting perf time to TSC",
+		.func = test__perf_time_to_tsc,
+	},
+#ifdef HAVE_DWARF_UNWIND_SUPPORT
+	{
+		.desc = "Test dwarf unwind",
+		.func = test__dwarf_unwind,
+	},
+#endif
+	{
+		.func = NULL,
+	},
+
+};
diff --git a/tools/perf/arch/x86/tests/dwarf-unwind.c b/tools/perf/arch/x86/tests/dwarf-unwind.c
index d8bbf7ad1681..7f209ce827bf 100644
--- a/tools/perf/arch/x86/tests/dwarf-unwind.c
+++ b/tools/perf/arch/x86/tests/dwarf-unwind.c
@@ -5,6 +5,7 @@
 #include "event.h"
 #include "debug.h"
 #include "tests/tests.h"
+#include "arch-tests.h"
 
 #define STACK_SIZE 8192
 
diff --git a/tools/perf/arch/x86/tests/perf-time-to-tsc.c b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
new file mode 100644
index 000000000000..5880768d1e5c
--- /dev/null
+++ b/tools/perf/arch/x86/tests/perf-time-to-tsc.c
@@ -0,0 +1,163 @@
+#include <stdio.h>
+#include <unistd.h>
+#include <linux/types.h>
+#include <sys/prctl.h>
+
+#include "parse-events.h"
+#include "evlist.h"
+#include "evsel.h"
+#include "thread_map.h"
+#include "cpumap.h"
+#include "tsc.h"
+#include "tests/tests.h"
+#include "arch-tests.h"
+
+#define CHECK__(x) {				\
+	while ((x) < 0) {			\
+		pr_debug(#x " failed!\n");	\
+		goto out_err;			\
+	}					\
+}
+
+#define CHECK_NOT_NULL__(x) {			\
+	while ((x) == NULL) {			\
+		pr_debug(#x " failed!\n");	\
+		goto out_err;			\
+	}					\
+}
+
+/**
+ * test__perf_time_to_tsc - test converting perf time to TSC.
+ *
+ * This function implements a test that checks that the conversion of perf time
+ * to and from TSC is consistent with the order of events.  If the test passes
+ * %0 is returned, otherwise %-1 is returned.  If TSC conversion is not
+ * supported then then the test passes but " (not supported)" is printed.
+ */
+int test__perf_time_to_tsc(void)
+{
+	struct record_opts opts = {
+		.mmap_pages	     = UINT_MAX,
+		.user_freq	     = UINT_MAX,
+		.user_interval	     = ULLONG_MAX,
+		.freq		     = 4000,
+		.target		     = {
+			.uses_mmap   = true,
+		},
+		.sample_time	     = true,
+	};
+	struct thread_map *threads = NULL;
+	struct cpu_map *cpus = NULL;
+	struct perf_evlist *evlist = NULL;
+	struct perf_evsel *evsel = NULL;
+	int err = -1, ret, i;
+	const char *comm1, *comm2;
+	struct perf_tsc_conversion tc;
+	struct perf_event_mmap_page *pc;
+	union perf_event *event;
+	u64 test_tsc, comm1_tsc, comm2_tsc;
+	u64 test_time, comm1_time = 0, comm2_time = 0;
+
+	threads = thread_map__new(-1, getpid(), UINT_MAX);
+	CHECK_NOT_NULL__(threads);
+
+	cpus = cpu_map__new(NULL);
+	CHECK_NOT_NULL__(cpus);
+
+	evlist = perf_evlist__new();
+	CHECK_NOT_NULL__(evlist);
+
+	perf_evlist__set_maps(evlist, cpus, threads);
+
+	CHECK__(parse_events(evlist, "cycles:u", NULL));
+
+	perf_evlist__config(evlist, &opts);
+
+	evsel = perf_evlist__first(evlist);
+
+	evsel->attr.comm = 1;
+	evsel->attr.disabled = 1;
+	evsel->attr.enable_on_exec = 0;
+
+	CHECK__(perf_evlist__open(evlist));
+
+	CHECK__(perf_evlist__mmap(evlist, UINT_MAX, false));
+
+	pc = evlist->mmap[0].base;
+	ret = perf_read_tsc_conversion(pc, &tc);
+	if (ret) {
+		if (ret == -EOPNOTSUPP) {
+			fprintf(stderr, " (not supported)");
+			return 0;
+		}
+		goto out_err;
+	}
+
+	perf_evlist__enable(evlist);
+
+	comm1 = "Test COMM 1";
+	CHECK__(prctl(PR_SET_NAME, (unsigned long)comm1, 0, 0, 0));
+
+	test_tsc = rdtsc();
+
+	comm2 = "Test COMM 2";
+	CHECK__(prctl(PR_SET_NAME, (unsigned long)comm2, 0, 0, 0));
+
+	perf_evlist__disable(evlist);
+
+	for (i = 0; i < evlist->nr_mmaps; i++) {
+		while ((event = perf_evlist__mmap_read(evlist, i)) != NULL) {
+			struct perf_sample sample;
+
+			if (event->header.type != PERF_RECORD_COMM ||
+			    (pid_t)event->comm.pid != getpid() ||
+			    (pid_t)event->comm.tid != getpid())
+				goto next_event;
+
+			if (strcmp(event->comm.comm, comm1) == 0) {
+				CHECK__(perf_evsel__parse_sample(evsel, event,
+								 &sample));
+				comm1_time = sample.time;
+			}
+			if (strcmp(event->comm.comm, comm2) == 0) {
+				CHECK__(perf_evsel__parse_sample(evsel, event,
+								 &sample));
+				comm2_time = sample.time;
+			}
+next_event:
+			perf_evlist__mmap_consume(evlist, i);
+		}
+	}
+
+	if (!comm1_time || !comm2_time)
+		goto out_err;
+
+	test_time = tsc_to_perf_time(test_tsc, &tc);
+	comm1_tsc = perf_time_to_tsc(comm1_time, &tc);
+	comm2_tsc = perf_time_to_tsc(comm2_time, &tc);
+
+	pr_debug("1st event perf time %"PRIu64" tsc %"PRIu64"\n",
+		 comm1_time, comm1_tsc);
+	pr_debug("rdtsc          time %"PRIu64" tsc %"PRIu64"\n",
+		 test_time, test_tsc);
+	pr_debug("2nd event perf time %"PRIu64" tsc %"PRIu64"\n",
+		 comm2_time, comm2_tsc);
+
+	if (test_time <= comm1_time ||
+	    test_time >= comm2_time)
+		goto out_err;
+
+	if (test_tsc <= comm1_tsc ||
+	    test_tsc >= comm2_tsc)
+		goto out_err;
+
+	err = 0;
+
+out_err:
+	if (evlist) {
+		perf_evlist__disable(evlist);
+		perf_evlist__delete(evlist);
+	}
+
+	return err;
+}
diff --git a/tools/perf/arch/x86/tests/rdpmc.c b/tools/perf/arch/x86/tests/rdpmc.c
new file mode 100644
index 000000000000..e7688214c7cf
--- /dev/null
+++ b/tools/perf/arch/x86/tests/rdpmc.c
@@ -0,0 +1,174 @@
+#include <unistd.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <sys/mman.h>
+#include <linux/types.h>
+#include "perf.h"
+#include "debug.h"
+#include "tests/tests.h"
+#include "cloexec.h"
+#include "arch-tests.h"
+
+static u64 rdpmc(unsigned int counter)
+{
+	unsigned int low, high;
+
+	asm volatile("rdpmc" : "=a" (low), "=d" (high) : "c" (counter));
+
+	return low | ((u64)high) << 32;
+}
+
+static u64 rdtsc(void)
+{
+	unsigned int low, high;
+
+	asm volatile("rdtsc" : "=a" (low), "=d" (high));
+
+	return low | ((u64)high) << 32;
+}
+
+static u64 mmap_read_self(void *addr)
+{
+	struct perf_event_mmap_page *pc = addr;
+	u32 seq, idx, time_mult = 0, time_shift = 0;
+	u64 count, cyc = 0, time_offset = 0, enabled, running, delta;
+
+	do {
+		seq = pc->lock;
+		barrier();
+
+		enabled = pc->time_enabled;
+		running = pc->time_running;
+
+		if (enabled != running) {
+			cyc = rdtsc();
+			time_mult = pc->time_mult;
+			time_shift = pc->time_shift;
+			time_offset = pc->time_offset;
+		}
+
+		idx = pc->index;
+		count = pc->offset;
+		if (idx)
+			count += rdpmc(idx - 1);
+
+		barrier();
+	} while (pc->lock != seq);
+
+	if (enabled != running) {
+		u64 quot, rem;
+
+		quot = (cyc >> time_shift);
+		rem = cyc & ((1 << time_shift) - 1);
+		delta = time_offset + quot * time_mult +
+			((rem * time_mult) >> time_shift);
+
+		enabled += delta;
+		if (idx)
+			running += delta;
+
+		quot = count / running;
+		rem = count % running;
+		count = quot * enabled + (rem * enabled) / running;
+	}
+
+	return count;
+}
+
+/*
+ * If the RDPMC instruction faults then signal this back to the test parent task:
+ */
+static void segfault_handler(int sig __maybe_unused,
+			     siginfo_t *info __maybe_unused,
+			     void *uc __maybe_unused)
+{
+	exit(-1);
+}
+
+static int __test__rdpmc(void)
+{
+	volatile int tmp = 0;
+	u64 i, loops = 1000;
+	int n;
+	int fd;
+	void *addr;
+	struct perf_event_attr attr = {
+		.type = PERF_TYPE_HARDWARE,
+		.config = PERF_COUNT_HW_INSTRUCTIONS,
+		.exclude_kernel = 1,
+	};
+	u64 delta_sum = 0;
+        struct sigaction sa;
+	char sbuf[STRERR_BUFSIZE];
+
+	sigfillset(&sa.sa_mask);
+	sa.sa_sigaction = segfault_handler;
+	sigaction(SIGSEGV, &sa, NULL);
+
+	fd = sys_perf_event_open(&attr, 0, -1, -1,
+				 perf_event_open_cloexec_flag());
+	if (fd < 0) {
+		pr_err("Error: sys_perf_event_open() syscall returned "
+		       "with %d (%s)\n", fd,
+		       strerror_r(errno, sbuf, sizeof(sbuf)));
+		return -1;
+	}
+
+	addr = mmap(NULL, page_size, PROT_READ, MAP_SHARED, fd, 0);
+	if (addr == (void *)(-1)) {
+		pr_err("Error: mmap() syscall returned with (%s)\n",
+		       strerror_r(errno, sbuf, sizeof(sbuf)));
+		goto out_close;
+	}
+
+	for (n = 0; n < 6; n++) {
+		u64 stamp, now, delta;
+
+		stamp = mmap_read_self(addr);
+
+		for (i = 0; i < loops; i++)
+			tmp++;
+
+		now = mmap_read_self(addr);
+		loops *= 10;
+
+		delta = now - stamp;
+		pr_debug("%14d: %14Lu\n", n, (long long)delta);
+
+		delta_sum += delta;
+	}
+
+	munmap(addr, page_size);
+	pr_debug("   ");
+out_close:
+	close(fd);
+
+	if (!delta_sum)
+		return -1;
+
+	return 0;
+}
+
+int test__rdpmc(void)
+{
+	int status = 0;
+	int wret = 0;
+	int ret;
+	int pid;
+
+	pid = fork();
+	if (pid < 0)
+		return -1;
+
+	if (!pid) {
+		ret = __test__rdpmc();
+
+		exit(ret);
+	}
+
+	wret = waitpid(pid, &status, 0);
+	if (wret < 0 || status)
+		return -1;
+
+	return 0;
+}
diff --git a/tools/perf/tests/Build b/tools/perf/tests/Build
index c1518bdd0f1b..fa41e4f265fe 100644
--- a/tools/perf/tests/Build
+++ b/tools/perf/tests/Build
@@ -8,7 +8,6 @@ perf-y += openat-syscall-all-cpus.o
 perf-y += openat-syscall-tp-fields.o
 perf-y += mmap-basic.o
 perf-y += perf-record.o
-perf-y += rdpmc.o
 perf-y += evsel-roundtrip-name.o
 perf-y += evsel-tp-sched.o
 perf-y += fdarray.o
@@ -34,8 +33,6 @@ perf-y += kmod-path.o
 perf-y += thread-map.o
 perf-y += llvm.o
 
-perf-$(CONFIG_X86) += perf-time-to-tsc.o
-
 ifeq ($(ARCH),$(filter $(ARCH),x86 arm arm64))
 perf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
 endif
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 8cf0601d1662..a1b2265eaf55 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -14,10 +14,17 @@
 #include "parse-options.h"
 #include "symbol.h"
 
-static struct test {
-	const char *desc;
-	int (*func)(void);
-} tests[] = {
+#if defined(__x86_64__) || defined(__i386__)
+#include "arch-tests.h"
+#else
+static struct test arch_tests[] = {
+	{
+		.func = NULL,
+	},
+};
+#endif
+
+static struct test generic_tests[] = {
 	{
 		.desc = "vmlinux symtab matches kallsyms",
 		.func = test__vmlinux_matches_kallsyms,
@@ -38,12 +45,6 @@ static struct test {
 		.desc = "parse events tests",
 		.func = test__parse_events,
 	},
-#if defined(__x86_64__) || defined(__i386__)
-	{
-		.desc = "x86 rdpmc test",
-		.func = test__rdpmc,
-	},
-#endif
 	{
 		.desc = "Validate PERF_RECORD_* events & perf_sample fields",
 		.func = test__PERF_RECORD,
@@ -104,12 +105,6 @@ static struct test {
 		.desc = "Test software clock events have valid period values",
 		.func = test__sw_clock_freq,
 	},
-#if defined(__x86_64__) || defined(__i386__)
-	{
-		.desc = "Test converting perf time to TSC",
-		.func = test__perf_time_to_tsc,
-	},
-#endif
 	{
 		.desc = "Test object code reading",
 		.func = test__code_reading,
@@ -126,14 +121,6 @@ static struct test {
 		.desc = "Test parsing with no sample_id_all bit set",
 		.func = test__parse_no_sample_id_all,
 	},
-#if defined(__x86_64__) || defined(__i386__) || defined(__arm__) || defined(__aarch64__)
-#ifdef HAVE_DWARF_UNWIND_SUPPORT
-	{
-		.desc = "Test dwarf unwind",
-		.func = test__dwarf_unwind,
-	},
-#endif
-#endif
 	{
 		.desc = "Test filtering hist entries",
 		.func = test__hists_filter,
@@ -183,6 +170,11 @@ static struct test {
 	},
 };
 
+static struct test *tests[] = {
+	generic_tests,
+	arch_tests,
+};
+
 static bool perf_test__matches(struct test *test, int curr, int argc, const char *argv[])
 {
 	int i;
@@ -237,7 +229,11 @@ static int run_test(struct test *test)
 	return err;
 }
 
-#define for_each_test(t)	 for (t = &tests[0]; t->func; t++)
+static unsigned int ___j;		/* This is obviously not thread-safe */
+
+#define for_each_test(t)	 				\
+	for (___j = 0; ___j < ARRAY_SIZE(tests); ___j++)	\
+		for (t = &tests[___j][0]; t->func; t++)
 
 static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 {
diff --git a/tools/perf/tests/dwarf-unwind.c b/tools/perf/tests/dwarf-unwind.c
index 40b36c462427..07221793a3ac 100644
--- a/tools/perf/tests/dwarf-unwind.c
+++ b/tools/perf/tests/dwarf-unwind.c
@@ -11,6 +11,10 @@
 #include "thread.h"
 #include "callchain.h"
 
+#if defined (__x86_64__) || defined (__i386__)
+#include "arch-tests.h"
+#endif
+
 /* For bsearch. We try to unwind functions in shared object. */
 #include <stdlib.h>
 
diff --git a/tools/perf/tests/perf-time-to-tsc.c b/tools/perf/tests/perf-time-to-tsc.c
deleted file mode 100644
index 5f49484f1abc..000000000000
--- a/tools/perf/tests/perf-time-to-tsc.c
+++ /dev/null
@@ -1,162 +0,0 @@
-#include <stdio.h>
-#include <unistd.h>
-#include <linux/types.h>
-#include <sys/prctl.h>
-
-#include "parse-events.h"
-#include "evlist.h"
-#include "evsel.h"
-#include "thread_map.h"
-#include "cpumap.h"
-#include "tsc.h"
-#include "tests.h"
-
-#define CHECK__(x) {				\
-	while ((x) < 0) {			\
-		pr_debug(#x " failed!\n");	\
-		goto out_err;			\
-	}					\
-}
-
-#define CHECK_NOT_NULL__(x) {			\
-	while ((x) == NULL) {			\
-		pr_debug(#x " failed!\n");	\
-		goto out_err;			\
-	}					\
-}
-
-/**
- * test__perf_time_to_tsc - test converting perf time to TSC.
- *
- * This function implements a test that checks that the conversion of perf time
- * to and from TSC is consistent with the order of events.  If the test passes
- * %0 is returned, otherwise %-1 is returned.  If TSC conversion is not
- * supported then then the test passes but " (not supported)" is printed.
- */
-int test__perf_time_to_tsc(void)
-{
-	struct record_opts opts = {
-		.mmap_pages	     = UINT_MAX,
-		.user_freq	     = UINT_MAX,
-		.user_interval	     = ULLONG_MAX,
-		.freq		     = 4000,
-		.target		     = {
-			.uses_mmap   = true,
-		},
-		.sample_time	     = true,
-	};
-	struct thread_map *threads = NULL;
-	struct cpu_map *cpus = NULL;
-	struct perf_evlist *evlist = NULL;
-	struct perf_evsel *evsel = NULL;
-	int err = -1, ret, i;
-	const char *comm1, *comm2;
-	struct perf_tsc_conversion tc;
-	struct perf_event_mmap_page *pc;
-	union perf_event *event;
-	u64 test_tsc, comm1_tsc, comm2_tsc;
-	u64 test_time, comm1_time = 0, comm2_time = 0;
-
-	threads = thread_map__new(-1, getpid(), UINT_MAX);
-	CHECK_NOT_NULL__(threads);
-
-	cpus = cpu_map__new(NULL);
-	CHECK_NOT_NULL__(cpus);
-
-	evlist = perf_evlist__new();
-	CHECK_NOT_NULL__(evlist);
-
-	perf_evlist__set_maps(evlist, cpus, threads);
-
-	CHECK__(parse_events(evlist, "cycles:u", NULL));
-
-	perf_evlist__config(evlist, &opts);
-
-	evsel = perf_evlist__first(evlist);
-
-	evsel->attr.comm = 1;
-	evsel->attr.disabled = 1;
-	evsel->attr.enable_on_exec = 0;
-
-	CHECK__(perf_evlist__open(evlist));
-
-	CHECK__(perf_evlist__mmap(evlist, UINT_MAX, false));
-
-	pc = evlist->mmap[0].base;
-	ret = perf_read_tsc_conversion(pc, &tc);
-	if (ret) {
-		if (ret == -EOPNOTSUPP) {
-			fprintf(stderr, " (not supported)");
-			return 0;
-		}
-		goto out_err;
-	}
-
-	perf_evlist__enable(evlist);
-
-	comm1 = "Test COMM 1";
-	CHECK__(prctl(PR_SET_NAME, (unsigned long)comm1, 0, 0, 0));
-
-	test_tsc = rdtsc();
-
-	comm2 = "Test COMM 2";
-	CHECK__(prctl(PR_SET_NAME, (unsigned long)comm2, 0, 0, 0));
-
-	perf_evlist__disable(evlist);
-
-	for (i = 0; i < evlist->nr_mmaps; i++) {
-		while ((event = perf_evlist__mmap_read(evlist, i)) != NULL) {
-			struct perf_sample sample;
-
-			if (event->header.type != PERF_RECORD_COMM ||
-			    (pid_t)event->comm.pid != getpid() ||
-			    (pid_t)event->comm.tid != getpid())
-				goto next_event;
-
-			if (strcmp(event->comm.comm, comm1) == 0) {
-				CHECK__(perf_evsel__parse_sample(evsel, event,
-								 &sample));
-				comm1_time = sample.time;
-			}
-			if (strcmp(event->comm.comm, comm2) == 0) {
-				CHECK__(perf_evsel__parse_sample(evsel, event,
-								 &sample));
-				comm2_time = sample.time;
-			}
-next_event:
-			perf_evlist__mmap_consume(evlist, i);
-		}
-	}
-
-	if (!comm1_time || !comm2_time)
-		goto out_err;
-
-	test_time = tsc_to_perf_time(test_tsc, &tc);
-	comm1_tsc = perf_time_to_tsc(comm1_time, &tc);
-	comm2_tsc = perf_time_to_tsc(comm2_time, &tc);
-
-	pr_debug("1st event perf time %"PRIu64" tsc %"PRIu64"\n",
-		 comm1_time, comm1_tsc);
-	pr_debug("rdtsc          time %"PRIu64" tsc %"PRIu64"\n",
-		 test_time, test_tsc);
-	pr_debug("2nd event perf time %"PRIu64" tsc %"PRIu64"\n",
-		 comm2_time, comm2_tsc);
-
-	if (test_time <= comm1_time ||
-	    test_time >= comm2_time)
-		goto out_err;
-
-	if (test_tsc <= comm1_tsc ||
-	    test_tsc >= comm2_tsc)
-		goto out_err;
-
-	err = 0;
-
-out_err:
-	if (evlist) {
-		perf_evlist__disable(evlist);
-		perf_evlist__delete(evlist);
-	}
-
-	return err;
-}
diff --git a/tools/perf/tests/rdpmc.c b/tools/perf/tests/rdpmc.c
deleted file mode 100644
index d31f2c4d9f64..000000000000
--- a/tools/perf/tests/rdpmc.c
+++ /dev/null
@@ -1,177 +0,0 @@
-#include <unistd.h>
-#include <stdlib.h>
-#include <signal.h>
-#include <sys/mman.h>
-#include <linux/types.h>
-#include "perf.h"
-#include "debug.h"
-#include "tests.h"
-#include "cloexec.h"
-
-#if defined(__x86_64__) || defined(__i386__)
-
-static u64 rdpmc(unsigned int counter)
-{
-	unsigned int low, high;
-
-	asm volatile("rdpmc" : "=a" (low), "=d" (high) : "c" (counter));
-
-	return low | ((u64)high) << 32;
-}
-
-static u64 rdtsc(void)
-{
-	unsigned int low, high;
-
-	asm volatile("rdtsc" : "=a" (low), "=d" (high));
-
-	return low | ((u64)high) << 32;
-}
-
-static u64 mmap_read_self(void *addr)
-{
-	struct perf_event_mmap_page *pc = addr;
-	u32 seq, idx, time_mult = 0, time_shift = 0;
-	u64 count, cyc = 0, time_offset = 0, enabled, running, delta;
-
-	do {
-		seq = pc->lock;
-		barrier();
-
-		enabled = pc->time_enabled;
-		running = pc->time_running;
-
-		if (enabled != running) {
-			cyc = rdtsc();
-			time_mult = pc->time_mult;
-			time_shift = pc->time_shift;
-			time_offset = pc->time_offset;
-		}
-
-		idx = pc->index;
-		count = pc->offset;
-		if (idx)
-			count += rdpmc(idx - 1);
-
-		barrier();
-	} while (pc->lock != seq);
-
-	if (enabled != running) {
-		u64 quot, rem;
-
-		quot = (cyc >> time_shift);
-		rem = cyc & ((1 << time_shift) - 1);
-		delta = time_offset + quot * time_mult +
-			((rem * time_mult) >> time_shift);
-
-		enabled += delta;
-		if (idx)
-			running += delta;
-
-		quot = count / running;
-		rem = count % running;
-		count = quot * enabled + (rem * enabled) / running;
-	}
-
-	return count;
-}
-
-/*
- * If the RDPMC instruction faults then signal this back to the test parent task:
- */
-static void segfault_handler(int sig __maybe_unused,
-			     siginfo_t *info __maybe_unused,
-			     void *uc __maybe_unused)
-{
-	exit(-1);
-}
-
-static int __test__rdpmc(void)
-{
-	volatile int tmp = 0;
-	u64 i, loops = 1000;
-	int n;
-	int fd;
-	void *addr;
-	struct perf_event_attr attr = {
-		.type = PERF_TYPE_HARDWARE,
-		.config = PERF_COUNT_HW_INSTRUCTIONS,
-		.exclude_kernel = 1,
-	};
-	u64 delta_sum = 0;
-        struct sigaction sa;
-	char sbuf[STRERR_BUFSIZE];
-
-	sigfillset(&sa.sa_mask);
-	sa.sa_sigaction = segfault_handler;
-	sigaction(SIGSEGV, &sa, NULL);
-
-	fd = sys_perf_event_open(&attr, 0, -1, -1,
-				 perf_event_open_cloexec_flag());
-	if (fd < 0) {
-		pr_err("Error: sys_perf_event_open() syscall returned "
-		       "with %d (%s)\n", fd,
-		       strerror_r(errno, sbuf, sizeof(sbuf)));
-		return -1;
-	}
-
-	addr = mmap(NULL, page_size, PROT_READ, MAP_SHARED, fd, 0);
-	if (addr == (void *)(-1)) {
-		pr_err("Error: mmap() syscall returned with (%s)\n",
-		       strerror_r(errno, sbuf, sizeof(sbuf)));
-		goto out_close;
-	}
-
-	for (n = 0; n < 6; n++) {
-		u64 stamp, now, delta;
-
-		stamp = mmap_read_self(addr);
-
-		for (i = 0; i < loops; i++)
-			tmp++;
-
-		now = mmap_read_self(addr);
-		loops *= 10;
-
-		delta = now - stamp;
-		pr_debug("%14d: %14Lu\n", n, (long long)delta);
-
-		delta_sum += delta;
-	}
-
-	munmap(addr, page_size);
-	pr_debug("   ");
-out_close:
-	close(fd);
-
-	if (!delta_sum)
-		return -1;
-
-	return 0;
-}
-
-int test__rdpmc(void)
-{
-	int status = 0;
-	int wret = 0;
-	int ret;
-	int pid;
-
-	pid = fork();
-	if (pid < 0)
-		return -1;
-
-	if (!pid) {
-		ret = __test__rdpmc();
-
-		exit(ret);
-	}
-
-	wret = waitpid(pid, &status, 0);
-	if (wret < 0 || status)
-		return -1;
-
-	return 0;
-}
-
-#endif
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index bf113a247987..cad677d202bb 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -24,13 +24,17 @@ enum {
 	TEST_SKIP = -2,
 };
 
+struct test {
+	const char *desc;
+	int (*func)(void);
+};
+
 /* Tests */
 int test__vmlinux_matches_kallsyms(void);
 int test__openat_syscall_event(void);
 int test__openat_syscall_event_on_all_cpus(void);
 int test__basic_mmap(void);
 int test__PERF_RECORD(void);
-int test__rdpmc(void);
 int test__perf_evsel__roundtrip_name_test(void);
 int test__perf_evsel__tp_sched_test(void);
 int test__syscall_openat_tp_fields(void);
@@ -46,7 +50,6 @@ int test__bp_signal(void);
 int test__bp_signal_overflow(void);
 int test__task_exit(void);
 int test__sw_clock_freq(void);
-int test__perf_time_to_tsc(void);
 int test__code_reading(void);
 int test__sample_parsing(void);
 int test__keep_tracking(void);
@@ -64,7 +67,7 @@ int test__kmod_path__parse(void);
 int test__thread_map(void);
 int test__llvm(void);
 
-#if defined(__x86_64__) || defined(__i386__) || defined(__arm__) || defined(__aarch64__)
+#if defined(__arm__) || defined(__aarch64__)
 #ifdef HAVE_DWARF_UNWIND_SUPPORT
 struct thread;
 struct perf_sample;
-- 
2.1.0

[PATCH 3/3] perf tests: Add Intel CQM test

[Matt Fleming]

From: Matt Fleming <matt.fleming@intel.com>

Peter reported that it's possible to trigger a WARN_ON_ONCE() in the
Intel CQM code by combining a hardware event and an Intel CQM
(software) event into a group, which was fixed in commit 2c534c0da0a6
("perf/x86/intel/cqm: Return cached counter value from IRQ context").
Unfortunately, the perf tools were not able to create this bundle and
we had to manually construct a test case.

For posterity, record Peter's proof of concept test case in tools/perf
so that it presents a model for how we can perform architecture
specific tests, or "arch tests", in perf in the future.

The particular issue triggered in the test case is that when the
counter for the hardware event overflows and triggers a PMI we'll read
both the hardware event and the software event counters.
Unfortunately, for CQM that involves performing an IPI to read the CQM
event counters on all sockets, which in NMI context triggers the
WARN_ON_ONCE().

Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Link: http://lkml.kernel.org/r/1437490509-15373-1-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Matt Fleming <matt.fleming@intel.com>
---
 tools/perf/arch/x86/tests/Build        |   1 +
 tools/perf/arch/x86/tests/arch-tests.c |   4 ++
 tools/perf/arch/x86/tests/intel-cqm.c  | 124 +++++++++++++++++++++++++++++++++
 3 files changed, 129 insertions(+)
 create mode 100644 tools/perf/arch/x86/tests/intel-cqm.c

diff --git a/tools/perf/arch/x86/tests/Build b/tools/perf/arch/x86/tests/Build
index 1381525dae06..32996f54e123 100644
--- a/tools/perf/arch/x86/tests/Build
+++ b/tools/perf/arch/x86/tests/Build
@@ -4,3 +4,4 @@ libperf-$(CONFIG_DWARF_UNWIND) += dwarf-unwind.o
 libperf-y += arch-tests.o
 libperf-y += rdpmc.o
 libperf-y += perf-time-to-tsc.o
+libperf-y += intel-cqm.o
diff --git a/tools/perf/arch/x86/tests/arch-tests.c b/tools/perf/arch/x86/tests/arch-tests.c
index 0004fe48dc7e..3ab31adc283a 100644
--- a/tools/perf/arch/x86/tests/arch-tests.c
+++ b/tools/perf/arch/x86/tests/arch-tests.c
@@ -18,6 +18,10 @@ struct test arch_tests[] = {
 	},
 #endif
 	{
+		.desc = "Test intel cqm nmi context read",
+		.func = test__intel_cqm_count_nmi_context,
+	},
+	{
 		.func = NULL,
 	},
 
diff --git a/tools/perf/arch/x86/tests/intel-cqm.c b/tools/perf/arch/x86/tests/intel-cqm.c
new file mode 100644
index 000000000000..d28c1b6a3b54
--- /dev/null
+++ b/tools/perf/arch/x86/tests/intel-cqm.c
@@ -0,0 +1,124 @@
+#include "tests/tests.h"
+#include "perf.h"
+#include "cloexec.h"
+#include "debug.h"
+#include "evlist.h"
+#include "evsel.h"
+#include "arch-tests.h"
+
+#include <sys/mman.h>
+#include <string.h>
+
+static pid_t spawn(void)
+{
+	pid_t pid;
+
+	pid = fork();
+	if (pid)
+		return pid;
+
+	while(1);
+		sleep(5);
+	return 0;
+}
+
+/*
+ * Create an event group that contains both a sampled hardware
+ * (cpu-cycles) and software (intel_cqm/llc_occupancy/) event. We then
+ * wait for the hardware perf counter to overflow and generate a PMI,
+ * which triggers an event read for both of the events in the group.
+ *
+ * Since reading Intel CQM event counters requires sending SMP IPIs, the
+ * CQM pmu needs to handle the above situation gracefully, and return
+ * the last read counter value to avoid triggering a WARN_ON_ONCE() in
+ * smp_call_function_many() caused by sending IPIs from NMI context.
+ */
+int test__intel_cqm_count_nmi_context(void)
+{
+	struct perf_evlist *evlist = NULL;
+	struct perf_evsel *evsel = NULL;
+	struct perf_event_attr pe;
+	int i, fd[2], flag, ret;
+	size_t mmap_len;
+	void *event;
+	pid_t pid;
+	int err = TEST_FAIL;
+
+	flag = perf_event_open_cloexec_flag();
+
+	evlist = perf_evlist__new();
+	if (!evlist) {
+		pr_debug("perf_evlist__new failed\n");
+		return TEST_FAIL;
+	}
+
+	ret = parse_events(evlist, "intel_cqm/llc_occupancy/", NULL);
+	if (ret) {
+		pr_debug("parse_events failed\n");
+		err = TEST_SKIP;
+		goto out;
+	}
+
+	evsel = perf_evlist__first(evlist);
+	if (!evsel) {
+		pr_debug("perf_evlist__first failed\n");
+		goto out;
+	}
+
+	memset(&pe, 0, sizeof(pe));
+	pe.size = sizeof(pe);
+
+	pe.type = PERF_TYPE_HARDWARE;
+	pe.config = PERF_COUNT_HW_CPU_CYCLES;
+	pe.read_format = PERF_FORMAT_GROUP;
+
+	pe.sample_period = 128;
+	pe.sample_type = PERF_SAMPLE_IP | PERF_SAMPLE_READ;
+
+	pid = spawn();
+
+	fd[0] = sys_perf_event_open(&pe, pid, -1, -1, flag);
+	if (fd[0] < 0) {
+		pr_debug("failed to open event\n");
+		goto out;
+	}
+
+	memset(&pe, 0, sizeof(pe));
+	pe.size = sizeof(pe);
+
+	pe.type = evsel->attr.type;
+	pe.config = evsel->attr.config;
+
+	fd[1] = sys_perf_event_open(&pe, pid, -1, fd[0], flag);
+	if (fd[1] < 0) {
+		pr_debug("failed to open event\n");
+		goto out;
+	}
+
+	/*
+	 * Pick a power-of-two number of pages + 1 for the meta-data
+	 * page (struct perf_event_mmap_page). See tools/perf/design.txt.
+	 */
+	mmap_len = page_size * 65;
+
+	event = mmap(NULL, mmap_len, PROT_READ, MAP_SHARED, fd[0], 0);
+	if (event == (void *)(-1)) {
+		pr_debug("failed to mmap %d\n", errno);
+		goto out;
+	}
+
+	sleep(1);
+
+	err = TEST_OK;
+
+	munmap(event, mmap_len);
+
+	for (i = 0; i < 2; i++)
+		close(fd[i]);
+
+	kill(pid, SIGKILL);
+	wait(NULL);
+out:
+	perf_evlist__delete(evlist);
+	return err;
+}
-- 
2.1.0

Re: [PATCH 2/3] perf tests: Add arch tests

[Jiri Olsa]

On Sat, Sep 05, 2015 at 08:02:21PM +0100, Matt Fleming wrote:

SNIP

> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 8cf0601d1662..a1b2265eaf55 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -14,10 +14,17 @@
>  #include "parse-options.h"
>  #include "symbol.h"
>  
> -static struct test {
> -	const char *desc;
> -	int (*func)(void);
> -} tests[] = {
> +#if defined(__x86_64__) || defined(__i386__)
> +#include "arch-tests.h"
> +#else
> +static struct test arch_tests[] = {
> +	{
> +		.func = NULL,
> +	},
> +};
> +#endif

this could be defined as __weak array so we dont need to have #if above

jirka

Re: [PATCH 2/3] perf tests: Add arch tests

[Jiri Olsa]

On Sat, Sep 05, 2015 at 08:02:21PM +0100, Matt Fleming wrote:

SNIP

>  };
>  
> +static struct test *tests[] = {
> +	generic_tests,
> +	arch_tests,
> +};
> +
>  static bool perf_test__matches(struct test *test, int curr, int argc, const char *argv[])
>  {
>  	int i;
> @@ -237,7 +229,11 @@ static int run_test(struct test *test)
>  	return err;
>  }
>  
> -#define for_each_test(t)	 for (t = &tests[0]; t->func; t++)
> +static unsigned int ___j;		/* This is obviously not thread-safe */
> +
> +#define for_each_test(t)	 				\
> +	for (___j = 0; ___j < ARRAY_SIZE(tests); ___j++)	\
> +		for (t = &tests[___j][0]; t->func; t++)

why not have ____j on stack and pas it into for_each_test

for_each_test(j, t)                                     
...

jirka

Re: [PATCH 2/3] perf tests: Add arch tests

[Jiri Olsa]

On Sat, Sep 05, 2015 at 08:02:21PM +0100, Matt Fleming wrote:

SNIP

>  
> +static struct test *tests[] = {
> +	generic_tests,
> +	arch_tests,
> +};
> +
>  static bool perf_test__matches(struct test *test, int curr, int argc, const char *argv[])
>  {
>  	int i;
> @@ -237,7 +229,11 @@ static int run_test(struct test *test)
>  	return err;
>  }
>  
> -#define for_each_test(t)	 for (t = &tests[0]; t->func; t++)
> +static unsigned int ___j;		/* This is obviously not thread-safe */
> +
> +#define for_each_test(t)	 				\
> +	for (___j = 0; ___j < ARRAY_SIZE(tests); ___j++)	\
> +		for (t = &tests[___j][0]; t->func; t++)
>  

could you also split the change on adding support for arch_tests
and another actually moving affected tests?

jirka

Re: [PATCH 1/3] perf tests: Introduce iterator function for tests

[Jiri Olsa]

On Sat, Sep 05, 2015 at 08:02:20PM +0100, Matt Fleming wrote:
> From: Matt Fleming <matt.fleming@intel.com>
> 
> In preparation for introducing more arrays of tests, e.g. "arch tests"
> (architecture-specific tests), abstract the code to iterate over the
> list of tests into a helper function.
> 
> This way, code that uses a 'struct test' doesn't need to worry about
> how the tests are grouped together and changes to the list of tests
> doesn't require changes to the code using it.
> 
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> Signed-off-by: Matt Fleming <matt.fleming@intel.com>

Acked-by: Jiri Olsa <jolsa@kernel.org>

thanks,
jirka

Re: [PATCH 0/3] perf tests: Add arch tests

[Jiri Olsa]

On Sat, Sep 05, 2015 at 08:02:19PM +0100, Matt Fleming wrote:
> From: Matt Fleming <matt.fleming@intel.com>
> 
> To encourage developers to contribute arch-specific tests without
> feeling guilty about messing up the generic perf tests, this patch
> series introduces "arch tests" and then adds a test for the Intel CQM
> perf code which was used to trigger a WARN_ON_ONCE(), fixed in commit
> 2c534c0da0a6 ("perf/x86/intel/cqm: Return cached counter value from
> IRQ context").
> 
> The concept of additional lists of tests, like the arch tests, can be
> taken further in the future, for instance Arnaldo has expressed
> interest in adding tests that require root permissions, etc.
> 
> The patch series is based on tip/perf/core.

please base this over Arnaldo's perf/core

jirka

Re: [PATCH 0/3] perf tests: Add arch tests

[Arnaldo Carvalho de Melo]

Em Mon, Sep 07, 2015 at 02:58:18PM +0200, Jiri Olsa escreveu:
> On Sat, Sep 05, 2015 at 08:02:19PM +0100, Matt Fleming wrote:
> > From: Matt Fleming <matt.fleming@intel.com>
> > 
> > To encourage developers to contribute arch-specific tests without
> > feeling guilty about messing up the generic perf tests, this patch
> > series introduces "arch tests" and then adds a test for the Intel CQM
> > perf code which was used to trigger a WARN_ON_ONCE(), fixed in commit
> > 2c534c0da0a6 ("perf/x86/intel/cqm: Return cached counter value from
> > IRQ context").
> > 
> > The concept of additional lists of tests, like the arch tests, can be
> > taken further in the future, for instance Arnaldo has expressed
> > interest in adding tests that require root permissions, etc.
> > 
> > The patch series is based on tip/perf/core.
> 
> please base this over Arnaldo's perf/core

Yeah, that is better, sometimes, like now, the difference from
tip/perf/core to acme/perf/core can grow as big as 50, 60 patches :-)

BTW I applied the first patch, with Jiri's ack, its in my perf/core
branch now.

- Arnaldo

Re: [PATCH 0/3] perf tests: Add arch tests

[Matt Fleming]

On Tue, 08 Sep, at 11:24:16AM, Arnaldo Carvalho de Melo wrote:
> Em Mon, Sep 07, 2015 at 02:58:18PM +0200, Jiri Olsa escreveu:
> > 
> > please base this over Arnaldo's perf/core
> 
> Yeah, that is better, sometimes, like now, the difference from
> tip/perf/core to acme/perf/core can grow as big as 50, 60 patches :-)
 
OK, will do.

> BTW I applied the first patch, with Jiri's ack, its in my perf/core

Thanks!

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 2/3] perf tests: Add arch tests

[Matt Fleming]

On Mon, 07 Sep, at 02:23:53PM, Jiri Olsa wrote:
> On Sat, Sep 05, 2015 at 08:02:21PM +0100, Matt Fleming wrote:
> 
> SNIP
> 
> > diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> > index 8cf0601d1662..a1b2265eaf55 100644
> > --- a/tools/perf/tests/builtin-test.c
> > +++ b/tools/perf/tests/builtin-test.c
> > @@ -14,10 +14,17 @@
> >  #include "parse-options.h"
> >  #include "symbol.h"
> >  
> > -static struct test {
> > -	const char *desc;
> > -	int (*func)(void);
> > -} tests[] = {
> > +#if defined(__x86_64__) || defined(__i386__)
> > +#include "arch-tests.h"
> > +#else
> > +static struct test arch_tests[] = {
> > +	{
> > +		.func = NULL,
> > +	},
> > +};
> > +#endif
> 
> this could be defined as __weak array so we dont need to have #if above

Yeah, good idea. I'll make that change.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 2/3] perf tests: Add arch tests

[Matt Fleming]

On Mon, 07 Sep, at 02:28:14PM, Jiri Olsa wrote:
> On Sat, Sep 05, 2015 at 08:02:21PM +0100, Matt Fleming wrote:
> 
> SNIP
> 
> >  };
> >  
> > +static struct test *tests[] = {
> > +	generic_tests,
> > +	arch_tests,
> > +};
> > +
> >  static bool perf_test__matches(struct test *test, int curr, int argc, const char *argv[])
> >  {
> >  	int i;
> > @@ -237,7 +229,11 @@ static int run_test(struct test *test)
> >  	return err;
> >  }
> >  
> > -#define for_each_test(t)	 for (t = &tests[0]; t->func; t++)
> > +static unsigned int ___j;		/* This is obviously not thread-safe */
> > +
> > +#define for_each_test(t)	 				\
> > +	for (___j = 0; ___j < ARRAY_SIZE(tests); ___j++)	\
> > +		for (t = &tests[___j][0]; t->func; t++)
> 
> why not have ____j on stack and pas it into for_each_test
> 
> for_each_test(j, t)                                     
> ...

Right, I made a conscious decision to not do that because I didn't
want the caller to have to care about providing an iterator variable.
It also makes the diff slightly bigger.

But I don't feel that strongly about it, so I'll make this change.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: [PATCH 2/3] perf tests: Add arch tests

[Matt Fleming]

On Mon, 07 Sep, at 02:29:54PM, Jiri Olsa wrote:
> On Sat, Sep 05, 2015 at 08:02:21PM +0100, Matt Fleming wrote:
> 
> SNIP
> 
> >  
> > +static struct test *tests[] = {
> > +	generic_tests,
> > +	arch_tests,
> > +};
> > +
> >  static bool perf_test__matches(struct test *test, int curr, int argc, const char *argv[])
> >  {
> >  	int i;
> > @@ -237,7 +229,11 @@ static int run_test(struct test *test)
> >  	return err;
> >  }
> >  
> > -#define for_each_test(t)	 for (t = &tests[0]; t->func; t++)
> > +static unsigned int ___j;		/* This is obviously not thread-safe */
> > +
> > +#define for_each_test(t)	 				\
> > +	for (___j = 0; ___j < ARRAY_SIZE(tests); ___j++)	\
> > +		for (t = &tests[___j][0]; t->func; t++)
> >  
> 
> could you also split the change on adding support for arch_tests
> and another actually moving affected tests?

Sure that makes sense.

-- 
Matt Fleming, Intel Open Source Technology Center

[tip:perf/core] perf tests: Introduce iterator function for tests

[tip-bot for Matt Fleming]

Commit-ID:  e8210cefb7e1ec0760a6fe581ad0727a2dcf8dd1
Gitweb:     http://git.kernel.org/tip/e8210cefb7e1ec0760a6fe581ad0727a2dcf8dd1
Author:     Matt Fleming <matt.fleming@intel.com>
AuthorDate: Sat, 5 Sep 2015 20:02:20 +0100
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 14 Sep 2015 12:50:18 -0300

perf tests: Introduce iterator function for tests

In preparation for introducing more arrays of tests, e.g. "arch tests"
(architecture-specific tests), abstract the code to iterate over the
list of tests into a helper function.

This way, code that uses a 'struct test' doesn't need to worry about how
the tests are grouped together and changes to the list of tests doesn't
require changes to the code using it.

Signed-off-by: Matt Fleming <matt.fleming@intel.com>
Acked-by: Jiri Olsa <jolsa@kernel.org>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Kanaka Juvva <kanaka.d.juvva@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Vikas Shivappa <vikas.shivappa@intel.com>
Cc: Vince Weaver <vince@deater.net>
Link: http://lkml.kernel.org/r/1441479742-15402-2-git-send-email-matt@codeblueprint.co.uk
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/tests/builtin-test.c | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 98b0b24..d9bf51d 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -195,7 +195,7 @@ static struct test {
 	},
 };
 
-static bool perf_test__matches(int curr, int argc, const char *argv[])
+static bool perf_test__matches(struct test *test, int curr, int argc, const char *argv[])
 {
 	int i;
 
@@ -212,7 +212,7 @@ static bool perf_test__matches(int curr, int argc, const char *argv[])
 			continue;
 		}
 
-		if (strstr(tests[curr].desc, argv[i]))
+		if (strstr(test->desc, argv[i]))
 			return true;
 	}
 
@@ -249,27 +249,28 @@ static int run_test(struct test *test)
 	return err;
 }
 
+#define for_each_test(t)	 for (t = &tests[0]; t->func; t++)
+
 static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 {
+	struct test *t;
 	int i = 0;
 	int width = 0;
 
-	while (tests[i].func) {
-		int len = strlen(tests[i].desc);
+	for_each_test(t) {
+		int len = strlen(t->desc);
 
 		if (width < len)
 			width = len;
-		++i;
 	}
 
-	i = 0;
-	while (tests[i].func) {
+	for_each_test(t) {
 		int curr = i++, err;
 
-		if (!perf_test__matches(curr, argc, argv))
+		if (!perf_test__matches(t, curr, argc, argv))
 			continue;
 
-		pr_info("%2d: %-*s:", i, width, tests[curr].desc);
+		pr_info("%2d: %-*s:", i, width, t->desc);
 
 		if (intlist__find(skiplist, i)) {
 			color_fprintf(stderr, PERF_COLOR_YELLOW, " Skip (user override)\n");
@@ -277,8 +278,8 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 		}
 
 		pr_debug("\n--- start ---\n");
-		err = run_test(&tests[curr]);
-		pr_debug("---- end ----\n%s:", tests[curr].desc);
+		err = run_test(t);
+		pr_debug("---- end ----\n%s:", t->desc);
 
 		switch (err) {
 		case TEST_OK:
@@ -299,15 +300,14 @@ static int __cmd_test(int argc, const char *argv[], struct intlist *skiplist)
 
 static int perf_test__list(int argc, const char **argv)
 {
+	struct test *t;
 	int i = 0;
 
-	while (tests[i].func) {
-		int curr = i++;
-
-		if (argc > 1 && !strstr(tests[curr].desc, argv[1]))
+	for_each_test(t) {
+		if (argc > 1 && !strstr(t->desc, argv[1]))
 			continue;
 
-		pr_info("%2d: %s\n", i, tests[curr].desc);
+		pr_info("%2d: %s\n", ++i, t->desc);
 	}
 
 	return 0;