2012-02-01 13:36:39

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 5/9] perf: Adding sysfs group format attribute for pmu device

On Tue, 2012-01-31 at 17:25 -0800, Corey Ashford wrote:
>
> One other comment that occurs to me is that it would be nice for perf to
> know when a supplied value is out of range, or will have undefined
> results. For example, a field may be 8 bits wide, but not all 8-bit
> values are legal. For example, there may be 208 events, and the codes
> may be somehwhat or even very sparsely encoded. So, ideally, a config
> field in sysfs might look like this:
>
> config1:0-7:0x0-0xd8,0xdb-0xe2,0xe4-0xe6
>
> This way perf could check for valid values before stuffing them into
> registers, and give a good error message to the user. If there is no
> restriction field, it would be assumed all of the possible values are valid.
>
> I think the kernel code needs to check for bad values as well, because
> people can bypass the restrictions exposed by sysfs and use the
> perf_events API directly.

Define bad? The only case where perf cares (from a kernel pov) is when
it can make the hardware melt and similar things. Programming the PMU in
a non-nonsensical but non-destructive way is fine, you get what you ask
for.



2012-02-02 00:33:57

by Corey Ashford

[permalink] [raw]
Subject: Re: [PATCH 5/9] perf: Adding sysfs group format attribute for pmu device

On 02/01/2012 05:36 AM, Peter Zijlstra wrote:
> On Tue, 2012-01-31 at 17:25 -0800, Corey Ashford wrote:
>>
>> One other comment that occurs to me is that it would be nice for perf to
>> know when a supplied value is out of range, or will have undefined
>> results. For example, a field may be 8 bits wide, but not all 8-bit
>> values are legal. For example, there may be 208 events, and the codes
>> may be somehwhat or even very sparsely encoded. So, ideally, a config
>> field in sysfs might look like this:
>>
>> config1:0-7:0x0-0xd8,0xdb-0xe2,0xe4-0xe6
>>
>> This way perf could check for valid values before stuffing them into
>> registers, and give a good error message to the user. If there is no
>> restriction field, it would be assumed all of the possible values are valid.
>>
>> I think the kernel code needs to check for bad values as well, because
>> people can bypass the restrictions exposed by sysfs and use the
>> perf_events API directly.
>
> Define bad? The only case where perf cares (from a kernel pov) is when
> it can make the hardware melt and similar things. Programming the PMU in
> a non-nonsensical but non-destructive way is fine, you get what you ask
> for.

The main definition of "bad" would be a known config that has no meaning
(or perhaps is improperly implemented on the chip). This would just
give the user some extra help that he might have incorrectly configured
the PMU. Perf could identify down to the specific field that may not be
set correctly. You could add an override switch to perf (--force-config
etc.) that would allow the user to bypass the check if he really wanted
to use that config.

It's also conceivable that there would be known PMU modes, that let's
say, stores values into a memory buffer (like IBS, etc), that if you use
a particular config, it would overwhelm the memory bus and choke the
processor down.

That said, this feature would just be nice to have, and is only on my
wish list.

Thanks for considering comments,

- Corey