Messages in this thread | | | Date | Mon, 25 May 2009 13:34:26 +0800 | From | Zhaolei <> | Subject | Re: Re: [PATCH v2 2/2] ftrace: clean up of using ftrace_event_enable_disable() |
| |
Frederic Weisbecker wrote: > On Fri, May 22, 2009 at 06:06:20PM +0800, Zhaolei wrote: >> Always use tracing_stop_cmdline_record() to enable/disable a event. >> >> Impact: cleanup, no functionality changed >> >> Signed-off-by: Zhao Lei <zhaolei@cn.fujitsu.com> >> --- >> kernel/trace/trace_events.c | 44 +++++++++++++----------------------------- >> 1 files changed, 14 insertions(+), 30 deletions(-) >> >> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c >> index df35e5e..16ef47a 100644 >> --- a/kernel/trace/trace_events.c >> +++ b/kernel/trace/trace_events.c >> @@ -76,26 +76,9 @@ static void trace_destroy_fields(struct ftrace_event_call *call) >> >> #endif /* CONFIG_MODULES */ >> >> -static void ftrace_clear_events(void) >> -{ >> - struct ftrace_event_call *call; >> - >> - mutex_lock(&event_mutex); >> - list_for_each_entry(call, &ftrace_events, list) { > > > Don't we have a "for_each_event" ?
Hello, Frederic
Thanks for your review.
IMHO, for_each_event is for iter each tracepoints in one module(or kernel), but we need to iter whole tracepoints in manage. So, list_for_each_entry(call, &ftrace_events, list) maybe better here.
Thanks Zhaolei
> > >> - >> - if (call->enabled) { >> - call->enabled = 0; >> - tracing_stop_cmdline_record(); >> - call->unregfunc(); >> - } >> - } >> - mutex_unlock(&event_mutex); >> -} >> - >> static void ftrace_event_enable_disable(struct ftrace_event_call *call, >> int enable) >> { >> - >> switch (enable) { >> case 0: >> if (call->enabled) { >> @@ -114,6 +97,17 @@ static void ftrace_event_enable_disable(struct ftrace_event_call *call, >> } >> } >> >> +static void ftrace_clear_events(void) >> +{ >> + struct ftrace_event_call *call; >> + >> + mutex_lock(&event_mutex); >> + list_for_each_entry(call, &ftrace_events, list) { >> + ftrace_event_enable_disable(call, 0); >> + } >> + mutex_unlock(&event_mutex); >> +} >> + >> /* >> * __ftrace_set_clr_event(NULL, NULL, NULL, set) will set/unset all events. >> */ >> @@ -1059,11 +1053,7 @@ static void trace_module_remove_events(struct module *mod) >> list_for_each_entry_safe(call, p, &ftrace_events, list) { >> if (call->mod == mod) { >> found = true; >> - if (call->enabled) { >> - call->enabled = 0; >> - tracing_stop_cmdline_record(); >> - call->unregfunc(); >> - } >> + ftrace_event_enable_disable(call, 0); >> if (call->event) >> unregister_ftrace_event(call->event); >> debugfs_remove_recursive(call->dir); >> @@ -1265,15 +1255,9 @@ static __init void event_trace_self_tests(void) >> continue; >> } >> >> - call->enabled = 1; >> - tracing_start_cmdline_record(); >> - call->regfunc(); >> - >> + ftrace_event_enable_disable(call, 1); >> event_test_stuff(); >> - >> - call->unregfunc(); >> - tracing_stop_cmdline_record(); >> - call->enabled = 0; >> + ftrace_event_enable_disable(call, 0); >> >> pr_cont("OK\n"); >> } > > > Acked-by: Frederic Weisbecker <fweisbec@gmail.com> > >
| |