[PATCH] tracing/filters: Defer pred allocation

[PATCH] tracing/filters: Defer pred allocation

[Li Zefan]

init_preds() allocates about 5392 bytes of memory (on x86_32) for
a TRACE_EVENT. With my config, at system boot total memory occupied
is:

	5392 * (642 + 15) == 3459KB

642 == cat available_events | wc -l
15 == number of dirs in events/ftrace

That's quite a lot, so we'd better defer memory allocation util
it's needed, that's when filter is used.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---

tracing/kprobe needs rebase after this patch..

---
 include/linux/ftrace_event.h       |    1 -
 include/linux/syscalls.h           |    2 -
 include/trace/ftrace.h             |    1 -
 kernel/trace/trace_events_filter.c |   50 +++++++++++++++++++++++++++++------
 kernel/trace/trace_export.c        |    1 -
 5 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index ace2da9..7554804 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -133,7 +133,6 @@ struct ftrace_event_call {
 #define MAX_FILTER_PRED		32
 #define MAX_FILTER_STR_VAL	128
 
-extern int init_preds(struct ftrace_event_call *call);
 extern void destroy_preds(struct ftrace_event_call *call);
 extern int filter_match_preds(struct ftrace_event_call *call, void *rec);
 extern int filter_current_check_discard(struct ftrace_event_call *call,
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index f124c89..a8e3782 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -177,7 +177,6 @@ static void prof_sysexit_disable_##sname(struct ftrace_event_call *event_call) \
 		event_enter_##sname.id = id;				\
 		set_syscall_enter_id(num, id);				\
 		INIT_LIST_HEAD(&event_enter_##sname.fields);		\
-		init_preds(&event_enter_##sname);			\
 		return 0;						\
 	}								\
 	TRACE_SYS_ENTER_PROFILE(sname);					\
@@ -214,7 +213,6 @@ static void prof_sysexit_disable_##sname(struct ftrace_event_call *event_call) \
 		event_exit_##sname.id = id;				\
 		set_syscall_exit_id(num, id);				\
 		INIT_LIST_HEAD(&event_exit_##sname.fields);		\
-		init_preds(&event_exit_##sname);			\
 		return 0;						\
 	}								\
 	TRACE_SYS_EXIT_PROFILE(sname);					\
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 57c56a9..bfbc842 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -622,7 +622,6 @@ static int ftrace_raw_init_event_##call(void)				\
 		return -ENODEV;						\
 	event_##call.id = id;						\
 	INIT_LIST_HEAD(&event_##call.fields);				\
-	init_preds(&event_##call);					\
 	return 0;							\
 }									\
 									\
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 9f03082..c6b2edf 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -309,7 +309,7 @@ void print_event_filter(struct ftrace_event_call *call, struct trace_seq *s)
 	struct event_filter *filter = call->filter;
 
 	mutex_lock(&event_mutex);
-	if (filter->filter_string)
+	if (filter && filter->filter_string)
 		trace_seq_printf(s, "%s\n", filter->filter_string);
 	else
 		trace_seq_printf(s, "none\n");
@@ -322,7 +322,7 @@ void print_subsystem_event_filter(struct event_subsystem *system,
 	struct event_filter *filter = system->filter;
 
 	mutex_lock(&event_mutex);
-	if (filter->filter_string)
+	if (filter && filter->filter_string)
 		trace_seq_printf(s, "%s\n", filter->filter_string);
 	else
 		trace_seq_printf(s, "none\n");
@@ -390,6 +390,9 @@ void destroy_preds(struct ftrace_event_call *call)
 	struct event_filter *filter = call->filter;
 	int i;
 
+	if (!filter)
+		return;
+
 	for (i = 0; i < MAX_FILTER_PRED; i++) {
 		if (filter->preds[i])
 			filter_free_pred(filter->preds[i]);
@@ -400,7 +403,7 @@ void destroy_preds(struct ftrace_event_call *call)
 	call->filter = NULL;
 }
 
-int init_preds(struct ftrace_event_call *call)
+static int init_preds(struct ftrace_event_call *call)
 {
 	struct event_filter *filter;
 	struct filter_pred *pred;
@@ -410,7 +413,6 @@ int init_preds(struct ftrace_event_call *call)
 	if (!call->filter)
 		return -ENOMEM;
 
-	call->filter_active = 0;
 	filter->n_preds = 0;
 
 	filter->preds = kzalloc(MAX_FILTER_PRED * sizeof(pred), GFP_KERNEL);
@@ -432,7 +434,28 @@ oom:
 
 	return -ENOMEM;
 }
-EXPORT_SYMBOL_GPL(init_preds);
+
+static int init_subsystem_preds(struct event_subsystem *system)
+{
+	struct ftrace_event_call *call;
+	int err;
+
+	list_for_each_entry(call, &ftrace_events, list) {
+		if (!call->define_fields)
+			continue;
+
+		if (strcmp(call->system, system->name) != 0)
+			continue;
+
+		if (!call->filter) {
+			err = init_preds(call);
+			if (err)
+				return err;
+		}
+	}
+
+	return 0;
+}
 
 enum {
 	FILTER_DISABLE_ALL,
@@ -449,6 +472,9 @@ static void filter_free_subsystem_preds(struct event_subsystem *system,
 		if (!call->define_fields)
 			continue;
 
+		if (strcmp(call->system, system->name) != 0)
+			continue;
+
 		if (flag == FILTER_INIT_NO_RESET) {
 			call->filter->no_reset = false;
 			continue;
@@ -457,10 +483,8 @@ static void filter_free_subsystem_preds(struct event_subsystem *system,
 		if (flag == FILTER_SKIP_NO_RESET && call->filter->no_reset)
 			continue;
 
-		if (!strcmp(call->system, system->name)) {
-			filter_disable_preds(call);
-			remove_filter_string(call->filter);
-		}
+		filter_disable_preds(call);
+		remove_filter_string(call->filter);
 	}
 }
 
@@ -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;
+
 	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);
+	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

Re: [PATCH] tracing/filters: Defer pred allocation

[Ingo Molnar]

* Li Zefan <lizf@cn.fujitsu.com> wrote:

> init_preds() allocates about 5392 bytes of memory (on x86_32) for
> a TRACE_EVENT. With my config, at system boot total memory occupied
> is:
> 
> 	5392 * (642 + 15) == 3459KB
> 
> 642 == cat available_events | wc -l
> 15 == number of dirs in events/ftrace
> 
> That's quite a lot, so we'd better defer memory allocation util
> it's needed, that's when filter is used.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

Applied, thanks!

> tracing/kprobe needs rebase after this patch..

You mean with many probes registered it has a lot of memory 
footprint? Instead of a rebase a merge of tracing/core into 
tracing/kprobes would be less intrusive.

	Ingo

Re: [PATCH] tracing/filters: Defer pred allocation

[Li Zefan]

Ingo Molnar wrote:
> * Li Zefan <lizf@cn.fujitsu.com> wrote:
> 
>> init_preds() allocates about 5392 bytes of memory (on x86_32) for
>> a TRACE_EVENT. With my config, at system boot total memory occupied
>> is:
>>
>> 	5392 * (642 + 15) == 3459KB
>>
>> 642 == cat available_events | wc -l
>> 15 == number of dirs in events/ftrace
>>
>> That's quite a lot, so we'd better defer memory allocation util
>> it's needed, that's when filter is used.
>>
>> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> 
> Applied, thanks!
> 
>> tracing/kprobe needs rebase after this patch..
> 
> You mean with many probes registered it has a lot of memory 

I think so, if filter is used.

> footprint? Instead of a rebase a merge of tracing/core into 
> tracing/kprobes would be less intrusive.
> 

Yeah, I meant this patch conflicts with some patches in
tracing/kprobe. :)

Re: [PATCH] tracing/filters: Defer pred allocation

[Masami Hiramatsu]

Li Zefan wrote:
> Ingo Molnar wrote:
>> * Li Zefan <lizf@cn.fujitsu.com> wrote:
>>
>>> init_preds() allocates about 5392 bytes of memory (on x86_32) for
>>> a TRACE_EVENT. With my config, at system boot total memory occupied
>>> is:
>>>
>>> 	5392 * (642 + 15) == 3459KB
>>>
>>> 642 == cat available_events | wc -l
>>> 15 == number of dirs in events/ftrace
>>>
>>> That's quite a lot, so we'd better defer memory allocation util
>>> it's needed, that's when filter is used.
>>>
>>> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
>>
>> Applied, thanks!
>>
>>> tracing/kprobe needs rebase after this patch..
>>
>> You mean with many probes registered it has a lot of memory 
> 
> I think so, if filter is used.
> 
>> footprint? Instead of a rebase a merge of tracing/core into 
>> tracing/kprobes would be less intrusive.
>>
> 
> Yeah, I meant this patch conflicts with some patches in
> tracing/kprobe. :)

Sure, that should be a small change. :-)

Thank you!

-- 
Masami Hiramatsu

Software Engineer
Hitachi Computer Products (America), Inc.
Software Solutions Division

e-mail: mhiramat@redhat.com

[tip:tracing/core] tracing/filters: Defer pred allocation

[tip-bot for Li Zefan]

Commit-ID:  8e254c1d183f0225ad21f9049641529e56cce4da
Gitweb:     http://git.kernel.org/tip/8e254c1d183f0225ad21f9049641529e56cce4da
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Mon, 31 Aug 2009 16:49:41 +0800
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Mon, 31 Aug 2009 10:58:08 +0200

tracing/filters: Defer pred allocation

init_preds() allocates about 5392 bytes of memory (on x86_32) for
a TRACE_EVENT. With my config, at system boot total memory occupied
is:

	5392 * (642 + 15) == 3459KB

642 == cat available_events | wc -l
15 == number of dirs in events/ftrace

That's quite a lot, so we'd better defer memory allocation util
it's needed, that's when filter is used.

Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Masami Hiramatsu <mhiramat@redhat.com>
LKML-Reference: <4A9B8EA5.6020700@cn.fujitsu.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>


---
 include/linux/ftrace_event.h       |    1 -
 include/linux/syscalls.h           |    2 -
 include/trace/ftrace.h             |    1 -
 kernel/trace/trace_events_filter.c |   50 +++++++++++++++++++++++++++++------
 kernel/trace/trace_export.c        |    1 -
 5 files changed, 41 insertions(+), 14 deletions(-)

diff --git a/include/linux/ftrace_event.h b/include/linux/ftrace_event.h
index ace2da9..7554804 100644
--- a/include/linux/ftrace_event.h
+++ b/include/linux/ftrace_event.h
@@ -133,7 +133,6 @@ struct ftrace_event_call {
 #define MAX_FILTER_PRED		32
 #define MAX_FILTER_STR_VAL	128
 
-extern int init_preds(struct ftrace_event_call *call);
 extern void destroy_preds(struct ftrace_event_call *call);
 extern int filter_match_preds(struct ftrace_event_call *call, void *rec);
 extern int filter_current_check_discard(struct ftrace_event_call *call,
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index f124c89..a8e3782 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -177,7 +177,6 @@ static void prof_sysexit_disable_##sname(struct ftrace_event_call *event_call) \
 		event_enter_##sname.id = id;				\
 		set_syscall_enter_id(num, id);				\
 		INIT_LIST_HEAD(&event_enter_##sname.fields);		\
-		init_preds(&event_enter_##sname);			\
 		return 0;						\
 	}								\
 	TRACE_SYS_ENTER_PROFILE(sname);					\
@@ -214,7 +213,6 @@ static void prof_sysexit_disable_##sname(struct ftrace_event_call *event_call) \
 		event_exit_##sname.id = id;				\
 		set_syscall_exit_id(num, id);				\
 		INIT_LIST_HEAD(&event_exit_##sname.fields);		\
-		init_preds(&event_exit_##sname);			\
 		return 0;						\
 	}								\
 	TRACE_SYS_EXIT_PROFILE(sname);					\
diff --git a/include/trace/ftrace.h b/include/trace/ftrace.h
index 57c56a9..bfbc842 100644
--- a/include/trace/ftrace.h
+++ b/include/trace/ftrace.h
@@ -622,7 +622,6 @@ static int ftrace_raw_init_event_##call(void)				\
 		return -ENODEV;						\
 	event_##call.id = id;						\
 	INIT_LIST_HEAD(&event_##call.fields);				\
-	init_preds(&event_##call);					\
 	return 0;							\
 }									\
 									\
diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index 9f03082..c6b2edf 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -309,7 +309,7 @@ void print_event_filter(struct ftrace_event_call *call, struct trace_seq *s)
 	struct event_filter *filter = call->filter;
 
 	mutex_lock(&event_mutex);
-	if (filter->filter_string)
+	if (filter && filter->filter_string)
 		trace_seq_printf(s, "%s\n", filter->filter_string);
 	else
 		trace_seq_printf(s, "none\n");
@@ -322,7 +322,7 @@ void print_subsystem_event_filter(struct event_subsystem *system,
 	struct event_filter *filter = system->filter;
 
 	mutex_lock(&event_mutex);
-	if (filter->filter_string)
+	if (filter && filter->filter_string)
 		trace_seq_printf(s, "%s\n", filter->filter_string);
 	else
 		trace_seq_printf(s, "none\n");
@@ -390,6 +390,9 @@ void destroy_preds(struct ftrace_event_call *call)
 	struct event_filter *filter = call->filter;
 	int i;
 
+	if (!filter)
+		return;
+
 	for (i = 0; i < MAX_FILTER_PRED; i++) {
 		if (filter->preds[i])
 			filter_free_pred(filter->preds[i]);
@@ -400,7 +403,7 @@ void destroy_preds(struct ftrace_event_call *call)
 	call->filter = NULL;
 }
 
-int init_preds(struct ftrace_event_call *call)
+static int init_preds(struct ftrace_event_call *call)
 {
 	struct event_filter *filter;
 	struct filter_pred *pred;
@@ -410,7 +413,6 @@ int init_preds(struct ftrace_event_call *call)
 	if (!call->filter)
 		return -ENOMEM;
 
-	call->filter_active = 0;
 	filter->n_preds = 0;
 
 	filter->preds = kzalloc(MAX_FILTER_PRED * sizeof(pred), GFP_KERNEL);
@@ -432,7 +434,28 @@ oom:
 
 	return -ENOMEM;
 }
-EXPORT_SYMBOL_GPL(init_preds);
+
+static int init_subsystem_preds(struct event_subsystem *system)
+{
+	struct ftrace_event_call *call;
+	int err;
+
+	list_for_each_entry(call, &ftrace_events, list) {
+		if (!call->define_fields)
+			continue;
+
+		if (strcmp(call->system, system->name) != 0)
+			continue;
+
+		if (!call->filter) {
+			err = init_preds(call);
+			if (err)
+				return err;
+		}
+	}
+
+	return 0;
+}
 
 enum {
 	FILTER_DISABLE_ALL,
@@ -449,6 +472,9 @@ static void filter_free_subsystem_preds(struct event_subsystem *system,
 		if (!call->define_fields)
 			continue;
 
+		if (strcmp(call->system, system->name) != 0)
+			continue;
+
 		if (flag == FILTER_INIT_NO_RESET) {
 			call->filter->no_reset = false;
 			continue;
@@ -457,10 +483,8 @@ static void filter_free_subsystem_preds(struct event_subsystem *system,
 		if (flag == FILTER_SKIP_NO_RESET && call->filter->no_reset)
 			continue;
 
-		if (!strcmp(call->system, system->name)) {
-			filter_disable_preds(call);
-			remove_filter_string(call->filter);
-		}
+		filter_disable_preds(call);
+		remove_filter_string(call->filter);
 	}
 }
 
@@ -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;
+
 	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);
+	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;							\
 }									\
 

Re: [PATCH] tracing/filters: Defer pred allocation

[Frederic Weisbecker]

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.

Re: [PATCH] tracing/filters: Defer pred allocation

[Li Zefan]

>> @@ -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.
> 

Oops! I thought I had added a check in it.

Thanks.

[PATCH] tracing/filters: Defer pred allocation, fix memory leak

[Li Zefan]

Check if we have already allocated memory for filter preds.

Spotted-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
---
 kernel/trace/trace_events_filter.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index c6b2edf..93660fb 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -409,6 +409,9 @@ static int init_preds(struct ftrace_event_call *call)
 	struct filter_pred *pred;
 	int i;
 
+	if (call->filter)
+		return 0;
+
 	filter = call->filter = kzalloc(sizeof(*filter), GFP_KERNEL);
 	if (!call->filter)
 		return -ENOMEM;
@@ -447,11 +450,9 @@ static int init_subsystem_preds(struct event_subsystem *system)
 		if (strcmp(call->system, system->name) != 0)
 			continue;
 
-		if (!call->filter) {
-			err = init_preds(call);
-			if (err)
-				return err;
-		}
+		err = init_preds(call);
+		if (err)
+			return err;
 	}
 
 	return 0;
-- 
1.6.3

Re: [PATCH] tracing/filters: Defer pred allocation, fix memory leak

[Frederic Weisbecker]

On Tue, Sep 01, 2009 at 01:31:38PM +0800, Li Zefan wrote:
> Check if we have already allocated memory for filter preds.
> 
> Spotted-by: Frederic Weisbecker <fweisbec@gmail.com>
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>


Acked-by: Frederic Weisbecker <fweisbec@gmail.com>


> ---
>  kernel/trace/trace_events_filter.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index c6b2edf..93660fb 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
> @@ -409,6 +409,9 @@ static int init_preds(struct ftrace_event_call *call)
>  	struct filter_pred *pred;
>  	int i;
>  
> +	if (call->filter)
> +		return 0;
> +
>  	filter = call->filter = kzalloc(sizeof(*filter), GFP_KERNEL);
>  	if (!call->filter)
>  		return -ENOMEM;
> @@ -447,11 +450,9 @@ static int init_subsystem_preds(struct event_subsystem *system)
>  		if (strcmp(call->system, system->name) != 0)
>  			continue;
>  
> -		if (!call->filter) {
> -			err = init_preds(call);
> -			if (err)
> -				return err;
> -		}
> +		err = init_preds(call);
> +		if (err)
> +			return err;
>  	}
>  
>  	return 0;
> -- 
> 1.6.3
> 

[tip:tracing/core] tracing/filters: Defer pred allocation, fix memory leak

[tip-bot for Li Zefan]

Commit-ID:  c58b43218c1a04a0bcf338ea47406c759ac28e11
Gitweb:     http://git.kernel.org/tip/c58b43218c1a04a0bcf338ea47406c759ac28e11
Author:     Li Zefan <lizf@cn.fujitsu.com>
AuthorDate: Tue, 1 Sep 2009 13:31:38 +0800
Committer:  Frederic Weisbecker <fweisbec@gmail.com>
CommitDate: Fri, 4 Sep 2009 23:22:33 +0200

tracing/filters: Defer pred allocation, fix memory leak

The predicates of an event and their filter structure are allocated
when we create an event filter for the first time.

These objects must be created once but each time we come with a new
filter, we overwrite such pre-existing allocation, if any.

Thus, this patch checks if the filter has already been allocated
before going ahead.

Spotted-by: Frederic Weisbecker <fweisbec@gmail.com>
Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tom Zanussi <tzanussi@gmail.com>
Cc: Masami Hiramatsu <mhiramat@redhat.com>
LKML-Reference: <4A9CB1BA.3060402@cn.fujitsu.com>
Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>


---
 kernel/trace/trace_events_filter.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
index c6b2edf..93660fb 100644
--- a/kernel/trace/trace_events_filter.c
+++ b/kernel/trace/trace_events_filter.c
@@ -409,6 +409,9 @@ static int init_preds(struct ftrace_event_call *call)
 	struct filter_pred *pred;
 	int i;
 
+	if (call->filter)
+		return 0;
+
 	filter = call->filter = kzalloc(sizeof(*filter), GFP_KERNEL);
 	if (!call->filter)
 		return -ENOMEM;
@@ -447,11 +450,9 @@ static int init_subsystem_preds(struct event_subsystem *system)
 		if (strcmp(call->system, system->name) != 0)
 			continue;
 
-		if (!call->filter) {
-			err = init_preds(call);
-			if (err)
-				return err;
-		}
+		err = init_preds(call);
+		if (err)
+			return err;
 	}
 
 	return 0;

Re: [PATCH] tracing/filters: Defer pred allocation

[Tom Zanussi]

On Mon, 2009-08-31 at 16:49 +0800, Li Zefan wrote:
> init_preds() allocates about 5392 bytes of memory (on x86_32) for
> a TRACE_EVENT. With my config, at system boot total memory occupied
> is:
> 
> 	5392 * (642 + 15) == 3459KB
> 
> 642 == cat available_events | wc -l
> 15 == number of dirs in events/ftrace
> 
> That's quite a lot, so we'd better defer memory allocation util
> it's needed, that's when filter is used.
> 
> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>

Other than the leak mentioned by Frederic, it looks good to me.

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