lkml.org 
[lkml]   [2014]   [Nov]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/6] trace: Replace seq_printf by simpler equivalents
On Fri, 12 Sep 2014 11:25:52 +0200
Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:

> Using seq_printf to print a simple string or a single character is a
> lot more expensive than it needs to be, since seq_puts and seq_putc
> exist.
>
> These patches do
>
> seq_printf(m, s) -> seq_puts(m, s)
> seq_printf(m, "%s", s) -> seq_puts(m, s)
> seq_printf(m, "%c", c) -> seq_putc(m, c)
>
> Subsequent patches will simplify further.
>
> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
> ---
> kernel/trace/ftrace.c | 30 ++++++++++++------------
> kernel/trace/trace.c | 44 ++++++++++++++++++------------------
> kernel/trace/trace_branch.c | 24 +++++++++-----------
> kernel/trace/trace_events.c | 4 ++--
> kernel/trace/trace_events_trigger.c | 2 +-
> kernel/trace/trace_functions.c | 2 +-
> kernel/trace/trace_functions_graph.c | 28 +++++++++++------------
> kernel/trace/trace_kprobe.c | 4 ++--
> kernel/trace/trace_uprobe.c | 2 +-
> 9 files changed, 69 insertions(+), 71 deletions(-)
>
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 5916a8e..7b9ce28 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -556,13 +556,13 @@ static int function_stat_cmp(void *p1, void *p2)
> static int function_stat_headers(struct seq_file *m)
> {
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> - seq_printf(m, " Function "
> - "Hit Time Avg s^2\n"
> - " -------- "
> - "--- ---- --- ---\n");
> + seq_puts(m,
> + " Function " "Hit Time Avg s^2\n"
> + " -------- " "--- ---- --- ---\n");

Please keep the original format. I know that it's considered bad form
to split strings like this, but I consider this one of the exceptions
to the rule.

> #else
> - seq_printf(m, " Function Hit\n"
> - " -------- ---\n");
> + seq_puts(m,
> + " Function Hit\n"
> + " -------- ---\n");
> #endif
> return 0;
> }



> @@ -3250,7 +3250,7 @@ static int t_show(struct seq_file *m, void *v)
> if (!t)
> return 0;
>
> - seq_printf(m, "%s", t->name);
> + seq_puts(m, t->name);

This is wrong and dangerous.

What happens if "t->name" contains "%d" or "%s"?


> if (t->next)
> seq_putc(m, ' ');
> else
> @@ -5752,10 +5752,10 @@ ftrace_snapshot_print(struct seq_file *m, unsigned long ip,
>
> seq_printf(m, "%ps:", (void *)ip);
>
> - seq_printf(m, "snapshot");
> + seq_puts(m, "snapshot");
>
> if (count == -1)
> - seq_printf(m, ":unlimited\n");
> + seq_puts(m, ":unlimited\n");
> else
> seq_printf(m, ":count=%ld\n", count);
>


> static int all_branch_stat_headers(struct seq_file *m)
> {
> - seq_printf(m, " miss hit %% ");
> - seq_printf(m, " Function "
> - " File Line\n"
> - " ------- --------- - "
> - " -------- "
> - " ---- ----\n");
> + seq_puts(m, " miss hit % ");
> + seq_puts(m,
> + " Function " " File Line\n" " ------- --------- - " " -------- " " ---- ----\n");

Again, keep the original formatting. We can fix it up later, but not
with this patch.


> return 0;
> }
>


> --- a/kernel/trace/trace_events_trigger.c
> +++ b/kernel/trace/trace_events_trigger.c
> @@ -373,7 +373,7 @@ event_trigger_print(const char *name, struct seq_file *m,
> {
> long count = (long)data;
>
> - seq_printf(m, "%s", name);
> + seq_puts(m, name);

Again, this is wrong and dangerous.

-- Steve

>
> if (count == -1)
> seq_puts(m, ":unlimited");




\
 
 \ /
  Last update: 2014-11-06 00:01    [W:0.145 / U:0.896 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site