Skip to contentSkip to conversation

tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph

PATCH In review Masami Hiramatsu (Google) 5 participants 39 messages Dec 26, 2024 – Feb 6, 2025

Subsystems (16)
Documentation x86 PowerPC Kselftest +12 more subsystems
lore
Share & tools
lore

Masami Hiramatsu (Google)

author maintainer
#0 lore raw

v22 cover tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph

Hi,

Here is the 22nd version of the series to re-implement the fprobe on function-graph tracer. The previous version is;

https://lore.kernel.org/all/173379652547.973433.2311391879173461183.stgit@devnote2/

This version is rebased on v6.13-rc4 with fixes on [3/20] for x86-32 and [5/20] for build error.

Overview
--------
This series rewrites the fprobe on this function-graph. The purposes of this change are;

1) Remove dependency of the rethook from fprobe so that we can reduce the return hook code and shadow stack.

2) Make 'ftrace_regs' the common trace interface for the function boundary.

1) Currently we have 2(or 3) different function return hook codes, the function-graph tracer and rethook (and legacy kretprobe). But since this is redundant and needs double maintenance cost, I would like to unify those. From the user's viewpoint, function- graph tracer is very useful to grasp the execution path. For this purpose, it is hard to use the rethook in the function-graph tracer, but the opposite is possible. (Strictly speaking, kretprobe can not use it because it requires 'pt_regs' for historical reasons.)

2) Now the fprobe provides the 'pt_regs' for its handler, but that is wrong for the function entry and exit. Moreover, depending on the architecture, there is no way to accurately reproduce 'pt_regs' outside of interrupt or exception handlers. This means fprobe should not use 'pt_regs' because it does not use such exceptions. (Conversely, kprobe should use 'pt_regs' because it is an abstract interface of the software breakpoint exception.)

This series changes fprobe to use function-graph tracer for tracing function entry and exit, instead of mixture of ftrace and rethook. Unlike the rethook which is a per-task list of system-wide allocated nodes, the function graph's ret_stack is a per-task shadow stack. Thus it does not need to set 'nr_maxactive' (which is the number of pre-allocated nodes).
Also the handlers will get the 'ftrace_regs' instead of 'pt_regs'. Since eBPF mulit_kprobe/multi_kretprobe events still use 'pt_regs' as their register interface, this changes it to convert 'ftrace_regs' to 'pt_regs'. Of course this conversion makes an incomplete 'pt_regs', so users must access only registers for function parameters or return value.

Design
------
Instead of using ftrace's function entry hook directly, the new fprobe is built on top of the function-graph's entry and return callbacks with 'ftrace_regs'.

Since the fprobe requires access to 'ftrace_regs', the architecture must support CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS and CONFIG_HAVE_FTRACE_GRAPH_FUNC, which enables to call function-graph entry callback with 'ftrace_regs', and also CONFIG_HAVE_FUNCTION_GRAPH_FREGS, which passes the ftrace_regs to return_to_handler.

All fprobes share a single function-graph ops (means shares a common ftrace filter) similar to the kprobe-on-ftrace. This needs another layer to find corresponding fprobe in the common function-graph callbacks, but has much better scalability, since the number of registered function-graph ops is limited.

In the entry callback, the fprobe runs its entry_handler and saves the address of 'fprobe' on the function-graph's shadow stack as data. The return callback decodes the data to get the 'fprobe' address, and runs the exit_handler.

The fprobe introduces two hash-tables, one is for entry callback which searches fprobes related to the given function address passed by entry callback. The other is for a return callback which checks if the given 'fprobe' data structure pointer is still valid. Note that it is possible to unregister fprobe before the return callback runs. Thus the address validation must be done before using it in the return callback.

Download
--------
This series can be applied against the v6.13-rc2 kernel.

This series can also be found below branch.

https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/log/?h=topic/fprobe-on-fgraph

Thank you,

---

Masami Hiramatsu (Google) (19):
fgraph: Get ftrace recursion lock in function_graph_enter
fgraph: Pass ftrace_regs to entryfunc
fgraph: Replace fgraph_ret_regs with ftrace_regs
fgraph: Pass ftrace_regs to retfunc
fprobe: Use ftrace_regs in fprobe entry handler
fprobe: Use ftrace_regs in fprobe exit handler
tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs
tracing: Add ftrace_fill_perf_regs() for perf event tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS
bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled
ftrace: Add CONFIG_HAVE_FTRACE_GRAPH_FUNC
fprobe: Rewrite fprobe on function-graph tracer
fprobe: Add fprobe_header encoding feature
tracing/fprobe: Remove nr_maxactive from fprobe
selftests: ftrace: Remove obsolate maxactive syntax check selftests/ftrace: Add a test case for repeating register/unregister fprobe
Documentation: probes: Update fprobe on function-graph tracer
ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr
bpf: Use ftrace_get_symaddr() for kprobe_multi probes

Sven Schnelle (1):
s390/tracing: Enable HAVE_FTRACE_GRAPH_FUNC

 Documentation/trace/fprobe.rst                     |   42 +
 arch/arm64/Kconfig                                 |    2 
 arch/arm64/include/asm/Kbuild                      |    1 
 arch/arm64/include/asm/ftrace.h                    |   51 +-
 arch/arm64/kernel/asm-offsets.c                    |   12 
 arch/arm64/kernel/entry-ftrace.S                   |   32 +
 arch/arm64/kernel/ftrace.c                         |   78 ++
 arch/loongarch/Kconfig                             |    4 
 arch/loongarch/include/asm/fprobe.h                |   12 
 arch/loongarch/include/asm/ftrace.h                |   32 -
 arch/loongarch/kernel/asm-offsets.c                |   12 
 arch/loongarch/kernel/ftrace_dyn.c                 |   10 
 arch/loongarch/kernel/mcount.S                     |   17 -
 arch/loongarch/kernel/mcount_dyn.S                 |   14 
 arch/powerpc/Kconfig                               |    1 
 arch/powerpc/include/asm/ftrace.h                  |   13 
 arch/powerpc/kernel/trace/ftrace.c                 |    8 
 arch/powerpc/kernel/trace/ftrace_64_pg.c           |   16 
 arch/riscv/Kconfig                                 |    3 
 arch/riscv/include/asm/Kbuild                      |    1 
 arch/riscv/include/asm/ftrace.h                    |   45 +
 arch/riscv/kernel/ftrace.c                         |   17 -
 arch/riscv/kernel/mcount.S                         |   24 -
 arch/s390/Kconfig                                  |    4 
 arch/s390/include/asm/fprobe.h                     |   10 
 arch/s390/include/asm/ftrace.h                     |   37 +
 arch/s390/kernel/asm-offsets.c                     |    6 
 arch/s390/kernel/entry.h                           |    1 
 arch/s390/kernel/ftrace.c                          |   48 -
 arch/s390/kernel/mcount.S                          |   23 -
 arch/x86/Kconfig                                   |    4 
 arch/x86/include/asm/Kbuild                        |    1 
 arch/x86/include/asm/ftrace.h                      |   54 +-
 arch/x86/kernel/ftrace.c                           |   47 +
 arch/x86/kernel/ftrace_32.S                        |   13 
 arch/x86/kernel/ftrace_64.S                        |   17 -
 include/asm-generic/fprobe.h                       |   46 +
 include/linux/fprobe.h                             |   62 +-
 include/linux/ftrace.h                             |  116 +++
 include/linux/ftrace_regs.h                        |    2 
 kernel/trace/Kconfig                               |   22 -
 kernel/trace/bpf_trace.c                           |   28 +
 kernel/trace/fgraph.c                              |   65 +-
 kernel/trace/fprobe.c                              |  664 +++++++++++++++-----
 kernel/trace/ftrace.c                              |    6 
 kernel/trace/trace.h                               |    6 
 kernel/trace/trace_fprobe.c                        |  146 ++--
 kernel/trace/trace_functions_graph.c               |   10 
 kernel/trace/trace_irqsoff.c                       |    6 
 kernel/trace/trace_probe_tmpl.h                    |    2 
 kernel/trace/trace_sched_wakeup.c                  |    6 
 kernel/trace/trace_selftest.c                      |   11 
 lib/test_fprobe.c                                  |   51 --
 samples/fprobe/fprobe_example.c                    |    4 
 .../test.d/dynevent/add_remove_fprobe_repeat.tc    |   19 +
 .../ftrace/test.d/dynevent/fprobe_syntax_errors.tc |    4 
 56 files changed, 1318 insertions(+), 670 deletions(-)
 create mode 100644 arch/loongarch/include/asm/fprobe.h
 create mode 100644 arch/s390/include/asm/fprobe.h
 create mode 100644 include/asm-generic/fprobe.h
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe_repeat.tc

--
Masami Hiramatsu (Google) <[email protected]>

author maintainer
#1 lore raw

PATCH v22 1/20 fgraph: Get ftrace recursion lock in function_graph_enter

From: Masami Hiramatsu (Google) <[email protected]>

Get the ftrace recursion lock in the generic function_graph_enter() instead of each architecture code.
This changes all function_graph tracer callbacks running in non-preemptive state. On x86 and powerpc, this is by default, but on the other architecutres, this will be new.

Cc Steven Rostedt <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Michael Ellerman <[email protected]>
Cc Nicholas Piggin <[email protected]>
Cc Christophe Leroy <[email protected]>
Cc Naveen N Rao <[email protected]>
Cc Madhavan Srinivasan <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Mathieu Desnoyers <[email protected]>

---
Changes in v21:
- Newly added.

---
 arch/powerpc/kernel/trace/ftrace.c       |    6 ------
 arch/powerpc/kernel/trace/ftrace_64_pg.c |    6 ------
 arch/x86/kernel/ftrace.c                 |    7 -------
 kernel/trace/fgraph.c                    |    8 +++++++-
 4 files changed, 7 insertions(+), 20 deletions(-)
arch/powerpc/kernel/trace/ftrace.c +0 -6
index 5ccd791761e8..e41daf2c4a31 100644
@@ -658,7 +658,6 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
 		       struct ftrace_ops *op, struct ftrace_regs *fregs)
 {
 	unsigned long sp = arch_ftrace_regs(fregs)->regs.gpr[1];
-	int bit;
 
 	if (unlikely(ftrace_graph_is_dead()))
 		goto out;
@@ -666,14 +665,9 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		goto out;
 
-	bit = ftrace_test_recursion_trylock(ip, parent_ip);
-	if (bit < 0)
-		goto out;
-
 	if (!function_graph_enter(parent_ip, ip, 0, (unsigned long *)sp))
 		parent_ip = ppc_function_entry(return_to_handler);
 
-	ftrace_test_recursion_unlock(bit);
 out:
 	arch_ftrace_regs(fregs)->regs.link = parent_ip;
 }
arch/powerpc/kernel/trace/ftrace_64_pg.c +0 -6
index 98787376eb87..8fb860b90ae1 100644
@@ -790,7 +790,6 @@ static unsigned long
 __prepare_ftrace_return(unsigned long parent, unsigned long ip, unsigned long sp)
 {
 	unsigned long return_hooker;
-	int bit;
 
 	if (unlikely(ftrace_graph_is_dead()))
 		goto out;
@@ -798,16 +797,11 @@ __prepare_ftrace_return(unsigned long parent, unsigned long ip, unsigned long sp
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		goto out;
 
-	bit = ftrace_test_recursion_trylock(ip, parent);
-	if (bit < 0)
-		goto out;
-
 	return_hooker = ppc_function_entry(return_to_handler);
 
 	if (!function_graph_enter(parent, ip, 0, (unsigned long *)sp))
 		parent = return_hooker;
 
-	ftrace_test_recursion_unlock(bit);
 out:
 	return parent;
 }
arch/x86/kernel/ftrace.c +0 -7
index 4dd0ad6c94d6..33f50c80f481 100644
@@ -615,7 +615,6 @@ void prepare_ftrace_return(unsigned long ip, unsigned long *parent,
 			   unsigned long frame_pointer)
 {
 	unsigned long return_hooker = (unsigned long)&return_to_handler;
-	int bit;
 
 	/*
 	 * When resuming from suspend-to-ram, this function can be indirectly
@@ -635,14 +634,8 @@ void prepare_ftrace_return(unsigned long ip, unsigned long *parent,
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		return;
 
-	bit = ftrace_test_recursion_trylock(ip, *parent);
-	if (bit < 0)
-		return;
-
 	if (!function_graph_enter(*parent, ip, frame_pointer, parent))
 		*parent = return_hooker;
-
-	ftrace_test_recursion_unlock(bit);
 }
 
 #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
kernel/trace/fgraph.c +7 -1
index ddedcb50917f..5c68d6109119 100644
@@ -650,8 +650,13 @@ int function_graph_enter(unsigned long ret, unsigned long func,
 	struct ftrace_graph_ent trace;
 	unsigned long bitmap = 0;
 	int offset;
+	int bit;
 	int i;
 
+	bit = ftrace_test_recursion_trylock(func, ret);
+	if (bit < 0)
+		return -EBUSY;
+
 	trace.func = func;
 	trace.depth = ++current->curr_ret_depth;
 
@@ -697,12 +702,13 @@ int function_graph_enter(unsigned long ret, unsigned long func,
 	 * flag, set that bit always.
 	 */
 	set_bitmap(current, offset, bitmap | BIT(0));
-
+	ftrace_test_recursion_unlock(bit);
 	return 0;
  out_ret:
 	current->curr_ret_stack -= FGRAPH_FRAME_OFFSET + 1;
  out:
 	current->curr_ret_depth--;
+	ftrace_test_recursion_unlock(bit);
 	return -EBUSY;
 }
 
author maintainer
#2 lore raw

PATCH v22 2/20 fgraph: Pass ftrace_regs to entryfunc

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

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

Note that the ftrace_regs can be NULL when the arch does NOT define:
HAVE_DYNAMIC_FTRACE_WITH_ARGS or HAVE_DYNAMIC_FTRACE_WITH_REGS. More specifically, if HAVE_DYNAMIC_FTRACE_WITH_REGS is defined but not the HAVE_DYNAMIC_FTRACE_WITH_ARGS, and the ftrace ops used to register the function callback does not set FTRACE_OPS_FL_SAVE_REGS. In this case, ftrace_regs can be NULL in user callback.

Cc Steven Rostedt <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Huacai Chen <[email protected]>
Cc WANG Xuerui <[email protected]>
Cc Michael Ellerman <[email protected]>
Cc Nicholas Piggin <[email protected]>
Cc Christophe Leroy <[email protected]>
Cc Naveen N Rao <[email protected]>
Cc Madhavan Srinivasan <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Albert Ou <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Mathieu Desnoyers <[email protected]>

---
Changes in v21:
- Remove ftrace_test_recursion_trylock() from new code.
Changes in v19:
- Remove unneeded FTRACE_OPS_FL_SAVE_ARGS from fgraph_ops.
Changes in v18:
- Remove unclear comment about regs->fp access on arm64.
Changes in v16:
- Add a note when the ftrace_regs can be NULL.
- Update against for the latest kernel.
Changes in v11:
- Update for the latest for-next branch.
Changes in v8:
- Just pass ftrace_regs to the handler instead of adding a new entryregfunc.
- Update riscv ftrace_graph_func().
Changes in v3:
- Update for new multiple fgraph.

---
 arch/arm64/kernel/ftrace.c               |   15 ++++++++++-
 arch/loongarch/kernel/ftrace_dyn.c       |   10 ++++++-
 arch/powerpc/kernel/trace/ftrace.c       |    2 +
 arch/powerpc/kernel/trace/ftrace_64_pg.c |   10 ++++---
 arch/riscv/kernel/ftrace.c               |   17 +++++++++++-
 arch/x86/kernel/ftrace.c                 |   42 ++++++++++++++++++++----------
 include/linux/ftrace.h                   |   17 +++++++++---
 kernel/trace/fgraph.c                    |   20 +++++++++-----
 kernel/trace/ftrace.c                    |    3 +-
 kernel/trace/trace.h                     |    3 +-
 kernel/trace/trace_functions_graph.c     |    3 +-
 kernel/trace/trace_irqsoff.c             |    3 +-
 kernel/trace/trace_sched_wakeup.c        |    3 +-
 kernel/trace/trace_selftest.c            |    8 ++++--
 14 files changed, 114 insertions(+), 42 deletions(-)
arch/arm64/kernel/ftrace.c +14 -1
index 245cb419ca24..570c38be833c 100644
@@ -481,7 +481,20 @@ void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent,
 void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
 		       struct ftrace_ops *op, struct ftrace_regs *fregs)
 {
-	prepare_ftrace_return(ip, &arch_ftrace_regs(fregs)->lr, arch_ftrace_regs(fregs)->fp);
+	unsigned long return_hooker = (unsigned long)&return_to_handler;
+	unsigned long frame_pointer = arch_ftrace_regs(fregs)->fp;
+	unsigned long *parent = &arch_ftrace_regs(fregs)->lr;
+	unsigned long old;
+
+	if (unlikely(atomic_read(&current->tracing_graph_pause)))
+		return;
+
+	old = *parent;
+
+	if (!function_graph_enter_regs(old, ip, frame_pointer,
+				       (void *)frame_pointer, fregs)) {
+		*parent = return_hooker;
+	}
 }
 #else
 /*
arch/loongarch/kernel/ftrace_dyn.c +9 -1
index 18056229e22e..25c9a4cfd5fa 100644
@@ -243,8 +243,16 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
 {
 	struct pt_regs *regs = &arch_ftrace_regs(fregs)->regs;
 	unsigned long *parent = (unsigned long *)&regs->regs[1];
+	unsigned long return_hooker = (unsigned long)&return_to_handler;
+	unsigned long old;
+
+	if (unlikely(atomic_read(&current->tracing_graph_pause)))
+		return;
+
+	old = *parent;
 
-	prepare_ftrace_return(ip, (unsigned long *)parent);
+	if (!function_graph_enter_regs(old, ip, 0, parent, fregs))
+		*parent = return_hooker;
 }
 #else
 static int ftrace_modify_graph_caller(bool enable)
arch/powerpc/kernel/trace/ftrace.c +1 -1
index e41daf2c4a31..2f776f137a89 100644
@@ -665,7 +665,7 @@ void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
 	if (unlikely(atomic_read(&current->tracing_graph_pause)))
 		goto out;
 
-	if (!function_graph_enter(parent_ip, ip, 0, (unsigned long *)sp))
+	if (!function_graph_enter_regs(parent_ip, ip, 0, (unsigned long *)sp, fregs))
 		parent_ip = ppc_function_entry(return_to_handler);
 
 out:
arch/powerpc/kernel/trace/ftrace_64_pg.c +6 -4
index 8fb860b90ae1..ac35015f04c6 100644
@@ -787,7 +787,8 @@ int ftrace_disable_ftrace_graph_caller(void)
  * in current thread info. Return the address we want to divert to.
  */
 static unsigned long
-__prepare_ftrace_return(unsigned long parent, unsigned long ip, unsigned long sp)
+__prepare_ftrace_return(unsigned long parent, unsigned long ip, unsigned long sp,
+			struct ftrace_regs *fregs)
 {
 	unsigned long return_hooker;
 
@@ -799,7 +800,7 @@ __prepare_ftrace_return(unsigned long parent, unsigned long ip, unsigned long sp
 
 	return_hooker = ppc_function_entry(return_to_handler);
 
-	if (!function_graph_enter(parent, ip, 0, (unsigned long *)sp))
+	if (!function_graph_enter_regs(parent, ip, 0, (unsigned long *)sp, fregs))
 		parent = return_hooker;
 
 out:
@@ -810,13 +811,14 @@ __prepare_ftrace_return(unsigned long parent, unsigned long ip, unsigned long sp
 void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
 		       struct ftrace_ops *op, struct ftrace_regs *fregs)
 {
-	arch_ftrace_regs(fregs)->regs.link = __prepare_ftrace_return(parent_ip, ip, arch_ftrace_regs(fregs)->regs.gpr[1]);
+	arch_ftrace_regs(fregs)->regs.link = __prepare_ftrace_return(parent_ip, ip,
+						arch_ftrace_regs(fregs)->regs.gpr[1], fregs);
 }
 #else
 unsigned long prepare_ftrace_return(unsigned long parent, unsigned long ip,
 				    unsigned long sp)
 {
-	return __prepare_ftrace_return(parent, ip, sp);
+	return __prepare_ftrace_return(parent, ip, sp, NULL);
 }
 #endif
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
arch/riscv/kernel/ftrace.c +16 -1
index 8cb9b211611d..3524db5e4fa0 100644
@@ -214,7 +214,22 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
 		       struct ftrace_ops *op, struct ftrace_regs *fregs)
 {
-	prepare_ftrace_return(&arch_ftrace_regs(fregs)->ra, ip, arch_ftrace_regs(fregs)->s0);
+	unsigned long return_hooker = (unsigned long)&return_to_handler;
+	unsigned long frame_pointer = arch_ftrace_regs(fregs)->s0;
+	unsigned long *parent = &arch_ftrace_regs(fregs)->ra;
+	unsigned long old;
+
+	if (unlikely(atomic_read(&current->tracing_graph_pause)))
+		return;
+
+	/*
+	 * We don't suffer access faults, so no extra fault-recovery assembly
+	 * is needed here.
+	 */
+	old = *parent;
+
+	if (!function_graph_enter_regs(old, ip, frame_pointer, parent, fregs))
+		*parent = return_hooker;
 }
 #else /* CONFIG_DYNAMIC_FTRACE_WITH_ARGS */
 extern void ftrace_graph_call(void);
arch/x86/kernel/ftrace.c +28 -14

64 lines not on this page · raw message

include/linux/ftrace.h +13 -4
index aa9ddd1e4bb6..c86ac786da3d 100644
@@ -1071,10 +1071,12 @@ struct fgraph_ops;
 typedef void (*trace_func_graph_ret_t)(struct ftrace_graph_ret *,
 				       struct fgraph_ops *); /* return */
 typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
-				      struct fgraph_ops *); /* entry */
+				      struct fgraph_ops *,
+				      struct ftrace_regs *); /* entry */
 
 extern int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
-				   struct fgraph_ops *gops);
+				   struct fgraph_ops *gops,
+				   struct ftrace_regs *fregs);
 bool ftrace_pids_enabled(struct ftrace_ops *ops);
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
@@ -1114,8 +1116,15 @@ struct ftrace_ret_stack {
 extern void return_to_handler(void);
 
 extern int
-function_graph_enter(unsigned long ret, unsigned long func,
-		     unsigned long frame_pointer, unsigned long *retp);
+function_graph_enter_regs(unsigned long ret, unsigned long func,
+			  unsigned long frame_pointer, unsigned long *retp,
+			  struct ftrace_regs *fregs);
+
+static inline int function_graph_enter(unsigned long ret, unsigned long func,
+				       unsigned long fp, unsigned long *retp)
+{
+	return function_graph_enter_regs(ret, func, fp, retp, NULL);
+}
 
 struct ftrace_ret_stack *
 ftrace_graph_get_ret_stack(struct task_struct *task, int skip);
kernel/trace/fgraph.c +12 -8
index 5c68d6109119..4791fd704e28 100644
@@ -292,7 +292,8 @@ static inline unsigned long make_data_type_val(int idx, int size, int offset)
 }
 
 /* ftrace_graph_entry set to this to tell some archs to run function graph */
-static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops)
+static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops,
+		     struct ftrace_regs *fregs)
 {
 	return 0;
 }
@@ -520,7 +521,8 @@ int __weak ftrace_disable_ftrace_graph_caller(void)
 #endif
 
 int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
-			    struct fgraph_ops *gops)
+			    struct fgraph_ops *gops,
+			    struct ftrace_regs *fregs)
 {
 	return 0;
 }
@@ -644,8 +646,9 @@ ftrace_push_return_trace(unsigned long ret, unsigned long func,
 #endif
 
 /* 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)
+int function_graph_enter_regs(unsigned long ret, unsigned long func,
+			      unsigned long frame_pointer, unsigned long *retp,
+			      struct ftrace_regs *fregs)
 {
 	struct ftrace_graph_ent trace;
 	unsigned long bitmap = 0;
@@ -668,7 +671,7 @@ int function_graph_enter(unsigned long ret, unsigned long func,
 	if (static_branch_likely(&fgraph_do_direct)) {
 		int save_curr_ret_stack = current->curr_ret_stack;
 
-		if (static_call(fgraph_func)(&trace, fgraph_direct_gops))
+		if (static_call(fgraph_func)(&trace, fgraph_direct_gops, fregs))
 			bitmap |= BIT(fgraph_direct_gops->idx);
 		else
 			/* Clear out any saved storage */
@@ -686,7 +689,7 @@ int function_graph_enter(unsigned long ret, unsigned long func,
 
 			save_curr_ret_stack = current->curr_ret_stack;
 			if (ftrace_ops_test(&gops->ops, func, NULL) &&
-			    gops->entryfunc(&trace, gops))
+			    gops->entryfunc(&trace, gops, fregs))
 				bitmap |= BIT(i);
 			else
 				/* Clear out any saved storage */
@@ -1180,7 +1183,8 @@ void ftrace_graph_exit_task(struct task_struct *t)
 
 #ifdef CONFIG_DYNAMIC_FTRACE
 static int fgraph_pid_func(struct ftrace_graph_ent *trace,
-			   struct fgraph_ops *gops)
+			   struct fgraph_ops *gops,
+			   struct ftrace_regs *fregs)
 {
 	struct trace_array *tr = gops->ops.private;
 	int pid;
@@ -1194,7 +1198,7 @@ static int fgraph_pid_func(struct ftrace_graph_ent *trace,
 			return 0;
 	}
 
-	return gops->saved_func(trace, gops);
+	return gops->saved_func(trace, gops, fregs);
 }
 
 void fgraph_update_pid_func(void)
kernel/trace/ftrace.c +2 -1
index 9b17efb1a87d..e04e058faccf 100644
@@ -827,7 +827,8 @@ struct profile_fgraph_data {
 };
 
 static int profile_graph_entry(struct ftrace_graph_ent *trace,
-			       struct fgraph_ops *gops)
+			       struct fgraph_ops *gops,
+			       struct ftrace_regs *fregs)
 {
 	struct profile_fgraph_data *profile_data;
 
kernel/trace/trace.h +2 -1
index 9691b47b5f3d..0f38f36a5a8a 100644
@@ -694,7 +694,8 @@ void trace_default_header(struct seq_file *m);
 void print_trace_header(struct seq_file *m, struct trace_iterator *iter);
 
 void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
-int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops);
+int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops,
+		      struct ftrace_regs *fregs);
 
 void tracing_start_cmdline_record(void);
 void tracing_stop_cmdline_record(void);
kernel/trace/trace_functions_graph.c +2 -1
index 5504b5e4e7b4..b62ad912d84f 100644
@@ -175,7 +175,8 @@ struct fgraph_times {
 };
 
 int trace_graph_entry(struct ftrace_graph_ent *trace,
-		      struct fgraph_ops *gops)
+		      struct fgraph_ops *gops,
+		      struct ftrace_regs *fregs)
 {
 	unsigned long *task_var = fgraph_get_task_var(gops);
 	struct trace_array *tr = gops->private;
kernel/trace/trace_irqsoff.c +2 -1
index fce064e20570..ad739d76fc86 100644
@@ -176,7 +176,8 @@ static int irqsoff_display_graph(struct trace_array *tr, int set)
 }
 
 static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,
