From: Andi Kleen <[email protected]>
Add a precise qualifier, like cpu/event=0x3c,precise=1/
This is needed so that the kernel can request enabling PEBS
on specific events. This is useful for mem-loads/mem-stores
Currently you have to known that mem-loads is a PEBS event
and use
perf record -e cpu/mem-loads/p ...
With this patch we can export the PEBSness of events in sysfs and
then allow
perf record -e cpu/mem-loads/ ...
or with the additional patch to automatically add cpu//
perf record -e mem-loads ...
Also useful for some other events added in later patches.
v2: Allow 3 as value
Signed-off-by: Andi Kleen <[email protected]>
---
tools/perf/util/parse-events.c | 6 ++++++
tools/perf/util/parse-events.h | 1 +
tools/perf/util/parse-events.l | 1 +
3 files changed, 8 insertions(+)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 995fc25..34f1470 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -568,6 +568,12 @@ do { \
case PARSE_EVENTS__TERM_TYPE_NAME:
CHECK_TYPE_VAL(STR);
break;
+ case PARSE_EVENTS__TERM_TYPE_PRECISE:
+ CHECK_TYPE_VAL(NUM);
+ if ((unsigned)term->val.num > 3)
+ return -EINVAL;
+ attr->precise_ip = term->val.num;
+ break;
default:
return -EINVAL;
}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index 8a48593..13d7c66 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -48,6 +48,7 @@ enum {
PARSE_EVENTS__TERM_TYPE_NAME,
PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD,
PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE,
+ PARSE_EVENTS__TERM_TYPE_PRECISE,
};
struct parse_events_term {
diff --git a/tools/perf/util/parse-events.l b/tools/perf/util/parse-events.l
index e9d1134..32a9000 100644
--- a/tools/perf/util/parse-events.l
+++ b/tools/perf/util/parse-events.l
@@ -169,6 +169,7 @@ period { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_SAMPLE_PERIOD); }
branch_type { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_BRANCH_SAMPLE_TYPE); }
, { return ','; }
"/" { BEGIN(INITIAL); return '/'; }
+precise { return term(yyscanner, PARSE_EVENTS__TERM_TYPE_PRECISE); }
{name_minus} { return str(yyscanner, PE_NAME); }
}
--
1.8.3.1
From: Andi Kleen <[email protected]>
With the earlier patches to automatically try cpu// and add
a precise sys attribute, we can now enable PEBS for the mem-loads,
mem-stores events everywhere.
This allows to use
perf record -e mem-loads ...
instead of
perf record -e cpu/mem-loads/p ...
Always use precise=2 even though it is costly pre-Haswell
Cc: [email protected]
v2: Different white space
v3: Always use precise=2, as people seem to think overhead doesn't matter.
v4: Longer lines
Signed-off-by: Andi Kleen <[email protected]>
---
arch/x86/kernel/cpu/perf_event_intel.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/cpu/perf_event_intel.c b/arch/x86/kernel/cpu/perf_event_intel.c
index fbc9210..1871866 100644
--- a/arch/x86/kernel/cpu/perf_event_intel.c
+++ b/arch/x86/kernel/cpu/perf_event_intel.c
@@ -176,9 +176,9 @@ static struct extra_reg intel_snbep_extra_regs[] __read_mostly = {
EVENT_EXTRA_END
};
-EVENT_ATTR_STR(mem-loads, mem_ld_nhm, "event=0x0b,umask=0x10,ldlat=3");
-EVENT_ATTR_STR(mem-loads, mem_ld_snb, "event=0xcd,umask=0x1,ldlat=3");
-EVENT_ATTR_STR(mem-stores, mem_st_snb, "event=0xcd,umask=0x2");
+EVENT_ATTR_STR(mem-loads, mem_ld_nhm, "event=0x0b,umask=0x10,ldlat=3,precise=2");
+EVENT_ATTR_STR(mem-loads, mem_ld_snb, "event=0xcd,umask=0x1,ldlat=3,precise=2");
+EVENT_ATTR_STR(mem-stores, mem_st_snb, "event=0xcd,umask=0x2,precise=2");
struct attribute *nhm_events_attrs[] = {
EVENT_PTR(mem_ld_nhm),
@@ -2034,8 +2034,9 @@ static __init void intel_nehalem_quirk(void)
}
}
-EVENT_ATTR_STR(mem-loads, mem_ld_hsw, "event=0xcd,umask=0x1,ldlat=3");
-EVENT_ATTR_STR(mem-stores, mem_st_hsw, "event=0xd0,umask=0x82")
+EVENT_ATTR_STR(mem-loads, mem_ld_hsw,
+ "event=0xcd,umask=0x1,ldlat=3,precise=2");
+EVENT_ATTR_STR(mem-stores, mem_st_hsw, "event=0xd0,umask=0x82,precise=2")
static struct attribute *hsw_events_attrs[] = {
EVENT_PTR(mem_ld_hsw),
--
1.8.3.1
On Mon, 22 Jul 2013, Andi Kleen wrote:
> From: Andi Kleen <[email protected]>
>
> Add a precise qualifier, like cpu/event=0x3c,precise=1/
So you're adding this to "events/" but not to "format/"?
This breaks the ABI, which specifies that the only fields
that can appear in a sysfs events specifier must exist under
the format directory.
This was what I worried about when I said people are going to start
leaking perf tool specific events into the kernel sysfs tree.
This will break various existing programs.
Vince
On Tue, Jul 23, 2013 at 01:40:40AM -0400, Vince Weaver wrote:
> On Mon, 22 Jul 2013, Andi Kleen wrote:
>
> > From: Andi Kleen <[email protected]>
> >
> > Add a precise qualifier, like cpu/event=0x3c,precise=1/
>
> So you're adding this to "events/" but not to "format/"?
Fair point, but note that precise_ip is a bitfield. Since there is no
way to express that currently the parser would need to special case it anyways.
So it could be just added as precise_ip: precise_ip in events/,
but I have some doubts that really helps anyone.
Or invent some new syntax to abstract the bitfields, but that also
would need changes.
> This will break various existing programs.
Like what?
-Andi
--
[email protected] -- Speaking for myself only
I hate having to justify why breaking the ABI is unacceptable.
On Mon, 22 Jul 2013, Andi Kleen wrote:
> On Tue, Jul 23, 2013 at 01:40:40AM -0400, Vince Weaver wrote:
> > On Mon, 22 Jul 2013, Andi Kleen wrote:
> >
> > > From: Andi Kleen <[email protected]>
> > >
> > > Add a precise qualifier, like cpu/event=0x3c,precise=1/
> >
> > So you're adding this to "events/" but not to "format/"?
>
> Fair point, but note that precise_ip is a bitfield. Since there is no
> way to express that currently the parser would need to special case it anyways.
It breaks the ABI. The events/* sysfs files are documented as only
holding values for the bitfields described in format/*.
The perf_event ABI is there for all tools, not to just be a thin shim
for the perf tool.
Even if the maintainers for whatever are going to just ignore the ABI,
then fine, but then I'd like to see a patch also updating the sysfs
ABI documentation and a patch to the perf_event_open.2 manpage would be
nice as well.
> > This will break various existing programs.
>
> Like what?
Why does it matter? It breaks multiple programs I work on.
A major one is the trinity fuzzer, which parses perf_event sysfs to
construct valid events.
The code won't segfault because I wrote the parser to be robust, but it
will spit a warning when invalid junk is put in the sysfs events
files, and I'll get e-mails about it spamming trinity runs with warning
messages within hours of such a commit hitting linus-git.
I'm a bit grumpy about this because I just finished fixing the fallout
from your last time breaking the ABI a few weeks ago when your broken code
started dumping non-hex fields into the sysfs event strings. I've learned
now that I have to go over your patches with a fine-tooth code because you
have no regard for the ABI.
Vince
On Tue, Jul 23, 2013 at 05:27:43PM -0400, Vince Weaver wrote:
>
> I hate having to justify why breaking the ABI is unacceptable.
Well it's a testing ABI, so we can do changes to it.
I hope you're not suggesting that perf cannot be extended anymore.
As you know, hardware PMUs are constantly evolving, and perf has evolve
along with them to stay useful.
> It breaks the ABI. The events/* sysfs files are documented as only
> holding values for the bitfields described in format/*.
Ok. Need to fix the documentation then for precise=1. I'll send patches.
Also can add it to format/*, but since it's not in config* it will
be an extension. Since it would be awkward to teach every parser
about all the flags, we could add a "flags" field that is handled
like config, and contains all the flags.
So format/precise would be
flags:15-16
on little endian. Looks good?
> I'm a bit grumpy about this because I just finished fixing the fallout
> from your last time breaking the ABI a few weeks ago when your broken code
> started dumping non-hex fields into the sysfs event strings. I've learned
Not sure what you're talking about?
iirc the only recent sysfs change of mine merged recently was adding
intx/intx_cp, and format files never have had hex numbers in it.
> now that I have to go over your patches with a fine-tooth code because you
> have no regard for the ABI.
Any useful code review is appreciated.
-Andi
--
[email protected] -- Speaking for myself only.
On 07/23/2013 06:51 PM, Andi Kleen wrote:
> On Tue, Jul 23, 2013 at 05:27:43PM -0400, Vince Weaver wrote:
>> >
>> >I hate having to justify why breaking the ABI is unacceptable.
> Well it's a testing ABI, so we can do changes to it.
The testing ABI has a simple policy about changes:
The interface can be changed to add new features, but the
current interface will not break by doing this, unless grave
errors or security problems are found in them.
It's probably fine to change a testing ABI once in a while, but when things
like trinity start breaking that often due to ABI changes in the same exact
place, that's too much IMO.
Thanks,
Sasha
On Tue, Jul 23, 2013 at 08:39:09PM -0400, Sasha Levin wrote:
> On 07/23/2013 06:51 PM, Andi Kleen wrote:
> >On Tue, Jul 23, 2013 at 05:27:43PM -0400, Vince Weaver wrote:
> >>>
> >>>I hate having to justify why breaking the ABI is unacceptable.
> >Well it's a testing ABI, so we can do changes to it.
>
> The testing ABI has a simple policy about changes:
>
> The interface can be changed to add new features, but the
> current interface will not break by doing this, unless grave
> errors or security problems are found in them.
>
> It's probably fine to change a testing ABI once in a while, but when things
> like trinity start breaking that often due to ABI changes in the same exact
> place, that's too much IMO.
It sounds like trinity is breaking (well printing a message, not really
breaking) on any addition. So if we follow that the perf sysfs interface
would be completely frozen and can never be extended over today.
I don't think it's a big problem that a test tool needs to be extended
when the software it's testing changes.
If there are enough other widely used programs that actually break from
additions probably would need a v2 of the sysfs interface for extensions
(with new file or directory names), and keep v1 frozen for
compatibility.
But I don't think that's the case today?
-Andi
--
[email protected] -- Speaking for myself only.
On Wed, 24 Jul 2013, Andi Kleen wrote:
> On Tue, Jul 23, 2013 at 08:39:09PM -0400, Sasha Levin wrote:
> It sounds like trinity is breaking (well printing a message, not really
> breaking) on any addition. So if we follow that the perf sysfs interface
> would be completely frozen and can never be extended over today.
Trinity is breaking on any *ABI-breaking* additions.
You can add arbitrary fields to sysfs events/* as long as there's
a corresponding format/* entry.
> I don't think it's a big problem that a test tool needs to be extended
> when the software it's testing changes.
Trinity isn't the only tool that would break, I just picked it as an
example because it's one that kernel devs have likely heard of.
Vince
On Wed, 24 Jul 2013, Andi Kleen wrote:
> On Tue, Jul 23, 2013 at 05:27:43PM -0400, Vince Weaver wrote:
> >
> So format/precise would be
>
> flags:15-16
>
> on little endian. Looks good?
I'd prefer if it were
precise_ip:0-1
as the code you use to set the value looks like
attr->precise_ip=2;
I don't think you can even set bits 15-16 of flags directly, can you?
Althought having to have a parser being able to parse the names of all 50+
perf_event->attr arguments, then parse hex/decimal strings from another
file in sysfs, then trying to map that to binary fields in a complicated
structure that is then passed to a syscall gets a bit stupid at some
point; this is one of the silliest interfaces I've had to deal with and
I've done low-level X11 programming before.
Ideally we could do something sane, such as maybe having the event
descriptions in a library in userspace where it belongs but it's a bit
late to close the barn door now.
> > I'm a bit grumpy about this because I just finished fixing the fallout
> > from your last time breaking the ABI a few weeks ago when your broken code
> > started dumping non-hex fields into the sysfs event strings. I've learned
>
> Not sure what you're talking about?
I'm pretty sure it was one of your patches that added the new memory stuff
which had "ldlat=1" in one of the events, and that was not a hex value
prefixed by 0x as specified in the ABI documentation and thus broke
various parsers.
By the time I caught this the perf_event maintainers declared it was too
late to do anything about it.
Vince
On Tue, 23 Jul 2013, Sasha Levin wrote:
> It's probably fine to change a testing ABI once in a while, but when things
> like trinity start breaking that often due to ABI changes in the same exact
> place, that's too much IMO.
The problem is they want the ABI to be "whatever our closely-coupled
userspace perf tool accepts as input" which is an often-changing
complicated (and undocumented) set of LEX and YACC parsers.
I guess we could just give in and declare that to be the official
perf sysfs interface in the ABI documentation.
It's frustrating trying to maintain tools that use the perf_event
interface because the inclusion of perf in the kernel is used as an excuse
to constantly break the ABI.
Vince
> I'd prefer if it were
>
> precise_ip:0-1
>
> as the code you use to set the value looks like
> attr->precise_ip=2;
> I don't think you can even set bits 15-16 of flags directly, can you?
Sorry I meant flags as an alias of "the 64bits currently occupied by the
bitfield". Perhaps the name choice was not very good.
"flags_bitfield" ?
So the tool would only need to know that, not every bit.
In theory it could be also generalized as a byte offset to perf_event,
but that may be overengineered.
-Andi
--
[email protected] -- Speaking for myself only.
On Wed, 24 Jul 2013, Andi Kleen wrote:
> Sorry I meant flags as an alias of "the 64bits currently occupied by the
> bitfield". Perhaps the name choice was not very good.
>
> "flags_bitfield" ?
>
> So the tool would only need to know that, not every bit.
>
> In theory it could be also generalized as a byte offset to perf_event,
> but that may be overengineered.
I somehow doubt this would be acceptable. If it were, we could have had a
somewhat better interface by just having the event fields be a list of
values without involving format/* at all, something like
config=0x58034;config1=0x20;precise_ip=0x4
For whatever reason things have to be human readable, and I don't think
just having an opaque 64-bit "flags" value will be accepted.
I'm likely wrong though, I have a very low accuracy rate for predicting
future perf_event design decisions.
This is all complicated by the intertwined nature of the perf_event ABI
and the perf tool, and the way that there's at least three or four
different ways to specify the same event from the perf tool command line.
Vince
* Andi Kleen <[email protected]> wrote:
> On Tue, Jul 23, 2013 at 05:27:43PM -0400, Vince Weaver wrote:
> >
> > I hate having to justify why breaking the ABI is unacceptable.
>
> Well it's a testing ABI, so we can do changes to it.
>
> I hope you're not suggesting that perf cannot be extended anymore.
It obviously should remain extensible, limiting it to 'config' is rather
stupid. If a parser sees something it cannot parse it should ignore that
event.
Your feature to export 'precise' requirements on events looks useful to
me. We could implement it not by special casing it implicitly but by
saying that if ../format/precise contains something like:
attr:240-241
then that's a natural extension of the config:X-Y format and should be
interpreted to mean mean 2 bits in the perf attr field. I.e. we could go
beyond the config bitfield.
Basically the whole perf_event_attr can be thought of as a 'giant
bitfield', in which we can specify values to export an enumerated list of
events from the kernel to tooling.
(Using attr:X-Y the config and config1 variants can be expressed as well,
as the config fields are inside the attr structure.)
The positions within the perf_attr are an ABI, so this would work pretty
well.
Thanks,
Ingo
> Your feature to export 'precise' requirements on events looks useful to
> me. We could implement it not by special casing it implicitly but by
> saying that if ../format/precise contains something like:
>
> attr:240-241
>
> then that's a natural extension of the config:X-Y format and should be
> interpreted to mean mean 2 bits in the perf attr field. I.e. we could go
> beyond the config bitfield.
>
> Basically the whole perf_event_attr can be thought of as a 'giant
> bitfield', in which we can specify values to export an enumerated list of
> events from the kernel to tooling.
>
> (Using attr:X-Y the config and config1 variants can be expressed as well,
> as the config fields are inside the attr structure.)
>
> The positions within the perf_attr are an ABI, so this would work pretty
> well.
Wouldn't we need different bits for each architecture then?
32bit/64bit, some archs with weird alignment rules, maybe different for
BE/LE too?
Ok I suppose it could be somehow auto generated in asm-offsets.c,
although I'm not sure how to get a bitfield offset there.
-Andi
--
[email protected] -- Speaking for myself only.
* Andi Kleen <[email protected]> wrote:
> > Your feature to export 'precise' requirements on events looks useful to
> > me. We could implement it not by special casing it implicitly but by
> > saying that if ../format/precise contains something like:
> >
> > attr:240-241
> >
> > then that's a natural extension of the config:X-Y format and should be
> > interpreted to mean mean 2 bits in the perf attr field. I.e. we could go
> > beyond the config bitfield.
> >
> > Basically the whole perf_event_attr can be thought of as a 'giant
> > bitfield', in which we can specify values to export an enumerated list of
> > events from the kernel to tooling.
> >
> > (Using attr:X-Y the config and config1 variants can be expressed as well,
> > as the config fields are inside the attr structure.)
> >
> > The positions within the perf_attr are an ABI, so this would work pretty
> > well.
>
> Wouldn't we need different bits for each architecture then? 32bit/64bit,
> some archs with weird alignment rules, maybe different for BE/LE too?
>
> Ok I suppose it could be somehow auto generated in asm-offsets.c,
> although I'm not sure how to get a bitfield offset there.
That, or we could indeed start adding specific field names as well, which
would have a natural position and order.
Thanks,
Ingo
On Thu, 12 Sep 2013, Ingo Molnar wrote:
> * Andi Kleen <[email protected]> wrote:
>
> > On Tue, Jul 23, 2013 at 05:27:43PM -0400, Vince Weaver wrote:
> > >
> > > I hate having to justify why breaking the ABI is unacceptable.
> >
> > Well it's a testing ABI, so we can do changes to it.
> >
> > I hope you're not suggesting that perf cannot be extended anymore.
>
> It obviously should remain extensible, limiting it to 'config' is rather
> stupid. If a parser sees something it cannot parse it should ignore that
> event.
Well
Documentation/ABI/testing/sysfs-bus-event_source-devices-events
implies that you won't find field names that aren't in a format file, and
Documentation/ABI/testing/sysfs-bus-event_source-devices-format
specifies that only "config" fields will appear there.
So if this will be changing you might want to update the documentation, or
just remove it altogether because it is getting increasingly inaccurate.
I've tried getting this documentation updated
( https://lkml.org/lkml/2013/7/25/2 )
but the patches have been ignored.
In any case, I notice perf also uses the "open" syscall, so I look forward
to your eventual creation of
/sys/bus/open/file_permissions/*
$ cat /sys/bus/open/file_permissions/write
flags=0101,mode=0666
Because what every system call needs is some sort of tree under sysfs
exporting various common settings in unswappable kernel memory that
require a complex LEX/YACC parser to interpret.
That's much simpler than using a library, or having a user looking
things up in a manpage and placing a few comments in their code.
Vince
On Thu, Sep 12, 2013 at 07:36:17PM +0200, Andi Kleen wrote:
> > Your feature to export 'precise' requirements on events looks useful to
> > me. We could implement it not by special casing it implicitly but by
> > saying that if ../format/precise contains something like:
> >
> > attr:240-241
Since we currently have the pattern $name:bits to mean
perf_event_attr::$name the above would imply and create a possible
collision with perf_event_attr::attr.
If we're going to do this I'd propose using something like _:240-241,
for while '_' is a valid name in C its not something we're ever going to
allow in perf_event_attr.
> > then that's a natural extension of the config:X-Y format and should be
> > interpreted to mean mean 2 bits in the perf attr field. I.e. we could go
> > beyond the config bitfield.
> >
> > Basically the whole perf_event_attr can be thought of as a 'giant
> > bitfield', in which we can specify values to export an enumerated list of
> > events from the kernel to tooling.
> >
> > (Using attr:X-Y the config and config1 variants can be expressed as well,
> > as the config fields are inside the attr structure.)
> >
> > The positions within the perf_attr are an ABI, so this would work pretty
> > well.
>
> Wouldn't we need different bits for each architecture then?
> 32bit/64bit, some archs with weird alignment rules, maybe different for
> BE/LE too?
Typically PMU drivers are per arch and all the format stuff is per pmu
driver so I'd not worry about that just yet.
But yes, while the perf_event_attr thing is ABI its not identical across
archs.
> Ok I suppose it could be somehow auto generated in asm-offsets.c,
> although I'm not sure how to get a bitfield offset there.
Yes, that is an unfortunate situation. I (and either Acme or Jolsa)
tried wrapping the bitfield in an anonymous union to create a named
variable for the entire u64 but older GCC completely fails with that.
* Peter Zijlstra <[email protected]> wrote:
> On Thu, Sep 12, 2013 at 07:36:17PM +0200, Andi Kleen wrote:
> > > Your feature to export 'precise' requirements on events looks useful to
> > > me. We could implement it not by special casing it implicitly but by
> > > saying that if ../format/precise contains something like:
> > >
> > > attr:240-241
>
> Since we currently have the pattern $name:bits to mean
> perf_event_attr::$name the above would imply and create a possible
> collision with perf_event_attr::attr.
>
> If we're going to do this I'd propose using something like _:240-241,
> for while '_' is a valid name in C its not something we're ever going to
> allow in perf_event_attr.
Ok - and I'm not against adding individual 'names' one by one as well,
that allows us to expose only the fields that relate to event
configuration.
For example if we added 'type' as well we could expose the generic,
hardware-independent events via sysfs as well.
( Eventually this scheme would be fit to expose more advanced events as
well: such as composite groups of events with simple arithmetic
operations between them. That would allow the exposure of E1+E2-E3 type
of simple calculations. )
> > Wouldn't we need different bits for each architecture then?
> > 32bit/64bit, some archs with weird alignment rules, maybe different
> > for BE/LE too?
>
> Typically PMU drivers are per arch and all the format stuff is per pmu
> driver so I'd not worry about that just yet.
ok.
> But yes, while the perf_event_attr thing is ABI its not identical across
> archs.
Yeah, like syscalls - it's not an on-disk format.
> > Ok I suppose it could be somehow auto generated in asm-offsets.c,
> > although I'm not sure how to get a bitfield offset there.
>
> Yes, that is an unfortunate situation. I (and either Acme or Jolsa)
> tried wrapping the bitfield in an anonymous union to create a named
> variable for the entire u64 but older GCC completely fails with that.
We could be careful with bitfields and enumerate their offsets explicitly,
with a build time testcase that makes sure that the offsets match reality.
Thanks,
Ingo
On Fri, Sep 13, 2013 at 11:50:57AM +0200, Ingo Molnar wrote:
> For example if we added 'type' as well we could expose the generic,
> hardware-independent events via sysfs as well.
Type is already fully implied by where you'll find the event in sysfs:
/sys/bus/event_sources/devices/$PMU/events/
needs
perf_event_attr::type := /sys/bus/event_sources/devices/$PMU/type
On Fri, 13 Sep 2013, Peter Zijlstra wrote:
> On Fri, Sep 13, 2013 at 11:50:57AM +0200, Ingo Molnar wrote:
> > For example if we added 'type' as well we could expose the generic,
> > hardware-independent events via sysfs as well.
>
> Type is already fully implied by where you'll find the event in sysfs:
>
> /sys/bus/event_sources/devices/$PMU/events/
>
> needs
>
> perf_event_attr::type := /sys/bus/event_sources/devices/$PMU/type
OK, fine, another question then.
Is there any reason these values have to be human-readable?
The only reason you are using this crazy format I can see is because it
makes maintaining your personal userspace implementation (perf) easier at
the expense of everyone else who want to use this interface.
Honestly, an interface like
cat /sys/bus/event_sources/devices/$PMU/events/new_event
size=320,0xdeadbeef,0xcafef00d,....,0x000000
when you just set up an array, copy in the values, then memcpy() it into
place on top of a struct attr is a million times easier than what you are
propsing:
1. A huge complicated LEX/YACC parser
2. The parser has to read in many different files under ../format/..
and build up a tree of names and shift/masks
3. The event is read in and then text has to be parsed, values read,
and then shifting-masking to get a value for each register
4. A mapping has to be in the code of the various (of the over 40+)
fields in the struct perf_attr field, and each value has to
be put at the proper offset
5. If ever a new field is added to struct perf_attr, then any event
using it breaks until your parser is updated with all the info
about this field.
It's huge, takes up non-swappable kernel mem with lots of individual sysfs
files, requires a complex parser for what should be just a simple
config setup, and is fragile when new fields are added.
But of course since perf is tightly coupled into the kernel source tree
you can get away with it. I guess I should just be glad you aren't
exporting it as XML or something.
Vince
> Typically PMU drivers are per arch and all the format stuff is per pmu
> driver so I'd not worry about that just yet.
That's a good point. So hard coded numbers would be fine for now.
> Yes, that is an unfortunate situation. I (and either Acme or Jolsa)
> tried wrapping the bitfield in an anonymous union to create a named
> variable for the entire u64 but older GCC completely fails with that.
In theory it's possible by parsing readelf output.
-Andi
--
[email protected] -- Speaking for myself only