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

Pass ftrace_regs to the fgraph_ops::retfunc(). If ftrace_regs is not
available, it passes a NULL instead. User callback function can access
some registers (including return address) via this ftrace_regs.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 Changes in v8:
  - Pass ftrace_regs to retfunc, instead of adding retregfunc.
 Changes in v6:
  - update to use ftrace_regs_get_return_value() because of reordering
    patches.
 Changes in v3:
  - Update for new multiple fgraph.
  - Save the return address to instruction pointer in ftrace_regs.
---
 include/linux/ftrace.h               |    3 ++-
 kernel/trace/fgraph.c                |   16 +++++++++++-----
 kernel/trace/ftrace.c                |    3 ++-
 kernel/trace/trace.h                 |    3 ++-
 kernel/trace/trace_functions_graph.c |    7 ++++---
 kernel/trace/trace_irqsoff.c         |    3 ++-
 kernel/trace/trace_sched_wakeup.c    |    3 ++-
 kernel/trace/trace_selftest.c        |    3 ++-
 8 files changed, 27 insertions(+), 14 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 13987cd63553..e7c41d9988e1 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1069,7 +1069,8 @@ 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 */
+				       struct fgraph_ops *,
+				       struct ftrace_regs *); /* return */
 typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
 				      struct fgraph_ops *,
 				      struct ftrace_regs *); /* entry */
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index 30bebe43607d..6a3e2db16aa4 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -297,7 +297,8 @@ static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops,
 }
 
 /* ftrace_graph_return set to this to tell some archs to run function graph */
-static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops)
+static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops,
+		       struct ftrace_regs *fregs)
 {
 }
 
@@ -491,7 +492,8 @@ int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
 }
 
 static void ftrace_graph_ret_stub(struct ftrace_graph_ret *trace,
-				  struct fgraph_ops *gops)
+				  struct fgraph_ops *gops,
+				  struct ftrace_regs *fregs)
 {
 }
 
@@ -787,6 +789,9 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
 	}
 
 	trace.rettime = trace_clock_local();
+	if (fregs)
+		ftrace_regs_set_instruction_pointer(fregs, ret);
+
 #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
 	trace.retval = ftrace_regs_get_return_value(fregs);
 #endif
@@ -796,7 +801,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
 #ifdef CONFIG_HAVE_STATIC_CALL
 	if (static_branch_likely(&fgraph_do_direct)) {
 		if (test_bit(fgraph_direct_gops->idx, &bitmap))
-			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops);
+			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops, fregs);
 	} else
 #endif
 	{
@@ -806,7 +811,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
 			if (gops == &fgraph_stub)
 				continue;
 
-			gops->retfunc(&trace, gops);
+			gops->retfunc(&trace, gops, fregs);
 		}
 	}
 
@@ -956,7 +961,8 @@ void ftrace_graph_sleep_time_control(bool enable)
  * Simply points to ftrace_stub, but with the proper protocol.
  * Defined by the linker script in linux/vmlinux.lds.h
  */
-void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
+void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
+		       struct ftrace_regs *fregs);
 
 /* The callbacks that hook a function */
 trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 775040a9f541..fd6c5a50c5e5 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -840,7 +840,8 @@ static int profile_graph_entry(struct ftrace_graph_ent *trace,
 }
 
 static void profile_graph_return(struct ftrace_graph_ret *trace,
-				 struct fgraph_ops *gops)
+				 struct fgraph_ops *gops,
+				 struct ftrace_regs *fregs)
 {
 	struct ftrace_ret_stack *ret_stack;
 	struct ftrace_profile_stat *stat;
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 28d8ad5e31e6..f4a3f75bd916 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -682,7 +682,8 @@ void trace_latency_header(struct seq_file *m);
 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);
+void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
+			struct ftrace_regs *fregs);
 int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops,
 		      struct ftrace_regs *fregs);
 
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index b9785fc919c9..241407000109 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -240,7 +240,7 @@ void __trace_graph_return(struct trace_array *tr,
 }
 
 void trace_graph_return(struct ftrace_graph_ret *trace,
-			struct fgraph_ops *gops)
+			struct fgraph_ops *gops, struct ftrace_regs *fregs)
 {
 	unsigned long *task_var = fgraph_get_task_var(gops);
 	struct trace_array *tr = gops->private;
@@ -270,7 +270,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
 }
 
 static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
-				      struct fgraph_ops *gops)
+				      struct fgraph_ops *gops,
+				      struct ftrace_regs *fregs)
 {
 	ftrace_graph_addr_finish(gops, trace);
 
@@ -283,7 +284,7 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
 	    (trace->rettime - trace->calltime < tracing_thresh))
 		return;
 	else
-		trace_graph_return(trace, gops);
+		trace_graph_return(trace, gops, fregs);
 }
 
 static struct fgraph_ops funcgraph_ops = {
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index ad739d76fc86..504de7a05498 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -208,7 +208,8 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,
 }
 
 static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
