diff mbox series

[v21,05/20] fprobe: Use ftrace_regs in fprobe entry handler

Message ID 173379659280.973433.14242629573844824837.stgit@devnote2 (mailing list archive)
State Superseded
Headers show
Series tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph | expand

Commit Message

Masami Hiramatsu (Google) Dec. 10, 2024, 2:09 a.m. UTC 
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

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.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Acked-by: Florent Revest <revest@chromium.org>
---
 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     |    6 +++++-
 lib/test_fprobe.c               |    4 ++--
 samples/fprobe/fprobe_example.c |    2 +-
 7 files changed, 20 insertions(+), 10 deletions(-)

Comments

Steven Rostedt Dec. 23, 2024, 9:42 p.m. UTC | #1 
On Tue, 10 Dec 2024 11:09:52 +0900
"Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:

> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> 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.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Hmm, my build failed on this patch. Perhaps I screwed up the rebase to v6.13-rc4?

Config attached:

[3 seconds] SUCCESS
make O=/work/build/nobackup/tracetest -j50 ... make[1]: Entering directory '/work/build/nobackup/tracetest'
  SYNC    include/config/auto.conf.cmd
  GEN     Makefile
  GEN     Makefile
mkdir -p /work/build/nobackup/tracetest/tools/objtool && make O=/work/build/nobackup/tracetest subdir=tools/objtool --no-print-directory -C objtool
  UPD     include/config/kernel.release
  UPD     include/generated/utsrelease.h
  INSTALL libsubcmd_headers
  CALL    /work/build/trace/nobackup/linux-test.git/scripts/checksyscalls.sh
  CC      init/version.o
  CC      net/sched/sch_generic.o
  CC      drivers/base/firmware_loader/main.o
  CC      security/integrity/ima/ima_init.o
  CC      kernel/sys.o
  CC      kernel/module/main.o
  CC      drivers/gpu/drm/drm_vblank.o
  CC      kernel/time/time.o
  AR      init/built-in.a
  CC      drivers/gpu/drm/ttm/ttm_bo.o
  CC      drivers/gpu/drm/ttm/ttm_bo_util.o
  CC      drivers/gpu/drm/qxl/qxl_ttm.o
  CC      kernel/trace/bpf_trace.o
  CC      kernel/trace/fprobe.o
  CC      kernel/trace/trace_fprobe.o
  AR      security/integrity/ima/built-in.a
  AR      security/integrity/built-in.a
  AR      security/built-in.a
/work/build/trace/nobackup/linux-test.git/kernel/trace/trace_fprobe.c: In function ‘__trace_fprobe_create’:
/work/build/trace/nobackup/linux-test.git/kernel/trace/trace_fprobe.c:1223:38: error: assignment to ‘fprobe_entry_cb’ {aka ‘int (*)(struct fprobe *, long unsigned int,  long unsigned int,  struct ftrace_regs *, void *)’} from incompatible pointer type ‘int (*)(struct fpro
be *, long unsigned int,  long unsigned int,  struct pt_regs *, void *)’ [-Wincompatible-pointer-types]
 1223 |                 tf->fp.entry_handler = trace_fprobe_entry_handler;
      |                                      ^
make[5]: *** [/work/build/trace/nobackup/linux-test.git/scripts/Makefile.build:194: kernel/trace/trace_fprobe.o] Error 1
make[5]: *** Waiting for unfinished jobs....
  AR      drivers/gpu/drm/qxl/built-in.a
  AR      drivers/base/firmware_loader/built-in.a
  AR      drivers/base/built-in.a
  AR      kernel/time/built-in.a
  AR      drivers/gpu/drm/ttm/built-in.a
  AR      drivers/gpu/drm/built-in.a
  AR      drivers/gpu/built-in.a
  AR      drivers/built-in.a
  AR      net/sched/built-in.a
  AR      net/built-in.a
  AR      kernel/module/built-in.a
make[4]: *** [/work/build/trace/nobackup/linux-test.git/scripts/Makefile.build:440: kernel/trace] Error 2
make[3]: *** [/work/build/trace/nobackup/linux-test.git/scripts/Makefile.build:440: kernel] Error 2
make[2]: *** [/work/build/trace/nobackup/linux-test.git/Makefile:1989: .] Error 2
make[1]: *** [/work/build/trace/nobackup/linux-test.git/Makefile:251: __sub-make] Error 2
make[1]: Leaving directory '/work/build/nobackup/tracetest'
make: *** [Makefile:251: __sub-make] Error 2
[10 seconds] FAILED!
make O=/work/build/nobackup/tracetest kernelrelease ... 6.13.0-rc4-test-00005-g21a983830d78-dirty
ssh -t root@tracetest 'rm -rf /lib/modules/*-test*'; git reset --hard ... HEAD is now at 21a983830d78 fprobe: Use ftrace_regs in fprobe entry handler
[1 second] SUCCESS
CRITICAL FAILURE... [TEST 3] failed build

-- Steve
Masami Hiramatsu (Google) Dec. 26, 2024, 2:08 a.m. UTC | #2 
On Mon, 23 Dec 2024 16:42:38 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 10 Dec 2024 11:09:52 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > 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.
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Hmm, my build failed on this patch. Perhaps I screwed up the rebase to v6.13-rc4?

Oops, no. That is a bug, which is fixed in [9/20]. Let me fix it.

Thanks!

