[PATCH 0/7] add function arguments to ftrace

[PATCH 0/7] add function arguments to ftrace

[Sven Schnelle]

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    |    }
[..]


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

[PATCH 1/7] tracing: add ftrace_regs to function_graph_enter()

[Sven Schnelle]

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(&current->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(&current->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

Re: [PATCH 0/7] add function arguments to ftrace

[Steven Rostedt]

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

Re: [PATCH 0/7] add function arguments to ftrace

[Sven Schnelle]

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

Re: [PATCH 0/7] add function arguments to ftrace

[Steven Rostedt]

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

Re: [PATCH 0/7] add function arguments to ftrace

[Masami Hiramatsu]

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>

Re: [PATCH 0/7] add function arguments to ftrace

[Sven Schnelle]

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

Re: [PATCH 0/7] add function arguments to ftrace

[Sven Schnelle]

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

Re: [PATCH 0/7] add function arguments to ftrace

[Masami Hiramatsu]

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>