Re: [PATCH 1/8] ARC: perf: support RAW events

[Peter Zijlstra]

On Wed, Jun 17, 2015 at 07:40:37PM +0530, Vineet Gupta wrote:
> > I would much prefer the raw thing to remain a number; it puts you in
> > pretty much the same place as most other archs, including x86.
> 
> Sure, but that doesn't mean we start using the index of event as a raw ID. There
> are zillion ways to configure cores and the index will certainly start varying.
> Which means that user has to determine at run time (using whatever /proc/xxx) what
> crun corresponds to. Why inflict such pain on poor guy.
> 
> The current raw event, despite representing an ASCII string is still a u64 number.
> So it is not more "raw" than what others have. We can get rid of the swapping
> business when setting up a raw event, by making sure that cached values from probe
> are already in same format.
> 
> But using index as a event id is no go sorry...

Well, you're still stuck converting an 8 char ASCII string to hex, which
arguably isn't too hard, but still rather ugly.

Furthermore, the 'raw' interface is what is used to pass in hardware
configuration otherwise not exposed. If all you currently have is event
idx that's not too interesting.

However, your patch 6 already introduces event modifiers:

+#define ARC_REG_PCT_CONFIG_USER        (1 << 18)       /* count in user mode */
+#define ARC_REG_PCT_CONFIG_KERN        (1 << 19)       /* count in kernel mode */

Those should also be available through the raw interface.

That is, your perf_event_attr::config should pretty much end up in your
hardware unmodified -- with the obvious exception of things like
'interrupt enable' because we want the kernel to be in control of that.

So if you 'overload' your TYPE_RAW to mean something else entirely, you
also loose the ability to specify event modifiers not otherwise modelled
in perf.


There are things you could do though; you could for instance interpret a
!0 perf_event_attr::config1 as the name field and replace the
perf_event_attr::config's event idx with whatever comes out of that
name->idx map -- and/or require perf_event_attr::config's event idx to
be 0 in that case.

This of course assumes there will never be an other valid use for
::config1.

Alternatively you can register a second pmu "cpu_name" and interpret its
::config as the name.


Also, ideally you'd convince acme/jolsa to extend the event parser with
this 8-char ASCII format so you can do all the text munging in userspace
and not have to do the ugly you propose here.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/