[RFC 0/2] report perf sampling failing due to PMU lacking overflow intr support

[RFC 0/2] report perf sampling failing due to PMU lacking overflow intr support

[Vineet Gupta]

Hi,

A couple of minor patches to print better PMU lacking overflow intr support
such as in ARC700 cores.

This is an ABI change so might be controversial hence RFC.

Despite triviality, I've seperated the change into 2 pacthes - for different maintainers
and also kept userspace change first to keep it bisectable.

Comments welcome !

Thx,
-Vineet

Vineet Gupta (2):
  tools/perf: Handle EOPNOTSUPP for sampling events
  perf/core: change errno for sampling event not supported in hardware

 kernel/events/core.c    | 2 +-
 tools/perf/util/evsel.c | 3 +++
 2 files changed, 4 insertions(+), 1 deletion(-)

-- 
2.5.0

[RFC 1/2] tools/perf: Handle EOPNOTSUPP for sampling events

[Vineet Gupta]

This allows (with next change to perf core) for calling out in userspace
the exact reason for perf record failing when PMU doesn't support
overflow interrupts

Note that this needs to be put ahead of existing precise_ip check as
that gets hit otherwise for the sampling fail case as well.

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 tools/perf/util/evsel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 738ce226002b..8bd220c63e19 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2403,6 +2403,9 @@ int perf_evsel__open_strerror(struct perf_evsel *evsel, struct target *target,
 	 "No such device - did you specify an out-of-range profile CPU?\n");
 		break;
 	case EOPNOTSUPP:
+		if (evsel->attr.sample_period != 0)
+			return scnprintf(msg, size, "%s",
+	"PMU Hardware doesn't support sampling/overflow-interrupts.");
 		if (evsel->attr.precise_ip)
 			return scnprintf(msg, size, "%s",
 	"\'precise\' request may not be supported. Try removing 'p' modifier.");
-- 
2.5.0

[tip:perf/core] tools/perf: Handle -EOPNOTSUPP for sampling events

[tip-bot for Vineet Gupta]

Commit-ID:  dc89e75a9412db5b1105a140182ec1e35a8351b4
Gitweb:     http://git.kernel.org/tip/dc89e75a9412db5b1105a140182ec1e35a8351b4
Author:     Vineet Gupta <Vineet.Gupta1@synopsys.com>
AuthorDate: Mon, 9 May 2016 15:07:39 +0530
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 3 Jun 2016 09:41:11 +0200

tools/perf: Handle -EOPNOTSUPP for sampling events

This allows (with a previous change to the perf error return ABI) for
calling out in userspace the exact reason for perf record failing
when PMU doesn't support overflow interrupts.

Note that this needs to be put ahead of existing precise_ip check as
that gets hit otherwise for the sampling fail case as well.

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: <acme@redhat.com>
Cc: <linux-snps-arc@lists.infradead.org>
Cc: <vincent.weaver@maine.edu>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Link: http://lkml.kernel.org/r/1462786660-2900-2-git-send-email-vgupta@synopsys.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 tools/perf/util/evsel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 245ac50..8d30cbd 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2384,6 +2384,9 @@ int perf_evsel__open_strerror(struct perf_evsel *evsel, struct target *target,
 	 "No such device - did you specify an out-of-range profile CPU?");
 		break;
 	case EOPNOTSUPP:
+		if (evsel->attr.sample_period != 0)
+			return scnprintf(msg, size, "%s",
+	"PMU Hardware doesn't support sampling/overflow-interrupts.");
 		if (evsel->attr.precise_ip)
 			return scnprintf(msg, size, "%s",
 	"\'precise\' request may not be supported. Try removing 'p' modifier.");

[RFC 2/2] perf/core: change errno for sampling event not supported in hardware

[Vineet Gupta]

This allows userspace to identify this case specifically from the
catch all error msg it prints currently.

This is an ABI change

Before
-------
| # perf record ls
| Error:
| The sys_perf_event_open() syscall returned with 524 (Unknown error 524)
| for event (cycles:ppp).
| /bin/dmesg may provide additional information.
| No CONFIG_PERF_EVENTS=y kernel support configured?

