2012-10-26 20:23:13

by Stephane Eranian

[permalink] [raw]
Subject: [BUG] perf parser: does not support arbitrary new sysfs events

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.


2012-10-27 20:34:45

by Jiri Olsa

[permalink] [raw]
Subject: Re: [BUG] perf parser: does not support arbitrary new sysfs events

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

2012-10-27 23:14:03

by Stephane Eranian

[permalink] [raw]
Subject: Re: [BUG] perf parser: does not support arbitrary new sysfs events

On Sat, Oct 27, 2012 at 10:34 PM, Jiri Olsa <[email protected]> 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

2012-10-27 23:47:48

by Jiri Olsa

[permalink] [raw]
Subject: Re: [BUG] perf parser: does not support arbitrary new sysfs events

On Sun, Oct 28, 2012 at 01:13:59AM +0200, Stephane Eranian wrote:
> On Sat, Oct 27, 2012 at 10:34 PM, Jiri Olsa <[email protected]> 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

2012-10-29 01:50:37

by Andi Kleen

[permalink] [raw]
Subject: Re: [BUG] perf parser: does not support arbitrary new sysfs events

Stephane Eranian <[email protected]> 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

--
[email protected] -- Speaking for myself only

2012-10-29 09:44:00

by Stephane Eranian

[permalink] [raw]
Subject: Re: [BUG] perf parser: does not support arbitrary new sysfs events

On Mon, Oct 29, 2012 at 2:50 AM, Andi Kleen <[email protected]> wrote:
> Stephane Eranian <[email protected]> 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.