Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1573115

Re: [for-next][PATCH 2/8] ftrace: Create a slight optimization on searching the ftrace_hash

Show all headers | View raw

Hi Steve,

On Fri, Feb 3, 2017 at 10:40 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>
>
> This is a micro-optimization, but as it has to deal with a fast path of the
> function tracer, these optimizations can be noticed.
>
> The ftrace_lookup_ip() returns true if the given ip is found in the hash. If
> it's not found or the hash is NULL, it returns false. But there's some cases
> that a NULL hash is a true, and the ftrace_hash_empty() is tested before
> calling ftrace_lookup_ip() in those cases. But as ftrace_lookup_ip() tests
> that first, that adds a few extra unneeded instructions in those cases.
>
> A new static "always_inlined" function is created that does not perform the
> hash empty test. This most only be used by callers that do the check first
> anyway, as an empty or NULL hash could cause a crash if a lookup is
> performed on it.
>
> Also add kernel doc for the ftrace_lookup_ip() main function.

It'd be nice if ftrace_graph_addr() was changed also.

Thanks,
Namhyung


>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> ---
>  kernel/trace/ftrace.c | 33 +++++++++++++++++++++++++--------
>  1 file changed, 25 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 89240f62061c..1595df0d7d79 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -1194,16 +1194,14 @@ ftrace_hash_key(struct ftrace_hash *hash, unsigned long ip)
>         return 0;
>  }
>
> -struct ftrace_func_entry *
> -ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip)
> +/* Only use this function if ftrace_hash_empty() has already been tested */
> +static __always_inline struct ftrace_func_entry *
> +__ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip)
>  {
>         unsigned long key;
>         struct ftrace_func_entry *entry;
>         struct hlist_head *hhd;
>
> -       if (ftrace_hash_empty(hash))
> -               return NULL;
> -
>         key = ftrace_hash_key(hash, ip);
>         hhd = &hash->buckets[key];
>
> @@ -1214,6 +1212,25 @@ ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip)
>         return NULL;
>  }
>
> +/**
> + * ftrace_lookup_ip - Test to see if an ip exists in an ftrace_hash
> + * @hash: The hash to look at
> + * @ip: The instruction pointer to test
> + *
> + * Search a given @hash to see if a given instruction pointer (@ip)
> + * exists in it.
> + *
> + * Returns the entry that holds the @ip if found. NULL otherwise.
> + */
> +struct ftrace_func_entry *
> +ftrace_lookup_ip(struct ftrace_hash *hash, unsigned long ip)
> +{
> +       if (ftrace_hash_empty(hash))
> +               return NULL;
> +
> +       return __ftrace_lookup_ip(hash, ip);
> +}
> +
>  static void __add_hash_entry(struct ftrace_hash *hash,
>                              struct ftrace_func_entry *entry)
>  {
> @@ -1463,9 +1480,9 @@ static bool hash_contains_ip(unsigned long ip,
>          * notrace hash is considered not in the notrace hash.
>          */
>         return (ftrace_hash_empty(hash->filter_hash) ||
> -               ftrace_lookup_ip(hash->filter_hash, ip)) &&
> +               __ftrace_lookup_ip(hash->filter_hash, ip)) &&
>                 (ftrace_hash_empty(hash->notrace_hash) ||
> -                !ftrace_lookup_ip(hash->notrace_hash, ip));
> +                !__ftrace_lookup_ip(hash->notrace_hash, ip));
>  }
>
>  /*
> @@ -2877,7 +2894,7 @@ ops_references_rec(struct ftrace_ops *ops, struct dyn_ftrace *rec)
>
>         /* The function must be in the filter */
>         if (!ftrace_hash_empty(ops->func_hash->filter_hash) &&
> -           !ftrace_lookup_ip(ops->func_hash->filter_hash, rec->ip))
> +           !__ftrace_lookup_ip(ops->func_hash->filter_hash, rec->ip))
>                 return 0;
>
>         /* If in notrace hash, we ignore it too */
> --
> 2.10.2
>
>

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[for-next][PATCH 0/8] tracing: Clean up hash logic for set_graph_function Steven Rostedt <rostedt@goodmis.org> - 2017-02-03 14:50 +0100
  [for-next][PATCH 1/8] tracing: Add ftrace_hash_key() helper function Steven Rostedt <rostedt@goodmis.org> - 2017-02-03 14:50 +0100
  [for-next][PATCH 3/8] ftrace: Replace (void *)1 with a meaningful macro name  FTRACE_GRAPH_EMPTY Steven Rostedt <rostedt@goodmis.org> - 2017-02-03 14:50 +0100
  [for-next][PATCH 7/8] ftrace: Do not hold references of ftrace_graph_{notrace_}hash out of  graph_lock Steven Rostedt <rostedt@goodmis.org> - 2017-02-03 14:50 +0100
  [for-next][PATCH 8/8] ftrace: Have set_graph_function handle multiple functions in one  write Steven Rostedt <rostedt@goodmis.org> - 2017-02-03 14:50 +0100
  [for-next][PATCH 4/8] ftrace: Reset fgd->hash in ftrace_graph_write() Steven Rostedt <rostedt@goodmis.org> - 2017-02-03 14:50 +0100
    Re: [for-next][PATCH 4/8] ftrace: Reset fgd->hash in ftrace_graph_write() Namhyung Kim <namhyung@kernel.org> - 2017-02-03 16:00 +0100
      Re: [for-next][PATCH 4/8] ftrace: Reset fgd->hash in  ftrace_graph_write() Steven Rostedt <rostedt@goodmis.org> - 2017-02-03 16:00 +0100
  [for-next][PATCH 2/8] ftrace: Create a slight optimization on searching the ftrace_hash Steven Rostedt <rostedt@goodmis.org> - 2017-02-03 14:50 +0100
    Re: [for-next][PATCH 2/8] ftrace: Create a slight optimization on  searching the ftrace_hash Namhyung Kim <namhyung@kernel.org> - 2017-02-03 15:30 +0100
      Re: [for-next][PATCH 2/8] ftrace: Create a slight optimization on  searching the ftrace_hash Steven Rostedt <rostedt@goodmis.org> - 2017-02-03 16:00 +0100
  [for-next][PATCH 6/8] tracing: Reset parser->buffer to allow multiple "puts" Steven Rostedt <rostedt@goodmis.org> - 2017-02-03 14:50 +0100
  [for-next][PATCH 5/8] ftrace: Have set_graph_functions handle write with RDWR Steven Rostedt <rostedt@goodmis.org> - 2017-02-03 14:50 +0100
  Re: [for-next][PATCH 0/8] tracing: Clean up hash logic for set_graph_function Namhyung Kim <namhyung@kernel.org> - 2017-02-03 16:20 +0100

csiph-web