linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [BUG] perf parser: does not support arbitrary new sysfs events
@ 2012-10-26 20:23 Stephane Eranian
  2012-10-27 20:34 ` Jiri Olsa
  2012-10-29  1:50 ` Andi Kleen
  0 siblings, 2 replies; 6+ messages in thread
From: Stephane Eranian @ 2012-10-26 20:23 UTC (permalink / raw)
  To: LKML; +Cc: Jiri Olsa, Peter Zijlstra, mingo, Namhyung Kim

Hi,

The latest round of perf parser changes broke my PEBS-LL patch series
(at the last minute). For PEBS-LL, I need to add to generic events but I want
to keep them PMU specific. As such, they need to live in the sysfs events
subdir: /sys/devices/cpu/events/mem-loads, sys/devices/cpu/events/mem-stores.

Given your latest rounds of sysfs event changes, I had to modify my kernel
patches to fit those two new events within your perf_pmu_events_attr tables.

But now, when I try to do:

$ perf record -e cpu/mem-loads/ ....

I get unsupported event. Looks at the syscall trace, it seems perf does not even
look into the sysfs subdir to find a possible match. I don't
understand that. What's
the point of sysfs event list if it is not used or cannot be extended?

Note that when I explicitly pass the content of the sysfs file to perf
record, it
works:

$ perf record -e cpu/event=0xcd,umask=0x1,ldlat=3/ ......

So this is clearly a problem with the lookup in sysfs.

Also if you have the mappings exposed now in sysfs, why keep the hardcoded
generic events as well? Or why have those events hardcoded in the parser as
well.

I don't understand all this parser code. I  get the feeling it is
getting a bit out of
hands already. But now, I am stuck. So could you fix my parser problem ASAP?

Thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] perf parser: does not support arbitrary new sysfs events
  2012-10-26 20:23 [BUG] perf parser: does not support arbitrary new sysfs events Stephane Eranian
@ 2012-10-27 20:34 ` Jiri Olsa
  2012-10-27 23:13   ` Stephane Eranian
  2012-10-29  1:50 ` Andi Kleen
  1 sibling, 1 reply; 6+ messages in thread
From: Jiri Olsa @ 2012-10-27 20:34 UTC (permalink / raw)
  To: Stephane Eranian; +Cc: LKML, Peter Zijlstra, mingo, Namhyung Kim

On Fri, Oct 26, 2012 at 10:23:09PM +0200, Stephane Eranian wrote:
> Hi,
> 
> The latest round of perf parser changes broke my PEBS-LL patch series
> (at the last minute). For PEBS-LL, I need to add to generic events but I want
> to keep them PMU specific. As such, they need to live in the sysfs events
> subdir: /sys/devices/cpu/events/mem-loads, sys/devices/cpu/events/mem-stores.
> 
> Given your latest rounds of sysfs event changes, I had to modify my kernel
> patches to fit those two new events within your perf_pmu_events_attr tables.
> 
> But now, when I try to do:
> 
> $ perf record -e cpu/mem-loads/ ....

I can try this only on on uncore events and hw events aliases and that seems to work

> 
> I get unsupported event. Looks at the syscall trace, it seems perf does not even
> look into the sysfs subdir to find a possible match. I don't
> understand that. What's
> the point of sysfs event list if it is not used or cannot be extended?
> 
> Note that when I explicitly pass the content of the sysfs file to perf
> record, it
> works:
> 
> $ perf record -e cpu/event=0xcd,umask=0x1,ldlat=3/ ......
> 
> So this is clearly a problem with the lookup in sysfs.
> 
> Also if you have the mappings exposed now in sysfs, why keep the hardcoded
> generic events as well? Or why have those events hardcoded in the parser as
> well.

having perf work on old kernels

> 
> I don't understand all this parser code. I  get the feeling it is
> getting a bit out of
> hands already. But now, I am stuck. So could you fix my parser problem ASAP?

yep, but need more details.. related patches would help

jirka

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] perf parser: does not support arbitrary new sysfs events
  2012-10-27 20:34 ` Jiri Olsa
