Re: [PATCH 7/7] tracing: add arguments to function tracer

On Fri, 6 Sep 2024 11:49:10 +0800
Zheng Yejian [off-list ref] wrote:

quoted

diff --git a/kernel/trace/trace_entries.h b/kernel/trace/trace_entries.h
index c47422b20908..f2021ab52da2 100644
--- a/kernel/trace/trace_entries.h
+++ b/kernel/trace/trace_entries.h

@@ -61,8 +61,11 @@ FTRACE_ENTRY_REG(function, ftrace_entry,
  	TRACE_FN,
  
  	F_STRUCT(
-		__field_fn(	unsigned long,	ip		)
-		__field_fn(	unsigned long,	parent_ip	)
+		__field_fn(	unsigned long,		ip		)
+		__field_fn(	unsigned long,		parent_ip	)
+#ifdef CONFIG_FUNCTION_TRACE_ARGS
+		__field_struct( struct ftrace_regs,	regs		)  

Only function arguments are printed, they are several registers in ftrace_regs,
would it be better to store what are needed?
Although different archs save function arguments in different registers, store
the entire ftrace_regs are much more simple..

Agreed, and I stated as much in my reply to patch 6.

This too will need two versions of the event. One will be function the
other will be function_args. And it will only record the necessary
arguments not the full ftrace_regs structure, as I plan on making that
structure have "zero size".

quoted

+#endif
  	),
  
  	F_printk(" %ps <-- %ps",  

F_printk should also match F_STRUCT, otherwise 'format' info may be incorrect,
it may confuse data parsing in user tools.

Well, it will just ignore the struct part, as its not listed.

-- Steve

quoted

diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index 3b0cea37e029..7ff651a0b45a 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c

@@ -25,6 +25,9 @@ static void
  function_trace_call(unsigned long ip, unsigned long parent_ip,
  		    struct ftrace_ops *op, struct ftrace_regs *fregs);
  static void
+function_args_trace_call(unsigned long ip, unsigned long parent_ip,
+			 struct ftrace_ops *op, struct ftrace_regs *fregs);