Now
-------
| # perf record ls
| Error:
| PMU Hardware doesn't support sampling/overflow-interrupts.

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 kernel/events/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 4e2ebf6f2f1f..41c5c7122987 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8435,7 +8435,7 @@ SYSCALL_DEFINE5(perf_event_open,
 
 	if (is_sampling_event(event)) {
 		if (event->pmu->capabilities & PERF_PMU_CAP_NO_INTERRUPT) {
-			err = -ENOTSUPP;
+			err = -EOPNOTSUPP;
 			goto err_alloc;
 		}
 	}
-- 
2.5.0

Re: [RFC 2/2] perf/core: change errno for sampling event not supported in hardware

[Vince Weaver]

On Mon, 9 May 2016, Vineet Gupta wrote:

> This allows userspace to identify this case specifically from the
> catch all error msg it prints currently.
> 
> This is an ABI change

An ABI change which will probably break things.

The original change from ENODEV to ENOTSUPP managed to break things 
although it took four kernel releases before anyone noticed.

The usage of ENOTSUPP was my fault, though I feel like at the time I was 
told that ENOTSUPP is for internal kernel usage and would be converted to 
EOPNOTSUPP when returning an error to userspace.  But now I 
can't find any sort of reference for that at all, except the fact that

	/usr/include/x86_64-linux-gnu/bits/errno.h

has
	/* Linux has no ENOTSUP error code.  */
	# define ENOTSUP EOPNOTSUPP

in it... but wait, that's ENOTSUP not ENOTSUPP.  Blargh.

Vince

Re: [RFC 2/2] perf/core: change errno for sampling event not supported in hardware

[Vineet Gupta]

On Monday 09 May 2016 07:24 PM, Vince Weaver wrote:
> On Mon, 9 May 2016, Vineet Gupta wrote:
> 
>> This allows userspace to identify this case specifically from the
>> catch all error msg it prints currently.
>>
>> This is an ABI change
> 
> An ABI change which will probably break things.


Right thats what I feared. But hold on, I don't think we need to change the ABI to
achieve what we want. Gosh why did I even take that path.

Currently the errno switch case in perf_evsel__open_strerror() in doesn't handle
ENOTSUPP. So how about we add that - augmented with the same sample_period !0
check to barf for lack of sampling support.

Do you see anything wrong with that ?

-Vineet

> 
> The original change from ENODEV to ENOTSUPP managed to break things 
> although it took four kernel releases before anyone noticed.
> 
> The usage of ENOTSUPP was my fault, though I feel like at the time I was 
> told that ENOTSUPP is for internal kernel usage and would be converted to 
> EOPNOTSUPP when returning an error to userspace.  But now I 
> can't find any sort of reference for that at all, except the fact that
> 
> 	/usr/include/x86_64-linux-gnu/bits/errno.h
> 
> has
> 	/* Linux has no ENOTSUP error code.  */
> 	# define ENOTSUP EOPNOTSUPP
> 
> in it... but wait, that's ENOTSUP not ENOTSUPP.  Blargh.
> 
> Vince
> 

Re: [RFC 2/2] perf/core: change errno for sampling event not supported in hardware

[Vince Weaver]

On Mon, 9 May 2016, Vineet Gupta wrote:

> On Monday 09 May 2016 07:24 PM, Vince Weaver wrote:
> > On Mon, 9 May 2016, Vineet Gupta wrote:
> > 
> >> This allows userspace to identify this case specifically from the
> >> catch all error msg it prints currently.
> >>
> >> This is an ABI change
> > 
> > An ABI change which will probably break things.
> 
> 
> Right thats what I feared. But hold on, I don't think we need to change the ABI to
> achieve what we want. Gosh why did I even take that path.
> 
> Currently the errno switch case in perf_evsel__open_strerror() in doesn't handle
> ENOTSUPP. So how about we add that - augmented with the same sample_period !0
> check to barf for lack of sampling support.
> 
> Do you see anything wrong with that ?

no, but it would be nice if one of the actual maintainers would chime in 
with an opinion.

In any case if ENOTSUPP is being returned to userspace I should update the 
perf_event manpage to reflect that.

Vince

Re: [RFC 2/2] perf/core: change errno for sampling event not supported in hardware

[Peter Zijlstra]

On Mon, May 09, 2016 at 10:53:43PM +0530, Vineet Gupta wrote:

> Right thats what I feared. But hold on, I don't think we need to change the ABI to
> achieve what we want. Gosh why did I even take that path.
> 
> Currently the errno switch case in perf_evsel__open_strerror() in doesn't handle
> ENOTSUPP. So how about we add that - augmented with the same sample_period !0
> check to barf for lack of sampling support.
> 
> Do you see anything wrong with that ?

Should work I think.

Re: [RFC 2/2] perf/core: change errno for sampling event not supported in hardware

[Vineet Gupta]

On Thursday 12 May 2016 01:06 AM, Peter Zijlstra wrote:
> On Mon, May 09, 2016 at 10:53:43PM +0530, Vineet Gupta wrote:
> 
>> > Right thats what I feared. But hold on, I don't think we need to change the ABI to
>> > achieve what we want. Gosh why did I even take that path.
>> > 
>> > Currently the errno switch case in perf_evsel__open_strerror() in doesn't handle
>> > ENOTSUPP. So how about we add that - augmented with the same sample_period !0
>> > check to barf for lack of sampling support.
>> > 
>> > Do you see anything wrong with that ?
>
> Should work I think.

Tried that and doesn't even compile. Reconfirms what Vince said, ENOTSUPP is not
exposed to userspace (being in include/linux and not include/uapi/linux)

Re: [RFC 2/2] perf/core: change errno for sampling event not supported in hardware

[Peter Zijlstra]

On Thu, May 12, 2016 at 11:58:43AM +0530, Vineet Gupta wrote:
> On Thursday 12 May 2016 01:06 AM, Peter Zijlstra wrote:
> > On Mon, May 09, 2016 at 10:53:43PM +0530, Vineet Gupta wrote:
> > 
> >> > Right thats what I feared. But hold on, I don't think we need to change the ABI to
> >> > achieve what we want. Gosh why did I even take that path.
> >> > 
> >> > Currently the errno switch case in perf_evsel__open_strerror() in doesn't handle
> >> > ENOTSUPP. So how about we add that - augmented with the same sample_period !0
> >> > check to barf for lack of sampling support.
> >> > 
> >> > Do you see anything wrong with that ?
> >
> > Should work I think.
> 
> Tried that and doesn't even compile. Reconfirms what Vince said, ENOTSUPP is not
> exposed to userspace (being in include/linux and not include/uapi/linux)

Durr, so what does userspace see?

Re: [RFC 2/2] perf/core: change errno for sampling event not supported in hardware

[Vineet Gupta]

On Thursday 12 May 2016 12:12 PM, Peter Zijlstra wrote:
>> Tried that and doesn't even compile. Reconfirms what Vince said, ENOTSUPP is not
>> > exposed to userspace (being in include/linux and not include/uapi/linux)
> Durr, so what does userspace see?

It sees the "value" of ENOTSUPP, i.e. 524 but there is no symbolic reference to it :-)

