include/linux/ftrace.h | 16 ++++-------- kernel/trace/fgraph.c | 45 ++++++++++++++++++++++------------ kernel/trace/ftrace.c | 3 +- kernel/trace/trace.h | 3 +- kernel/trace/trace_functions_graph.c | 14 +++++------ kernel/trace/trace_irqsoff.c | 3 +- kernel/trace/trace_sched_wakeup.c | 3 +- kernel/trace/trace_selftest.c | 8 ++---- 8 files changed, 48 insertions(+), 47 deletions(-)
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Simplify return address printing in the function graph tracer by removing
fgraph_extras. Since this feature is only used by the function graph
tracer and the feature flags can directly accessible from the function
graph tracer, fgraph_extras can be removed from the fgraph callback.
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
include/linux/ftrace.h | 16 ++++--------
kernel/trace/fgraph.c | 45 ++++++++++++++++++++++------------
kernel/trace/ftrace.c | 3 +-
kernel/trace/trace.h | 3 +-
kernel/trace/trace_functions_graph.c | 14 +++++------
kernel/trace/trace_irqsoff.c | 3 +-
kernel/trace/trace_sched_wakeup.c | 3 +-
kernel/trace/trace_selftest.c | 8 ++----
8 files changed, 48 insertions(+), 47 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 2ac3b3b53cd0..997e1f0335b6 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1068,29 +1068,20 @@ struct ftrace_graph_ret {
unsigned long long rettime;
} __packed;
-struct fgraph_extras;
struct fgraph_ops;
/* Type of the callback handlers for tracing function graph*/
typedef void (*trace_func_graph_ret_t)(struct ftrace_graph_ret *,
struct fgraph_ops *); /* return */
typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
- struct fgraph_ops *,
- struct fgraph_extras *); /* entry */
+ struct fgraph_ops *); /* entry */
extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
- struct fgraph_ops *gops,
- struct fgraph_extras *extras);
+ struct fgraph_ops *gops);
bool ftrace_pids_enabled(struct ftrace_ops *ops);
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-/* Used to convey some extra datas when creating a graph entry */
-struct fgraph_extras {
- u32 flags;
- unsigned long retaddr;
-};
-
struct fgraph_ops {
trace_func_graph_ent_t entryfunc;
trace_func_graph_ret_t retfunc;
@@ -1131,12 +1122,15 @@ function_graph_enter(unsigned long ret, unsigned long func,
struct ftrace_ret_stack *
ftrace_graph_get_ret_stack(struct task_struct *task, int skip);
+unsigned long ftrace_graph_top_ret_addr(struct task_struct *task);
unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
unsigned long ret, unsigned long *retp);
unsigned long *fgraph_get_task_var(struct fgraph_ops *gops);
u32 graph_tracer_flags_get(u32 flags);
+#define graph_tracer_flags_is_set(flags) \
+ (graph_tracer_flags_get(flags) == (flags))
/*
* Sometimes we don't want to trace a function with the function
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 27e523f01ed2..ee829d65f301 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -290,8 +290,7 @@ static inline unsigned long make_data_type_val(int idx, int size, int offset)
}
/* ftrace_graph_entry set to this to tell some archs to run function graph */
-static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops,
- struct fgraph_extras *extras)
+static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops)
{
return 0;
}
@@ -519,8 +518,7 @@ int __weak ftrace_disable_ftrace_graph_caller(void)
#endif
int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
- struct fgraph_ops *gops,
- struct fgraph_extras *extras)
+ struct fgraph_ops *gops)
{
return 0;
}
@@ -648,20 +646,13 @@ int function_graph_enter(unsigned long ret, unsigned long func,
unsigned long frame_pointer, unsigned long *retp)
{
struct ftrace_graph_ent trace;
- struct fgraph_extras extras;
unsigned long bitmap = 0;
int offset;
int i;
- int idx = 0;
trace.func = func;
trace.depth = ++current->curr_ret_depth;
- extras.flags = graph_tracer_flags_get(TRACE_GRAPH_PRINT_RETADDR);
- if (IS_ENABLED(CONFIG_FUNCTION_GRAPH_RETADDR)
- && extras.flags & TRACE_GRAPH_PRINT_RETADDR)
- extras.retaddr = ftrace_graph_ret_addr(current, &idx, ret, retp);
-
offset = ftrace_push_return_trace(ret, func, frame_pointer, retp, 0);
if (offset < 0)
goto out;
@@ -670,7 +661,7 @@ int function_graph_enter(unsigned long ret, unsigned long func,
if (static_branch_likely(&fgraph_do_direct)) {
int save_curr_ret_stack = current->curr_ret_stack;
- if (static_call(fgraph_func)(&trace, fgraph_direct_gops, &extras))
+ if (static_call(fgraph_func)(&trace, fgraph_direct_gops))
bitmap |= BIT(fgraph_direct_gops->idx);
else
/* Clear out any saved storage */
@@ -688,7 +679,7 @@ int function_graph_enter(unsigned long ret, unsigned long func,
save_curr_ret_stack = current->curr_ret_stack;
if (ftrace_ops_test(&gops->ops, func, NULL) &&
- gops->entryfunc(&trace, gops, &extras))
+ gops->entryfunc(&trace, gops))
bitmap |= BIT(i);
else
/* Clear out any saved storage */
@@ -905,6 +896,29 @@ ftrace_graph_get_ret_stack(struct task_struct *task, int idx)
return ret_stack;
}
+/**
+ * ftrace_graph_top_ret_addr - return the top return address in the shadow stack
+ * @task: The task to read the shadow stack from.
+ *
+ * Return the first return address on the shadow stack of the @task, which is
+ * not the fgraph's return_to_handler.
+ */
+unsigned long ftrace_graph_top_ret_addr(struct task_struct *task)
+{
+ unsigned long return_handler = (unsigned long)dereference_kernel_function_descriptor(return_to_handler);
+ struct ftrace_ret_stack *ret_stack = NULL;
+ int offset = task->curr_ret_stack;
+
+ if (offset < 0)
+ return 0;
+
+ do {
+ ret_stack = get_ret_stack(task, offset, &offset);
+ } while (ret_stack && ret_stack->ret == return_handler);
+
+ return ret_stack ? ret_stack->ret : 0;
+}
+
/**
* ftrace_graph_ret_addr - return the original value of the return address
* @task: The task the unwinder is being executed on
@@ -1145,8 +1159,7 @@ void ftrace_graph_exit_task(struct task_struct *t)
#ifdef CONFIG_DYNAMIC_FTRACE
static int fgraph_pid_func(struct ftrace_graph_ent *trace,
- struct fgraph_ops *gops,
- struct fgraph_extras *extras)
+ struct fgraph_ops *gops)
{
struct trace_array *tr = gops->ops.private;
int pid;
@@ -1160,7 +1173,7 @@ static int fgraph_pid_func(struct ftrace_graph_ent *trace,
return 0;
}
- return gops->saved_func(trace, gops, NULL);
+ return gops->saved_func(trace, gops);
}
void fgraph_update_pid_func(void)
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 5d87dac83b80..cae388122ca8 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -827,8 +827,7 @@ struct profile_fgraph_data {
};
static int profile_graph_entry(struct ftrace_graph_ent *trace,
- struct fgraph_ops *gops,
- struct fgraph_extras *extras)
+ struct fgraph_ops *gops)
{
struct profile_fgraph_data *profile_data;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 13f08f257c0b..6adf48ef4312 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -695,8 +695,7 @@ void trace_default_header(struct seq_file *m);
void print_trace_header(struct seq_file *m, struct trace_iterator *iter);
void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
-int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops,
- struct fgraph_extras *extras);
+int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops);
void tracing_start_cmdline_record(void);
void tracing_stop_cmdline_record(void);
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 3dd63ae2afe8..e3cda79653ee 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -179,8 +179,7 @@ struct fgraph_times {
};
int trace_graph_entry(struct ftrace_graph_ent *trace,
- struct fgraph_ops *gops,
- struct fgraph_extras *extras)
+ struct fgraph_ops *gops)
{
unsigned long *task_var = fgraph_get_task_var(gops);
struct trace_array *tr = gops->private;
@@ -246,11 +245,12 @@ int trace_graph_entry(struct ftrace_graph_ent *trace,
disabled = atomic_inc_return(&data->disabled);
if (likely(disabled == 1)) {
trace_ctx = tracing_gen_ctx_flags(flags);
- if (unlikely(IS_ENABLED(CONFIG_FUNCTION_GRAPH_RETADDR) && extras
- && (extras->flags & TRACE_GRAPH_PRINT_RETADDR)))
- ret = __trace_graph_retaddr_entry(tr, trace, trace_ctx,
- extras->retaddr);
- else
+ if (unlikely(IS_ENABLED(CONFIG_FUNCTION_GRAPH_RETADDR) &&
+ graph_tracer_flags_is_set(TRACE_GRAPH_PRINT_RETADDR))) {
+ unsigned long retaddr = ftrace_graph_top_ret_addr(current);
+
+ ret = __trace_graph_retaddr_entry(tr, trace, trace_ctx, retaddr);
+ } else
ret = __trace_graph_entry(tr, trace, trace_ctx);
} else {
ret = 0;
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index eb3aa36cf10f..fce064e20570 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -176,8 +176,7 @@ static int irqsoff_display_graph(struct trace_array *tr, int set)
}
static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,
- struct fgraph_ops *gops,
- struct fgraph_extras *extras)
+ struct fgraph_ops *gops)
{
struct trace_array *tr = irqsoff_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
index 155de2551507..ae2ace5e515a 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -113,8 +113,7 @@ static int wakeup_display_graph(struct trace_array *tr, int set)
}
static int wakeup_graph_entry(struct ftrace_graph_ent *trace,
- struct fgraph_ops *gops,
- struct fgraph_extras *extras)
+ struct fgraph_ops *gops)
{
struct trace_array *tr = wakeup_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index fbb99f8c8062..d3a14ae47e26 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -774,8 +774,7 @@ struct fgraph_fixture {
};
static __init int store_entry(struct ftrace_graph_ent *trace,
- struct fgraph_ops *gops,
- struct fgraph_extras *extras)
+ struct fgraph_ops *gops)
{
struct fgraph_fixture *fixture = container_of(gops, struct fgraph_fixture, gops);
const char *type = fixture->store_type_name;
@@ -1026,8 +1025,7 @@ static unsigned int graph_hang_thresh;
/* Wrap the real function entry probe to avoid possible hanging */
static int trace_graph_entry_watchdog(struct ftrace_graph_ent *trace,
- struct fgraph_ops *gops,
- struct fgraph_extras *extras)
+ struct fgraph_ops *gops)
{
/* This is harmlessly racy, we want to approximately detect a hang */
if (unlikely(++graph_hang_thresh > GRAPH_MAX_FUNC_TEST)) {
@@ -1041,7 +1039,7 @@ static int trace_graph_entry_watchdog(struct ftrace_graph_ent *trace,
return 0;
}
- return trace_graph_entry(trace, gops, NULL);
+ return trace_graph_entry(trace, gops);
}
static struct fgraph_ops fgraph_ops __initdata = {On Thu, Oct 10, 2024 at 7:35 AM Masami Hiramatsu (Google)
<mhiramat@kernel.org> wrote:
>
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
>
> Simplify return address printing in the function graph tracer by removing
> fgraph_extras. Since this feature is only used by the function graph
> tracer and the feature flags can directly accessible from the function
> graph tracer, fgraph_extras can be removed from the fgraph callback.
>
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
> include/linux/ftrace.h | 16 ++++--------
> kernel/trace/fgraph.c | 45 ++++++++++++++++++++++------------
> kernel/trace/ftrace.c | 3 +-
> kernel/trace/trace.h | 3 +-
> kernel/trace/trace_functions_graph.c | 14 +++++------
> kernel/trace/trace_irqsoff.c | 3 +-
> kernel/trace/trace_sched_wakeup.c | 3 +-
> kernel/trace/trace_selftest.c | 8 ++----
> 8 files changed, 48 insertions(+), 47 deletions(-)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 2ac3b3b53cd0..997e1f0335b6 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -1068,29 +1068,20 @@ struct ftrace_graph_ret {
> unsigned long long rettime;
> } __packed;
>
> -struct fgraph_extras;
> struct fgraph_ops;
>
> /* Type of the callback handlers for tracing function graph*/
> typedef void (*trace_func_graph_ret_t)(struct ftrace_graph_ret *,
> struct fgraph_ops *); /* return */
> typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
> - struct fgraph_ops *,
> - struct fgraph_extras *); /* entry */
> + struct fgraph_ops *); /* entry */
>
> extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
> - struct fgraph_ops *gops,
> - struct fgraph_extras *extras);
> + struct fgraph_ops *gops);
> bool ftrace_pids_enabled(struct ftrace_ops *ops);
>
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>
> -/* Used to convey some extra datas when creating a graph entry */
> -struct fgraph_extras {
> - u32 flags;
> - unsigned long retaddr;
> -};
> -
> struct fgraph_ops {
> trace_func_graph_ent_t entryfunc;
> trace_func_graph_ret_t retfunc;
> @@ -1131,12 +1122,15 @@ function_graph_enter(unsigned long ret, unsigned long func,
>
> struct ftrace_ret_stack *
> ftrace_graph_get_ret_stack(struct task_struct *task, int skip);
> +unsigned long ftrace_graph_top_ret_addr(struct task_struct *task);
>
> unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
> unsigned long ret, unsigned long *retp);
> unsigned long *fgraph_get_task_var(struct fgraph_ops *gops);
>
> u32 graph_tracer_flags_get(u32 flags);
> +#define graph_tracer_flags_is_set(flags) \
> + (graph_tracer_flags_get(flags) == (flags))
Do we need to retain the function graph_tracer_flags_get? it will be
only invoked in the
file trace_functions_graph.c, so we may access the tracer_flags
variable directly.
>
> /*
> * Sometimes we don't want to trace a function with the function
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 27e523f01ed2..ee829d65f301 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -290,8 +290,7 @@ static inline unsigned long make_data_type_val(int idx, int size, int offset)
> }
>
> /* ftrace_graph_entry set to this to tell some archs to run function graph */
> -static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops,
> - struct fgraph_extras *extras)
> +static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops)
> {
> return 0;
> }
> @@ -519,8 +518,7 @@ int __weak ftrace_disable_ftrace_graph_caller(void)
> #endif
>
> int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
> - struct fgraph_ops *gops,
> - struct fgraph_extras *extras)
> + struct fgraph_ops *gops)
> {
> return 0;
> }
> @@ -648,20 +646,13 @@ int function_graph_enter(unsigned long ret, unsigned long func,
> unsigned long frame_pointer, unsigned long *retp)
> {
> struct ftrace_graph_ent trace;
> - struct fgraph_extras extras;
> unsigned long bitmap = 0;
> int offset;
> int i;
> - int idx = 0;
>
> trace.func = func;
> trace.depth = ++current->curr_ret_depth;
>
> - extras.flags = graph_tracer_flags_get(TRACE_GRAPH_PRINT_RETADDR);
> - if (IS_ENABLED(CONFIG_FUNCTION_GRAPH_RETADDR)
> - && extras.flags & TRACE_GRAPH_PRINT_RETADDR)
> - extras.retaddr = ftrace_graph_ret_addr(current, &idx, ret, retp);
> -
> offset = ftrace_push_return_trace(ret, func, frame_pointer, retp, 0);
> if (offset < 0)
> goto out;
> @@ -670,7 +661,7 @@ int function_graph_enter(unsigned long ret, unsigned long func,
> if (static_branch_likely(&fgraph_do_direct)) {
> int save_curr_ret_stack = current->curr_ret_stack;
>
> - if (static_call(fgraph_func)(&trace, fgraph_direct_gops, &extras))
> + if (static_call(fgraph_func)(&trace, fgraph_direct_gops))
> bitmap |= BIT(fgraph_direct_gops->idx);
> else
> /* Clear out any saved storage */
> @@ -688,7 +679,7 @@ int function_graph_enter(unsigned long ret, unsigned long func,
>
> save_curr_ret_stack = current->curr_ret_stack;
> if (ftrace_ops_test(&gops->ops, func, NULL) &&
> - gops->entryfunc(&trace, gops, &extras))
> + gops->entryfunc(&trace, gops))
> bitmap |= BIT(i);
> else
> /* Clear out any saved storage */
> @@ -905,6 +896,29 @@ ftrace_graph_get_ret_stack(struct task_struct *task, int idx)
> return ret_stack;
> }
>
> +/**
> + * ftrace_graph_top_ret_addr - return the top return address in the shadow stack
> + * @task: The task to read the shadow stack from.
> + *
> + * Return the first return address on the shadow stack of the @task, which is
> + * not the fgraph's return_to_handler.
> + */
> +unsigned long ftrace_graph_top_ret_addr(struct task_struct *task)
> +{
> + unsigned long return_handler = (unsigned long)dereference_kernel_function_descriptor(return_to_handler);
> + struct ftrace_ret_stack *ret_stack = NULL;
> + int offset = task->curr_ret_stack;
> +
> + if (offset < 0)
> + return 0;
> +
> + do {
> + ret_stack = get_ret_stack(task, offset, &offset);
> + } while (ret_stack && ret_stack->ret == return_handler);
> +
> + return ret_stack ? ret_stack->ret : 0;
> +}
> +
> /**
> * ftrace_graph_ret_addr - return the original value of the return address
> * @task: The task the unwinder is being executed on
> @@ -1145,8 +1159,7 @@ void ftrace_graph_exit_task(struct task_struct *t)
>
> #ifdef CONFIG_DYNAMIC_FTRACE
> static int fgraph_pid_func(struct ftrace_graph_ent *trace,
> - struct fgraph_ops *gops,
> - struct fgraph_extras *extras)
> + struct fgraph_ops *gops)
> {
> struct trace_array *tr = gops->ops.private;
> int pid;
> @@ -1160,7 +1173,7 @@ static int fgraph_pid_func(struct ftrace_graph_ent *trace,
> return 0;
> }
>
> - return gops->saved_func(trace, gops, NULL);
> + return gops->saved_func(trace, gops);
> }
>
> void fgraph_update_pid_func(void)
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 5d87dac83b80..cae388122ca8 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -827,8 +827,7 @@ struct profile_fgraph_data {
> };
>
> static int profile_graph_entry(struct ftrace_graph_ent *trace,
> - struct fgraph_ops *gops,
> - struct fgraph_extras *extras)
> + struct fgraph_ops *gops)
> {
> struct profile_fgraph_data *profile_data;
>
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 13f08f257c0b..6adf48ef4312 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -695,8 +695,7 @@ void trace_default_header(struct seq_file *m);
> void print_trace_header(struct seq_file *m, struct trace_iterator *iter);
>
> void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
> -int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops,
> - struct fgraph_extras *extras);
> +int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops);
>
> void tracing_start_cmdline_record(void);
> void tracing_stop_cmdline_record(void);
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index 3dd63ae2afe8..e3cda79653ee 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -179,8 +179,7 @@ struct fgraph_times {
> };
>
> int trace_graph_entry(struct ftrace_graph_ent *trace,
> - struct fgraph_ops *gops,
> - struct fgraph_extras *extras)
> + struct fgraph_ops *gops)
> {
> unsigned long *task_var = fgraph_get_task_var(gops);
> struct trace_array *tr = gops->private;
> @@ -246,11 +245,12 @@ int trace_graph_entry(struct ftrace_graph_ent *trace,
> disabled = atomic_inc_return(&data->disabled);
> if (likely(disabled == 1)) {
> trace_ctx = tracing_gen_ctx_flags(flags);
> - if (unlikely(IS_ENABLED(CONFIG_FUNCTION_GRAPH_RETADDR) && extras
> - && (extras->flags & TRACE_GRAPH_PRINT_RETADDR)))
> - ret = __trace_graph_retaddr_entry(tr, trace, trace_ctx,
> - extras->retaddr);
> - else
> + if (unlikely(IS_ENABLED(CONFIG_FUNCTION_GRAPH_RETADDR) &&
> + graph_tracer_flags_is_set(TRACE_GRAPH_PRINT_RETADDR))) {
> + unsigned long retaddr = ftrace_graph_top_ret_addr(current);
> +
> + ret = __trace_graph_retaddr_entry(tr, trace, trace_ctx, retaddr);
> + } else
> ret = __trace_graph_entry(tr, trace, trace_ctx);
> } else {
> ret = 0;
> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
> index eb3aa36cf10f..fce064e20570 100644
> --- a/kernel/trace/trace_irqsoff.c
> +++ b/kernel/trace/trace_irqsoff.c
> @@ -176,8 +176,7 @@ static int irqsoff_display_graph(struct trace_array *tr, int set)
> }
>
> static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,
> - struct fgraph_ops *gops,
> - struct fgraph_extras *extras)
> + struct fgraph_ops *gops)
> {
> struct trace_array *tr = irqsoff_trace;
> struct trace_array_cpu *data;
> diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
> index 155de2551507..ae2ace5e515a 100644
> --- a/kernel/trace/trace_sched_wakeup.c
> +++ b/kernel/trace/trace_sched_wakeup.c
> @@ -113,8 +113,7 @@ static int wakeup_display_graph(struct trace_array *tr, int set)
> }
>
> static int wakeup_graph_entry(struct ftrace_graph_ent *trace,
> - struct fgraph_ops *gops,
> - struct fgraph_extras *extras)
> + struct fgraph_ops *gops)
> {
> struct trace_array *tr = wakeup_trace;
> struct trace_array_cpu *data;
> diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
> index fbb99f8c8062..d3a14ae47e26 100644
> --- a/kernel/trace/trace_selftest.c
> +++ b/kernel/trace/trace_selftest.c
> @@ -774,8 +774,7 @@ struct fgraph_fixture {
> };
>
> static __init int store_entry(struct ftrace_graph_ent *trace,
> - struct fgraph_ops *gops,
> - struct fgraph_extras *extras)
> + struct fgraph_ops *gops)
> {
> struct fgraph_fixture *fixture = container_of(gops, struct fgraph_fixture, gops);
> const char *type = fixture->store_type_name;
> @@ -1026,8 +1025,7 @@ static unsigned int graph_hang_thresh;
>
> /* Wrap the real function entry probe to avoid possible hanging */
> static int trace_graph_entry_watchdog(struct ftrace_graph_ent *trace,
> - struct fgraph_ops *gops,
> - struct fgraph_extras *extras)
> + struct fgraph_ops *gops)
> {
> /* This is harmlessly racy, we want to approximately detect a hang */
> if (unlikely(++graph_hang_thresh > GRAPH_MAX_FUNC_TEST)) {
> @@ -1041,7 +1039,7 @@ static int trace_graph_entry_watchdog(struct ftrace_graph_ent *trace,
> return 0;
> }
>
> - return trace_graph_entry(trace, gops, NULL);
> + return trace_graph_entry(trace, gops);
> }
>
> static struct fgraph_ops fgraph_ops __initdata = {
>
On Thu, 10 Oct 2024 10:40:02 +0800
Donglin Peng <dolinux.peng@gmail.com> wrote:
> On Thu, Oct 10, 2024 at 7:35 AM Masami Hiramatsu (Google)
> <mhiramat@kernel.org> wrote:
> >
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> >
> > Simplify return address printing in the function graph tracer by removing
> > fgraph_extras. Since this feature is only used by the function graph
> > tracer and the feature flags can directly accessible from the function
> > graph tracer, fgraph_extras can be removed from the fgraph callback.
> >
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> > include/linux/ftrace.h | 16 ++++--------
> > kernel/trace/fgraph.c | 45 ++++++++++++++++++++++------------
> > kernel/trace/ftrace.c | 3 +-
> > kernel/trace/trace.h | 3 +-
> > kernel/trace/trace_functions_graph.c | 14 +++++------
> > kernel/trace/trace_irqsoff.c | 3 +-
> > kernel/trace/trace_sched_wakeup.c | 3 +-
> > kernel/trace/trace_selftest.c | 8 ++----
> > 8 files changed, 48 insertions(+), 47 deletions(-)
> >
> > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > index 2ac3b3b53cd0..997e1f0335b6 100644
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
> > @@ -1068,29 +1068,20 @@ struct ftrace_graph_ret {
> > unsigned long long rettime;
> > } __packed;
> >
> > -struct fgraph_extras;
> > struct fgraph_ops;
> >
> > /* Type of the callback handlers for tracing function graph*/
> > typedef void (*trace_func_graph_ret_t)(struct ftrace_graph_ret *,
> > struct fgraph_ops *); /* return */
> > typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
> > - struct fgraph_ops *,
> > - struct fgraph_extras *); /* entry */
> > + struct fgraph_ops *); /* entry */
> >
> > extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
> > - struct fgraph_ops *gops,
> > - struct fgraph_extras *extras);
> > + struct fgraph_ops *gops);
> > bool ftrace_pids_enabled(struct ftrace_ops *ops);
> >
> > #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> >
> > -/* Used to convey some extra datas when creating a graph entry */
> > -struct fgraph_extras {
> > - u32 flags;
> > - unsigned long retaddr;
> > -};
> > -
> > struct fgraph_ops {
> > trace_func_graph_ent_t entryfunc;
> > trace_func_graph_ret_t retfunc;
> > @@ -1131,12 +1122,15 @@ function_graph_enter(unsigned long ret, unsigned long func,
> >
> > struct ftrace_ret_stack *
> > ftrace_graph_get_ret_stack(struct task_struct *task, int skip);
> > +unsigned long ftrace_graph_top_ret_addr(struct task_struct *task);
> >
> > unsigned long ftrace_graph_ret_addr(struct task_struct *task, int *idx,
> > unsigned long ret, unsigned long *retp);
> > unsigned long *fgraph_get_task_var(struct fgraph_ops *gops);
> >
> > u32 graph_tracer_flags_get(u32 flags);
> > +#define graph_tracer_flags_is_set(flags) \
> > + (graph_tracer_flags_get(flags) == (flags))
>
> Do we need to retain the function graph_tracer_flags_get? it will be
> only invoked in the
> file trace_functions_graph.c, so we may access the tracer_flags
> variable directly.
Good catch!
Let me send v2.
Thanks!
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
© 2016 - 2024 Red Hat, Inc.