Messages in this thread | | | Date | Thu, 11 Mar 2010 13:46:08 +0100 | From | Ingo Molnar <> | Subject | Re: [RFC] [PATCH 1/1] perf: add support for arch-dependent symbolic event names to "perf stat" |
| |
* Corey Ashford <cjashfor@linux.vnet.ibm.com> wrote:
> On 3/3/2010 6:30 PM, Corey Ashford wrote: > >For your review, this patch adds support for arch-dependent symbolic > >event names to the "perf stat" tool, and could be expanded to other > >"perf *" commands fairly easily, I suspect. > > > >To support arch-dependent event names without adding arch-dependent code > >to perf, I added a callout mechanism whereby perf will look for the > >environment variable: PERF_ARCH_DEP_LIB, and if it exists, it will try > >to open it as a shared object. If that succeeds, it looks for the symbol > >"parse_arch_dep_event". If that exists, that function will be called by > >parse_events() before all of the other event parsing functions in > >parse-events.c. It is passed the same arguments as the other > >parse_*_event functions, namely the event string and a pointer to an > >event attribute structure. > > > >As the code existed, "perf stat" would print out the count results, but > >for raw events (which is how arch-dependent events are supported in > >perf_events), it would just print out a raw code. This is not > >acceptable, especially when a symbolic name was placed on the command > >line. So I changed the code to save away the event name that was passed > >on the command line, rather than doing a reverse translation to an event > >string based on the event type and config fields of the attr structure. > >In this way, there's no need for a reverse translation function in the > >arch-dependent library; only a event string->attr struct function is > >needed. > > > >I could well be missing something, but I don't understand why reverse > >translation is ever needed in perf, as long as the tool keeps track of > >the original event strings. > > A couple of follow-up comments on this patch: > > This functionality was designed to provide a generalized interface to an > external event name -> attr struct library, such as libpfm4. libpfm4 has an > interface that nearly exactly matches parse_*_event() profiles, so it's > quite easy to write a small wrapper function to call libpfm4's function. > > Ingo Molnar discussed adding some visibility to the arch-dependent event > names through some other interface, such as through /sys/devices/pmus > perhaps, but that discussion is a long way (as far as I know) from having > something usable today. So you could think of this external library > approach to be a stop-gap until something better is developed. When/if that > new event naming mechanism becomes available, we can easily remove this > external library support from perf.
I'm quite much against stop-gap measures like this - they tend to become tomorrow's impossible-to-remove quirk.
If you want extensible events you can already do it by providing an ftrace tracepoint event via TRACE_EVENT. They are easy to add and ad-hoc, and are supported throughout by perf.
That could be librarized further by providing an /eventfs or /proc/events interface to enumerate them.
Or if you want to extend the perf events namespace ABI you can send patches for that as well. (It's not a big issue if a particular event is currently only supported on Power for example - as long as you make a good effort naming and structuring it in a reasonably generic way.)
Thanks,
Ingo
| |