Messages in this thread | | | Date | Fri, 27 Jan 2012 13:08:38 -0800 | From | Corey Ashford <> | Subject | Re: [PATCH 5/9] perf: Adding sysfs group format attribute for pmu device |
| |
On 01/27/2012 06:34 AM, Jiri Olsa wrote: > Adding sysfs group 'format' attribute for pmu device that > contains a syntax description on how to construct raw events. > > The event configuration is described in following > struct pefr_event_attr attributes: > > config > config1 > config2 > > Each sysfs attribute within the format attribute group, > describes mapping of name and bitfield definition within > one of above attributes. > > eg: > "/sys/...<dev>/format/event" contains "config:0-7" > "/sys/...<dev>/format/umask" contains "config:8-15" > "/sys/...<dev>/format/usr" contains "config:16" > > the attribute value syntax is: > > line: config ':' bits > config: 'config' | 'config1' | 'config2" > bits: bits ',' bit_term | bit_term > bit_term: VALUE '-' VALUE | VALUE > > Adding format_defined bool to the struct pmu to specify wether > pmu defines its own set of format attributes (within the > attr_groups member) or the default format attributes should be > used: > "/sys/...<dev>/format/config" contains "config:0-63" > "/sys/...<dev>/format/config1" contains "config1:0-63" > "/sys/...<dev>/format/config2" contains "config2:0-63"
Hi Jiri,
I've been out of the perf_events loop for some time, but I did finally notice your patch series thread.
I think what you've done is very good and I'm excited to see progress in this area. However, it's not clear to me that it is as generalized as it needs to be for some PMU's. I say this because not all events on a given PMU will have the same needed fields.
As an example, the IBM PowerEN processor has roughly 20 different PMU's on it. Some of those PMU's are quite complex and divide their events up into subsets, each with different fields. For example, some events may have a PID matching field, and others may have an bus number matching field, or matching mode field, etc. The fields are different widths, and may overlap in the config/1/2 space.
It seems that there are two approaches you could take:
1) Keep your format, but allow the fields to overlap in the bit space. For example:
"/sys/...<dev>/format/event" contains "config:0-7" "/sys/...<dev>/format/pidmatch" contains "config:8-15" "/sys/...<dev>/format/busmatch" contains "config:8-13"
Note that busmatch overlaps pidmatch
2) Create event groups that have their overlapping config space separated out:
"/sys/...<dev>/format/event" contains "config:0-7"
"/sys/...<dev>.1/format/pidmatch" contains "config:8-15"
"/sys/...<dev>.2/format/busmatch" contains "config:8-13"
Notice the .1 and .2 on the <dev>.
This might help the user understand which fields go together. I'm not sold on the .1 syntax... you could do it as <dev>.<event-group-name>/ or <dev>/<event-group-name>/... or whatever seems to make the most sense and is relatively easy to implement and use.
- Corey
| |