perf: regression -- missing /sys/devices/system/cpu/perf_events

perf: regression -- missing /sys/devices/system/cpu/perf_events

[Vince Weaver]

I know this is a bit late, but for some reason our users sit on these 
things and then they aren't willing to take it up with linux-kernel 
themselves

> > > commit 15ac9a395a753cb28c674e7ea80386ffdff21785
> > > Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > > Date:   Mon Sep 6 15:51:45 2010 +0200
> > > 
> > >    perf: Remove the sysfs bits

removed the /sys/devices/system/cpu/perf_events
I thought things in /sys were stable ABI?

Apparently it's common for people to have scripts to check if 
perf_events is available in a kernel by checking the existence of
/sys/devices/system/cpu/perf_events, even if they didn't use the
files within.

Now that 2.6.38 kernels are starting to hit the distros we're getting 
complaints that it's missing.

Vince

Re: perf: regression -- missing /sys/devices/system/cpu/perf_events

[Peter Zijlstra]

On Tue, 2011-05-24 at 09:59 -0400, Vince Weaver wrote:
> I know this is a bit late, but for some reason our users sit on these 
> things and then they aren't willing to take it up with linux-kernel 
> themselves
> 
> > > > commit 15ac9a395a753cb28c674e7ea80386ffdff21785
> > > > Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > > > Date:   Mon Sep 6 15:51:45 2010 +0200
> > > > 
> > > >    perf: Remove the sysfs bits
> 
> removed the /sys/devices/system/cpu/perf_events
> I thought things in /sys were stable ABI?

if only.. that stuff changes way too often (not to say that's a good
thing).

> Apparently it's common for people to have scripts to check if 
> perf_events is available in a kernel by checking the existence of
> /sys/devices/system/cpu/perf_events, even if they didn't use the
> files within.

A much more reliable way is simply doing the syscall and seeing what
happens. But if you want to poke around in sysfs, /sys/bus/event_source/
is the new location.

> Now that 2.6.38 kernels are starting to hit the distros we're getting 
> complaints that it's missing.

Urgh, they'd been broken long before.. and I hadn't received any
complaints from people about that. I didn't see the point in keeping
broken interfaces around, esp. since with moving to multiple-pmu they
don't make any sense at all.

Ingo, any idea what to do here?

Re: perf: regression -- missing /sys/devices/system/cpu/perf_events

[Vince Weaver]

On Tue, 24 May 2011, Peter Zijlstra wrote:

> A much more reliable way is simply doing the syscall and seeing what
> happens. But if you want to poke around in sysfs, /sys/bus/event_source/
> is the new location.

It's been suggested that they can look for the existence of:
  /proc/sys/kernel/perf_event_paranoid

is that something not likely to go away?

Vince   

Re: perf: regression -- missing /sys/devices/system/cpu/perf_events

[Ingo Molnar]

* Vince Weaver <vweaver1@eecs.utk.edu> wrote:

> On Tue, 24 May 2011, Peter Zijlstra wrote:
> 
> > A much more reliable way is simply doing the syscall and seeing what
> > happens. But if you want to poke around in sysfs, /sys/bus/event_source/
> > is the new location.
> 
> It's been suggested that they can look for the existence of:
>   /proc/sys/kernel/perf_event_paranoid
> 
> is that something not likely to go away?

Well, we indeed do not remove kernel parameters, but checking for that would 
break on systems (or chroot environments) that do not have /proc mounted or 
visible.

So, what is wrong with the method Peter suggested: the presence of the perf 
syscall (it not returning -ENOSYS) is bona fide evidence that perf is 
available.

Thanks,

	Ingo

Re: perf: regression -- missing /sys/devices/system/cpu/perf_events

[Vince Weaver]

On Tue, 24 May 2011, Ingo Molnar wrote:
> 
> So, what is wrong with the method Peter suggested: the presence of the perf 
> syscall (it not returning -ENOSYS) is bona fide evidence that perf is 
> available.

it's just hard to do that from a shell script.

also, running the perf syscall can be tricky if you have a new kernel but 
an older set of header files that doesn't have the syscall number defined.

