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

> On Tue, 20 Aug 2024 18:11:09 -0400
> Steven Rostedt <rost...@goodmis.org> wrote:
> 
> > On Wed, 21 Aug 2024 07:05:39 +0900
> > Masami Hiramatsu (Google) <mhira...@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) <mhira...@kernel.org>

Reply via email to