-			       struct fgraph_ops *gops)
+			       struct fgraph_ops *gops,
+			       struct ftrace_regs *fregs)
 {
 	struct trace_array *tr = irqsoff_trace;
 	struct trace_array_cpu *data;
kernel/trace/trace_sched_wakeup.c +2 -1
index d6c7f18daa15..0d9e1075d815 100644
@@ -113,7 +113,8 @@ static int wakeup_display_graph(struct trace_array *tr, int set)
 }
 
 static int wakeup_graph_entry(struct ftrace_graph_ent *trace,
-			      struct fgraph_ops *gops)
+			      struct fgraph_ops *gops,
+			      struct ftrace_regs *fregs)
 {
 	struct trace_array *tr = wakeup_trace;
 	struct trace_array_cpu *data;
kernel/trace/trace_selftest.c +5 -3
index 38b5754790c9..f54493f8783d 100644
@@ -774,7 +774,8 @@ struct fgraph_fixture {
 };
 
 static __init int store_entry(struct ftrace_graph_ent *trace,
-			      struct fgraph_ops *gops)
+			      struct fgraph_ops *gops,
+			      struct ftrace_regs *fregs)
 {
 	struct fgraph_fixture *fixture = container_of(gops, struct fgraph_fixture, gops);
 	const char *type = fixture->store_type_name;
@@ -1025,7 +1026,8 @@ static unsigned int graph_hang_thresh;
 
 /* Wrap the real function entry probe to avoid possible hanging */
 static int trace_graph_entry_watchdog(struct ftrace_graph_ent *trace,
-				      struct fgraph_ops *gops)
+				      struct fgraph_ops *gops,
+				      struct ftrace_regs *fregs)
 {
 	/* This is harmlessly racy, we want to approximately detect a hang */
 	if (unlikely(++graph_hang_thresh > GRAPH_MAX_FUNC_TEST)) {
@@ -1039,7 +1041,7 @@ static int trace_graph_entry_watchdog(struct ftrace_graph_ent *trace,
 		return 0;
 	}
 
-	return trace_graph_entry(trace, gops);
+	return trace_graph_entry(trace, gops, fregs);
 }
 
 static struct fgraph_ops fgraph_ops __initdata  = {
author maintainer
#3 lore raw

PATCH v22 3/20 fgraph: Replace fgraph_ret_regs with ftrace_regs

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Use ftrace_regs instead of fgraph_ret_regs for tracing return value on function_graph tracer because of simplifying the callback interface.

The CONFIG_HAVE_FUNCTION_GRAPH_RETVAL is also replaced by CONFIG_HAVE_FUNCTION_GRAPH_FREGS.

Cc Catalin Marinas <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Huacai Chen <[email protected]>
Cc WANG Xuerui <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Albert Ou <[email protected]>
Cc Vasily Gorbik <[email protected]>
Cc Alexander Gordeev <[email protected]>
Cc Heiko Carstens <[email protected]>
Cc Christian Borntraeger <[email protected]>
Cc Sven Schnelle <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Mathieu Desnoyers <[email protected]>

---
Changes in v21:
- Fix x86-32 asm code to recover registers from stack correctly.
Changes in v18:
- Use PTREGS_SIZE instead of redefining FRAME_SIZE on i386.
Changes in v17:
- Fixes s390 return_to_handler according to Heiko's advice.
Changes in v16:
- According to the recent ftrace_regs.h change, override ftrace_regs_get_frame_pointer() if needed.
- s390: keep stack_frame on stack, just replace fgraph_ret_regs with ftrace_regs.
Changes in v8:
- Newly added.

---
 arch/arm64/Kconfig                  |    1 +
 arch/arm64/include/asm/ftrace.h     |   23 ++++++-----------------
 arch/arm64/kernel/asm-offsets.c     |   12 ------------
 arch/arm64/kernel/entry-ftrace.S    |   32 ++++++++++++++++++--------------
 arch/loongarch/Kconfig              |    2 +-
 arch/loongarch/include/asm/ftrace.h |   26 ++++----------------------
 arch/loongarch/kernel/asm-offsets.c |   12 ------------
 arch/loongarch/kernel/mcount.S      |   17 ++++++++++-------
 arch/loongarch/kernel/mcount_dyn.S  |   14 +++++++-------
 arch/riscv/Kconfig                  |    2 +-
 arch/riscv/include/asm/ftrace.h     |   26 +++++---------------------
 arch/riscv/kernel/mcount.S          |   24 +++++++++++++-----------
 arch/s390/Kconfig                   |    2 +-
 arch/s390/include/asm/ftrace.h      |   24 +++++++-----------------
 arch/s390/kernel/asm-offsets.c      |    6 ------
 arch/s390/kernel/mcount.S           |   12 ++++++------
 arch/x86/Kconfig                    |    2 +-
 arch/x86/include/asm/ftrace.h       |   20 --------------------
 arch/x86/kernel/ftrace_32.S         |   13 +++++++------
 arch/x86/kernel/ftrace_64.S         |   17 +++++++++--------
 include/linux/ftrace.h              |   12 +++++++++---
 include/linux/ftrace_regs.h         |    2 ++
 kernel/trace/Kconfig                |    4 ++--
 kernel/trace/fgraph.c               |   21 +++++++++------------
 24 files changed, 119 insertions(+), 207 deletions(-)
arch/arm64/Kconfig +1 -0
index 100570a048c5..5f086777dad9 100644
@@ -219,6 +219,7 @@ config ARM64
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_TRACER
 	select HAVE_FUNCTION_ERROR_INJECTION
+	select HAVE_FUNCTION_GRAPH_FREGS
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_GRAPH_RETVAL
 	select HAVE_GCC_PLUGINS
arch/arm64/include/asm/ftrace.h +6 -17
index 5ccff4de7f09..b5fa57b61378 100644
@@ -129,6 +129,12 @@ ftrace_override_function_with_return(struct ftrace_regs *fregs)
 	arch_ftrace_regs(fregs)->pc = arch_ftrace_regs(fregs)->lr;
 }
 
+static __always_inline unsigned long
+ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs)
+{
+	return arch_ftrace_regs(fregs)->fp;
+}
+
 int ftrace_regs_query_register_offset(const char *name);
 
 int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec);
@@ -186,23 +192,6 @@ static inline bool arch_syscall_match_sym_name(const char *sym,
 
 #ifndef __ASSEMBLY__
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
-struct fgraph_ret_regs {
-	/* x0 - x7 */
-	unsigned long regs[8];
-
-	unsigned long fp;
-	unsigned long __unused;
-};
-
-static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs)
-{
-	return ret_regs->regs[0];
-}
-
-static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs)
-{
-	return ret_regs->fp;
-}
 
 void prepare_ftrace_return(unsigned long self_addr, unsigned long *parent,
 			   unsigned long frame_pointer);
arch/arm64/kernel/asm-offsets.c +0 -12
index 29bf85dacffe..eb1a840e4110 100644
@@ -179,18 +179,6 @@ int main(void)
   DEFINE(FTRACE_OPS_FUNC,		offsetof(struct ftrace_ops, func));
 #endif
   BLANK();
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-  DEFINE(FGRET_REGS_X0,			offsetof(struct fgraph_ret_regs, regs[0]));
-  DEFINE(FGRET_REGS_X1,			offsetof(struct fgraph_ret_regs, regs[1]));
-  DEFINE(FGRET_REGS_X2,			offsetof(struct fgraph_ret_regs, regs[2]));
-  DEFINE(FGRET_REGS_X3,			offsetof(struct fgraph_ret_regs, regs[3]));
-  DEFINE(FGRET_REGS_X4,			offsetof(struct fgraph_ret_regs, regs[4]));
-  DEFINE(FGRET_REGS_X5,			offsetof(struct fgraph_ret_regs, regs[5]));
-  DEFINE(FGRET_REGS_X6,			offsetof(struct fgraph_ret_regs, regs[6]));
-  DEFINE(FGRET_REGS_X7,			offsetof(struct fgraph_ret_regs, regs[7]));
-  DEFINE(FGRET_REGS_FP,			offsetof(struct fgraph_ret_regs, fp));
-  DEFINE(FGRET_REGS_SIZE,		sizeof(struct fgraph_ret_regs));
-#endif
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
   DEFINE(FTRACE_OPS_DIRECT_CALL,	offsetof(struct ftrace_ops, direct_call));
 #endif
arch/arm64/kernel/entry-ftrace.S +18 -14
index f0c16640ef21..169ccf600066 100644
@@ -329,24 +329,28 @@ SYM_FUNC_END(ftrace_stub_graph)
  * @fp is checked against the value passed by ftrace_graph_caller().
  */
 SYM_CODE_START(return_to_handler)
-	/* save return value regs */
-	sub sp, sp, #FGRET_REGS_SIZE
-	stp x0, x1, [sp, #FGRET_REGS_X0]
-	stp x2, x3, [sp, #FGRET_REGS_X2]
-	stp x4, x5, [sp, #FGRET_REGS_X4]
-	stp x6, x7, [sp, #FGRET_REGS_X6]
-	str x29,    [sp, #FGRET_REGS_FP]	// parent's fp
+	/* Make room for ftrace_regs */
+	sub	sp, sp, #FREGS_SIZE
+
+	/* Save return value regs */
+	stp	x0, x1, [sp, #FREGS_X0]
+	stp	x2, x3, [sp, #FREGS_X2]
+	stp	x4, x5, [sp, #FREGS_X4]
+	stp	x6, x7, [sp, #FREGS_X6]
+
+	/* Save the callsite's FP */
+	str	x29, [sp, #FREGS_FP]
 
 	mov	x0, sp
-	bl	ftrace_return_to_handler	// addr = ftrace_return_to_hander(regs);
+	bl	ftrace_return_to_handler	// addr = ftrace_return_to_hander(fregs);
 	mov	x30, x0				// restore the original return address
 
-	/* restore return value regs */
-	ldp x0, x1, [sp, #FGRET_REGS_X0]
-	ldp x2, x3, [sp, #FGRET_REGS_X2]
-	ldp x4, x5, [sp, #FGRET_REGS_X4]
-	ldp x6, x7, [sp, #FGRET_REGS_X6]
-	add sp, sp, #FGRET_REGS_SIZE
+	/* Restore return value regs */
+	ldp	x0, x1, [sp, #FREGS_X0]
+	ldp	x2, x3, [sp, #FREGS_X2]
+	ldp	x4, x5, [sp, #FREGS_X4]
+	ldp	x6, x7, [sp, #FREGS_X6]
+	add	sp, sp, #FREGS_SIZE
 
 	ret
 SYM_CODE_END(return_to_handler)
arch/loongarch/Kconfig +1 -1
index dae3a9104ca6..49f5bfc00e5a 100644
@@ -137,7 +137,7 @@ config LOONGARCH
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_ARG_ACCESS_API
 	select HAVE_FUNCTION_ERROR_INJECTION
-	select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER
+	select HAVE_FUNCTION_GRAPH_FREGS
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_TRACER
 	select HAVE_GCC_PLUGINS
arch/loongarch/include/asm/ftrace.h +4 -22
index 8f13eaeaa325..ceb3e3d9c0d3 100644
@@ -57,6 +57,10 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip)
 	instruction_pointer_set(&arch_ftrace_regs(fregs)->regs, ip);
 }
 
+#undef ftrace_regs_get_frame_pointer
+#define ftrace_regs_get_frame_pointer(fregs) \
+	(arch_ftrace_regs(fregs)->regs.regs[22])
+
 #define ftrace_graph_func ftrace_graph_func
 void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
 		       struct ftrace_ops *op, struct ftrace_regs *fregs);
@@ -78,26 +82,4 @@ __arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
 
 #endif /* CONFIG_FUNCTION_TRACER */
 
-#ifndef __ASSEMBLY__
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-struct fgraph_ret_regs {
-	/* a0 - a1 */
-	unsigned long regs[2];
-
-	unsigned long fp;
-	unsigned long __unused;
-};
-
-static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs)
-{
-	return ret_regs->regs[0];
-}
-
-static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs)
-{
-	return ret_regs->fp;
-}
-#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */
-#endif
-
 #endif /* _ASM_LOONGARCH_FTRACE_H */
arch/loongarch/kernel/asm-offsets.c +0 -12
index 049c5c3e370c..8be1c38ad8eb 100644
@@ -280,18 +280,6 @@ static void __used output_pbe_defines(void)
 }
 #endif
 
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-static void __used output_fgraph_ret_regs_defines(void)
-{
-	COMMENT("LoongArch fgraph_ret_regs offsets.");
-	OFFSET(FGRET_REGS_A0, fgraph_ret_regs, regs[0]);
-	OFFSET(FGRET_REGS_A1, fgraph_ret_regs, regs[1]);
-	OFFSET(FGRET_REGS_FP, fgraph_ret_regs, fp);
-	DEFINE(FGRET_REGS_SIZE, sizeof(struct fgraph_ret_regs));
-	BLANK();
-}
-#endif
-
 static void __used output_kvm_defines(void)
 {
 	COMMENT("KVM/LoongArch Specific offsets.");
arch/loongarch/kernel/mcount.S +10 -7
index 3015896016a0..b6850503e061 100644
@@ -79,10 +79,11 @@ SYM_FUNC_START(ftrace_graph_caller)
 SYM_FUNC_END(ftrace_graph_caller)
 
 SYM_FUNC_START(return_to_handler)
-	PTR_ADDI	sp, sp, -FGRET_REGS_SIZE
-	PTR_S		a0, sp, FGRET_REGS_A0
-	PTR_S		a1, sp, FGRET_REGS_A1
-	PTR_S		zero, sp, FGRET_REGS_FP
+	/* Save return value regs */
+	PTR_ADDI	sp, sp, -PT_SIZE
+	PTR_S		a0, sp, PT_R4
+	PTR_S		a1, sp, PT_R5
+	PTR_S		zero, sp, PT_R22
 
 	move		a0, sp
 	bl		ftrace_return_to_handler
@@ -90,9 +91,11 @@ SYM_FUNC_START(return_to_handler)
 	/* Restore the real parent address: a0 -> ra */
 	move		ra, a0
 
-	PTR_L		a0, sp, FGRET_REGS_A0
-	PTR_L		a1, sp, FGRET_REGS_A1
-	PTR_ADDI	sp, sp, FGRET_REGS_SIZE
+	/* Restore return value regs */
+	PTR_L		a0, sp, PT_R4
+	PTR_L		a1, sp, PT_R5
+	PTR_ADDI	sp, sp, PT_SIZE
+
 	jr		ra
 SYM_FUNC_END(return_to_handler)
 #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
arch/loongarch/kernel/mcount_dyn.S +7 -7
index 0c65cf09110c..d6b474ad1d5e 100644
@@ -140,19 +140,19 @@ SYM_CODE_END(ftrace_graph_caller)
 SYM_CODE_START(return_to_handler)
 	UNWIND_HINT_UNDEFINED
 	/* Save return value regs */
-	PTR_ADDI	sp, sp, -FGRET_REGS_SIZE
-	PTR_S		a0, sp, FGRET_REGS_A0
-	PTR_S		a1, sp, FGRET_REGS_A1
-	PTR_S		zero, sp, FGRET_REGS_FP
+	PTR_ADDI	sp, sp, -PT_SIZE
+	PTR_S		a0, sp, PT_R4
+	PTR_S		a1, sp, PT_R5
+	PTR_S		zero, sp, PT_R22
 
 	move		a0, sp
 	bl		ftrace_return_to_handler
 	move		ra, a0
 
 	/* Restore return value regs */
-	PTR_L		a0, sp, FGRET_REGS_A0
-	PTR_L		a1, sp, FGRET_REGS_A1
-	PTR_ADDI	sp, sp, FGRET_REGS_SIZE
+	PTR_L		a0, sp, PT_R4
+	PTR_L		a1, sp, PT_R5
+	PTR_ADDI	sp, sp, PT_SIZE
 
 	jr		ra
 SYM_CODE_END(return_to_handler)
arch/riscv/Kconfig +1 -1
index d4a7ca0388c0..1e807c61258f 100644
@@ -148,7 +148,7 @@ config RISCV
 	select HAVE_DYNAMIC_FTRACE_WITH_ARGS if HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
 	select HAVE_FUNCTION_GRAPH_TRACER
-	select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER
+	select HAVE_FUNCTION_GRAPH_FREGS
 	select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
 	select HAVE_EBPF_JIT if MMU
 	select HAVE_GUP_FAST if MMU
arch/riscv/include/asm/ftrace.h +5 -21
index 3d66437a1029..9372f8d7036f 100644
@@ -168,6 +168,11 @@ static __always_inline unsigned long ftrace_regs_get_stack_pointer(const struct
 	return arch_ftrace_regs(fregs)->sp;
 }
 
+static __always_inline unsigned long ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs)
+{
+	return arch_ftrace_regs(fregs)->s0;
+}
+
 static __always_inline unsigned long ftrace_regs_get_argument(struct ftrace_regs *fregs,
 							      unsigned int n)
 {
@@ -208,25 +213,4 @@ static inline void arch_ftrace_set_direct_caller(struct ftrace_regs *fregs, unsi
 
 #endif /* CONFIG_DYNAMIC_FTRACE */
 
-#ifndef __ASSEMBLY__
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-struct fgraph_ret_regs {
-	unsigned long a1;
-	unsigned long a0;
-	unsigned long s0;
-	unsigned long ra;
-};
-
-static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs)
-{
-	return ret_regs->a0;
-}
-
-static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs)
-{
-	return ret_regs->s0;
-}
-#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */
-#endif
-
 #endif /* _ASM_RISCV_FTRACE_H */
arch/riscv/kernel/mcount.S +13 -11
index 3a42f6287909..068168046e0e 100644
@@ -12,6 +12,8 @@
 #include <asm/asm-offsets.h>
 #include <asm/ftrace.h>
 
+#define ABI_SIZE_ON_STACK	80
+
 	.text
 
 	.macro SAVE_ABI_STATE
@@ -26,12 +28,12 @@
 	 * register if a0 was not saved.
 	 */
 	.macro SAVE_RET_ABI_STATE
-	addi	sp, sp, -4*SZREG
-	REG_S	s0, 2*SZREG(sp)
-	REG_S	ra, 3*SZREG(sp)
-	REG_S	a0, 1*SZREG(sp)
-	REG_S	a1, 0*SZREG(sp)
-	addi	s0, sp, 4*SZREG
+	addi	sp, sp, -ABI_SIZE_ON_STACK
+	REG_S	ra, 1*SZREG(sp)
+	REG_S	s0, 8*SZREG(sp)
+	REG_S	a0, 10*SZREG(sp)
+	REG_S	a1, 11*SZREG(sp)
+	addi	s0, sp, ABI_SIZE_ON_STACK
 	.endm
 
 	.macro RESTORE_ABI_STATE
@@ -41,11 +43,11 @@
 	.endm
 
 	.macro RESTORE_RET_ABI_STATE
-	REG_L	ra, 3*SZREG(sp)
-	REG_L	s0, 2*SZREG(sp)
-	REG_L	a0, 1*SZREG(sp)
-	REG_L	a1, 0*SZREG(sp)
-	addi	sp, sp, 4*SZREG
+	REG_L	ra, 1*SZREG(sp)
+	REG_L	s0, 8*SZREG(sp)
+	REG_L	a0, 10*SZREG(sp)
+	REG_L	a1, 11*SZREG(sp)
+	addi	sp, sp, ABI_SIZE_ON_STACK
 	.endm
 
 SYM_TYPED_FUNC_START(ftrace_stub)
arch/s390/Kconfig +1 -1
index 0077969170e8..102029e56cf0 100644
@@ -192,7 +192,7 @@ config S390
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_ARG_ACCESS_API
 	select HAVE_FUNCTION_ERROR_INJECTION
-	select HAVE_FUNCTION_GRAPH_RETVAL
+	select HAVE_FUNCTION_GRAPH_FREGS
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_TRACER
 	select HAVE_GCC_PLUGINS
arch/s390/include/asm/ftrace.h +7 -17
index fc97d75dc752..5c94c1fc1bc1 100644
@@ -62,23 +62,6 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *
 	return NULL;
 }
 
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-struct fgraph_ret_regs {
-	unsigned long gpr2;
-	unsigned long fp;
-};
-
-static __always_inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs)
-{
-	return ret_regs->gpr2;
-}
-
-static __always_inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs)
-{
-	return ret_regs->fp;
-}
-#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
 static __always_inline void
 ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
 				    unsigned long ip)
@@ -86,6 +69,13 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
 	arch_ftrace_regs(fregs)->regs.psw.addr = ip;
 }
 
+#undef ftrace_regs_get_frame_pointer
+static __always_inline unsigned long
+ftrace_regs_get_frame_pointer(struct ftrace_regs *fregs)
+{
+	return ftrace_regs_get_stack_pointer(fregs);
+}
+
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 /*
  * When an ftrace registered caller is tracing a function that is
arch/s390/kernel/asm-offsets.c +0 -6
index 862a9140528e..36709112ae7a 100644
@@ -175,12 +175,6 @@ int main(void)
 	DEFINE(OLDMEM_SIZE, PARMAREA + offsetof(struct parmarea, oldmem_size));
 	DEFINE(COMMAND_LINE, PARMAREA + offsetof(struct parmarea, command_line));
 	DEFINE(MAX_COMMAND_LINE_SIZE, PARMAREA + offsetof(struct parmarea, max_command_line_size));
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-	/* function graph return value tracing */
-	OFFSET(__FGRAPH_RET_GPR2, fgraph_ret_regs, gpr2);
-	OFFSET(__FGRAPH_RET_FP, fgraph_ret_regs, fp);
-	DEFINE(__FGRAPH_RET_SIZE, sizeof(struct fgraph_ret_regs));
-#endif
 	OFFSET(__FTRACE_REGS_PT_REGS, __arch_ftrace_regs, regs);
 	DEFINE(__FTRACE_REGS_SIZE, sizeof(struct __arch_ftrace_regs));
 
arch/s390/kernel/mcount.S +6 -6
index 7e267ef63a7f..2b628aa3d809 100644
@@ -134,14 +134,14 @@ SYM_CODE_END(ftrace_common)
 SYM_FUNC_START(return_to_handler)
 	stmg	%r2,%r5,32(%r15)
 	lgr	%r1,%r15
-	aghi	%r15,-(STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE)
+	# allocate ftrace_regs and stack frame for ftrace_return_to_handler
+	aghi	%r15,-STACK_FRAME_SIZE_FREGS
 	stg	%r1,__SF_BACKCHAIN(%r15)
-	la	%r3,STACK_FRAME_OVERHEAD(%r15)
-	stg	%r1,__FGRAPH_RET_FP(%r3)
-	stg	%r2,__FGRAPH_RET_GPR2(%r3)
-	lgr	%r2,%r3
+	stg	%r2,(STACK_FREGS_PTREGS_GPRS+2*8)(%r15)
+	stg	%r1,(STACK_FREGS_PTREGS_GPRS+15*8)(%r15)
+	la	%r2,STACK_FRAME_OVERHEAD(%r15)
 	brasl	%r14,ftrace_return_to_handler
-	aghi	%r15,STACK_FRAME_OVERHEAD+__FGRAPH_RET_SIZE
+	aghi	%r15,STACK_FRAME_SIZE_FREGS
 	lgr	%r14,%r2
 	lmg	%r2,%r5,32(%r15)
 	BR_EX	%r14
arch/x86/Kconfig +1 -1
index 9d7bd0ae48c4..ff0d7e07c611 100644
@@ -235,7 +235,7 @@ config X86
 	select HAVE_GUP_FAST
 	select HAVE_FENTRY			if X86_64 || DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD
-	select HAVE_FUNCTION_GRAPH_RETVAL	if HAVE_FUNCTION_GRAPH_TRACER
+	select HAVE_FUNCTION_GRAPH_FREGS	if HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_GRAPH_TRACER	if X86_32 || (X86_64 && DYNAMIC_FTRACE)
 	select HAVE_FUNCTION_TRACER
 	select HAVE_GCC_PLUGINS
arch/x86/include/asm/ftrace.h +0 -20
index 6e8cf0fa48fc..d61407c680c2 100644
@@ -134,24 +134,4 @@ static inline bool arch_trace_is_compat_syscall(struct pt_regs *regs)
 #endif /* !COMPILE_OFFSETS */
 #endif /* !__ASSEMBLY__ */
 
-#ifndef __ASSEMBLY__
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-struct fgraph_ret_regs {
-	unsigned long ax;
-	unsigned long dx;
-	unsigned long bp;
-};
-
-static inline unsigned long fgraph_ret_regs_return_value(struct fgraph_ret_regs *ret_regs)
-{
-	return ret_regs->ax;
-}
-
-static inline unsigned long fgraph_ret_regs_frame_pointer(struct fgraph_ret_regs *ret_regs)
-{
-	return ret_regs->bp;
-}
-#endif /* ifdef CONFIG_FUNCTION_GRAPH_TRACER */
-#endif
-
 #endif /* _ASM_X86_FTRACE_H */
arch/x86/kernel/ftrace_32.S +7 -6
index 58d9ed50fe61..f4e0c3361234 100644
@@ -187,14 +187,15 @@ SYM_CODE_END(ftrace_graph_caller)
 
 .globl return_to_handler
 return_to_handler:
-	pushl	$0
-	pushl	%edx
-	pushl	%eax
+	subl	$(PTREGS_SIZE), %esp
+	movl	$0, PT_EBP(%esp)
+	movl	%edx, PT_EDX(%esp)
+	movl	%eax, PT_EAX(%esp)
 	movl	%esp, %eax
 	call	ftrace_return_to_handler
 	movl	%eax, %ecx
-	popl	%eax
-	popl	%edx
-	addl	$4, %esp		# skip ebp
+	movl	PT_EAX(%esp), %eax
+	movl	PT_EDX(%esp), %edx
+	addl	$(PTREGS_SIZE), %esp
 	JMP_NOSPEC ecx
 #endif
arch/x86/kernel/ftrace_64.S +9 -8
index 214f30e9f0c0..d51647228596 100644
@@ -348,21 +348,22 @@ STACK_FRAME_NON_STANDARD_FP(__fentry__)
 SYM_CODE_START(return_to_handler)
 	UNWIND_HINT_UNDEFINED
 	ANNOTATE_NOENDBR
-	subq  $24, %rsp
 
-	/* Save the return values */
-	movq %rax, (%rsp)
-	movq %rdx, 8(%rsp)
-	movq %rbp, 16(%rsp)
+	/* Save ftrace_regs for function exit context  */
+	subq $(FRAME_SIZE), %rsp
+
+	movq %rax, RAX(%rsp)
+	movq %rdx, RDX(%rsp)
+	movq %rbp, RBP(%rsp)
 	movq %rsp, %rdi
 
 	call ftrace_return_to_handler
 
 	movq %rax, %rdi
-	movq 8(%rsp), %rdx
-	movq (%rsp), %rax
+	movq RDX(%rsp), %rdx
+	movq RAX(%rsp), %rax
 
-	addq $24, %rsp
+	addq $(FRAME_SIZE), %rsp
 	/*
 	 * Jump back to the old return address. This cannot be JMP_NOSPEC rdi
 	 * since IBT would demand that contain ENDBR, which simply isn't so for
include/linux/ftrace.h +9 -3
index c86ac786da3d..069f270bd7ae 100644
@@ -43,9 +43,8 @@ struct dyn_ftrace;
 
 char *arch_ftrace_match_adjust(char *str, const char *search);
 
-#ifdef CONFIG_HAVE_FUNCTION_GRAPH_RETVAL
-struct fgraph_ret_regs;
-unsigned long ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs);
+#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS
+unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs);
 #else
 unsigned long ftrace_return_to_handler(unsigned long frame_pointer);
 #endif
@@ -134,6 +133,13 @@ extern int ftrace_enabled;
  * Also, architecture dependent fields can be used for internal process.
  * (e.g. orig_ax on x86_64)
  *
+ * Basically, ftrace_regs stores the registers related to the context.
+ * On function entry, registers for function parameters and hooking the
+ * function call are stored, and on function exit, registers for function
+ * return value and frame pointers are stored.
+ *
+ * And also, it dpends on the context that which registers are restored
+ * from the ftrace_regs.
  * On the function entry, those registers will be restored except for
  * the stack pointer, so that user can change the function parameters
  * and instruction pointer (e.g. live patching.)
include/linux/ftrace_regs.h +2 -0
index be1ed0c891d0..bbc1873ca6b8 100644
@@ -30,6 +30,8 @@ struct ftrace_regs;
 	override_function_with_return(&arch_ftrace_regs(fregs)->regs)
 #define ftrace_regs_query_register_offset(name) \
 	regs_query_register_offset(name)
+#define ftrace_regs_get_frame_pointer(fregs) \
+	frame_pointer(&arch_ftrace_regs(fregs)->regs)
 
 #endif /* HAVE_ARCH_FTRACE_REGS */
 
kernel/trace/Kconfig +2 -2
index 74c2b1d43bb9..c5ab2a561272 100644
@@ -31,7 +31,7 @@ config HAVE_FUNCTION_GRAPH_TRACER
 	help
 	  See Documentation/trace/ftrace-design.rst
 
-config HAVE_FUNCTION_GRAPH_RETVAL
+config HAVE_FUNCTION_GRAPH_FREGS
 	bool
 
 config HAVE_DYNAMIC_FTRACE
@@ -232,7 +232,7 @@ config FUNCTION_GRAPH_TRACER
 
 config FUNCTION_GRAPH_RETVAL
 	bool "Kernel Function Graph Return Value"
-	depends on HAVE_FUNCTION_GRAPH_RETVAL
+	depends on HAVE_FUNCTION_GRAPH_FREGS
 	depends on FUNCTION_GRAPH_TRACER
 	default n
 	help
kernel/trace/fgraph.c +9 -12
index 4791fd704e28..51196f10d96e 100644
@@ -801,15 +801,12 @@ static struct notifier_block ftrace_suspend_notifier = {
 	.notifier_call = ftrace_suspend_notifier_call,
 };
 
-/* fgraph_ret_regs is not defined without CONFIG_FUNCTION_GRAPH_RETVAL */
-struct fgraph_ret_regs;
-
 /*
  * Send the trace to the ring-buffer.
  * @return the original return address.
  */
-static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs,
-						unsigned long frame_pointer)
+static inline unsigned long
+__ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointer)
 {
 	struct ftrace_ret_stack *ret_stack;
 	struct ftrace_graph_ret trace;
@@ -829,7 +826,7 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs
 
 	trace.rettime = trace_clock_local();
 #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
-	trace.retval = fgraph_ret_regs_return_value(ret_regs);
+	trace.retval = ftrace_regs_get_return_value(fregs);
 #endif
 
 	bitmap = get_bitmap_bits(current, offset);
@@ -864,14 +861,14 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs
 }
 
 /*
- * After all architecures have selected HAVE_FUNCTION_GRAPH_RETVAL, we can
- * leave only ftrace_return_to_handler(ret_regs).
+ * After all architecures have selected HAVE_FUNCTION_GRAPH_FREGS, we can
+ * leave only ftrace_return_to_handler(fregs).
  */
-#ifdef CONFIG_HAVE_FUNCTION_GRAPH_RETVAL
-unsigned long ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs)
+#ifdef CONFIG_HAVE_FUNCTION_GRAPH_FREGS
+unsigned long ftrace_return_to_handler(struct ftrace_regs *fregs)
 {
-	return __ftrace_return_to_handler(ret_regs,
-				fgraph_ret_regs_frame_pointer(ret_regs));
+	return __ftrace_return_to_handler(fregs,
+				ftrace_regs_get_frame_pointer(fregs));
 }
 #else
 unsigned long ftrace_return_to_handler(unsigned long frame_pointer)
author maintainer
#4 lore raw

PATCH v22 4/20 fgraph: Pass ftrace_regs to retfunc

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

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

---
Changes in v8:
- Pass ftrace_regs to retfunc, instead of adding retregfunc.
Changes in v6:
- update to use ftrace_regs_get_return_value() because of reordering patches.
Changes in v3:
- Update for new multiple fgraph.
- Save the return address to instruction pointer in ftrace_regs.

---
 include/linux/ftrace.h               |    3 ++-
 kernel/trace/fgraph.c                |   16 +++++++++++-----
 kernel/trace/ftrace.c                |    3 ++-
 kernel/trace/trace.h                 |    3 ++-
 kernel/trace/trace_functions_graph.c |    7 ++++---
 kernel/trace/trace_irqsoff.c         |    3 ++-
 kernel/trace/trace_sched_wakeup.c    |    3 ++-
 kernel/trace/trace_selftest.c        |    3 ++-
 8 files changed, 27 insertions(+), 14 deletions(-)
include/linux/ftrace.h +2 -1
index 069f270bd7ae..9a1e768e47da 100644
@@ -1075,7 +1075,8 @@ struct fgraph_ops;
 
 /* Type of the callback handlers for tracing function graph*/
 typedef void (*trace_func_graph_ret_t)(struct ftrace_graph_ret *,
-				       struct fgraph_ops *); /* return */
+				       struct fgraph_ops *,
+				       struct ftrace_regs *); /* return */
 typedef int (*trace_func_graph_ent_t)(struct ftrace_graph_ent *,
 				      struct fgraph_ops *,
 				      struct ftrace_regs *); /* entry */
kernel/trace/fgraph.c +11 -5
index 51196f10d96e..c928527251e3 100644
@@ -299,7 +299,8 @@ static int entry_run(struct ftrace_graph_ent *trace, struct fgraph_ops *ops,
 }
 
 /* ftrace_graph_return set to this to tell some archs to run function graph */
-static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops)
+static void return_run(struct ftrace_graph_ret *trace, struct fgraph_ops *ops,
+		       struct ftrace_regs *fregs)
 {
 }
 
@@ -528,7 +529,8 @@ int ftrace_graph_entry_stub(struct ftrace_graph_ent *trace,
 }
 
 static void ftrace_graph_ret_stub(struct ftrace_graph_ret *trace,
-				  struct fgraph_ops *gops)
+				  struct fgraph_ops *gops,
+				  struct ftrace_regs *fregs)
 {
 }
 
@@ -825,6 +827,9 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
 	}
 
 	trace.rettime = trace_clock_local();
