Re: [PATCH] perf_counter: extensible perf_counter_attr

[Ingo Molnar <mingo@elte.hu>]

* 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. ]

> 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.

	Ingo