Messages in this thread Patch in this message | | | Date | Mon, 31 Aug 2009 19:41:08 +0200 | From | Frederic Weisbecker <> | Subject | Re: [PATCH] tracing/filters: Defer pred allocation |
| |
On Mon, Aug 31, 2009 at 04:49:41PM +0800, Li Zefan wrote: > @@ -1094,6 +1118,10 @@ int apply_event_filter(struct ftrace_event_call *call, char *filter_string) > > mutex_lock(&event_mutex); > > + err = init_preds(call); > + if (err) > + goto out_unlock;
Hmm, but what happens if the filter already has its preds initialized by a previous filter?
The first thing that init_preds() does is:
filter = call->filter = kzalloc(sizeof(*filter), GFP_KERNEL);
That looks like a memory leak.
> if (!strcmp(strstrip(filter_string), "0")) { > filter_disable_preds(call); > remove_filter_string(call->filter); > @@ -1139,6 +1167,10 @@ int apply_subsystem_event_filter(struct event_subsystem *system, > > mutex_lock(&event_mutex); > > + err = init_subsystem_preds(system);
Whereas this one has a check: if (!call->filter) alloc...
> + if (err) > + goto out_unlock; > + > if (!strcmp(strstrip(filter_string), "0")) { > filter_free_subsystem_preds(system, FILTER_DISABLE_ALL); > remove_filter_string(system->filter); > diff --git a/kernel/trace/trace_export.c b/kernel/trace/trace_export.c > index 029a91f..df1bf6e 100644 > --- a/kernel/trace/trace_export.c > +++ b/kernel/trace/trace_export.c > @@ -135,7 +135,6 @@ __attribute__((section("_ftrace_events"))) event_##call = { \ > static int ftrace_raw_init_event_##call(void) \ > { \ > INIT_LIST_HEAD(&event_##call.fields); \ > - init_preds(&event_##call); \ > return 0; \ > } \ > > -- > 1.6.3 >
Other than the above doubts, that looks good.
| |