Messages in this thread | | | From | Alexander Shishkin <> | Subject | Re: [PATCH v0 3/5] perf: Introduce instruction trace filtering | Date | Fri, 11 Dec 2015 17:12:47 +0200 |
| |
Peter Zijlstra <peterz@infradead.org> writes:
> On Fri, Dec 11, 2015 at 02:23:36PM +0000, Mark Rutland wrote: >> On Fri, Dec 11, 2015 at 03:02:01PM +0100, Peter Zijlstra wrote: >> > On Fri, Dec 11, 2015 at 03:36:36PM +0200, Alexander Shishkin wrote: >> > >> > > The pmu driver interface basically adds an extra callback to the >> > > pmu driver structure, which validates the filter configuration proposed >> > > by the user against what the hardware is actually capable of doing >> > > and translates it into something that pmu::start can program into >> > > hardware. >> > >> > > @@ -388,12 +393,38 @@ struct pmu { >> > > void (*free_aux) (void *aux); /* optional */ >> > > >> > > /* >> > > + * Validate instruction tracing filters: make sure hw supports the >> > > + * requested configuration and number of filters. >> > > + * >> > > + * Configure instruction tracing filters: translate hw-agnostic filter >> > > + * into hardware configuration in event::hw::itrace_filters >> > > + */ >> > > + int (*itrace_filter_setup) (struct perf_event *event); /* optional */ >> > > + >> > > + /* >> > > * Filter events for PMU-specific reasons. >> > > */ >> > > int (*filter_match) (struct perf_event *event); /* optional */ >> > > }; >> > >> > Any reason you cannot use pmu::filter_match ? >> >> Maybe I've misunderstood your point, but the two seem quite different. >> >> We introduced pmu::filter_match to apply a SW filter each time we >> installed events from a context. We use that on ARM to avoid programming >> big events into little cores and vice-versa. >> >> As far as I can see, itrace_filter_setup is closer in operation to >> event_init. It can fail at configuration time (long before scheduling >> events to cores), and leaves the actual filtering to the HW. > > Ah indeed. I got confused by the similarities in the signature and both > having filter in the name. > > Alexander, maybe then add a filter parameter to validate to the > signature, otherwise you have to change the event state in order to > validate it which leaves you with an invalid configured event in case it > doesn't validate.
The validation part also checks that we have not more filters than hardware supports, so this callback walks through the list and counts them up, iow, it looks at all the filters in one go.
In case validation fails (and it can only fail in the ioctl() path), the event itself is still in a valid state though, we just decline the filters supplied by the user.
I could split this callback in two: one for validation and one for the stop/update_filters/start cadence, but I don't see if that helps much either.
Thanks, -- Alex
| |