linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH v2] trace: Move trace event enable from fs_initcall to early_initcall
@ 2012-08-17 12:03 Ezequiel Garcia
  2012-08-17 15:28 ` Steven Rostedt
  0 siblings, 1 reply; 7+ messages in thread
From: Ezequiel Garcia @ 2012-08-17 12:03 UTC (permalink / raw)
  To: Steven Rostedt, linux-kernel
  Cc: Pekka Enberg, Frederic Weisbecker, Ingo Molnar, tim.bird,
	lizefan, Ezequiel Garcia

This patch splits trace event initialization in two stages:
 * ftrace enable
 * sysfs event entry creation

This allows to capture trace events from an earlier point
by using 'trace_event' kernel parameter and is important
to trace boot-up allocations.

Cc: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Ezequiel Garcia <elezegarcia@gmail.com>
---
 Changes from v1:
   * Incorporate review comments from Steven

 kernel/trace/trace_events.c |  104 +++++++++++++++++++++++++++++--------------
 1 files changed, 70 insertions(+), 34 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 29111da..5bc1cd1 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1199,6 +1199,31 @@ event_create_dir(struct ftrace_event_call *call, struct dentry *d_events,
 	return 0;
 }
 
+static void event_remove(struct ftrace_event_call *call)
+{
+	ftrace_event_enable_disable(call, 0);
+	if (call->event.funcs)
+		__unregister_ftrace_event(&call->event);
+	list_del(&call->list);
+}
+
+static int event_init(struct ftrace_event_call *call)
+{
+	int ret = 0;
+
+	if (WARN_ON(!call->name))
+		return -EINVAL;
+
+	if (call->class->raw_init) {
+		ret = call->class->raw_init(call);
+		if (ret < 0 && ret != -ENOSYS)
+			pr_warn("Could not initialize trace events/%s\n",
+				call->name);
+	}
+
+	return ret;
+}
+
 static int
 __trace_add_event_call(struct ftrace_event_call *call, struct module *mod,
 		       const struct file_operations *id,
@@ -1209,19 +1234,9 @@ __trace_add_event_call(struct ftrace_event_call *call, struct module *mod,
 	struct dentry *d_events;
 	int ret;
 
-	/* The linker may leave blanks */
-	if (!call->name)
-		return -EINVAL;
-
-	if (call->class->raw_init) {
-		ret = call->class->raw_init(call);
-		if (ret < 0) {
-			if (ret != -ENOSYS)
-				pr_warning("Could not initialize trace events/%s\n",
-					   call->name);
-			return ret;
-		}
-	}
+	ret = event_init(call);
+	if (ret < 0)
+		return ret;
 
 	d_events = event_trace_events_dir();
 	if (!d_events)
@@ -1272,13 +1287,10 @@ static void remove_subsystem_dir(const char *name)
  */
 static void __trace_remove_event_call(struct ftrace_event_call *call)
 {
-	ftrace_event_enable_disable(call, 0);
-	if (call->event.funcs)
-		__unregister_ftrace_event(&call->event);
-	debugfs_remove_recursive(call->dir);
-	list_del(&call->list);
+	event_remove(call);
 	trace_destroy_fields(call);
 	destroy_preds(call);
+	debugfs_remove_recursive(call->dir);
 	remove_subsystem_dir(call->class->system);
 }
 
@@ -1450,6 +1462,36 @@ static __init int setup_trace_event(char *str)
 }
 __setup("trace_event=", setup_trace_event);
 
+static __init int event_trace_enable(void)
+{
+	struct ftrace_event_call **iter, *call;
+	char *buf = bootup_event_buf;
+	char *token;
+	int ret;
+
+	for_each_event(iter, __start_ftrace_events, __stop_ftrace_events) {
+
+		call = *iter;
+		ret = event_init(call);
+		if (!ret)
+			list_add(&call->list, &ftrace_events);
+	}
+
+	while (true) {
+		token = strsep(&buf, ",");
+
+		if (!token)
+			break;
+		if (!*token)
+			continue;
+
+		ret = ftrace_set_clr_event(token, 1);
+		if (ret)
+			pr_warn("Failed to enable trace event: %s\n", token);
+	}
+	return 0;
+}
+
 static __init int event_trace_init(void)
 {
 	struct ftrace_event_call **call;
@@ -1457,8 +1499,6 @@ static __init int event_trace_init(void)
 	struct dentry *entry;
 	struct dentry *d_events;
 	int ret;
-	char *buf = bootup_event_buf;
-	char *token;
 
 	d_tracer = tracing_init_dentry();
 	if (!d_tracer)
@@ -1497,24 +1537,19 @@ static __init int event_trace_init(void)
 	if (trace_define_common_fields())
 		pr_warning("tracing: Failed to allocate common fields");
 
+	/*
+	 * Early initialization already enabled ftrace event.
+	 * Now it's only necessary to create the event directory.
+	 */
 	for_each_event(call, __start_ftrace_events, __stop_ftrace_events) {
-		__trace_add_event_call(*call, NULL, &ftrace_event_id_fops,
+
+		ret = event_create_dir(*call, d_events,
+				       &ftrace_event_id_fops,
 				       &ftrace_enable_fops,
 				       &ftrace_event_filter_fops,
 				       &ftrace_event_format_fops);
-	}
-
-	while (true) {
-		token = strsep(&buf, ",");
-
-		if (!token)
-			break;
-		if (!*token)
-			continue;
-
-		ret = ftrace_set_clr_event(token, 1);
-		if (ret)
-			pr_warning("Failed to enable trace event: %s\n", token);
+		if (ret < 0)
+			event_remove(*call);
 	}
 
 	ret = register_module_notifier(&trace_module_nb);
@@ -1523,6 +1558,7 @@ static __init int event_trace_init(void)
 
 	return 0;
 }
+early_initcall(event_trace_enable);
 fs_initcall(event_trace_init);
 
 #ifdef CONFIG_FTRACE_STARTUP_TEST
-- 
1.7.8.6


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2] trace: Move trace event enable from fs_initcall to early_initcall
  2012-08-17 12:03 [RFC PATCH v2] trace: Move trace event enable from fs_initcall to early_initcall Ezequiel Garcia
@ 2012-08-17 15:28 ` Steven Rostedt
  2012-08-17 16:56   ` Ezequiel Garcia
  2012-09-05  1:18   ` Ezequiel Garcia
  0 siblings, 2 replies; 7+ messages in thread
