lkml.org 
[lkml]   [2011]   [Nov]   [2]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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
>
>
>


\
 
 \ /
  Last update: 2011-11-03 02:27    [W:0.092 / U:1.056 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site