-				 struct fgraph_ops *gops)
+				 struct fgraph_ops *gops,
+				 struct ftrace_regs *fregs)
 {
 	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 23360a2700de..9ffbd9326898 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -144,7 +144,8 @@ static int wakeup_graph_entry(struct ftrace_graph_ent *trace,
 }
 
 static void wakeup_graph_return(struct ftrace_graph_ret *trace,
-				struct fgraph_ops *gops)
+				struct fgraph_ops *gops,
+				struct ftrace_regs *fregs)
 {
 	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 89067f02094a..1ebd0899238f 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -807,7 +807,8 @@ static __init int store_entry(struct ftrace_graph_ent *trace,
 }
 
 static __init void store_return(struct ftrace_graph_ret *trace,
-				struct fgraph_ops *gops)
+				struct fgraph_ops *gops,
+				struct ftrace_regs *fregs)
 {
 	struct fgraph_fixture *fixture = container_of(gops, struct fgraph_fixture, gops);
 	const char *type = fixture->store_type_name;

Can I get an Acked-by from the X86 maintainers for this patch?

Thanks!

-- Steve

[ Note this is modifies the return side ]

On Fri, 13 Sep 2024 00:09:02 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Pass ftrace_regs to the fgraph_ops::retfunc(). If ftrace_regs is not
> available, it passes a NULL instead. User callback function can access
> some registers (including return address) via this ftrace_regs.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  Changes in v8:
>   - Pass ftrace_regs to retfunc, instead of adding retregfunc.
>  Changes in v6:
>   - update to use ftrace_regs_get_return_value() because of reordering
>     patches.
>  Changes in v3:
>   - Update for new multiple fgraph.
>   - Save the return address to instruction pointer in ftrace_regs.
> ---
>  include/linux/ftrace.h               |    3 ++-
>  kernel/trace/fgraph.c                |   16 +++++++++++-----
>  kernel/trace/ftrace.c                |    3 ++-
>  kernel/trace/trace.h                 |    3 ++-
>  kernel/trace/trace_functions_graph.c |    7 ++++---
>  kernel/trace/trace_irqsoff.c         |    3 ++-
>  kernel/trace/trace_sched_wakeup.c    |    3 ++-
>  kernel/trace/trace_selftest.c        |    3 ++-
>  8 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 13987cd63553..e7c41d9988e1 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -1069,7 +1069,8 @@ 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 */
> +				       struct fgraph_ops *,
> +				       struct ftrace_regs *); /* return */
>  typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
>  				      struct fgraph_ops *,
>  				      struct ftrace_regs *); /* entry */
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 30bebe43607d..6a3e2db16aa4 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -297,7 +297,8 @@ static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops,
>  }
>  
>  /* ftrace_graph_return set to this to tell some archs to run function graph */
> -static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops)
> +static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops,
> +		       struct ftrace_regs *fregs)
>  {
>  }
>  
> @@ -491,7 +492,8 @@ int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
>  }
>  
>  static void ftrace_graph_ret_stub(struct ftrace_graph_ret *trace,
> -				  struct fgraph_ops *gops)
> +				  struct fgraph_ops *gops,
> +				  struct ftrace_regs *fregs)
>  {
>  }
>  
> @@ -787,6 +789,9 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  	}
>  
>  	trace.rettime = trace_clock_local();
> +	if (fregs)
> +		ftrace_regs_set_instruction_pointer(fregs, ret);
> +
>  #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
>  	trace.retval = ftrace_regs_get_return_value(fregs);
>  #endif
> @@ -796,7 +801,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  #ifdef CONFIG_HAVE_STATIC_CALL
>  	if (static_branch_likely(&fgraph_do_direct)) {
>  		if (test_bit(fgraph_direct_gops->idx, &bitmap))
> -			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops);
> +			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops, fregs);
>  	} else
>  #endif
>  	{
> @@ -806,7 +811,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  			if (gops == &fgraph_stub)
>  				continue;
>  
> -			gops->retfunc(&trace, gops);
> +			gops->retfunc(&trace, gops, fregs);
>  		}
>  	}
>  
> @@ -956,7 +961,8 @@ void ftrace_graph_sleep_time_control(bool enable)
>   * Simply points to ftrace_stub, but with the proper protocol.
>   * Defined by the linker script in linux/vmlinux.lds.h
>   */
> -void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
> +void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
> +		       struct ftrace_regs *fregs);
>  
>  /* The callbacks that hook a function */
>  trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 775040a9f541..fd6c5a50c5e5 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -840,7 +840,8 @@ static int profile_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void profile_graph_return(struct ftrace_graph_ret *trace,
> -				 struct fgraph_ops *gops)
> +				 struct fgraph_ops *gops,
> +				 struct ftrace_regs *fregs)
>  {
>  	struct ftrace_ret_stack *ret_stack;
>  	struct ftrace_profile_stat *stat;
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 28d8ad5e31e6..f4a3f75bd916 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -682,7 +682,8 @@ void trace_latency_header(struct seq_file *m);
>  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);
> +void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
> +			struct ftrace_regs *fregs);
>  int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops,
>  		      struct ftrace_regs *fregs);
>  
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index b9785fc919c9..241407000109 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -240,7 +240,7 @@ void __trace_graph_return(struct trace_array *tr,
>  }
>  
>  void trace_graph_return(struct ftrace_graph_ret *trace,
> -			struct fgraph_ops *gops)
> +			struct fgraph_ops *gops, struct ftrace_regs *fregs)
>  {
>  	unsigned long *task_var = fgraph_get_task_var(gops);
>  	struct trace_array *tr = gops->private;
> @@ -270,7 +270,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
>  }
>  
>  static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
> -				      struct fgraph_ops *gops)
> +				      struct fgraph_ops *gops,
> +				      struct ftrace_regs *fregs)
>  {
>  	ftrace_graph_addr_finish(gops, trace);
>  
> @@ -283,7 +284,7 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
>  	    (trace->rettime - trace->calltime < tracing_thresh))
>  		return;
>  	else
> -		trace_graph_return(trace, gops);
> +		trace_graph_return(trace, gops, fregs);
>  }
>  
>  static struct fgraph_ops funcgraph_ops = {
> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
> index ad739d76fc86..504de7a05498 100644
> --- a/kernel/trace/trace_irqsoff.c
> +++ b/kernel/trace/trace_irqsoff.c
> @@ -208,7 +208,8 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
> -				 struct fgraph_ops *gops)
> +				 struct fgraph_ops *gops,
> +				 struct ftrace_regs *fregs)
>  {
>  	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 23360a2700de..9ffbd9326898 100644
> --- a/kernel/trace/trace_sched_wakeup.c
> +++ b/kernel/trace/trace_sched_wakeup.c
> @@ -144,7 +144,8 @@ static int wakeup_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void wakeup_graph_return(struct ftrace_graph_ret *trace,
> -				struct fgraph_ops *gops)
> +				struct fgraph_ops *gops,
> +				struct ftrace_regs *fregs)
>  {
>  	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 89067f02094a..1ebd0899238f 100644
> --- a/kernel/trace/trace_selftest.c
> +++ b/kernel/trace/trace_selftest.c
> @@ -807,7 +807,8 @@ static __init int store_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static __init void store_return(struct ftrace_graph_ret *trace,
> -				struct fgraph_ops *gops)
> +				struct fgraph_ops *gops,
> +				struct ftrace_regs *fregs)
>  {
>  	struct fgraph_fixture *fixture = container_of(gops, struct fgraph_fixture, gops);
>  	const char *type = fixture->store_type_name;

Can I get an Acked-by from the RISC-V maintainers for this patch?

Thanks!

-- Steve

[ Note this is modifies the return side ]

On Fri, 13 Sep 2024 00:09:02 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Pass ftrace_regs to the fgraph_ops::retfunc(). If ftrace_regs is not
> available, it passes a NULL instead. User callback function can access
> some registers (including return address) via this ftrace_regs.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  Changes in v8:
>   - Pass ftrace_regs to retfunc, instead of adding retregfunc.
>  Changes in v6:
>   - update to use ftrace_regs_get_return_value() because of reordering
>     patches.
>  Changes in v3:
>   - Update for new multiple fgraph.
>   - Save the return address to instruction pointer in ftrace_regs.
> ---
>  include/linux/ftrace.h               |    3 ++-
>  kernel/trace/fgraph.c                |   16 +++++++++++-----
>  kernel/trace/ftrace.c                |    3 ++-
>  kernel/trace/trace.h                 |    3 ++-
>  kernel/trace/trace_functions_graph.c |    7 ++++---
>  kernel/trace/trace_irqsoff.c         |    3 ++-
>  kernel/trace/trace_sched_wakeup.c    |    3 ++-
>  kernel/trace/trace_selftest.c        |    3 ++-
>  8 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 13987cd63553..e7c41d9988e1 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -1069,7 +1069,8 @@ 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 */
> +				       struct fgraph_ops *,
> +				       struct ftrace_regs *); /* return */
>  typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
>  				      struct fgraph_ops *,
>  				      struct ftrace_regs *); /* entry */
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 30bebe43607d..6a3e2db16aa4 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -297,7 +297,8 @@ static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops,
>  }
>  
>  /* ftrace_graph_return set to this to tell some archs to run function graph */
> -static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops)
> +static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops,
> +		       struct ftrace_regs *fregs)
>  {
>  }
>  
> @@ -491,7 +492,8 @@ int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
>  }
>  
>  static void ftrace_graph_ret_stub(struct ftrace_graph_ret *trace,
> -				  struct fgraph_ops *gops)
> +				  struct fgraph_ops *gops,
> +				  struct ftrace_regs *fregs)
>  {
>  }
>  
> @@ -787,6 +789,9 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  	}
>  
>  	trace.rettime = trace_clock_local();
> +	if (fregs)
> +		ftrace_regs_set_instruction_pointer(fregs, ret);
> +
>  #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
>  	trace.retval = ftrace_regs_get_return_value(fregs);
>  #endif
> @@ -796,7 +801,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  #ifdef CONFIG_HAVE_STATIC_CALL
>  	if (static_branch_likely(&fgraph_do_direct)) {
>  		if (test_bit(fgraph_direct_gops->idx, &bitmap))
> -			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops);
> +			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops, fregs);
>  	} else
>  #endif
>  	{
> @@ -806,7 +811,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  			if (gops == &fgraph_stub)
>  				continue;
>  
> -			gops->retfunc(&trace, gops);
> +			gops->retfunc(&trace, gops, fregs);
>  		}
>  	}
>  
> @@ -956,7 +961,8 @@ void ftrace_graph_sleep_time_control(bool enable)
>   * Simply points to ftrace_stub, but with the proper protocol.
>   * Defined by the linker script in linux/vmlinux.lds.h
>   */
> -void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
> +void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
> +		       struct ftrace_regs *fregs);
>  
>  /* The callbacks that hook a function */
>  trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 775040a9f541..fd6c5a50c5e5 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -840,7 +840,8 @@ static int profile_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void profile_graph_return(struct ftrace_graph_ret *trace,
> -				 struct fgraph_ops *gops)
> +				 struct fgraph_ops *gops,
> +				 struct ftrace_regs *fregs)
>  {
>  	struct ftrace_ret_stack *ret_stack;
>  	struct ftrace_profile_stat *stat;
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 28d8ad5e31e6..f4a3f75bd916 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -682,7 +682,8 @@ void trace_latency_header(struct seq_file *m);
>  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);
> +void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
> +			struct ftrace_regs *fregs);
>  int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops,
>  		      struct ftrace_regs *fregs);
>  
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index b9785fc919c9..241407000109 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -240,7 +240,7 @@ void __trace_graph_return(struct trace_array *tr,
>  }
>  
>  void trace_graph_return(struct ftrace_graph_ret *trace,
> -			struct fgraph_ops *gops)
> +			struct fgraph_ops *gops, struct ftrace_regs *fregs)
>  {
>  	unsigned long *task_var = fgraph_get_task_var(gops);
>  	struct trace_array *tr = gops->private;
> @@ -270,7 +270,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
>  }
>  
>  static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
> -				      struct fgraph_ops *gops)
> +				      struct fgraph_ops *gops,
> +				      struct ftrace_regs *fregs)
>  {
>  	ftrace_graph_addr_finish(gops, trace);
>  
> @@ -283,7 +284,7 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
>  	    (trace->rettime - trace->calltime < tracing_thresh))
>  		return;
>  	else
> -		trace_graph_return(trace, gops);
> +		trace_graph_return(trace, gops, fregs);
>  }
>  
>  static struct fgraph_ops funcgraph_ops = {
> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
> index ad739d76fc86..504de7a05498 100644
> --- a/kernel/trace/trace_irqsoff.c
> +++ b/kernel/trace/trace_irqsoff.c
> @@ -208,7 +208,8 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
> -				 struct fgraph_ops *gops)
> +				 struct fgraph_ops *gops,
> +				 struct ftrace_regs *fregs)
>  {
>  	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 23360a2700de..9ffbd9326898 100644
> --- a/kernel/trace/trace_sched_wakeup.c
> +++ b/kernel/trace/trace_sched_wakeup.c
> @@ -144,7 +144,8 @@ static int wakeup_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void wakeup_graph_return(struct ftrace_graph_ret *trace,
> -				struct fgraph_ops *gops)
> +				struct fgraph_ops *gops,
> +				struct ftrace_regs *fregs)
>  {
>  	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 89067f02094a..1ebd0899238f 100644
> --- a/kernel/trace/trace_selftest.c
> +++ b/kernel/trace/trace_selftest.c
> @@ -807,7 +807,8 @@ static __init int store_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static __init void store_return(struct ftrace_graph_ret *trace,
> -				struct fgraph_ops *gops)
> +				struct fgraph_ops *gops,
> +				struct ftrace_regs *fregs)
>  {
>  	struct fgraph_fixture *fixture = container_of(gops, struct fgraph_fixture, gops);
>  	const char *type = fixture->store_type_name;

Can I get an Acked-by from the POWERPC maintainers for this patch?

Thanks!

-- Steve

[ Note this is modifies the return side ]

On Fri, 13 Sep 2024 00:09:02 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Pass ftrace_regs to the fgraph_ops::retfunc(). If ftrace_regs is not
> available, it passes a NULL instead. User callback function can access
> some registers (including return address) via this ftrace_regs.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  Changes in v8:
>   - Pass ftrace_regs to retfunc, instead of adding retregfunc.
>  Changes in v6:
>   - update to use ftrace_regs_get_return_value() because of reordering
>     patches.
>  Changes in v3:
>   - Update for new multiple fgraph.
>   - Save the return address to instruction pointer in ftrace_regs.
> ---
>  include/linux/ftrace.h               |    3 ++-
>  kernel/trace/fgraph.c                |   16 +++++++++++-----
>  kernel/trace/ftrace.c                |    3 ++-
>  kernel/trace/trace.h                 |    3 ++-
>  kernel/trace/trace_functions_graph.c |    7 ++++---
>  kernel/trace/trace_irqsoff.c         |    3 ++-
>  kernel/trace/trace_sched_wakeup.c    |    3 ++-
>  kernel/trace/trace_selftest.c        |    3 ++-
>  8 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 13987cd63553..e7c41d9988e1 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -1069,7 +1069,8 @@ 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 */
> +				       struct fgraph_ops *,
> +				       struct ftrace_regs *); /* return */
>  typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
>  				      struct fgraph_ops *,
>  				      struct ftrace_regs *); /* entry */
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 30bebe43607d..6a3e2db16aa4 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -297,7 +297,8 @@ static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops,
>  }
>  
>  /* ftrace_graph_return set to this to tell some archs to run function graph */
> -static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops)
> +static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops,
> +		       struct ftrace_regs *fregs)
>  {
>  }
>  
> @@ -491,7 +492,8 @@ int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
>  }
>  
>  static void ftrace_graph_ret_stub(struct ftrace_graph_ret *trace,
> -				  struct fgraph_ops *gops)
> +				  struct fgraph_ops *gops,
> +				  struct ftrace_regs *fregs)
>  {
>  }
>  
> @@ -787,6 +789,9 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  	}
>  
>  	trace.rettime = trace_clock_local();
> +	if (fregs)
> +		ftrace_regs_set_instruction_pointer(fregs, ret);
> +
>  #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
>  	trace.retval = ftrace_regs_get_return_value(fregs);
>  #endif
> @@ -796,7 +801,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  #ifdef CONFIG_HAVE_STATIC_CALL
>  	if (static_branch_likely(&fgraph_do_direct)) {
>  		if (test_bit(fgraph_direct_gops->idx, &bitmap))
> -			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops);
> +			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops, fregs);
>  	} else
>  #endif
>  	{
> @@ -806,7 +811,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  			if (gops == &fgraph_stub)
>  				continue;
>  
> -			gops->retfunc(&trace, gops);
> +			gops->retfunc(&trace, gops, fregs);
>  		}
>  	}
>  
> @@ -956,7 +961,8 @@ void ftrace_graph_sleep_time_control(bool enable)
>   * Simply points to ftrace_stub, but with the proper protocol.
>   * Defined by the linker script in linux/vmlinux.lds.h
>   */
> -void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
> +void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
> +		       struct ftrace_regs *fregs);
>  
>  /* The callbacks that hook a function */
>  trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 775040a9f541..fd6c5a50c5e5 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -840,7 +840,8 @@ static int profile_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void profile_graph_return(struct ftrace_graph_ret *trace,
> -				 struct fgraph_ops *gops)
> +				 struct fgraph_ops *gops,
> +				 struct ftrace_regs *fregs)
>  {
>  	struct ftrace_ret_stack *ret_stack;
>  	struct ftrace_profile_stat *stat;
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 28d8ad5e31e6..f4a3f75bd916 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -682,7 +682,8 @@ void trace_latency_header(struct seq_file *m);
>  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);
> +void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
> +			struct ftrace_regs *fregs);
>  int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops,
>  		      struct ftrace_regs *fregs);
>  
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index b9785fc919c9..241407000109 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -240,7 +240,7 @@ void __trace_graph_return(struct trace_array *tr,
>  }
>  
>  void trace_graph_return(struct ftrace_graph_ret *trace,
> -			struct fgraph_ops *gops)
> +			struct fgraph_ops *gops, struct ftrace_regs *fregs)
>  {
>  	unsigned long *task_var = fgraph_get_task_var(gops);
>  	struct trace_array *tr = gops->private;
> @@ -270,7 +270,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
>  }
>  
>  static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
> -				      struct fgraph_ops *gops)
> +				      struct fgraph_ops *gops,
> +				      struct ftrace_regs *fregs)
>  {
>  	ftrace_graph_addr_finish(gops, trace);
>  
> @@ -283,7 +284,7 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
>  	    (trace->rettime - trace->calltime < tracing_thresh))
>  		return;
>  	else
> -		trace_graph_return(trace, gops);
> +		trace_graph_return(trace, gops, fregs);
>  }
>  
>  static struct fgraph_ops funcgraph_ops = {
> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
> index ad739d76fc86..504de7a05498 100644
> --- a/kernel/trace/trace_irqsoff.c
> +++ b/kernel/trace/trace_irqsoff.c
> @@ -208,7 +208,8 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
> -				 struct fgraph_ops *gops)
> +				 struct fgraph_ops *gops,
> +				 struct ftrace_regs *fregs)
>  {
>  	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 23360a2700de..9ffbd9326898 100644
> --- a/kernel/trace/trace_sched_wakeup.c
> +++ b/kernel/trace/trace_sched_wakeup.c
> @@ -144,7 +144,8 @@ static int wakeup_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void wakeup_graph_return(struct ftrace_graph_ret *trace,
> -				struct fgraph_ops *gops)
> +				struct fgraph_ops *gops,
> +				struct ftrace_regs *fregs)
>  {
>  	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 89067f02094a..1ebd0899238f 100644
> --- a/kernel/trace/trace_selftest.c
> +++ b/kernel/trace/trace_selftest.c
> @@ -807,7 +807,8 @@ static __init int store_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static __init void store_return(struct ftrace_graph_ret *trace,
> -				struct fgraph_ops *gops)
> +				struct fgraph_ops *gops,
> +				struct ftrace_regs *fregs)
>  {
>  	struct fgraph_fixture *fixture = container_of(gops, struct fgraph_fixture, gops);
>  	const char *type = fixture->store_type_name;

Can I get an Acked-by from the LOONGARCH maintainers for this patch?

Thanks!

-- Steve

[ Note this is modifies the return side ]

On Fri, 13 Sep 2024 00:09:02 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Pass ftrace_regs to the fgraph_ops::retfunc(). If ftrace_regs is not
> available, it passes a NULL instead. User callback function can access
> some registers (including return address) via this ftrace_regs.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  Changes in v8:
>   - Pass ftrace_regs to retfunc, instead of adding retregfunc.
>  Changes in v6:
>   - update to use ftrace_regs_get_return_value() because of reordering
>     patches.
>  Changes in v3:
>   - Update for new multiple fgraph.
>   - Save the return address to instruction pointer in ftrace_regs.
> ---
>  include/linux/ftrace.h               |    3 ++-
>  kernel/trace/fgraph.c                |   16 +++++++++++-----
>  kernel/trace/ftrace.c                |    3 ++-
>  kernel/trace/trace.h                 |    3 ++-
>  kernel/trace/trace_functions_graph.c |    7 ++++---
>  kernel/trace/trace_irqsoff.c         |    3 ++-
>  kernel/trace/trace_sched_wakeup.c    |    3 ++-
>  kernel/trace/trace_selftest.c        |    3 ++-
>  8 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 13987cd63553..e7c41d9988e1 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -1069,7 +1069,8 @@ 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 */
> +				       struct fgraph_ops *,
> +				       struct ftrace_regs *); /* return */
>  typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
>  				      struct fgraph_ops *,
>  				      struct ftrace_regs *); /* entry */
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 30bebe43607d..6a3e2db16aa4 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -297,7 +297,8 @@ static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops,
>  }
>  
>  /* ftrace_graph_return set to this to tell some archs to run function graph */
> -static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops)
> +static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops,
> +		       struct ftrace_regs *fregs)
>  {
>  }
>  
> @@ -491,7 +492,8 @@ int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
>  }
>  
>  static void ftrace_graph_ret_stub(struct ftrace_graph_ret *trace,
> -				  struct fgraph_ops *gops)
> +				  struct fgraph_ops *gops,
> +				  struct ftrace_regs *fregs)
>  {
>  }
>  
> @@ -787,6 +789,9 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  	}
>  
>  	trace.rettime = trace_clock_local();
> +	if (fregs)
> +		ftrace_regs_set_instruction_pointer(fregs, ret);
> +
>  #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
>  	trace.retval = ftrace_regs_get_return_value(fregs);
>  #endif
> @@ -796,7 +801,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  #ifdef CONFIG_HAVE_STATIC_CALL
>  	if (static_branch_likely(&fgraph_do_direct)) {
>  		if (test_bit(fgraph_direct_gops->idx, &bitmap))
> -			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops);
> +			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops, fregs);
>  	} else
>  #endif
>  	{
> @@ -806,7 +811,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  			if (gops == &fgraph_stub)
>  				continue;
>  
> -			gops->retfunc(&trace, gops);
> +			gops->retfunc(&trace, gops, fregs);
>  		}
>  	}
>  
> @@ -956,7 +961,8 @@ void ftrace_graph_sleep_time_control(bool enable)
>   * Simply points to ftrace_stub, but with the proper protocol.
>   * Defined by the linker script in linux/vmlinux.lds.h
>   */
> -void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
> +void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
> +		       struct ftrace_regs *fregs);
>  
>  /* The callbacks that hook a function */
>  trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 775040a9f541..fd6c5a50c5e5 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -840,7 +840,8 @@ static int profile_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void profile_graph_return(struct ftrace_graph_ret *trace,
> -				 struct fgraph_ops *gops)
> +				 struct fgraph_ops *gops,
> +				 struct ftrace_regs *fregs)
>  {
>  	struct ftrace_ret_stack *ret_stack;
>  	struct ftrace_profile_stat *stat;
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 28d8ad5e31e6..f4a3f75bd916 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -682,7 +682,8 @@ void trace_latency_header(struct seq_file *m);
>  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);
> +void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
> +			struct ftrace_regs *fregs);
>  int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops,
>  		      struct ftrace_regs *fregs);
>  
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index b9785fc919c9..241407000109 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -240,7 +240,7 @@ void __trace_graph_return(struct trace_array *tr,
>  }
>  
>  void trace_graph_return(struct ftrace_graph_ret *trace,
> -			struct fgraph_ops *gops)
> +			struct fgraph_ops *gops, struct ftrace_regs *fregs)
>  {
>  	unsigned long *task_var = fgraph_get_task_var(gops);
>  	struct trace_array *tr = gops->private;
> @@ -270,7 +270,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
>  }
>  
>  static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
> -				      struct fgraph_ops *gops)
> +				      struct fgraph_ops *gops,
> +				      struct ftrace_regs *fregs)
>  {
>  	ftrace_graph_addr_finish(gops, trace);
>  
> @@ -283,7 +284,7 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
>  	    (trace->rettime - trace->calltime < tracing_thresh))
>  		return;
>  	else
> -		trace_graph_return(trace, gops);
> +		trace_graph_return(trace, gops, fregs);
>  }
>  
>  static struct fgraph_ops funcgraph_ops = {
> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
> index ad739d76fc86..504de7a05498 100644
> --- a/kernel/trace/trace_irqsoff.c
> +++ b/kernel/trace/trace_irqsoff.c
> @@ -208,7 +208,8 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
> -				 struct fgraph_ops *gops)
> +				 struct fgraph_ops *gops,
> +				 struct ftrace_regs *fregs)
>  {
>  	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 23360a2700de..9ffbd9326898 100644
> --- a/kernel/trace/trace_sched_wakeup.c
> +++ b/kernel/trace/trace_sched_wakeup.c
> @@ -144,7 +144,8 @@ static int wakeup_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void wakeup_graph_return(struct ftrace_graph_ret *trace,
> -				struct fgraph_ops *gops)
> +				struct fgraph_ops *gops,
> +				struct ftrace_regs *fregs)
>  {
>  	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 89067f02094a..1ebd0899238f 100644
> --- a/kernel/trace/trace_selftest.c
> +++ b/kernel/trace/trace_selftest.c
> @@ -807,7 +807,8 @@ static __init int store_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static __init void store_return(struct ftrace_graph_ret *trace,
> -				struct fgraph_ops *gops)
> +				struct fgraph_ops *gops,
> +				struct ftrace_regs *fregs)
>  {
>  	struct fgraph_fixture *fixture = container_of(gops, struct fgraph_fixture, gops);
>  	const char *type = fixture->store_type_name;

Can I get an Acked-by from the AARCH64 maintainers for this patch?

Thanks!

-- Steve

[ Note this is modifies the return side ]

On Fri, 13 Sep 2024 00:09:02 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Pass ftrace_regs to the fgraph_ops::retfunc(). If ftrace_regs is not
> available, it passes a NULL instead. User callback function can access
> some registers (including return address) via this ftrace_regs.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  Changes in v8:
>   - Pass ftrace_regs to retfunc, instead of adding retregfunc.
>  Changes in v6:
>   - update to use ftrace_regs_get_return_value() because of reordering
>     patches.
>  Changes in v3:
>   - Update for new multiple fgraph.
>   - Save the return address to instruction pointer in ftrace_regs.
> ---
>  include/linux/ftrace.h               |    3 ++-
>  kernel/trace/fgraph.c                |   16 +++++++++++-----
>  kernel/trace/ftrace.c                |    3 ++-
>  kernel/trace/trace.h                 |    3 ++-
>  kernel/trace/trace_functions_graph.c |    7 ++++---
>  kernel/trace/trace_irqsoff.c         |    3 ++-
>  kernel/trace/trace_sched_wakeup.c    |    3 ++-
>  kernel/trace/trace_selftest.c        |    3 ++-
>  8 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 13987cd63553..e7c41d9988e1 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -1069,7 +1069,8 @@ 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 */
> +				       struct fgraph_ops *,
> +				       struct ftrace_regs *); /* return */
>  typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
>  				      struct fgraph_ops *,
>  				      struct ftrace_regs *); /* entry */
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index 30bebe43607d..6a3e2db16aa4 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -297,7 +297,8 @@ static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops,
>  }
>  
>  /* ftrace_graph_return set to this to tell some archs to run function graph */
> -static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops)
> +static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops,
> +		       struct ftrace_regs *fregs)
>  {
>  }
>  
> @@ -491,7 +492,8 @@ int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
>  }
>  
>  static void ftrace_graph_ret_stub(struct ftrace_graph_ret *trace,
> -				  struct fgraph_ops *gops)
> +				  struct fgraph_ops *gops,
> +				  struct ftrace_regs *fregs)
>  {
>  }
>  
> @@ -787,6 +789,9 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  	}
>  
>  	trace.rettime = trace_clock_local();
> +	if (fregs)
> +		ftrace_regs_set_instruction_pointer(fregs, ret);
> +
>  #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
>  	trace.retval = ftrace_regs_get_return_value(fregs);
>  #endif
> @@ -796,7 +801,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  #ifdef CONFIG_HAVE_STATIC_CALL
>  	if (static_branch_likely(&fgraph_do_direct)) {
>  		if (test_bit(fgraph_direct_gops->idx, &bitmap))
> -			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops);
> +			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops, fregs);
>  	} else
>  #endif
>  	{
> @@ -806,7 +811,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
>  			if (gops == &fgraph_stub)
>  				continue;
>  
> -			gops->retfunc(&trace, gops);
> +			gops->retfunc(&trace, gops, fregs);
>  		}
>  	}
>  
> @@ -956,7 +961,8 @@ void ftrace_graph_sleep_time_control(bool enable)
>   * Simply points to ftrace_stub, but with the proper protocol.
>   * Defined by the linker script in linux/vmlinux.lds.h
>   */
> -void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
> +void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
> +		       struct ftrace_regs *fregs);
>  
>  /* The callbacks that hook a function */
>  trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 775040a9f541..fd6c5a50c5e5 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -840,7 +840,8 @@ static int profile_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void profile_graph_return(struct ftrace_graph_ret *trace,
> -				 struct fgraph_ops *gops)
> +				 struct fgraph_ops *gops,
> +				 struct ftrace_regs *fregs)
>  {
>  	struct ftrace_ret_stack *ret_stack;
>  	struct ftrace_profile_stat *stat;
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index 28d8ad5e31e6..f4a3f75bd916 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -682,7 +682,8 @@ void trace_latency_header(struct seq_file *m);
>  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);
> +void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
> +			struct ftrace_regs *fregs);
>  int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops,
>  		      struct ftrace_regs *fregs);
>  
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index b9785fc919c9..241407000109 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -240,7 +240,7 @@ void __trace_graph_return(struct trace_array *tr,
>  }
>  
>  void trace_graph_return(struct ftrace_graph_ret *trace,
> -			struct fgraph_ops *gops)
> +			struct fgraph_ops *gops, struct ftrace_regs *fregs)
>  {
>  	unsigned long *task_var = fgraph_get_task_var(gops);
>  	struct trace_array *tr = gops->private;
> @@ -270,7 +270,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
>  }
>  
>  static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
> -				      struct fgraph_ops *gops)
> +				      struct fgraph_ops *gops,
> +				      struct ftrace_regs *fregs)
>  {
>  	ftrace_graph_addr_finish(gops, trace);
>  
> @@ -283,7 +284,7 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
>  	    (trace->rettime - trace->calltime < tracing_thresh))
>  		return;
>  	else
> -		trace_graph_return(trace, gops);
> +		trace_graph_return(trace, gops, fregs);
>  }
>  
>  static struct fgraph_ops funcgraph_ops = {
> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
> index ad739d76fc86..504de7a05498 100644
> --- a/kernel/trace/trace_irqsoff.c
> +++ b/kernel/trace/trace_irqsoff.c
> @@ -208,7 +208,8 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
> -				 struct fgraph_ops *gops)
> +				 struct fgraph_ops *gops,
> +				 struct ftrace_regs *fregs)
>  {
>  	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 23360a2700de..9ffbd9326898 100644
> --- a/kernel/trace/trace_sched_wakeup.c
> +++ b/kernel/trace/trace_sched_wakeup.c
> @@ -144,7 +144,8 @@ static int wakeup_graph_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static void wakeup_graph_return(struct ftrace_graph_ret *trace,
> -				struct fgraph_ops *gops)
> +				struct fgraph_ops *gops,
> +				struct ftrace_regs *fregs)
>  {
>  	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 89067f02094a..1ebd0899238f 100644
> --- a/kernel/trace/trace_selftest.c
> +++ b/kernel/trace/trace_selftest.c
> @@ -807,7 +807,8 @@ static __init int store_entry(struct ftrace_graph_ent *trace,
>  }
>  
>  static __init void store_return(struct ftrace_graph_ret *trace,
> -				struct fgraph_ops *gops)
> +				struct fgraph_ops *gops,
> +				struct ftrace_regs *fregs)
>  {
>  	struct fgraph_fixture *fixture = container_of(gops, struct fgraph_fixture, gops);
>  	const char *type = fixture->store_type_name;

On Sun, Sep 15, 2024 at 04:49:20AM -0400, Steven Rostedt wrote:
> On Fri, 13 Sep 2024 00:09:02 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > Pass ftrace_regs to the fgraph_ops::retfunc(). If ftrace_regs is not
> > available, it passes a NULL instead. User callback function can access
> > some registers (including return address) via this ftrace_regs.
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > ---
> >  Changes in v8:
> >   - Pass ftrace_regs to retfunc, instead of adding retregfunc.
> >  Changes in v6:
> >   - update to use ftrace_regs_get_return_value() because of reordering
> >     patches.
> >  Changes in v3:
> >   - Update for new multiple fgraph.
> >   - Save the return address to instruction pointer in ftrace_regs.
> > ---
> >  include/linux/ftrace.h               |    3 ++-
> >  kernel/trace/fgraph.c                |   16 +++++++++++-----
> >  kernel/trace/ftrace.c                |    3 ++-
> >  kernel/trace/trace.h                 |    3 ++-
> >  kernel/trace/trace_functions_graph.c |    7 ++++---
> >  kernel/trace/trace_irqsoff.c         |    3 ++-
> >  kernel/trace/trace_sched_wakeup.c    |    3 ++-
> >  kernel/trace/trace_selftest.c        |    3 ++-
> >  8 files changed, 27 insertions(+), 14 deletions(-)
> > 
> > diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> > index 13987cd63553..e7c41d9988e1 100644
> > --- a/include/linux/ftrace.h
> > +++ b/include/linux/ftrace.h
> > @@ -1069,7 +1069,8 @@ 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 */
> > +				       struct fgraph_ops *,
> > +				       struct ftrace_regs *); /* return */
> >  typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
> >  				      struct fgraph_ops *,
> >  				      struct ftrace_regs *); /* entry */
> > diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> > index 30bebe43607d..6a3e2db16aa4 100644
> > --- a/kernel/trace/fgraph.c
> > +++ b/kernel/trace/fgraph.c
> > @@ -297,7 +297,8 @@ static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops,
> >  }
> >  
> >  /* ftrace_graph_return set to this to tell some archs to run function graph */
> > -static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops)
> > +static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops,
> > +		       struct ftrace_regs *fregs)
> >  {
> >  }
> >  
> > @@ -491,7 +492,8 @@ int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
> >  }
> >  
> >  static void ftrace_graph_ret_stub(struct ftrace_graph_ret *trace,
> > -				  struct fgraph_ops *gops)
> > +				  struct fgraph_ops *gops,
> > +				  struct ftrace_regs *fregs)
> >  {
> >  }
> >  
> > @@ -787,6 +789,9 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
> >  	}
> >  
> >  	trace.rettime = trace_clock_local();
> > +	if (fregs)
> > +		ftrace_regs_set_instruction_pointer(fregs, ret);