From: Steven Rostedt @ 2012-08-17 15:28 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-kernel, Pekka Enberg, Frederic Weisbecker, Ingo Molnar,
	tim.bird, lizefan

On Fri, 2012-08-17 at 09:03 -0300, Ezequiel Garcia wrote:
> This patch splits trace event initialization in two stages:
>  * ftrace enable
>  * sysfs event entry creation
> 
> This allows to capture trace events from an earlier point
> by using 'trace_event' kernel parameter and is important
> to trace boot-up allocations.
> 

This is much better, I'll give it some tests and add it to my 3.7 queue.

Thanks,

-- Steve



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2] trace: Move trace event enable from fs_initcall to early_initcall
  2012-08-17 15:28 ` Steven Rostedt
@ 2012-08-17 16:56   ` Ezequiel Garcia
  2012-09-05  1:18   ` Ezequiel Garcia
  1 sibling, 0 replies; 7+ messages in thread
From: Ezequiel Garcia @ 2012-08-17 16:56 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Pekka Enberg, Frederic Weisbecker, Ingo Molnar,
	tim.bird, lizefan

On Fri, Aug 17, 2012 at 12:28 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 2012-08-17 at 09:03 -0300, Ezequiel Garcia wrote:
>> This patch splits trace event initialization in two stages:
>>  * ftrace enable
>>  * sysfs event entry creation
>>
>> This allows to capture trace events from an earlier point
>> by using 'trace_event' kernel parameter and is important
>> to trace boot-up allocations.
>>
>
> This is much better, I'll give it some tests and add it to my 3.7 queue.
>

Great. Is the event_remove() code, okey? I wasn't too sure about this part.

And also: do you think we could try to only early-enable those events
that were setup with 'trace_event' boot parameter?

I tried but couldn't do it without looping through the complete
__start_ftrace_event array and comparing each entry.

Is this even worthy?

Thanks for reviewing,
Ezequiel.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2] trace: Move trace event enable from fs_initcall to early_initcall
  2012-08-17 15:28 ` Steven Rostedt
  2012-08-17 16:56   ` Ezequiel Garcia
@ 2012-09-05  1:18   ` Ezequiel Garcia
  2012-09-05 17:53     ` Steven Rostedt
  1 sibling, 1 reply; 7+ messages in thread
From: Ezequiel Garcia @ 2012-09-05  1:18 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Pekka Enberg, Frederic Weisbecker, Ingo Molnar,
	tim.bird, lizefan

Hi Steven,

On 8/17/12, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 2012-08-17 at 09:03 -0300, Ezequiel Garcia wrote:
>> This patch splits trace event initialization in two stages:
>>  * ftrace enable
>>  * sysfs event entry creation
>>
>> This allows to capture trace events from an earlier point
>> by using 'trace_event' kernel parameter and is important
>> to trace boot-up allocations.
>>
>
> This is much better, I'll give it some tests and add it to my 3.7 queue.
>

Ping? Will you add this to your tree?

Thanks!
Ezequiel.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2] trace: Move trace event enable from fs_initcall to early_initcall
  2012-09-05  1:18   ` Ezequiel Garcia
@ 2012-09-05 17:53     ` Steven Rostedt
  2012-09-06  2:46       ` Steven Rostedt
  0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2012-09-05 17:53 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-kernel, Pekka Enberg, Frederic Weisbecker, Ingo Molnar,
	tim.bird, lizefan

