[PATCH] Tracing: do export trace_set_clr_event

[PATCH] Tracing: do export trace_set_clr_event

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

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
---
 kernel/trace/trace_events.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 0725eea..711965b 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -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
-- 
1.7.2.3

Re: [PATCH] Tracing: do export trace_set_clr_event

[Christoph Hellwig]

On Mon, Nov 08, 2010 at 02:05:12PM +0800, Yuanhan Liu wrote:
> 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?

Re: [PATCH] Tracing: do export trace_set_clr_event

[Chris Wilson]

On Mon, 8 Nov 2010 04:56:10 -0500, Christoph Hellwig <hch@infradead.org> wrote:
> On Mon, Nov 08, 2010 at 02:05:12PM +0800, Yuanhan Liu wrote:
> > 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

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH] Tracing: do export trace_set_clr_event

[Steven Rostedt]

On Mon, 2010-11-08 at 10:14 +0000, Chris Wilson wrote:

> 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

Re: [PATCH] Tracing: do export trace_set_clr_event

[Christoph Hellwig]

On Mon, Nov 08, 2010 at 09:02:14AM -0500, Steven Rostedt wrote:
> 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.

Re: [PATCH] Tracing: do export trace_set_clr_event

[Steven Rostedt]

On Mon, 2010-11-08 at 09:14 -0500, Christoph Hellwig wrote:
> On Mon, Nov 08, 2010 at 09:02:14AM -0500, Steven Rostedt wrote:
> > 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

Re: [PATCH] Tracing: do export trace_set_clr_event

[Yuanhan Liu]

On Mon, Nov 08, 2010 at 09:23:04AM -0500, Steven Rostedt wrote:
> On Mon, 2010-11-08 at 09:14 -0500, Christoph Hellwig wrote:
> > On Mon, Nov 08, 2010 at 09:02:14AM -0500, Steven Rostedt wrote:
> > > 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.

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

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

[tip-bot for Yuanhan Liu]

Commit-ID:  56355b83e2a24ce7e1870c8479205e2cdd332225
Gitweb:     http://git.kernel.org/tip/56355b83e2a24ce7e1870c8479205e2cdd332225
Author:     Yuanhan Liu <yuanhan.liu@linux.intel.com>
AuthorDate: Mon, 8 Nov 2010 14:05:12 +0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
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.

Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
LKML-Reference: <1289196312-25323-1-git-send-email-yuanhan.liu@linux.intel.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 kernel/trace/trace_events.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index e1d579b..e88f74f 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -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