+	if (fregs)
+		ftrace_regs_set_instruction_pointer(fregs, ret);
+
 #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
 	trace.retval = ftrace_regs_get_return_value(fregs);
 #endif
@@ -834,7 +839,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
 #ifdef CONFIG_HAVE_STATIC_CALL
 	if (static_branch_likely(&fgraph_do_direct)) {
 		if (test_bit(fgraph_direct_gops->idx, &bitmap))
-			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops);
+			static_call(fgraph_retfunc)(&trace, fgraph_direct_gops, fregs);
 	} else
 #endif
 	{
@@ -844,7 +849,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
 			if (gops == &fgraph_stub)
 				continue;
 
-			gops->retfunc(&trace, gops);
+			gops->retfunc(&trace, gops, fregs);
 		}
 	}
 
@@ -1016,7 +1021,8 @@ void ftrace_graph_sleep_time_control(bool enable)
  * Simply points to ftrace_stub, but with the proper protocol.
  * Defined by the linker script in linux/vmlinux.lds.h
  */
-void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
+void ftrace_stub_graph(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
+		       struct ftrace_regs *fregs);
 
 /* The callbacks that hook a function */
 trace_func_graph_ret_t ftrace_graph_return = ftrace_stub_graph;
kernel/trace/ftrace.c +2 -1
index e04e058faccf..bd2fc0274115 100644
@@ -850,7 +850,8 @@ static int profile_graph_entry(struct ftrace_graph_ent *trace,
 }
 
 static void profile_graph_return(struct ftrace_graph_ret *trace,
-				 struct fgraph_ops *gops)
+				 struct fgraph_ops *gops,
+				 struct ftrace_regs *fregs)
 {
 	struct profile_fgraph_data *profile_data;
 	struct ftrace_profile_stat *stat;
kernel/trace/trace.h +2 -1
index 0f38f36a5a8a..5f3e68a8d8a0 100644
@@ -693,7 +693,8 @@ void trace_latency_header(struct seq_file *m);
 void trace_default_header(struct seq_file *m);
 void print_trace_header(struct seq_file *m, struct trace_iterator *iter);
 
-void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops);
+void trace_graph_return(struct ftrace_graph_ret *trace, struct fgraph_ops *gops,
+			struct ftrace_regs *fregs);
 int trace_graph_entry(struct ftrace_graph_ent *trace, struct fgraph_ops *gops,
 		      struct ftrace_regs *fregs);
 
kernel/trace/trace_functions_graph.c +4 -3
index b62ad912d84f..d0e4f412c298 100644
@@ -315,7 +315,7 @@ static void handle_nosleeptime(struct ftrace_graph_ret *trace,
 }
 
 void trace_graph_return(struct ftrace_graph_ret *trace,
-			struct fgraph_ops *gops)
+			struct fgraph_ops *gops, struct ftrace_regs *fregs)
 {
 	unsigned long *task_var = fgraph_get_task_var(gops);
 	struct trace_array *tr = gops->private;
@@ -355,7 +355,8 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
 }
 
 static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
-				      struct fgraph_ops *gops)
+				      struct fgraph_ops *gops,
+				      struct ftrace_regs *fregs)
 {
 	struct fgraph_times *ftimes;
 	int size;
@@ -379,7 +380,7 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
 	    (trace->rettime - ftimes->calltime < tracing_thresh))
 		return;
 	else