@ 2012-10-27 23:13   ` Stephane Eranian
  2012-10-27 23:47     ` Jiri Olsa
  0 siblings, 1 reply; 6+ messages in thread
From: Stephane Eranian @ 2012-10-27 23:13 UTC (permalink / raw)
  To: Jiri Olsa; +Cc: LKML, Peter Zijlstra, mingo, Namhyung Kim

On Sat, Oct 27, 2012 at 10:34 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> On Fri, Oct 26, 2012 at 10:23:09PM +0200, Stephane Eranian wrote:
>> Hi,
>>
>> The latest round of perf parser changes broke my PEBS-LL patch series
>> (at the last minute). For PEBS-LL, I need to add to generic events but I want
>> to keep them PMU specific. As such, they need to live in the sysfs events
>> subdir: /sys/devices/cpu/events/mem-loads, sys/devices/cpu/events/mem-stores.
>>
>> Given your latest rounds of sysfs event changes, I had to modify my kernel
>> patches to fit those two new events within your perf_pmu_events_attr tables.
>>
>> But now, when I try to do:
>>
>> $ perf record -e cpu/mem-loads/ ....
>
> I can try this only on on uncore events and hw events aliases and that seems to work
>
I know it works there. I don't understand why it does not work with cpu/.

Just add an encoding that has no hardcoded equivalent. I bet you will
reproduce the problem.

In my patch set, I have extended your perf_pmu_events_attr struct to
also accept an already preformed event_str. I can send you that extension
if you want.


>>
>> I get unsupported event. Looks at the syscall trace, it seems perf does not even
>> look into the sysfs subdir to find a possible match. I don't
>> understand that. What's
>> the point of sysfs event list if it is not used or cannot be extended?
>>
>> Note that when I explicitly pass the content of the sysfs file to perf
>> record, it
>> works:
>>
>> $ perf record -e cpu/event=0xcd,umask=0x1,ldlat=3/ ......
>>
>> So this is clearly a problem with the lookup in sysfs.
>>
>> Also if you have the mappings exposed now in sysfs, why keep the hardcoded
>> generic events as well? Or why have those events hardcoded in the parser as
>> well.
>
> having perf work on old kernels
>
>>
>> I don't understand all this parser code. I  get the feeling it is
>> getting a bit out of
>> hands already. But now, I am stuck. So could you fix my parser problem ASAP?
>
> yep, but need more details.. related patches would help
>
> jirka

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] perf parser: does not support arbitrary new sysfs events
  2012-10-27 23:13   ` Stephane Eranian
@ 2012-10-27 23:47     ` Jiri Olsa
  0 siblings, 0 replies; 6+ messages in thread
From: Jiri Olsa @ 2012-10-27 23:47 UTC (permalink / raw)
  To: Stephane Eranian; +Cc: LKML, Peter Zijlstra, mingo, Namhyung Kim

On Sun, Oct 28, 2012 at 01:13:59AM +0200, Stephane Eranian wrote:
> On Sat, Oct 27, 2012 at 10:34 PM, Jiri Olsa <jolsa@redhat.com> wrote:
> > On Fri, Oct 26, 2012 at 10:23:09PM +0200, Stephane Eranian wrote:
> >> Hi,
> >>
> >> The latest round of perf parser changes broke my PEBS-LL patch series
> >> (at the last minute). For PEBS-LL, I need to add to generic events but I want
> >> to keep them PMU specific. As such, they need to live in the sysfs events
> >> subdir: /sys/devices/cpu/events/mem-loads, sys/devices/cpu/events/mem-stores.
> >>
> >> Given your latest rounds of sysfs event changes, I had to modify my kernel
> >> patches to fit those two new events within your perf_pmu_events_attr tables.
> >>
> >> But now, when I try to do:
> >>
> >> $ perf record -e cpu/mem-loads/ ....
> >
> > I can try this only on on uncore events and hw events aliases and that seems to work
> >
> I know it works there. I don't understand why it does not work with cpu/.
> 
> Just add an encoding that has no hardcoded equivalent. I bet you will
> reproduce the problem.
> 
> In my patch set, I have extended your perf_pmu_events_attr struct to
> also accept an already preformed event_str. I can send you that extension
> if you want.