Vince
vweaver1@eecs.utk.edu

Re: perf: regression -- missing /sys/devices/system/cpu/perf_events

[David Ahern]

On 05/24/11 14:12, Vince Weaver wrote:
> On Tue, 24 May 2011, Ingo Molnar wrote:
>>
>> So, what is wrong with the method Peter suggested: the presence of the perf 
>> syscall (it not returning -ENOSYS) is bona fide evidence that perf is 
>> available.
> 
> it's just hard to do that from a shell script.

What about kallsyms:

grep sys_perf_event_open /proc/kallsyms

even with the new security feature you should be able to see that it
exists. The name has been the same since the counters->events rename in
cdd6c48. egrep for both names for kernels older than 2.6.31.

David


> 
> also, running the perf syscall can be tricky if you have a new kernel but 
> an older set of header files that doesn't have the syscall number defined.
> 
> Vince
> vweaver1@eecs.utk.edu
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

Re: perf: regression -- missing /sys/devices/system/cpu/perf_events

[Ingo Molnar]

* David Ahern <dsahern@gmail.com> wrote:

> On 05/24/11 14:12, Vince Weaver wrote:
> > On Tue, 24 May 2011, Ingo Molnar wrote:
> >>
> >> So, what is wrong with the method Peter suggested: the presence of the perf 
> >> syscall (it not returning -ENOSYS) is bona fide evidence that perf is 
> >> available.
> > 
> > it's just hard to do that from a shell script.
> 
> What about kallsyms:
> 
> grep sys_perf_event_open /proc/kallsyms

that looks pretty roundabout and expensive - kallsyms can be large.

Nor is there a guarantee that the function will always be called 
sys_perf_event_open() - we already renamed it from sys_perf_counter_open() as 
you yourself mentioned it :-)

Plus the kernel can be built without /proc/kallsyms, and root can chmod off the 
file permissions of /proc/kallsyms for unprivileged user-space as well. So it's 
not a particularly robust check.

I agree with Vince that as far as shell scripts are concerned, checking 
/proc/sys/kernel/perf_event_paranoid would work best - and it works better than 
having to check the perf syscall.

Vince: mind sending a patch that adds a comment to perf_event_paranoid that 
userspace relies on the existence of that file as a feature check? Having such 
reminders in the code works even better than frequent testing ;-)

As far as the actual PAPI library goes i really hope it checks the syscall 
itself: that's much faster and more robust than an 
access("/proc/sys/kernel/perf_event_paranoid") call ...

Thanks,

	Ingo

[patch] perf - comment /proc/sys/kernel/perf_event_paranoid to be part of user ABI

[Vince Weaver]

On Tue, 24 May 2011, Ingo Molnar wrote:

> I agree with Vince that as far as shell scripts are concerned, checking 
> /proc/sys/kernel/perf_event_paranoid would work best - and it works better than 
> having to check the perf syscall.
> 
> Vince: mind sending a patch that adds a comment to perf_event_paranoid that 
> userspace relies on the existence of that file as a feature check? Having such 
> reminders in the code works even better than frequent testing ;-)

Such a patch is included below.  Not sure if this is exactly what you 
meant.

> As far as the actual PAPI library goes i really hope it checks the syscall 
> itself: that's much faster and more robust than an 
> access("/proc/sys/kernel/perf_event_paranoid") call ...

For PAPI itself we decide with substrate to use at compile time.
This casme up because one of the vendors who ships PAPI on various kernel 
revisions had a script to choose the right package to install, and as of 
2.6.37 this broke due to /sys/devices/system/cpu/perf_events going away.

Thanks,

Vince
vweaver1@eecs.utk.edu

Signed-off-by: Vince Weaver <vweaver1@eecs.utk.edu>

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 4fc9244..cbdc7bc 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -938,6 +938,8 @@ static struct ctl_table kern_table[] = {
 	},
 #endif
 #ifdef CONFIG_PERF_EVENTS
