Re: [PATCH] tracing/events: Add module tracepoints

[Frederic Weisbecker <fweisbec@gmail.com>]

On Mon, Jul 27, 2009 at 09:41:46AM +0800, Li Zefan wrote:
> Frederic Weisbecker wrote:
> > On Tue, Jul 21, 2009 at 04:56:33PM +0800, Li Zefan wrote:
> >> Add trace points to trace module_load, module_free, module_get,
> >> module_put and module_request, and use trace_event facility
> >> to get the trace output.
> >>
> >> Here's the sample output:
> >>
> >>      TASK-PID    CPU#    TIMESTAMP  FUNCTION
> >>         | |       |          |         |
> >>     <...>-42    [000]     1.758380: module_request: fb0 wait=1 call_site=fb_open
> >>     ...
> >>     <...>-60    [000]     3.269403: module_load: scsi_wait_scan 
> >>     <...>-60    [000]     3.269432: module_put: scsi_wait_scan call_site=sys_init_module refcnt=0
> >>     <...>-61    [001]     3.273168: module_free: scsi_wait_scan
> >>     ...
> >>     <...>-1021  [000]    13.836081: module_load: sunrpc 
> >>     <...>-1021  [000]    13.840589: module_put: sunrpc call_site=sys_init_module refcnt=-1
> >>     <...>-1027  [000]    13.848098: module_get: sunrpc call_site=try_module_get refcnt=0
> >>     <...>-1027  [000]    13.848308: module_get: sunrpc call_site=get_filesystem refcnt=1
> >>     <...>-1027  [000]    13.848692: module_put: sunrpc call_site=put_filesystem refcnt=0
> >>     ...
> >>  modprobe-2587  [001]  1088.437213: module_load: trace_events_sample F
> >>  modprobe-2587  [001]  1088.437786: module_put: trace_events_sample call_site=sys_init_module refcnt=0
> >>
> >>
> >> Note:
> >>
> >> - the taints flag can be 'F', 'C' and/or 'P' if mod->taints != 0
> >>
> >> - the module refcnt is percpu, so it can be negative in a specific cpu
> >>
> >> Signed-off-by: Li Zefan <lizf@cn.fujitsu.com>
> > 
> > 
> > Nice.
> > 
> > Just two worries about it.
> > 
> > The ring buffer are flushed on module unloading right?
> > That won't make it easy to perform module event tracing.
> > 
> 
> Yes, but only when the module has "_ftrace_events" section.
> They are ext4 and gfs2 for the current kernel.


Ah, good point.

 
> > Also the events selftests do a lot of random things to trigger
> > each kind of events, I guess some new others will be needed to
> > tests these, unless they will seem to fail on every selftests.
> > Although I can't imagine a module loading/unloading for
> > every ftrace event selftest... I guess these will require
> > a specific treatement and also will need to be selftested once
> > the filesystem is set to be able to load modules.
> > 
> 
> It's nice to have more selftests but I don't think it is a
> necessity.
> 
> Actually the events selftests just confirm the events won't
> crash the system, and no trace entries are generated by them
> in the selftests.


Indeed, forget about these worries :)

Thanks.