-		trace_graph_return(trace, gops);
+		trace_graph_return(trace, gops, fregs);
 }
 
 static struct fgraph_ops funcgraph_ops = {
kernel/trace/trace_irqsoff.c +2 -1
index ad739d76fc86..504de7a05498 100644
@@ -208,7 +208,8 @@ static int irqsoff_graph_entry(struct ftrace_graph_ent *trace,
 }
 
 static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
-				 struct fgraph_ops *gops)
+				 struct fgraph_ops *gops,
+				 struct ftrace_regs *fregs)
 {
 	struct trace_array *tr = irqsoff_trace;
 	struct trace_array_cpu *data;
kernel/trace/trace_sched_wakeup.c +2 -1
index 0d9e1075d815..8165382a174a 100644
@@ -144,7 +144,8 @@ static int wakeup_graph_entry(struct ftrace_graph_ent *trace,
 }
 
 static void wakeup_graph_return(struct ftrace_graph_ret *trace,
-				struct fgraph_ops *gops)
+				struct fgraph_ops *gops,
+				struct ftrace_regs *fregs)
 {
 	struct trace_array *tr = wakeup_trace;
 	struct trace_array_cpu *data;
kernel/trace/trace_selftest.c +2 -1
index f54493f8783d..d88c44f1dfa5 100644
@@ -808,7 +808,8 @@ static __init int store_entry(struct ftrace_graph_ent *trace,
 }
 
 static __init void store_return(struct ftrace_graph_ret *trace,
-				struct fgraph_ops *gops)
+				struct fgraph_ops *gops,
+				struct ftrace_regs *fregs)
 {
 	struct fgraph_fixture *fixture = container_of(gops, struct fgraph_fixture, gops);
 	const char *type = fixture->store_type_name;
author maintainer
#5 lore raw

PATCH v22 5/20 fprobe: Use ftrace_regs in fprobe entry handler

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

This allows fprobes to be available with CONFIG_DYNAMIC_FTRACE_WITH_ARGS instead of CONFIG_DYNAMIC_FTRACE_WITH_REGS, then we can enable fprobe on arm64.

---
Changes in v22:
- Fix a build error: Pass ftrace_regs to trace_fprobe_entry_handler()
Changes in v6:
- Keep using SAVE_REGS flag to avoid breaking bpf kprobe-multi test.

---
 include/linux/fprobe.h          |    2 +-
 kernel/trace/Kconfig            |    3 ++-
 kernel/trace/bpf_trace.c        |   10 +++++++---
 kernel/trace/fprobe.c           |    3 ++-
 kernel/trace/trace_fprobe.c     |   11 ++++++++---
 lib/test_fprobe.c               |    4 ++--
 samples/fprobe/fprobe_example.c |    2 +-
 7 files changed, 23 insertions(+), 12 deletions(-)
include/linux/fprobe.h +1 -1
index f39869588117..ca64ee5e45d2 100644
@@ -10,7 +10,7 @@
 struct fprobe;
 
 typedef int (*fprobe_entry_cb)(struct fprobe *fp, unsigned long entry_ip,
-			       unsigned long ret_ip, struct pt_regs *regs,
+			       unsigned long ret_ip, struct ftrace_regs *regs,
 			       void *entry_data);
 
 typedef void (*fprobe_exit_cb)(struct fprobe *fp, unsigned long entry_ip,
kernel/trace/Kconfig +2 -1
index c5ab2a561272..f10ca86fbfad 100644
@@ -297,7 +297,7 @@ config DYNAMIC_FTRACE_WITH_ARGS
 config FPROBE
 	bool "Kernel Function Probe (fprobe)"
 	depends on FUNCTION_TRACER
-	depends on DYNAMIC_FTRACE_WITH_REGS
+	depends on DYNAMIC_FTRACE_WITH_REGS || DYNAMIC_FTRACE_WITH_ARGS
 	depends on HAVE_RETHOOK
 	select RETHOOK
 	default n
@@ -682,6 +682,7 @@ config FPROBE_EVENTS
 	select TRACING
 	select PROBE_EVENTS
 	select DYNAMIC_EVENTS
+	depends on DYNAMIC_FTRACE_WITH_REGS
 	default y
 	help
 	  This allows user to add tracing events on the function entry and
kernel/trace/bpf_trace.c +7 -3
index 1b8db5aee9d3..7bb2e6ecd31f 100644
@@ -2561,7 +2561,7 @@ struct bpf_session_run_ctx {
 	void *data;
 };
 
-#ifdef CONFIG_FPROBE
+#if defined(CONFIG_FPROBE) && defined(CONFIG_DYNAMIC_FTRACE_WITH_REGS)
 struct bpf_kprobe_multi_link {
 	struct bpf_link link;
 	struct fprobe fp;
@@ -2813,12 +2813,16 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
 
 static int
 kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
-			  unsigned long ret_ip, struct pt_regs *regs,
+			  unsigned long ret_ip, struct ftrace_regs *fregs,
 			  void *data)
 {
+	struct pt_regs *regs = ftrace_get_regs(fregs);
 	struct bpf_kprobe_multi_link *link;
 	int err;
 
+	if (!regs)
+		return 0;
+
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
 	err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs, false, data);
 	return is_kprobe_session(link->link.prog) ? err : 0;
@@ -3093,7 +3097,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	kvfree(cookies);
 	return err;
 }
-#else /* !CONFIG_FPROBE */
+#else /* !CONFIG_FPROBE || !CONFIG_DYNAMIC_FTRACE_WITH_REGS */
 int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 {
 	return -EOPNOTSUPP;
kernel/trace/fprobe.c +2 -1
index 9ff018245840..3d3789283873 100644
@@ -46,7 +46,7 @@ static inline void __fprobe_handler(unsigned long ip, unsigned long parent_ip,
 	}
 
 	if (fp->entry_handler)
-		ret = fp->entry_handler(fp, ip, parent_ip, ftrace_get_regs(fregs), entry_data);
+		ret = fp->entry_handler(fp, ip, parent_ip, fregs, entry_data);
 
 	/* If entry_handler returns !0, nmissed is not counted. */
 	if (rh) {
@@ -182,6 +182,7 @@ static void fprobe_init(struct fprobe *fp)
 		fp->ops.func = fprobe_kprobe_handler;
 	else
 		fp->ops.func = fprobe_handler;
+
 	fp->ops.flags |= FTRACE_OPS_FL_SAVE_REGS;
 }
 
kernel/trace/trace_fprobe.c +8 -3
index c62d1629cffe..0f254685e26a 100644
@@ -217,12 +217,13 @@ NOKPROBE_SYMBOL(fentry_trace_func);
 
 /* function exit handler */
 static int trace_fprobe_entry_handler(struct fprobe *fp, unsigned long entry_ip,
-				unsigned long ret_ip, struct pt_regs *regs,
+				unsigned long ret_ip, struct ftrace_regs *fregs,
 				void *entry_data)
 {
 	struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
+	struct pt_regs *regs = ftrace_get_regs(fregs);
 
-	if (tf->tp.entry_arg)
+	if (regs && tf->tp.entry_arg)
 		store_trace_entry_data(entry_data, &tf->tp, regs);
 
 	return 0;
@@ -339,12 +340,16 @@ NOKPROBE_SYMBOL(fexit_perf_func);
 #endif	/* CONFIG_PERF_EVENTS */
 
 static int fentry_dispatcher(struct fprobe *fp, unsigned long entry_ip,
-			     unsigned long ret_ip, struct pt_regs *regs,
+			     unsigned long ret_ip, struct ftrace_regs *fregs,
 			     void *entry_data)
 {
 	struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
+	struct pt_regs *regs = ftrace_get_regs(fregs);
 	int ret = 0;
 
+	if (!regs)
+		return 0;
+
 	if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE))
 		fentry_trace_func(tf, entry_ip, regs);
 #ifdef CONFIG_PERF_EVENTS
lib/test_fprobe.c +2 -2
index 24de0e5ff859..ff607babba18 100644
@@ -40,7 +40,7 @@ static noinline u32 fprobe_selftest_nest_target(u32 value, u32 (*nest)(u32))
 
 static notrace int fp_entry_handler(struct fprobe *fp, unsigned long ip,
 				    unsigned long ret_ip,
-				    struct pt_regs *regs, void *data)
+				    struct ftrace_regs *fregs, void *data)
 {
 	KUNIT_EXPECT_FALSE(current_test, preemptible());
 	/* This can be called on the fprobe_selftest_target and the fprobe_selftest_target2 */
@@ -81,7 +81,7 @@ static notrace void fp_exit_handler(struct fprobe *fp, unsigned long ip,
 
 static notrace int nest_entry_handler(struct fprobe *fp, unsigned long ip,
 				      unsigned long ret_ip,
-				      struct pt_regs *regs, void *data)
+				      struct ftrace_regs *fregs, void *data)
 {
 	KUNIT_EXPECT_FALSE(current_test, preemptible());
 	return 0;
samples/fprobe/fprobe_example.c +1 -1
index 0a50b05add96..c234afae52d6 100644
@@ -50,7 +50,7 @@ static void show_backtrace(void)
 
 static int sample_entry_handler(struct fprobe *fp, unsigned long ip,
 				unsigned long ret_ip,
-				struct pt_regs *regs, void *data)
+				struct ftrace_regs *fregs, void *data)
 {
 	if (use_trace)
 		/*
author maintainer
#6 lore raw

PATCH v22 6/20 fprobe: Use ftrace_regs in fprobe exit handler

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Change the fprobe exit handler to use ftrace_regs structure instead of pt_regs. This also introduce HAVE_FTRACE_REGS_HAVING_PT_REGS which means the ftrace_regs is including the pt_regs so that ftrace_regs can provide pt_regs without memory allocation.
Fprobe introduces a new dependency with that.

Cc Huacai Chen <[email protected]>
Cc WANG Xuerui <[email protected]>
Cc Vasily Gorbik <[email protected]>
Cc Alexander Gordeev <[email protected]>
Cc Christian Borntraeger <[email protected]>
Cc Sven Schnelle <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Mathieu Desnoyers <[email protected]>
Cc Song Liu <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc KP Singh <[email protected]>
Cc Matt Bobrowski <[email protected]>
Cc Alexei Starovoitov <[email protected]>
Cc Daniel Borkmann <[email protected]>
Cc Andrii Nakryiko <[email protected]>
Cc Martin KaFai Lau <[email protected]>
Cc Eduard Zingerman <[email protected]>
Cc Yonghong Song <[email protected]>
Cc John Fastabend <[email protected]>
Cc Stanislav Fomichev <[email protected]>
Cc Hao Luo <[email protected]>
Cc Andrew Morton <[email protected]>

---
Changes in v16:
- Rename HAVE_PT_REGS_TO_FTRACE_REGS_CAST to HAVE_FTRACE_REGS_HAVING_PT_REGS.
Changes in v3:
- Use ftrace_regs_get_return_value()
Changes from previous series: NOTHING, just forward ported.

---
 arch/loongarch/Kconfig          |    1 +
 arch/s390/Kconfig               |    1 +
 arch/x86/Kconfig                |    1 +
 include/linux/fprobe.h          |    2 +-
 include/linux/ftrace.h          |    6 ++++++
 kernel/trace/Kconfig            |    7 +++++++
 kernel/trace/bpf_trace.c        |    6 +++++-
 kernel/trace/fprobe.c           |    3 ++-
 kernel/trace/trace_fprobe.c     |    6 +++++-
 lib/test_fprobe.c               |    6 +++---
 samples/fprobe/fprobe_example.c |    2 +-
 11 files changed, 33 insertions(+), 8 deletions(-)
arch/loongarch/Kconfig +1 -0
index 49f5bfc00e5a..6396615ec035 100644
@@ -128,6 +128,7 @@ config LOONGARCH
 	select HAVE_DMA_CONTIGUOUS
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_DYNAMIC_FTRACE_WITH_ARGS
+	select HAVE_FTRACE_REGS_HAVING_PT_REGS
 	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS
 	select HAVE_EBPF_JIT
arch/s390/Kconfig +1 -0
index 102029e56cf0..d8eee56c10b6 100644
@@ -183,6 +183,7 @@ config S390
 	select HAVE_DMA_CONTIGUOUS
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_DYNAMIC_FTRACE_WITH_ARGS
+	select HAVE_FTRACE_REGS_HAVING_PT_REGS
 	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS
 	select HAVE_EBPF_JIT if HAVE_MARCH_Z196_FEATURES
arch/x86/Kconfig +1 -0
index ff0d7e07c611..6cb420783ef3 100644
@@ -225,6 +225,7 @@ config X86
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS
 	select HAVE_DYNAMIC_FTRACE_WITH_ARGS	if X86_64
+	select HAVE_FTRACE_REGS_HAVING_PT_REGS	if X86_64
 	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 	select HAVE_SAMPLE_FTRACE_DIRECT	if X86_64
 	select HAVE_SAMPLE_FTRACE_DIRECT_MULTI	if X86_64
include/linux/fprobe.h +1 -1
index ca64ee5e45d2..ef609bcca0f9 100644
@@ -14,7 +14,7 @@ typedef int (*fprobe_entry_cb)(struct fprobe *fp, unsigned long entry_ip,
 			       void *entry_data);
 
 typedef void (*fprobe_exit_cb)(struct fprobe *fp, unsigned long entry_ip,
-			       unsigned long ret_ip, struct pt_regs *regs,
+			       unsigned long ret_ip, struct ftrace_regs *regs,
 			       void *entry_data);
 
 /**
include/linux/ftrace.h +6 -0
index 9a1e768e47da..bf8bb6c10553 100644
@@ -176,6 +176,12 @@ static inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *fregs)
 #define ftrace_regs_set_instruction_pointer(fregs, ip) do { } while (0)
 #endif /* CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */
 
+#ifdef CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS
+
+static_assert(sizeof(struct pt_regs) == ftrace_regs_size());
+
+#endif /* CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS */
+
 static __always_inline struct pt_regs *ftrace_get_regs(struct ftrace_regs *fregs)
 {
 	if (!fregs)
kernel/trace/Kconfig +7 -0
index f10ca86fbfad..7f8165f2049a 100644
@@ -57,6 +57,12 @@ config HAVE_DYNAMIC_FTRACE_WITH_ARGS
 	 This allows for use of ftrace_regs_get_argument() and
 	 ftrace_regs_get_stack_pointer().
 
+config HAVE_FTRACE_REGS_HAVING_PT_REGS
+	bool
+	help
+	 If this is set, ftrace_regs has pt_regs, thus it can convert to
+	 pt_regs without allocating memory.
+
 config HAVE_DYNAMIC_FTRACE_NO_PATCHABLE
 	bool
 	help
@@ -298,6 +304,7 @@ config FPROBE
 	bool "Kernel Function Probe (fprobe)"
 	depends on FUNCTION_TRACER
 	depends on DYNAMIC_FTRACE_WITH_REGS || DYNAMIC_FTRACE_WITH_ARGS
+	depends on HAVE_FTRACE_REGS_HAVING_PT_REGS || !HAVE_DYNAMIC_FTRACE_WITH_ARGS
 	depends on HAVE_RETHOOK
 	select RETHOOK
 	default n
kernel/trace/bpf_trace.c +5 -1
index 7bb2e6ecd31f..e469fcbed210 100644
@@ -2830,10 +2830,14 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
 
 static void
 kprobe_multi_link_exit_handler(struct fprobe *fp, unsigned long fentry_ip,
-			       unsigned long ret_ip, struct pt_regs *regs,
+			       unsigned long ret_ip, struct ftrace_regs *fregs,
 			       void *data)
 {
 	struct bpf_kprobe_multi_link *link;
+	struct pt_regs *regs = ftrace_get_regs(fregs);
+
+	if (!regs)
+		return;
 
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
 	kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs, true, data);
kernel/trace/fprobe.c +2 -1
index 3d3789283873..90a3c8e2bbdf 100644
@@ -124,6 +124,7 @@ static void fprobe_exit_handler(struct rethook_node *rh, void *data,
 {
 	struct fprobe *fp = (struct fprobe *)data;
 	struct fprobe_rethook_node *fpr;
+	struct ftrace_regs *fregs = (struct ftrace_regs *)regs;
 	int bit;
 
 	if (!fp || fprobe_disabled(fp))
@@ -141,7 +142,7 @@ static void fprobe_exit_handler(struct rethook_node *rh, void *data,
 		return;
 	}
 
-	fp->exit_handler(fp, fpr->entry_ip, ret_ip, regs,
+	fp->exit_handler(fp, fpr->entry_ip, ret_ip, fregs,
 			 fp->entry_data_size ? (void *)fpr->data : NULL);
 	ftrace_test_recursion_unlock(bit);
 }
kernel/trace/trace_fprobe.c +5 -1
index 0f254685e26a..ed49d21269cf 100644
@@ -361,10 +361,14 @@ static int fentry_dispatcher(struct fprobe *fp, unsigned long entry_ip,
 NOKPROBE_SYMBOL(fentry_dispatcher);
 
 static void fexit_dispatcher(struct fprobe *fp, unsigned long entry_ip,
-			     unsigned long ret_ip, struct pt_regs *regs,
+			     unsigned long ret_ip, struct ftrace_regs *fregs,
 			     void *entry_data)
 {
 	struct trace_fprobe *tf = container_of(fp, struct trace_fprobe, fp);
+	struct pt_regs *regs = ftrace_get_regs(fregs);
+
+	if (!regs)
+		return;
 
 	if (trace_probe_test_flag(&tf->tp, TP_FLAG_TRACE))
 		fexit_trace_func(tf, entry_ip, ret_ip, regs, entry_data);
lib/test_fprobe.c +3 -3
index ff607babba18..271ce0caeec0 100644
@@ -59,9 +59,9 @@ static notrace int fp_entry_handler(struct fprobe *fp, unsigned long ip,
 
 static notrace void fp_exit_handler(struct fprobe *fp, unsigned long ip,
 				    unsigned long ret_ip,
-				    struct pt_regs *regs, void *data)
+				    struct ftrace_regs *fregs, void *data)
 {
-	unsigned long ret = regs_return_value(regs);
+	unsigned long ret = ftrace_regs_get_return_value(fregs);
 
 	KUNIT_EXPECT_FALSE(current_test, preemptible());
 	if (ip != target_ip) {
@@ -89,7 +89,7 @@ static notrace int nest_entry_handler(struct fprobe *fp, unsigned long ip,
 
 static notrace void nest_exit_handler(struct fprobe *fp, unsigned long ip,
 				      unsigned long ret_ip,
-				      struct pt_regs *regs, void *data)
+				      struct ftrace_regs *fregs, void *data)
 {
 	KUNIT_EXPECT_FALSE(current_test, preemptible());
 	KUNIT_EXPECT_EQ(current_test, ip, target_nest_ip);
samples/fprobe/fprobe_example.c +1 -1
index c234afae52d6..bfe98ce826f3 100644
@@ -67,7 +67,7 @@ static int sample_entry_handler(struct fprobe *fp, unsigned long ip,
 }
 
 static void sample_exit_handler(struct fprobe *fp, unsigned long ip,
-				unsigned long ret_ip, struct pt_regs *regs,
+				unsigned long ret_ip, struct ftrace_regs *regs,
 				void *data)
 {
 	unsigned long rip = ret_ip;
author maintainer
#7 lore raw

PATCH v22 7/20 tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Add ftrace_partial_regs() which converts the ftrace_regs to pt_regs. This is for the eBPF which needs this to keep the same pt_regs interface to access registers.
Thus when replacing the pt_regs with ftrace_regs in fprobes (which is used by kprobe_multi eBPF event), this will be used.

If the architecture defines its own ftrace_regs, this copies partial registers to pt_regs and returns it. If not, ftrace_regs is the same as pt_regs and ftrace_partial_regs() will return ftrace_regs::regs.

Cc Steven Rostedt <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Albert Ou <[email protected]>

---
Changes in v18:
- Fix to use sizeof() for calculating array size.
Changes in v14:
- Add riscv change.
Changes in v8:
- Add the reason why this required in changelog.
Changes from previous series: NOTHING, just forward ported.

---
 arch/arm64/include/asm/ftrace.h |   13 +++++++++++++
 arch/riscv/include/asm/ftrace.h |   14 ++++++++++++++
 include/linux/ftrace.h          |   17 +++++++++++++++++
 3 files changed, 44 insertions(+)
arch/arm64/include/asm/ftrace.h +13 -0
index b5fa57b61378..09210f853f12 100644
@@ -135,6 +135,19 @@ ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs)
 	return arch_ftrace_regs(fregs)->fp;
 }
 
+static __always_inline struct pt_regs *
+ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs)
+{
+	struct __arch_ftrace_regs *afregs = arch_ftrace_regs(fregs);
+
+	memcpy(regs->regs, afregs->regs, sizeof(afregs->regs));
+	regs->sp = afregs->sp;
+	regs->pc = afregs->pc;
+	regs->regs[29] = afregs->fp;
+	regs->regs[30] = afregs->lr;
+	return regs;
+}
+
 int ftrace_regs_query_register_offset(const char *name);
 
 int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec);
arch/riscv/include/asm/ftrace.h +14 -0
index 9372f8d7036f..7064a530794b 100644
@@ -197,6 +197,20 @@ static __always_inline void ftrace_override_function_with_return(struct ftrace_r
 	arch_ftrace_regs(fregs)->epc = arch_ftrace_regs(fregs)->ra;
 }
 
+static __always_inline struct pt_regs *
+ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs)
+{
+	struct __arch_ftrace_regs *afregs = arch_ftrace_regs(fregs);
+
+	memcpy(&regs->a0, afregs->args, sizeof(afregs->args));
+	regs->epc = afregs->epc;
+	regs->ra = afregs->ra;
+	regs->sp = afregs->sp;
+	regs->s0 = afregs->s0;
+	regs->t1 = afregs->t1;
+	return regs;
+}
+
 int ftrace_regs_query_register_offset(const char *name);
 
 void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
include/linux/ftrace.h +17 -0
index bf8bb6c10553..ad2b46e1d5b0 100644
@@ -190,6 +190,23 @@ static __always_inline struct pt_regs *ftrace_get_regs(struct ftrace_regs *fregs
 	return arch_ftrace_get_regs(fregs);
 }
 
+#if !defined(CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS) || \
+	defined(CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS)
+
+static __always_inline struct pt_regs *
+ftrace_partial_regs(struct ftrace_regs *fregs, struct pt_regs *regs)
+{
+	/*
+	 * If CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS=y, ftrace_regs memory
+	 * layout is including pt_regs. So always returns that address.
+	 * Since arch_ftrace_get_regs() will check some members and may return
+	 * NULL, we can not use it.
+	 */
+	return &arch_ftrace_regs(fregs)->regs;
+}
+
+#endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS || CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS */
+
 /*
  * When true, the ftrace_regs_{get,set}_*() functions may be used on fregs.
  * Note: this can be true even when ftrace_get_regs() cannot provide a pt_regs.
author maintainer
#8 lore raw

PATCH v22 8/20 tracing: Add ftrace_fill_perf_regs() for perf event

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Add ftrace_fill_perf_regs() which should be compatible with the perf_fetch_caller_regs(). In other words, the pt_regs returned from the ftrace_fill_perf_regs() must satisfy 'user_mode(regs) == false' and can be used for stack tracing.

Cc Steven Rostedt <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Michael Ellerman <[email protected]>
Cc Nicholas Piggin <[email protected]>
Cc Christophe Leroy <[email protected]>
Cc Naveen N Rao <[email protected]>
Cc Madhavan Srinivasan <[email protected]>
Cc Vasily Gorbik <[email protected]>
Cc Alexander Gordeev <[email protected]>
Cc Christian Borntraeger <[email protected]>
Cc Sven Schnelle <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>

---
Changes in v16:
- Fix s390 to clear psw.mask according to Heiko's suggestion.

---
 arch/arm64/include/asm/ftrace.h   |    7 +++++++
 arch/powerpc/include/asm/ftrace.h |    7 +++++++
 arch/s390/include/asm/ftrace.h    |    6 ++++++
 arch/x86/include/asm/ftrace.h     |    7 +++++++
 include/linux/ftrace.h            |   31 +++++++++++++++++++++++++++++++
 5 files changed, 58 insertions(+)
arch/arm64/include/asm/ftrace.h +7 -0
index 09210f853f12..10e56522122a 100644
@@ -148,6 +148,13 @@ ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs)
 	return regs;
 }
 
+#define arch_ftrace_fill_perf_regs(fregs, _regs) do {		\
+		(_regs)->pc = arch_ftrace_regs(fregs)->pc;			\
+		(_regs)->regs[29] = arch_ftrace_regs(fregs)->fp;		\
+		(_regs)->sp = arch_ftrace_regs(fregs)->sp;			\
+		(_regs)->pstate = PSR_MODE_EL1h;		\
+	} while (0)
+
 int ftrace_regs_query_register_offset(const char *name);
 
 int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec);
arch/powerpc/include/asm/ftrace.h +7 -0
index db481b336bca..fe181bafdca4 100644
@@ -43,6 +43,13 @@ static __always_inline struct pt_regs *arch_ftrace_get_regs(struct ftrace_regs *
 	return arch_ftrace_regs(fregs)->regs.msr ? &arch_ftrace_regs(fregs)->regs : NULL;
 }
 
+#define arch_ftrace_fill_perf_regs(fregs, _regs) do {		\
+		(_regs)->result = 0;				\
+		(_regs)->nip = arch_ftrace_regs(fregs)->regs.nip;		\
+		(_regs)->gpr[1] = arch_ftrace_regs(fregs)->regs.gpr[1];		\
+		asm volatile("mfmsr %0" : "=r" ((_regs)->msr));	\
+	} while (0)
+
 static __always_inline void
 ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
 				    unsigned long ip)
arch/s390/include/asm/ftrace.h +6 -0
index 5c94c1fc1bc1..5b7cb49c41ee 100644
@@ -76,6 +76,12 @@ ftrace_regs_get_frame_pointer(struct ftrace_regs *fregs)
 	return ftrace_regs_get_stack_pointer(fregs);
 }
 
+#define arch_ftrace_fill_perf_regs(fregs, _regs)	 do {		\
+		(_regs)->psw.mask = 0;					\
+		(_regs)->psw.addr = arch_ftrace_regs(fregs)->regs.psw.addr;		\
+		(_regs)->gprs[15] = arch_ftrace_regs(fregs)->regs.gprs[15];		\
+	} while (0)
+
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 /*
  * When an ftrace registered caller is tracing a function that is
arch/x86/include/asm/ftrace.h +7 -0
index d61407c680c2..7e06f8c7937a 100644
@@ -47,6 +47,13 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)
 	return &arch_ftrace_regs(fregs)->regs;
 }
 
+#define arch_ftrace_fill_perf_regs(fregs, _regs) do {	\
+		(_regs)->ip = arch_ftrace_regs(fregs)->regs.ip;		\
+		(_regs)->sp = arch_ftrace_regs(fregs)->regs.sp;		\
+		(_regs)->cs = __KERNEL_CS;		\
+		(_regs)->flags = 0;			\
+	} while (0)
+
 #define ftrace_regs_set_instruction_pointer(fregs, _ip)	\
 	do { arch_ftrace_regs(fregs)->regs.ip = (_ip); } while (0)
 
include/linux/ftrace.h +31 -0
index ad2b46e1d5b0..6d29c640697c 100644
@@ -207,6 +207,37 @@ ftrace_partial_regs(struct ftrace_regs *fregs, struct pt_regs *regs)
 
 #endif /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS || CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS */
 
+#ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
+
+/*
+ * Please define arch dependent pt_regs which compatible to the
+ * perf_arch_fetch_caller_regs() but based on ftrace_regs.
+ * This requires
+ *   - user_mode(_regs) returns false (always kernel mode).
+ *   - able to use the _regs for stack trace.
+ */
+#ifndef arch_ftrace_fill_perf_regs
+/* As same as perf_arch_fetch_caller_regs(), do nothing by default */
+#define arch_ftrace_fill_perf_regs(fregs, _regs) do {} while (0)
+#endif
+
+static __always_inline struct pt_regs *
+ftrace_fill_perf_regs(struct ftrace_regs *fregs, struct pt_regs *regs)
+{
+	arch_ftrace_fill_perf_regs(fregs, regs);
+	return regs;
+}
+
+#else /* !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS */
+
+static __always_inline struct pt_regs *
+ftrace_fill_perf_regs(struct ftrace_regs *fregs, struct pt_regs *regs)
+{
+	return &arch_ftrace_regs(fregs)->regs;
+}
+
+#endif
+
 /*
  * When true, the ftrace_regs_{get,set}_*() functions may be used on fregs.
  * Note: this can be true even when ftrace_get_regs() cannot provide a pt_regs.
author maintainer
#9 lore raw

PATCH v22 9/20 tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Allow fprobe events to be enabled with CONFIG_DYNAMIC_FTRACE_WITH_ARGS. With this change, fprobe events mostly use ftrace_regs instead of pt_regs. Note that if the arch doesn't enable HAVE_FTRACE_REGS_HAVING_PT_REGS, fprobe events will not be able to be used from perf.

---
Changes in v9:
- Copy store_trace_entry_data() as store_fprobe_entry_data() for fprobe.
Chagnes in v3:
- Use ftrace_regs_get_return_value().
Changes in v2:
- Define ftrace_regs_get_kernel_stack_nth() for !CONFIG_HAVE_REGS_AND_STACK_ACCESS_API.
Changes from previous series: Update against the new series.

---
 include/linux/ftrace.h          |   17 ++++++
 kernel/trace/Kconfig            |    1 
 kernel/trace/trace_fprobe.c     |  108 +++++++++++++++++++++++++--------------
 kernel/trace/trace_probe_tmpl.h |    2 -
 4 files changed, 86 insertions(+), 42 deletions(-)
include/linux/ftrace.h +17 -0
index 6d29c640697c..4c553fe9c026 100644
@@ -250,6 +250,23 @@ static __always_inline bool ftrace_regs_has_args(struct ftrace_regs *fregs)
 	return ftrace_get_regs(fregs) != NULL;
 }
 
+#ifdef CONFIG_HAVE_REGS_AND_STACK_ACCESS_API
+static __always_inline unsigned long
+ftrace_regs_get_kernel_stack_nth(struct ftrace_regs *fregs, unsigned int nth)
+{
+	unsigned long *stackp;
+
+	stackp = (unsigned long *)ftrace_regs_get_stack_pointer(fregs);
+	if (((unsigned long)(stackp + nth) & ~(THREAD_SIZE - 1)) ==
+	    ((unsigned long)stackp & ~(THREAD_SIZE - 1)))
+		return *(stackp + nth);
+
+	return 0;
+}
+#else /* !CONFIG_HAVE_REGS_AND_STACK_ACCESS_API */
+#define ftrace_regs_get_kernel_stack_nth(fregs, nth)	(0L)
+#endif /* CONFIG_HAVE_REGS_AND_STACK_ACCESS_API */
+
 typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
 			      struct ftrace_ops *op, struct ftrace_regs *fregs);
 
kernel/trace/Kconfig +0 -1
index 7f8165f2049a..82654bbfad9a 100644
@@ -689,7 +689,6 @@ config FPROBE_EVENTS
 	select TRACING
 	select PROBE_EVENTS
 	select DYNAMIC_EVENTS
-	depends on DYNAMIC_FTRACE_WITH_REGS
 	default y
 	help
 	  This allows user to add tracing events on the function entry and
kernel/trace/trace_fprobe.c +68 -40

272 lines not on this page · raw message

kernel/trace/trace_probe_tmpl.h +1 -1
index 2caf0d2afb32..f39b37fcdb3b 100644
@@ -232,7 +232,7 @@ process_fetch_insn_bottom(struct fetch_insn *code, unsigned long val,
 
 /* Sum up total data length for dynamic arrays (strings) */
 static nokprobe_inline int
-__get_data_size(struct trace_probe *tp, struct pt_regs *regs, void *edata)
+__get_data_size(struct trace_probe *tp, void *regs, void *edata)
 {
 	struct probe_arg *arg;
 	int i, len, ret = 0;
author maintainer
#10 lore raw

PATCH v22 10/20 bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Enable kprobe_multi feature if CONFIG_FPROBE is enabled. The pt_regs is converted from ftrace_regs by ftrace_partial_regs(), thus some registers may always returns 0. But it should be enough for function entry (access arguments) and exit (access return value).

---
Changes in v9:
- Avoid wasting memory for bpf_kprobe_multi_pt_regs when CONFIG_HAVE_PT_REGS_TO_FTRACE_REGS_CAST=y

---
 kernel/trace/bpf_trace.c |   27 ++++++++++++++-------------
 1 file changed, 14 insertions(+), 13 deletions(-)
kernel/trace/bpf_trace.c +14 -13
index e469fcbed210..863351559334 100644
@@ -2561,7 +2561,7 @@ struct bpf_session_run_ctx {
 	void *data;
 };
 
-#if defined(CONFIG_FPROBE) && defined(CONFIG_DYNAMIC_FTRACE_WITH_REGS)
+#ifdef CONFIG_FPROBE
 struct bpf_kprobe_multi_link {
 	struct bpf_link link;
 	struct fprobe fp;
@@ -2584,6 +2584,13 @@ struct user_syms {
 	char *buf;
 };
 
+#ifndef CONFIG_HAVE_FTRACE_REGS_HAVING_PT_REGS
+static DEFINE_PER_CPU(struct pt_regs, bpf_kprobe_multi_pt_regs);
+#define bpf_kprobe_multi_pt_regs_ptr()	this_cpu_ptr(&bpf_kprobe_multi_pt_regs)
+#else
+#define bpf_kprobe_multi_pt_regs_ptr()	(NULL)
+#endif
+
 static int copy_user_syms(struct user_syms *us, unsigned long __user *usyms, u32 cnt)
 {
 	unsigned long __user usymbol;
@@ -2778,7 +2785,7 @@ static u64 bpf_kprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
 
 static int
 kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
-			   unsigned long entry_ip, struct pt_regs *regs,
+			   unsigned long entry_ip, struct ftrace_regs *fregs,
 			   bool is_return, void *data)
 {
 	struct bpf_kprobe_multi_run_ctx run_ctx = {
@@ -2790,6 +2797,7 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
 		.entry_ip = entry_ip,
 	};
 	struct bpf_run_ctx *old_run_ctx;
+	struct pt_regs *regs;
 	int err;
 
 	if (unlikely(__this_cpu_inc_return(bpf_prog_active) != 1)) {
@@ -2800,6 +2808,7 @@ kprobe_multi_link_prog_run(struct bpf_kprobe_multi_link *link,
 
 	migrate_disable();
 	rcu_read_lock();
+	regs = ftrace_partial_regs(fregs, bpf_kprobe_multi_pt_regs_ptr());
 	old_run_ctx = bpf_set_run_ctx(&run_ctx.session_ctx.run_ctx);
 	err = bpf_prog_run(link->link.prog, regs);
 	bpf_reset_run_ctx(old_run_ctx);
@@ -2816,15 +2825,11 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
 			  unsigned long ret_ip, struct ftrace_regs *fregs,
 			  void *data)
 {
-	struct pt_regs *regs = ftrace_get_regs(fregs);
 	struct bpf_kprobe_multi_link *link;
 	int err;
 
-	if (!regs)
-		return 0;
-
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
-	err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs, false, data);
+	err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), fregs, false, data);
 	return is_kprobe_session(link->link.prog) ? err : 0;
 }
 
@@ -2834,13 +2839,9 @@ kprobe_multi_link_exit_handler(struct fprobe *fp, unsigned long fentry_ip,
 			       void *data)
 {
 	struct bpf_kprobe_multi_link *link;
-	struct pt_regs *regs = ftrace_get_regs(fregs);
-
-	if (!regs)
-		return;
 
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
-	kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), regs, true, data);
+	kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), fregs, true, data);
 }
 
 static int symbols_cmp_r(const void *a, const void *b, const void *priv)
@@ -3101,7 +3102,7 @@ int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *pr
 	kvfree(cookies);
 	return err;
 }
-#else /* !CONFIG_FPROBE || !CONFIG_DYNAMIC_FTRACE_WITH_REGS */
+#else /* !CONFIG_FPROBE */
 int bpf_kprobe_multi_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
 {
 	return -EOPNOTSUPP;
author maintainer
#11 lore raw

PATCH v22 11/20 ftrace: Add CONFIG_HAVE_FTRACE_GRAPH_FUNC

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Add CONFIG_HAVE_FTRACE_GRAPH_FUNC kconfig in addition to ftrace_graph_func macro check. This is for the other feature (e.g. FPROBE) which requires to access ftrace_regs from fgraph_ops::entryfunc() can avoid compiling if the fgraph can not pass the valid ftrace_regs.

Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Huacai Chen <[email protected]>
Cc WANG Xuerui <[email protected]>
Cc Michael Ellerman <[email protected]>
Cc Nicholas Piggin <[email protected]>
Cc Christophe Leroy <[email protected]>
Cc Naveen N Rao <[email protected]>
Cc Madhavan Srinivasan <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Albert Ou <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Mathieu Desnoyers <[email protected]>

---
Changes in v8:
- Newly added.

---
 arch/arm64/Kconfig     |    1 +
 arch/loongarch/Kconfig |    1 +
 arch/powerpc/Kconfig   |    1 +
 arch/riscv/Kconfig     |    1 +
 arch/x86/Kconfig       |    1 +
 kernel/trace/Kconfig   |    5 +++++
 6 files changed, 10 insertions(+)
arch/arm64/Kconfig +1 -0
index 5f086777dad9..a8644a5af9fb 100644
@@ -216,6 +216,7 @@ config ARM64
 	select HAVE_SAMPLE_FTRACE_DIRECT_MULTI
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS
 	select HAVE_GUP_FAST
+	select HAVE_FTRACE_GRAPH_FUNC
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_TRACER
 	select HAVE_FUNCTION_ERROR_INJECTION
arch/loongarch/Kconfig +1 -0
index 6396615ec035..fe0d9e549ca9 100644
@@ -135,6 +135,7 @@ config LOONGARCH
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS if !ARCH_STRICT_ALIGN
 	select HAVE_EXIT_THREAD
 	select HAVE_GUP_FAST
+	select HAVE_FTRACE_GRAPH_FUNC
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_ARG_ACCESS_API
 	select HAVE_FUNCTION_ERROR_INJECTION
arch/powerpc/Kconfig +1 -0
index a0ce777f9706..c28349ad1ac2 100644
@@ -240,6 +240,7 @@ config PPC
 	select HAVE_EBPF_JIT
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS
 	select HAVE_GUP_FAST
+	select HAVE_FTRACE_GRAPH_FUNC
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_ARG_ACCESS_API
 	select HAVE_FUNCTION_DESCRIPTORS	if PPC64_ELF_ABI_V1
arch/riscv/Kconfig +1 -0
index 1e807c61258f..c736e349f222 100644
@@ -146,6 +146,7 @@ config RISCV
 	select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE)
 	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 	select HAVE_DYNAMIC_FTRACE_WITH_ARGS if HAVE_DYNAMIC_FTRACE
+	select HAVE_FTRACE_GRAPH_FUNC
 	select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_GRAPH_FREGS
arch/x86/Kconfig +1 -0
index 6cb420783ef3..db435d159c1b 100644
@@ -235,6 +235,7 @@ config X86
 	select HAVE_EXIT_THREAD
 	select HAVE_GUP_FAST
 	select HAVE_FENTRY			if X86_64 || DYNAMIC_FTRACE
+	select HAVE_FTRACE_GRAPH_FUNC		if HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_GRAPH_FREGS	if HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_GRAPH_TRACER	if X86_32 || (X86_64 && DYNAMIC_FTRACE)
kernel/trace/Kconfig +5 -0
index 82654bbfad9a..2fc55a1a88aa 100644
@@ -34,6 +34,11 @@ config HAVE_FUNCTION_GRAPH_TRACER
 config HAVE_FUNCTION_GRAPH_FREGS
 	bool
 
+config HAVE_FTRACE_GRAPH_FUNC
+	bool
+	help
+	  True if ftrace_graph_func() is defined.
+
 config HAVE_DYNAMIC_FTRACE
 	bool
 	help
author maintainer
#12 lore raw

PATCH v22 12/20 s390/tracing: Enable HAVE_FTRACE_GRAPH_FUNC

In reply to Masami Hiramatsu (Google)

From: Sven Schnelle <[email protected]>

Add ftrace_graph_func() which is required for fprobe to access registers. This also eliminates the need for calling prepare_ftrace_return() from ftrace_caller().

---
Changes in v19:
- Newly added.

---
 arch/s390/Kconfig              |    1 +
 arch/s390/include/asm/ftrace.h |    5 ++++
 arch/s390/kernel/entry.h       |    1 -
 arch/s390/kernel/ftrace.c      |   48 ++++++++++++----------------------------
 arch/s390/kernel/mcount.S      |   11 ---------
 5 files changed, 20 insertions(+), 46 deletions(-)
arch/s390/Kconfig +1 -0
index d8eee56c10b6..622ea2e9a87e 100644
@@ -190,6 +190,7 @@ config S390
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS
 	select HAVE_GUP_FAST
 	select HAVE_FENTRY
+	select HAVE_FTRACE_GRAPH_FUNC
 	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_FUNCTION_ARG_ACCESS_API
 	select HAVE_FUNCTION_ERROR_INJECTION
arch/s390/include/asm/ftrace.h +5 -0
index 5b7cb49c41ee..fd3f0fe9f7b3 100644
@@ -39,6 +39,7 @@ struct dyn_arch_ftrace { };
 
 struct module;
 struct dyn_ftrace;
+struct ftrace_ops;
 
 bool ftrace_need_init_nop(void);
 #define ftrace_need_init_nop ftrace_need_init_nop
@@ -122,6 +123,10 @@ static inline bool arch_syscall_match_sym_name(const char *sym,
 	return !strcmp(sym + 7, name) || !strcmp(sym + 8, name);
 }
 
+void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
+		       struct ftrace_ops *op, struct ftrace_regs *fregs);
+#define ftrace_graph_func ftrace_graph_func
+
 #endif /* __ASSEMBLY__ */
 
 #ifdef CONFIG_FUNCTION_TRACER
arch/s390/kernel/entry.h +0 -1
index 21969520f947..a1f28879c87e 100644
@@ -41,7 +41,6 @@ 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);
 
 struct s390_mmap_arg_struct;
 struct fadvise64_64_args;
arch/s390/kernel/ftrace.c +14 -34

57 lines not on this page · raw message

arch/s390/kernel/mcount.S +0 -11
index 2b628aa3d809..1fec370fecf4 100644
@@ -104,17 +104,6 @@ SYM_CODE_START(ftrace_common)
 	lgr	%r3,%r14
 	la	%r5,STACK_FREGS(%r15)
 	BASR_EX	%r14,%r1
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-# The j instruction gets runtime patched to a nop instruction.
-# See ftrace_enable_ftrace_graph_caller.
-SYM_INNER_LABEL(ftrace_graph_caller, SYM_L_GLOBAL)
-	j	.Lftrace_graph_caller_end
-	lmg	%r2,%r3,(STACK_FREGS_PTREGS_GPRS+14*8)(%r15)
-	lg	%r4,(STACK_FREGS_PTREGS_PSW+8)(%r15)
-	brasl	%r14,prepare_ftrace_return
-	stg	%r2,(STACK_FREGS_PTREGS_GPRS+14*8)(%r15)
-.Lftrace_graph_caller_end:
-#endif
 	lg	%r0,(STACK_FREGS_PTREGS_PSW+8)(%r15)
 #ifdef MARCH_HAS_Z196_FEATURES
 	ltg	%r1,STACK_FREGS_PTREGS_ORIG_GPR2(%r15)
author maintainer
#13 lore raw

PATCH v22 13/20 fprobe: Rewrite fprobe on function-graph tracer

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Rewrite fprobe implementation on function-graph tracer. Major API changes are:
- 'nr_maxactive' field is deprecated.
- This depends on CONFIG_DYNAMIC_FTRACE_WITH_ARGS or !CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS, and CONFIG_HAVE_FUNCTION_GRAPH_FREGS. So currently works only on x86_64.
- Currently the entry size is limited in 15 * sizeof(long).
- If there is too many fprobe exit handler set on the same function, it will fail to probe.

Cc Steven Rostedt <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Huacai Chen <[email protected]>
Cc WANG Xuerui <[email protected]>
Cc Michael Ellerman <[email protected]>
Cc Nicholas Piggin <[email protected]>
Cc Christophe Leroy <[email protected]>
Cc Naveen N Rao <[email protected]>
Cc Madhavan Srinivasan <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Albert Ou <[email protected]>
Cc Vasily Gorbik <[email protected]>
Cc Alexander Gordeev <[email protected]>
Cc Christian Borntraeger <[email protected]>
Cc Sven Schnelle <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Mathieu Desnoyers <[email protected]>
Cc Andrew Morton <[email protected]>

---
Changes in v20:
- Make CONFIG_FPROBE "default n" so that it does NOT enable function graph tracer enabled by default.
Changes in v19:
- Fix comments on struct fprobe_hlist_node.
- Add __counted_by() to struct fprobe_hlist::array.
Changes in v18:
- Use raw fprobe address and data size instead of encoding data size in fprobe header value.
- Instead, allow arch to define its own encode/decode fprobe header.
- Fix kconfig dependency issue to depend on DYNAMIC_FTRACE_WITH_ARGS.
Changes in v14:
- Add ftrace_regs_get_return_addresss() for riscv.
Changes in v12:
- Skip updating ftrace hash if not required.
Changes in v9:
- Remove unneeded prototype of ftrace_regs_get_return_address().
- Fix entry data address calculation.
- Remove DIV_ROUND_UP() from hotpath.
Changes in v8:
- Use trace_func_graph_ret/ent_t for fgraph_ops.
- Update CONFIG_FPROBE dependencies.
- Add ftrace_regs_get_return_address() for each arch.
Changes in v3:
- Update for new reserve_data/retrieve_data API.
- Fix internal push/pop on fgraph data logic so that it can correctly save/restore the returning fprobes.
Changes in v2:
- Add more lockdep_assert_held(fprobe_mutex)
- Use READ_ONCE() and WRITE_ONCE() for fprobe_hlist_node::fp.
- Add NOKPROBE_SYMBOL() for the functions which is called from entry/exit callback.

---
 arch/arm64/include/asm/ftrace.h     |    6 
 arch/loongarch/include/asm/ftrace.h |    6 
 arch/powerpc/include/asm/ftrace.h   |    6 
 arch/riscv/include/asm/ftrace.h     |    5 
 arch/s390/include/asm/ftrace.h      |    6 
 arch/x86/include/asm/ftrace.h       |    6 
 include/linux/fprobe.h              |   58 ++-
 kernel/trace/Kconfig                |    8 
 kernel/trace/fprobe.c               |  637 +++++++++++++++++++++++++----------
 lib/test_fprobe.c                   |   45 --
 10 files changed, 538 insertions(+), 245 deletions(-)
arch/arm64/include/asm/ftrace.h +6 -0
index 10e56522122a..876e88ad4119 100644
@@ -135,6 +135,12 @@ ftrace_regs_get_frame_pointer(const struct ftrace_regs *fregs)
 	return arch_ftrace_regs(fregs)->fp;
 }
 
