lkml.org 
[lkml]   [2018]   [Oct]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] tracing: merge seq_print_sym_short() and seq_print_sym_offset()

Hi Ramus,

Thanks for sending these patches. I have some small nits though.

First, please send a cover letter whenever sending more than one patch.
It just groups them better in my inbox.

The second is embedded below.

On Fri, 26 Oct 2018 23:13:44 +0200
Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> These two functions are nearly identical, so we can avoid some code
> duplication by moving the conditional into a common implementation.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> kernel/trace/trace_output.c | 34 +++++++---------------------------
> 1 file changed, 7 insertions(+), 27 deletions(-)
>
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index 6e6cc64faa38..501311dcfca6 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -339,34 +339,17 @@ static inline const char *kretprobed(const char *name)
> #endif /* CONFIG_KRETPROBES */
>
> static void
> -seq_print_sym_short(struct trace_seq *s, const char *fmt, unsigned long address)
> +seq_print_sym(struct trace_seq *s, const char *fmt, unsigned long address,
> + bool with_offset)

Just call the variable "offset". It's rather obvious what that means.

The other patches look good, but can you send a v2 of the series, with
these updates?

Thanks!

-- Steve


> {
> char str[KSYM_SYMBOL_LEN];
> #ifdef CONFIG_KALLSYMS
> const char *name;
>
> - kallsyms_lookup(address, NULL, NULL, NULL, str);
> -
> - name = kretprobed(str);
> -
> - if (name && strlen(name)) {
> - trace_seq_printf(s, fmt, name);
> - return;
> - }
> -#endif
> - snprintf(str, KSYM_SYMBOL_LEN, "0x%08lx", address);
> - trace_seq_printf(s, fmt, str);
> -}
> -
> -static void
> -seq_print_sym_offset(struct trace_seq *s, const char *fmt,
> - unsigned long address)
> -{
> - char str[KSYM_SYMBOL_LEN];
> -#ifdef CONFIG_KALLSYMS
> - const char *name;
> -
> - sprint_symbol(str, address);
> + if (with_offset)
> + sprint_symbol(str, address);
> + else
> + kallsyms_lookup(address, NULL, NULL, NULL, str);
> name = kretprobed(str);
>
> if (name && strlen(name)) {
> @@ -424,10 +407,7 @@ seq_print_ip_sym(struct trace_seq *s, unsigned long ip, unsigned long sym_flags)
> goto out;
> }
>
> - if (sym_flags & TRACE_ITER_SYM_OFFSET)
> - seq_print_sym_offset(s, "%s", ip);
> - else
> - seq_print_sym_short(s, "%s", ip);
> + seq_print_sym(s, "%s", ip, sym_flags & TRACE_ITER_SYM_OFFSET);
>
> if (sym_flags & TRACE_ITER_SYM_ADDR)
> trace_seq_printf(s, " <" IP_FMT ">", ip);

\
 
 \ /
  Last update: 2018-10-26 23:43    [W:0.097 / U:25.908 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site