[RFC 0/3] perf tests: Adding precise event attribute test

[RFC 0/3] perf tests: Adding precise event attribute test

[Jiri Olsa]

hi,
as per issues with precise attribute [1], adding simple test to
catch it next time. 

Patch 2 adds sysfs 'precise' attribute containing maximum value
for perf_event_attr::precise_ip.

Patch 3 adds simple test checking the attribute and creating cycle
events with all possible precise values.

I based this on revert (Patch 1) discussed in [1]. I'm not sure
this revert will tak place at the end, but I wanted to get some
feedback on the 'precise' attribute first.

Attached patches:
  1/3 Revert "perf: Require exclude_guest to use PEBS - kernel side enforcement"
  2/3 perf x86: Add precise sysfs attribute
  3/3 perf tests: Add precise event attribute test

thanks,
jirka

[1] http://marc.info/?l=linux-kernel&m=135536724805356&w=2

Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 arch/x86/kernel/cpu/perf_event.c | 38 ++++++++++++++++++++++++--------------
 tools/perf/Makefile              |  1 +
 tools/perf/tests/builtin-test.c  |  4 ++++
 tools/perf/tests/precise.c       | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h         |  1 +
 5 files changed, 109 insertions(+), 14 deletions(-)

[PATCH 1/3] Revert "perf: Require exclude_guest to use PEBS - kernel side enforcement"

[Jiri Olsa]

This reverts commit 20b279ddb38ca42f8863cec07b4d45ec24589f13.

Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 arch/x86/kernel/cpu/perf_event.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 4428fd1..6774c17 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -340,9 +340,6 @@ int x86_setup_perfctr(struct perf_event *event)
 		/* BTS is currently only allowed for user-mode. */
 		if (!attr->exclude_kernel)
 			return -EOPNOTSUPP;
-
-		if (!attr->exclude_guest)
-			return -EOPNOTSUPP;
 	}
 
 	hwc->config |= config;