+static __always_inline unsigned long
+ftrace_regs_get_return_address(const struct ftrace_regs *fregs)
+{
+	return arch_ftrace_regs(fregs)->lr;
+}
+
 static __always_inline struct pt_regs *
 ftrace_partial_regs(const struct ftrace_regs *fregs, struct pt_regs *regs)
 {
arch/loongarch/include/asm/ftrace.h +6 -0
index ceb3e3d9c0d3..6e0a99763a9a 100644
@@ -61,6 +61,12 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs, unsigned long ip)
 #define ftrace_regs_get_frame_pointer(fregs) \
 	(arch_ftrace_regs(fregs)->regs.regs[22])
 
+static __always_inline unsigned long
+ftrace_regs_get_return_address(struct ftrace_regs *fregs)
+{
+	return *(unsigned long *)(arch_ftrace_regs(fregs)->regs.regs[1]);
+}
+
 #define ftrace_graph_func ftrace_graph_func
 void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
 		       struct ftrace_ops *op, struct ftrace_regs *fregs);
arch/powerpc/include/asm/ftrace.h +6 -0
index fe181bafdca4..82da7c7a1d12 100644
@@ -57,6 +57,12 @@ ftrace_regs_set_instruction_pointer(struct ftrace_regs *fregs,
 	regs_set_return_ip(&arch_ftrace_regs(fregs)->regs, ip);
 }
 
+static __always_inline unsigned long
+ftrace_regs_get_return_address(struct ftrace_regs *fregs)
+{
+	return arch_ftrace_regs(fregs)->regs.link;
+}
+
 struct ftrace_ops;
 
 #define ftrace_graph_func ftrace_graph_func
arch/riscv/include/asm/ftrace.h +5 -0
index 7064a530794b..c4721ce44ca4 100644
@@ -186,6 +186,11 @@ static __always_inline unsigned long ftrace_regs_get_return_value(const struct f
 	return arch_ftrace_regs(fregs)->a0;
 }
 
+static __always_inline unsigned long ftrace_regs_get_return_address(const struct ftrace_regs *fregs)
+{
+	return arch_ftrace_regs(fregs)->ra;
+}
+
 static __always_inline void ftrace_regs_set_return_value(struct ftrace_regs *fregs,
 							 unsigned long ret)
 {
arch/s390/include/asm/ftrace.h +6 -0
index fd3f0fe9f7b3..a3b73a4f626e 100644
@@ -77,6 +77,12 @@ ftrace_regs_get_frame_pointer(struct ftrace_regs *fregs)
 	return ftrace_regs_get_stack_pointer(fregs);
 }
 
+static __always_inline unsigned long
+ftrace_regs_get_return_address(const struct ftrace_regs *fregs)
+{
+	return arch_ftrace_regs(fregs)->regs.gprs[14];
+}
+
 #define arch_ftrace_fill_perf_regs(fregs, _regs)	 do {		\
 		(_regs)->psw.mask = 0;					\
 		(_regs)->psw.addr = arch_ftrace_regs(fregs)->regs.psw.addr;		\
arch/x86/include/asm/ftrace.h +6 -0
index 7e06f8c7937a..cc92c99ef276 100644
@@ -58,6 +58,12 @@ arch_ftrace_get_regs(struct ftrace_regs *fregs)
 	do { arch_ftrace_regs(fregs)->regs.ip = (_ip); } while (0)
 
 
+static __always_inline unsigned long
+ftrace_regs_get_return_address(struct ftrace_regs *fregs)
+{
+	return *(unsigned long *)ftrace_regs_get_stack_pointer(fregs);
+}
+
 struct ftrace_ops;
 #define ftrace_graph_func ftrace_graph_func
 void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
include/linux/fprobe.h +43 -15

92 lines not on this page · raw message

kernel/trace/Kconfig +3 -5
index 2fc55a1a88aa..d570b8b9c0a9 100644
@@ -307,11 +307,9 @@ config DYNAMIC_FTRACE_WITH_ARGS
 
 config FPROBE
 	bool "Kernel Function Probe (fprobe)"
-	depends on FUNCTION_TRACER
-	depends on DYNAMIC_FTRACE_WITH_REGS || DYNAMIC_FTRACE_WITH_ARGS
-	depends on HAVE_FTRACE_REGS_HAVING_PT_REGS || !HAVE_DYNAMIC_FTRACE_WITH_ARGS
-	depends on HAVE_RETHOOK
-	select RETHOOK
+	depends on HAVE_FUNCTION_GRAPH_FREGS && HAVE_FTRACE_GRAPH_FUNC
+	depends on DYNAMIC_FTRACE_WITH_ARGS
+	select FUNCTION_GRAPH_TRACER
 	default n
 	help
 	  This option enables kernel function probe (fprobe) based on ftrace.
kernel/trace/fprobe.c +457 -180

767 lines not on this page · raw message

lib/test_fprobe.c +0 -45

85 lines not on this page · raw message

author maintainer
#14 lore raw

PATCH v22 14/20 fprobe: Add fprobe_header encoding feature

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Fprobe store its data structure address and size on the fgraph return stack by __fprobe_header. But most 64bit architecture can combine those to one unsigned long value because 4 MSB in the kernel address are the same. With this encoding, fprobe can consume less space on ret_stack.

This introduces asm/fprobe.h to define arch dependent encode/decode macros. Note that since fprobe depends on CONFIG_HAVE_FUNCTION_GRAPH_FREGS, currently only arm64, loongarch, riscv, s390 and x86 are supported.

Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Huacai Chen <[email protected]>
Cc WANG Xuerui <[email protected]>
Cc Paul Walmsley <[email protected]>
Cc Palmer Dabbelt <[email protected]>
Cc Albert Ou <[email protected]>
Cc Vasily Gorbik <[email protected]>
Cc Alexander Gordeev <[email protected]>
Cc Christian Borntraeger <[email protected]>
Cc Sven Schnelle <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc Dave Hansen <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc Steven Rostedt <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Mathieu Desnoyers <[email protected]>

---
Changes in v19:
- Fix to use Kbuild for generic arm/fprobe.h
- Check CONFIG_64BIT in asm-generic/fprobe.h.

---
 arch/arm64/include/asm/Kbuild       |    1 +
 arch/loongarch/include/asm/fprobe.h |   12 +++++++++
 arch/riscv/include/asm/Kbuild       |    1 +
 arch/s390/include/asm/fprobe.h      |   10 ++++++++
 arch/x86/include/asm/Kbuild         |    1 +
 include/asm-generic/fprobe.h        |   46 +++++++++++++++++++++++++++++++++++
 kernel/trace/fprobe.c               |   29 ++++++++++++++++++++++
 7 files changed, 100 insertions(+)
 create mode 100644 arch/loongarch/include/asm/fprobe.h
 create mode 100644 arch/s390/include/asm/fprobe.h
 create mode 100644 include/asm-generic/fprobe.h
arch/arm64/include/asm/Kbuild +1 -0
index 4e350df9a02d..d2ff8f6c3231 100644
@@ -8,6 +8,7 @@ syscall-y += unistd_32.h
 syscall-y += unistd_compat_32.h
 
 generic-y += early_ioremap.h
+generic-y += fprobe.h
 generic-y += mcs_spinlock.h
 generic-y += mmzone.h
 generic-y += qrwlock.h
arch/loongarch/include/asm/fprobe.h new file +12 -0
new file mode 100644
index 000000000000..7af3b3126caf
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_LOONGARCH_FPROBE_H
+#define _ASM_LOONGARCH_FPROBE_H
+
+/*
+ * Explicitly undef ARCH_DEFINE_ENCODE_FPROBE_HEADER, because loongarch does not
+ * have enough number of fixed MSBs of the address of kernel objects for
+ * encoding the size of data in fprobe_header. Use 2-entries encoding instead.
+ */
+#undef ARCH_DEFINE_ENCODE_FPROBE_HEADER
+
+#endif /* _ASM_LOONGARCH_FPROBE_H */
arch/riscv/include/asm/Kbuild +1 -0
index de13d5a234f8..bd5fc9403295 100644
@@ -4,6 +4,7 @@ syscall-y += syscall_table_64.h
 
 generic-y += early_ioremap.h
 generic-y += flat.h
+generic-y += fprobe.h
 generic-y += kvm_para.h
 generic-y += mmzone.h
 generic-y += mcs_spinlock.h
arch/s390/include/asm/fprobe.h new file +10 -0
new file mode 100644
index 000000000000..5ef600b372f4
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_S390_FPROBE_H
+#define _ASM_S390_FPROBE_H
+
+#include <asm-generic/fprobe.h>
+
+#undef FPROBE_HEADER_MSB_PATTERN
+#define FPROBE_HEADER_MSB_PATTERN 0
+
+#endif /* _ASM_S390_FPROBE_H */
arch/x86/include/asm/Kbuild +1 -0
index 6c23d1661b17..58f4ddecc5fa 100644
@@ -10,5 +10,6 @@ generated-y += unistd_64_x32.h
 generated-y += xen-hypercalls.h
 
 generic-y += early_ioremap.h
+generic-y += fprobe.h
 generic-y += mcs_spinlock.h
 generic-y += mmzone.h
include/asm-generic/fprobe.h new file +46 -0

46 lines not on this page · raw message

kernel/trace/fprobe.c +29 -0
index ed9c1d79426a..2560b312ad57 100644
@@ -13,6 +13,8 @@
 #include <linux/slab.h>
 #include <linux/sort.h>
 
+#include <asm/fprobe.h>
+
 #include "trace.h"
 
 #define FPROBE_IP_HASH_BITS 8
@@ -143,6 +145,31 @@ static int del_fprobe_hash(struct fprobe *fp)
 	return 0;
 }
 
+#ifdef ARCH_DEFINE_ENCODE_FPROBE_HEADER
+
+/* The arch should encode fprobe_header info into one unsigned long */
+#define FPROBE_HEADER_SIZE_IN_LONG	1
+
+static inline bool write_fprobe_header(unsigned long *stack,
+					struct fprobe *fp, unsigned int size_words)
+{
+	if (WARN_ON_ONCE(size_words > MAX_FPROBE_DATA_SIZE_WORD ||
+			 !arch_fprobe_header_encodable(fp)))
+		return false;
+
+	*stack = arch_encode_fprobe_header(fp, size_words);
+	return true;
+}
+
+static inline void read_fprobe_header(unsigned long *stack,
+					struct fprobe **fp, unsigned int *size_words)
+{
+	*fp = arch_decode_fprobe_header_fp(*stack);
+	*size_words = arch_decode_fprobe_header_size(*stack);
+}
+
+#else
+
 /* Generic fprobe_header */
 struct __fprobe_header {
 	struct fprobe *fp;
@@ -173,6 +200,8 @@ static inline void read_fprobe_header(unsigned long *stack,
 	*size_words = fph->size_words;
 }
 
+#endif
+
 /*
  * fprobe shadow stack management:
  * Since fprobe shares a single fgraph_ops, it needs to share the stack entry
author maintainer
#15 lore raw

PATCH v22 15/20 tracing/fprobe: Remove nr_maxactive from fprobe

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Remove depercated fprobe::nr_maxactive. This involves fprobe events to rejects the maxactive number.

---
Changes in v2:
- Newly added.

---
 include/linux/fprobe.h      |    2 --
 kernel/trace/trace_fprobe.c |   43 ++++++-------------------------------------
 2 files changed, 6 insertions(+), 39 deletions(-)
include/linux/fprobe.h +0 -2
index 91337bcb452f..702099f08929 100644
@@ -54,7 +54,6 @@ struct fprobe_hlist {
  * @nmissed: The counter for missing events.
  * @flags: The status flag.
  * @entry_data_size: The private data storage size.
- * @nr_maxactive: The max number of active functions. (*deprecated)
  * @entry_handler: The callback function for function entry.
  * @exit_handler: The callback function for function exit.
  * @hlist_array: The fprobe_hlist for fprobe search from IP hash table.
@@ -63,7 +62,6 @@ struct fprobe {
 	unsigned long		nmissed;
 	unsigned int		flags;
 	size_t			entry_data_size;
-	int			nr_maxactive;
 
 	fprobe_entry_cb entry_handler;
 	fprobe_exit_cb  exit_handler;
kernel/trace/trace_fprobe.c +6 -37

92 lines not on this page · raw message

author maintainer
#16 lore raw

PATCH v22 16/20 selftests: ftrace: Remove obsolate maxactive syntax check

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Since the fprobe event does not support maxactive anymore, stop testing the maxactive syntax error checking.

---
 .../ftrace/test.d/dynevent/fprobe_syntax_errors.tc |    4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)
tools/testing/selftests/ftrace/test.d/dynevent/fprobe_syntax_errors.tc +1 -3
index 61877d166451..c9425a34fae3 100644
@@ -16,9 +16,7 @@ aarch64)
   REG=%r0 ;;
 esac
 
-check_error 'f^100 vfs_read'		# MAXACT_NO_KPROBE
-check_error 'f^1a111 vfs_read'		# BAD_MAXACT
-check_error 'f^100000 vfs_read'		# MAXACT_TOO_BIG
+check_error 'f^100 vfs_read'		# BAD_MAXACT
 
 check_error 'f ^non_exist_func'		# BAD_PROBE_ADDR (enoent)
 check_error 'f ^vfs_read+10'		# BAD_PROBE_ADDR
author maintainer
#17 lore raw

PATCH v22 17/20 selftests/ftrace: Add a test case for repeating register/unregister fprobe

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

This test case repeats define and undefine the fprobe dynamic event to ensure that the fprobe does not cause any issue with such operations.

---
 .../test.d/dynevent/add_remove_fprobe_repeat.tc    |   19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe_repeat.tc
tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe_repeat.tc new file +19 -0
new file mode 100644
index 000000000000..b4ad09237e2a
@@ -0,0 +1,19 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# description: Generic dynamic event - Repeating add/remove fprobe events
+# requires: dynamic_events "f[:[<group>/][<event>]] <func-name>[%return] [<args>]":README
+
+echo 0 > events/enable
+echo > dynamic_events
+
+PLACE=$FUNCTION_FORK
+REPEAT_TIMES=64
+
+for i in `seq 1 $REPEAT_TIMES`; do
+  echo "f:myevent $PLACE" >> dynamic_events
+  grep -q myevent dynamic_events
+  test -d events/fprobes/myevent
+  echo > dynamic_events
+done
+
+clear_trace
author maintainer
#18 lore raw

PATCH v22 18/20 Documentation: probes: Update fprobe on function-graph tracer

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Update fprobe documentation for the new fprobe on function-graph tracer. This includes some bahvior changes and pt_regs to ftrace_regs interface change.

---
Changes in v19.1:
- Fix typo
- function-graph tracer -> function graph tracing feature in ftrace.
Changes in v2:
- Update @fregs parameter explanation.

---
 Documentation/trace/fprobe.rst |   42 ++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 15 deletions(-)
Documentation/trace/fprobe.rst +27 -15
index 196f52386aaa..71cd40472d36 100644
@@ -9,9 +9,10 @@ Fprobe - Function entry/exit probe
 Introduction
 ============
 
-Fprobe is a function entry/exit probe mechanism based on ftrace.
-Instead of using ftrace full feature, if you only want to attach callbacks
-on function entry and exit, similar to the kprobes and kretprobes, you can
+Fprobe is a function entry/exit probe based on the function-graph tracing
+feature in ftrace.
+Instead of tracing all functions, if you want to attach callbacks on specific
+function entry and exit, similar to the kprobes and kretprobes, you can
 use fprobe. Compared with kprobes and kretprobes, fprobe gives faster
 instrumentation for multiple functions with single handler. This document
 describes how to use fprobe.
@@ -91,12 +92,14 @@ The prototype of the entry/exit callback function are as follows:
 
 .. code-block:: c
 
- int entry_callback(struct fprobe *fp, unsigned long entry_ip, unsigned long ret_ip, struct pt_regs *regs, void *entry_data);
+ int entry_callback(struct fprobe *fp, unsigned long entry_ip, unsigned long ret_ip, struct ftrace_regs *fregs, void *entry_data);
 
- void exit_callback(struct fprobe *fp, unsigned long entry_ip, unsigned long ret_ip, struct pt_regs *regs, void *entry_data);
+ void exit_callback(struct fprobe *fp, unsigned long entry_ip, unsigned long ret_ip, struct ftrace_regs *fregs, void *entry_data);
 
-Note that the @entry_ip is saved at function entry and passed to exit handler.
-If the entry callback function returns !0, the corresponding exit callback will be cancelled.
+Note that the @entry_ip is saved at function entry and passed to exit
+handler.
+If the entry callback function returns !0, the corresponding exit callback
+will be cancelled.
 
 @fp
         This is the address of `fprobe` data structure related to this handler.
@@ -112,12 +115,10 @@ If the entry callback function returns !0, the corresponding exit callback will
         This is the return address that the traced function will return to,
         somewhere in the caller. This can be used at both entry and exit.
 
-@regs
-        This is the `pt_regs` data structure at the entry and exit. Note that
-        the instruction pointer of @regs may be different from the @entry_ip
-        in the entry_handler. If you need traced instruction pointer, you need
-        to use @entry_ip. On the other hand, in the exit_handler, the instruction
-        pointer of @regs is set to the current return address.
+@fregs
+        This is the `ftrace_regs` data structure at the entry and exit. This
+        includes the function parameters, or the return values. So user can
+        access thos values via appropriate `ftrace_regs_*` APIs.
 
 @entry_data
         This is a local storage to share the data between entry and exit handlers.
@@ -125,6 +126,17 @@ If the entry callback function returns !0, the corresponding exit callback will
         and `entry_data_size` field when registering the fprobe, the storage is
         allocated and passed to both `entry_handler` and `exit_handler`.
 
+Entry data size and exit handlers on the same function
+======================================================
+
+Since the entry data is passed via per-task stack and it has limited size,
+the entry data size per probe is limited to `15 * sizeof(long)`. You also need
+to take care that the different fprobes are probing on the same function, this
+limit becomes smaller. The entry data size is aligned to `sizeof(long)` and
+each fprobe which has exit handler uses a `sizeof(long)` space on the stack,
+you should keep the number of fprobes on the same function as small as
+possible.
+
 Share the callbacks with kprobes
 ================================
 
@@ -165,8 +177,8 @@ This counter counts up when;
  - fprobe fails to take ftrace_recursion lock. This usually means that a function
    which is traced by other ftrace users is called from the entry_handler.
 
- - fprobe fails to setup the function exit because of the shortage of rethook
-   (the shadow stack for hooking the function return.)
+ - fprobe fails to setup the function exit because of failing to allocate the
+   data buffer from the per-task shadow stack.
 
 The `fprobe::nmissed` field counts up in both cases. Therefore, the former
 skips both of entry and exit callback and the latter skips the exit
author maintainer
#19 lore raw

PATCH v22 19/20 ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

This introduces ftrace_get_symaddr() which tries to convert fentry_ip passed by ftrace or fgraph callback to symaddr without calling kallsyms API. It returns the symbol address or 0 if it fails to convert it.

---
Changes in v21:
- On arm64, fix the macro name to ftrace_get_symaddr() correctly.
- Define ftrace_get_symaddr() outside of CONFIG_DYNAMIC_FTRACE.
Changes in v19:
- Newly added.

---
 arch/arm64/include/asm/ftrace.h |    2 +
 arch/arm64/kernel/ftrace.c      |   63 +++++++++++++++++++++++++++++++++++++++
 arch/x86/include/asm/ftrace.h   |   21 +++++++++++++
 include/linux/ftrace.h          |   13 ++++++++
 4 files changed, 99 insertions(+)
arch/arm64/include/asm/ftrace.h +2 -0
index 876e88ad4119..bfe3ce9df197 100644
@@ -52,6 +52,8 @@ extern unsigned long ftrace_graph_call;
 extern void return_to_handler(void);
 
 unsigned long ftrace_call_adjust(unsigned long addr);
+unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip);
+#define ftrace_get_symaddr(fentry_ip) arch_ftrace_get_symaddr(fentry_ip)
 
 #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
 #define HAVE_ARCH_FTRACE_REGS
arch/arm64/kernel/ftrace.c +63 -0

69 lines not on this page · raw message

arch/x86/include/asm/ftrace.h +21 -0
index cc92c99ef276..f9cb4d07df58 100644
@@ -34,6 +34,27 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
 	return addr;
 }
 
+static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
+{
+#ifdef CONFIG_X86_KERNEL_IBT
+	u32 instr;
+
+	/* We want to be extra safe in case entry ip is on the page edge,
+	 * but otherwise we need to avoid get_kernel_nofault()'s overhead.
+	 */
+	if ((fentry_ip & ~PAGE_MASK) < ENDBR_INSN_SIZE) {
+		if (get_kernel_nofault(instr, (u32 *)(fentry_ip - ENDBR_INSN_SIZE)))
+			return fentry_ip;
+	} else {
+		instr = *(u32 *)(fentry_ip - ENDBR_INSN_SIZE);
+	}
+	if (is_endbr(instr))
+		fentry_ip -= ENDBR_INSN_SIZE;
+#endif
+	return fentry_ip;
+}
+#define ftrace_get_symaddr(fentry_ip)	arch_ftrace_get_symaddr(fentry_ip)
+
 #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
 
 #include <linux/ftrace_regs.h>
include/linux/ftrace.h +13 -0
index 4c553fe9c026..07092dfb21a4 100644
@@ -622,6 +622,19 @@ enum {
 	FTRACE_MAY_SLEEP		= (1 << 5),
 };
 
+/* Arches can override ftrace_get_symaddr() to convert fentry_ip to symaddr. */
+#ifndef ftrace_get_symaddr
+/**
+ * ftrace_get_symaddr - return the symbol address from fentry_ip
+ * @fentry_ip: the address of ftrace location
+ *
+ * Get the symbol address from @fentry_ip (fast path). If there is no fast
+ * search path, this returns 0.
+ * User may need to use kallsyms API to find the symbol address.
+ */
+#define ftrace_get_symaddr(fentry_ip) (0)
+#endif
+
 #ifdef CONFIG_DYNAMIC_FTRACE
 
 void ftrace_arch_code_modify_prepare(void);
#20 lore raw

Re: [PATCH v22 19/20] ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr

Hello,

I got errors building Linux 6.14-rc1 that were solved by reverting this patch and the one after (19/20 and 20/20).

Errors look like:

In file included from ./arch/x86/include/asm/asm-prototypes.h:2,
                 from <stdin>:3:
./arch/x86/include/asm/ftrace.h: In function 'arch_ftrace_get_symaddr':
./arch/x86/include/asm/ftrace.h:46:21: error: implicit declaration of function 'get_kernel_nofault' [-Wimplicit-function-declaration] 46 |                 if (get_kernel_nofault(instr, (u32 *)(fentry_ip - ENDBR_INSN_SIZE)))
      | ^~~~~~~~~~~~~~~~~~

Will send .config on request if needed.

Le 26/12/2024 à 06:15, Masami Hiramatsu (Google) a écrit :

Quoted 169 lines

From: Masami Hiramatsu (Google) <[email protected]>

This introduces ftrace_get_symaddr() which tries to convert fentry_ip passed by ftrace or fgraph callback to symaddr without calling kallsyms API. It returns the symbol address or 0 if it fails to convert it.

---
Changes in v21:
- On arm64, fix the macro name to ftrace_get_symaddr() correctly.
- Define ftrace_get_symaddr() outside of CONFIG_DYNAMIC_FTRACE.
Changes in v19:
- Newly added.

---
  arch/arm64/include/asm/ftrace.h |    2 +
  arch/arm64/kernel/ftrace.c      |   63 +++++++++++++++++++++++++++++++++++++++
  arch/x86/include/asm/ftrace.h   |   21 +++++++++++++
  include/linux/ftrace.h          |   13 ++++++++
  4 files changed, 99 insertions(+)
arch/arm64/include/asm/ftrace.h +2 -0
index 876e88ad4119..bfe3ce9df197 100644
@@ -52,6 +52,8 @@ extern unsigned long ftrace_graph_call;
  extern void return_to_handler(void);
 
  unsigned long ftrace_call_adjust(unsigned long addr);
+unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip);
+#define ftrace_get_symaddr(fentry_ip) arch_ftrace_get_symaddr(fentry_ip)
 
  #ifdef CONFIG_DYNAMIC_FTRACE_WITH_ARGS
  #define HAVE_ARCH_FTRACE_REGS
arch/arm64/kernel/ftrace.c +63 -0
index 570c38be833c..d7c0d023dfe5 100644
@@ -143,6 +143,69 @@ unsigned long ftrace_call_adjust(unsigned long addr)
  	return addr;
  }
 
+/* Convert fentry_ip to the symbol address without kallsyms */
+unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
+{
+	u32 insn;
+
+	/*
+	 * When using patchable-function-entry without pre-function NOPS, ftrace
+	 * entry is the address of the first NOP after the function entry point.
+	 *
+	 * The compiler has either generated:
+	 *
+	 * func+00:	func:	NOP		// To be patched to MOV X9, LR
+	 * func+04:		NOP		// To be patched to BL <caller>
+	 *
+	 * Or:
+	 *
+	 * func-04:		BTI	C
+	 * func+00:	func:	NOP		// To be patched to MOV X9, LR
+	 * func+04:		NOP		// To be patched to BL <caller>
+	 *
+	 * The fentry_ip is the address of `BL <caller>` which is at `func + 4`
+	 * bytes in either case.
+	 */
+	if (!IS_ENABLED(CONFIG_DYNAMIC_FTRACE_WITH_CALL_OPS))
+		return fentry_ip - AARCH64_INSN_SIZE;
+
+	/*
+	 * When using patchable-function-entry with pre-function NOPs, BTI is
+	 * a bit different.
+	 *
+	 * func+00:	func:	NOP		// To be patched to MOV X9, LR
+	 * func+04:		NOP		// To be patched to BL <caller>
+	 *
+	 * Or:
+	 *
+	 * func+00:	func:	BTI	C
+	 * func+04:		NOP		// To be patched to MOV X9, LR
+	 * func+08:		NOP		// To be patched to BL <caller>
+	 *
+	 * The fentry_ip is the address of `BL <caller>` which is at either
+	 * `func + 4` or `func + 8` depends on whether there is a BTI.
+	 */
+
+	/* If there is no BTI, the func address should be one instruction before. */
+	if (!IS_ENABLED(CONFIG_ARM64_BTI_KERNEL))
+		return fentry_ip - AARCH64_INSN_SIZE;
+
+	/* We want to be extra safe in case entry ip is on the page edge,
+	 * but otherwise we need to avoid get_kernel_nofault()'s overhead.
+	 */
+	if ((fentry_ip & ~PAGE_MASK) < AARCH64_INSN_SIZE * 2) {
+		if (get_kernel_nofault(insn, (u32 *)(fentry_ip - AARCH64_INSN_SIZE * 2)))
+			return 0;
+	} else {
+		insn = *(u32 *)(fentry_ip - AARCH64_INSN_SIZE * 2);
+	}
+
+	if (aarch64_insn_is_bti(le32_to_cpu((__le32)insn)))
+		return fentry_ip - AARCH64_INSN_SIZE * 2;
+
+	return fentry_ip - AARCH64_INSN_SIZE;
+}
+
  /*
   * Replace a single instruction, which may be a branch or NOP.
   * If @validate == true, a replaced instruction is checked against 'old'.
arch/x86/include/asm/ftrace.h +21 -0
index cc92c99ef276..f9cb4d07df58 100644
@@ -34,6 +34,27 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
  	return addr;
  }
 
+static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
+{
+#ifdef CONFIG_X86_KERNEL_IBT
+	u32 instr;
+
+	/* We want to be extra safe in case entry ip is on the page edge,
+	 * but otherwise we need to avoid get_kernel_nofault()'s overhead.
+	 */
+	if ((fentry_ip & ~PAGE_MASK) < ENDBR_INSN_SIZE) {
+		if (get_kernel_nofault(instr, (u32 *)(fentry_ip - ENDBR_INSN_SIZE)))
+			return fentry_ip;
+	} else {
+		instr = *(u32 *)(fentry_ip - ENDBR_INSN_SIZE);
+	}
+	if (is_endbr(instr))
+		fentry_ip -= ENDBR_INSN_SIZE;
+#endif
+	return fentry_ip;
+}
+#define ftrace_get_symaddr(fentry_ip)	arch_ftrace_get_symaddr(fentry_ip)
+
  #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
 
  #include <linux/ftrace_regs.h>
