Skip to contentSkip to conversation

Tracing: do export trace_set_clr_event

PATCH In review Yuanhan Liu 4 participants 8 messages Nov 8, 2010 – Mar 11, 2011 Tracing

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Tracing: do export trace_set_clr_event

Trace event belongs to a module does exist only when the module is loaded. Well, we can use trace_set_clr_event funtion to enable some trace event at the module init routine, so that we will not miss something while loading then module.

So, Export the trace_set_clr_event function so that module can use it.

Cc Steven Rostedt <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
---
 kernel/trace/trace_events.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
kernel/trace/trace_events.c +1 -0
index 0725eea..711965b 100644
@@ -320,6 +320,7 @@ int trace_set_clr_event(const char *system, const char *event, int set)
 {
 	return __ftrace_set_clr_event(NULL, system, event, set);
 }
+EXPORT_SYMBOL_GPL(trace_set_clr_event);
 
 /* 128 should be much more than enough */
 #define EVENT_BUF_SIZE		127
Signature · 1 line
1.7.2.3
#1 lore raw
Quoted 6 lines · Yuanhan Liu

Trace event belongs to a module does exist only when the module is loaded. Well, we can use trace_set_clr_event funtion to enable some trace event at the module init routine, so that we will not miss something while loading then module.

So, Export the trace_set_clr_event function so that module can use it.

Which module?

#2 lore raw
Quoted 9 lines · Christoph Hellwig
Quoted 6 lines · Yuanhan Liu

Trace event belongs to a module does exist only when the module is loaded. Well, we can use trace_set_clr_event funtion to enable some trace event at the module init routine, so that we will not miss something while loading then module.

So, Export the trace_set_clr_event function so that module can use it.

Which module?

drm/i915. The scenario is that we want to enable a tracepoint to record (selected) register writes both at runtime and module-load time. mmiotrace is not quite sufficient for our needs, since we want to be enable recording around a troublesome application from within X (i.e. without having to ask the user to reload the module after enabling mmiotrace). We also want to exclude the bit-banging registers from being traced since they generate voluminous noise. So we settled on adding a tracepoint to the our register read/write routines.

The question then became how to enable register tracing upon module load? This was the simpler proposal, add a module parameter to enable the tracepoints - hence the exporting of this routine. The alternative that we are mulling over is whether it is possible for ftrace to parse a trace= on every module load to manipulate the tracepoints within that module. -Chris

Signature · 1 line
Chris Wilson, Intel Open Source Technology Centre
maintainer
#3 lore raw
Quoted 5 lines · Chris Wilson

The question then became how to enable register tracing upon module load? This was the simpler proposal, add a module parameter to enable the tracepoints - hence the exporting of this routine. The alternative that we are mulling over is whether it is possible for ftrace to parse a trace= on every module load to manipulate the tracepoints within that module.

I like the trace=on parameter much better. If that is set we could enable the tracepoints of that module at load time. I really do not want to export the function that was proposed in that patch.

-- Steve

#4 lore raw
Quoted 3 lines · Steven Rostedt

I like the trace=on parameter much better. If that is set we could enable the tracepoints of that module at load time. I really do not want to export the function that was proposed in that patch.

Yes. Adding generic support in the module loader to turn on tracepoint seems like the much better long term strategy. Even better if it allows turning on individual points instead of all or nothing.

maintainer
#5 lore raw
Quoted 8 lines · Christoph Hellwig
Quoted 3 lines · Steven Rostedt

I like the trace=on parameter much better. If that is set we could enable the tracepoints of that module at load time. I really do not want to export the function that was proposed in that patch.

Yes. Adding generic support in the module loader to turn on tracepoint seems like the much better long term strategy. Even better if it allows turning on individual points instead of all or nothing.

I was thinking the same. How about this:

trace=1 - all tracepoints in the module is enabled trace=0 - same as leaving it off

trace=name - a specific tracepoint is enabled, using the simple globs that set_event allows.

trace=name1,name2,name3 - for more than one tracepoint.

-- Steve

#6 lore raw
Quoted 12 lines · Steven Rostedt
Quoted 8 lines · Christoph Hellwig
Quoted 3 lines · Steven Rostedt

I like the trace=on parameter much better. If that is set we could enable the tracepoints of that module at load time. I really do not want to export the function that was proposed in that patch.

Yes. Adding generic support in the module loader to turn on tracepoint seems like the much better long term strategy. Even better if it allows turning on individual points instead of all or nothing.

I was thinking the same. How about this:

Quoted 7 lines

trace=1 - all tracepoints in the module is enabled trace=0 - same as leaving it off

trace=name - a specific tracepoint is enabled, using the simple globs that set_event allows.

trace=name1,name2,name3 - for more than one tracepoint.

Yes, agreed. I thought that's the way to make a generate interface for active trace event on module load.

I have some other ideas for this:
1. For make it be consistent with the Documentation/trace/events.txt, I guess we should use:
trace=* - all tracepoints in the module is enabled
trace=NULL - same as leaving it off

2. How about make it be module specific? Since there is aready a global boot option for this: trace_event=[event_list], which doesn't work if the coresponding event doesn't exist. I think it's the module's task to enable it's own event at load time. Take i915 module as example, if user put something like following in the boot command line:

i915.trace=i915_reg_rw,i915_gem_object_create

then the i915 module will try to enable these two envent at the init function.

so, how about the following pseudo-code which should be in trace_events.c:

void trace_enable_module_event(struct module *mod, const char *event_list)

{
	for_each_token(token, event_list) {
		for_each_event(event, mod->trace_events) {
			if (strcmp(mod->name, token) == 0) 
				ftrace_set_clr_event(token, 1);
		}
	}
}

then, on the i915 side:
i915_init ()

{
	....
	....
	if (event_list)
		trace_enable_module_event(THIS_MODULE, i915_trace);
	...
}

Comments?

Thanks,

--
Yuanhan Liu

#7 lore raw

[tip:perf/core] tracing: Export trace_set_clr_event()

In reply to Yuanhan Liu
Commit-ID:  56355b83e2a24ce7e1870c8479205e2cdd332225
Gitweb:     http://git.kernel.org/tip/56355b83e2a24ce7e1870c8479205e2cdd332225
Author:     Yuanhan Liu <[email protected]>
AuthorDate: Mon, 8 Nov 2010 14:05:12 +0800
Committer:  Steven Rostedt <[email protected]>
CommitDate: Thu, 10 Mar 2011 10:34:51 -0500

tracing: Export trace_set_clr_event()

Trace events belonging to a module only exists when the module is loaded. Well, we can use trace_set_clr_event funtion to enable some trace event at the module init routine, so that we will not miss something while loading then module.

So, Export the trace_set_clr_event function so that module can use it.

LKML-Reference: <[email protected]>

Cc Steven Rostedt <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Ingo Molnar <[email protected]>
---
 kernel/trace/trace_events.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
kernel/trace/trace_events.c +1 -0
index e1d579b..e88f74f 100644
@@ -325,6 +325,7 @@ int trace_set_clr_event(const char *system, const char *event, int set)
 {
 	return __ftrace_set_clr_event(NULL, system, event, set);
 }
+EXPORT_SYMBOL_GPL(trace_set_clr_event);
 
 /* 128 should be much more than enough */
 #define EVENT_BUF_SIZE		127