* [PATCH 1/7] tracing: add ftrace_regs to function_graph_enter()
2024-09-04 6:58 [PATCH 0/7] add function arguments to ftrace Sven Schnelle
@ 2024-09-04 6:58 ` Sven Schnelle
2024-09-04 6:58 ` [PATCH 2/7] x86/tracing: pass " Sven Schnelle
` (6 subsequent siblings)
7 siblings, 0 replies; 28+ messages in thread
From: Sven Schnelle @ 2024-09-04 6:58 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
Russell King, Catalin Marinas, Will Deacon, Guo Ren, Huacai Chen,
WANG Xuerui, Michal Simek, Thomas Bogendoerfer,
James E.J. Bottomley, Helge Deller, Michael Ellerman,
Nicholas Piggin, Christophe Leroy, Naveen N Rao, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Heiko Carstens, Vasily Gorbik,
Alexander Gordeev, Christian Borntraeger, Yoshinori Sato,
Rich Felker, John Paul Adrian Glaubitz, David S. Miller,
Andreas Larsson, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin
Cc: linux-kernel, linux-trace-kernel, linux-arm-kernel, linux-csky,
loongarch, linux-mips, linux-parisc, linuxppc-dev, linux-riscv,
linux-s390, linux-sh, sparclinux
Will be used later for showing function arguments in the function
graph tracer.
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
arch/arm/kernel/ftrace.c | 2 +-
arch/arm64/kernel/ftrace.c | 2 +-
arch/csky/kernel/ftrace.c | 2 +-
arch/loongarch/kernel/ftrace.c | 2 +-
arch/loongarch/kernel/ftrace_dyn.c | 2 +-
arch/microblaze/kernel/ftrace.c | 2 +-
arch/mips/kernel/ftrace.c | 2 +-
arch/parisc/kernel/ftrace.c | 2 +-
arch/powerpc/kernel/trace/ftrace.c | 2 +-
arch/riscv/kernel/ftrace.c | 2 +-
arch/s390/kernel/ftrace.c | 2 +-
arch/sh/kernel/ftrace.c | 2 +-
arch/sparc/kernel/ftrace.c | 2 +-
arch/x86/kernel/ftrace.c | 2 +-
include/linux/ftrace.h | 3 ++-
kernel/trace/fgraph.c | 3 ++-
16 files changed, 18 insertions(+), 16 deletions(-)
diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index e61591f33a6c..1f8802439e34 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -267,7 +267,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
old = *parent;
*parent = return_hooker;
- if (function_graph_enter(old, self_addr, frame_pointer, NULL))
+ if (function_graph_enter(old, self_addr, frame_pointer, NULL, NULL))
*parent = old;
}
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index a650f5e11fc5..686fbebb0432 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -472,7 +472,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent,
old = *parent;
if (!function_graph_enter(old, self_addr, frame_pointer,
- (void *)frame_pointer)) {
+ (void *)frame_pointer, NULL)) {
*parent = return_hooker;
}
}
diff --git a/arch/csky/kernel/ftrace.c b/arch/csky/kernel/ftrace.c
index 50bfcf129078..c12af268c1cb 100644
--- a/arch/csky/kernel/ftrace.c
+++ b/arch/csky/kernel/ftrace.c
@@ -156,7 +156,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
old = *parent;
if (!function_graph_enter(old, self_addr,
- *(unsigned long *)frame_pointer, parent)) {
+ *(unsigned long *)frame_pointer, parent, NULL)) {
/*
* For csky-gcc function has sub-call:
* subi sp, sp, 8
diff --git a/arch/loongarch/kernel/ftrace.c b/arch/loongarch/kernel/ftrace.c
index 8c3ec1bc7aad..43d908b01718 100644
--- a/arch/loongarch/kernel/ftrace.c
+++ b/arch/loongarch/kernel/ftrace.c
@@ -61,7 +61,7 @@ void prepare_ftrace_return(unsigned long self_addr,
if (ftrace_get_parent_ra_addr(self_addr, &ra_off))
goto out;
- if (!function_graph_enter(old, self_addr, 0, NULL))
+ if (!function_graph_enter(old, self_addr, 0, NULL, NULL))
*(unsigned long *)(callsite_sp + ra_off) = return_hooker;
return;
diff --git a/arch/loongarch/kernel/ftrace_dyn.c b/arch/loongarch/kernel/ftrace_dyn.c
index bff058317062..eab16231d09d 100644
--- a/arch/loongarch/kernel/ftrace_dyn.c
+++ b/arch/loongarch/kernel/ftrace_dyn.c
@@ -233,7 +233,7 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent)
old = *parent;
- if (!function_graph_enter(old, self_addr, 0, parent))
+ if (!function_graph_enter(old, self_addr, 0, parent, NULL))
*parent = return_hooker;
}
diff --git a/arch/microblaze/kernel/ftrace.c b/arch/microblaze/kernel/ftrace.c
index 188749d62709..009800d7e54f 100644
--- a/arch/microblaze/kernel/ftrace.c
+++ b/arch/microblaze/kernel/ftrace.c
@@ -62,7 +62,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
return;
}
- if (function_graph_enter(old, self_addr, 0, NULL))
+ if (function_graph_enter(old, self_addr, 0, NULL, NULL))
*parent = old;
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index 8c401e42301c..65f29de35a59 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -362,7 +362,7 @@ void prepare_ftrace_return(unsigned long *parent_ra_addr, unsigned long self_ra,
insns = core_kernel_text(self_ra) ? 2 : MCOUNT_OFFSET_INSNS + 1;
self_ra -= (MCOUNT_INSN_SIZE * insns);
- if (function_graph_enter(old_parent_ra, self_ra, fp, NULL))
+ if (function_graph_enter(old_parent_ra, self_ra, fp, NULL, NULL))
*parent_ra_addr = old_parent_ra;
return;
out:
diff --git a/arch/parisc/kernel/ftrace.c b/arch/parisc/kernel/ftrace.c
index c91f9c2e61ed..c8d926f057a6 100644
--- a/arch/parisc/kernel/ftrace.c
+++ b/arch/parisc/kernel/ftrace.c
@@ -45,7 +45,7 @@ static void __hot prepare_ftrace_return(unsigned long *parent,
old = *parent;
- if (!function_graph_enter(old, self_addr, 0, NULL))
+ if (!function_graph_enter(old, self_addr, 0, NULL, NULL))
/* activate parisc_return_to_handler() as return point */
*parent = (unsigned long) &parisc_return_to_handler;
}
diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
index d8d6b4fd9a14..8a24d6eabb64 100644
--- a/arch/powerpc/kernel/trace/ftrace.c
+++ b/arch/powerpc/kernel/trace/ftrace.c
@@ -434,7 +434,7 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
if (bit < 0)
goto out;
- if (!function_graph_enter(parent_ip, ip, 0, (unsigned long *)sp))
+ if (!function_graph_enter(parent_ip, ip, 0, (unsigned long *)sp, NULL))
parent_ip = ppc_function_entry(return_to_handler);
ftrace_test_recursion_unlock(bit);
diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index 4b95c574fd04..b45985265b29 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -205,7 +205,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
*/
old = *parent;
- if (!function_graph_enter(old, self_addr, frame_pointer, parent))
+ if (!function_graph_enter(old, self_addr, frame_pointer, parent, NULL))
*parent = return_hooker;
}
diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index 0b6e62d1d8b8..cf9ee90ae216 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -273,7 +273,7 @@ unsigned long prepare_ftrace_return(unsigned long ra, unsigned long sp,
if (unlikely(atomic_read(¤t->tracing_graph_pause)))
goto out;
ip -= MCOUNT_INSN_SIZE;
- if (!function_graph_enter(ra, ip, 0, (void *) sp))
+ if (!function_graph_enter(ra, ip, 0, (void *) sp, NULL))
ra = (unsigned long) return_to_handler;
out:
return ra;
diff --git a/arch/sh/kernel/ftrace.c b/arch/sh/kernel/ftrace.c
index 930001bb8c6a..a9a0a1238214 100644
--- a/arch/sh/kernel/ftrace.c
+++ b/arch/sh/kernel/ftrace.c
@@ -359,7 +359,7 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr)
return;
}
- if (function_graph_enter(old, self_addr, 0, NULL))
+ if (function_graph_enter(old, self_addr, 0, NULL, NULL))
__raw_writel(old, parent);
}
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
diff --git a/arch/sparc/kernel/ftrace.c b/arch/sparc/kernel/ftrace.c
index eaead3da8e03..9ad77a2d9bc4 100644
--- a/arch/sparc/kernel/ftrace.c
+++ b/arch/sparc/kernel/ftrace.c
@@ -125,7 +125,7 @@ unsigned long prepare_ftrace_return(unsigned long parent,
if (unlikely(atomic_read(¤t->tracing_graph_pause)))
return parent + 8UL;
- if (function_graph_enter(parent, self_addr, frame_pointer, NULL))
+ if (function_graph_enter(parent, self_addr, frame_pointer, NULL, NULL))
return parent + 8UL;
return return_hooker;
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 8da0e66ca22d..b325f7e7e39a 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -637,7 +637,7 @@ void prepare_ftrace_return(unsigned long ip, unsigned long *parent,
if (bit < 0)
return;
- if (!function_graph_enter(*parent, ip, frame_pointer, parent))
+ if (!function_graph_enter(*parent, ip, frame_pointer, parent, NULL))
*parent = return_hooker;
ftrace_test_recursion_unlock(bit);
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index fd5e84d0ec47..56d91041ecd2 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1083,7 +1083,8 @@ extern void return_to_handler(void);
extern int
function_graph_enter(unsigned long ret, unsigned long func,
- unsigned long frame_pointer, unsigned long *retp);
+ unsigned long frame_pointer, unsigned long *retp,
+ struct ftrace_regs *fregs);
struct ftrace_ret_stack *
ftrace_graph_get_ret_stack(struct task_struct *task, int skip);
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index d1d5ea2d0a1b..fa62ebfa0711 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -613,7 +613,8 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func,
/* If the caller does not use ftrace, call this function. */
int function_graph_enter(unsigned long ret, unsigned long func,
- unsigned long frame_pointer, unsigned long *retp)
+ unsigned long frame_pointer, unsigned long *retp,
+ struct ftrace_regs *fregs)
{
struct ftrace_graph_ent trace;
unsigned long bitmap = 0;
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 2/7] x86/tracing: pass ftrace_regs to function_graph_enter()
2024-09-04 6:58 [PATCH 0/7] add function arguments to ftrace Sven Schnelle
2024-09-04 6:58 ` [PATCH 1/7] tracing: add ftrace_regs to function_graph_enter() Sven Schnelle
@ 2024-09-04 6:58 ` Sven Schnelle
2024-09-05 8:58 ` kernel test robot
2024-09-04 6:58 ` [PATCH 3/7] s390/tracing: " Sven Schnelle
` (5 subsequent siblings)
7 siblings, 1 reply; 28+ messages in thread
From: Sven Schnelle @ 2024-09-04 6:58 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen, x86,
H. Peter Anvin
Cc: linux-kernel, linux-trace-kernel
Will be used later for printing function argument in the
function graph tracer.
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
arch/x86/include/asm/ftrace.h | 2 +-
arch/x86/kernel/ftrace.c | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index 0152a81d9b4a..9843d0c08e61 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -105,7 +105,7 @@ struct dyn_arch_ftrace {
#ifndef __ASSEMBLY__
void prepare_ftrace_return(unsigned long ip, unsigned long *parent,
- unsigned long frame_pointer);
+ unsigned long frame_pointer, struct ftrace_regs *regs);
#if defined(CONFIG_FUNCTION_TRACER) && defined(CONFIG_DYNAMIC_FTRACE)
extern void set_ftrace_ops_ro(void);
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index b325f7e7e39a..a016b82de5e2 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -610,7 +610,7 @@ int ftrace_disable_ftrace_graph_caller(void)
* in current thread info.
*/
void prepare_ftrace_return(unsigned long ip, unsigned long *parent,
- unsigned long frame_pointer)
+ unsigned long frame_pointer, struct ftrace_regs *fregs)
{
unsigned long return_hooker = (unsigned long)&return_to_handler;
int bit;
@@ -637,7 +637,7 @@ void prepare_ftrace_return(unsigned long ip, unsigned long *parent,
if (bit < 0)
return;
- if (!function_graph_enter(*parent, ip, frame_pointer, parent, NULL))
+ if (!function_graph_enter(*parent, ip, frame_pointer, parent, fregs))
*parent = return_hooker;
ftrace_test_recursion_unlock(bit);
@@ -650,7 +650,7 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
struct pt_regs *regs = &fregs->regs;
unsigned long *stack = (unsigned long *)kernel_stack_pointer(regs);
- prepare_ftrace_return(ip, (unsigned long *)stack, 0);
+ prepare_ftrace_return(ip, (unsigned long *)stack, 0, fregs);
}
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 2/7] x86/tracing: pass ftrace_regs to function_graph_enter()
2024-09-04 6:58 ` [PATCH 2/7] x86/tracing: pass " Sven Schnelle
@ 2024-09-05 8:58 ` kernel test robot
0 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2024-09-05 8:58 UTC (permalink / raw)
To: Sven Schnelle, Steven Rostedt, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
Dave Hansen, x86, H. Peter Anvin
Cc: oe-kbuild-all, linux-kernel, linux-trace-kernel
Hi Sven,
kernel test robot noticed the following build warnings:
[auto build test WARNING on s390/features]
[also build test WARNING on tip/x86/core linus/master v6.11-rc6 next-20240904]
[cannot apply to rostedt-trace/for-next rostedt-trace/for-next-urgent]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sven-Schnelle/tracing-add-ftrace_regs-to-function_graph_enter/20240904-150232
base: https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
patch link: https://lore.kernel.org/r/20240904065908.1009086-3-svens%40linux.ibm.com
patch subject: [PATCH 2/7] x86/tracing: pass ftrace_regs to function_graph_enter()
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20240905/202409051620.5YBbYREt-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240905/202409051620.5YBbYREt-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409051620.5YBbYREt-lkp@intel.com/
All warnings (new ones prefixed by >>):
In file included from include/linux/ftrace.h:23,
from include/linux/kprobes.h:28,
from arch/x86/kernel/process_64.c:35:
>> arch/x86/include/asm/ftrace.h:108:64: warning: 'struct ftrace_regs' declared inside parameter list will not be visible outside of this definition or declaration
108 | unsigned long frame_pointer, struct ftrace_regs *regs);
| ^~~~~~~~~~~
vim +108 arch/x86/include/asm/ftrace.h
106
107 void prepare_ftrace_return(unsigned long ip, unsigned long *parent,
> 108 unsigned long frame_pointer, struct ftrace_regs *regs);
109
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/7] s390/tracing: pass ftrace_regs to function_graph_enter()
2024-09-04 6:58 [PATCH 0/7] add function arguments to ftrace Sven Schnelle
2024-09-04 6:58 ` [PATCH 1/7] tracing: add ftrace_regs to function_graph_enter() Sven Schnelle
2024-09-04 6:58 ` [PATCH 2/7] x86/tracing: pass " Sven Schnelle
@ 2024-09-04 6:58 ` Sven Schnelle
2024-09-04 6:58 ` [PATCH 4/7] Add print_function_args() Sven Schnelle
` (4 subsequent siblings)
7 siblings, 0 replies; 28+ messages in thread
From: Sven Schnelle @ 2024-09-04 6:58 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
Heiko Carstens, Vasily Gorbik, Alexander Gordeev,
Christian Borntraeger
Cc: linux-kernel, linux-trace-kernel, linux-s390
Will be used later to print function arguments in the function
graph tracer.
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
arch/s390/kernel/entry.h | 4 +++-
arch/s390/kernel/ftrace.c | 4 ++--
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/s390/kernel/entry.h b/arch/s390/kernel/entry.h
index 21969520f947..d0a5fbaad987 100644
--- a/arch/s390/kernel/entry.h
+++ b/arch/s390/kernel/entry.h
@@ -5,6 +5,7 @@
#include <linux/percpu.h>
#include <linux/types.h>
#include <linux/signal.h>
+#include <linux/ftrace.h>
#include <asm/extable.h>
#include <asm/ptrace.h>
#include <asm/idle.h>
@@ -41,7 +42,8 @@ void do_restart(void *arg);
void __init startup_init(void);
void die(struct pt_regs *regs, const char *str);
int setup_profiling_timer(unsigned int multiplier);
-unsigned long prepare_ftrace_return(unsigned long parent, unsigned long sp, unsigned long ip);
+unsigned long prepare_ftrace_return(unsigned long parent, unsigned long sp, unsigned long ip,
+ struct ftrace_regs *regs);
struct s390_mmap_arg_struct;
struct fadvise64_64_args;
diff --git a/arch/s390/kernel/ftrace.c b/arch/s390/kernel/ftrace.c
index cf9ee90ae216..dd77b656d9b9 100644
--- a/arch/s390/kernel/ftrace.c
+++ b/arch/s390/kernel/ftrace.c
@@ -266,14 +266,14 @@ void ftrace_arch_code_modify_post_process(void)
* in current thread info.
*/
unsigned long prepare_ftrace_return(unsigned long ra, unsigned long sp,
- unsigned long ip)
+ unsigned long ip, struct ftrace_regs *regs)
{
if (unlikely(ftrace_graph_is_dead()))
goto out;
if (unlikely(atomic_read(¤t->tracing_graph_pause)))
goto out;
ip -= MCOUNT_INSN_SIZE;
- if (!function_graph_enter(ra, ip, 0, (void *) sp, NULL))
+ if (!function_graph_enter(ra, ip, 0, (void *) sp, regs))
ra = (unsigned long) return_to_handler;
out:
return ra;
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH 4/7] Add print_function_args()
2024-09-04 6:58 [PATCH 0/7] add function arguments to ftrace Sven Schnelle
` (2 preceding siblings ...)
2024-09-04 6:58 ` [PATCH 3/7] s390/tracing: " Sven Schnelle
@ 2024-09-04 6:58 ` Sven Schnelle
2024-09-08 15:17 ` Masami Hiramatsu
2024-09-09 3:13 ` Donglin Peng
2024-09-04 6:58 ` [PATCH 5/7] tracing: add config option for print arguments in ftrace Sven Schnelle
` (3 subsequent siblings)
7 siblings, 2 replies; 28+ messages in thread
From: Sven Schnelle @ 2024-09-04 6:58 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel, bpf
Add a function to decode argument types with the help of BTF. Will
be used to display arguments in the function and function graph
tracer.
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
kernel/trace/trace_output.c | 68 +++++++++++++++++++++++++++++++++++++
kernel/trace/trace_output.h | 9 +++++
2 files changed, 77 insertions(+)
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index d8b302d01083..70405c4cceb6 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -12,8 +12,11 @@
#include <linux/sched/clock.h>
#include <linux/sched/mm.h>
#include <linux/idr.h>
+#include <linux/btf.h>
+#include <linux/bpf.h>
#include "trace_output.h"
+#include "trace_btf.h"
/* must be a power of 2 */
#define EVENT_HASHSIZE 128
@@ -669,6 +672,71 @@ int trace_print_lat_context(struct trace_iterator *iter)
return !trace_seq_has_overflowed(s);
}
+#ifdef CONFIG_FUNCTION_TRACE_ARGS
+void print_function_args(struct trace_seq *s, struct ftrace_regs *fregs,
+ unsigned long func)
+{
+ const struct btf_param *param;
+ const struct btf_type *t;
+ const char *param_name;
+ char name[KSYM_NAME_LEN];
+ unsigned long arg;
+ struct btf *btf;
+ s32 tid, nr = 0;
+ int i;
+
+ trace_seq_printf(s, "(");
+
+ if (!ftrace_regs_has_args(fregs))
+ goto out;
+ if (lookup_symbol_name(func, name))
+ goto out;
+
+ btf = bpf_get_btf_vmlinux();
+ if (IS_ERR_OR_NULL(btf))
+ goto out;
+
+ t = btf_find_func_proto(name, &btf);
+ if (IS_ERR_OR_NULL(t))
+ goto out;
+
+ param = btf_get_func_param(t, &nr);
+ if (!param)
+ goto out_put;
+
+ for (i = 0; i < nr; i++) {
+ arg = ftrace_regs_get_argument(fregs, i);
+
+ param_name = btf_name_by_offset(btf, param[i].name_off);
+ if (param_name)
+ trace_seq_printf(s, "%s = ", param_name);
+ t = btf_type_skip_modifiers(btf, param[i].type, &tid);
+ if (!t)
+ continue;
+ switch (BTF_INFO_KIND(t->info)) {
+ case BTF_KIND_PTR:
+ trace_seq_printf(s, "0x%lx", arg);
+ break;
+ case BTF_KIND_INT:
+ trace_seq_printf(s, "%ld", arg);
+ break;
+ case BTF_KIND_ENUM:
+ trace_seq_printf(s, "%ld", arg);
+ break;
+ default:
+ trace_seq_printf(s, "0x%lx (%d)", arg, BTF_INFO_KIND(param[i].type));
+ break;
+ }
+ if (i < nr - 1)
+ trace_seq_printf(s, ", ");
+ }
+out_put:
+ btf_put(btf);
+out:
+ trace_seq_printf(s, ")");
+}
+#endif
+
/**
* ftrace_find_event - find a registered event
* @type: the type of event to look for
diff --git a/kernel/trace/trace_output.h b/kernel/trace/trace_output.h
index dca40f1f1da4..a21d8ce606f7 100644
--- a/kernel/trace/trace_output.h
+++ b/kernel/trace/trace_output.h
@@ -41,5 +41,14 @@ extern struct rw_semaphore trace_event_sem;
#define SEQ_PUT_HEX_FIELD(s, x) \
trace_seq_putmem_hex(s, &(x), sizeof(x))
+#ifdef CONFIG_FUNCTION_TRACE_ARGS
+void print_function_args(struct trace_seq *s, struct ftrace_regs *fregs,
+ unsigned long func);
+#else
+static inline void print_function_args(struct trace_seq *s, struct ftrace_regs *fregs,
+ unsigned long func) {
+ trace_seq_puts(s, "()");
+}
+#endif
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 4/7] Add print_function_args()
2024-09-04 6:58 ` [PATCH 4/7] Add print_function_args() Sven Schnelle
@ 2024-09-08 15:17 ` Masami Hiramatsu
2024-09-09 3:13 ` Donglin Peng
1 sibling, 0 replies; 28+ messages in thread
From: Masami Hiramatsu @ 2024-09-08 15:17 UTC (permalink / raw)
To: Sven Schnelle
Cc: Steven Rostedt, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, bpf
On Wed, 4 Sep 2024 08:58:58 +0200
Sven Schnelle <svens@linux.ibm.com> wrote:
> Add a function to decode argument types with the help of BTF. Will
> be used to display arguments in the function and function graph
> tracer.
>
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> ---
> kernel/trace/trace_output.c | 68 +++++++++++++++++++++++++++++++++++++
> kernel/trace/trace_output.h | 9 +++++
> 2 files changed, 77 insertions(+)
>
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index d8b302d01083..70405c4cceb6 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -12,8 +12,11 @@
> #include <linux/sched/clock.h>
> #include <linux/sched/mm.h>
> #include <linux/idr.h>
> +#include <linux/btf.h>
> +#include <linux/bpf.h>
>
> #include "trace_output.h"
> +#include "trace_btf.h"
>
> /* must be a power of 2 */
> #define EVENT_HASHSIZE 128
> @@ -669,6 +672,71 @@ int trace_print_lat_context(struct trace_iterator *iter)
> return !trace_seq_has_overflowed(s);
> }
>
> +#ifdef CONFIG_FUNCTION_TRACE_ARGS
> +void print_function_args(struct trace_seq *s, struct ftrace_regs *fregs,
> + unsigned long func)
> +{
> + const struct btf_param *param;
> + const struct btf_type *t;
> + const char *param_name;
> + char name[KSYM_NAME_LEN];
> + unsigned long arg;
> + struct btf *btf;
> + s32 tid, nr = 0;
> + int i;
> +
> + trace_seq_printf(s, "(");
> +
> + if (!ftrace_regs_has_args(fregs))
> + goto out;
> + if (lookup_symbol_name(func, name))
> + goto out;
> +
> + btf = bpf_get_btf_vmlinux();
> + if (IS_ERR_OR_NULL(btf))
> + goto out;
> +
> + t = btf_find_func_proto(name, &btf);
> + if (IS_ERR_OR_NULL(t))
> + goto out;
> +
> + param = btf_get_func_param(t, &nr);
> + if (!param)
> + goto out_put;
> +
> + for (i = 0; i < nr; i++) {
> + arg = ftrace_regs_get_argument(fregs, i);
> +
> + param_name = btf_name_by_offset(btf, param[i].name_off);
> + if (param_name)
> + trace_seq_printf(s, "%s = ", param_name);
> + t = btf_type_skip_modifiers(btf, param[i].type, &tid);
> + if (!t)
> + continue;
> + switch (BTF_INFO_KIND(t->info)) {
> + case BTF_KIND_PTR:
> + trace_seq_printf(s, "0x%lx", arg);
> + break;
> + case BTF_KIND_INT:
> + trace_seq_printf(s, "%ld", arg);
Don't we check the size and signed? :)
> + break;
> + case BTF_KIND_ENUM:
> + trace_seq_printf(s, "%ld", arg);
nit: %d? (enum is equal to the int type)
BTW, this series splits the patches by coding, not functionality.
For the first review, it is OK. But eventually those should be merged.
Thank you,
> + break;
> + default:
> + trace_seq_printf(s, "0x%lx (%d)", arg, BTF_INFO_KIND(param[i].type));
> + break;
> + }
> + if (i < nr - 1)
> + trace_seq_printf(s, ", ");
> + }
> +out_put:
> + btf_put(btf);
> +out:
> + trace_seq_printf(s, ")");
> +}
> +#endif
> +
> /**
> * ftrace_find_event - find a registered event
> * @type: the type of event to look for
> diff --git a/kernel/trace/trace_output.h b/kernel/trace/trace_output.h
> index dca40f1f1da4..a21d8ce606f7 100644
> --- a/kernel/trace/trace_output.h
> +++ b/kernel/trace/trace_output.h
> @@ -41,5 +41,14 @@ extern struct rw_semaphore trace_event_sem;
> #define SEQ_PUT_HEX_FIELD(s, x) \
> trace_seq_putmem_hex(s, &(x), sizeof(x))
>
> +#ifdef CONFIG_FUNCTION_TRACE_ARGS
> +void print_function_args(struct trace_seq *s, struct ftrace_regs *fregs,
> + unsigned long func);
> +#else
> +static inline void print_function_args(struct trace_seq *s, struct ftrace_regs *fregs,
> + unsigned long func) {
> + trace_seq_puts(s, "()");
> +}
> +#endif
> #endif
>
> --
> 2.43.0
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 4/7] Add print_function_args()
2024-09-04 6:58 ` [PATCH 4/7] Add print_function_args() Sven Schnelle
2024-09-08 15:17 ` Masami Hiramatsu
@ 2024-09-09 3:13 ` Donglin Peng
1 sibling, 0 replies; 28+ messages in thread
From: Donglin Peng @ 2024-09-09 3:13 UTC (permalink / raw)
To: Sven Schnelle
Cc: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
linux-kernel, linux-trace-kernel, bpf
On Wed, Sep 4, 2024 at 2:59 PM Sven Schnelle <svens@linux.ibm.com> wrote:
>
> Add a function to decode argument types with the help of BTF. Will
> be used to display arguments in the function and function graph
> tracer.
>
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> ---
> kernel/trace/trace_output.c | 68 +++++++++++++++++++++++++++++++++++++
> kernel/trace/trace_output.h | 9 +++++
> 2 files changed, 77 insertions(+)
>
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index d8b302d01083..70405c4cceb6 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -12,8 +12,11 @@
> #include <linux/sched/clock.h>
> #include <linux/sched/mm.h>
> #include <linux/idr.h>
> +#include <linux/btf.h>
> +#include <linux/bpf.h>
>
> #include "trace_output.h"
> +#include "trace_btf.h"
>
> /* must be a power of 2 */
> #define EVENT_HASHSIZE 128
> @@ -669,6 +672,71 @@ int trace_print_lat_context(struct trace_iterator *iter)
> return !trace_seq_has_overflowed(s);
> }
>
> +#ifdef CONFIG_FUNCTION_TRACE_ARGS
> +void print_function_args(struct trace_seq *s, struct ftrace_regs *fregs,
> + unsigned long func)
> +{
> + const struct btf_param *param;
> + const struct btf_type *t;
> + const char *param_name;
> + char name[KSYM_NAME_LEN];
> + unsigned long arg;
> + struct btf *btf;
> + s32 tid, nr = 0;
> + int i;
> +
> + trace_seq_printf(s, "(");
> +
> + if (!ftrace_regs_has_args(fregs))
> + goto out;
> + if (lookup_symbol_name(func, name))
> + goto out;
> +
> + btf = bpf_get_btf_vmlinux();
> + if (IS_ERR_OR_NULL(btf))
> + goto out;
> +
> + t = btf_find_func_proto(name, &btf);
This is an excellent feature, and I am crafting a series of patches aimed
at enhancing the search performance for locating the btf by its name.
> + if (IS_ERR_OR_NULL(t))
> + goto out;
> +
> + param = btf_get_func_param(t, &nr);
> + if (!param)
> + goto out_put;
> +
> + for (i = 0; i < nr; i++) {
> + arg = ftrace_regs_get_argument(fregs, i);
> +
> + param_name = btf_name_by_offset(btf, param[i].name_off);
> + if (param_name)
> + trace_seq_printf(s, "%s = ", param_name);
> + t = btf_type_skip_modifiers(btf, param[i].type, &tid);
> + if (!t)
> + continue;
> + switch (BTF_INFO_KIND(t->info)) {
> + case BTF_KIND_PTR:
> + trace_seq_printf(s, "0x%lx", arg);
> + break;
> + case BTF_KIND_INT:
> + trace_seq_printf(s, "%ld", arg);
> + break;
> + case BTF_KIND_ENUM:
> + trace_seq_printf(s, "%ld", arg);
> + break;
> + default:
> + trace_seq_printf(s, "0x%lx (%d)", arg, BTF_INFO_KIND(param[i].type));
> + break;
> + }
> + if (i < nr - 1)
> + trace_seq_printf(s, ", ");
> + }
> +out_put:
> + btf_put(btf);
> +out:
> + trace_seq_printf(s, ")");
> +}
> +#endif
> +
> /**
> * ftrace_find_event - find a registered event
> * @type: the type of event to look for
> diff --git a/kernel/trace/trace_output.h b/kernel/trace/trace_output.h
> index dca40f1f1da4..a21d8ce606f7 100644
> --- a/kernel/trace/trace_output.h
> +++ b/kernel/trace/trace_output.h
> @@ -41,5 +41,14 @@ extern struct rw_semaphore trace_event_sem;
> #define SEQ_PUT_HEX_FIELD(s, x) \
> trace_seq_putmem_hex(s, &(x), sizeof(x))
>
> +#ifdef CONFIG_FUNCTION_TRACE_ARGS
> +void print_function_args(struct trace_seq *s, struct ftrace_regs *fregs,
> + unsigned long func);
> +#else
> +static inline void print_function_args(struct trace_seq *s, struct ftrace_regs *fregs,
> + unsigned long func) {
> + trace_seq_puts(s, "()");
> +}
> +#endif
> #endif
>
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 5/7] tracing: add config option for print arguments in ftrace
2024-09-04 6:58 [PATCH 0/7] add function arguments to ftrace Sven Schnelle
` (3 preceding siblings ...)
2024-09-04 6:58 ` [PATCH 4/7] Add print_function_args() Sven Schnelle
@ 2024-09-04 6:58 ` Sven Schnelle
2024-09-06 3:36 ` Zheng Yejian
2024-09-04 6:59 ` [PATCH 6/7] tracing: add support for function argument to graph tracer Sven Schnelle
` (2 subsequent siblings)
7 siblings, 1 reply; 28+ messages in thread
From: Sven Schnelle @ 2024-09-04 6:58 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel
Add a config option to disable/enable function argument
printing support during runtime.
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
kernel/trace/Kconfig | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index 721c3b221048..8b9b6cdf39ac 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -242,6 +242,18 @@ config FUNCTION_GRAPH_RETVAL
enable it via the trace option funcgraph-retval.
See Documentation/trace/ftrace.rst
+config FUNCTION_TRACE_ARGS
+ bool "Kernel Function Tracer Arguments"
+ depends on HAVE_FUNCTION_ARG_ACCESS_API
+ depends on DEBUG_INFO_BTF && BPF_SYSCALL
+ default n
+ help
+ Support recording and printing of function arguments when using
+ the function tracer or function graph tracer. This feature is off
+ by default, and can be enabled via the trace option func-args (for
+ the function tracer) and funcgraph-args (for the function graph
+ tracer).
+
config DYNAMIC_FTRACE
bool "enable/disable function tracing dynamically"
depends on FUNCTION_TRACER
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 5/7] tracing: add config option for print arguments in ftrace
2024-09-04 6:58 ` [PATCH 5/7] tracing: add config option for print arguments in ftrace Sven Schnelle
@ 2024-09-06 3:36 ` Zheng Yejian
2024-09-08 13:30 ` Masami Hiramatsu
0 siblings, 1 reply; 28+ messages in thread
From: Zheng Yejian @ 2024-09-06 3:36 UTC (permalink / raw)
To: Sven Schnelle, Steven Rostedt, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel
On 2024/9/4 14:58, Sven Schnelle wrote:
> Add a config option to disable/enable function argument
> printing support during runtime.
>
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> ---
> kernel/trace/Kconfig | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> index 721c3b221048..8b9b6cdf39ac 100644
> --- a/kernel/trace/Kconfig
> +++ b/kernel/trace/Kconfig
> @@ -242,6 +242,18 @@ config FUNCTION_GRAPH_RETVAL
> enable it via the trace option funcgraph-retval.
> See Documentation/trace/ftrace.rst
>
> +config FUNCTION_TRACE_ARGS
> + bool "Kernel Function Tracer Arguments"
> + depends on HAVE_FUNCTION_ARG_ACCESS_API
> + depends on DEBUG_INFO_BTF && BPF_SYSCALL
Nice feature!
Just a nit, DEBUG_INFO_BTF currently depends on BPF_SYSCALL,
so BPF_SYSCALL may not be necessary here. This feature
also doesn't seem to depend on bpf.
> + default n
> + help
> + Support recording and printing of function arguments when using
> + the function tracer or function graph tracer. This feature is off
> + by default, and can be enabled via the trace option func-args (for
> + the function tracer) and funcgraph-args (for the function graph
> + tracer).
> +
> config DYNAMIC_FTRACE
> bool "enable/disable function tracing dynamically"
> depends on FUNCTION_TRACER
--
Thanks,
Zheng Yejian
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/7] tracing: add config option for print arguments in ftrace
2024-09-06 3:36 ` Zheng Yejian
@ 2024-09-08 13:30 ` Masami Hiramatsu
2024-09-09 8:16 ` Sven Schnelle
0 siblings, 1 reply; 28+ messages in thread
From: Masami Hiramatsu @ 2024-09-08 13:30 UTC (permalink / raw)
To: Zheng Yejian
Cc: Sven Schnelle, Steven Rostedt, Mark Rutland, Mathieu Desnoyers,
linux-kernel, linux-trace-kernel
On Fri, 6 Sep 2024 11:36:11 +0800
Zheng Yejian <zhengyejian@huaweicloud.com> wrote:
> On 2024/9/4 14:58, Sven Schnelle wrote:
> > Add a config option to disable/enable function argument
> > printing support during runtime.
> >
> > Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> > ---
> > kernel/trace/Kconfig | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> > index 721c3b221048..8b9b6cdf39ac 100644
> > --- a/kernel/trace/Kconfig
> > +++ b/kernel/trace/Kconfig
> > @@ -242,6 +242,18 @@ config FUNCTION_GRAPH_RETVAL
> > enable it via the trace option funcgraph-retval.
> > See Documentation/trace/ftrace.rst
> >
> > +config FUNCTION_TRACE_ARGS
> > + bool "Kernel Function Tracer Arguments"
> > + depends on HAVE_FUNCTION_ARG_ACCESS_API
> > + depends on DEBUG_INFO_BTF && BPF_SYSCALL
>
> Nice feature!
>
> Just a nit, DEBUG_INFO_BTF currently depends on BPF_SYSCALL,
> so BPF_SYSCALL may not be necessary here. This feature
> also doesn't seem to depend on bpf.
Indeed. Sven, you can check the PROBE_EVENTS_BTF_ARGS as
an example.
config PROBE_EVENTS_BTF_ARGS
depends on HAVE_FUNCTION_ARG_ACCESS_API
depends on FPROBE_EVENTS || KPROBE_EVENTS
depends on DEBUG_INFO_BTF && BPF_SYSCALL
bool "Support BTF function arguments for probe events"
Thank you,
>
> > + default n
> > + help
> > + Support recording and printing of function arguments when using
> > + the function tracer or function graph tracer. This feature is off
> > + by default, and can be enabled via the trace option func-args (for
> > + the function tracer) and funcgraph-args (for the function graph
> > + tracer).
> > +
> > config DYNAMIC_FTRACE
> > bool "enable/disable function tracing dynamically"
> > depends on FUNCTION_TRACER
>
> --
> Thanks,
> Zheng Yejian
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 5/7] tracing: add config option for print arguments in ftrace
2024-09-08 13:30 ` Masami Hiramatsu
@ 2024-09-09 8:16 ` Sven Schnelle
2024-09-09 13:56 ` Masami Hiramatsu
0 siblings, 1 reply; 28+ messages in thread
From: Sven Schnelle @ 2024-09-09 8:16 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Zheng Yejian, Steven Rostedt, Mark Rutland, Mathieu Desnoyers,
linux-kernel, linux-trace-kernel
Masami Hiramatsu (Google) <mhiramat@kernel.org> writes:
> On Fri, 6 Sep 2024 11:36:11 +0800
> Zheng Yejian <zhengyejian@huaweicloud.com> wrote:
>
>> On 2024/9/4 14:58, Sven Schnelle wrote:
>> > Add a config option to disable/enable function argument
>> > printing support during runtime.
>> >
>> > Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
>> > ---
>> > kernel/trace/Kconfig | 12 ++++++++++++
>> > 1 file changed, 12 insertions(+)
>> >
>> > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
>> > index 721c3b221048..8b9b6cdf39ac 100644
>> > --- a/kernel/trace/Kconfig
>> > +++ b/kernel/trace/Kconfig
>> > @@ -242,6 +242,18 @@ config FUNCTION_GRAPH_RETVAL
>> > enable it via the trace option funcgraph-retval.
>> > See Documentation/trace/ftrace.rst
>> >
>> > +config FUNCTION_TRACE_ARGS
>> > + bool "Kernel Function Tracer Arguments"
>> > + depends on HAVE_FUNCTION_ARG_ACCESS_API
>> > + depends on DEBUG_INFO_BTF && BPF_SYSCALL
>>
>> Nice feature!
>>
>> Just a nit, DEBUG_INFO_BTF currently depends on BPF_SYSCALL,
>> so BPF_SYSCALL may not be necessary here. This feature
>> also doesn't seem to depend on bpf.
>
> Indeed. Sven, you can check the PROBE_EVENTS_BTF_ARGS as
> an example.
>
> config PROBE_EVENTS_BTF_ARGS
> depends on HAVE_FUNCTION_ARG_ACCESS_API
> depends on FPROBE_EVENTS || KPROBE_EVENTS
> depends on DEBUG_INFO_BTF && BPF_SYSCALL
> bool "Support BTF function arguments for probe events"
Now i'm confused - Zheng wrote that DEBUG_INFO_BTF depends on
BPF_SYSCALL. I just verified that. So i could just remove BPF_SYSCALL
from the dependencies - but your example also has BPF_SYSCALL listed.
Regards
Sven
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 5/7] tracing: add config option for print arguments in ftrace
2024-09-09 8:16 ` Sven Schnelle
@ 2024-09-09 13:56 ` Masami Hiramatsu
0 siblings, 0 replies; 28+ messages in thread
From: Masami Hiramatsu @ 2024-09-09 13:56 UTC (permalink / raw)
To: Sven Schnelle
Cc: Zheng Yejian, Steven Rostedt, Mark Rutland, Mathieu Desnoyers,
linux-kernel, linux-trace-kernel
On Mon, 09 Sep 2024 10:16:55 +0200
Sven Schnelle <svens@linux.ibm.com> wrote:
> Masami Hiramatsu (Google) <mhiramat@kernel.org> writes:
>
> > On Fri, 6 Sep 2024 11:36:11 +0800
> > Zheng Yejian <zhengyejian@huaweicloud.com> wrote:
> >
> >> On 2024/9/4 14:58, Sven Schnelle wrote:
> >> > Add a config option to disable/enable function argument
> >> > printing support during runtime.
> >> >
> >> > Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> >> > ---
> >> > kernel/trace/Kconfig | 12 ++++++++++++
> >> > 1 file changed, 12 insertions(+)
> >> >
> >> > diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
> >> > index 721c3b221048..8b9b6cdf39ac 100644
> >> > --- a/kernel/trace/Kconfig
> >> > +++ b/kernel/trace/Kconfig
> >> > @@ -242,6 +242,18 @@ config FUNCTION_GRAPH_RETVAL
> >> > enable it via the trace option funcgraph-retval.
> >> > See Documentation/trace/ftrace.rst
> >> >
> >> > +config FUNCTION_TRACE_ARGS
> >> > + bool "Kernel Function Tracer Arguments"
> >> > + depends on HAVE_FUNCTION_ARG_ACCESS_API
> >> > + depends on DEBUG_INFO_BTF && BPF_SYSCALL
> >>
> >> Nice feature!
> >>
> >> Just a nit, DEBUG_INFO_BTF currently depends on BPF_SYSCALL,
> >> so BPF_SYSCALL may not be necessary here. This feature
> >> also doesn't seem to depend on bpf.
> >
> > Indeed. Sven, you can check the PROBE_EVENTS_BTF_ARGS as
> > an example.
> >
> > config PROBE_EVENTS_BTF_ARGS
> > depends on HAVE_FUNCTION_ARG_ACCESS_API
> > depends on FPROBE_EVENTS || KPROBE_EVENTS
> > depends on DEBUG_INFO_BTF && BPF_SYSCALL
> > bool "Support BTF function arguments for probe events"
>
> Now i'm confused - Zheng wrote that DEBUG_INFO_BTF depends on
> BPF_SYSCALL. I just verified that. So i could just remove BPF_SYSCALL
> from the dependencies - but your example also has BPF_SYSCALL listed.
Ah, sorry for confusion. In this case, just need DEBUG_INFO_BTF.
Hmm, I think I also need to remove BPF_SYSCALL.
Thanks,
>
> Regards
> Sven
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 6/7] tracing: add support for function argument to graph tracer
2024-09-04 6:58 [PATCH 0/7] add function arguments to ftrace Sven Schnelle
` (4 preceding siblings ...)
2024-09-04 6:58 ` [PATCH 5/7] tracing: add config option for print arguments in ftrace Sven Schnelle
@ 2024-09-04 6:59 ` Sven Schnelle
2024-09-05 8:05 ` kernel test robot
` (2 more replies)
2024-09-04 6:59 ` [PATCH 7/7] tracing: add arguments to function tracer Sven Schnelle
2024-09-05 15:16 ` [PATCH 0/7] add function arguments to ftrace Steven Rostedt
7 siblings, 3 replies; 28+ messages in thread
From: Sven Schnelle @ 2024-09-04 6:59 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel, bpf
Wire up the code to print function arguments in the function graph
tracer. This functionality can be enabled/disabled during compile
time by setting CONFIG_FUNCTION_TRACE_ARGS and during runtime with
options/funcgraph-args.
Example usage:
6) | dummy_xmit [dummy](skb = 0x8887c100, dev = 0x872ca000) {
6) | consume_skb(skb = 0x8887c100) {
6) | skb_release_head_state(skb = 0x8887c100) {
6) 0.178 us | sock_wfree(skb = 0x8887c100)
6) 0.627 us | }
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
include/linux/ftrace.h | 1 +
kernel/trace/fgraph.c | 6 ++-
kernel/trace/trace_functions_graph.c | 74 ++++++++++++++--------------
3 files changed, 44 insertions(+), 37 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 56d91041ecd2..5d0ff66f8a70 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -1010,6 +1010,7 @@ static inline void ftrace_init(void) { }
* to remove extra padding at the end.
*/
struct ftrace_graph_ent {
+ struct ftrace_regs regs;
unsigned long func; /* Current function */
int depth;
} __packed;
diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
index fa62ebfa0711..f4bb10c0aa52 100644
--- a/kernel/trace/fgraph.c
+++ b/kernel/trace/fgraph.c
@@ -614,7 +614,7 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func,
/* If the caller does not use ftrace, call this function. */
int function_graph_enter(unsigned long ret, unsigned long func,
unsigned long frame_pointer, unsigned long *retp,
- struct ftrace_regs *fregs)
+ struct ftrace_regs *fregs)
{
struct ftrace_graph_ent trace;
unsigned long bitmap = 0;
@@ -623,6 +623,10 @@ int function_graph_enter(unsigned long ret, unsigned long func,
trace.func = func;
trace.depth = ++current->curr_ret_depth;
+ if (IS_ENABLED(CONFIG_FUNCTION_TRACE_ARGS) && fregs)
+ trace.regs = *fregs;
+ else
+ memset(&trace.regs, 0, sizeof(struct ftrace_regs));
offset = ftrace_push_return_trace(ret, func, frame_pointer, retp, 0);
if (offset < 0)
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 13d0387ac6a6..be0cee52944a 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -12,6 +12,8 @@
#include <linux/interrupt.h>
#include <linux/slab.h>
#include <linux/fs.h>
+#include <linux/btf.h>
+#include <linux/bpf.h>
#include "trace.h"
#include "trace_output.h"
@@ -63,6 +65,9 @@ static struct tracer_opt trace_opts[] = {
{ TRACER_OPT(funcgraph-retval, TRACE_GRAPH_PRINT_RETVAL) },
/* Display function return value in hexadecimal format ? */
{ TRACER_OPT(funcgraph-retval-hex, TRACE_GRAPH_PRINT_RETVAL_HEX) },
+#endif
+#ifdef CONFIG_FUNCTION_TRACE_ARGS
+ { TRACER_OPT(funcgraph-args, TRACE_GRAPH_ARGS) },
#endif
/* Include sleep time (scheduled out) between entry and return */
{ TRACER_OPT(sleep-time, TRACE_GRAPH_SLEEP_TIME) },
@@ -653,7 +658,7 @@ print_graph_duration(struct trace_array *tr, unsigned long long duration,
#define __TRACE_GRAPH_PRINT_RETVAL TRACE_GRAPH_PRINT_RETVAL
static void print_graph_retval(struct trace_seq *s, unsigned long retval,
- bool leaf, void *func, bool hex_format)
+ bool hex_format)
{
unsigned long err_code = 0;
@@ -673,28 +678,17 @@ static void print_graph_retval(struct trace_seq *s, unsigned long retval,
err_code = 0;
done:
- if (leaf) {
- if (hex_format || (err_code == 0))
- trace_seq_printf(s, "%ps(); /* = 0x%lx */\n",
- func, retval);
- else
- trace_seq_printf(s, "%ps(); /* = %ld */\n",
- func, err_code);
- } else {
- if (hex_format || (err_code == 0))
- trace_seq_printf(s, "} /* %ps = 0x%lx */\n",
- func, retval);
- else
- trace_seq_printf(s, "} /* %ps = %ld */\n",
- func, err_code);
- }
+ if (hex_format || (err_code == 0))
+ trace_seq_printf(s, " /* = 0x%lx */", retval);
+ else
+ trace_seq_printf(s, " /* = %ld */", err_code);
}
#else
#define __TRACE_GRAPH_PRINT_RETVAL 0
-#define print_graph_retval(_seq, _retval, _leaf, _func, _format) do {} while (0)
+#define print_graph_retval(_seq, _retval, _format) do {} while (0)
#endif
@@ -741,16 +735,20 @@ print_graph_entry_leaf(struct trace_iterator *iter,
/* Function */
for (i = 0; i < call->depth * TRACE_GRAPH_INDENT; i++)
trace_seq_putc(s, ' ');
+ trace_seq_printf(s, "%ps", (void *)graph_ret->func);
+ if (flags & TRACE_GRAPH_ARGS)
+ print_function_args(s, &call->regs, graph_ret->func);
+ else
+ trace_seq_puts(s, "();");
/*
* Write out the function return value if the option function-retval is
* enabled.
*/
if (flags & __TRACE_GRAPH_PRINT_RETVAL)
- print_graph_retval(s, graph_ret->retval, true, (void *)call->func,
- !!(flags & TRACE_GRAPH_PRINT_RETVAL_HEX));
- else
- trace_seq_printf(s, "%ps();\n", (void *)call->func);
+ print_graph_retval(s, graph_ret->retval,
+ !!(flags & TRACE_GRAPH_PRINT_RETVAL_HEX));
+ trace_seq_printf(s, "\n");
print_graph_irq(iter, graph_ret->func, TRACE_GRAPH_RET,
cpu, iter->ent->pid, flags);
@@ -788,7 +786,12 @@ print_graph_entry_nested(struct trace_iterator *iter,
for (i = 0; i < call->depth * TRACE_GRAPH_INDENT; i++)
trace_seq_putc(s, ' ');
- trace_seq_printf(s, "%ps() {\n", (void *)call->func);
+ trace_seq_printf(s, "%ps", (void *)call->func);
+ if (flags & TRACE_GRAPH_ARGS)
+ print_function_args(s, &call->regs, call->func);
+ else
+ trace_seq_puts(s, "()");
+ trace_seq_printf(s, " {\n");
if (trace_seq_has_overflowed(s))
return TRACE_TYPE_PARTIAL_LINE;
@@ -1028,27 +1031,26 @@ print_graph_return(struct ftrace_graph_ret *trace, struct trace_seq *s,
for (i = 0; i < trace->depth * TRACE_GRAPH_INDENT; i++)
trace_seq_putc(s, ' ');
+ /*
+ * If the return function does not have a matching entry,
+ * then the entry was lost. Instead of just printing
+ * the '}' and letting the user guess what function this
+ * belongs to, write out the function name. Always do
+ * that if the funcgraph-tail option is enabled.
+ */
+ if (func_match && !(flags & TRACE_GRAPH_PRINT_TAIL))
+ trace_seq_puts(s, "}");
+ else
+ trace_seq_printf(s, "} /* %ps */", (void *)trace->func);
/*
* Always write out the function name and its return value if the
* function-retval option is enabled.
*/
if (flags & __TRACE_GRAPH_PRINT_RETVAL) {
- print_graph_retval(s, trace->retval, false, (void *)trace->func,
+ print_graph_retval(s, trace->retval,
!!(flags & TRACE_GRAPH_PRINT_RETVAL_HEX));
- } else {
- /*
- * If the return function does not have a matching entry,
- * then the entry was lost. Instead of just printing
- * the '}' and letting the user guess what function this
- * belongs to, write out the function name. Always do
- * that if the funcgraph-tail option is enabled.
- */
- if (func_match && !(flags & TRACE_GRAPH_PRINT_TAIL))
- trace_seq_puts(s, "}\n");
- else
- trace_seq_printf(s, "} /* %ps */\n", (void *)trace->func);
}
-
+ trace_seq_printf(s, "\n");
/* Overrun */
if (flags & TRACE_GRAPH_PRINT_OVERRUN)
trace_seq_printf(s, " (Overruns: %u)\n",
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 6/7] tracing: add support for function argument to graph tracer
2024-09-04 6:59 ` [PATCH 6/7] tracing: add support for function argument to graph tracer Sven Schnelle
@ 2024-09-05 8:05 ` kernel test robot
2024-09-05 8:36 ` kernel test robot
2024-10-04 22:40 ` Steven Rostedt
2 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2024-09-05 8:05 UTC (permalink / raw)
To: Sven Schnelle, Steven Rostedt, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers
Cc: oe-kbuild-all, linux-kernel, linux-trace-kernel, bpf
Hi Sven,
kernel test robot noticed the following build errors:
[auto build test ERROR on s390/features]
[also build test ERROR on tip/x86/core linus/master v6.11-rc6]
[cannot apply to rostedt-trace/for-next rostedt-trace/for-next-urgent next-20240904]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sven-Schnelle/tracing-add-ftrace_regs-to-function_graph_enter/20240904-150232
base: https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
patch link: https://lore.kernel.org/r/20240904065908.1009086-7-svens%40linux.ibm.com
patch subject: [PATCH 6/7] tracing: add support for function argument to graph tracer
config: openrisc-allnoconfig (https://download.01.org/0day-ci/archive/20240905/202409051539.ZmGpuZIP-lkp@intel.com/config)
compiler: or1k-linux-gcc (GCC) 14.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240905/202409051539.ZmGpuZIP-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409051539.ZmGpuZIP-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from include/linux/perf_event.h:52,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from include/linux/syscalls.h:93,
from kernel/time/time.c:33:
>> include/linux/ftrace.h:1013:28: error: field 'regs' has incomplete type
1013 | struct ftrace_regs regs;
| ^~~~
--
In file included from include/linux/perf_event.h:52,
from include/linux/trace_events.h:10,
from include/trace/syscall.h:7,
from include/linux/syscalls.h:93,
from kernel/time/hrtimer.c:30:
>> include/linux/ftrace.h:1013:28: error: field 'regs' has incomplete type
1013 | struct ftrace_regs regs;
| ^~~~
kernel/time/hrtimer.c:121:35: warning: initialized field overwritten [-Woverride-init]
121 | [CLOCK_REALTIME] = HRTIMER_BASE_REALTIME,
| ^~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:121:35: note: (near initialization for 'hrtimer_clock_to_base_table[0]')
kernel/time/hrtimer.c:122:35: warning: initialized field overwritten [-Woverride-init]
122 | [CLOCK_MONOTONIC] = HRTIMER_BASE_MONOTONIC,
| ^~~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:122:35: note: (near initialization for 'hrtimer_clock_to_base_table[1]')
kernel/time/hrtimer.c:123:35: warning: initialized field overwritten [-Woverride-init]
123 | [CLOCK_BOOTTIME] = HRTIMER_BASE_BOOTTIME,
| ^~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:123:35: note: (near initialization for 'hrtimer_clock_to_base_table[7]')
kernel/time/hrtimer.c:124:35: warning: initialized field overwritten [-Woverride-init]
124 | [CLOCK_TAI] = HRTIMER_BASE_TAI,
| ^~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:124:35: note: (near initialization for 'hrtimer_clock_to_base_table[11]')
vim +/regs +1013 include/linux/ftrace.h
1006
1007 /*
1008 * Structure that defines an entry function trace.
1009 * It's already packed but the attribute "packed" is needed
1010 * to remove extra padding at the end.
1011 */
1012 struct ftrace_graph_ent {
> 1013 struct ftrace_regs regs;
1014 unsigned long func; /* Current function */
1015 int depth;
1016 } __packed;
1017
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 6/7] tracing: add support for function argument to graph tracer
2024-09-04 6:59 ` [PATCH 6/7] tracing: add support for function argument to graph tracer Sven Schnelle
2024-09-05 8:05 ` kernel test robot
@ 2024-09-05 8:36 ` kernel test robot
2024-10-04 22:40 ` Steven Rostedt
2 siblings, 0 replies; 28+ messages in thread
From: kernel test robot @ 2024-09-05 8:36 UTC (permalink / raw)
To: Sven Schnelle, Steven Rostedt, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers
Cc: llvm, oe-kbuild-all, linux-kernel, linux-trace-kernel, bpf
Hi Sven,
kernel test robot noticed the following build errors:
[auto build test ERROR on s390/features]
[also build test ERROR on tip/x86/core linus/master v6.11-rc6]
[cannot apply to rostedt-trace/for-next rostedt-trace/for-next-urgent next-20240904]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sven-Schnelle/tracing-add-ftrace_regs-to-function_graph_enter/20240904-150232
base: https://git.kernel.org/pub/scm/linux/kernel/git/s390/linux.git features
patch link: https://lore.kernel.org/r/20240904065908.1009086-7-svens%40linux.ibm.com
patch subject: [PATCH 6/7] tracing: add support for function argument to graph tracer
config: um-allnoconfig (https://download.01.org/0day-ci/archive/20240905/202409051644.nZ4Nj2uc-lkp@intel.com/config)
compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240905/202409051644.nZ4Nj2uc-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202409051644.nZ4Nj2uc-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from kernel/fork.c:34:
In file included from include/linux/mempolicy.h:15:
In file included from include/linux/pagemap.h:11:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from kernel/fork.c:34:
In file included from include/linux/mempolicy.h:15:
In file included from include/linux/pagemap.h:11:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from kernel/fork.c:34:
In file included from include/linux/mempolicy.h:15:
In file included from include/linux/pagemap.h:11:
In file included from include/linux/highmem.h:12:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
693 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
701 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
709 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
718 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
727 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
736 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
In file included from kernel/fork.c:56:
In file included from include/linux/syscalls.h:93:
In file included from include/trace/syscall.h:7:
In file included from include/linux/trace_events.h:10:
In file included from include/linux/perf_event.h:52:
>> include/linux/ftrace.h:1013:21: error: field has incomplete type 'struct ftrace_regs'
1013 | struct ftrace_regs regs;
| ^
include/linux/ftrace.h:41:8: note: forward declaration of 'struct ftrace_regs'
41 | struct ftrace_regs;
| ^
12 warnings and 1 error generated.
--
In file included from kernel/signal.c:27:
In file included from include/linux/tty.h:11:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:40:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from kernel/signal.c:27:
In file included from include/linux/tty.h:11:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:40:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from kernel/signal.c:27:
In file included from include/linux/tty.h:11:
In file included from include/linux/tty_port.h:5:
In file included from include/linux/kfifo.h:40:
In file included from include/linux/dma-mapping.h:11:
In file included from include/linux/scatterlist.h:9:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
693 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
701 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
709 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
718 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
727 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
736 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
In file included from kernel/signal.c:31:
In file included from include/linux/syscalls.h:93:
In file included from include/trace/syscall.h:7:
In file included from include/linux/trace_events.h:10:
In file included from include/linux/perf_event.h:52:
>> include/linux/ftrace.h:1013:21: error: field has incomplete type 'struct ftrace_regs'
1013 | struct ftrace_regs regs;
| ^
include/linux/ftrace.h:41:8: note: forward declaration of 'struct ftrace_regs'
41 | struct ftrace_regs;
| ^
kernel/signal.c:140:37: warning: array index 3 is past the end of the array (that has type 'unsigned long[2]') [-Warray-bounds]
140 | case 4: ready = signal->sig[3] &~ blocked->sig[3];
| ^ ~
arch/x86/include/asm/signal.h:24:2: note: array 'sig' declared here
24 | unsigned long sig[_NSIG_WORDS];
| ^
kernel/signal.c:140:19: warning: array index 3 is past the end of the array (that has type 'unsigned long[2]') [-Warray-bounds]
140 | case 4: ready = signal->sig[3] &~ blocked->sig[3];
| ^ ~
arch/x86/include/asm/signal.h:24:2: note: array 'sig' declared here
24 | unsigned long sig[_NSIG_WORDS];
| ^
kernel/signal.c:141:30: warning: array index 2 is past the end of the array (that has type 'unsigned long[2]') [-Warray-bounds]
141 | ready |= signal->sig[2] &~ blocked->sig[2];
| ^ ~
arch/x86/include/asm/signal.h:24:2: note: array 'sig' declared here
24 | unsigned long sig[_NSIG_WORDS];
| ^
kernel/signal.c:141:12: warning: array index 2 is past the end of the array (that has type 'unsigned long[2]') [-Warray-bounds]
141 | ready |= signal->sig[2] &~ blocked->sig[2];
| ^ ~
arch/x86/include/asm/signal.h:24:2: note: array 'sig' declared here
24 | unsigned long sig[_NSIG_WORDS];
| ^
16 warnings and 1 error generated.
--
In file included from kernel/time/hrtimer.c:30:
In file included from include/linux/syscalls.h:93:
In file included from include/trace/syscall.h:7:
In file included from include/linux/trace_events.h:9:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:548:31: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
548 | val = __raw_readb(PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:561:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
561 | val = __le16_to_cpu((__le16 __force)__raw_readw(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:37:51: note: expanded from macro '__le16_to_cpu'
37 | #define __le16_to_cpu(x) ((__force __u16)(__le16)(x))
| ^
In file included from kernel/time/hrtimer.c:30:
In file included from include/linux/syscalls.h:93:
In file included from include/trace/syscall.h:7:
In file included from include/linux/trace_events.h:9:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:574:61: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
574 | val = __le32_to_cpu((__le32 __force)__raw_readl(PCI_IOBASE + addr));
| ~~~~~~~~~~ ^
include/uapi/linux/byteorder/little_endian.h:35:51: note: expanded from macro '__le32_to_cpu'
35 | #define __le32_to_cpu(x) ((__force __u32)(__le32)(x))
| ^
In file included from kernel/time/hrtimer.c:30:
In file included from include/linux/syscalls.h:93:
In file included from include/trace/syscall.h:7:
In file included from include/linux/trace_events.h:9:
In file included from include/linux/hardirq.h:11:
In file included from arch/um/include/asm/hardirq.h:5:
In file included from include/asm-generic/hardirq.h:17:
In file included from include/linux/irq.h:20:
In file included from include/linux/io.h:14:
In file included from arch/um/include/asm/io.h:24:
include/asm-generic/io.h:585:33: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
585 | __raw_writeb(value, PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:595:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
595 | __raw_writew((u16 __force)cpu_to_le16(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:605:59: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
605 | __raw_writel((u32 __force)cpu_to_le32(value), PCI_IOBASE + addr);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:693:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
693 | readsb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:701:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
701 | readsw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:709:20: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
709 | readsl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:718:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
718 | writesb(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:727:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
727 | writesw(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
include/asm-generic/io.h:736:21: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
736 | writesl(PCI_IOBASE + addr, buffer, count);
| ~~~~~~~~~~ ^
In file included from kernel/time/hrtimer.c:30:
In file included from include/linux/syscalls.h:93:
In file included from include/trace/syscall.h:7:
In file included from include/linux/trace_events.h:10:
In file included from include/linux/perf_event.h:52:
>> include/linux/ftrace.h:1013:21: error: field has incomplete type 'struct ftrace_regs'
1013 | struct ftrace_regs regs;
| ^
include/linux/ftrace.h:41:8: note: forward declaration of 'struct ftrace_regs'
41 | struct ftrace_regs;
| ^
kernel/time/hrtimer.c:121:21: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
121 | [CLOCK_REALTIME] = HRTIMER_BASE_REALTIME,
| ^~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:119:27: note: previous initialization is here
119 | [0 ... MAX_CLOCKS - 1] = HRTIMER_MAX_CLOCK_BASES,
| ^~~~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:122:22: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
122 | [CLOCK_MONOTONIC] = HRTIMER_BASE_MONOTONIC,
| ^~~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:119:27: note: previous initialization is here
119 | [0 ... MAX_CLOCKS - 1] = HRTIMER_MAX_CLOCK_BASES,
| ^~~~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:123:21: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
123 | [CLOCK_BOOTTIME] = HRTIMER_BASE_BOOTTIME,
| ^~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:119:27: note: previous initialization is here
119 | [0 ... MAX_CLOCKS - 1] = HRTIMER_MAX_CLOCK_BASES,
| ^~~~~~~~~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:124:17: warning: initializer overrides prior initialization of this subobject [-Winitializer-overrides]
124 | [CLOCK_TAI] = HRTIMER_BASE_TAI,
| ^~~~~~~~~~~~~~~~
kernel/time/hrtimer.c:119:27: note: previous initialization is here
119 | [0 ... MAX_CLOCKS - 1] = HRTIMER_MAX_CLOCK_BASES,
| ^~~~~~~~~~~~~~~~~~~~~~~
16 warnings and 1 error generated.
vim +1013 include/linux/ftrace.h
1006
1007 /*
1008 * Structure that defines an entry function trace.
1009 * It's already packed but the attribute "packed" is needed
1010 * to remove extra padding at the end.
1011 */
1012 struct ftrace_graph_ent {
> 1013 struct ftrace_regs regs;
1014 unsigned long func; /* Current function */
1015 int depth;
1016 } __packed;
1017
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 6/7] tracing: add support for function argument to graph tracer
2024-09-04 6:59 ` [PATCH 6/7] tracing: add support for function argument to graph tracer Sven Schnelle
2024-09-05 8:05 ` kernel test robot
2024-09-05 8:36 ` kernel test robot
@ 2024-10-04 22:40 ` Steven Rostedt
2024-10-07 6:37 ` Sven Schnelle
2 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2024-10-04 22:40 UTC (permalink / raw)
To: Sven Schnelle
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, bpf
On Wed, 4 Sep 2024 08:59:00 +0200
Sven Schnelle <svens@linux.ibm.com> wrote:
> Wire up the code to print function arguments in the function graph
> tracer. This functionality can be enabled/disabled during compile
> time by setting CONFIG_FUNCTION_TRACE_ARGS and during runtime with
> options/funcgraph-args.
I finally got around to looking at your patches. Do you plan on still
working on them? I really like this feature, and I'm willing to do the work
too if you have other things on your plate.
>
> Example usage:
>
> 6) | dummy_xmit [dummy](skb = 0x8887c100, dev = 0x872ca000) {
> 6) | consume_skb(skb = 0x8887c100) {
> 6) | skb_release_head_state(skb = 0x8887c100) {
> 6) 0.178 us | sock_wfree(skb = 0x8887c100)
> 6) 0.627 us | }
>
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> ---
> include/linux/ftrace.h | 1 +
> kernel/trace/fgraph.c | 6 ++-
> kernel/trace/trace_functions_graph.c | 74 ++++++++++++++--------------
> 3 files changed, 44 insertions(+), 37 deletions(-)
BTW, this is missing:
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 2f8017f8d34d..8a218b39d11d 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -879,6 +879,7 @@ static __always_inline bool ftrace_hash_empty(struct ftrace_hash *hash)
#define TRACE_GRAPH_GRAPH_TIME 0x400
#define TRACE_GRAPH_PRINT_RETVAL 0x800
#define TRACE_GRAPH_PRINT_RETVAL_HEX 0x1000
+#define TRACE_GRAPH_ARGS 0x2000
#define TRACE_GRAPH_PRINT_FILL_SHIFT 28
#define TRACE_GRAPH_PRINT_FILL_MASK (0x3 << TRACE_GRAPH_PRINT_FILL_SHIFT)
that you added in patch 7, but is needed for this patch, where it does not
build without it.
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 56d91041ecd2..5d0ff66f8a70 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -1010,6 +1010,7 @@ static inline void ftrace_init(void) { }
> * to remove extra padding at the end.
> */
> struct ftrace_graph_ent {
> + struct ftrace_regs regs;
> unsigned long func; /* Current function */
> int depth;
> } __packed;
This should have a different event type, to not waste the ring buffer when
not needed.
struct ftrace_graph_ent_args {
struct ftrace_regs_args fargs;
unsigned long func; /* Current function */
int depth;
} __packed;
But also, we need to create a new structure, as nothing should depend on
the size of ftrace_regs (we plan on hiding that completely). I can add a
"struct ftrace_regs_args" that will hold just the args for each arch.
Especially for archs (like x86) where ftrace_regs can be pt_regs in size,
where most of the space is just wasted. Then we can do a:
ftrace_regs_copy_args(fregs, &entry->addr);
And then:
char buf[ftrace_regs_size()];
struct ftrace_regs *fregs = &buf;
ftrace_regs_from_args(fregs, &entry->addr);
to get the arguments.
> diff --git a/kernel/trace/fgraph.c b/kernel/trace/fgraph.c
> index fa62ebfa0711..f4bb10c0aa52 100644
> --- a/kernel/trace/fgraph.c
> +++ b/kernel/trace/fgraph.c
> @@ -614,7 +614,7 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func,
> /* If the caller does not use ftrace, call this function. */
> int function_graph_enter(unsigned long ret, unsigned long func,
> unsigned long frame_pointer, unsigned long *retp,
> - struct ftrace_regs *fregs)
> + struct ftrace_regs *fregs)
> {
> struct ftrace_graph_ent trace;
> unsigned long bitmap = 0;
> @@ -623,6 +623,10 @@ int function_graph_enter(unsigned long ret, unsigned long func,
>
> trace.func = func;
> trace.depth = ++current->curr_ret_depth;
> + if (IS_ENABLED(CONFIG_FUNCTION_TRACE_ARGS) && fregs)
> + trace.regs = *fregs;
> + else
> + memset(&trace.regs, 0, sizeof(struct ftrace_regs));
>
> offset = ftrace_push_return_trace(ret, func, frame_pointer, retp, 0);
> if (offset < 0)
> diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
> index 13d0387ac6a6..be0cee52944a 100644
> --- a/kernel/trace/trace_functions_graph.c
> +++ b/kernel/trace/trace_functions_graph.c
> @@ -12,6 +12,8 @@
> #include <linux/interrupt.h>
> #include <linux/slab.h>
> #include <linux/fs.h>
> +#include <linux/btf.h>
> +#include <linux/bpf.h>
>
> #include "trace.h"
> #include "trace_output.h"
> @@ -63,6 +65,9 @@ static struct tracer_opt trace_opts[] = {
> { TRACER_OPT(funcgraph-retval, TRACE_GRAPH_PRINT_RETVAL) },
> /* Display function return value in hexadecimal format ? */
> { TRACER_OPT(funcgraph-retval-hex, TRACE_GRAPH_PRINT_RETVAL_HEX) },
> +#endif
> +#ifdef CONFIG_FUNCTION_TRACE_ARGS
> + { TRACER_OPT(funcgraph-args, TRACE_GRAPH_ARGS) },
> #endif
> /* Include sleep time (scheduled out) between entry and return */
> { TRACER_OPT(sleep-time, TRACE_GRAPH_SLEEP_TIME) },
> @@ -653,7 +658,7 @@ print_graph_duration(struct trace_array *tr, unsigned long long duration,
> #define __TRACE_GRAPH_PRINT_RETVAL TRACE_GRAPH_PRINT_RETVAL
>
> static void print_graph_retval(struct trace_seq *s, unsigned long retval,
> - bool leaf, void *func, bool hex_format)
> + bool hex_format)
> {
> unsigned long err_code = 0;
>
> @@ -673,28 +678,17 @@ static void print_graph_retval(struct trace_seq *s, unsigned long retval,
> err_code = 0;
>
> done:
> - if (leaf) {
> - if (hex_format || (err_code == 0))
> - trace_seq_printf(s, "%ps(); /* = 0x%lx */\n",
> - func, retval);
> - else
> - trace_seq_printf(s, "%ps(); /* = %ld */\n",
> - func, err_code);
> - } else {
> - if (hex_format || (err_code == 0))
> - trace_seq_printf(s, "} /* %ps = 0x%lx */\n",
> - func, retval);
> - else
> - trace_seq_printf(s, "} /* %ps = %ld */\n",
> - func, err_code);
> - }
> + if (hex_format || (err_code == 0))
> + trace_seq_printf(s, " /* = 0x%lx */", retval);
> + else
> + trace_seq_printf(s, " /* = %ld */", err_code);
> }
>
> #else
>
> #define __TRACE_GRAPH_PRINT_RETVAL 0
>
> -#define print_graph_retval(_seq, _retval, _leaf, _func, _format) do {} while (0)
> +#define print_graph_retval(_seq, _retval, _format) do {} while (0)
>
> #endif
>
> @@ -741,16 +735,20 @@ print_graph_entry_leaf(struct trace_iterator *iter,
> /* Function */
> for (i = 0; i < call->depth * TRACE_GRAPH_INDENT; i++)
> trace_seq_putc(s, ' ');
> + trace_seq_printf(s, "%ps", (void *)graph_ret->func);
> + if (flags & TRACE_GRAPH_ARGS)
> + print_function_args(s, &call->regs, graph_ret->func);
Ideally, the flag is going to be set when args is recorded and not used for
printing. If the event is the ftrace_ent_args() this will print the
arguments, otherwise it does not.
To simplify these functions, we probably need to have a:
union fgraph_entry {
struct ftrace_graph_ent *normal;
struct ftrace_graph_ent_args *args;
};
And switch depending which is which (the header of both is the same as is
for all entries).
-- Steve
> + else
> + trace_seq_puts(s, "();");
>
> /*
> * Write out the function return value if the option function-retval is
> * enabled.
> */
> if (flags & __TRACE_GRAPH_PRINT_RETVAL)
> - print_graph_retval(s, graph_ret->retval, true, (void *)call->func,
> - !!(flags & TRACE_GRAPH_PRINT_RETVAL_HEX));
> - else
> - trace_seq_printf(s, "%ps();\n", (void *)call->func);
> + print_graph_retval(s, graph_ret->retval,
> + !!(flags & TRACE_GRAPH_PRINT_RETVAL_HEX));
> + trace_seq_printf(s, "\n");
>
> print_graph_irq(iter, graph_ret->func, TRACE_GRAPH_RET,
> cpu, iter->ent->pid, flags);
> @@ -788,7 +786,12 @@ print_graph_entry_nested(struct trace_iterator *iter,
> for (i = 0; i < call->depth * TRACE_GRAPH_INDENT; i++)
> trace_seq_putc(s, ' ');
>
> - trace_seq_printf(s, "%ps() {\n", (void *)call->func);
> + trace_seq_printf(s, "%ps", (void *)call->func);
> + if (flags & TRACE_GRAPH_ARGS)
> + print_function_args(s, &call->regs, call->func);
> + else
> + trace_seq_puts(s, "()");
> + trace_seq_printf(s, " {\n");
>
> if (trace_seq_has_overflowed(s))
> return TRACE_TYPE_PARTIAL_LINE;
> @@ -1028,27 +1031,26 @@ print_graph_return(struct ftrace_graph_ret *trace, struct trace_seq *s,
> for (i = 0; i < trace->depth * TRACE_GRAPH_INDENT; i++)
> trace_seq_putc(s, ' ');
>
> + /*
> + * If the return function does not have a matching entry,
> + * then the entry was lost. Instead of just printing
> + * the '}' and letting the user guess what function this
> + * belongs to, write out the function name. Always do
> + * that if the funcgraph-tail option is enabled.
> + */
> + if (func_match && !(flags & TRACE_GRAPH_PRINT_TAIL))
> + trace_seq_puts(s, "}");
> + else
> + trace_seq_printf(s, "} /* %ps */", (void *)trace->func);
> /*
> * Always write out the function name and its return value if the
> * function-retval option is enabled.
> */
> if (flags & __TRACE_GRAPH_PRINT_RETVAL) {
> - print_graph_retval(s, trace->retval, false, (void *)trace->func,
> + print_graph_retval(s, trace->retval,
> !!(flags & TRACE_GRAPH_PRINT_RETVAL_HEX));
> - } else {
> - /*
> - * If the return function does not have a matching entry,
> - * then the entry was lost. Instead of just printing
> - * the '}' and letting the user guess what function this
> - * belongs to, write out the function name. Always do
> - * that if the funcgraph-tail option is enabled.
> - */
> - if (func_match && !(flags & TRACE_GRAPH_PRINT_TAIL))
> - trace_seq_puts(s, "}\n");
> - else
> - trace_seq_printf(s, "} /* %ps */\n", (void *)trace->func);
> }
> -
> + trace_seq_printf(s, "\n");
> /* Overrun */
> if (flags & TRACE_GRAPH_PRINT_OVERRUN)
> trace_seq_printf(s, " (Overruns: %u)\n",
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 6/7] tracing: add support for function argument to graph tracer
2024-10-04 22:40 ` Steven Rostedt
@ 2024-10-07 6:37 ` Sven Schnelle
0 siblings, 0 replies; 28+ messages in thread
From: Sven Schnelle @ 2024-10-07 6:37 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, linux-kernel,
linux-trace-kernel, bpf
Steven Rostedt <rostedt@goodmis.org> writes:
> On Wed, 4 Sep 2024 08:59:00 +0200
> Sven Schnelle <svens@linux.ibm.com> wrote:
>
>> Wire up the code to print function arguments in the function graph
>> tracer. This functionality can be enabled/disabled during compile
>> time by setting CONFIG_FUNCTION_TRACE_ARGS and during runtime with
>> options/funcgraph-args.
>
> I finally got around to looking at your patches. Do you plan on still
> working on them? I really like this feature, and I'm willing to do the work
> too if you have other things on your plate.
Yes, working on other things, so feel free to take over.
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 7/7] tracing: add arguments to function tracer
2024-09-04 6:58 [PATCH 0/7] add function arguments to ftrace Sven Schnelle
` (5 preceding siblings ...)
2024-09-04 6:59 ` [PATCH 6/7] tracing: add support for function argument to graph tracer Sven Schnelle
@ 2024-09-04 6:59 ` Sven Schnelle
2024-09-06 3:49 ` Zheng Yejian
2024-09-05 15:16 ` [PATCH 0/7] add function arguments to ftrace Steven Rostedt
7 siblings, 1 reply; 28+ messages in thread
From: Sven Schnelle @ 2024-09-04 6:59 UTC (permalink / raw)
To: Steven Rostedt, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel
Wire up the code to print function arguments in the function tracer.
This functionality can be enabled/disabled during compile time by
setting CONFIG_FUNCTION_TRACE_ARGS and during runtime with
options/func-args.
ping-689 [004] b.... 77.170220: dummy_xmit(skb = 0x82904800, dev = 0x882d0000) <-dev_hard_start_xmit
Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
---
kernel/trace/trace.c | 8 +++++-
kernel/trace/trace.h | 4 ++-
kernel/trace/trace_entries.h | 7 +++--
kernel/trace/trace_functions.c | 46 +++++++++++++++++++++++++++----
kernel/trace/trace_irqsoff.c | 4 +--
kernel/trace/trace_output.c | 14 ++++++++--
kernel/trace/trace_sched_wakeup.c | 4 +--
7 files changed, 71 insertions(+), 16 deletions(-)
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index ebe7ce2f5f4a..8118e4c8c649 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2864,7 +2864,7 @@ trace_buffer_unlock_commit_nostack(struct trace_buffer *buffer,
void
trace_function(struct trace_array *tr, unsigned long ip, unsigned long
- parent_ip, unsigned int trace_ctx)
+ parent_ip, unsigned int trace_ctx, struct ftrace_regs *regs)
{
struct trace_event_call *call = &event_function;
struct trace_buffer *buffer = tr->array_buffer.buffer;
@@ -2878,6 +2878,12 @@ trace_function(struct trace_array *tr, unsigned long ip, unsigned long
entry = ring_buffer_event_data(event);
entry->ip = ip;
entry->parent_ip = parent_ip;
+#ifdef CONFIG_FUNCTION_TRACE_ARGS
+ if (regs)
+ entry->regs = *regs;
+ else
+ memset(&entry->regs, 0, sizeof(struct ftrace_regs));
+#endif
if (!call_filter_check_discard(call, entry, buffer, event)) {
if (static_branch_unlikely(&trace_function_exports_enabled))
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index bd3e3069300e..13cf6f97f7e0 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -673,7 +673,8 @@ unsigned long trace_total_entries(struct trace_array *tr);
void trace_function(struct trace_array *tr,
unsigned long ip,
unsigned long parent_ip,
- unsigned int trace_ctx);
+ unsigned int trace_ctx,
+ struct ftrace_regs *regs);
void trace_graph_function(struct trace_array *tr,
unsigned long ip,
unsigned long parent_ip,
@@ -870,6 +871,7 @@ static __always_inline bool ftrace_hash_empty(struct ftrace_hash *hash)
#define TRACE_GRAPH_GRAPH_TIME 0x400
#define TRACE_GRAPH_PRINT_RETVAL 0x800
#define TRACE_GRAPH_PRINT_RETVAL_HEX 0x1000
+#define TRACE_GRAPH_ARGS 0x2000
#define TRACE_GRAPH_PRINT_FILL_SHIFT 28
#define TRACE_GRAPH_PRINT_FILL_MASK (0x3 << TRACE_GRAPH_PRINT_FILL_SHIFT)
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 )
+#endif
),
F_printk(" %ps <-- %ps",
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);
+static void
function_stack_trace_call(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *op, struct ftrace_regs *fregs);
static void
@@ -42,9 +45,10 @@ enum {
TRACE_FUNC_NO_OPTS = 0x0, /* No flags set. */
TRACE_FUNC_OPT_STACK = 0x1,
TRACE_FUNC_OPT_NO_REPEATS = 0x2,
+ TRACE_FUNC_OPT_ARGS = 0x4,
/* Update this to next highest bit. */
- TRACE_FUNC_OPT_HIGHEST_BIT = 0x4
+ TRACE_FUNC_OPT_HIGHEST_BIT = 0x8
};
#define TRACE_FUNC_OPT_MASK (TRACE_FUNC_OPT_HIGHEST_BIT - 1)
@@ -114,6 +118,8 @@ static ftrace_func_t select_trace_function(u32 flags_val)
switch (flags_val & TRACE_FUNC_OPT_MASK) {
case TRACE_FUNC_NO_OPTS:
return function_trace_call;
+ case TRACE_FUNC_OPT_ARGS:
+ return function_args_trace_call;
case TRACE_FUNC_OPT_STACK:
return function_stack_trace_call;
case TRACE_FUNC_OPT_NO_REPEATS:
@@ -198,7 +204,34 @@ function_trace_call(unsigned long ip, unsigned long parent_ip,
cpu = smp_processor_id();
data = per_cpu_ptr(tr->array_buffer.data, cpu);
if (!atomic_read(&data->disabled))
- trace_function(tr, ip, parent_ip, trace_ctx);
+ trace_function(tr, ip, parent_ip, trace_ctx, NULL);
+
+ ftrace_test_recursion_unlock(bit);
+}
+
+static void
+function_args_trace_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op, struct ftrace_regs *fregs)
+{
+ struct trace_array *tr = op->private;
+ struct trace_array_cpu *data;
+ unsigned int trace_ctx;
+ int bit;
+ int cpu;
+
+ if (unlikely(!tr->function_enabled))
+ return;
+
+ bit = ftrace_test_recursion_trylock(ip, parent_ip);
+ if (bit < 0)
+ return;
+
+ trace_ctx = tracing_gen_ctx();
+
+ cpu = smp_processor_id();
+ data = per_cpu_ptr(tr->array_buffer.data, cpu);
+ if (!atomic_read(&data->disabled))
+ trace_function(tr, ip, parent_ip, trace_ctx, fregs);
ftrace_test_recursion_unlock(bit);
}
@@ -247,7 +280,7 @@ function_stack_trace_call(unsigned long ip, unsigned long parent_ip,
if (likely(disabled == 1)) {
trace_ctx = tracing_gen_ctx_flags(flags);
- trace_function(tr, ip, parent_ip, trace_ctx);
+ trace_function(tr, ip, parent_ip, trace_ctx, NULL);
#ifdef CONFIG_UNWINDER_FRAME_POINTER
if (ftrace_pids_enabled(op))
skip++;
@@ -329,7 +362,7 @@ function_no_repeats_trace_call(unsigned long ip, unsigned long parent_ip,
trace_ctx = tracing_gen_ctx_flags(flags);
process_repeats(tr, ip, parent_ip, last_info, trace_ctx);
- trace_function(tr, ip, parent_ip, trace_ctx);
+ trace_function(tr, ip, parent_ip, trace_ctx, NULL);
out:
ftrace_test_recursion_unlock(bit);
@@ -368,7 +401,7 @@ function_stack_no_repeats_trace_call(unsigned long ip, unsigned long parent_ip,
trace_ctx = tracing_gen_ctx_flags(flags);
process_repeats(tr, ip, parent_ip, last_info, trace_ctx);
- trace_function(tr, ip, parent_ip, trace_ctx);
+ trace_function(tr, ip, parent_ip, trace_ctx, NULL);
__trace_stack(tr, trace_ctx, STACK_SKIP);
}
@@ -382,6 +415,9 @@ static struct tracer_opt func_opts[] = {
{ TRACER_OPT(func_stack_trace, TRACE_FUNC_OPT_STACK) },
#endif
{ TRACER_OPT(func-no-repeats, TRACE_FUNC_OPT_NO_REPEATS) },
+#ifdef CONFIG_FUNCTION_TRACE_ARGS
+ { TRACER_OPT(func-args, TRACE_FUNC_OPT_ARGS) },
+#endif
{ } /* Always set a last empty entry */
};
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index fce064e20570..096002c99d70 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -150,7 +150,7 @@ irqsoff_tracer_call(unsigned long ip, unsigned long parent_ip,
trace_ctx = tracing_gen_ctx_flags(flags);
- trace_function(tr, ip, parent_ip, trace_ctx);
+ trace_function(tr, ip, parent_ip, trace_ctx, fregs);
atomic_dec(&data->disabled);
}
@@ -278,7 +278,7 @@ __trace_function(struct trace_array *tr,
if (is_graph(tr))
trace_graph_function(tr, ip, parent_ip, trace_ctx);
else
- trace_function(tr, ip, parent_ip, trace_ctx);
+ trace_function(tr, ip, parent_ip, trace_ctx, NULL);
}
#else
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index 70405c4cceb6..8fdee7b9cdba 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -1058,9 +1058,13 @@ enum print_line_t trace_nop_print(struct trace_iterator *iter, int flags,
}
static void print_fn_trace(struct trace_seq *s, unsigned long ip,
- unsigned long parent_ip, int flags)
+ unsigned long parent_ip,
+ struct ftrace_regs *fregs,
+ int flags)
{
seq_print_ip_sym(s, ip, flags);
+ if (fregs)
+ print_function_args(s, fregs, ip);
if ((flags & TRACE_ITER_PRINT_PARENT) && parent_ip) {
trace_seq_puts(s, " <-");
@@ -1074,10 +1078,14 @@ static enum print_line_t trace_fn_trace(struct trace_iterator *iter, int flags,
{
struct ftrace_entry *field;
struct trace_seq *s = &iter->seq;
+ struct ftrace_regs *fregs = NULL;
trace_assign_type(field, iter->ent);
- print_fn_trace(s, field->ip, field->parent_ip, flags);
+#if IS_ENABLED(CONFIG_FUNCTION_TRACE_ARGS)
+ fregs = &field->regs;
+#endif
+ print_fn_trace(s, field->ip, field->parent_ip, fregs, flags);
trace_seq_putc(s, '\n');
return trace_handle_return(s);
@@ -1742,7 +1750,7 @@ trace_func_repeats_print(struct trace_iterator *iter, int flags,
trace_assign_type(field, iter->ent);
- print_fn_trace(s, field->ip, field->parent_ip, flags);
+ print_fn_trace(s, field->ip, field->parent_ip, NULL, flags);
trace_seq_printf(s, " (repeats: %u, last_ts:", field->count);
trace_print_time(s, iter,
iter->ts - FUNC_REPEATS_GET_DELTA_TS(field));
diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
index 130ca7e7787e..39043c955761 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -224,7 +224,7 @@ wakeup_tracer_call(unsigned long ip, unsigned long parent_ip,
return;
local_irq_save(flags);
- trace_function(tr, ip, parent_ip, trace_ctx);
+ trace_function(tr, ip, parent_ip, trace_ctx, fregs);
local_irq_restore(flags);
atomic_dec(&data->disabled);
@@ -309,7 +309,7 @@ __trace_function(struct trace_array *tr,
if (is_graph(tr))
trace_graph_function(tr, ip, parent_ip, trace_ctx);
else
- trace_function(tr, ip, parent_ip, trace_ctx);
+ trace_function(tr, ip, parent_ip, trace_ctx, NULL);
}
static int wakeup_flag_changed(struct trace_array *tr, u32 mask, int set)
--
2.43.0
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH 7/7] tracing: add arguments to function tracer
2024-09-04 6:59 ` [PATCH 7/7] tracing: add arguments to function tracer Sven Schnelle
@ 2024-09-06 3:49 ` Zheng Yejian
2024-10-04 22:43 ` Steven Rostedt
0 siblings, 1 reply; 28+ messages in thread
From: Zheng Yejian @ 2024-09-06 3:49 UTC (permalink / raw)
To: Sven Schnelle, Steven Rostedt, Masami Hiramatsu, Mark Rutland,
Mathieu Desnoyers
Cc: linux-kernel, linux-trace-kernel
On 2024/9/4 14:59, Sven Schnelle wrote:
> Wire up the code to print function arguments in the function tracer.
> This functionality can be enabled/disabled during compile time by
> setting CONFIG_FUNCTION_TRACE_ARGS and during runtime with
> options/func-args.
>
> ping-689 [004] b.... 77.170220: dummy_xmit(skb = 0x82904800, dev = 0x882d0000) <-dev_hard_start_xmit
>
> Signed-off-by: Sven Schnelle <svens@linux.ibm.com>
> ---
> kernel/trace/trace.c | 8 +++++-
> kernel/trace/trace.h | 4 ++-
> kernel/trace/trace_entries.h | 7 +++--
> kernel/trace/trace_functions.c | 46 +++++++++++++++++++++++++++----
> kernel/trace/trace_irqsoff.c | 4 +--
> kernel/trace/trace_output.c | 14 ++++++++--
> kernel/trace/trace_sched_wakeup.c | 4 +--
> 7 files changed, 71 insertions(+), 16 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index ebe7ce2f5f4a..8118e4c8c649 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2864,7 +2864,7 @@ trace_buffer_unlock_commit_nostack(struct trace_buffer *buffer,
>
> void
> trace_function(struct trace_array *tr, unsigned long ip, unsigned long
> - parent_ip, unsigned int trace_ctx)
> + parent_ip, unsigned int trace_ctx, struct ftrace_regs *regs)
> {
> struct trace_event_call *call = &event_function;
> struct trace_buffer *buffer = tr->array_buffer.buffer;
> @@ -2878,6 +2878,12 @@ trace_function(struct trace_array *tr, unsigned long ip, unsigned long
> entry = ring_buffer_event_data(event);
> entry->ip = ip;
> entry->parent_ip = parent_ip;
> +#ifdef CONFIG_FUNCTION_TRACE_ARGS
> + if (regs)
> + entry->regs = *regs;
> + else
> + memset(&entry->regs, 0, sizeof(struct ftrace_regs));
> +#endif
>
> if (!call_filter_check_discard(call, entry, buffer, event)) {
> if (static_branch_unlikely(&trace_function_exports_enabled))
> diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
> index bd3e3069300e..13cf6f97f7e0 100644
> --- a/kernel/trace/trace.h
> +++ b/kernel/trace/trace.h
> @@ -673,7 +673,8 @@ unsigned long trace_total_entries(struct trace_array *tr);
> void trace_function(struct trace_array *tr,
> unsigned long ip,
> unsigned long parent_ip,
> - unsigned int trace_ctx);
> + unsigned int trace_ctx,
> + struct ftrace_regs *regs);
> void trace_graph_function(struct trace_array *tr,
> unsigned long ip,
> unsigned long parent_ip,
> @@ -870,6 +871,7 @@ static __always_inline bool ftrace_hash_empty(struct ftrace_hash *hash)
> #define TRACE_GRAPH_GRAPH_TIME 0x400
> #define TRACE_GRAPH_PRINT_RETVAL 0x800
> #define TRACE_GRAPH_PRINT_RETVAL_HEX 0x1000
> +#define TRACE_GRAPH_ARGS 0x2000
> #define TRACE_GRAPH_PRINT_FILL_SHIFT 28
> #define TRACE_GRAPH_PRINT_FILL_MASK (0x3 << TRACE_GRAPH_PRINT_FILL_SHIFT)
>
> 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..
> +#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.
> 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);
> +static void
> function_stack_trace_call(unsigned long ip, unsigned long parent_ip,
> struct ftrace_ops *op, struct ftrace_regs *fregs);
> static void
> @@ -42,9 +45,10 @@ enum {
> TRACE_FUNC_NO_OPTS = 0x0, /* No flags set. */
> TRACE_FUNC_OPT_STACK = 0x1,
> TRACE_FUNC_OPT_NO_REPEATS = 0x2,
> + TRACE_FUNC_OPT_ARGS = 0x4,
>
> /* Update this to next highest bit. */
> - TRACE_FUNC_OPT_HIGHEST_BIT = 0x4
> + TRACE_FUNC_OPT_HIGHEST_BIT = 0x8
> };
>
> #define TRACE_FUNC_OPT_MASK (TRACE_FUNC_OPT_HIGHEST_BIT - 1)
> @@ -114,6 +118,8 @@ static ftrace_func_t select_trace_function(u32 flags_val)
> switch (flags_val & TRACE_FUNC_OPT_MASK) {
> case TRACE_FUNC_NO_OPTS:
> return function_trace_call;
> + case TRACE_FUNC_OPT_ARGS:
> + return function_args_trace_call;
> case TRACE_FUNC_OPT_STACK:
> return function_stack_trace_call;
> case TRACE_FUNC_OPT_NO_REPEATS:
> @@ -198,7 +204,34 @@ function_trace_call(unsigned long ip, unsigned long parent_ip,
> cpu = smp_processor_id();
> data = per_cpu_ptr(tr->array_buffer.data, cpu);
> if (!atomic_read(&data->disabled))
> - trace_function(tr, ip, parent_ip, trace_ctx);
> + trace_function(tr, ip, parent_ip, trace_ctx, NULL);
> +
> + ftrace_test_recursion_unlock(bit);
> +}
> +
> +static void
> +function_args_trace_call(unsigned long ip, unsigned long parent_ip,
> + struct ftrace_ops *op, struct ftrace_regs *fregs)
> +{
> + struct trace_array *tr = op->private;
> + struct trace_array_cpu *data;
> + unsigned int trace_ctx;
> + int bit;
> + int cpu;
> +
> + if (unlikely(!tr->function_enabled))
> + return;
> +
> + bit = ftrace_test_recursion_trylock(ip, parent_ip);
> + if (bit < 0)
> + return;
> +
> + trace_ctx = tracing_gen_ctx();
> +
> + cpu = smp_processor_id();
> + data = per_cpu_ptr(tr->array_buffer.data, cpu);
> + if (!atomic_read(&data->disabled))
> + trace_function(tr, ip, parent_ip, trace_ctx, fregs);
>
> ftrace_test_recursion_unlock(bit);
> }
> @@ -247,7 +280,7 @@ function_stack_trace_call(unsigned long ip, unsigned long parent_ip,
>
> if (likely(disabled == 1)) {
> trace_ctx = tracing_gen_ctx_flags(flags);
> - trace_function(tr, ip, parent_ip, trace_ctx);
> + trace_function(tr, ip, parent_ip, trace_ctx, NULL);
> #ifdef CONFIG_UNWINDER_FRAME_POINTER
> if (ftrace_pids_enabled(op))
> skip++;
> @@ -329,7 +362,7 @@ function_no_repeats_trace_call(unsigned long ip, unsigned long parent_ip,
> trace_ctx = tracing_gen_ctx_flags(flags);
> process_repeats(tr, ip, parent_ip, last_info, trace_ctx);
>
> - trace_function(tr, ip, parent_ip, trace_ctx);
> + trace_function(tr, ip, parent_ip, trace_ctx, NULL);
>
> out:
> ftrace_test_recursion_unlock(bit);
> @@ -368,7 +401,7 @@ function_stack_no_repeats_trace_call(unsigned long ip, unsigned long parent_ip,
> trace_ctx = tracing_gen_ctx_flags(flags);
> process_repeats(tr, ip, parent_ip, last_info, trace_ctx);
>
> - trace_function(tr, ip, parent_ip, trace_ctx);
> + trace_function(tr, ip, parent_ip, trace_ctx, NULL);
> __trace_stack(tr, trace_ctx, STACK_SKIP);
> }
>
> @@ -382,6 +415,9 @@ static struct tracer_opt func_opts[] = {
> { TRACER_OPT(func_stack_trace, TRACE_FUNC_OPT_STACK) },
> #endif
> { TRACER_OPT(func-no-repeats, TRACE_FUNC_OPT_NO_REPEATS) },
> +#ifdef CONFIG_FUNCTION_TRACE_ARGS
> + { TRACER_OPT(func-args, TRACE_FUNC_OPT_ARGS) },
> +#endif
> { } /* Always set a last empty entry */
> };
>
> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
> index fce064e20570..096002c99d70 100644
> --- a/kernel/trace/trace_irqsoff.c
> +++ b/kernel/trace/trace_irqsoff.c
> @@ -150,7 +150,7 @@ irqsoff_tracer_call(unsigned long ip, unsigned long parent_ip,
>
> trace_ctx = tracing_gen_ctx_flags(flags);
>
> - trace_function(tr, ip, parent_ip, trace_ctx);
> + trace_function(tr, ip, parent_ip, trace_ctx, fregs);
>
> atomic_dec(&data->disabled);
> }
> @@ -278,7 +278,7 @@ __trace_function(struct trace_array *tr,
> if (is_graph(tr))
> trace_graph_function(tr, ip, parent_ip, trace_ctx);
> else
> - trace_function(tr, ip, parent_ip, trace_ctx);
> + trace_function(tr, ip, parent_ip, trace_ctx, NULL);
> }
>
> #else
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index 70405c4cceb6..8fdee7b9cdba 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -1058,9 +1058,13 @@ enum print_line_t trace_nop_print(struct trace_iterator *iter, int flags,
> }
>
> static void print_fn_trace(struct trace_seq *s, unsigned long ip,
> - unsigned long parent_ip, int flags)
> + unsigned long parent_ip,
> + struct ftrace_regs *fregs,
> + int flags)
> {
> seq_print_ip_sym(s, ip, flags);
> + if (fregs)
> + print_function_args(s, fregs, ip);
>
> if ((flags & TRACE_ITER_PRINT_PARENT) && parent_ip) {
> trace_seq_puts(s, " <-");
> @@ -1074,10 +1078,14 @@ static enum print_line_t trace_fn_trace(struct trace_iterator *iter, int flags,
> {
> struct ftrace_entry *field;
> struct trace_seq *s = &iter->seq;
> + struct ftrace_regs *fregs = NULL;
>
> trace_assign_type(field, iter->ent);
>
> - print_fn_trace(s, field->ip, field->parent_ip, flags);
> +#if IS_ENABLED(CONFIG_FUNCTION_TRACE_ARGS)
> + fregs = &field->regs;
> +#endif
> + print_fn_trace(s, field->ip, field->parent_ip, fregs, flags);
> trace_seq_putc(s, '\n');
>
> return trace_handle_return(s);
> @@ -1742,7 +1750,7 @@ trace_func_repeats_print(struct trace_iterator *iter, int flags,
>
> trace_assign_type(field, iter->ent);
>
> - print_fn_trace(s, field->ip, field->parent_ip, flags);
> + print_fn_trace(s, field->ip, field->parent_ip, NULL, flags);
> trace_seq_printf(s, " (repeats: %u, last_ts:", field->count);
> trace_print_time(s, iter,
> iter->ts - FUNC_REPEATS_GET_DELTA_TS(field));
> diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
> index 130ca7e7787e..39043c955761 100644
> --- a/kernel/trace/trace_sched_wakeup.c
> +++ b/kernel/trace/trace_sched_wakeup.c
> @@ -224,7 +224,7 @@ wakeup_tracer_call(unsigned long ip, unsigned long parent_ip,
> return;
>
> local_irq_save(flags);
> - trace_function(tr, ip, parent_ip, trace_ctx);
> + trace_function(tr, ip, parent_ip, trace_ctx, fregs);
> local_irq_restore(flags);
>
> atomic_dec(&data->disabled);
> @@ -309,7 +309,7 @@ __trace_function(struct trace_array *tr,
> if (is_graph(tr))
> trace_graph_function(tr, ip, parent_ip, trace_ctx);
> else
> - trace_function(tr, ip, parent_ip, trace_ctx);
> + trace_function(tr, ip, parent_ip, trace_ctx, NULL);
> }
>
> static int wakeup_flag_changed(struct trace_array *tr, u32 mask, int set)
--
Thanks,
Zheng Yejian
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 7/7] tracing: add arguments to function tracer
2024-09-06 3:49 ` Zheng Yejian
@ 2024-10-04 22:43 ` Steven Rostedt
0 siblings, 0 replies; 28+ messages in thread
From: Steven Rostedt @ 2024-10-04 22:43 UTC (permalink / raw)
To: Zheng Yejian
Cc: Sven Schnelle, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
linux-kernel, linux-trace-kernel
On Fri, 6 Sep 2024 11:49:10 +0800
Zheng Yejian <zhengyejian@huaweicloud.com> wrote:
> > 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".
>
> > +#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
>
> > 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);
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/7] add function arguments to ftrace
2024-09-04 6:58 [PATCH 0/7] add function arguments to ftrace Sven Schnelle
` (6 preceding siblings ...)
2024-09-04 6:59 ` [PATCH 7/7] tracing: add arguments to function tracer Sven Schnelle
@ 2024-09-05 15:16 ` Steven Rostedt
2024-09-06 6:18 ` Sven Schnelle
7 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2024-09-05 15:16 UTC (permalink / raw)
To: Sven Schnelle
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Guo Ren, linux-kernel,
linux-trace-kernel, linux-riscv, linux-csky
On Wed, 4 Sep 2024 08:58:54 +0200
Sven Schnelle <svens@linux.ibm.com> wrote:
> These patches add support for printing function arguments in ftrace.
>
> Example usage:
>
> function tracer:
>
> cd /sys/kernel/tracing/
> echo icmp_rcv >set_ftrace_filter
> echo function >current_tracer
> echo 1 >options/func-args
> ping -c 10 127.0.0.1
> [..]
> cat trace
> [..]
> ping-1277 [030] ..s1. 39.120939: icmp_rcv(skb = 0xa0ecab00) <-ip_protocol_deliver_rcu
> ping-1277 [030] ..s1. 39.120946: icmp_rcv(skb = 0xa0ecac00) <-ip_protocol_deliver_rcu
> ping-1277 [030] ..s1. 40.179724: icmp_rcv(skb = 0xa0ecab00) <-ip_protocol_deliver_rcu
> ping-1277 [030] ..s1. 40.179730: icmp_rcv(skb = 0xa0ecac00) <-ip_protocol_deliver_rcu
> ping-1277 [030] ..s1. 41.219700: icmp_rcv(skb = 0xa0ecab00) <-ip_protocol_deliver_rcu
> ping-1277 [030] ..s1. 41.219706: icmp_rcv(skb = 0xa0ecac00) <-ip_protocol_deliver_rcu
> ping-1277 [030] ..s1. 42.259717: icmp_rcv(skb = 0xa0ecab00) <-ip_protocol_deliver_rcu
> ping-1277 [030] ..s1. 42.259725: icmp_rcv(skb = 0xa0ecac00) <-ip_protocol_deliver_rcu
> ping-1277 [030] ..s1. 43.299735: icmp_rcv(skb = 0xa0ecab00) <-ip_protocol_deliver_rcu
> ping-1277 [030] ..s1. 43.299742: icmp_rcv(skb = 0xa0ecac00) <-ip_protocol_deliver_rcu
>
> function graph:
>
> cd /sys/kernel/tracing
> echo icmp_rcv >set_graph_function
> echo function_graph >current_tracer
> echo 1 >options/funcgraph-args
>
> ping -c 1 127.0.0.1
>
> cat trace
>
> 30) | icmp_rcv(skb = 0xa0ecab00) {
> 30) | __skb_checksum_complete(skb = 0xa0ecab00) {
> 30) | skb_checksum(skb = 0xa0ecab00, offset = 0, len = 64, csum = 0) {
> 30) | __skb_checksum(skb = 0xa0ecab00, offset = 0, len = 64, csum = 0, ops = 0x232e0327a88) {
> 30) 0.418 us | csum_partial(buff = 0xa0d20924, len = 64, sum = 0)
> 30) 0.985 us | }
> 30) 1.463 us | }
> 30) 2.039 us | }
> [..]
>
First I want to say THANK YOU!!!!
This has been on my TODO list for far too long. I never got the time to
work on it :-p
Anyway, this is something I definitely want added. But I'm going to guess
that there is going to be issues with it and I doubt it will be ready for
the next merge window. I'm currently focused on some other things and also
have to get ready for next weeks travels (I'll be in Prague for GNU Cauldron,
then Vienna for Plumbers and OSS EU, then to Paris for Kernel Recipes!).
But I most definitely want this in. Hopefully by 6.13. This may be
something I can review on the plane (if I get my slides done).
Again, thanks for doing this!
-- Steve
>
> Sven Schnelle (7):
> tracing: add ftrace_regs to function_graph_enter()
> x86/tracing: pass ftrace_regs to function_graph_enter()
> s390/tracing: pass ftrace_regs to function_graph_enter()
> Add print_function_args()
> tracing: add config option for print arguments in ftrace
> tracing: add support for function argument to graph tracer
> tracing: add arguments to function tracer
>
> arch/arm/kernel/ftrace.c | 2 +-
> arch/arm64/kernel/ftrace.c | 2 +-
> arch/csky/kernel/ftrace.c | 2 +-
> arch/loongarch/kernel/ftrace.c | 2 +-
> arch/loongarch/kernel/ftrace_dyn.c | 2 +-
> arch/microblaze/kernel/ftrace.c | 2 +-
> arch/mips/kernel/ftrace.c | 2 +-
> arch/parisc/kernel/ftrace.c | 2 +-
> arch/powerpc/kernel/trace/ftrace.c | 2 +-
> arch/riscv/kernel/ftrace.c | 2 +-
> arch/s390/kernel/entry.h | 4 +-
> arch/s390/kernel/ftrace.c | 4 +-
> arch/sh/kernel/ftrace.c | 2 +-
> arch/sparc/kernel/ftrace.c | 2 +-
> arch/x86/include/asm/ftrace.h | 2 +-
> arch/x86/kernel/ftrace.c | 6 +-
> include/linux/ftrace.h | 4 +-
> kernel/trace/Kconfig | 12 ++++
> kernel/trace/fgraph.c | 7 ++-
> kernel/trace/trace.c | 8 ++-
> kernel/trace/trace.h | 4 +-
> kernel/trace/trace_entries.h | 7 ++-
> kernel/trace/trace_functions.c | 46 ++++++++++++++--
> kernel/trace/trace_functions_graph.c | 74 +++++++++++++------------
> kernel/trace/trace_irqsoff.c | 4 +-
> kernel/trace/trace_output.c | 82 +++++++++++++++++++++++++++-
> kernel/trace/trace_output.h | 9 +++
> kernel/trace/trace_sched_wakeup.c | 4 +-
> 28 files changed, 228 insertions(+), 73 deletions(-)
>
> --
> 2.43.0
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 0/7] add function arguments to ftrace
2024-09-05 15:16 ` [PATCH 0/7] add function arguments to ftrace Steven Rostedt
@ 2024-09-06 6:18 ` Sven Schnelle
2024-09-06 14:07 ` Steven Rostedt
0 siblings, 1 reply; 28+ messages in thread
From: Sven Schnelle @ 2024-09-06 6:18 UTC (permalink / raw)
To: Steven Rostedt
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Guo Ren, linux-kernel,
linux-trace-kernel, linux-riscv, linux-csky
Steven Rostedt <rostedt@goodmis.org> writes:
> On Wed, 4 Sep 2024 08:58:54 +0200
> Sven Schnelle <svens@linux.ibm.com> wrote:
>
>> These patches add support for printing function arguments in ftrace.
> First I want to say THANK YOU!!!!
>
> This has been on my TODO list for far too long. I never got the time to
> work on it :-p
>
> Anyway, this is something I definitely want added. But I'm going to guess
> that there is going to be issues with it and I doubt it will be ready for
> the next merge window. I'm currently focused on some other things and also
> have to get ready for next weeks travels (I'll be in Prague for GNU Cauldron,
> then Vienna for Plumbers and OSS EU, then to Paris for Kernel Recipes!).
>
> But I most definitely want this in. Hopefully by 6.13. This may be
> something I can review on the plane (if I get my slides done).
Thanks! It's been hanging in my git repo for quite a while, so no need
to rush.
One thing i learned after submitting the series is that struct
ftrace_regs depends on CONFIG_FUNCTION_TRACER, so it cannot be used
with the graph tracer. So either we make it available unconditionally,
or use some other data structure. Would like to hear your opinion on
that, but i'll wait for the review after your travel because there
are likely other issues that needs to be fixed as well.
Thanks,
Sven
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/7] add function arguments to ftrace
2024-09-06 6:18 ` Sven Schnelle
@ 2024-09-06 14:07 ` Steven Rostedt
2024-09-08 13:28 ` Masami Hiramatsu
0 siblings, 1 reply; 28+ messages in thread
From: Steven Rostedt @ 2024-09-06 14:07 UTC (permalink / raw)
To: Sven Schnelle
Cc: Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Guo Ren, linux-kernel,
linux-trace-kernel, linux-riscv, linux-csky
On Fri, 06 Sep 2024 08:18:02 +0200
Sven Schnelle <svens@linux.ibm.com> wrote:
> One thing i learned after submitting the series is that struct
> ftrace_regs depends on CONFIG_FUNCTION_TRACER, so it cannot be used
> with the graph tracer. So either we make it available unconditionally,
> or use some other data structure. Would like to hear your opinion on
> that, but i'll wait for the review after your travel because there
> are likely other issues that needs to be fixed as well.
Hmm, I thought the graph tracer depends on function tracer? Anyway, the
configs should be cleaned up. I would like to make CONFIG_FTRACE just mean
the function hook mechanism (mcount,fentry,etc) and not be used for the
tracing system.
Anyway, we can just make ftrace_regs defined outside any config for now.
-- Steve
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/7] add function arguments to ftrace
2024-09-06 14:07 ` Steven Rostedt
@ 2024-09-08 13:28 ` Masami Hiramatsu
2024-09-09 7:52 ` Sven Schnelle
0 siblings, 1 reply; 28+ messages in thread
From: Masami Hiramatsu @ 2024-09-08 13:28 UTC (permalink / raw)
To: Steven Rostedt
Cc: Sven Schnelle, Masami Hiramatsu, Mark Rutland, Mathieu Desnoyers,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Guo Ren, linux-kernel,
linux-trace-kernel, linux-riscv, linux-csky
On Fri, 6 Sep 2024 10:07:38 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:
> On Fri, 06 Sep 2024 08:18:02 +0200
> Sven Schnelle <svens@linux.ibm.com> wrote:
>
>
> > One thing i learned after submitting the series is that struct
> > ftrace_regs depends on CONFIG_FUNCTION_TRACER, so it cannot be used
> > with the graph tracer.
Yeah, this is solved by my series [1].
[1] https://patchwork.kernel.org/project/linux-trace-kernel/patch/172398532480.293426.13232399076477198126.stgit@devnote2/
So I think this series is easier to apply after my series, which
passes fgraph_regs in return handler.
Thanks,
> > So either we make it available unconditionally,
> > or use some other data structure. Would like to hear your opinion on
> > that, but i'll wait for the review after your travel because there
> > are likely other issues that needs to be fixed as well.
>
> Hmm, I thought the graph tracer depends on function tracer? Anyway, the
> configs should be cleaned up. I would like to make CONFIG_FTRACE just mean
> the function hook mechanism (mcount,fentry,etc) and not be used for the
> tracing system.
>
> Anyway, we can just make ftrace_regs defined outside any config for now.
>
> -- Steve
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/7] add function arguments to ftrace
2024-09-08 13:28 ` Masami Hiramatsu
@ 2024-09-09 7:52 ` Sven Schnelle
2024-09-13 6:03 ` Sven Schnelle
0 siblings, 1 reply; 28+ messages in thread
From: Sven Schnelle @ 2024-09-09 7:52 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, Mark Rutland, Mathieu Desnoyers, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Guo Ren, linux-kernel,
linux-trace-kernel, linux-riscv, linux-csky
Masami Hiramatsu (Google) <mhiramat@kernel.org> writes:
> On Fri, 6 Sep 2024 10:07:38 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
>
>> On Fri, 06 Sep 2024 08:18:02 +0200
>> Sven Schnelle <svens@linux.ibm.com> wrote:
>>
>>
>> > One thing i learned after submitting the series is that struct
>> > ftrace_regs depends on CONFIG_FUNCTION_TRACER, so it cannot be used
>> > with the graph tracer.
>
> Yeah, this is solved by my series [1].
>
> [1] https://patchwork.kernel.org/project/linux-trace-kernel/patch/172398532480.293426.13232399076477198126.stgit@devnote2/
>
> So I think this series is easier to apply after my series, which
> passes fgraph_regs in return handler.
Thanks, i'll rebase my changes on top of your patches then.
Regards
Sven
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/7] add function arguments to ftrace
2024-09-09 7:52 ` Sven Schnelle
@ 2024-09-13 6:03 ` Sven Schnelle
2024-09-20 8:17 ` Masami Hiramatsu
0 siblings, 1 reply; 28+ messages in thread
From: Sven Schnelle @ 2024-09-13 6:03 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: Steven Rostedt, Mark Rutland, Mathieu Desnoyers, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Guo Ren, linux-kernel,
linux-trace-kernel, linux-riscv, linux-csky
Sven Schnelle <svens@linux.ibm.com> writes:
> Masami Hiramatsu (Google) <mhiramat@kernel.org> writes:
>
>> On Fri, 6 Sep 2024 10:07:38 -0400
>> Steven Rostedt <rostedt@goodmis.org> wrote:
>>
>>> On Fri, 06 Sep 2024 08:18:02 +0200
>>> Sven Schnelle <svens@linux.ibm.com> wrote:
>>>
>>>
>>> > One thing i learned after submitting the series is that struct
>>> > ftrace_regs depends on CONFIG_FUNCTION_TRACER, so it cannot be used
>>> > with the graph tracer.
>>
>> Yeah, this is solved by my series [1].
>>
>> [1] https://patchwork.kernel.org/project/linux-trace-kernel/patch/172398532480.293426.13232399076477198126.stgit@devnote2/
>>
>> So I think this series is easier to apply after my series, which
>> passes fgraph_regs in return handler.
>
> Thanks, i'll rebase my changes on top of your patches then.
While doing so i noticed that i completely forgot about arguments
located on the stack. The current patchset tries to read from the
current kernel stack, which is obviously wrong. So either the tracer
needs to save the stack frame in the ringbuffer (which would be quite
a lot of data), or ftrace only prints arguments located in registers.
Also not nice. Opinions?
Thanks
Sven
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 0/7] add function arguments to ftrace
2024-09-13 6:03 ` Sven Schnelle
@ 2024-09-20 8:17 ` Masami Hiramatsu
0 siblings, 0 replies; 28+ messages in thread
From: Masami Hiramatsu @ 2024-09-20 8:17 UTC (permalink / raw)
To: Sven Schnelle
Cc: Steven Rostedt, Mark Rutland, Mathieu Desnoyers, Paul Walmsley,
Palmer Dabbelt, Albert Ou, Guo Ren, linux-kernel,
linux-trace-kernel, linux-riscv, linux-csky
On Fri, 13 Sep 2024 08:03:51 +0200
Sven Schnelle <svens@linux.ibm.com> wrote:
> Sven Schnelle <svens@linux.ibm.com> writes:
>
> > Masami Hiramatsu (Google) <mhiramat@kernel.org> writes:
> >
> >> On Fri, 6 Sep 2024 10:07:38 -0400
> >> Steven Rostedt <rostedt@goodmis.org> wrote:
> >>
> >>> On Fri, 06 Sep 2024 08:18:02 +0200
> >>> Sven Schnelle <svens@linux.ibm.com> wrote:
> >>>
> >>>
> >>> > One thing i learned after submitting the series is that struct
> >>> > ftrace_regs depends on CONFIG_FUNCTION_TRACER, so it cannot be used
> >>> > with the graph tracer.
> >>
> >> Yeah, this is solved by my series [1].
> >>
> >> [1] https://patchwork.kernel.org/project/linux-trace-kernel/patch/172398532480.293426.13232399076477198126.stgit@devnote2/
> >>
> >> So I think this series is easier to apply after my series, which
> >> passes fgraph_regs in return handler.
> >
> > Thanks, i'll rebase my changes on top of your patches then.
>
> While doing so i noticed that i completely forgot about arguments
> located on the stack. The current patchset tries to read from the
> current kernel stack, which is obviously wrong. So either the tracer
> needs to save the stack frame in the ringbuffer (which would be quite
> a lot of data), or ftrace only prints arguments located in registers.
> Also not nice. Opinions?
We can limit it to first 6 arguments in the ftrace_regs by default,
no need to save all of them. We can add an option to specify how
many stack entries (but it is set 0 by default).
Thank you,
>
> Thanks
> Sven
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
^ permalink raw reply [flat|nested] 28+ messages in thread