include/linux/ftrace.h +13 -0
index 4c553fe9c026..07092dfb21a4 100644
@@ -622,6 +622,19 @@ enum {
  	FTRACE_MAY_SLEEP		= (1 << 5),
  };
 
+/* Arches can override ftrace_get_symaddr() to convert fentry_ip to symaddr. */
+#ifndef ftrace_get_symaddr
+/**
+ * ftrace_get_symaddr - return the symbol address from fentry_ip
+ * @fentry_ip: the address of ftrace location
+ *
+ * Get the symbol address from @fentry_ip (fast path). If there is no fast
+ * search path, this returns 0.
+ * User may need to use kallsyms API to find the symbol address.
+ */
+#define ftrace_get_symaddr(fentry_ip) (0)
+#endif
+
  #ifdef CONFIG_DYNAMIC_FTRACE
 
  void ftrace_arch_code_modify_prepare(void);
author maintainer
#21 lore raw

Re: [PATCH v22 19/20] ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr

Quoted 16 lines · On Mon, 3 Feb 2025 22:33:48 +0100 Gabriel de Perthuis <[email protected]>…

Hello,

I got errors building Linux 6.14-rc1 that were solved by reverting this patch and the one after (19/20 and 20/20).

Errors look like:

In file included from ./arch/x86/include/asm/asm-prototypes.h:2,
                 from <stdin>:3:
./arch/x86/include/asm/ftrace.h: In function 'arch_ftrace_get_symaddr':
./arch/x86/include/asm/ftrace.h:46:21: error: implicit declaration of function 'get_kernel_nofault' [-Wimplicit-function-declaration]
   46 |                 if (get_kernel_nofault(instr, (u32 *)(fentry_ip
- ENDBR_INSN_SIZE)))
      | ^~~~~~~~~~~~~~~~~~

Quoted 1 line

Will send .config on request if needed.

Thanks for the report!

-------<arch/x86/include/asm/asm-prototypes.h>
/* SPDX-License-Identifier: GPL-2.0 */
#include <asm/ftrace.h>
#include <linux/uaccess.h>
-----

Ah, that's why... get_kernel_nofault() is defined in linux/uaccess.h. I also found that is_endbr() is in asm/ibt.h.

Can you try this?

Thank you,

arch/x86/include/asm/ftrace.h +20 -17
index f9cb4d07df58..d24d7c71253f 100644
@@ -16,24 +16,9 @@
 # include <asm/ibt.h>
 /* Add offset for endbr64 if IBT enabled */
 # define FTRACE_MCOUNT_MAX_OFFSET	ENDBR_INSN_SIZE
-#endif
-
-#ifdef CONFIG_DYNAMIC_FTRACE
-#define ARCH_SUPPORTS_FTRACE_OPS 1
-#endif
-
-#ifndef __ASSEMBLY__
-extern void __fentry__(void);
-
-static inline unsigned long ftrace_call_adjust(unsigned long addr)
-{
-	/*
-	 * addr is the address of the mcount call instruction.
-	 * recordmcount does the necessary offset calculation.
-	 */
-	return addr;
-}
 
+#include <linux/uaccess.h>
+/* This only supports fentry based ftrace. */
 static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
 {
 #ifdef CONFIG_X86_KERNEL_IBT
@@ -55,6 +40,24 @@ static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
 }
 #define ftrace_get_symaddr(fentry_ip)	arch_ftrace_get_symaddr(fentry_ip)
 
+#endif
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+#define ARCH_SUPPORTS_FTRACE_OPS 1
+#endif
+
+#ifndef __ASSEMBLY__
+extern void __fentry__(void);
+
+static inline unsigned long ftrace_call_adjust(unsigned long addr)
+{
+	/*
+	 * addr is the address of the mcount call instruction.
+	 * recordmcount does the necessary offset calculation.
+	 */
+	return addr;
+}
+
 #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
 
 #include <linux/ftrace_regs.h>
Signature · 1 line
Masami Hiramatsu (Google) <[email protected]>
#22 lore raw

Re: [PATCH v22 19/20] ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr

Thank you for the prompt response!
Sorry, this doesn't build.
New, different errors:

AS arch/x86/kernel/ftrace_64.o
In file included from ./include/linux/kcsan-checks.h:13,

                 from ./include/linux/instrumented.h:12,
                 from ./include/linux/uaccess.h:6,
                 from ./arch/x86/include/asm/ftrace.h:20,
                 from arch/x86/kernel/ftrace_64.S:11:

./include/linux/compiler_attributes.h:91:20: error: missing binary operator before token "("
91 | #if __has_attribute(__copy__)
| ^
./include/linux/compiler_attributes.h:103:20: error: missing binary operator before token "("
103 | #if __has_attribute(__diagnose_as_builtin__)
| ^
./include/linux/compiler_attributes.h:126:20: error: missing binary operator before token "("
126 | #if __has_attribute(__designated_init__)
| ^
./include/linux/compiler_attributes.h:137:20: error: missing binary operator before token "("
137 | #if __has_attribute(__error__)
| ^
./include/linux/compiler_attributes.h:148:20: error: missing binary operator before token "("
148 | #if __has_attribute(__externally_visible__)
| ^
./include/linux/compiler_attributes.h:185:20: error: missing binary operator before token "("
185 | #if __has_attribute(__no_caller_saved_registers__)
| ^
./include/linux/compiler_attributes.h:196:20: error: missing binary operator before token "("
196 | #if __has_attribute(__noclone__)
| ^
./include/linux/compiler_attributes.h:213:20: error: missing binary operator before token "("
213 | #if __has_attribute(__fallthrough__)
| ^
./include/linux/compiler_attributes.h:239:20: error: missing binary operator before token "("
239 | #if __has_attribute(__nonstring__)
| ^
./include/linux/compiler_attributes.h:251:20: error: missing binary operator before token "("
251 | #if __has_attribute(__no_profile_instrument_function__)
| ^
./include/linux/compiler_attributes.h:270:20: error: missing binary operator before token "("
270 | #if __has_attribute(__no_stack_protector__)
| ^
./include/linux/compiler_attributes.h:281:20: error: missing binary operator before token "("
281 | #if __has_attribute(__overloadable__)
| ^
./include/linux/compiler_attributes.h:300:20: error: missing binary operator before token "("
300 | #if __has_attribute(__pass_dynamic_object_size__)
| ^
./include/linux/compiler_attributes.h:305:20: error: missing binary operator before token "("
305 | #if __has_attribute(__pass_object_size__)
| ^
./include/linux/compiler_attributes.h:329:20: error: missing binary operator before token "("
329 | #if __has_attribute(__uninitialized__)
| ^
./include/linux/compiler_attributes.h:375:20: error: missing binary operator before token "("
375 | #if __has_attribute(__warning__)
| ^
./include/linux/compiler_attributes.h:392:20: error: missing binary operator before token "("
392 | #if __has_attribute(disable_sanitizer_instrumentation)
| ^
In file included from ./include/linux/mm_types_task.h:11,

                 from ./include/linux/sched.h:38,
                 from ./include/linux/uaccess.h:9:
./include/linux/align.h:8:9: warning: "ALIGN" redefined
    8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
      |         ^~~~~
In file included from ./include/linux/export.h:6,
                 from arch/x86/kernel/ftrace_64.S:6:
./include/linux/linkage.h:103:9: note: this is the location of the
previous definition
  103 | #define ALIGN __ALIGN
      |         ^~~~~

make[7]: *** [scripts/Makefile.build:339: arch/x86/kernel/ftrace_64.o] Error 1

Am attaching the .config so you can reproduce these easily.

Quoted 98 lines · Masami Hiramatsu
Quoted 9 lines · On Mon, 3 Feb 2025 22:33:48 +0100 Gabriel de Perthuis <[email protected]>…

Hello,

I got errors building Linux 6.14-rc1 that were solved by reverting this patch and the one after (19/20 and 20/20).

Errors look like:

In file included from ./arch/x86/include/asm/asm-prototypes.h:2,

                  from <stdin>:3:
Quoted 8 lines

./arch/x86/include/asm/ftrace.h: In function 'arch_ftrace_get_symaddr':
./arch/x86/include/asm/ftrace.h:46:21: error: implicit declaration of function 'get_kernel_nofault' [-Wimplicit-function-declaration]
46 | if (get_kernel_nofault(instr, (u32 *)(fentry_ip
- ENDBR_INSN_SIZE)))

       | ^~~~~~~~~~~~~~~~~~

Will send .config on request if needed.

Thanks for the report!

-------<arch/x86/include/asm/asm-prototypes.h>
/* SPDX-License-Identifier: GPL-2.0 */
#include <asm/ftrace.h>
#include <linux/uaccess.h>
-----

Ah, that's why... get_kernel_nofault() is defined in linux/uaccess.h. I also found that is_endbr() is in asm/ibt.h.

Can you try this?

Thank you,

arch/x86/include/asm/ftrace.h +20 -18
index f9cb4d07df58..d24d7c71253f 100644
@@ -16,24 +16,9 @@
 # include <asm/ibt.h>
 /* Add offset for endbr64 if IBT enabled */
 # define FTRACE_MCOUNT_MAX_OFFSET      ENDBR_INSN_SIZE
-#endif
-
-#ifdef CONFIG_DYNAMIC_FTRACE
-#define ARCH_SUPPORTS_FTRACE_OPS 1
-#endif
-
-#ifndef __ASSEMBLY__
-extern void __fentry__(void);
-
-static inline unsigned long ftrace_call_adjust(unsigned long addr)
-{
-       /*
-        * addr is the address of the mcount call instruction.
-        * recordmcount does the necessary offset calculation.
-        */
-       return addr;
-}
 
+#include <linux/uaccess.h>
+/* This only supports fentry based ftrace. */
 static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
 {
 #ifdef CONFIG_X86_KERNEL_IBT
@@ -55,6 +40,24 @@ static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
 }
 #define ftrace_get_symaddr(fentry_ip)  arch_ftrace_get_symaddr(fentry_ip)
 
+#endif
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+#define ARCH_SUPPORTS_FTRACE_OPS 1
+#endif
+
+#ifndef __ASSEMBLY__
+extern void __fentry__(void);
+
+static inline unsigned long ftrace_call_adjust(unsigned long addr)
+{
+       /*
+        * addr is the address of the mcount call instruction.
+        * recordmcount does the necessary offset calculation.
+        */
+       return addr;
+}
+
 #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
 
 #include <linux/ftrace_regs.h>
 
 
--

Masami Hiramatsu (Google) <[email protected]>

author maintainer
#23 lore raw

Re: [PATCH v22 19/20] ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr

Quoted 2 lines · On Tue, 4 Feb 2025 15:19:45 +0100 Gabriel de Perthuis <[email protected]>…

Thank you for the prompt response!
Sorry, this doesn't build.

Thanks for testing!
OK, so including linux/uaccess.h in asm/ftrace.h will cause another issue. This is a good information!
I'll move the ftrace_call_adjust() to arch/x86/kernel/ftrace.c then.

Thank you!

Quoted 191 lines

New, different errors:

AS arch/x86/kernel/ftrace_64.o
In file included from ./include/linux/kcsan-checks.h:13,

                 from ./include/linux/instrumented.h:12,
                 from ./include/linux/uaccess.h:6,
                 from ./arch/x86/include/asm/ftrace.h:20,
                 from arch/x86/kernel/ftrace_64.S:11:

./include/linux/compiler_attributes.h:91:20: error: missing binary operator before token "("
91 | #if __has_attribute(__copy__)
| ^
./include/linux/compiler_attributes.h:103:20: error: missing binary operator before token "("
103 | #if __has_attribute(__diagnose_as_builtin__)
| ^
./include/linux/compiler_attributes.h:126:20: error: missing binary operator before token "("
126 | #if __has_attribute(__designated_init__)
| ^
./include/linux/compiler_attributes.h:137:20: error: missing binary operator before token "("
137 | #if __has_attribute(__error__)
| ^
./include/linux/compiler_attributes.h:148:20: error: missing binary operator before token "("
148 | #if __has_attribute(__externally_visible__)
| ^
./include/linux/compiler_attributes.h:185:20: error: missing binary operator before token "("
185 | #if __has_attribute(__no_caller_saved_registers__)
| ^
./include/linux/compiler_attributes.h:196:20: error: missing binary operator before token "("
196 | #if __has_attribute(__noclone__)
| ^
./include/linux/compiler_attributes.h:213:20: error: missing binary operator before token "("
213 | #if __has_attribute(__fallthrough__)
| ^
./include/linux/compiler_attributes.h:239:20: error: missing binary operator before token "("
239 | #if __has_attribute(__nonstring__)
| ^
./include/linux/compiler_attributes.h:251:20: error: missing binary operator before token "("
251 | #if __has_attribute(__no_profile_instrument_function__)
| ^
./include/linux/compiler_attributes.h:270:20: error: missing binary operator before token "("
270 | #if __has_attribute(__no_stack_protector__)
| ^
./include/linux/compiler_attributes.h:281:20: error: missing binary operator before token "("
281 | #if __has_attribute(__overloadable__)
| ^
./include/linux/compiler_attributes.h:300:20: error: missing binary operator before token "("
300 | #if __has_attribute(__pass_dynamic_object_size__)
| ^
./include/linux/compiler_attributes.h:305:20: error: missing binary operator before token "("
305 | #if __has_attribute(__pass_object_size__)
| ^
./include/linux/compiler_attributes.h:329:20: error: missing binary operator before token "("
329 | #if __has_attribute(__uninitialized__)
| ^
./include/linux/compiler_attributes.h:375:20: error: missing binary operator before token "("
375 | #if __has_attribute(__warning__)
| ^
./include/linux/compiler_attributes.h:392:20: error: missing binary operator before token "("
392 | #if __has_attribute(disable_sanitizer_instrumentation)
| ^
In file included from ./include/linux/mm_types_task.h:11,

                 from ./include/linux/sched.h:38,
                 from ./include/linux/uaccess.h:9:
./include/linux/align.h:8:9: warning: "ALIGN" redefined
    8 | #define ALIGN(x, a)             __ALIGN_KERNEL((x), (a))
      |         ^~~~~
In file included from ./include/linux/export.h:6,
                 from arch/x86/kernel/ftrace_64.S:6:
./include/linux/linkage.h:103:9: note: this is the location of the
previous definition
  103 | #define ALIGN __ALIGN
      |         ^~~~~

make[7]: *** [scripts/Makefile.build:339: arch/x86/kernel/ftrace_64.o] Error 1

Am attaching the .config so you can reproduce these easily.

Quoted 98 lines · Masami Hiramatsu
Quoted 9 lines · On Mon, 3 Feb 2025 22:33:48 +0100 Gabriel de Perthuis <[email protected]>…

Hello,

I got errors building Linux 6.14-rc1 that were solved by reverting this patch and the one after (19/20 and 20/20).

Errors look like:

In file included from ./arch/x86/include/asm/asm-prototypes.h:2,

                  from <stdin>:3:
Quoted 8 lines

./arch/x86/include/asm/ftrace.h: In function 'arch_ftrace_get_symaddr':
./arch/x86/include/asm/ftrace.h:46:21: error: implicit declaration of function 'get_kernel_nofault' [-Wimplicit-function-declaration]
46 | if (get_kernel_nofault(instr, (u32 *)(fentry_ip
- ENDBR_INSN_SIZE)))

       | ^~~~~~~~~~~~~~~~~~

Will send .config on request if needed.

Thanks for the report!

-------<arch/x86/include/asm/asm-prototypes.h>
/* SPDX-License-Identifier: GPL-2.0 */
#include <asm/ftrace.h>
#include <linux/uaccess.h>
-----

Ah, that's why... get_kernel_nofault() is defined in linux/uaccess.h. I also found that is_endbr() is in asm/ibt.h.

Can you try this?

Thank you,

arch/x86/include/asm/ftrace.h +20 -18
index f9cb4d07df58..d24d7c71253f 100644
@@ -16,24 +16,9 @@
 # include <asm/ibt.h>
 /* Add offset for endbr64 if IBT enabled */
 # define FTRACE_MCOUNT_MAX_OFFSET      ENDBR_INSN_SIZE
-#endif
-
-#ifdef CONFIG_DYNAMIC_FTRACE
-#define ARCH_SUPPORTS_FTRACE_OPS 1
-#endif
-
-#ifndef __ASSEMBLY__
-extern void __fentry__(void);
-
-static inline unsigned long ftrace_call_adjust(unsigned long addr)
-{
-       /*
-        * addr is the address of the mcount call instruction.
-        * recordmcount does the necessary offset calculation.
-        */
-       return addr;
-}
 
+#include <linux/uaccess.h>
+/* This only supports fentry based ftrace. */
 static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
 {
 #ifdef CONFIG_X86_KERNEL_IBT
@@ -55,6 +40,24 @@ static inline unsigned long arch_ftrace_get_symaddr(unsigned long fentry_ip)
 }
 #define ftrace_get_symaddr(fentry_ip)  arch_ftrace_get_symaddr(fentry_ip)
 
+#endif
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+#define ARCH_SUPPORTS_FTRACE_OPS 1
+#endif
+
+#ifndef __ASSEMBLY__
+extern void __fentry__(void);
+
+static inline unsigned long ftrace_call_adjust(unsigned long addr)
+{
+       /*
+        * addr is the address of the mcount call instruction.
+        * recordmcount does the necessary offset calculation.
+        */
+       return addr;
+}
+
 #ifdef CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS
 
 #include <linux/ftrace_regs.h>
 
 
--

Masami Hiramatsu (Google) <[email protected]>

Signature · 1 line
Masami Hiramatsu (Google) <[email protected]>
author maintainer
#24 lore raw

PATCH v22 20/20 bpf: Use ftrace_get_symaddr() for kprobe_multi probes

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Add ftrace_get_entry_ip() which is only for ftrace based probes, and use it for kprobe multi probes because they are based on fprobe which uses ftrace instead of kprobes.

---
Changes in v21:
- Use new ftreace_get_symaddr() only for kprobe multi probes.
Changes in v19:
- Use ftrace_get_symaddr() instead of introducing new arch dependent code.
- Also, replace x86 code with ftrace_get_symaddr(), which does the same thing.

---
 kernel/trace/bpf_trace.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
kernel/trace/bpf_trace.c +11 -2
index 863351559334..47beb70187d7 100644
@@ -1070,6 +1070,13 @@ static unsigned long get_entry_ip(unsigned long fentry_ip)
 #define get_entry_ip(fentry_ip) fentry_ip
 #endif
 
+static unsigned long ftrace_get_entry_ip(unsigned long fentry_ip)
+{
+	unsigned long ip = ftrace_get_symaddr(fentry_ip);
+
+	return ip ? : fentry_ip;
+}
+
 BPF_CALL_1(bpf_get_func_ip_kprobe, struct pt_regs *, regs)
 {
 	struct bpf_trace_run_ctx *run_ctx __maybe_unused;
@@ -2829,7 +2836,8 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
 	int err;
 
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
-	err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), fregs, false, data);
+	err = kprobe_multi_link_prog_run(link, ftrace_get_entry_ip(fentry_ip),
+					 fregs, false, data);
 	return is_kprobe_session(link->link.prog) ? err : 0;
 }
 
@@ -2841,7 +2849,8 @@ kprobe_multi_link_exit_handler(struct fprobe *fp, unsigned long fentry_ip,
 	struct bpf_kprobe_multi_link *link;
 
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
-	kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), fregs, true, data);
+	kprobe_multi_link_prog_run(link, ftrace_get_entry_ip(fentry_ip),
+				   fregs, true, data);
 }
 
 static int symbols_cmp_r(const void *a, const void *b, const void *priv)
maintainer
#25 lore raw

Re: [PATCH v22 20/20] bpf: Use ftrace_get_symaddr() for kprobe_multi probes

Quoted 7 lines · On Thu, 26 Dec 2024 14:15:25 +0900 "Masami Hiramatsu (Google)"…

From: Masami Hiramatsu (Google) <[email protected]>

Add ftrace_get_entry_ip() which is only for ftrace based probes, and use it for kprobe multi probes because they are based on fprobe which uses ftrace instead of kprobes.

On my 32bit build, I hit a new warning;

kernel/trace/bpf_trace.c:1073:22: warning: ‘ftrace_get_entry_ip’ defined but not used [-Wunused-function]
1073 | static unsigned long ftrace_get_entry_ip(unsigned long fentry_ip)

      |                      ^~~~~~~~~~~~~~~~~~~

Config attached.

-- Steve

Quoted 49 lines

---
Changes in v21:
- Use new ftreace_get_symaddr() only for kprobe multi probes.
Changes in v19:
- Use ftrace_get_symaddr() instead of introducing new arch dependent code.
- Also, replace x86 code with ftrace_get_symaddr(), which does the same thing.

---
 kernel/trace/bpf_trace.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
kernel/trace/bpf_trace.c +11 -2
index 863351559334..47beb70187d7 100644
@@ -1070,6 +1070,13 @@ static unsigned long get_entry_ip(unsigned long fentry_ip)
 #define get_entry_ip(fentry_ip) fentry_ip
 #endif
 
+static unsigned long ftrace_get_entry_ip(unsigned long fentry_ip)
+{
+	unsigned long ip = ftrace_get_symaddr(fentry_ip);
+
+	return ip ? : fentry_ip;
+}
+
 BPF_CALL_1(bpf_get_func_ip_kprobe, struct pt_regs *, regs)
 {
 	struct bpf_trace_run_ctx *run_ctx __maybe_unused;
@@ -2829,7 +2836,8 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
 	int err;
 
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
-	err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), fregs, false, data);
+	err = kprobe_multi_link_prog_run(link, ftrace_get_entry_ip(fentry_ip),
+					 fregs, false, data);
 	return is_kprobe_session(link->link.prog) ? err : 0;
 }
 
@@ -2841,7 +2849,8 @@ kprobe_multi_link_exit_handler(struct fprobe *fp, unsigned long fentry_ip,
 	struct bpf_kprobe_multi_link *link;
 
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
-	kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), fregs, true, data);
+	kprobe_multi_link_prog_run(link, ftrace_get_entry_ip(fentry_ip),
+				   fregs, true, data);
 }
 
 static int symbols_cmp_r(const void *a, const void *b, const void *priv)
maintainer
#26 lore raw

Re: [PATCH v22 20/20] bpf: Use ftrace_get_symaddr() for kprobe_multi probes

Quoted 11 lines · On Thu, 26 Dec 2024 21:23:39 -0500 Steven Rostedt <[email protected]> wrote:
Quoted 7 lines · On Thu, 26 Dec 2024 14:15:25 +0900 "Masami Hiramatsu (Google)"…

From: Masami Hiramatsu (Google) <[email protected]>

Add ftrace_get_entry_ip() which is only for ftrace based probes, and use it for kprobe multi probes because they are based on fprobe which uses ftrace instead of kprobes.

Quoted 7 lines

On my 32bit build, I hit a new warning;

kernel/trace/bpf_trace.c:1073:22: warning: ‘ftrace_get_entry_ip’ defined but not used [-Wunused-function]
1073 | static unsigned long ftrace_get_entry_ip(unsigned long fentry_ip)

      |                      ^~~~~~~~~~~~~~~~~~~

Config attached.

Since this is the last patch of the series, just send a new patch for this one. I'll continue testing the rest of the patches.

-- Steve

maintainer
#27 lore raw

Re: [PATCH v22 20/20] bpf: Use ftrace_get_symaddr() for kprobe_multi probes

Quoted 11 lines · On Thu, 26 Dec 2024 21:24:53 -0500 Steven Rostedt <[email protected]> wrote:
Quoted 7 lines

On my 32bit build, I hit a new warning;

kernel/trace/bpf_trace.c:1073:22: warning: ‘ftrace_get_entry_ip’ defined but not used [-Wunused-function]
1073 | static unsigned long ftrace_get_entry_ip(unsigned long fentry_ip)

      |                      ^~~~~~~~~~~~~~~~~~~

Config attached.

Since this is the last patch of the series, just send a new patch for this one. I'll continue testing the rest of the patches.

The tests passed without this patch so I applied them to my for-next branch.

If you fix this, you just need to send this patch only.

Thanks,

-- Steve

author maintainer
#28 lore raw

Re: [PATCH v22 20/20] bpf: Use ftrace_get_symaddr() for kprobe_multi probes

Quoted 9 lines · On Fri, 27 Dec 2024 10:24:52 -0500 Steven Rostedt <[email protected]> wrote:
Quoted 6 lines · On Thu, 26 Dec 2024 21:24:53 -0500 Steven Rostedt <[email protected]> wrote:
Quoted 6 lines

On my 32bit build, I hit a new warning;

kernel/trace/bpf_trace.c:1073:22: warning: ‘ftrace_get_entry_ip’ defined but not used [-Wunused-function]
1073 | static unsigned long ftrace_get_entry_ip(unsigned long fentry_ip)

      |                      ^~~~~~~~~~~~~~~~~~~
Quoted 7 lines
Quoted 5 lines
Quoted 1 line

Config attached.

Since this is the last patch of the series, just send a new patch for this one. I'll continue testing the rest of the patches.

The tests passed without this patch so I applied them to my for-next branch.

Thanks!

Quoted 2 lines

If you fix this, you just need to send this patch only.

OK, let me fix this issue.

Quoted 4 lines

Thanks,

-- Steve

Signature · 1 line
Masami Hiramatsu (Google) <[email protected]>
author maintainer
#29 lore raw

PATCH v23 1/1 bpf: Use ftrace_get_symaddr() for kprobe_multi probes

In reply to Steven Rostedt

From: Masami Hiramatsu (Google) <[email protected]>

Add ftrace_get_entry_ip() which is only for ftrace based probes, and use it for kprobe multi probes because they are based on fprobe which uses ftrace instead of kprobes.

---
Changes in v23:
- Move ftrace_get_symaddr() in CONFIG_FPROBE ifdef block.
Changes in v21:
- Use new ftreace_get_symaddr() only for kprobe multi probes.
Changes in v19:
- Use ftrace_get_symaddr() instead of introducing new arch dependent code.
- Also, replace x86 code with ftrace_get_symaddr(), which does the same thing.

---
 kernel/trace/bpf_trace.c |   13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
kernel/trace/bpf_trace.c +11 -2
index 863351559334..9bfd52913a5b 100644
@@ -2591,6 +2591,13 @@ static DEFINE_PER_CPU(struct pt_regs, bpf_kprobe_multi_pt_regs);
 #define bpf_kprobe_multi_pt_regs_ptr()	(NULL)
 #endif
 
+static unsigned long ftrace_get_entry_ip(unsigned long fentry_ip)
+{
+	unsigned long ip = ftrace_get_symaddr(fentry_ip);
+
+	return ip ? : fentry_ip;
+}
+
 static int copy_user_syms(struct user_syms *us, unsigned long __user *usyms, u32 cnt)
 {
 	unsigned long __user usymbol;
@@ -2829,7 +2836,8 @@ kprobe_multi_link_handler(struct fprobe *fp, unsigned long fentry_ip,
 	int err;
 
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
-	err = kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), fregs, false, data);
+	err = kprobe_multi_link_prog_run(link, ftrace_get_entry_ip(fentry_ip),
+					 fregs, false, data);
 	return is_kprobe_session(link->link.prog) ? err : 0;
 }
 