@@ -385,9 +382,6 @@ int x86_pmu_hw_config(struct perf_event *event)
 	if (event->attr.precise_ip) {
 		int precise = 0;
 
-		if (!event->attr.exclude_guest)
-			return -EOPNOTSUPP;
-
 		/* Support for constant skid */
 		if (x86_pmu.pebs_active && !x86_pmu.pebs_broken) {
 			precise++;
-- 
1.7.11.7

Re: [PATCH 1/3] Revert "perf: Require exclude_guest to use PEBS - kernel side enforcement"

[David Ahern]

On 12/19/12 8:07 AM, Jiri Olsa wrote:
> This reverts commit 20b279ddb38ca42f8863cec07b4d45ec24589f13.
>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>

As a courtesy please CC people involved in a commit on any revert request.

Arnaldo: please do not take this patch; I have proposed an alternative. 
Just waiting to hear back on its acceptability.

David

Re: [PATCH 1/3] Revert "perf: Require exclude_guest to use PEBS - kernel side enforcement"

[Jiri Olsa]

On Sat, Dec 22, 2012 at 12:21:23PM -0700, David Ahern wrote:
> On 12/19/12 8:07 AM, Jiri Olsa wrote:
> >This reverts commit 20b279ddb38ca42f8863cec07b4d45ec24589f13.
> >
> >Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> >Cc: Frederic Weisbecker <fweisbec@gmail.com>
> >Cc: Ingo Molnar <mingo@elte.hu>
> >Cc: Namhyung Kim <namhyung@kernel.org>
> >Cc: Paul Mackerras <paulus@samba.org>
> >Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> >Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> As a courtesy please CC people involved in a commit on any revert request.
> 
> Arnaldo: please do not take this patch; I have proposed an
> alternative. Just waiting to hear back on its acceptability.
> 
> David

this one was not meant to be applied.. just wanted the precise
to work, so I could test other stuff.. I did mention that in:

  [RFC 0/3] perf tests: Adding precise event attribute test
  ---
  I based this on revert (Patch 1) discussed in [1]. I'm not sure
  this revert will tak place at the end, but I wanted to get some
  feedback on the 'precise' attribute first.
  ...
  [1] http://marc.info/?l=linux-kernel&m=135536724805356&w=2
  ---

jirka

[PATCH 2/3] perf x86: Add precise sysfs attribute

[Jiri Olsa]

Adding sysfs precise attribute for cpu device (/sys/devices/cpu)
to show the maximum value for perf event precise attribute.

This will be initialy used for automated precise event perf test
and could be helpful otherwise. Arnaldo suggested to use this as
a guide for default adding of precise attribute for HW events.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 arch/x86/kernel/cpu/perf_event.c | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/arch/x86/kernel/cpu/perf_event.c b/arch/x86/kernel/cpu/perf_event.c
index 6774c17..190995e 100644
--- a/arch/x86/kernel/cpu/perf_event.c
+++ b/arch/x86/kernel/cpu/perf_event.c
@@ -377,19 +377,26 @@ static inline int precise_br_compat(struct perf_event *event)
 	return m == b;
 }
 
-int x86_pmu_hw_config(struct perf_event *event)
+static int x86_get_precise(void)
 {
-	if (event->attr.precise_ip) {
-		int precise = 0;
+	int precise = 0;
 
-		/* Support for constant skid */
-		if (x86_pmu.pebs_active && !x86_pmu.pebs_broken) {
+	/* Support for constant skid */
+	if (x86_pmu.pebs_active && !x86_pmu.pebs_broken) {
+		precise++;
+
+		/* Support for IP fixup */
+		if (x86_pmu.lbr_nr)
 			precise++;
+	}
 
-			/* Support for IP fixup */
-			if (x86_pmu.lbr_nr)
-				precise++;
-		}
+	return precise;
+}
+
+int x86_pmu_hw_config(struct perf_event *event)
+{
+	if (event->attr.precise_ip) {
+		int precise = x86_get_precise();
 
 		if (event->attr.precise_ip > precise)
 			return -EOPNOTSUPP;
@@ -1700,6 +1707,13 @@ static int x86_pmu_event_init(struct perf_event *event)
 	return err;
 }
 
+static ssize_t get_attr_precise(struct device *cdev,
+				struct device_attribute *attr,
+				char *buf)
+{
+	return snprintf(buf, 10, "%d\n", x86_get_precise());
+}
+
 static int x86_pmu_event_idx(struct perf_event *event)
 {
 	int idx = event->hw.idx;
@@ -1752,9 +1766,11 @@ static ssize_t set_attr_rdpmc(struct device *cdev,
 }
 
 static DEVICE_ATTR(rdpmc, S_IRUSR | S_IWUSR, get_attr_rdpmc, set_attr_rdpmc);
+static DEVICE_ATTR(precise, S_IRUGO, get_attr_precise, NULL);
 
 static struct attribute *x86_pmu_attrs[] = {
 	&dev_attr_rdpmc.attr,
+	&dev_attr_precise.attr,
 	NULL,
 };
 
-- 
1.7.11.7

Re: [PATCH 2/3] perf x86: Add precise sysfs attribute

[Greg KH]

On Wed, Dec 19, 2012 at 04:07:21PM +0100, Jiri Olsa wrote:
> Adding sysfs precise attribute for cpu device (/sys/devices/cpu)
> to show the maximum value for perf event precise attribute.
> 
> This will be initialy used for automated precise event perf test
> and could be helpful otherwise. Arnaldo suggested to use this as
> a guide for default adding of precise attribute for HW events.
> 
> Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  arch/x86/kernel/cpu/perf_event.c | 34 +++++++++++++++++++++++++---------
>  1 file changed, 25 insertions(+), 9 deletions(-)

As you are adding a new sysfs file, you must also document it with an
entry in Documentation/ABI/

thanks,

greg k-h

Re: [PATCH 2/3] perf x86: Add precise sysfs attribute

[Jiri Olsa]

On Wed, Dec 19, 2012 at 09:34:30PM -0800, Greg KH wrote:
> On Wed, Dec 19, 2012 at 04:07:21PM +0100, Jiri Olsa wrote:
> > Adding sysfs precise attribute for cpu device (/sys/devices/cpu)
> > to show the maximum value for perf event precise attribute.
> > 
> > This will be initialy used for automated precise event perf test
> > and could be helpful otherwise. Arnaldo suggested to use this as
> > a guide for default adding of precise attribute for HW events.
> > 
> > Signed-off-by: Jiri Olsa <jolsa@redhat.com>
> > Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
> > Cc: Frederic Weisbecker <fweisbec@gmail.com>
> > Cc: Ingo Molnar <mingo@elte.hu>
> > Cc: Namhyung Kim <namhyung@kernel.org>
> > Cc: Paul Mackerras <paulus@samba.org>
> > Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
> > ---
> >  arch/x86/kernel/cpu/perf_event.c | 34 +++++++++++++++++++++++++---------
> >  1 file changed, 25 insertions(+), 9 deletions(-)
> 
> As you are adding a new sysfs file, you must also document it with an
> entry in Documentation/ABI/

will do, thanks
jirka

Re: [PATCH 2/3] perf x86: Add precise sysfs attribute

[Namhyung Kim]

On Wed, Dec 19, 2012 at 04:07:21PM +0100, Jiri Olsa wrote:
> Adding sysfs precise attribute for cpu device (/sys/devices/cpu)
> to show the maximum value for perf event precise attribute.
> 
> This will be initialy used for automated precise event perf test
> and could be helpful otherwise. Arnaldo suggested to use this as
> a guide for default adding of precise attribute for HW events.

Although I'm not aware of which architectures support precise feature,
wouldn't it better putting this on architecture independent rather
than just for x86 with fallback routine returning 0?

Thanks,
Namhyung

Re: [PATCH 2/3] perf x86: Add precise sysfs attribute

[Jiri Olsa]

On Thu, Dec 20, 2012 at 05:39:25PM +0900, Namhyung Kim wrote:
> On Wed, Dec 19, 2012 at 04:07:21PM +0100, Jiri Olsa wrote:
> > Adding sysfs precise attribute for cpu device (/sys/devices/cpu)
> > to show the maximum value for perf event precise attribute.
> > 
> > This will be initialy used for automated precise event perf test
> > and could be helpful otherwise. Arnaldo suggested to use this as
> > a guide for default adding of precise attribute for HW events.
> 
> Although I'm not aware of which architectures support precise feature,
> wouldn't it better putting this on architecture independent rather
> than just for x86 with fallback routine returning 0?

same here.. :) no idea if there's same concept in other archs

Anyway, I'd say it's not big change so once other archs get
interested, it's easy to make it global one.

Same like for event aliases moving to ppc now:
http://marc.info/?l=linux-kernel&m=135590210618224&w=2

thanks,
jirka

[PATCH 3/3] perf tests: Add precise event attribute test

[Jiri Olsa]

From: "jolsa@redhat.com" <jolsa@redhat.com>

The test detects the precise attribute availability and try
to open perf event with each allowed precise attribute value.

Signed-off-by: Jiri Olsa <jolsa@redhat.com>
Cc: Corey Ashford <cjashfor@linux.vnet.ibm.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile             |  1 +
 tools/perf/tests/builtin-test.c |  4 +++
 tools/perf/tests/precise.c      | 79 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/tests.h        |  1 +
 4 files changed, 85 insertions(+)
 create mode 100644 tools/perf/tests/precise.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index dbf1c35..03aae70 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -489,6 +489,7 @@ LIB_OBJS += $(OUTPUT)tests/evsel-tp-sched.o
 LIB_OBJS += $(OUTPUT)tests/pmu.o
 LIB_OBJS += $(OUTPUT)tests/hists_link.o
 LIB_OBJS += $(OUTPUT)tests/python-use.o
+LIB_OBJS += $(OUTPUT)tests/precise.o
 
 BUILTIN_OBJS += $(OUTPUT)builtin-annotate.o
 BUILTIN_OBJS += $(OUTPUT)builtin-bench.o
diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 6a5dee2..9903af2 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -77,6 +77,10 @@ static struct test {
 		.func = test__python_use,
 	},
 	{
+		.desc = "Test precise event attribute",
+		.func = test__precise,
+	},
+	{
 		.func = NULL,
 	},
 };
diff --git a/tools/perf/tests/precise.c b/tools/perf/tests/precise.c
new file mode 100644
index 0000000..e3eedab
--- /dev/null
+++ b/tools/perf/tests/precise.c
@@ -0,0 +1,79 @@
+#include <linux/kernel.h>
+#include <linux/perf_event.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include "perf.h"
+#include "tests.h"
+#include "util.h"
+#include "sysfs.h"
+
+static int get_precise(void)
+{
+	struct stat st;
+	char path[PATH_MAX];
+	int precise = 0;
+
+	scnprintf(path, PATH_MAX, "%s/devices/cpu/precise",
+		  sysfs_find_mountpoint());
+
+	if (!lstat(path, &st)) {
+		FILE *file;
+
+		file = fopen(path, "r");
+		if (!file)
+			return -1;
+
+		if (1 != fscanf(file, "%d", &precise)) {
+			pr_debug("failed to read precise info\n");
+			precise = -1;
+		}
+
+		fclose(file);
+	}
+
+	return precise;
+}
+
+static int event_open_precise(int precise)
+{
+	struct perf_event_attr attr = {
+		.type		= PERF_TYPE_HARDWARE,
+		.config		= PERF_COUNT_HW_CPU_CYCLES,
+		.precise_ip	= precise,
+	};
+	int fd;
+
+	pr_debug("open cycles event with precise %d\n", precise);
+
+	fd = sys_perf_event_open(&attr, 0, -1, -1, 0);
+	if (fd < 0) {
+		pr_debug("failed to open event, syscall returned "
+			 "with %d (%s)\n", fd, strerror(errno));
+		return -1;
+	}
+
+	close(fd);
+	return 0;
+
+}
+
+int test__precise(void)
+{
+	int precise = get_precise();
+	int i;
+
+	if (!precise) {
+		pr_debug("no precise info or support\n");
+		return TEST_SKIP;
+	}
+
+	if (precise < 0)
+		return TEST_FAIL;
+
+	for (i = 1; i <= precise; i++)
+		if (event_open_precise(i))
+			return TEST_FAIL;
+
+	return TEST_OK;
+}
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index 5de0be1..ff6db12 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -23,5 +23,6 @@ int test__dso_data(void);
 int test__parse_events(void);
 int test__hists_link(void);
 int test__python_use(void);
+int test__precise(void);
 
 #endif /* TESTS_H */
-- 
1.7.11.7