that would be great

jirka

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] perf parser: does not support arbitrary new sysfs events
  2012-10-26 20:23 [BUG] perf parser: does not support arbitrary new sysfs events Stephane Eranian
  2012-10-27 20:34 ` Jiri Olsa
@ 2012-10-29  1:50 ` Andi Kleen
  2012-10-29  9:43   ` Stephane Eranian
  1 sibling, 1 reply; 6+ messages in thread
From: Andi Kleen @ 2012-10-29  1:50 UTC (permalink / raw)
  To: Stephane Eranian; +Cc: LKML, Jiri Olsa, Peter Zijlstra, mingo, Namhyung Kim

Stephane Eranian <eranian@google.com> writes:

> Hi,
>
> The latest round of perf parser changes broke my PEBS-LL patch series
> (at the last minute). For PEBS-LL, I need to add to generic events but I want
> to keep them PMU specific. As such, they need to live in the sysfs events
> subdir: /sys/devices/cpu/events/mem-loads, sys/devices/cpu/events/mem-stores.
>
> Given your latest rounds of sysfs event changes, I had to modify my kernel
> patches to fit those two new events within your perf_pmu_events_attr tables.
>
> But now, when I try to do:
>
> $ perf record -e cpu/mem-loads/ ....

- is not supported in an event name. I fixed this in my patchkit

Yes the sysfs stuff in general is quite fragile.

-Andi

-- 
ak@linux.intel.com -- Speaking for myself only

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [BUG] perf parser: does not support arbitrary new sysfs events
  2012-10-29  1:50 ` Andi Kleen
@ 2012-10-29  9:43   ` Stephane Eranian
  0 siblings, 0 replies; 6+ messages in thread
From: Stephane Eranian @ 2012-10-29  9:43 UTC (permalink / raw)
  To: Andi Kleen; +Cc: LKML, Jiri Olsa, Peter Zijlstra, mingo, Namhyung Kim

On Mon, Oct 29, 2012 at 2:50 AM, Andi Kleen <andi@firstfloor.org> wrote:
> Stephane Eranian <eranian@google.com> writes:
>
>> Hi,
>>
>> The latest round of perf parser changes broke my PEBS-LL patch series
>> (at the last minute). For PEBS-LL, I need to add to generic events but I want
>> to keep them PMU specific. As such, they need to live in the sysfs events
>> subdir: /sys/devices/cpu/events/mem-loads, sys/devices/cpu/events/mem-stores.
>>
>> Given your latest rounds of sysfs event changes, I had to modify my kernel
>> patches to fit those two new events within your perf_pmu_events_attr tables.
>>
>> But now, when I try to do:
>>
>> $ perf record -e cpu/mem-loads/ ....
>
> - is not supported in an event name. I fixed this in my patchkit
>
Yes, this was indeed the problem. Glad you tracked it down.

We need this patch ASAP.

> Yes the sysfs stuff in general is quite fragile.
>
I would say complicated rather than fragile.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-10-29  9:44 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-10-26 20:23 [BUG] perf parser: does not support arbitrary new sysfs events Stephane Eranian
2012-10-27 20:34 ` Jiri Olsa
2012-10-27 23:13   ` Stephane Eranian
2012-10-27 23:47     ` Jiri Olsa
2012-10-29  1:50 ` Andi Kleen
2012-10-29  9:43   ` Stephane Eranian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).