> 
> Config attached:
> 
> [3 seconds] SUCCESS
> make O=/work/build/nobackup/tracetest -j50 ... make[1]: Entering directory '/work/build/nobackup/tracetest'
>   SYNC    include/config/auto.conf.cmd
>   GEN     Makefile
>   GEN     Makefile
> mkdir -p /work/build/nobackup/tracetest/tools/objtool && make O=/work/build/nobackup/tracetest subdir=tools/objtool --no-print-directory -C objtool
>   UPD     include/config/kernel.release
>   UPD     include/generated/utsrelease.h
>   INSTALL libsubcmd_headers
>   CALL    /work/build/trace/nobackup/linux-test.git/scripts/checksyscalls.sh
>   CC      init/version.o
>   CC      net/sched/sch_generic.o
>   CC      drivers/base/firmware_loader/main.o
>   CC      security/integrity/ima/ima_init.o
>   CC      kernel/sys.o
>   CC      kernel/module/main.o
>   CC      drivers/gpu/drm/drm_vblank.o
>   CC      kernel/time/time.o
>   AR      init/built-in.a
>   CC      drivers/gpu/drm/ttm/ttm_bo.o
>   CC      drivers/gpu/drm/ttm/ttm_bo_util.o
>   CC      drivers/gpu/drm/qxl/qxl_ttm.o
>   CC      kernel/trace/bpf_trace.o
>   CC      kernel/trace/fprobe.o
>   CC      kernel/trace/trace_fprobe.o
>   AR      security/integrity/ima/built-in.a
>   AR      security/integrity/built-in.a
>   AR      security/built-in.a
> /work/build/trace/nobackup/linux-test.git/kernel/trace/trace_fprobe.c: In function ‘__trace_fprobe_create’:
> /work/build/trace/nobackup/linux-test.git/kernel/trace/trace_fprobe.c:1223:38: error: assignment to ‘fprobe_entry_cb’ {aka ‘int (*)(struct fprobe *, long unsigned int,  long unsigned int,  struct ftrace_regs *, void *)’} from incompatible pointer type ‘int (*)(struct fpro
> be *, long unsigned int,  long unsigned int,  struct pt_regs *, void *)’ [-Wincompatible-pointer-types]
>  1223 |                 tf->fp.entry_handler = trace_fprobe_entry_handler;
>       |                                      ^
> make[5]: *** [/work/build/trace/nobackup/linux-test.git/scripts/Makefile.build:194: kernel/trace/trace_fprobe.o] Error 1
> make[5]: *** Waiting for unfinished jobs....
>   AR      drivers/gpu/drm/qxl/built-in.a
>   AR      drivers/base/firmware_loader/built-in.a
>   AR      drivers/base/built-in.a
>   AR      kernel/time/built-in.a
>   AR      drivers/gpu/drm/ttm/built-in.a
>   AR      drivers/gpu/drm/built-in.a
>   AR      drivers/gpu/built-in.a
>   AR      drivers/built-in.a
>   AR      net/sched/built-in.a
>   AR      net/built-in.a
>   AR      kernel/module/built-in.a
> make[4]: *** [/work/build/trace/nobackup/linux-test.git/scripts/Makefile.build:440: kernel/trace] Error 2
> make[3]: *** [/work/build/trace/nobackup/linux-test.git/scripts/Makefile.build:440: kernel] Error 2
> make[2]: *** [/work/build/trace/nobackup/linux-test.git/Makefile:1989: .] Error 2
> make[1]: *** [/work/build/trace/nobackup/linux-test.git/Makefile:251: __sub-make] Error 2
> make[1]: Leaving directory '/work/build/nobackup/tracetest'
> make: *** [Makefile:251: __sub-make] Error 2
> [10 seconds] FAILED!
> make O=/work/build/nobackup/tracetest kernelrelease ... 6.13.0-rc4-test-00005-g21a983830d78-dirty
> ssh -t root@tracetest 'rm -rf /lib/modules/*-test*'; git reset --hard ... HEAD is now at 21a983830d78 fprobe: Use ftrace_regs in fprobe entry handler
> [1 second] SUCCESS
> CRITICAL FAILURE... [TEST 3] failed build
> 
> -- Steve
diff mbox series

Patch

diff --git a/include/linux/fprobe.h b/include/linux/fprobe.h
index f39869588117..ca64ee5e45d2 100644
--- a/include/linux/fprobe.h
+++ b/include/linux/fprobe.h
@@ -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,
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index c5ab2a561272..f10ca86fbfad 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -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
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 949a3870946c..ec83f1975e9e 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -2550,7 +2550,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;
@@ -2802,12 +2802,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;
@@ -3082,7 +3086,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;
diff --git a/kernel/trace/fprobe.c b/kernel/trace/fprobe.c
index 9ff018245840..3d3789283873 100644
--- a/kernel/trace/fprobe.c
+++ b/kernel/trace/fprobe.c
@@ -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;
 }
 
diff --git a/kernel/trace/trace_fprobe.c b/kernel/trace/trace_fprobe.c
index c62d1629cffe..af54191a35b5 100644
--- a/kernel/trace/trace_fprobe.c
+++ b/kernel/trace/trace_fprobe.c
@@ -339,12 +339,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
diff --git a/lib/test_fprobe.c b/lib/test_fprobe.c
index 24de0e5ff859..ff607babba18 100644
--- a/lib/test_fprobe.c
+++ b/lib/test_fprobe.c
@@ -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;
diff --git a/samples/fprobe/fprobe_example.c b/samples/fprobe/fprobe_example.c
index 0a50b05add96..c234afae52d6 100644
--- a/samples/fprobe/fprobe_example.c
+++ b/samples/fprobe/fprobe_example.c
@@ -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)
 		/*