Messages in this thread | | | Date | Fri, 26 Oct 2018 17:41:33 -0400 | From | Steven Rostedt <> | Subject | Re: [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);
| |