linux-kernel - Re: [PATCH 5/9] perf: Adding sysfs group format attribute for pmu device

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <4F231256.8080905@linux.vnet.ibm.com>
Date:	Fri, 27 Jan 2012 13:08:38 -0800
From:	Corey Ashford <cjashfor@...ux.vnet.ibm.com>
To:	Jiri Olsa <jolsa@...hat.com>
CC:	acme@...hat.com, a.p.zijlstra@...llo.nl, mingo@...e.hu,
	paulus@...ba.org, linux-kernel@...r.kernel.org
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

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