lkml.org 
[lkml]   [2009]   [Jul]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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!



\
 
 \ /
  Last update: 2009-07-17 06:07    [W:0.068 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site