Re: [RFC 2/2] perf/core: change errno for sampling event not supported in hardware

[Peter Zijlstra]

On Thu, May 12, 2016 at 12:24:25PM +0530, Vineet Gupta wrote:
> On Thursday 12 May 2016 12:12 PM, Peter Zijlstra wrote:
> >> Tried that and doesn't even compile. Reconfirms what Vince said, ENOTSUPP is not
> >> > exposed to userspace (being in include/linux and not include/uapi/linux)
> > Durr, so what does userspace see?
> 
> It sees the "value" of ENOTSUPP, i.e. 524 but there is no symbolic reference to it :-)

Ah.. which might be a hint that nobody is actually explicitly testing
for this and we might just get away with changing the ABI.

Vince, what say you; shall we try and get away with it? ;-)

Re: [RFC 2/2] perf/core: change errno for sampling event not supported in hardware

[Vince Weaver]

On Thu, 12 May 2016, Peter Zijlstra wrote:

> Ah.. which might be a hint that nobody is actually explicitly testing
> for this and we might just get away with changing the ABI.
> 
> Vince, what say you; shall we try and get away with it? ;-)

It's probably worth trying.

The only other time anyone else noticed was in this thread
	https://lkml.org/lkml/2015/6/11/136
and I think the conclusion at the end of that was also that it's probably 
worth changing.

Vince

Re: [RFC 2/2] perf/core: change errno for sampling event not supported in hardware

[Vineet Gupta]

On Friday 13 May 2016 04:34 AM, Vince Weaver wrote:
> On Thu, 12 May 2016, Peter Zijlstra wrote:
> 
>> Ah.. which might be a hint that nobody is actually explicitly testing
>> for this and we might just get away with changing the ABI.
>>
>> Vince, what say you; shall we try and get away with it? ;-)
> 
> It's probably worth trying.
> 
> The only other time anyone else noticed was in this thread
> 	https://lkml.org/lkml/2015/6/11/136
> and I think the conclusion at the end of that was also that it's probably 
> worth changing.

I presume that the RFC patches I posted are fine or do we want any changes.

[PATCH-REBASED 0/2] report perf sampling failing due to PMU lacking overflow intr support

[Vineet Gupta]

Hi,

This is a repost of RFC [1], rebased on 4.7-rc1, to print pretty PMU lacking
overflow interrupt support if user tries perf sampling.

At the time of RFC review, both PeterZ and Vince seemed convinced to go ahead
with this patch despite the ABI change [2].

Thx,
-Vineet


[1] http://lists.infradead.org/pipermail/linux-snps-arc/2016-May/001014.html
[2] http://lists.infradead.org/pipermail/linux-snps-arc/2016-May/001032.html

