On Wed, 19 Feb 2025 17:04:40 -0500
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> From: Steven Rostedt <rostedt@xxxxxxxxxxx>
>
> When adding a new fprobe, it will update the function hash to the
> functions the fprobe is attached to and register with function graph to
> have it call the registered functions. The fprobe_graph_active variable
> keeps track of the number of fprobes that are using function graph.
>
> If two fprobes attach to the same function, it increments the
> fprobe_graph_active for each of them. But when they are removed, the first
> fprobe to be removed will see that the function it is attached to is also
> used by another fprobe and it will not remove that function from
> function_graph. The logic will skip decrementing the fprobe_graph_active
> variable.
>
> This causes the fprobe_graph_active variable to not go to zero when all
> fprobes are removed, and in doing so it does not unregister from
> function graph. As the fgraph ops hash will now be empty, and an empty
> filter hash means all functions are enabled, this triggers function graph
> to add a callback to the fprobe infrastructure for every function!
>
> # echo "f:myevent1 kernel_clone" >> /sys/kernel/tracing/dynamic_events
> # echo "f:myevent2 kernel_clone%return" >> /sys/kernel/tracing/dynamic_events
> # cat /sys/kernel/tracing/enabled_functions
> kernel_clone (1) tramp: 0xffffffffc0024000 (ftrace_graph_func+0x0/0x60) ->ftrace_graph_func+0x0/0x60
>
> # > /sys/kernel/tracing/dynamic_events
> # cat /sys/kernel/tracing/enabled_functions
> trace_initcall_start_cb (1) tramp: 0xffffffffc0026000 (function_trace_call+0x0/0x170) ->function_trace_call+0x0/0x170
> run_init_process (1) tramp: 0xffffffffc0026000 (function_trace_call+0x0/0x170) ->function_trace_call+0x0/0x170
> try_to_run_init_process (1) tramp: 0xffffffffc0026000 (function_trace_call+0x0/0x170) ->function_trace_call+0x0/0x170
> x86_pmu_show_pmu_cap (1) tramp: 0xffffffffc0026000 (function_trace_call+0x0/0x170) ->function_trace_call+0x0/0x170
> cleanup_rapl_pmus (1) tramp: 0xffffffffc0026000 (function_trace_call+0x0/0x170) ->function_trace_call+0x0/0x170
> uncore_free_pcibus_map (1) tramp: 0xffffffffc0026000 (function_trace_call+0x0/0x170) ->function_trace_call+0x0/0x170
> uncore_types_exit (1) tramp: 0xffffffffc0026000 (function_trace_call+0x0/0x170) ->function_trace_call+0x0/0x170
> uncore_pci_exit.part.0 (1) tramp: 0xffffffffc0026000 (function_trace_call+0x0/0x170) ->function_trace_call+0x0/0x170
> kvm_shutdown (1) tramp: 0xffffffffc0026000 (function_trace_call+0x0/0x170) ->function_trace_call+0x0/0x170
> vmx_dump_msrs (1) tramp: 0xffffffffc0026000 (function_trace_call+0x0/0x170) ->function_trace_call+0x0/0x170
> [..]
>
> # cat /sys/kernel/tracing/enabled_functions | wc -l
> 54702
>
> If a fprobe is being removed and all its functions are also traced by
> other fprobes, still decrement the fprobe_graph_active counter.
>
This looks good to me.
Acked-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
And please push this series via your branch because the selftest
updates should cover the series.
Thanks!
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 4346ba1604093 ("fprobe: Rewrite fprobe on function-graph tracer")
> Closes: https://lore.kernel.org/all/20250217114918.10397-A-hca@xxxxxxxxxxxxx/
> Reported-by: Heiko Carstens <hca@xxxxxxxxxxxxx>
> Signed-off-by: Steven Rostedt (Google) <rostedt@xxxxxxxxxxx>
> ---
> Changes since v1: https://lore.kernel.org/20250218193126.619197190@xxxxxxxxxxx
>
> - Move the check into fprobe_graph_remove_ips() to keep it matching
> with fprobe_graph_add_ips() (Masami Hiramatsu)
>
> kernel/trace/fprobe.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 62e8f7d56602..33082c4e8154 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -407,7 +407,8 @@ static void fprobe_graph_remove_ips(unsigned long *addrs, int num)
> if (!fprobe_graph_active)
> unregister_ftrace_graph(&fprobe_graph_ops);
>
> - ftrace_set_filter_ips(&fprobe_graph_ops.ops, addrs, num, 1, 0);
> + if (num)
> + ftrace_set_filter_ips(&fprobe_graph_ops.ops, addrs, num, 1, 0);
> }
>
> static int symbols_cmp(const void *a, const void *b)
> @@ -677,8 +678,7 @@ int unregister_fprobe(struct fprobe *fp)
> }
> del_fprobe_hash(fp);
>
> - if (count)
> - fprobe_graph_remove_ips(addrs, count);
> + fprobe_graph_remove_ips(addrs, count);
>
> kfree_rcu(hlist_array, rcu);
> fp->hlist_array = NULL;
> --
> 2.47.2
>
>
--
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