Where does the instruction pointer get used after this? The arm64
'return_to_handler' function doesn't look at it when we return.

Will

On Tue, 17 Sep 2024 11:08:48 +0100
Will Deacon <will@kernel.org> wrote:

> > > @@ -787,6 +789,9 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
> > >  	}
> > >  
> > >  	trace.rettime = trace_clock_local();
> > > +	if (fregs)
> > > +		ftrace_regs_set_instruction_pointer(fregs, ret);  
> 
> Where does the instruction pointer get used after this? The arm64
> 'return_to_handler' function doesn't look at it when we return.

It's for the hooks to the return instruction. kretprobes will start using
function graph tracer to hook to a return of a function (via fprobes), and
the callbacks will need access to the return pointer. The callbacks get
passed the ftrace_regs, and this is how they can see what the function is
returning to. For example, BPF programs will need this.

So it's not needed for the infrastructure, only the callbacks that hook to
it.

-- Steve

On Mon, 30 Sep 2024 15:03:02 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 17 Sep 2024 11:08:48 +0100
> Will Deacon <will@kernel.org> wrote:
> 
> > > > @@ -787,6 +789,9 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
> > > >  	}
> > > >  
> > > >  	trace.rettime = trace_clock_local();
> > > > +	if (fregs)
> > > > +		ftrace_regs_set_instruction_pointer(fregs, ret);  
> > 
> > Where does the instruction pointer get used after this? The arm64
> > 'return_to_handler' function doesn't look at it when we return.
> 
> It's for the hooks to the return instruction. kretprobes will start using

not kretprobes, but fprobe. kretprobes continue using rethook.

> function graph tracer to hook to a return of a function (via fprobes), and
> the callbacks will need access to the return pointer. The callbacks get
> passed the ftrace_regs, and this is how they can see what the function is
> returning to. For example, BPF programs will need this.
> 
> So it's not needed for the infrastructure, only the callbacks that hook to
> it.

Yes, it will be used for showing where to return in the fprobe exit event.
More specifically, in the fprobe_return()@kernel/trace/fprobe.c in PATCH 13/19,
it is extracted from fregs.

+static void fprobe_return(struct ftrace_graph_ret *trace,
+			  struct fgraph_ops *gops,
+			  struct ftrace_regs *fregs)
+{
+	unsigned long *fgraph_data = NULL;
+	unsigned long ret_ip;
+	unsigned long val;
+	struct fprobe *fp;
+	int size, curr;
+	int size_words;
+
+	fgraph_data = (unsigned long *)fgraph_retrieve_data(gops->idx, &size);
+	if (WARN_ON_ONCE(!fgraph_data))
 		return;
+	size_words = SIZE_IN_LONG(size);
+	ret_ip = ftrace_regs_get_instruction_pointer(fregs);
+

Thank you,

> 
> -- Steve


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