2013-07-03 19:33:39

by Vince Weaver

[permalink] [raw]
Subject: perf/Documentation/ABI -- add some documentation for perf_event sysfs usage


Add some documentation for the perf_event related
/sys/bus/event_source/bus/devices/
files, since in theory it's a stable interface and the only
current documentation is some lex and yacc files in the
perf tools directory.

Signed-off-by: Vince Weaver <[email protected]>

diff --git a/Documentation/ABI/stable/sysfs-bus-event_source-devices b/Documentation/ABI/stable/sysfs-bus-event_source-devices
new file mode 100644
index 0000000..9ef862f
--- /dev/null
+++ b/Documentation/ABI/stable/sysfs-bus-event_source-devices
@@ -0,0 +1,39 @@
+What: /sys/bus/event_source/bus/devices/*/events/*
+Date: Oct 2012
+KernelVersion: 3.8
+Contact: Jiri Olsa <[email protected]>
+Description:
+ Each file in /sys/bus/event_source/bus/devices/*/events/
+ represents a predefined perf_event event.
+
+ The contents of the file are a string describing
+ fields to set when generating raw events for
+ the perf_event_open() system call.
+
+ The layout of the file is
+ attr=value[,attr=value]...
+ Where attr is the name of a bitfield described
+ in /sys/bus/event_source/bus/devices/*/format/*
+ and value is either a hex (starting with 0x) or decimal
+ value.
+
+
+What: /sys/bus/event_source/bus/devices/*/format/*
+Date: Oct 2012
+KernelVersion: 3.8
+Contact: Jiri Olsa <[email protected]>
+Description:
+ Each file in /sys/bus/event_source/bus/devices/*/format/
+ represents a description of a bitfield used when
+ constructing raw perf_event_open() raw events.
+
+ The contents of the file are a string describing
+ the name of the field and its range.
+
+ The layout of the file is
+ field:low[-high]
+ Where field is the field name, low is the low bit
+ of the range, and the optional high value is the high
+ bit of the range. The low and high values are base-10
+ integers.
+


2013-07-04 03:15:04

by Vince Weaver

[permalink] [raw]
Subject: Re: perf/Documentation/ABI -- add some documentation for perf_event sysfs usage

On Wed, 3 Jul 2013, Vince Weaver wrote:

>
> Add some documentation for the perf_event related
> /sys/bus/event_source/bus/devices/
> files, since in theory it's a stable interface and the only
> current documentation is some lex and yacc files in the
> perf tools directory.

OK, don't I feel silly, these values were already documented in
Documentation/ABI/testing/sysfs-bus-event_source-devices-events
rather than ABI/stable

To be fair I missed it because the documentation seemed to come from the
Power developers and not through perf_event

anyway, that documentation says the values will be hex only, which was
broken by

commit f9134f36aed59ab55c0ab1a4618dd455f15aef5f
Author: Andi Kleen <[email protected]>
Date: Mon Jun 17 17:36:52 2013 -0700

perf/x86/intel: Add mem-loads/stores support for Haswell


which added:
EVENT_ATTR_STR(mem-loads, mem_ld_hsw, "event=0xcd,umask=0x1,ldlat=3");

(note ldlat is not in hex).

This broke the trinity fuzzer (which scans to look for events to use)
as well as some of my personal tools.

Should the ldlat value be fixed to be hex? Or should we ammend the ABI
document to allow decimal?

Vince

2013-07-04 09:03:38

by Peter Zijlstra

[permalink] [raw]
Subject: Re: perf/Documentation/ABI -- add some documentation for perf_event sysfs usage

On Wed, Jul 03, 2013 at 11:14:40PM -0400, Vince Weaver wrote:
> On Wed, 3 Jul 2013, Vince Weaver wrote:

> OK, don't I feel silly, these values were already documented in
> Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> rather than ABI/stable
>
> To be fair I missed it because the documentation seemed to come from the
> Power developers and not through perf_event
>
> anyway, that documentation says the values will be hex only, which was
> broken by
>
> commit f9134f36aed59ab55c0ab1a4618dd455f15aef5f
> Author: Andi Kleen <[email protected]>
> Date: Mon Jun 17 17:36:52 2013 -0700
>
> perf/x86/intel: Add mem-loads/stores support for Haswell
>
>
> which added:
> EVENT_ATTR_STR(mem-loads, mem_ld_hsw, "event=0xcd,umask=0x1,ldlat=3");
>
> (note ldlat is not in hex).
>
> This broke the trinity fuzzer (which scans to look for events to use)
> as well as some of my personal tools.
>
> Should the ldlat value be fixed to be hex? Or should we ammend the ABI
> document to allow decimal?

I don't see a good reason not to allow decimal as well. Jolsa?

2013-07-04 22:03:24

by Jiri Olsa

[permalink] [raw]
Subject: Re: perf/Documentation/ABI -- add some documentation for perf_event sysfs usage