On Tue, 2012-09-04 at 22:18 -0300, Ezequiel Garcia wrote:
> Hi Steven,
> 
> On 8/17/12, Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Fri, 2012-08-17 at 09:03 -0300, Ezequiel Garcia wrote:
> >> This patch splits trace event initialization in two stages:
> >>  * ftrace enable
> >>  * sysfs event entry creation
> >>
> >> This allows to capture trace events from an earlier point
> >> by using 'trace_event' kernel parameter and is important
> >> to trace boot-up allocations.
> >>
> >
> > This is much better, I'll give it some tests and add it to my 3.7 queue.
> >
> 
> Ping? Will you add this to your tree?

Yeah, I haven't been able to test it yet. I've been stuck trying to
figure out bugs on other patches I have queued. I'll let you know when I
do apply it though.

Thanks,

-- Steve



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2] trace: Move trace event enable from fs_initcall to early_initcall
  2012-09-05 17:53     ` Steven Rostedt
@ 2012-09-06  2:46       ` Steven Rostedt
  2012-09-07 16:36         ` Ezequiel Garcia
  0 siblings, 1 reply; 7+ messages in thread
From: Steven Rostedt @ 2012-09-06  2:46 UTC (permalink / raw)
  To: Ezequiel Garcia
  Cc: linux-kernel, Pekka Enberg, Frederic Weisbecker, Ingo Molnar,
	tim.bird, lizefan

On Wed, 2012-09-05 at 13:53 -0400, Steven Rostedt wrote:
> On Tue, 2012-09-04 at 22:18 -0300, Ezequiel Garcia wrote:
> > Hi Steven,
> > 
> > On 8/17/12, Steven Rostedt <rostedt@goodmis.org> wrote:
> > > On Fri, 2012-08-17 at 09:03 -0300, Ezequiel Garcia wrote:
> > >> This patch splits trace event initialization in two stages:
> > >>  * ftrace enable
> > >>  * sysfs event entry creation
> > >>
> > >> This allows to capture trace events from an earlier point
> > >> by using 'trace_event' kernel parameter and is important
> > >> to trace boot-up allocations.
> > >>
> > >
> > > This is much better, I'll give it some tests and add it to my 3.7 queue.
> > >
> > 
> > Ping? Will you add this to your tree?
> 
> Yeah, I haven't been able to test it yet. I've been stuck trying to
> figure out bugs on other patches I have queued. I'll let you know when I
> do apply it though.

Just tested it and it failed :-/

Enable CONFIG_FTRACE_SELFTEST and CONFIG_FTRACE_STARTUP_TEST, along with
events and syscall events. The syscall event self test fails with this
patch applied.

-- Steve



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC PATCH v2] trace: Move trace event enable from fs_initcall to early_initcall
  2012-09-06  2:46       ` Steven Rostedt
@ 2012-09-07 16:36         ` Ezequiel Garcia
  0 siblings, 0 replies; 7+ messages in thread
From: Ezequiel Garcia @ 2012-09-07 16:36 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: linux-kernel, Pekka Enberg, Frederic Weisbecker, Ingo Molnar,
	tim.bird, lizefan

Hi Steven,

On Wed, Sep 5, 2012 at 11:46 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 2012-09-05 at 13:53 -0400, Steven Rostedt wrote:
>> On Tue, 2012-09-04 at 22:18 -0300, Ezequiel Garcia wrote:
>> > Hi Steven,
>> >
>> > On 8/17/12, Steven Rostedt <rostedt@goodmis.org> wrote:
>> > > On Fri, 2012-08-17 at 09:03 -0300, Ezequiel Garcia wrote:
>> > >> This patch splits trace event initialization in two stages:
>> > >>  * ftrace enable
>> > >>  * sysfs event entry creation
>> > >>
>> > >> This allows to capture trace events from an earlier point
>> > >> by using 'trace_event' kernel parameter and is important
>> > >> to trace boot-up allocations.
>> > >>
>> > >
>> > > This is much better, I'll give it some tests and add it to my 3.7 queue.
>> > >
>> >
>> > Ping? Will you add this to your tree?
>>
>> Yeah, I haven't been able to test it yet. I've been stuck trying to
>> figure out bugs on other patches I have queued. I'll let you know when I
>> do apply it though.
>
> Just tested it and it failed :-/
>

Damn!

> Enable CONFIG_FTRACE_SELFTEST and CONFIG_FTRACE_STARTUP_TEST, along with
> events and syscall events. The syscall event self test fails with this
> patch applied.
>

Ok, I'll resend a v3.

Sorry for not testing this myself and thanks,
Ezequiel.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-09-07 16:36 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-17 12:03 [RFC PATCH v2] trace: Move trace event enable from fs_initcall to early_initcall Ezequiel Garcia
2012-08-17 15:28 ` Steven Rostedt
2012-08-17 16:56   ` Ezequiel Garcia
2012-09-05  1:18   ` Ezequiel Garcia
2012-09-05 17:53     ` Steven Rostedt
2012-09-06  2:46       ` Steven Rostedt
2012-09-07 16:36         ` Ezequiel Garcia

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).