On Fri, 18 Oct 2024 21:43:00 -0400 Steven Rostedt <[email protected]> wrote:
> From: Steven Rostedt <[email protected]> > > The function graph infrastructure allocates a shadow stack for every task > when enabled. This includes the idle tasks. The first time the function > graph is invoked, the shadow stacks are created and never freed until the > task exits. This includes the idle tasks. > > Only the idle tasks that were for online CPUs had their shadow stacks > created when function graph tracing started. If function graph tracing is > enabled and a CPU comes online, the idle task representing that CPU will > not have its shadow stack created, and all function graph tracing for that > idle task will be silently dropped. > > Instead, use the CPU hotplug mechanism to allocate the idle shadow stacks. > This will include idle tasks for CPUs that come online during tracing. > > This issue can be reproduced by: > > # cd /sys/kernel/tracing > # echo 0 > /sys/devices/system/cpu/cpu1/online > # echo 0 > set_ftrace_pid > # echo function_graph > current_tracer > # echo 1 > options/funcgraph-proc > # echo 1 > /sys/devices/system/cpu/cpu1 > # grep '<idle>' per_cpu/cpu1/trace | head > > Before, nothing would show up. > > After: > 1) <idle>-0 | 0.811 us | > __enqueue_entity(); > 1) <idle>-0 | 5.626 us | } /* enqueue_entity > */ > 1) <idle>-0 | | > dl_server_update_idle_time() { > 1) <idle>-0 | | > dl_scaled_delta_exec() { > 1) <idle>-0 | 0.450 us | > arch_scale_cpu_capacity(); > 1) <idle>-0 | 1.242 us | } > 1) <idle>-0 | 1.908 us | } > 1) <idle>-0 | | dl_server_start() { > 1) <idle>-0 | | > enqueue_dl_entity() { > 1) <idle>-0 | | > task_contending() { > > Note, if tracing stops and restarts, the old way would then initialize > the onlined CPUs. > Looks good to me, except one comment below; Acked-by: Masami Hiramatsu (Google) <[email protected]> [...] > int register_ftrace_graph(struct fgraph_ops *gops) > { > + static bool fgraph_initialized; > int command = 0; > int ret = 0; > int i = -1; > > mutex_lock(&ftrace_lock); > > + if (!fgraph_initialized) { > + ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "fgraph_idle_init", Nit: Maybe it is better to call it as "tracing/fgraph:online" ? Thank you, > + fgraph_cpu_init, NULL); > + if (ret < 0) { > + pr_warn("fgraph: Error to init cpu hotplug support\n"); > + return ret; > + } > + fgraph_initialized = true; > + ret = 0; > + } > + > if (!fgraph_array[0]) { > /* The array must always have real data on it */ > for (i = 0; i < FGRAPH_ARRAY_SIZE; i++) > -- > 2.45.2 > -- Masami Hiramatsu (Google) <[email protected]>
