Re: [PATCH v0 0/5] perf: Introduce instruction trace filtering

[Alexander Shishkin <alexander.shishkin@linux.intel.com>]

Mathieu Poirier <mathieu.poirier@linaro.org> writes:

> On 11 December 2015 at 06:36, Alexander Shishkin
> <alexander.shishkin@linux.intel.com> wrote:
> Alex, Peter and al,
>
> As I mentioned in a previous reply I think this patchset is aiming in
> the right direction.  Here we are dealing with address range
> filtering, something that is common to both IntelPT and CS, but what
> happens when we want introduce options that aren't generic to all
> tracers and still want to us the ioctl method?

Are we still talking about filtering options or more like event
configuration? First, we need to understand if one particular feature or
an option should be enabled for the entire lifetime of an event or if it
should be configurable on the fly. For the former, you can use
attr.config and friends, for the latter it makes sense to use an ioctl.

If we're still talking about address range filtering, there is one
difference in coresight that I'm aware of, which is specifying
individual addresses as start/stop triggers (as opposed to enable
ranges) and that is already taken care of in the current parser, I
should probably write a comment to make it more apparent.

So my approach was to not consider them to be architecture specific
features, but simply features that either are or aren't supported by a
particular pmu.

> Can we make the current scheme more extensible or generic so that
> adding more architecture specific option is easily feasible?

Well, the bigger question is, how do you represent a very architecture
specific option in the core structures. I have one solution to that that
is described above. It shouldn't take much architecture-specific code to
handle each new option, unless it something that really only makes sense
for one architecture/pmu.

All that said, one could still extend the current code quite easily to
fit completely non-generic things. In the default clause, in the parser
function, one could add:

  if (state == IF_STATE_ACTION)
      if (event->pmu->itrace_filter_parse(event, &filter))
          goto fail;

if one really had to.

Regards,
--
Alex