Re: [RFC PATCH 10/32] function_graph: Have the instances use their own ftrace_ops for filtering

Masami Hiramatsu
2023-11-07T01:48:02Z

On Mon, 6 Nov 2023 01:08:32 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Steven Rostedt (VMware) <rostedt@goodmis.org>
>
> Allow for instances to have their own ftrace_ops part of the fgraph_ops that
> makes the funtion_graph tracer filter on the set_ftrace_filter file of the
> instance and not the top instance.
>
> Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> include/linux/ftrace.h | 1 +
> kernel/trace/fgraph.c | 60 +++++++++++++++++++++-------------
> kernel/trace/ftrace.c | 6 ++-
> kernel/trace/trace.h | 16 +++++----
> kernel/trace/trace_functions.c | 2 +
> kernel/trace/trace_functions_graph.c | 8 +++--
> 6 files changed, 58 insertions(+), 35 deletions(-)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 7fd044ae3da5..9dab365c6023 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -1044,6 +1044,7 @@ extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace, struct fgraph
> struct fgraph_ops {
> trace_func_graph_ent_t entryfunc;
> trace_func_graph_ret_t retfunc;
> + struct ftrace_ops ops; /* for the hash lists */
> void *private;
> };
>
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 1e8c17f70b84..0642f3281b64 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -17,14 +17,6 @@
> #include "ftrace_internal.h"
> #include "trace.h"
>
> -#ifdef CONFIG_DYNAMIC_FTRACE
> -#define ASSIGN_OPS_HASH(opsname, val) \
> - .func_hash = val, \
> - .local_hash.regex_lock = __MUTEX_INITIALIZER(opsname.local_hash.regex_lock),
> -#else
> -#define ASSIGN_OPS_HASH(opsname, val)
> -#endif
> -
> #define FGRAPH_RET_SIZE sizeof(struct ftrace_ret_stack)
> #define FGRAPH_RET_INDEX (FGRAPH_RET_SIZE / sizeof(long))
>
> @@ -338,9 +330,6 @@ int function_graph_enter(unsigned long ret, unsigned long func,
> return -EBUSY;
> #endif
>
> - if (!ftrace_ops_test(&global_ops, func, NULL))
> - return -EBUSY;
> -
> trace.func = func;
> trace.depth = ++current->curr_ret_depth;
>
> @@ -361,7 +350,8 @@ int function_graph_enter(unsigned long ret, unsigned long func,
> atomic_inc(&current->trace_overrun);
> break;
> }
> - if (fgraph_array[i]->entryfunc(&trace, fgraph_array[i])) {
> + if (ftrace_ops_test(&gops->ops, func, NULL) &&
> + gops->entryfunc(&trace, gops)) {
> offset = current->curr_ret_stack;
> /* Check the top level stored word */
> type = get_fgraph_type(current, offset - 1);
> @@ -656,17 +646,25 @@ unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
> }
> #endif /* HAVE_FUNCTION_GRAPH_RET_ADDR_PTR */
>
> -static struct ftrace_ops graph_ops = {
> - .func = ftrace_graph_func,
> - .flags = FTRACE_OPS_FL_INITIALIZED |
> - FTRACE_OPS_FL_PID |
> - FTRACE_OPS_GRAPH_STUB,
> +void fgraph_init_ops(struct ftrace_ops *dst_ops,
> + struct ftrace_ops *src_ops)
> +{
> + dst_ops->func = ftrace_stub;
> + dst_ops->flags = FTRACE_OPS_FL_PID | FTRACE_OPS_FL_STUB;

This needs to use FTRACE_OPS_GRAPH_STUB instead of FTRACE_OPS_FL_STUB,
because commit 0c0593b45c9b ("x86/ftrace: Make function graph use ftrace
directly") introduced this flag to switch the mode. (fgraph on ftrace)

> +
> #ifdef FTRACE_GRAPH_TRAMP_ADDR
> - .trampoline = FTRACE_GRAPH_TRAMP_ADDR,
> + dst_ops->trampoline = FTRACE_GRAPH_TRAMP_ADDR;
> /* trampoline_size is only needed for dynamically allocated tramps */
> #endif
> - ASSIGN_OPS_HASH(graph_ops, &global_ops.local_hash)
> -};
> +
> +#ifdef CONFIG_DYNAMIC_FTRACE
> + if (src_ops) {
> + dst_ops->func_hash = &src_ops->local_hash;
> + mutex_init(&dst_ops->local_hash.regex_lock);
> + dst_ops->flags |= FTRACE_OPS_FL_INITIALIZED;
> + }
> +#endif
> +}
>
> void ftrace_graph_sleep_time_control(bool enable)
> {
> @@ -871,11 +869,20 @@ static int start_graph_tracing(void)
>
> int register_ftrace_graph(struct fgraph_ops *gops)
> {
> + int command = 0;
> int ret = 0;
> int i;
>
> mutex_lock(&ftrace_lock);
>
> + if (!gops->ops.func) {
> + gops->ops.flags |= FTRACE_OPS_FL_STUB;

Ditto.

Thanks,

--
Masami Hiramatsu (Google) <mhiramat@kernel.org>