On Tue, 18 Feb 2025 14:30:36 -0500
Steven Rostedt <rost...@goodmis.org> wrote:
> From: Steven Rostedt <rost...@goodmis.org>
>
> 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.
>
Ah, thanks! But I would like to change the fix a bit since
fprobe_graph_remove_ips() must be paired with fprobe_graph_add_ips()
to hide fprobe_graph_active counting.
Can you use this version?
diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index 2560b312ad57..7d282c08549e 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -409,7 +409,8 @@ static void fprobe_graph_remove_ips(unsigned long *addrs,
int num)
return;
}
- 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)
@@ -679,8 +680,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;
> Cc: sta...@vger.kernel.org
> Fixes: 4346ba1604093 ("fprobe: Rewrite fprobe on function-graph tracer")
> Closes: https://lore.kernel.org/all/20250217114918.10397-a-...@linux.ibm.com/
> Reported-by: Heiko Carstens <h...@linux.ibm.com>
> Signed-off-by: Steven Rostedt (Google) <rost...@goodmis.org>
> ---
> kernel/trace/fprobe.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
> index 2560b312ad57..90241091ca61 100644
> --- a/kernel/trace/fprobe.c
> +++ b/kernel/trace/fprobe.c
> @@ -681,6 +681,8 @@ int unregister_fprobe(struct fprobe *fp)
>
> if (count)
> fprobe_graph_remove_ips(addrs, count);
> + else
> + fprobe_graph_active--;
>
> kfree_rcu(hlist_array, rcu);
> fp->hlist_array = NULL;
> --
> 2.47.2
>
>
--
Masami Hiramatsu (Google) <mhira...@kernel.org>
