ftrace: Do not have print_graph_retval() add a newline
authorSteven Rostedt <rostedt@goodmis.org>
Fri, 11 Apr 2025 17:30:15 +0000 (13:30 -0400)
committerSteven Rostedt (Google) <rostedt@goodmis.org>
Sat, 12 Apr 2025 16:13:30 +0000 (12:13 -0400)
The retval and retaddr options for function_graph tracer will add a
comment at the end of a function for both leaf and non leaf functions that
looks like:

               __wake_up_common(); /* ret=0x1 */

               } /* pick_next_task_fair ret=0x0 */

The function print_graph_retval() adds a newline after the "*/". But if
that's not called, the caller function needs to make sure there's a
newline added.

This is confusing and when the function parameters code was added, it
added a newline even when calling print_graph_retval() as the fact that
the print_graph_retval() function prints a newline isn't obvious.

This caused an extra newline to be printed and that made it fail the
selftests when the retval option was set, as the selftests were not
expecting blank lines being injected into the trace.

Instead of having print_graph_retval() print a newline, just have the
caller always print the newline regardless if it calls print_graph_retval()
or not. This not only fixes this bug, but it also simplifies the code.

Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Link: https://lore.kernel.org/20250411133015.015ca393@gandalf.local.home
Reported-by: Mark Brown <broonie@kernel.org>
Tested-by: Mark Brown <broonie@kernel.org>
Closes: https://lore.kernel.org/all/ccc40f2b-4b9e-4abd-8daf-d22fce2a86f0@sirena.org.uk/
Fixes: ff5c9c576e754 ("ftrace: Add support for function argument to graph tracer")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
kernel/trace/trace_functions_graph.c

index 2f077d4158e5f6cc5f320a611baf67b89948b615..0c357a89c58e01ea0b437e8cb71bfd63368ac065 100644 (file)
@@ -880,8 +880,6 @@ static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entr
 
                if (print_retval || print_retaddr)
                        trace_seq_puts(s, " /*");
-               else
-                       trace_seq_putc(s, '\n');
        } else {
                print_retaddr = false;
                trace_seq_printf(s, "} /* %ps", func);
@@ -899,7 +897,7 @@ static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entr
        }
 
        if (!entry || print_retval || print_retaddr)
-               trace_seq_puts(s, " */\n");
+               trace_seq_puts(s, " */");
 }
 
 #else
@@ -975,7 +973,7 @@ print_graph_entry_leaf(struct trace_iterator *iter,
                } else
                        trace_seq_puts(s, "();");
        }
-       trace_seq_printf(s, "\n");
+       trace_seq_putc(s, '\n');
 
        print_graph_irq(iter, graph_ret->func, TRACE_GRAPH_RET,
                        cpu, iter->ent->pid, flags);
@@ -1313,10 +1311,11 @@ print_graph_return(struct ftrace_graph_ret_entry *retentry, struct trace_seq *s,
                 * that if the funcgraph-tail option is enabled.
                 */
                if (func_match && !(flags & TRACE_GRAPH_PRINT_TAIL))
-                       trace_seq_puts(s, "}\n");
+                       trace_seq_puts(s, "}");
                else
-                       trace_seq_printf(s, "} /* %ps */\n", (void *)func);
+                       trace_seq_printf(s, "} /* %ps */", (void *)func);
        }
+       trace_seq_putc(s, '\n');
 
        /* Overrun */
        if (flags & TRACE_GRAPH_PRINT_OVERRUN)