On Thu, Jul 04, 2013 at 11:02:53AM +0200, Peter Zijlstra wrote:
> On Wed, Jul 03, 2013 at 11:14:40PM -0400, Vince Weaver wrote:
> > On Wed, 3 Jul 2013, Vince Weaver wrote:
>
> > OK, don't I feel silly, these values were already documented in
> > Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> > rather than ABI/stable
> >
> > To be fair I missed it because the documentation seemed to come from the
> > Power developers and not through perf_event
> >
> > anyway, that documentation says the values will be hex only, which was
> > broken by
> >
> > commit f9134f36aed59ab55c0ab1a4618dd455f15aef5f
> > Author: Andi Kleen <[email protected]>
> > Date: Mon Jun 17 17:36:52 2013 -0700
> >
> > perf/x86/intel: Add mem-loads/stores support for Haswell
> >
> >
> > which added:
> > EVENT_ATTR_STR(mem-loads, mem_ld_hsw, "event=0xcd,umask=0x1,ldlat=3");
> >
> > (note ldlat is not in hex).
> >
> > This broke the trinity fuzzer (which scans to look for events to use)
> > as well as some of my personal tools.
> >
> > Should the ldlat value be fixed to be hex? Or should we ammend the ABI
> > document to allow decimal?
>
> I don't see a good reason not to allow decimal as well. Jolsa?

yep, no technical problem with decimal

hum, the doc mentions 'event' term only, which IS hex only AFAICS ;-)

I think this docs should be updated and either describe all
allowed terms or be generic enough to cover all of them.

jirka

2013-07-05 15:28:16

by Vince Weaver

[permalink] [raw]
Subject: Re: perf/Documentation/ABI -- add some documentation for perf_event sysfs usage

On Fri, 5 Jul 2013, Jiri Olsa wrote:

> On Thu, Jul 04, 2013 at 11:02:53AM +0200, Peter Zijlstra wrote:
> > On Wed, Jul 03, 2013 at 11:14:40PM -0400, Vince Weaver wrote:

> > > Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> > >
> > > Should the ldlat value be fixed to be hex? Or should we ammend the ABI
> > > document to allow decimal?
> >
> > I don't see a good reason not to allow decimal as well. Jolsa?
>
> yep, no technical problem with decimal
>
> hum, the doc mentions 'event' term only, which IS hex only AFAICS ;-)
>
> I think this docs should be updated and either describe all
> allowed terms or be generic enough to cover all of them.

So you're saying the official kernel ABI should be "whatever the
userspace perf tool happens to accept"?

That's not really useful, especially as perf doesn't distinguish between
event strings read from sysfs and those passed on the perf command line.
The ABI documentation in effect ends up being a pointer to a mostly
incomprehensible lex/yacc file.

I think we should just remove the
Documentation/ABI/testing/sysfs-bus-event_source-devices-events
file as it's misleading. Userspace broke and no one cares.

It's not even easy to audit all places in the kernel that create the sysfs
event files because each architecture does it differently. And there's no
sane way to unit test this on a new kernel release because the values
printed depend on the hardware you have, so without a full range of all
families of cpus for all architectures you never know when someone has
added a decimal value, or started depending on { characters, etc.

This is the problem with perf in the tools directory, any other user of
the ABI is eternally second-class.

Vince

2013-07-07 14:56:48

by Jiri Olsa

[permalink] [raw]
Subject: Re: perf/Documentation/ABI -- add some documentation for perf_event sysfs usage

On Fri, Jul 05, 2013 at 11:28:08AM -0400, Vince Weaver wrote:
> On Fri, 5 Jul 2013, Jiri Olsa wrote:
>
> > On Thu, Jul 04, 2013 at 11:02:53AM +0200, Peter Zijlstra wrote:
> > > On Wed, Jul 03, 2013 at 11:14:40PM -0400, Vince Weaver wrote:
>
> > > > Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> > > >
> > > > Should the ldlat value be fixed to be hex? Or should we ammend the ABI
> > > > document to allow decimal?
> > >
> > > I don't see a good reason not to allow decimal as well. Jolsa?
> >
> > yep, no technical problem with decimal
> >
> > hum, the doc mentions 'event' term only, which IS hex only AFAICS ;-)
> >
> > I think this docs should be updated and either describe all
> > allowed terms or be generic enough to cover all of them.
>
> So you're saying the official kernel ABI should be "whatever the
> userspace perf tool happens to accept"?

nope, I meant whatever term is used by kernel to describe the event

'event, umask, edge, pc, any, inv, cmask, ldat'

please check x86_event_sysfs_show function

>
> That's not really useful, especially as perf doesn't distinguish between
> event strings read from sysfs and those passed on the perf command line.
> The ABI documentation in effect ends up being a pointer to a mostly
> incomprehensible lex/yacc file.

the documentation is missing, no argument here :-\
'sysfs-bus-event_source-devices-events' could be more descriptive

>
> I think we should just remove the
> Documentation/ABI/testing/sysfs-bus-event_source-devices-events
> file as it's misleading. Userspace broke and no one cares.
>
> It's not even easy to audit all places in the kernel that create the sysfs
> event files because each architecture does it differently. And there's no
> sane way to unit test this on a new kernel release because the values
> printed depend on the hardware you have, so without a full range of all
> families of cpus for all architectures you never know when someone has
> added a decimal value, or started depending on { characters, etc.
>
> This is the problem with perf in the tools directory, any other user of
> the ABI is eternally second-class.

with updated documentation I dont see the issue

jirka