Re: [BUG] perf stat: useless output for raw events with new event parser

[Robert Richter <robert.richter@amd.com>]

On 26.04.12 17:39:32, Peter Zijlstra wrote:
> On Thu, 2012-04-26 at 16:45 +0200, Robert Richter wrote:
> > It is totally ok to have parser support for this. I simply do not see
> > why we need to put the encoding into sysfs. We somehow know on which
> > hardware we run and the parser should already know how to setup the
> > syscall. So parsing the above finally ends in calling of something
> > like:
> > 
> >  setup_event_for_some_pmu(event, 0x4e2, 0xf8);
> > 
> > We don't need any description of bit masks in sysfs for this. 
> 
> Its the kernel side decoding perf_event_attr, so it seems sensible to
> also describe this encoding from the kernel.

For perfctr we have:

 /sys/bus/event_source/devices/cpu/format/inv: config:23
 /sys/bus/event_source/devices/cpu/format/edge: config:18
 /sys/bus/event_source/devices/cpu/format/cmask: config:24-31
 /sys/bus/event_source/devices/cpu/format/event: config:0-7,32-35
 /sys/bus/event_source/devices/cpu/format/umask: config:8-15

The kernel does not en- or decode anything in the config value. It is
directly passed to the pmu with some validation of the values.
Everything else is in userland since it composes the syscall.

The kernel must now contain code like this:

 PMU_FORMAT_ATTR(event,  "config:0-7,32-35");
 PMU_FORMAT_ATTR(umask,  "config:8-15"   );
 PMU_FORMAT_ATTR(edge,   "config:18"     );
 PMU_FORMAT_ATTR(inv,    "config:23"     );
 PMU_FORMAT_ATTR(cmask,  "config:24-31"  );

Which is unrelated to anything else, duplicates the effort to maintain
bit masks and thus is more error-prone. Besides this there is no need
for it because the values are fix and do not change. We simply know
the format of the config value already, so the format entries are of
no use.

One could argue that feeding a generic pmu setup with the format
configuration reduces the need to modify userland, we have same code
for various archs. But if I have the choice I rather update my perf
tool chain than rebooting the kernel to update perf.

> Currently we mostly match the hardware encoding, but there's no strict
> requirement to do so, we can already see some of that with the extra_reg
> stuff, perf_event_attr::config1 can mean different things depending on
> the event.

Of course the config values of the syscall could be translated into a
different hardware configuration. But its layout is always spec'ed
somewhere and needs no description in sysfs.

> Keeping all this information in two places just seems like asking for it
> to get out of sync.

All this could reside in userland at one place too. Also, the syscall
definition is sufficient as interface description and both sides must
handle any differences of kernel or userland implementations.

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center