Vineet Gupta (2):
  tools/perf: Handle EOPNOTSUPP for sampling events
  perf/core: change errno for sampling event not supported in hardware

 kernel/events/core.c    | 2 +-
 tools/perf/util/evsel.c | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

-- 
2.5.0

[PATCH-REBASED 1/2] tools/perf: Handle EOPNOTSUPP for sampling events

[Vineet Gupta]

This allows (with next change to perf core) for calling out in userspace
the exact reason for perf record failing when PMU doesn't support
overflow interrupts

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 tools/perf/util/evsel.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 5d7037ef7d3b..1578e67c7357 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -2372,6 +2372,13 @@ int perf_evsel__open_strerror(struct perf_evsel *evsel, struct target *target,
 	 "No such device - did you specify an out-of-range profile CPU?");
 		break;
 	case EOPNOTSUPP:
+		/*
+		 * Needs to be tested ahead of precise_ip check as
+		 * that seems to be true for this case as well
+		 */
+		if (evsel->attr.sample_period != 0)
+			return scnprintf(msg, size, "%s",
+	"PMU Hardware doesn't support sampling/overflow-interrupts.");
 		if (evsel->attr.precise_ip)
 			return scnprintf(msg, size, "%s",
 	"\'precise\' request may not be supported. Try removing 'p' modifier.");
-- 
2.5.0

[PATCH-REBASED 2/2] perf/core: change errno for sampling event not supported in hardware

[Vineet Gupta]

This allows userspace to identify this case specifically from the
catch all error msg it prints currently.

This is an ABI change

Before
-------
| # perf record ls
| Error:
| The sys_perf_event_open() syscall returned with 524 (Unknown error 524)
| for event (cycles:ppp).
| /bin/dmesg may provide additional information.
| No CONFIG_PERF_EVENTS=y kernel support configured?

Now
-------
| # perf record ls
| Error:
| PMU Hardware doesn't support sampling/overflow-interrupts.

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
---
 kernel/events/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 274450efea90..a156ec0cb420 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -9271,7 +9271,7 @@ SYSCALL_DEFINE5(perf_event_open,
 
 	if (is_sampling_event(event)) {
 		if (event->pmu->capabilities & PERF_PMU_CAP_NO_INTERRUPT) {
-			err = -ENOTSUPP;
+			err = -EOPNOTSUPP;
 			goto err_alloc;
 		}
 	}
-- 
2.5.0

[tip:perf/core] perf/abi: Change the errno for sampling event not supported in hardware

[tip-bot for Vineet Gupta]

Commit-ID:  a1396555abff9ff9b74c2e4da13e27e81fd094b2
Gitweb:     http://git.kernel.org/tip/a1396555abff9ff9b74c2e4da13e27e81fd094b2
Author:     Vineet Gupta <Vineet.Gupta1@synopsys.com>
AuthorDate: Mon, 9 May 2016 15:07:40 +0530
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 3 Jun 2016 09:40:42 +0200

perf/abi: Change the errno for sampling event not supported in hardware

Change the return code for sampling event not supported from -ENOTSUPP
to -EOPNOTSUPP.

This allows userspace to identify this case specifically, instead of
printing the catch-all error message it did previously.

Technically this is an ABI change, but we think we can get away
with it.

Old behavior:
 -------
 | # perf record ls
 | Error:
 | The sys_perf_event_open() syscall returned with 524 (Unknown error 524)
 | for event (cycles:ppp).
 | /bin/dmesg may provide additional information.
 | No CONFIG_PERF_EVENTS=y kernel support configured?

New behavior:
 -------
 | # perf record ls
 | Error:
 | PMU Hardware doesn't support sampling/overflow-interrupts.

Signed-off-by: Vineet Gupta <vgupta@synopsys.com>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: <acme@redhat.com>
Cc: <linux-snps-arc@lists.infradead.org>
Cc: <vincent.weaver@maine.edu>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Vince Weaver <vincent.weaver@maine.edu>
Cc: Vineet Gupta <Vineet.Gupta1@synopsys.com>
Link: http://lkml.kernel.org/r/1462786660-2900-3-git-send-email-vgupta@synopsys.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 kernel/events/core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index f94f164..5d48306 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -9309,7 +9309,7 @@ SYSCALL_DEFINE5(perf_event_open,
 
 	if (is_sampling_event(event)) {
 		if (event->pmu->capabilities & PERF_PMU_CAP_NO_INTERRUPT) {
-			err = -ENOTSUPP;
+			err = -EOPNOTSUPP;
 			goto err_alloc;
 		}
 	}