@@ -2841,7 +2849,8 @@ kprobe_multi_link_exit_handler(struct fprobe *fp, unsigned long fentry_ip,
 	struct bpf_kprobe_multi_link *link;
 
 	link = container_of(fp, struct bpf_kprobe_multi_link, fp);
-	kprobe_multi_link_prog_run(link, get_entry_ip(fentry_ip), fregs, true, data);
+	kprobe_multi_link_prog_run(link, ftrace_get_entry_ip(fentry_ip),
+				   fregs, true, data);
 }
 
 static int symbols_cmp_r(const void *a, const void *b, const void *priv)
#30 lore raw
In reply to Masami Hiramatsu (Google)
Quoted 9 lines · Masami Hiramatsu (Google)

Hi,

Here is the 22nd version of the series to re-implement the fprobe on function-graph tracer. The previous version is;

https://lore.kernel.org/all/173379652547.973433.2311391879173461183.stgit@devnote2/

This version is rebased on v6.13-rc4 with fixes on [3/20] for x86-32 and [5/20] for build error.

hi,
I ran the bench and I'm seeing native_sched_clock being used again kretprobe_multi bench:

     5.85%  bench            [kernel.kallsyms]                                        [k] native_sched_clock
            |
            ---native_sched_clock
               sched_clock
               |
                --5.83%--trace_clock_local
                          ftrace_return_to_handler
                          return_to_handler
                          syscall
                          bpf_prog_test_run_opts
                          trigger_producer_batch
                          start_thread
                          __GI___clone3

I recall we tried to fix that before with [1] change, but that replaced later with [2] changes

When I remove the trace_clock_local call in __ftrace_return_to_handler than the kretprobe-multi gets much faster (see last block below), so it seems worth to make it optional

there's some decrease in kprobe_multi benchmark compared to base numbers, which I'm not sure yet why, but other than that it seems ok

base:

        kprobe         :   12.873 ± 0.011M/s
        kprobe-multi   :   13.088 ± 0.052M/s
        kretprobe      :    6.339 ± 0.003M/s
        kretprobe-multi:    7.240 ± 0.002M/s

fprobe_on_fgraph:

        kprobe         :   12.816 ± 0.002M/s
        kprobe-multi   :   12.126 ± 0.004M/s
        kretprobe      :    6.305 ± 0.018M/s
        kretprobe-multi:    7.740 ± 0.003M/s

removed native_sched_clock call:

        kprobe         :   12.850 ± 0.006M/s
        kprobe-multi   :   12.115 ± 0.006M/s
        kretprobe      :    6.270 ± 0.017M/s
        kretprobe-multi:    9.190 ± 0.005M/s

happy new year ;-) thanks,

jirka

[1] https://lore.kernel.org/bpf/172615389864.133222.14452329708227900626.stgit@devnote2/
[2] https://lore.kernel.org/all/[email protected]/

Quoted 175 lines

Overview
--------
This series rewrites the fprobe on this function-graph. The purposes of this change are;

1) Remove dependency of the rethook from fprobe so that we can reduce the return hook code and shadow stack.

2) Make 'ftrace_regs' the common trace interface for the function boundary.

1) Currently we have 2(or 3) different function return hook codes, the function-graph tracer and rethook (and legacy kretprobe). But since this is redundant and needs double maintenance cost, I would like to unify those. From the user's viewpoint, function- graph tracer is very useful to grasp the execution path. For this purpose, it is hard to use the rethook in the function-graph tracer, but the opposite is possible. (Strictly speaking, kretprobe can not use it because it requires 'pt_regs' for historical reasons.)

2) Now the fprobe provides the 'pt_regs' for its handler, but that is wrong for the function entry and exit. Moreover, depending on the architecture, there is no way to accurately reproduce 'pt_regs' outside of interrupt or exception handlers. This means fprobe should not use 'pt_regs' because it does not use such exceptions. (Conversely, kprobe should use 'pt_regs' because it is an abstract interface of the software breakpoint exception.)

This series changes fprobe to use function-graph tracer for tracing function entry and exit, instead of mixture of ftrace and rethook. Unlike the rethook which is a per-task list of system-wide allocated nodes, the function graph's ret_stack is a per-task shadow stack. Thus it does not need to set 'nr_maxactive' (which is the number of pre-allocated nodes).
Also the handlers will get the 'ftrace_regs' instead of 'pt_regs'. Since eBPF mulit_kprobe/multi_kretprobe events still use 'pt_regs' as their register interface, this changes it to convert 'ftrace_regs' to 'pt_regs'. Of course this conversion makes an incomplete 'pt_regs', so users must access only registers for function parameters or return value.

Design
------
Instead of using ftrace's function entry hook directly, the new fprobe is built on top of the function-graph's entry and return callbacks with 'ftrace_regs'.

Since the fprobe requires access to 'ftrace_regs', the architecture must support CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS and CONFIG_HAVE_FTRACE_GRAPH_FUNC, which enables to call function-graph entry callback with 'ftrace_regs', and also CONFIG_HAVE_FUNCTION_GRAPH_FREGS, which passes the ftrace_regs to return_to_handler.

All fprobes share a single function-graph ops (means shares a common ftrace filter) similar to the kprobe-on-ftrace. This needs another layer to find corresponding fprobe in the common function-graph callbacks, but has much better scalability, since the number of registered function-graph ops is limited.

In the entry callback, the fprobe runs its entry_handler and saves the address of 'fprobe' on the function-graph's shadow stack as data. The return callback decodes the data to get the 'fprobe' address, and runs the exit_handler.

The fprobe introduces two hash-tables, one is for entry callback which searches fprobes related to the given function address passed by entry callback. The other is for a return callback which checks if the given 'fprobe' data structure pointer is still valid. Note that it is possible to unregister fprobe before the return callback runs. Thus the address validation must be done before using it in the return callback.

Download
--------
This series can be applied against the v6.13-rc2 kernel.

This series can also be found below branch.

https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/log/?h=topic/fprobe-on-fgraph

Thank you,

---

Masami Hiramatsu (Google) (19):
fgraph: Get ftrace recursion lock in function_graph_enter
fgraph: Pass ftrace_regs to entryfunc
fgraph: Replace fgraph_ret_regs with ftrace_regs
fgraph: Pass ftrace_regs to retfunc
fprobe: Use ftrace_regs in fprobe entry handler
fprobe: Use ftrace_regs in fprobe exit handler
tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs
tracing: Add ftrace_fill_perf_regs() for perf event tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS
bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled
ftrace: Add CONFIG_HAVE_FTRACE_GRAPH_FUNC
fprobe: Rewrite fprobe on function-graph tracer
fprobe: Add fprobe_header encoding feature
tracing/fprobe: Remove nr_maxactive from fprobe
selftests: ftrace: Remove obsolate maxactive syntax check selftests/ftrace: Add a test case for repeating register/unregister fprobe
Documentation: probes: Update fprobe on function-graph tracer
ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr
bpf: Use ftrace_get_symaddr() for kprobe_multi probes

Sven Schnelle (1):
s390/tracing: Enable HAVE_FTRACE_GRAPH_FUNC

 Documentation/trace/fprobe.rst                     |   42 +
 arch/arm64/Kconfig                                 |    2 
 arch/arm64/include/asm/Kbuild                      |    1 
 arch/arm64/include/asm/ftrace.h                    |   51 +-
 arch/arm64/kernel/asm-offsets.c                    |   12 
 arch/arm64/kernel/entry-ftrace.S                   |   32 +
 arch/arm64/kernel/ftrace.c                         |   78 ++
 arch/loongarch/Kconfig                             |    4 
 arch/loongarch/include/asm/fprobe.h                |   12 
 arch/loongarch/include/asm/ftrace.h                |   32 -
 arch/loongarch/kernel/asm-offsets.c                |   12 
 arch/loongarch/kernel/ftrace_dyn.c                 |   10 
 arch/loongarch/kernel/mcount.S                     |   17 -
 arch/loongarch/kernel/mcount_dyn.S                 |   14 
 arch/powerpc/Kconfig                               |    1 
 arch/powerpc/include/asm/ftrace.h                  |   13 
 arch/powerpc/kernel/trace/ftrace.c                 |    8 
 arch/powerpc/kernel/trace/ftrace_64_pg.c           |   16 
 arch/riscv/Kconfig                                 |    3 
 arch/riscv/include/asm/Kbuild                      |    1 
 arch/riscv/include/asm/ftrace.h                    |   45 +
 arch/riscv/kernel/ftrace.c                         |   17 -
 arch/riscv/kernel/mcount.S                         |   24 -
 arch/s390/Kconfig                                  |    4 
 arch/s390/include/asm/fprobe.h                     |   10 
 arch/s390/include/asm/ftrace.h                     |   37 +
 arch/s390/kernel/asm-offsets.c                     |    6 
 arch/s390/kernel/entry.h                           |    1 
 arch/s390/kernel/ftrace.c                          |   48 -
 arch/s390/kernel/mcount.S                          |   23 -
 arch/x86/Kconfig                                   |    4 
 arch/x86/include/asm/Kbuild                        |    1 
 arch/x86/include/asm/ftrace.h                      |   54 +-
 arch/x86/kernel/ftrace.c                           |   47 +
 arch/x86/kernel/ftrace_32.S                        |   13 
 arch/x86/kernel/ftrace_64.S                        |   17 -
 include/asm-generic/fprobe.h                       |   46 +
 include/linux/fprobe.h                             |   62 +-
 include/linux/ftrace.h                             |  116 +++
 include/linux/ftrace_regs.h                        |    2 
 kernel/trace/Kconfig                               |   22 -
 kernel/trace/bpf_trace.c                           |   28 +
 kernel/trace/fgraph.c                              |   65 +-
 kernel/trace/fprobe.c                              |  664 +++++++++++++++-----
 kernel/trace/ftrace.c                              |    6 
 kernel/trace/trace.h                               |    6 
 kernel/trace/trace_fprobe.c                        |  146 ++--
 kernel/trace/trace_functions_graph.c               |   10 
 kernel/trace/trace_irqsoff.c                       |    6 
 kernel/trace/trace_probe_tmpl.h                    |    2 
 kernel/trace/trace_sched_wakeup.c                  |    6 
 kernel/trace/trace_selftest.c                      |   11 
 lib/test_fprobe.c                                  |   51 --
 samples/fprobe/fprobe_example.c                    |    4 
 .../test.d/dynevent/add_remove_fprobe_repeat.tc    |   19 +
 .../ftrace/test.d/dynevent/fprobe_syntax_errors.tc |    4 
 56 files changed, 1318 insertions(+), 670 deletions(-)
 create mode 100644 arch/loongarch/include/asm/fprobe.h
 create mode 100644 arch/s390/include/asm/fprobe.h
 create mode 100644 include/asm-generic/fprobe.h
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe_repeat.tc

--
Masami Hiramatsu (Google) <[email protected]>

#31 lore raw
Quoted 17 lines · Jiri Olsa
Quoted 9 lines · Masami Hiramatsu (Google)

Hi,

Here is the 22nd version of the series to re-implement the fprobe on function-graph tracer. The previous version is;

https://lore.kernel.org/all/173379652547.973433.2311391879173461183.stgit@devnote2/

This version is rebased on v6.13-rc4 with fixes on [3/20] for x86-32 and [5/20] for build error.

hi,
I ran the bench and I'm seeing native_sched_clock being used again kretprobe_multi bench:

Quoted 10 lines
     5.85%  bench            [kernel.kallsyms]                                        [k] native_sched_clock
            |
            ---native_sched_clock
               sched_clock
               |
                --5.83%--trace_clock_local
                          ftrace_return_to_handler
                          return_to_handler
                          syscall
                          bpf_prog_test_run_opts

completely unrelated, Jiri, but we should stop using bpf_prog_test_run_opts() for benchmarking. It goes through FD refcounting, which is unnecessary tiny overhead, but more importantly it causes cache line bouncing between multiple CPUs (when doing multi-threaded benchmarks), which skews and limits results.

Quoted 41 lines
                          trigger_producer_batch
                          start_thread
                          __GI___clone3

I recall we tried to fix that before with [1] change, but that replaced later with [2] changes

When I remove the trace_clock_local call in __ftrace_return_to_handler than the kretprobe-multi gets much faster (see last block below), so it seems worth to make it optional

there's some decrease in kprobe_multi benchmark compared to base numbers, which I'm not sure yet why, but other than that it seems ok

base:

        kprobe         :   12.873 ± 0.011M/s
        kprobe-multi   :   13.088 ± 0.052M/s
        kretprobe      :    6.339 ± 0.003M/s
        kretprobe-multi:    7.240 ± 0.002M/s

fprobe_on_fgraph:

        kprobe         :   12.816 ± 0.002M/s
        kprobe-multi   :   12.126 ± 0.004M/s
        kretprobe      :    6.305 ± 0.018M/s
        kretprobe-multi:    7.740 ± 0.003M/s

removed native_sched_clock call:

        kprobe         :   12.850 ± 0.006M/s
        kprobe-multi   :   12.115 ± 0.006M/s
        kretprobe      :    6.270 ± 0.017M/s
        kretprobe-multi:    9.190 ± 0.005M/s

happy new year ;-) thanks,

jirka

[1] https://lore.kernel.org/bpf/172615389864.133222.14452329708227900626.stgit@devnote2/
[2] https://lore.kernel.org/all/[email protected]/

[...]

#32 lore raw
Quoted 29 lines · Andrii Nakryiko
Quoted 17 lines · Jiri Olsa
Quoted 9 lines · Masami Hiramatsu (Google)

Hi,

Here is the 22nd version of the series to re-implement the fprobe on function-graph tracer. The previous version is;

https://lore.kernel.org/all/173379652547.973433.2311391879173461183.stgit@devnote2/

This version is rebased on v6.13-rc4 with fixes on [3/20] for x86-32 and [5/20] for build error.

hi,
I ran the bench and I'm seeing native_sched_clock being used again kretprobe_multi bench:

Quoted 10 lines
     5.85%  bench            [kernel.kallsyms]                                        [k] native_sched_clock
            |
            ---native_sched_clock
               sched_clock
               |
                --5.83%--trace_clock_local
                          ftrace_return_to_handler
                          return_to_handler
                          syscall
                          bpf_prog_test_run_opts
Quoted 5 lines

completely unrelated, Jiri, but we should stop using bpf_prog_test_run_opts() for benchmarking. It goes through FD refcounting, which is unnecessary tiny overhead, but more importantly it causes cache line bouncing between multiple CPUs (when doing multi-threaded benchmarks), which skews and limits results.

so you mean to switch directly to attaching/hitting kernel functions or perhaps better have kernel module for that?

jirka

Quoted 44 lines
Quoted 42 lines
                          trigger_producer_batch
                          start_thread
                          __GI___clone3

I recall we tried to fix that before with [1] change, but that replaced later with [2] changes

When I remove the trace_clock_local call in __ftrace_return_to_handler than the kretprobe-multi gets much faster (see last block below), so it seems worth to make it optional

there's some decrease in kprobe_multi benchmark compared to base numbers, which I'm not sure yet why, but other than that it seems ok

base:

        kprobe         :   12.873 ± 0.011M/s
        kprobe-multi   :   13.088 ± 0.052M/s
        kretprobe      :    6.339 ± 0.003M/s
        kretprobe-multi:    7.240 ± 0.002M/s

fprobe_on_fgraph:

        kprobe         :   12.816 ± 0.002M/s
        kprobe-multi   :   12.126 ± 0.004M/s
        kretprobe      :    6.305 ± 0.018M/s
        kretprobe-multi:    7.740 ± 0.003M/s

removed native_sched_clock call:

        kprobe         :   12.850 ± 0.006M/s
        kprobe-multi   :   12.115 ± 0.006M/s
        kretprobe      :    6.270 ± 0.017M/s
        kretprobe-multi:    9.190 ± 0.005M/s

happy new year ;-) thanks,

jirka

[1] https://lore.kernel.org/bpf/172615389864.133222.14452329708227900626.stgit@devnote2/
[2] https://lore.kernel.org/all/[email protected]/

[...]

#33 lore raw
Quoted 31 lines · Jiri Olsa
Quoted 29 lines · Andrii Nakryiko
Quoted 27 lines · Jiri Olsa
Quoted 9 lines · Masami Hiramatsu (Google)

Hi,

Here is the 22nd version of the series to re-implement the fprobe on function-graph tracer. The previous version is;

https://lore.kernel.org/all/173379652547.973433.2311391879173461183.stgit@devnote2/

This version is rebased on v6.13-rc4 with fixes on [3/20] for x86-32 and [5/20] for build error.

hi,
I ran the bench and I'm seeing native_sched_clock being used again kretprobe_multi bench:

     5.85%  bench            [kernel.kallsyms]                                        [k] native_sched_clock
            |
            ---native_sched_clock
               sched_clock
               |
                --5.83%--trace_clock_local
                          ftrace_return_to_handler
                          return_to_handler
                          syscall
                          bpf_prog_test_run_opts
Quoted 9 lines
Quoted 5 lines

completely unrelated, Jiri, but we should stop using bpf_prog_test_run_opts() for benchmarking. It goes through FD refcounting, which is unnecessary tiny overhead, but more importantly it causes cache line bouncing between multiple CPUs (when doing multi-threaded benchmarks), which skews and limits results.

so you mean to switch directly to attaching/hitting kernel functions or perhaps better have kernel module for that?

yes, cheap syscall (getpgid or something). Not a kernel module, that's logistical hassle.

Quoted 46 lines

jirka

Quoted 44 lines
Quoted 42 lines
                          trigger_producer_batch
                          start_thread
                          __GI___clone3

I recall we tried to fix that before with [1] change, but that replaced later with [2] changes

When I remove the trace_clock_local call in __ftrace_return_to_handler than the kretprobe-multi gets much faster (see last block below), so it seems worth to make it optional

there's some decrease in kprobe_multi benchmark compared to base numbers, which I'm not sure yet why, but other than that it seems ok

base:

        kprobe         :   12.873 ± 0.011M/s
        kprobe-multi   :   13.088 ± 0.052M/s
        kretprobe      :    6.339 ± 0.003M/s
        kretprobe-multi:    7.240 ± 0.002M/s

fprobe_on_fgraph:

        kprobe         :   12.816 ± 0.002M/s
        kprobe-multi   :   12.126 ± 0.004M/s
        kretprobe      :    6.305 ± 0.018M/s
        kretprobe-multi:    7.740 ± 0.003M/s

removed native_sched_clock call:

        kprobe         :   12.850 ± 0.006M/s
        kprobe-multi   :   12.115 ± 0.006M/s
        kretprobe      :    6.270 ± 0.017M/s
        kretprobe-multi:    9.190 ± 0.005M/s

happy new year ;-) thanks,

jirka

[1] https://lore.kernel.org/bpf/172615389864.133222.14452329708227900626.stgit@devnote2/
[2] https://lore.kernel.org/all/[email protected]/

[...]

author maintainer
#34 lore raw
In reply to Jiri Olsa
Quoted 52 lines · On Thu, 2 Jan 2025 14:20:58 +0100 Jiri Olsa <[email protected]> wrote:
Quoted 9 lines · Masami Hiramatsu (Google)

Hi,

Here is the 22nd version of the series to re-implement the fprobe on function-graph tracer. The previous version is;

https://lore.kernel.org/all/173379652547.973433.2311391879173461183.stgit@devnote2/

This version is rebased on v6.13-rc4 with fixes on [3/20] for x86-32 and [5/20] for build error.

hi,
I ran the bench and I'm seeing native_sched_clock being used again kretprobe_multi bench:

     5.85%  bench            [kernel.kallsyms]                                        [k] native_sched_clock
            |
            ---native_sched_clock
               sched_clock
               |
                --5.83%--trace_clock_local
                          ftrace_return_to_handler
                          return_to_handler
                          syscall
                          bpf_prog_test_run_opts
                          trigger_producer_batch
                          start_thread
                          __GI___clone3

I recall we tried to fix that before with [1] change, but that replaced later with [2] changes

When I remove the trace_clock_local call in __ftrace_return_to_handler than the kretprobe-multi gets much faster (see last block below), so it seems worth to make it optional

there's some decrease in kprobe_multi benchmark compared to base numbers, which I'm not sure yet why, but other than that it seems ok

base:

        kprobe         :   12.873 ± 0.011M/s
        kprobe-multi   :   13.088 ± 0.052M/s
        kretprobe      :    6.339 ± 0.003M/s
        kretprobe-multi:    7.240 ± 0.002M/s

fprobe_on_fgraph:

        kprobe         :   12.816 ± 0.002M/s
        kprobe-multi   :   12.126 ± 0.004M/s
        kretprobe      :    6.305 ± 0.018M/s
        kretprobe-multi:    7.740 ± 0.003M/s
Quoted 5 lines

removed native_sched_clock call:

        kprobe         :   12.850 ± 0.006M/s
        kprobe-multi   :   12.115 ± 0.006M/s
        kretprobe      :    6.270 ± 0.017M/s
        kretprobe-multi:    9.190 ± 0.005M/s

Hmm, this still look good. Ok, let me move the trace_clock_local() to the function_graph.

Thanks!

Quoted 185 lines

happy new year ;-) thanks,

jirka

[1] https://lore.kernel.org/bpf/172615389864.133222.14452329708227900626.stgit@devnote2/
[2] https://lore.kernel.org/all/[email protected]/

Quoted 175 lines

Overview
--------
This series rewrites the fprobe on this function-graph. The purposes of this change are;

1) Remove dependency of the rethook from fprobe so that we can reduce the return hook code and shadow stack.

2) Make 'ftrace_regs' the common trace interface for the function boundary.

1) Currently we have 2(or 3) different function return hook codes, the function-graph tracer and rethook (and legacy kretprobe). But since this is redundant and needs double maintenance cost, I would like to unify those. From the user's viewpoint, function- graph tracer is very useful to grasp the execution path. For this purpose, it is hard to use the rethook in the function-graph tracer, but the opposite is possible. (Strictly speaking, kretprobe can not use it because it requires 'pt_regs' for historical reasons.)

2) Now the fprobe provides the 'pt_regs' for its handler, but that is wrong for the function entry and exit. Moreover, depending on the architecture, there is no way to accurately reproduce 'pt_regs' outside of interrupt or exception handlers. This means fprobe should not use 'pt_regs' because it does not use such exceptions. (Conversely, kprobe should use 'pt_regs' because it is an abstract interface of the software breakpoint exception.)

This series changes fprobe to use function-graph tracer for tracing function entry and exit, instead of mixture of ftrace and rethook. Unlike the rethook which is a per-task list of system-wide allocated nodes, the function graph's ret_stack is a per-task shadow stack. Thus it does not need to set 'nr_maxactive' (which is the number of pre-allocated nodes).
Also the handlers will get the 'ftrace_regs' instead of 'pt_regs'. Since eBPF mulit_kprobe/multi_kretprobe events still use 'pt_regs' as their register interface, this changes it to convert 'ftrace_regs' to 'pt_regs'. Of course this conversion makes an incomplete 'pt_regs', so users must access only registers for function parameters or return value.

Design
------
Instead of using ftrace's function entry hook directly, the new fprobe is built on top of the function-graph's entry and return callbacks with 'ftrace_regs'.

Since the fprobe requires access to 'ftrace_regs', the architecture must support CONFIG_HAVE_DYNAMIC_FTRACE_WITH_ARGS and CONFIG_HAVE_FTRACE_GRAPH_FUNC, which enables to call function-graph entry callback with 'ftrace_regs', and also CONFIG_HAVE_FUNCTION_GRAPH_FREGS, which passes the ftrace_regs to return_to_handler.

All fprobes share a single function-graph ops (means shares a common ftrace filter) similar to the kprobe-on-ftrace. This needs another layer to find corresponding fprobe in the common function-graph callbacks, but has much better scalability, since the number of registered function-graph ops is limited.

In the entry callback, the fprobe runs its entry_handler and saves the address of 'fprobe' on the function-graph's shadow stack as data. The return callback decodes the data to get the 'fprobe' address, and runs the exit_handler.

The fprobe introduces two hash-tables, one is for entry callback which searches fprobes related to the given function address passed by entry callback. The other is for a return callback which checks if the given 'fprobe' data structure pointer is still valid. Note that it is possible to unregister fprobe before the return callback runs. Thus the address validation must be done before using it in the return callback.

Download
--------
This series can be applied against the v6.13-rc2 kernel.

This series can also be found below branch.

https://git.kernel.org/pub/scm/linux/kernel/git/mhiramat/linux.git/log/?h=topic/fprobe-on-fgraph

Thank you,

---

Masami Hiramatsu (Google) (19):
fgraph: Get ftrace recursion lock in function_graph_enter
fgraph: Pass ftrace_regs to entryfunc
fgraph: Replace fgraph_ret_regs with ftrace_regs
fgraph: Pass ftrace_regs to retfunc
fprobe: Use ftrace_regs in fprobe entry handler
fprobe: Use ftrace_regs in fprobe exit handler
tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs
tracing: Add ftrace_fill_perf_regs() for perf event tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS
bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled
ftrace: Add CONFIG_HAVE_FTRACE_GRAPH_FUNC
fprobe: Rewrite fprobe on function-graph tracer
fprobe: Add fprobe_header encoding feature
tracing/fprobe: Remove nr_maxactive from fprobe
selftests: ftrace: Remove obsolate maxactive syntax check selftests/ftrace: Add a test case for repeating register/unregister fprobe
Documentation: probes: Update fprobe on function-graph tracer
ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr
bpf: Use ftrace_get_symaddr() for kprobe_multi probes

Sven Schnelle (1):
s390/tracing: Enable HAVE_FTRACE_GRAPH_FUNC

 Documentation/trace/fprobe.rst                     |   42 +
 arch/arm64/Kconfig                                 |    2 
 arch/arm64/include/asm/Kbuild                      |    1 
 arch/arm64/include/asm/ftrace.h                    |   51 +-
 arch/arm64/kernel/asm-offsets.c                    |   12 
 arch/arm64/kernel/entry-ftrace.S                   |   32 +
 arch/arm64/kernel/ftrace.c                         |   78 ++
 arch/loongarch/Kconfig                             |    4 
 arch/loongarch/include/asm/fprobe.h                |   12 
 arch/loongarch/include/asm/ftrace.h                |   32 -
 arch/loongarch/kernel/asm-offsets.c                |   12 
 arch/loongarch/kernel/ftrace_dyn.c                 |   10 
 arch/loongarch/kernel/mcount.S                     |   17 -
 arch/loongarch/kernel/mcount_dyn.S                 |   14 
 arch/powerpc/Kconfig                               |    1 
 arch/powerpc/include/asm/ftrace.h                  |   13 
 arch/powerpc/kernel/trace/ftrace.c                 |    8 
 arch/powerpc/kernel/trace/ftrace_64_pg.c           |   16 
 arch/riscv/Kconfig                                 |    3 
 arch/riscv/include/asm/Kbuild                      |    1 
 arch/riscv/include/asm/ftrace.h                    |   45 +
 arch/riscv/kernel/ftrace.c                         |   17 -
 arch/riscv/kernel/mcount.S                         |   24 -
 arch/s390/Kconfig                                  |    4 
 arch/s390/include/asm/fprobe.h                     |   10 
 arch/s390/include/asm/ftrace.h                     |   37 +
 arch/s390/kernel/asm-offsets.c                     |    6 
 arch/s390/kernel/entry.h                           |    1 
 arch/s390/kernel/ftrace.c                          |   48 -
 arch/s390/kernel/mcount.S                          |   23 -
 arch/x86/Kconfig                                   |    4 
 arch/x86/include/asm/Kbuild                        |    1 
 arch/x86/include/asm/ftrace.h                      |   54 +-
 arch/x86/kernel/ftrace.c                           |   47 +
 arch/x86/kernel/ftrace_32.S                        |   13 
 arch/x86/kernel/ftrace_64.S                        |   17 -
 include/asm-generic/fprobe.h                       |   46 +
 include/linux/fprobe.h                             |   62 +-
 include/linux/ftrace.h                             |  116 +++
 include/linux/ftrace_regs.h                        |    2 
 kernel/trace/Kconfig                               |   22 -
 kernel/trace/bpf_trace.c                           |   28 +
 kernel/trace/fgraph.c                              |   65 +-
 kernel/trace/fprobe.c                              |  664 +++++++++++++++-----
 kernel/trace/ftrace.c                              |    6 
 kernel/trace/trace.h                               |    6 
 kernel/trace/trace_fprobe.c                        |  146 ++--
 kernel/trace/trace_functions_graph.c               |   10 
 kernel/trace/trace_irqsoff.c                       |    6 
 kernel/trace/trace_probe_tmpl.h                    |    2 
 kernel/trace/trace_sched_wakeup.c                  |    6 
 kernel/trace/trace_selftest.c                      |   11 
 lib/test_fprobe.c                                  |   51 --
 samples/fprobe/fprobe_example.c                    |    4 
 .../test.d/dynevent/add_remove_fprobe_repeat.tc    |   19 +
 .../ftrace/test.d/dynevent/fprobe_syntax_errors.tc |    4 
 56 files changed, 1318 insertions(+), 670 deletions(-)
 create mode 100644 arch/loongarch/include/asm/fprobe.h
 create mode 100644 arch/s390/include/asm/fprobe.h
 create mode 100644 include/asm-generic/fprobe.h
 create mode 100644 tools/testing/selftests/ftrace/test.d/dynevent/add_remove_fprobe_repeat.tc

