Messages in this thread | | | From | "Zhaolei" <> | Subject | Re: [PATCH v2 1/2] ftrace: Add task_comm support for trace_event | Date | Mon, 25 May 2009 11:54:27 +0800 |
| |
* From: "Frederic Weisbecker" <fweisbec@gmail.com> > Hi, > > > On Fri, May 22, 2009 at 06:05:37PM +0800, Zhaolei wrote: >> If we use trace_event alone(without function trace, .etc), >> it can't output enough task command information. >> >> Before patch: >> # echo 1 > debugfs/tracing/events/sched/sched_switch/enable >> # cat debugfs/tracing/trace >> # tracer: nop >> # >> # TASK-PID CPU# TIMESTAMP FUNCTION >> # | | | | | >> <...>-2289 [000] 526276.724790: sched_switch: task bash:2289 [120] ==> sshd:2287 [120] >> <...>-2287 [000] 526276.725231: sched_switch: task sshd:2287 [120] ==> bash:2289 [120] >> <...>-2289 [000] 526276.725452: sched_switch: task bash:2289 [120] ==> sshd:2287 [120] >> <...>-2287 [000] 526276.727181: sched_switch: task sshd:2287 [120] ==> swapper:0 [140] >> <idle>-0 [000] 526277.032734: sched_switch: task swapper:0 [140] ==> events/0:5 [115] >> <...>-5 [000] 526277.032782: sched_switch: task events/0:5 [115] ==> swapper:0 [140] >> ... >> >> After patch: >> # tracer: nop >> # >> # TASK-PID CPU# TIMESTAMP FUNCTION >> # | | | | | >> bash-2269 [000] 527347.989229: sched_switch: task bash:2269 [120] ==> sshd:2267 [120] >> sshd-2267 [000] 527347.990960: sched_switch: task sshd:2267 [120] ==> bash:2269 [120] >> bash-2269 [000] 527347.991143: sched_switch: task bash:2269 [120] ==> sshd:2267 [120] >> sshd-2267 [000] 527347.992959: sched_switch: task sshd:2267 [120] ==> swapper:0 [140] >> <idle>-0 [000] 527348.531989: sched_switch: task swapper:0 [140] ==> events/0:5 [115] >> events/0-5 [000] 527348.532115: sched_switch: task events/0:5 [115] ==> swapper:0 [140] >> ... >> >> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com> > > > Thanks! > This is fine but I think it can be factorized. > > You could call start_cmdline_record() from > > ftrace_raw_reg_event_##call() > > and the stop in > > ftrace_raw_unreg_event_##call() > > No?
Hello, Frederic
Thanks for your advice.
Actually, I considered to put start_cmdline_record() into ftrace_raw_reg_event_##call(), but finally I selected to put it into tracing_start_cmdline_record().
IMHO, we have following reason: 1: It can make source more readable. Read function is more easy than read macro. 2: These two way have same performance. 3: Put start_cmdline_record() into ftrace_event_enable_disable() will reduce binary file size than ftrace_raw_reg_event_##call().
So I think put start_cmdline_record() into ftrace_event_enable_disable() maybe better.
What is your opinion?
Thanks Zhaolei
| |