Re: [BUG] tracing: dynamic ftrace selftest detected failures

Masami Hiramatsu
2024-08-21T00:21:50Z

On Wed, 21 Aug 2024 08:43:51 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> On Tue, 20 Aug 2024 18:11:09 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
> > On Wed, 21 Aug 2024 07:05:39 +0900
> > Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> >
> >
> > > Does the noinline attribute prevent embedding callsite too? I mean
> > >
> > > extern callee()
> > >
> > > noinline callee()
> > > {
> > > ...
> > > }
> > >
> > > caller()
> > > {
> > > callee() // (*)
> > > }
> > >
> > > In this case, does noinline prevent LTO to embed the callee at the callsite(*)
> > > or prevent LTO remove the callee() symbol?
> > >
> >
> > Even though we have it passed as a parameter, I think the compiler and
> > linker is smart enough to see that and notice its use, and that the
> > function passed in is a nop, which doesn't break the flow.
> >
> > Can you add the __used and see if it fixes it?
>
> Adding __used to DYN_FTRACE_TEST_NAME() and DYN_FTRACE_TEST_NAME2() does
> not change, the test still fails. Hmm, what about makes the caller
> (trace_selftest_startup_dynamic_tracing()) called via a function pointer?
> In that case, wouldn't it be subject to constant propagetion?
>
> Let me try.

OK, it is succeeded! Calling `caller` via global function pointer makes
it run as we expected. It passed the dynamic_ftrace test, but other tests
still fails. Those need to be called via function pointer too.

[ 1.851324] Testing dynamic ftrace: PASSED
[ 2.083329] Testing dynamic ftrace ops #1:
[ 2.173751] (0 0 0 0 0) FAILED!
[ 2.182337] ------------[ cut here ]------------
[ 2.183323] WARNING: CPU: 0 PID: 1 at kernel/trace/trace.c:2143 run_tracer_sel0
[ 2.184323] Modules linked in:

Anyway, here is what I did.

diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index 97f1e4bc47dc..9663bc777888 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -353,9 +353,10 @@ static int trace_selftest_ops(struct trace_array *tr, int cnt)
}

/* Test dynamic code modification and ftrace filters */
-static int trace_selftest_startup_dynamic_tracing(struct tracer *trace,
- struct trace_array *tr,
- int (*func)(void))
+static int noinline
+trace_selftest_startup_dynamic_tracing(struct tracer *trace,
+ struct trace_array *tr,
+ int (*func)(void))
{
int save_ftrace_enabled = ftrace_enabled;
unsigned long count;
@@ -569,10 +570,22 @@ trace_selftest_function_recursion(void)
return ret;
}
#else
-# define trace_selftest_startup_dynamic_tracing(trace, tr, func) ({ 0; })
+static int trace_selftest_startup_dynamic_tracing(struct tracer *trace,
+ struct trace_array *tr,
+ int (*func)(void))
+{
+ if (!trace || !tr || !func)
+ return -EINVAL;
+ return 0;
+}
# define trace_selftest_function_recursion() ({ 0; })
#endif /* CONFIG_DYNAMIC_FTRACE */

+int (*global_trace_selftest_startup_dynamic_tracing)(struct tracer *trace,
+ struct trace_array *tr,
+ int (*func)(void))
+ = trace_selftest_startup_dynamic_tracing;
+
static enum {
TRACE_SELFTEST_REGS_START,
TRACE_SELFTEST_REGS_FOUND,
@@ -732,7 +745,7 @@ trace_selftest_startup_function(struct tracer *trace, struct trace_array *tr)
goto out;
}

- ret = trace_selftest_startup_dynamic_tracing(trace, tr,
+ ret = global_trace_selftest_startup_dynamic_tracing(trace, tr,
DYN_FTRACE_TEST_NAME);
if (ret)
goto out;

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>