Messages in this thread | | | Date | Thu, 03 Nov 2011 09:25:51 +0800 | From | Li Zefan <> | Subject | Re: [PATCH] tracing: Restore system filter behavior |
| |
Steven Rostedt wrote: > On Wed, 2011-11-02 at 14:51 -0400, Steven Rostedt wrote: >> On Wed, 2011-11-02 at 14:22 -0400, Steven Rostedt wrote: >> >>> The above shows how things are just ambiguous. I have no problem in >>> using the top filter to set multiple events. But the top filter should >>> not keep the state of what was set. Perhaps just have system event >>> filters always show default text. Like: >>> >>> # cat /debug/tracing/events/sched/filter >>> ### global filter ### >>> # Use this to set multiple event filters >>> # Only affects events that have the event fields specified in the filter >>> >>> >>> I'll add your patch, but are you OK with the above always printing for >>> system event filters? Just to remove the ambiguous state. >> >> >> As the filter is also used to show errors, I'll only have it print the >> "message" if the filter worked. If there's an error, then the error >> message will display instead. >> > > What's your thought on the below patch? >
I'm fine with this idea.
a comment below..
> -- Steve > > diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c > index 86040d9..6dee2b5 100644 > --- a/kernel/trace/trace_events_filter.c > +++ b/kernel/trace/trace_events_filter.c > @@ -27,6 +27,12 @@ > #include "trace.h" > #include "trace_output.h" > > +#define DEFAULT_SYS_FILTER_MESSAGE \ > + "### global filter ###\n" \ > + "# Use this to set filters for multiple events.\n" \ > + "# Only events with the given fields will be affected.\n" \ > + "# If no events are modified, an error message will be displayed here" > + > enum filter_op_ids > { > OP_OR, > @@ -646,7 +652,7 @@ void print_subsystem_event_filter(struct event_subsystem *system, > if (filter && filter->filter_string) > trace_seq_printf(s, "%s\n", filter->filter_string); > else > - trace_seq_printf(s, "none\n"); > + trace_seq_printf(s, DEFAULT_SYS_FILTER_MESSAGE "\n"); > mutex_unlock(&event_mutex); > } > > @@ -1838,7 +1844,8 @@ int apply_subsystem_event_filter(struct event_subsystem *system, > if (!filter) > goto out; > > - replace_filter_string(filter, filter_string); > + /* System filters just show a default message */ > + replace_filter_string(filter, DEFAULT_SYS_FILTER_MESSAGE);
Seems we can call remove_filter_string() instead to save memory.
> /* > * No event actually uses the system filter > * we can free it without synchronize_sched(). > @@ -1848,14 +1855,12 @@ int apply_subsystem_event_filter(struct event_subsystem *system, > > parse_init(ps, filter_ops, filter_string); > err = filter_parse(ps); > - if (err) { > - append_filter_err(ps, system->filter); > - goto out; > - } > + if (err) > + goto err_filter; > > err = replace_system_preds(system, ps, filter_string); > if (err) > - append_filter_err(ps, system->filter); > + goto err_filter; > > out: > filter_opstack_clear(ps); > @@ -1865,6 +1870,11 @@ out_unlock: > mutex_unlock(&event_mutex); > > return err; > + > +err_filter: > + replace_filter_string(filter, filter_string); > + append_filter_err(ps, system->filter); > + goto out; > } > > #ifdef CONFIG_PERF_EVENTS > > >
| |