Re: [PATCH v2] tracing/filters: Improve subsystem filter

[Tom Zanussi <tzanussi@gmail.com>]

On Tue, 2009-07-21 at 14:35 +0800, Li Zefan wrote:
> Tom Zanussi wrote:
> > Hi,
> > 
> > On Mon, 2009-07-20 at 10:20 +0800, Li Zefan wrote:
> >> Currently a subsystem filter should be applicable to all events
> >> under the subsystem, and if it failed, all the event filters
> >> will be cleared. Those behaviors make subsys filter much less
> >> useful:
> >>
> >>   # echo 'vec == 1' > irq/softirq_entry/filter
> >>   # echo 'irq == 5' > irq/filter
> >>   bash: echo: write error: Invalid argument
> >>   # cat irq/softirq_entry/filter
> >>   none
> >>
> >> I'd expect it set the filter for irq_handler_entry/exit, and
> >> not touch softirq_entry/exit.
> >>
> >> The basic idea is, try to see if the filter can be applied
> >> to which events, and then just apply to the those events:
> >>
> >>   # echo 'vec == 1' > softirq_entry/filter
> >>   # echo 'irq == 5' > filter
> >>   # cat irq_handler_entry/filter
> >>   irq == 5
> >>   # cat softirq_entry/filter
> >>   vec == 1
> >>
> > 
> > It looks like it accomplishes the goal, but I wonder if you could
> > simplify it by getting rid of the no_reset flag and all the state-saving
> > while keeping the dry_run param on filter_add_pred to use from
> > filter_add_subsysystem_pred() e.g. something like this:
> > 
> > filter_add_subsystem_pred(...)
> > {
> > 	...
> > 	list_for_each_entry(call, &ftrace_events, list) {
> > 		err = filter_add_pred(ps, call, pred, dry_run = 1);
> > 		if (!err)
> > 			filter_add_pred(ps, call, pred, dry_run = 0);
> 
> This is the same with just call "filter_add_pred(ps, call, pred)"
> 
> > 		...
> > 	}
> > }
> > 
> > IIRC the state-saving was necessary if the idea was to back out of (or
> > avoid setting) all the filters if one failed, but the new model is to
> > set whichever apply while leaving the rest alone.  I think the simpler
> > approach above accomplishes that, but I may be missing something or have
> > forgotten the original motivation for doing it this way.
> > 
> 
> This won't work as expected. The state-saving is necessary, because
> if any pred is not appliable for an event, the whole operation should
> fail for that event.
> 
> What this patch does is apply the filter to those events that the
> filter is appliable, but not apply parts of the filter to each event.
> 

Yeah, you're right - I was confusing preds with filters when reading
this.

Acked-by: Tom Zanussi <tzanussi@gmail.com>


> Imaging the result for those filters with the above code:
> 
>   # echo 'irq == 5 && vec == 1' > filter
>   # echo 'irq == 5 || vec == 1' > filter
>   # echo 'irq == 5 || foo == bar' > fitler
>   # echo '(irq == 5 && vec == 1) || irq == 2' > filter
>