+	/* Userspace relies on this file existing as a check for */
+	/* perf_events being enabled.  Do not remove!            */
 	{
 		.procname	= "perf_event_paranoid",
 		.data		= &sysctl_perf_event_paranoid,

Re: [patch] perf - comment /proc/sys/kernel/perf_event_paranoid to be part of user ABI

[Ingo Molnar]

* Vince Weaver <vweaver1@eecs.utk.edu> wrote:

> On Tue, 24 May 2011, Ingo Molnar wrote:
> 
> > I agree with Vince that as far as shell scripts are concerned, checking 
> > /proc/sys/kernel/perf_event_paranoid would work best - and it works better than 
> > having to check the perf syscall.
> > 
> > Vince: mind sending a patch that adds a comment to perf_event_paranoid that 
> > userspace relies on the existence of that file as a feature check? Having such 
> > reminders in the code works even better than frequent testing ;-)
> 
> Such a patch is included below.  Not sure if this is exactly what you 
> meant.

Yeah, that's exactly what i meant - we don't need more really.

Most sysctls are not ABIs (there's no userspace that relies on them) 
so the general attitude is to change them freely and backtrack if 
something breaks unexpectedly. We can avoid that by commenting the 
dependency.

Thanks,

	Ingo

[tip:perf/core] perf: Comment /proc/sys/kernel/perf_event_paranoid to be part of user ABI

[tip-bot for Vince Weaver]

Commit-ID:  aa4a221875873d2a1f9656cb7fd7e545e952b4fa
Gitweb:     http://git.kernel.org/tip/aa4a221875873d2a1f9656cb7fd7e545e952b4fa
Author:     Vince Weaver <vweaver1@eecs.utk.edu>
AuthorDate: Fri, 3 Jun 2011 17:54:40 -0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sat, 4 Jun 2011 12:22:04 +0200

perf: Comment /proc/sys/kernel/perf_event_paranoid to be part of user ABI

Turns out that distro packages use this file as an indicator of
the perf event subsystem - this is easier to check for from scripts
than the existence of the system call.

This is easy enough to keep around for the kernel, so add a
comment to make sure it stays so.

Signed-off-by: Vince Weaver <vweaver1@eecs.utk.edu>
Cc: David Ahern <dsahern@gmail.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: paulus@samba.org
Cc: acme@redhat.com
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Link: http://lkml.kernel.org/r/alpine.DEB.2.00.1106031751170.29381@cl320.eecs.utk.edu
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/sysctl.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 4fc9244..f175d98 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -938,6 +938,12 @@ static struct ctl_table kern_table[] = {
 	},
 #endif
 #ifdef CONFIG_PERF_EVENTS
+	/*
+	 * User-space scripts rely on the existence of this file
+	 * as a feature check for perf_events being enabled.
+	 *
+	 * So it's an ABI, do not remove!
+	 */
 	{
 		.procname	= "perf_event_paranoid",
 		.data		= &sysctl_perf_event_paranoid,

Re: perf: regression -- missing /sys/devices/system/cpu/perf_events

[Ingo Molnar]

* Vince Weaver <vweaver1@eecs.utk.edu> wrote:

> On Tue, 24 May 2011, Ingo Molnar wrote:
> > 
> > So, what is wrong with the method Peter suggested: the presence of the perf 
> > syscall (it not returning -ENOSYS) is bona fide evidence that perf is 
> > available.
> 
> it's just hard to do that from a shell script.

Yeah.

> also, running the perf syscall can be tricky if you have a new kernel but an 
> older set of header files that doesn't have the syscall number defined.

I suspect you could add some quick band-aid like:

#ifndef __NR_perf_event_open
# ifdef __i386__
#  define __NR_perf_event_open 336
# endif
# ifdef __x86_64__ 
#  define __NR_perf_event_open 298
# endif
# ifdef __powerpc__
#  define __NR_perf_event_open 319
# endif
# ifdef __arm__
#  define __NR_perf_event_open 364
# endif
#endif

#ifndef __NR_perf_event_open
# error Please add the __NR_perf_event_open definition for this architecture!
#endif

This should cover 99% of the users - and fill in the table as people report 
build failures :)

Thanks,

	Ingo