Re: [PATCH 1/2] tools, perf: Add a precise event qualifier v2

[Vince Weaver]

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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/