Messages in this thread | | | Date | Fri, 17 Jul 2009 00:05:29 -0400 | From | Frederic Weisbecker <> | Subject | Re: [PATCH 2/2] tracing/function: simplify for __ftrace_replace_code() |
| |
On Wed, Jul 15, 2009 at 12:32:15PM +0800, Xiao Guangrong wrote: > Rewrite the __ftrace_replace_code() function, let it's simple, but > not change the code's logic. > > First, we get the state we want to set, if the record has the same > state, then do nothing, if not, enable/disable it. > > Signed-off-by: Xiao Guangrong <xiaoguangrong@cn.fujitsu.com> > --- > kernel/trace/ftrace.c | 75 +++++++++++++----------------------------------- > 1 files changed, 21 insertions(+), 54 deletions(-) > > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c > index 4521c77..18aeb8d 100644 > --- a/kernel/trace/ftrace.c > +++ b/kernel/trace/ftrace.c > @@ -1016,71 +1016,38 @@ static int > __ftrace_replace_code(struct dyn_ftrace *rec, int enable) > { > unsigned long ftrace_addr; > - unsigned long ip, fl; > + unsigned long flag = 0UL; > > ftrace_addr = (unsigned long)FTRACE_ADDR; > > - ip = rec->ip; > - > /* > - * If this record is not to be traced and > - * it is not enabled then do nothing. > + * If this record is not to be traced or we want to disable it, > + * then disable it. > * > - * If this record is not to be traced and > - * it is enabled then disable it. > + * If we want to enable it and filtering is off, then enable it. > * > + * If we want to enable it and filtering is on, enable it only if > + * it's filtered > */ > - if (rec->flags & FTRACE_FL_NOTRACE) { > - if (rec->flags & FTRACE_FL_ENABLED) > - rec->flags &= ~FTRACE_FL_ENABLED; > - else > - return 0; > - > - } else if (ftrace_filtered && enable) { > - /* > - * Filtering is on: > - */ > - > - fl = rec->flags & (FTRACE_FL_FILTER | FTRACE_FL_ENABLED); > - > - /* Record is filtered and enabled, do nothing */ > - if (fl == (FTRACE_FL_FILTER | FTRACE_FL_ENABLED)) > - return 0; > - > - /* Record is not filtered or enabled, do nothing */ > - if (!fl) > - return 0; > - > - /* Record is not filtered but enabled, disable it */ > - if (fl == FTRACE_FL_ENABLED) > - rec->flags &= ~FTRACE_FL_ENABLED; > - else > - /* Otherwise record is filtered but not enabled, enable it */ > - rec->flags |= FTRACE_FL_ENABLED; > - } else { > - /* Disable or not filtered */ > - > - if (enable) { > - /* if record is enabled, do nothing */ > - if (rec->flags & FTRACE_FL_ENABLED) > - return 0; > - > - rec->flags |= FTRACE_FL_ENABLED; > - > - } else { > + if (enable && !(rec->flags & FTRACE_FL_NOTRACE)) { > + if (ftrace_filtered) { > + if (rec->flags & FTRACE_FL_FILTER) > + flag = FTRACE_FL_ENABLED; > + } else > + flag = FTRACE_FL_ENABLED;
The above can be factorized in
if (!ftrace_filtered || rec->flags & FTRACE_FL_FILTER) flag = FTRACE_FL_ENABLED
> + } > > - /* if record is not enabled, do nothing */ > - if (!(rec->flags & FTRACE_FL_ENABLED)) > - return 0; > + /* If not change this record's state, then do nothing */ > + if ((rec->flags & FTRACE_FL_ENABLED) == flag) > + return 0; > > - rec->flags &= ~FTRACE_FL_ENABLED; > - } > + if (flag) { > + rec->flags |= FTRACE_FL_ENABLED; > + return ftrace_make_call(rec, ftrace_addr); > } > > - if (rec->flags & FTRACE_FL_ENABLED) > - return ftrace_make_call(rec, ftrace_addr); > - else > - return ftrace_make_nop(NULL, rec, ftrace_addr); > + rec->flags &= ~FTRACE_FL_ENABLED; > + return ftrace_make_nop(NULL, rec, ftrace_addr); > } > > static void ftrace_replace_code(int enable) > -- > 1.6.1.2 > >
Nice simplification. I'm queuing it for .32 (I will just change with my comment above).
Thanks!
| |