Re: [PATCH] perf_counter: extensible perf_counter_attr

Corey Ashford
2009-06-09T08:02:45Z

Ingo Molnar wrote:
> * Corey Ashford <cjashfor@linux.vnet.ibm.com> wrote:
>
>
>> If I understand you correctly, you would simply make
>> perf_counter_attr larger every time you want to add a new
>> attribute. Users using the new attributes would call
>> sys_perf_counter_open with a larger attr_size value.
>>
>
> Yes, exactly. Basically ABIs in Linux only get extended (never
> shrunk and never changed) so it's not like we ever want to (or can)
> shrink the size of the structure or change its semantics.
>
> Each future extension gives the structure a new, unique size - which
> also acts as an 'ABI version' identifier, in a pretty robust way. We
> check this 'ABI version' (the structure size) in the kernel code so
> it's not just a passive 'version field' thing.
>
> Here are the various compatibility variations:
>
> - same-version kernel and user-space: they both use the same
> attr_size value and support the full set of features.
>
> - old user-space running on new kernel: works fine, as the kernel
> will do a short copy and zero out the remaining attributes.
>
> - new user-space running on old kernel: the kernel returns -ENOTSUP
> and user-space has a choice to refuse to run cleanly - or, if an
> old ABI version is widespread, might chose to utilize the old,
> smaller attribute structure size field (at the cost of not using
> new attribute features, obviously).
>
> ( Additional detail: in the size mismatch failure case the kernel
> should write back the supported size into attr_size, so that
> user-space knows which precise ABI variant it deals with on the
> kernel side. )
>
> This kind of ABI maintenance method has a number of substantial
> advantages:
>
> - It is very compatible (see above)
>
> - It is extensible easily and in an unlimited way - we just extend
> the structure size.
>
> - It is very clean on the kernel side and the user side as well,
> because we just have a single attribute structure.
>
> - It makes the ABI 'version' field an _active_ component of
> functionality - so there is no way for subtle breakages to slip
> in.
>
> - New attributes are prime-time members of the attribute structure,
> not second-class citizens that first have to be read in via an
> elaborate chaining mechanism at extra cost.
>
> [ If only all our syscall ABIs used this technique :-) It would be
> so much easier to extend syscalls cleanly - without having to go
> through the expensive and time-consuming process to add new
> syscalls. ]
>
Thanks for the detailed explanation :)

>> What about arch-dependent attributes? Would you want to place
>> them all in the perf_counter_attr struct? I suppose this could be
>> done by #include'ing an arch-specific .h file.
>>
>
> What arch-dependent attributes are you thinking about? In the
> perfcounters subsystem we want to support PMU and other performance
> analysis features in a way that makes it possible for all
> architectures to make use of them.
>
> So 'arch dependent attributes' per se are bad and against the
> perfcounters design. "Generic perfcounter feature only supported by
> a single architecture initially" is better.
>
Well, I think Intel has PEBS and AMD has some similar mechanism. I
would guess that at some point you would want to provide access to those
PMU features via these attributes. Since these mechanisms are very
chip-specific, I don't think you would want to try to create an
arch-independent interface to them. There may be future mechanisms that
only make sense on one particular chip design, and would therefore not
be a candidate for wider use, but would still make sense to provide some
support for that mechanism via the attributes.

Did you have some different plan for PEBS (etc.) ?

- Corey