Re: [PATCH] fgraph: Use CPU hotplug mechanism to initialize idle shadow stacks

[Steven Rostedt]

On Wed, 11 Dec 2024 15:23:05 +0100
Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:

> If I boot without any tracing enabled from the cmdline and:
> 
> echo 0 > tracing_on
> echo function_graph > current_tracer
> echo do_idle > set_graph_function
> echo 1 > tracing_on
> 
> I don't get any output either.
> 
> It works for other functions, such as
> 
> echo ktime_get > set_graph_function
> 
> It seems it's the set_graph_function thing that isn't working
> with do_idle at all after this patch. Why just this function...
> The function is clearly there:
> 
> cat available_filter_functions | grep do_idle
> do_idle
> 
> I can also verify that this function is indeed getting invoked
> by adding prints to it (it's invoked all the time on any normal
> system). Does this have something to do with the context
> where do_idle is called? It's all really confusing...

Yeah, I figured it out. That commit moved the initialization before
fgraph was registered, and we had in ftrace_graph_init_idle_task():

        if (ftrace_graph_active) {
                unsigned long *ret_stack;
                        
                ret_stack = per_cpu(idle_ret_stack, cpu);
                if (!ret_stack) {
                        ret_stack = kmalloc(SHADOW_STACK_SIZE, GFP_KERNEL);
                        if (!ret_stack)
                                return;
                        per_cpu(idle_ret_stack, cpu) = ret_stack;
                }
                graph_init_task(t, ret_stack);
        }

But because ftrace_graph_active was not set yet, the initialization
didn't happen.

Can you try this patch?

-- Steve

diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 43f4e3f57438..4706a7dce93a 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -1160,13 +1160,19 @@ void fgraph_update_pid_func(void)
 static int start_graph_tracing(void)
 {
 	unsigned long **ret_stack_list;
-	int ret;
+	int ret, cpu;
 
 	ret_stack_list = kmalloc(SHADOW_STACK_SIZE, GFP_KERNEL);
 
 	if (!ret_stack_list)
 		return -ENOMEM;
 
+	/* The cpu_boot init_task->ret_stack will never be freed */
+	for_each_online_cpu(cpu) {
+		if (!idle_task(cpu)->ret_stack)
+			ftrace_graph_init_idle_task(idle_task(cpu), cpu);
+	}
+
 	do {
 		ret = alloc_retstack_tasklist(ret_stack_list);
 	} while (ret == -EAGAIN);