Messages in this thread | | | Subject | Re: [PATCH v2] tracing/filters: Improve subsystem filter | From | Tom Zanussi <> | Date | Tue, 21 Jul 2009 02:22:03 -0500 |
| |
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 >
| |