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

[Peter Zijlstra <peterz@infradead.org>]

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.