--
Masami Hiramatsu (Google) <[email protected]>

Signature · 1 line
Masami Hiramatsu (Google) <[email protected]>
author maintainer
#35 lore raw

PATCH 1/1 fgraph: Move trace_clock_local() for return time to function_graph tracer

In reply to Jiri Olsa

From: Masami Hiramatsu (Google) <[email protected]>

Since the ftrace_graph_ret::rettime is only referred in the function_graph tracer, the trace_clock_local() call in fgraph is just an overhead for other fgraph users.

Move the trace_clock_local() for recording return time to function_graph tracer and the rettime field is just zeroed in the fgraph side. That rettime field is updated by one of the function_graph tracer and cached for other function_graph tracer instances.

According to Jiri's report[1], removing this function will gain fprobe performance ~27%.

[1] https://lore.kernel.org/all/Z3aSuql3fnXMVMoM@krava/

---
 include/linux/ftrace.h               |   11 +++++++++++
 kernel/trace/fgraph.c                |    2 +-
 kernel/trace/trace_functions_graph.c |    2 ++
 3 files changed, 14 insertions(+), 1 deletion(-)
include/linux/ftrace.h +11 -0
index 07092dfb21a4..a2fb7360d6e2 100644
@@ -1155,6 +1155,17 @@ struct ftrace_graph_ret {
 	unsigned long long rettime;
 } __packed;
 
+static inline void ftrace_graph_ret_record_time(struct ftrace_graph_ret *trace)
+{
+	if (!trace->rettime)
+		trace->rettime = trace_clock_local();
+}
+
+static inline void ftrace_graph_ret_init_time(struct ftrace_graph_ret *trace)
+{
+	trace->rettime = 0;
+}
+
 struct fgraph_ops;
 
 /* Type of the callback handlers for tracing function graph*/
kernel/trace/fgraph.c +1 -1
index c928527251e3..bc1e5f0493b6 100644
@@ -826,7 +826,7 @@ __ftrace_return_to_handler(struct ftrace_regs *fregs, unsigned long frame_pointe
 		return (unsigned long)panic;
 	}
 
-	trace.rettime = trace_clock_local();
+	ftrace_graph_ret_init_time(&trace);
 	if (fregs)
 		ftrace_regs_set_instruction_pointer(fregs, ret);
 
kernel/trace/trace_functions_graph.c +2 -0
index d0e4f412c298..7e4d91d42d8e 100644
@@ -327,6 +327,7 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
 	int size;
 	int cpu;
 
+	ftrace_graph_ret_record_time(trace);
 	ftrace_graph_addr_finish(gops, trace);
 
 	if (*task_var & TRACE_GRAPH_NOTRACE) {
@@ -361,6 +362,7 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
 	struct fgraph_times *ftimes;
 	int size;
 
+	ftrace_graph_ret_record_time(trace);
 	ftrace_graph_addr_finish(gops, trace);
 
 	if (trace_recursion_test(TRACE_GRAPH_NOTRACE_BIT)) {
maintainer
#36 lore raw

Re: [PATCH] fgraph: Move trace_clock_local() for return time to function_graph tracer

Quoted 11 lines · On Sun, 12 Jan 2025 14:26:35 +0900 "Masami Hiramatsu (Google)"…

From: Masami Hiramatsu (Google) <[email protected]>

Since the ftrace_graph_ret::rettime is only referred in the function_graph tracer, the trace_clock_local() call in fgraph is just an overhead for other fgraph users.

Move the trace_clock_local() for recording return time to function_graph tracer and the rettime field is just zeroed in the fgraph side. That rettime field is updated by one of the function_graph tracer and cached for other function_graph tracer instances.

Quoted 7 lines

According to Jiri's report[1], removing this function will gain fprobe performance ~27%.

[1] https://lore.kernel.org/all/Z3aSuql3fnXMVMoM@krava/

I'd rather just nuke the calltime and rettime from struct ftrace_graph_ret.

How about something like this instead?

[ based on: https://lore.kernel.org/linux-trace-kernel/[email protected]/ ]

I can start testing it (it compiles and boots).

 include/linux/ftrace.h               |  2 --
 kernel/trace/fgraph.c                |  1 -
 kernel/trace/trace.h                 |  4 +++-
 kernel/trace/trace_entries.h         |  8 +++----
 kernel/trace/trace_functions_graph.c | 33 ++++++++++++++++------------
 kernel/trace/trace_irqsoff.c         |  5 +++--
 kernel/trace/trace_sched_wakeup.c    |  6 +++--
 7 files changed, 33 insertions(+), 26 deletions(-)
include/linux/ftrace.h +0 -2
index aa9ddd1e4bb6..848ddf0d8f89 100644
@@ -1061,8 +1061,6 @@ struct ftrace_graph_ret {
 	int depth;
 	/* Number of functions that overran the depth limit for current task */
 	unsigned int overrun;
-	unsigned long long calltime;
-	unsigned long long rettime;
 } __packed;
 
 struct fgraph_ops;
kernel/trace/fgraph.c +0 -1
index 30e3ddc8a8a8..a7e6c8488b7f 100644
@@ -818,7 +818,6 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs
 		return (unsigned long)panic;
 	}
 
-	trace.rettime = trace_clock_local();
 #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
 	trace.retval = fgraph_ret_regs_return_value(ret_regs);
 #endif
kernel/trace/trace.h +3 -1
index 9691b47b5f3d..467a8143fbb9 100644
@@ -911,7 +911,9 @@ extern int __trace_graph_retaddr_entry(struct trace_array *tr,
 				unsigned long retaddr);
 extern void __trace_graph_return(struct trace_array *tr,
 				 struct ftrace_graph_ret *trace,
-				 unsigned int trace_ctx);
+				 unsigned int trace_ctx,
+				 u64 calltime, u64 rettime);
+
 extern void init_array_fgraph_ops(struct trace_array *tr, struct ftrace_ops *ops);
 extern int allocate_fgraph_ops(struct trace_array *tr, struct ftrace_ops *ops);
 extern void free_fgraph_ops(struct trace_array *tr);
kernel/trace/trace_entries.h +4 -4
index 82fd174ebbe0..fbfb396905a6 100644
@@ -124,8 +124,8 @@ FTRACE_ENTRY_PACKED(funcgraph_exit, ftrace_graph_ret_entry,
 		__field_packed(	unsigned long,	ret,		retval	)
 		__field_packed(	int,		ret,		depth	)
 		__field_packed(	unsigned int,	ret,		overrun	)
-		__field_packed(	unsigned long long, ret,	calltime)
-		__field_packed(	unsigned long long, ret,	rettime	)
+		__field(unsigned long long,	calltime		)
+		__field(unsigned long long,	rettime			)
 	),
 
 	F_printk("<-- %ps (%d) (start: %llx  end: %llx) over: %d retval: %lx",
@@ -146,8 +146,8 @@ FTRACE_ENTRY_PACKED(funcgraph_exit, ftrace_graph_ret_entry,
 		__field_packed(	unsigned long,	ret,		func	)
 		__field_packed(	int,		ret,		depth	)
 		__field_packed(	unsigned int,	ret,		overrun	)
-		__field_packed(	unsigned long long, ret,	calltime)
-		__field_packed(	unsigned long long, ret,	rettime	)
+		__field(unsigned long long,	calltime		)
+		__field(unsigned long long,	rettime			)
 	),
 
 	F_printk("<-- %ps (%d) (start: %llx  end: %llx) over: %d",
kernel/trace/trace_functions_graph.c +19 -14
index 5504b5e4e7b4..7bff133c1543 100644
@@ -270,12 +270,10 @@ __trace_graph_function(struct trace_array *tr,
 	struct ftrace_graph_ret ret = {
 		.func     = ip,
 		.depth    = 0,
-		.calltime = time,
-		.rettime  = time,
 	};
 
 	__trace_graph_entry(tr, &ent, trace_ctx);
-	__trace_graph_return(tr, &ret, trace_ctx);
+	__trace_graph_return(tr, &ret, trace_ctx, time, time);
 }
 
 void
@@ -287,8 +285,9 @@ trace_graph_function(struct trace_array *tr,
 }
 
 void __trace_graph_return(struct trace_array *tr,
-				struct ftrace_graph_ret *trace,
-				unsigned int trace_ctx)
+			  struct ftrace_graph_ret *trace,
+			  unsigned int trace_ctx,
+			  u64 calltime, u64 rettime)
 {
 	struct ring_buffer_event *event;
 	struct trace_buffer *buffer = tr->array_buffer.buffer;
@@ -300,6 +299,8 @@ void __trace_graph_return(struct trace_array *tr,
 		return;
 	entry	= ring_buffer_event_data(event);
 	entry->ret				= *trace;
+	entry->calltime				= calltime;
+	entry->rettime				= rettime;
 	trace_buffer_unlock_commit_nostack(buffer, event);
 }
 
@@ -322,10 +323,13 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
 	struct fgraph_times *ftimes;
 	unsigned long flags;
 	unsigned int trace_ctx;
+	u64 calltime, rettime;
 	long disabled;
 	int size;
 	int cpu;
 
+	rettime = trace_clock_local();
+
 	ftrace_graph_addr_finish(gops, trace);
 
 	if (*task_var & TRACE_GRAPH_NOTRACE) {
@@ -339,7 +343,7 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
 
 	handle_nosleeptime(trace, ftimes, size);
 
-	trace->calltime = ftimes->calltime;
+	calltime = ftimes->calltime;
 
 	local_irq_save(flags);
 	cpu = raw_smp_processor_id();
@@ -347,7 +351,7 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
 	disabled = atomic_inc_return(&data->disabled);
 	if (likely(disabled == 1)) {
 		trace_ctx = tracing_gen_ctx_flags(flags);
-		__trace_graph_return(tr, trace, trace_ctx);
+		__trace_graph_return(tr, trace, trace_ctx, calltime, rettime);
 	}
 	atomic_dec(&data->disabled);
 	local_irq_restore(flags);
@@ -372,10 +376,8 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
 
 	handle_nosleeptime(trace, ftimes, size);
 
-	trace->calltime = ftimes->calltime;
-
 	if (tracing_thresh &&
-	    (trace->rettime - ftimes->calltime < tracing_thresh))
+	    (trace_clock_local() - ftimes->calltime < tracing_thresh))
 		return;
 	else
 		trace_graph_return(trace, gops);
@@ -861,7 +863,7 @@ print_graph_entry_leaf(struct trace_iterator *iter,
 
 	graph_ret = &ret_entry->ret;
 	call = &entry->graph_ent;
-	duration = graph_ret->rettime - graph_ret->calltime;
+	duration = ret_entry->rettime - ret_entry->calltime;
 
 	func = call->func + iter->tr->text_delta;
 
@@ -1142,11 +1144,14 @@ print_graph_entry(struct ftrace_graph_ent_entry *field, struct trace_seq *s,
 }
 
 static enum print_line_t
-print_graph_return(struct ftrace_graph_ret *trace, struct trace_seq *s,
+print_graph_return(struct ftrace_graph_ret_entry *retentry, struct trace_seq *s,
 		   struct trace_entry *ent, struct trace_iterator *iter,
 		   u32 flags)
 {
-	unsigned long long duration = trace->rettime - trace->calltime;
+	struct ftrace_graph_ret *trace = &retentry->ret;
+	u64 calltime = retentry->calltime;
+	u64 rettime = retentry->rettime;
+	unsigned long long duration = rettime - calltime;
 	struct fgraph_data *data = iter->private;
 	struct trace_array *tr = iter->tr;
 	unsigned long func;
@@ -1347,7 +1352,7 @@ print_graph_function_flags(struct trace_iterator *iter, u32 flags)
 	case TRACE_GRAPH_RET: {
 		struct ftrace_graph_ret_entry *field;
 		trace_assign_type(field, entry);
-		return print_graph_return(&field->ret, s, entry, iter, flags);
+		return print_graph_return(field, s, entry, iter, flags);
 	}
 	case TRACE_STACK:
 	case TRACE_FN:
kernel/trace/trace_irqsoff.c +3 -2
index a4e799c1e767..1d3646ce9c34 100644
@@ -221,6 +221,7 @@ static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
 	unsigned long flags;
 	unsigned int trace_ctx;
 	u64 *calltime;
+	u64 rettime;
 	int size;
 
 	ftrace_graph_addr_finish(gops, trace);
@@ -228,13 +229,13 @@ static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
 	if (!func_prolog_dec(tr, &data, &flags))
 		return;
 
+	rettime = trace_clock_local();
 	calltime = fgraph_retrieve_data(gops->idx, &size);
 	if (!calltime)
 		return;
-	trace->calltime = *calltime;
 
 	trace_ctx = tracing_gen_ctx_flags(flags);
-	__trace_graph_return(tr, trace, trace_ctx);
+	__trace_graph_return(tr, trace, trace_ctx, *calltime, rettime);
 	atomic_dec(&data->disabled);
 }
 
kernel/trace/trace_sched_wakeup.c +4 -2
index c58292e424d5..c4bbaae2a2a3 100644
@@ -156,6 +156,7 @@ static void wakeup_graph_return(struct ftrace_graph_ret *trace,
 	struct trace_array_cpu *data;
 	unsigned int trace_ctx;
 	u64 *calltime;
+	u64 rettime;
 	int size;
 
 	ftrace_graph_addr_finish(gops, trace);
@@ -163,12 +164,13 @@ static void wakeup_graph_return(struct ftrace_graph_ret *trace,
 	if (!func_prolog_preempt_disable(tr, &data, &trace_ctx))
 		return;
 
+	rettime = trace_clock_local();
+
 	calltime = fgraph_retrieve_data(gops->idx, &size);
 	if (!calltime)
 		return;
-	trace->calltime = *calltime;
 
-	__trace_graph_return(tr, trace, trace_ctx);
+	__trace_graph_return(tr, trace, trace_ctx, *calltime, rettime);
 	atomic_dec(&data->disabled);
 
 	preempt_enable_notrace();
Signature · 3 lines
2.45.2

-- Steve
author maintainer Reviewed-by
#37 lore raw

Re: [PATCH] fgraph: Move trace_clock_local() for return time to function_graph tracer

Quoted 17 lines · On Mon, 13 Jan 2025 19:54:49 -0500 Steven Rostedt <[email protected]> wrote:
Quoted 14 lines · On Sun, 12 Jan 2025 14:26:35 +0900 "Masami Hiramatsu (Google)"…

From: Masami Hiramatsu (Google) <[email protected]>

Since the ftrace_graph_ret::rettime is only referred in the function_graph tracer, the trace_clock_local() call in fgraph is just an overhead for other fgraph users.

Move the trace_clock_local() for recording return time to function_graph tracer and the rettime field is just zeroed in the fgraph side. That rettime field is updated by one of the function_graph tracer and cached for other function_graph tracer instances.

According to Jiri's report[1], removing this function will gain fprobe performance ~27%.

Quoted 6 lines
Quoted 4 lines

[1] https://lore.kernel.org/all/Z3aSuql3fnXMVMoM@krava/

I'd rather just nuke the calltime and rettime from struct ftrace_graph_ret.

Yeah, this one is good to me. But this will make a slightly different time for the same function when we enable multiple function graph tracer on different instances (mine records it once and reuse it but this will record on each instance). We may need to notice it on the document. :)

Thank you,

Quoted 260 lines

How about something like this instead?

[ based on: https://lore.kernel.org/linux-trace-kernel/[email protected]/ ]

I can start testing it (it compiles and boots).

 include/linux/ftrace.h               |  2 --
 kernel/trace/fgraph.c                |  1 -
 kernel/trace/trace.h                 |  4 +++-
 kernel/trace/trace_entries.h         |  8 +++----
 kernel/trace/trace_functions_graph.c | 33 ++++++++++++++++------------
 kernel/trace/trace_irqsoff.c         |  5 +++--
 kernel/trace/trace_sched_wakeup.c    |  6 +++--
 7 files changed, 33 insertions(+), 26 deletions(-)
include/linux/ftrace.h +0 -2
index aa9ddd1e4bb6..848ddf0d8f89 100644
@@ -1061,8 +1061,6 @@ struct ftrace_graph_ret {
 	int depth;
 	/* Number of functions that overran the depth limit for current task */
 	unsigned int overrun;
-	unsigned long long calltime;
-	unsigned long long rettime;
 } __packed;
 
 struct fgraph_ops;
kernel/trace/fgraph.c +0 -1
index 30e3ddc8a8a8..a7e6c8488b7f 100644
@@ -818,7 +818,6 @@ static unsigned long __ftrace_return_to_handler(struct fgraph_ret_regs *ret_regs
 		return (unsigned long)panic;
 	}
 
-	trace.rettime = trace_clock_local();
 #ifdef CONFIG_FUNCTION_GRAPH_RETVAL
 	trace.retval = fgraph_ret_regs_return_value(ret_regs);
 #endif
kernel/trace/trace.h +3 -1
index 9691b47b5f3d..467a8143fbb9 100644
@@ -911,7 +911,9 @@ extern int __trace_graph_retaddr_entry(struct trace_array *tr,
 				unsigned long retaddr);
 extern void __trace_graph_return(struct trace_array *tr,
 				 struct ftrace_graph_ret *trace,
-				 unsigned int trace_ctx);
+				 unsigned int trace_ctx,
+				 u64 calltime, u64 rettime);
+
 extern void init_array_fgraph_ops(struct trace_array *tr, struct ftrace_ops *ops);
 extern int allocate_fgraph_ops(struct trace_array *tr, struct ftrace_ops *ops);
 extern void free_fgraph_ops(struct trace_array *tr);
kernel/trace/trace_entries.h +4 -4
index 82fd174ebbe0..fbfb396905a6 100644
@@ -124,8 +124,8 @@ FTRACE_ENTRY_PACKED(funcgraph_exit, ftrace_graph_ret_entry,
 		__field_packed(	unsigned long,	ret,		retval	)
 		__field_packed(	int,		ret,		depth	)
 		__field_packed(	unsigned int,	ret,		overrun	)
-		__field_packed(	unsigned long long, ret,	calltime)
-		__field_packed(	unsigned long long, ret,	rettime	)
+		__field(unsigned long long,	calltime		)
+		__field(unsigned long long,	rettime			)
 	),
 
 	F_printk("<-- %ps (%d) (start: %llx  end: %llx) over: %d retval: %lx",
@@ -146,8 +146,8 @@ FTRACE_ENTRY_PACKED(funcgraph_exit, ftrace_graph_ret_entry,
 		__field_packed(	unsigned long,	ret,		func	)
 		__field_packed(	int,		ret,		depth	)
 		__field_packed(	unsigned int,	ret,		overrun	)
-		__field_packed(	unsigned long long, ret,	calltime)
-		__field_packed(	unsigned long long, ret,	rettime	)
+		__field(unsigned long long,	calltime		)
+		__field(unsigned long long,	rettime			)
 	),
 
 	F_printk("<-- %ps (%d) (start: %llx  end: %llx) over: %d",
kernel/trace/trace_functions_graph.c +19 -14
index 5504b5e4e7b4..7bff133c1543 100644
@@ -270,12 +270,10 @@ __trace_graph_function(struct trace_array *tr,
 	struct ftrace_graph_ret ret = {
 		.func     = ip,
 		.depth    = 0,
-		.calltime = time,
-		.rettime  = time,
 	};
 
 	__trace_graph_entry(tr, &ent, trace_ctx);
-	__trace_graph_return(tr, &ret, trace_ctx);
+	__trace_graph_return(tr, &ret, trace_ctx, time, time);
 }
 
 void
@@ -287,8 +285,9 @@ trace_graph_function(struct trace_array *tr,
 }
 
 void __trace_graph_return(struct trace_array *tr,
-				struct ftrace_graph_ret *trace,
-				unsigned int trace_ctx)
+			  struct ftrace_graph_ret *trace,
+			  unsigned int trace_ctx,
+			  u64 calltime, u64 rettime)
 {
 	struct ring_buffer_event *event;
 	struct trace_buffer *buffer = tr->array_buffer.buffer;
@@ -300,6 +299,8 @@ void __trace_graph_return(struct trace_array *tr,
 		return;
 	entry	= ring_buffer_event_data(event);
 	entry->ret				= *trace;
+	entry->calltime				= calltime;
+	entry->rettime				= rettime;
 	trace_buffer_unlock_commit_nostack(buffer, event);
 }
 
@@ -322,10 +323,13 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
 	struct fgraph_times *ftimes;
 	unsigned long flags;
 	unsigned int trace_ctx;
+	u64 calltime, rettime;
 	long disabled;
 	int size;
 	int cpu;
 
+	rettime = trace_clock_local();
+
 	ftrace_graph_addr_finish(gops, trace);
 
 	if (*task_var & TRACE_GRAPH_NOTRACE) {
@@ -339,7 +343,7 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
 
 	handle_nosleeptime(trace, ftimes, size);
 
-	trace->calltime = ftimes->calltime;
+	calltime = ftimes->calltime;
 
 	local_irq_save(flags);
 	cpu = raw_smp_processor_id();
@@ -347,7 +351,7 @@ void trace_graph_return(struct ftrace_graph_ret *trace,
 	disabled = atomic_inc_return(&data->disabled);
 	if (likely(disabled == 1)) {
 		trace_ctx = tracing_gen_ctx_flags(flags);
-		__trace_graph_return(tr, trace, trace_ctx);
+		__trace_graph_return(tr, trace, trace_ctx, calltime, rettime);
 	}
 	atomic_dec(&data->disabled);
 	local_irq_restore(flags);
@@ -372,10 +376,8 @@ static void trace_graph_thresh_return(struct ftrace_graph_ret *trace,
 
 	handle_nosleeptime(trace, ftimes, size);
 
-	trace->calltime = ftimes->calltime;
-
 	if (tracing_thresh &&
-	    (trace->rettime - ftimes->calltime < tracing_thresh))
+	    (trace_clock_local() - ftimes->calltime < tracing_thresh))
 		return;
 	else
 		trace_graph_return(trace, gops);
@@ -861,7 +863,7 @@ print_graph_entry_leaf(struct trace_iterator *iter,
 
 	graph_ret = &ret_entry->ret;
 	call = &entry->graph_ent;
-	duration = graph_ret->rettime - graph_ret->calltime;
+	duration = ret_entry->rettime - ret_entry->calltime;
 
 	func = call->func + iter->tr->text_delta;
 
@@ -1142,11 +1144,14 @@ print_graph_entry(struct ftrace_graph_ent_entry *field, struct trace_seq *s,
 }
 
 static enum print_line_t
-print_graph_return(struct ftrace_graph_ret *trace, struct trace_seq *s,
+print_graph_return(struct ftrace_graph_ret_entry *retentry, struct trace_seq *s,
 		   struct trace_entry *ent, struct trace_iterator *iter,
 		   u32 flags)
 {
-	unsigned long long duration = trace->rettime - trace->calltime;
+	struct ftrace_graph_ret *trace = &retentry->ret;
+	u64 calltime = retentry->calltime;
+	u64 rettime = retentry->rettime;
+	unsigned long long duration = rettime - calltime;
 	struct fgraph_data *data = iter->private;
 	struct trace_array *tr = iter->tr;
 	unsigned long func;
@@ -1347,7 +1352,7 @@ print_graph_function_flags(struct trace_iterator *iter, u32 flags)
 	case TRACE_GRAPH_RET: {
 		struct ftrace_graph_ret_entry *field;
 		trace_assign_type(field, entry);
-		return print_graph_return(&field->ret, s, entry, iter, flags);
+		return print_graph_return(field, s, entry, iter, flags);
 	}
 	case TRACE_STACK:
 	case TRACE_FN:
kernel/trace/trace_irqsoff.c +3 -2
index a4e799c1e767..1d3646ce9c34 100644
@@ -221,6 +221,7 @@ static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
 	unsigned long flags;
 	unsigned int trace_ctx;
 	u64 *calltime;
+	u64 rettime;
 	int size;
 
 	ftrace_graph_addr_finish(gops, trace);
@@ -228,13 +229,13 @@ static void irqsoff_graph_return(struct ftrace_graph_ret *trace,
 	if (!func_prolog_dec(tr, &data, &flags))
 		return;
 
+	rettime = trace_clock_local();
 	calltime = fgraph_retrieve_data(gops->idx, &size);
 	if (!calltime)
 		return;
-	trace->calltime = *calltime;
 
 	trace_ctx = tracing_gen_ctx_flags(flags);
-	__trace_graph_return(tr, trace, trace_ctx);
+	__trace_graph_return(tr, trace, trace_ctx, *calltime, rettime);
 	atomic_dec(&data->disabled);
 }
 
kernel/trace/trace_sched_wakeup.c +4 -2
index c58292e424d5..c4bbaae2a2a3 100644
@@ -156,6 +156,7 @@ static void wakeup_graph_return(struct ftrace_graph_ret *trace,
 	struct trace_array_cpu *data;
 	unsigned int trace_ctx;
 	u64 *calltime;
+	u64 rettime;
 	int size;
 
 	ftrace_graph_addr_finish(gops, trace);
@@ -163,12 +164,13 @@ static void wakeup_graph_return(struct ftrace_graph_ret *trace,
 	if (!func_prolog_preempt_disable(tr, &data, &trace_ctx))
 		return;
 
+	rettime = trace_clock_local();
+
 	calltime = fgraph_retrieve_data(gops->idx, &size);
 	if (!calltime)
 		return;
-	trace->calltime = *calltime;
 
-	__trace_graph_return(tr, trace, trace_ctx);
+	__trace_graph_return(tr, trace, trace_ctx, *calltime, rettime);
 	atomic_dec(&data->disabled);
 
 	preempt_enable_notrace();
Signature · 3 lines
2.45.2

-- Steve
Signature · 1 line
Masami Hiramatsu (Google) <[email protected]>
maintainer
#38 lore raw

Re: [PATCH] fgraph: Move trace_clock_local() for return time to function_graph tracer

Quoted 6 lines · On Tue, 14 Jan 2025 10:18:06 +0900 Masami Hiramatsu (Google)…
Quoted 1 line

I'd rather just nuke the calltime and rettime from struct ftrace_graph_ret.

Yeah, this one is good to me. But this will make a slightly different time for the same function when we enable multiple function graph tracer on different instances (mine records it once and reuse it but this will record on each instance). We may need to notice it on the document. :)

And I believe it's separate for each instance now anyway, so I was going to send this to Linus as well:

Documentation/trace/ftrace.rst +6 -0
index 272464bb7c60..2b74f96d09d5 100644
@@ -810,6 +810,12 @@ Here is the list of current tracers that may be configured.
 	to draw a graph of function calls similar to C code
 	source.
 
+	Note that the function graph calculates the timings of when the
+	function starts and returns internally and for each instance. If
+	there are two instances that run function graph tracer and traces
+	the same functions, the length of the timings may be slightly off as
+	each read the timestamp separately and not at the same time.
+
   "blk"
 
 	The block tracer. The tracer used by the blktrace user
Quoted 2 lines

Thanks,

-- Steve