Linux Trace Kernel
 help / color / mirror / Atom feed
* [RFC PATCH v3 10/17] unwind_user: Enable archs that pass RA in a register
From: Jens Remus @ 2025-12-08 17:15 UTC (permalink / raw)
  To: linux-kernel, linux-trace-kernel, linux-s390, bpf, x86,
	Steven Rostedt
  Cc: Jens Remus, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich,
	Josh Poimboeuf, Masami Hiramatsu, Mathieu Desnoyers,
	Peter Zijlstra, Ingo Molnar, Jiri Olsa, Arnaldo Carvalho de Melo,
	Namhyung Kim, Thomas Gleixner, Andrii Nakryiko, Indu Bhagat,
	Jose E. Marchesi, Beau Belgrave, Linus Torvalds, Andrew Morton,
	Florian Weimer, Kees Cook, Carlos O'Donell, Sam James,
	Dylan Hatch
In-Reply-To: <20251208171559.2029709-1-jremus@linux.ibm.com>

Not all architectures have the return address (RA) in user space saved
on the stack on function entry, such as x86-64 does due to its CALL
instruction pushing the RA onto the stack.  Architectures/ABIs, such as
s390, also do not necessarily enforce to save the RA in user space on
the stack in the function prologue or even at all, for instance in leaf
functions.

Treat a RA offset from CFA of zero as indication that the RA is not
saved (on the stack).  For the topmost frame treat it as indication that
the RA is in the link/RA register, such as on arm64 and s390, and obtain
it from there.  For non-topmost frames treat it as error, as the RA must
be saved.

Additionally allow the SP to be unchanged in the topmost frame, for
architectures where SP at function entry == SP at call site, such as
arm64 and s390.

Note that treating a RA offset from CFA of zero as indication that
the RA is not saved on the stack additionally allows for architectures,
such as s390, where the frame pointer (FP) may be saved without the RA
being saved as well.  Provided that such architectures represent this
in SFrame by encoding the "missing" RA offset using a padding RA offset
with a value of zero.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v2:
    - Reword commit subject and message.
    - Rename config option USER_RA_REG to UNWIND_USER_RA_REG and reword
      help text to mention both link and return address register. (Josh)
    - Move dummy user_return_address() from linux/ptrace.h to
      linux/unwind_user.h, rename to unwind_user_get_ra_reg(),
      return -EINVAL, and guard by !CONFIG_HAVE_UNWIND_USER_RA_REG. (Josh)
    - Do not check for !IS_ENABLED(CONFIG_HAVE_USER_RA_REG), as the dummy
      implementation of user_return_address() returns -EINVAL.
    - Drop config option USER_RA_REG / UNWIND_USER_RA_REG, as it is of
      no value any longer.
    - Drop topmost checks from unwind user sframe, as they are already
      done by unwind user. (Josh)

 include/linux/unwind_user.h |  9 +++++++++
 kernel/unwind/sframe.c      |  6 ++----
 kernel/unwind/user.c        | 17 +++++++++++++----
 3 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h
index 64618618febd..bc2edae39955 100644
--- a/include/linux/unwind_user.h
+++ b/include/linux/unwind_user.h
@@ -23,6 +23,15 @@ static inline bool unwind_user_at_function_start(struct pt_regs *regs)
 #define unwind_user_at_function_start unwind_user_at_function_start
 #endif
 
+#ifndef unwind_user_get_ra_reg
+static inline int unwind_user_get_ra_reg(unsigned long *val)
+{
+	WARN_ON_ONCE(1);
+	return -EINVAL;
+}
+#define unwind_user_get_ra_reg unwind_user_get_ra_reg
+#endif
+
 int unwind_user(struct unwind_stacktrace *trace, unsigned int max_entries);
 
 #endif /* _LINUX_UNWIND_USER_H */
diff --git a/kernel/unwind/sframe.c b/kernel/unwind/sframe.c
index 7952b041dd23..38b3577f5253 100644
--- a/kernel/unwind/sframe.c
+++ b/kernel/unwind/sframe.c
@@ -228,10 +228,8 @@ static __always_inline int __read_fre(struct sframe_section *sec,
 	offset_count--;
 
 	ra_off = sec->ra_off;
-	if (!ra_off) {
-		if (!offset_count--)
-			return -EFAULT;
-
+	if (!ra_off && offset_count) {
+		offset_count--;
 		UNSAFE_GET_USER_INC(ra_off, cur, offset_size, Efault);
 	}
 
diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index 6c75a7411871..58e1549cd9f4 100644
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -50,16 +50,25 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 
 	/* Get the Stack Pointer (SP) */
 	sp = cfa + frame->sp_off;
-	/* Make sure that stack is not going in wrong direction */
-	if (sp <= state->sp)
+	/*
+	 * Make sure that stack is not going in wrong direction.  Allow SP
+	 * to be unchanged for the topmost frame, by subtracting topmost,
+	 * which is either 0 or 1.
+	 */
+	if (sp <= state->sp - state->topmost)
 		return -EINVAL;
 	/* Make sure that the address is word aligned */
 	if (sp & (state->ws - 1))
 		return -EINVAL;
 
 	/* Get the Return Address (RA) */
-	if (get_user_word(&ra, cfa, frame->ra_off, state->ws))
-		return -EINVAL;
+	if (frame->ra_off) {
+		if (get_user_word(&ra, cfa, frame->ra_off, state->ws))
+			return -EINVAL;
+	} else {
+		if (!state->topmost || unwind_user_get_ra_reg(&ra))
+			return -EINVAL;
+	}
 
 	/* Get the Frame Pointer (FP) */
 	if (frame->fp_off && get_user_word(&fp, cfa, frame->fp_off, state->ws))
-- 
2.51.0


^ permalink raw reply related
* [RFC PATCH v3 01/17] unwind_user: Enhance comments on get CFA, FP, and RA
From: Jens Remus @ 2025-12-08 17:15 UTC (permalink / raw)
  To: linux-kernel, linux-trace-kernel, linux-s390, bpf, x86,
	Steven Rostedt
  Cc: Jens Remus, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich,
	Josh Poimboeuf, Masami Hiramatsu, Mathieu Desnoyers,
	Peter Zijlstra, Ingo Molnar, Jiri Olsa, Arnaldo Carvalho de Melo,
	Namhyung Kim, Thomas Gleixner, Andrii Nakryiko, Indu Bhagat,
	Jose E. Marchesi, Beau Belgrave, Linus Torvalds, Andrew Morton,
	Florian Weimer, Kees Cook, Carlos O'Donell, Sam James,
	Dylan Hatch
In-Reply-To: <20251208171559.2029709-1-jremus@linux.ibm.com>

Move the comment "Get the Canonical Frame Address (CFA)" to the top
of the sequence of statements that actually get the CFA.  Reword the
comment "Find the Return Address (RA)" to "Get ...", as the statements
actually get the RA.  Add a respective comment to the statements that
get the FP.  This will be useful once future commits extend the logic
to get the RA and FP.

While at it align the comment on the "stack going in wrong direction"
check to the following one on the "address is word aligned" check.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
 kernel/unwind/user.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index d053295b1f7e..f81c36ab2861 100644
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -38,6 +38,7 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 		return 0;
 	}
 
+	/* Get the Canonical Frame Address (CFA) */
 	if (frame->use_fp) {
 		if (state->fp < state->sp)
 			return -EINVAL;
@@ -45,11 +46,9 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 	} else {
 		cfa = state->sp;
 	}
-
-	/* Get the Canonical Frame Address (CFA) */
 	cfa += frame->cfa_off;
 
-	/* stack going in wrong direction? */
+	/* Make sure that stack is not going in wrong direction */
 	if (cfa <= state->sp)
 		return -EINVAL;
 
@@ -57,10 +56,11 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 	if (cfa & (state->ws - 1))
 		return -EINVAL;
 
-	/* Find the Return Address (RA) */
+	/* Get the Return Address (RA) */
 	if (get_user_word(&ra, cfa, frame->ra_off, state->ws))
 		return -EINVAL;
 
+	/* Get the Frame Pointer (FP) */
 	if (frame->fp_off && get_user_word(&fp, cfa, frame->fp_off, state->ws))
 		return -EINVAL;
 
-- 
2.51.0


^ permalink raw reply related
* Re: [PATCH] powerpc64/bpf: support direct_call on livepatch function
From: Hari Bathini @ 2025-12-08 16:35 UTC (permalink / raw)
  To: Naveen N Rao
  Cc: Madhavan Srinivasan, linuxppc-dev, Christophe Leroy,
	Michael Ellerman, Nicholas Piggin, bpf, Alexei Starovoitov,
	Daniel Borkmann, Andrii Nakryiko, Song Liu, Jiri Olsa,
	Viktor Malik, live-patching, Josh Poimboeuf, Joe Lawrence,
	Jiri Kosina, linux-trace-kernel, Steven Rostedt, Masami Hiramatsu,
	Mark Rutland, Shung-Hsi Yu
In-Reply-To: <nuinyo7o7uniemqqmoboctwrkkwkuv77nt7yk6td6eb3x43hv2@2lukfuvcmcko>

Thanks for the review, Naveen.
I was on leave for sometime and could not look into it in a while
after that.

On 15/10/25 11:48 am, Naveen N Rao wrote:
> On Fri, Oct 10, 2025 at 12:47:21PM +0530, Hari Bathini wrote:
>>
>>
>> On 09/10/25 4:57 pm, Naveen N Rao wrote:
>>> On Thu, Oct 09, 2025 at 11:19:45AM +0530, Hari Bathini wrote:
>>>>
>>>>
>>>> On 08/10/25 1:43 pm, Naveen N Rao wrote:
>>>>> On Mon, Oct 06, 2025 at 06:50:20PM +0530, Hari Bathini wrote:
>>>>>>
>>>>>>
>>>>>> On 06/10/25 1:22 pm, Naveen N Rao wrote:
>>>>>>> On Fri, Oct 03, 2025 at 12:57:54AM +0530, Hari Bathini wrote:
>>>>>>>> Today, livepatch takes precedence over direct_call. Instead, save the
>>>>>>>> state and make direct_call before handling livepatch.
>>>>>>>
>>>>>>> If we call into the BPF trampoline first and if we have
>>>>>>> BPF_TRAMP_F_CALL_ORIG set, does this result in the BPF trampoline
>>>>>>> calling the new copy of the live-patched function or the old one?
>>>>>>
>>>>>> Naveen, calls the new copy of the live-patched function..
>>>>>
>>>>> Hmm... I'm probably missing something.
>>>>>
>>>>> With ftrace OOL stubs, what I recall is that BPF trampoline derives the
>>>>> original function address from the OOL stub (which would be associated
>>>>> with the original function, not the livepatch one).
>>>>
>>>> Trampoline derives the address from LR.
>>>
>>> Does it? I'm referring to BPF_TRAMP_F_CALL_ORIG handling in
>>> __arch_prepare_bpf_trampoline().
>>
>>
>>> LR at BPF trampoline entry points at
>>> the ftrace OOL stub. We recover the "real LR" pointing to the function
>>> being traced from there so that we can call into it from within the BPF
>>> trampoline.
>>
>> Naveen, from the snippet in livepatch_handler code shared below,
>> the LR at BPF trmapoline entry points at the 'nop' after the call
>> to trampoline with 'bnectrl cr1' in the updated livepatch_handler.
>>
>> Mimic'ing ftrace OOL branch instruction in livepatch_handler
>> with 'b	1f' (the instruction after nop) to ensure the trmapoline
>> derives the real LR to '1f' and jumps back into the livepatch_handler..
>>
>> +       /* Jump to the direct_call */
>> +       bnectrl cr1
>> +
>> +       /*
>> +        * The address to jump after direct call is deduced based on ftrace
>> OOL stub sequence.
>> +        * The seemingly insignificant couple of instructions below is to
>> mimic that here to
>> +        * jump back to the livepatch handler code below.
>> +        */
>> +       nop
>> +       b       1f
>> +
>> +       /*
>> +        * Restore the state for livepatching from the livepatch stack.
>> +        * Before that, check if livepatch stack is intact. Use r0 for it.
>> +        */
>> +1:     mtctr   r0
> 
> Ah, so you are faking a ftrace OOL stub here. But, won't this mean that

Yeah.

> bpf_get_func_ip() won't return the function address anymore?

Right. I do agree it can have issues in some scenarios.

> 
> One of the other thoughts I had was if we could stuff the function
> address into the ftrace OOL stub. I had considered this back when I
> implemented the OOL stubs, but didn't do it due to the extra memory
> requirement. However, given the dance we're having to do, I'm now
> thinking that may make sense and can simplify the code. If we can also
> hook into livepatch, then we should be able to update the function
> address in the stub to point to the new address and the trampoline
> should then "just work" since it already saves/restores the TOC [We may
> additionally have to update the function IP in _R12, but that would be a
> minor change overall]
> 
> We will still need a way to restore livepatch TOC if the BPF trampoline
> doesn't itself call into the function, but we may be able to handle that
> if we change the return address to jump to a stub that restores the TOC
> from the livepatch stack.

Sounds doable. Looking into a couple of other things at the moment
though. Will try out this suggestion and get back post that.
Having said that, your thoughts on whether the current approach
is a viable option if bpf_get_func_ip() can be fixed somehow?

- Hari

^ permalink raw reply
* [PATCH v1 1/1] bpf: Mark BPF printing functions with __printf() attribute
From: Andy Shevchenko @ 2025-12-08 16:18 UTC (permalink / raw)
  To: Alexei Starovoitov, bpf, linux-kernel, linux-trace-kernel
  Cc: Daniel Borkmann, John Fastabend, Andrii Nakryiko,
	Martin KaFai Lau, Eduard Zingerman, Song Liu, Yonghong Song,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Matt Bobrowski,
	Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Alan Maguire,
	Andy Shevchenko, kernel test robot

The printing functions in BPF code are using printf() type of format,
and compiler is not happy about them as is:

kernel/bpf/helpers.c:1069:9: error: function ‘____bpf_snprintf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
 1069 |         err = bstr_printf(str, str_size, fmt, data.bin_args);
      |         ^~~

kernel/trace/bpf_trace.c:377:9: error: function ‘____bpf_trace_printk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
  377 |         ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
      |         ^~~

kernel/trace/bpf_trace.c:433:9: error: function ‘____bpf_trace_vprintk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
  433 |         ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
      |         ^~~

kernel/trace/bpf_trace.c:475:9: error: function ‘____bpf_seq_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
  475 |         seq_bprintf(m, fmt, data.bin_args);
      |         ^~~~~~~~~~~

Fix the compilation errors by adding __printf() attribute. For that
we need to pass it down to the BPF_CALL_x() and wrap into PRINTF_BPF_CALL_*()
to make code neater.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202512061425.x0qTt9ww-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202512061640.9hKTnB8p-lkp@intel.com/
Closes: https://lore.kernel.org/oe-kbuild-all/202512081321.2h9ThWTg-lkp@intel.com/
Fixes: 10aceb629e19 ("bpf: Add bpf_trace_vprintk helper")
Fixes: 7b15523a989b ("bpf: Add a bpf_snprintf helper")
Fixes: 492e639f0c22 ("bpf: Add bpf_seq_printf and bpf_seq_write helpers")
Fixes: f3694e001238 ("bpf: add BPF_CALL_x macros for declaring helpers")
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---

This is combined change and I think there is no need to split it, but if required
I can do it in a four changes. Note, the culprits are older than 4 years and stable
kernels anyway don't go that deep nowadays.

 include/linux/filter.h   | 24 +++++++++++++++---------
 kernel/bpf/helpers.c     |  2 +-
 kernel/trace/bpf_trace.c |  6 +++---
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index fd54fed8f95f..31034a74af22 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -607,9 +607,9 @@ static inline bool insn_is_cast_user(const struct bpf_insn *insn)
 	__BPF_MAP(n, __BPF_DECL_ARGS, __BPF_N, u64, __ur_1, u64, __ur_2,       \
 		  u64, __ur_3, u64, __ur_4, u64, __ur_5)
 
-#define BPF_CALL_x(x, attr, name, ...)					       \
+#define BPF_CALL_x(x, __attr, attr, name, ...)				       \
 	static __always_inline						       \
-	u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));   \
+	__attr u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));     \
 	typedef u64 (*btf_##name)(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__)); \
 	attr u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__));    \
 	attr u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__))     \
@@ -620,14 +620,20 @@ static inline bool insn_is_cast_user(const struct bpf_insn *insn)
 	u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__))
 
 #define __NOATTR
-#define BPF_CALL_0(name, ...)	BPF_CALL_x(0, __NOATTR, name, __VA_ARGS__)
-#define BPF_CALL_1(name, ...)	BPF_CALL_x(1, __NOATTR, name, __VA_ARGS__)
-#define BPF_CALL_2(name, ...)	BPF_CALL_x(2, __NOATTR, name, __VA_ARGS__)
-#define BPF_CALL_3(name, ...)	BPF_CALL_x(3, __NOATTR, name, __VA_ARGS__)
-#define BPF_CALL_4(name, ...)	BPF_CALL_x(4, __NOATTR, name, __VA_ARGS__)
-#define BPF_CALL_5(name, ...)	BPF_CALL_x(5, __NOATTR, name, __VA_ARGS__)
+#define BPF_CALL_0(name, ...)	BPF_CALL_x(0, __NOATTR, __NOATTR, name, __VA_ARGS__)
+#define BPF_CALL_1(name, ...)	BPF_CALL_x(1, __NOATTR, __NOATTR, name, __VA_ARGS__)
+#define BPF_CALL_2(name, ...)	BPF_CALL_x(2, __NOATTR, __NOATTR, name, __VA_ARGS__)
+#define BPF_CALL_3(name, ...)	BPF_CALL_x(3, __NOATTR, __NOATTR, name, __VA_ARGS__)
+#define BPF_CALL_4(name, ...)	BPF_CALL_x(4, __NOATTR, __NOATTR, name, __VA_ARGS__)
+#define BPF_CALL_5(name, ...)	BPF_CALL_x(5, __NOATTR, __NOATTR, name, __VA_ARGS__)
 
-#define NOTRACE_BPF_CALL_1(name, ...)	BPF_CALL_x(1, notrace, name, __VA_ARGS__)
+#define PRINTF_BPF_CALL_x(p, name, x, ...)				       \
+	BPF_CALL_x(x, __printf(p, 0), __NOATTR, name, __VA_ARGS__)
+
+#define PRINTF_BPF_CALL_4(p, name, ...)		PRINTF_BPF_CALL_x(p, name, 4, __VA_ARGS__)
+#define PRINTF_BPF_CALL_5(p, name, ...)		PRINTF_BPF_CALL_x(p, name, 5, __VA_ARGS__)
+
+#define NOTRACE_BPF_CALL_1(name, ...)	BPF_CALL_x(1, __NOATTR, notrace, name, __VA_ARGS__)
 
 #define bpf_ctx_range(TYPE, MEMBER)						\
 	offsetof(TYPE, MEMBER) ... offsetofend(TYPE, MEMBER) - 1
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index db72b96f9c8c..cbc66865e3dc 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1046,7 +1046,7 @@ int bpf_bprintf_prepare(const char *fmt, u32 fmt_size, const u64 *raw_args,
 	return err;
 }
 
-BPF_CALL_5(bpf_snprintf, char *, str, u32, str_size, char *, fmt,
+PRINTF_BPF_CALL_5(3, bpf_snprintf, char *, str, u32, str_size, char *, fmt,
 	   const void *, args, u32, data_len)
 {
 	struct bpf_bprintf_data data = {
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d57727abaade..5fd46b4bcf48 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -359,7 +359,7 @@ static const struct bpf_func_proto bpf_probe_write_user_proto = {
 #define MAX_TRACE_PRINTK_VARARGS	3
 #define BPF_TRACE_PRINTK_SIZE		1024
 
-BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
+PRINTF_BPF_CALL_5(1, bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
 	   u64, arg2, u64, arg3)
 {
 	u64 args[MAX_TRACE_PRINTK_VARARGS] = { arg1, arg2, arg3 };
@@ -412,7 +412,7 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void)
 	return &bpf_trace_printk_proto;
 }
 
-BPF_CALL_4(bpf_trace_vprintk, char *, fmt, u32, fmt_size, const void *, args,
+PRINTF_BPF_CALL_4(1, bpf_trace_vprintk, char *, fmt, u32, fmt_size, const void *, args,
 	   u32, data_len)
 {
 	struct bpf_bprintf_data data = {
@@ -455,7 +455,7 @@ const struct bpf_func_proto *bpf_get_trace_vprintk_proto(void)
 	return &bpf_trace_vprintk_proto;
 }
 
-BPF_CALL_5(bpf_seq_printf, struct seq_file *, m, char *, fmt, u32, fmt_size,
+PRINTF_BPF_CALL_5(2, bpf_seq_printf, struct seq_file *, m, char *, fmt, u32, fmt_size,
 	   const void *, args, u32, data_len)
 {
 	struct bpf_bprintf_data data = {
-- 
2.50.1


^ permalink raw reply related
* Re: [PATCH 3/3] powerpc64/ftrace: workaround clang recording GEP in __patchable_function_entries
From: Hari Bathini @ 2025-12-08 16:08 UTC (permalink / raw)
  To: Naveen N Rao
  Cc: linuxppc-dev, Madhavan Srinivasan, Christophe Leroy,
	Michael Ellerman, linux-trace-kernel, Mark Rutland,
	Steven Rostedt, Masami Hiramatsu, llvm, Maryam Moghadas
In-Reply-To: <sjheprnafkraauotmi2fwjz6l6fha5te7jdp74t4zfh3jvs2if@vjpey276fsti>



On 24/11/25 11:25 am, Naveen N Rao wrote:
> On Sun, Nov 09, 2025 at 02:34:05AM +0530, Hari Bathini wrote:
>> Support for -fpatchable-function-entry on ppc64le was added in Clang
>> with [1]. However, when no prefix NOPs are specified - as is the case
>> with CONFIG_PPC_FTRACE_OUT_OF_LINE - the first NOP is emitted at LEP,
>> but Clang records the Global Entry Point (GEP) unlike GCC which does
>> record the Local Entry Point (LEP). Issue [2] has been raised to align
>> Clang's behavior with GCC. As a temporary workaround to ensure ftrace
> 
> Any reason to put in a "temporary" workaround rather than waiting for
> this to be fixed?

Pushing for this temporary workaround as the fix ETA for clang is not
available at the moment and the temporary workaround is likely
harmless even on a clang version with the fix included..

- Hari

^ permalink raw reply
* [PATCH v4 1/4] unwind_user: Enhance comments on get CFA, FP, and RA
From: Jens Remus @ 2025-12-08 16:03 UTC (permalink / raw)
  To: linux-kernel, linux-trace-kernel, x86, Steven Rostedt,
	Peter Zijlstra
  Cc: Jens Remus, Josh Poimboeuf, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, Mathieu Desnoyers,
	Indu Bhagat, Jose E. Marchesi, Heiko Carstens, Vasily Gorbik,
	Ilya Leoshkevich, Linus Torvalds
In-Reply-To: <20251208160352.1363040-1-jremus@linux.ibm.com>

Move the comment "Get the Canonical Frame Address (CFA)" to the top
of the sequence of statements that actually get the CFA.  Reword the
comment "Find the Return Address (RA)" to "Get ...", as the statements
actually get the RA.  Add a respective comment to the statements that
get the FP.  This will be useful once future commits extend the logic
to get the RA and FP.

While at it align the comment on the "stack going in wrong direction"
check to the following one on the "address is word aligned" check.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
 kernel/unwind/user.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index 39e270789444..0ca434f86e73 100644
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -31,6 +31,7 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 {
 	unsigned long cfa, fp, ra;
 
+	/* Get the Canonical Frame Address (CFA) */
 	if (frame->use_fp) {
 		if (state->fp < state->sp)
 			return -EINVAL;
@@ -38,11 +39,9 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 	} else {
 		cfa = state->sp;
 	}
-
-	/* Get the Canonical Frame Address (CFA) */
 	cfa += frame->cfa_off;
 
-	/* stack going in wrong direction? */
+	/* Make sure that stack is not going in wrong direction */
 	if (cfa <= state->sp)
 		return -EINVAL;
 
@@ -50,10 +49,11 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 	if (cfa & (state->ws - 1))
 		return -EINVAL;
 
-	/* Find the Return Address (RA) */
+	/* Get the Return Address (RA) */
 	if (get_user_word(&ra, cfa, frame->ra_off, state->ws))
 		return -EINVAL;
 
+	/* Get the Frame Pointer (FP) */
 	if (frame->fp_off && get_user_word(&fp, cfa, frame->fp_off, state->ws))
 		return -EINVAL;
 
-- 
2.51.0


^ permalink raw reply related
* [PATCH v4 4/4] x86/unwind_user: Simplify unwind_user_word_size()
From: Jens Remus @ 2025-12-08 16:03 UTC (permalink / raw)
  To: linux-kernel, linux-trace-kernel, x86, Steven Rostedt,
	Peter Zijlstra
  Cc: Jens Remus, Josh Poimboeuf, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, Mathieu Desnoyers,
	Indu Bhagat, Jose E. Marchesi, Heiko Carstens, Vasily Gorbik,
	Ilya Leoshkevich, Linus Torvalds
In-Reply-To: <20251208160352.1363040-1-jremus@linux.ibm.com>

Get rid of superfluous ifdef and return explicit word size depending on
32-bit or 64-bit mode.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v4:
    - New patch. (Linus)
      https://lore.kernel.org/all/CAHk-=wh4_BPvniQZqvEQ4cCC3WfvQqruWk0b1Yek+0d5S1LuxQ@mail.gmail.com/
    
    This aligns to sizeof_long() in arch/x86/kernel/uprobes.c.

 arch/x86/include/asm/unwind_user.h | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/arch/x86/include/asm/unwind_user.h b/arch/x86/include/asm/unwind_user.h
index 7f1229b33d06..6e469044e4de 100644
--- a/arch/x86/include/asm/unwind_user.h
+++ b/arch/x86/include/asm/unwind_user.h
@@ -12,11 +12,7 @@ static inline int unwind_user_word_size(struct pt_regs *regs)
 	/* We can't unwind VM86 stacks */
 	if (regs->flags & X86_VM_MASK)
 		return 0;
-#ifdef CONFIG_X86_64
-	if (!user_64bit_mode(regs))
-		return sizeof(int);
-#endif
-	return sizeof(long);
+	return user_64bit_mode(regs) ? 8 : 4;
 }
 
 #endif /* CONFIG_UNWIND_USER */
-- 
2.51.0


^ permalink raw reply related
* [PATCH v4 2/4] unwind_user/fp: Use dummies instead of ifdef
From: Jens Remus @ 2025-12-08 16:03 UTC (permalink / raw)
  To: linux-kernel, linux-trace-kernel, x86, Steven Rostedt,
	Peter Zijlstra
  Cc: Jens Remus, Josh Poimboeuf, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, Mathieu Desnoyers,
	Indu Bhagat, Jose E. Marchesi, Heiko Carstens, Vasily Gorbik,
	Ilya Leoshkevich, Linus Torvalds
In-Reply-To: <20251208160352.1363040-1-jremus@linux.ibm.com>

This simplifies the code.   unwind_user_next_fp() does not need to
return -EINVAL if config option HAVE_UNWIND_USER_FP is disabled, as
unwind_user_start() will then not select this unwind method and
unwind_user_next() will therefore not call it.

Provide (1) a dummy definition of ARCH_INIT_USER_FP_FRAME, if the unwind
user method HAVE_UNWIND_USER_FP is not enabled, (2) a common fallback
definition of unwind_user_at_function_start() which returns false, and
(3) a common dummy definition of ARCH_INIT_USER_FP_ENTRY_FRAME.

Note that enabling the config option HAVE_UNWIND_USER_FP without
defining ARCH_INIT_USER_FP_FRAME triggers a compile error, which is
helpful when implementing support for this unwind user method in an
architecture.  Enabling the config option when providing an arch-
specific unwind_user_at_function_start() definition makes it necessary
to also provide an arch-specific ARCH_INIT_USER_FP_ENTRY_FRAME
definition.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v3:
    - Remove comment on #endif. (Ingo)
    
    Changes in v2:
    - Add parameter ws to ARCH_INIT_USER_{FP_FRAME|FP_ENTRY_FRAME}.
    - Provide common fallback of unwind_user_at_function_start().
    - Provide common dummy of ARCH_INIT_USER_FP_ENTRY_FRAME.
    - Reword commit message accordingly.

 arch/x86/include/asm/unwind_user.h |  1 +
 include/linux/unwind_user.h        | 18 ++++++++++++++++--
 kernel/unwind/user.c               |  4 ----
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/unwind_user.h b/arch/x86/include/asm/unwind_user.h
index 12064284bc4e..971ffe937d50 100644
--- a/arch/x86/include/asm/unwind_user.h
+++ b/arch/x86/include/asm/unwind_user.h
@@ -35,6 +35,7 @@ static inline bool unwind_user_at_function_start(struct pt_regs *regs)
 {
 	return is_uprobe_at_func_entry(regs);
 }
+#define unwind_user_at_function_start unwind_user_at_function_start
 
 #endif /* CONFIG_HAVE_UNWIND_USER_FP */
 
diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h
index 7f7282516bf5..64618618febd 100644
--- a/include/linux/unwind_user.h
+++ b/include/linux/unwind_user.h
@@ -5,8 +5,22 @@
 #include <linux/unwind_user_types.h>
 #include <asm/unwind_user.h>
 
-#ifndef ARCH_INIT_USER_FP_FRAME
- #define ARCH_INIT_USER_FP_FRAME
+#ifndef CONFIG_HAVE_UNWIND_USER_FP
+
+#define ARCH_INIT_USER_FP_FRAME(ws)
+
+#endif
+
+#ifndef ARCH_INIT_USER_FP_ENTRY_FRAME
+#define ARCH_INIT_USER_FP_ENTRY_FRAME(ws)
+#endif
+
+#ifndef unwind_user_at_function_start
+static inline bool unwind_user_at_function_start(struct pt_regs *regs)
+{
+	return false;
+}
+#define unwind_user_at_function_start unwind_user_at_function_start
 #endif
 
 int unwind_user(struct unwind_stacktrace *trace, unsigned int max_entries);
diff --git a/kernel/unwind/user.c b/kernel/unwind/user.c
index 0ca434f86e73..90ab3c1a205e 100644
--- a/kernel/unwind/user.c
+++ b/kernel/unwind/user.c
@@ -67,7 +67,6 @@ static int unwind_user_next_common(struct unwind_user_state *state,
 
 static int unwind_user_next_fp(struct unwind_user_state *state)
 {
-#ifdef CONFIG_HAVE_UNWIND_USER_FP
 	struct pt_regs *regs = task_pt_regs(current);
 
 	if (state->topmost && unwind_user_at_function_start(regs)) {
@@ -81,9 +80,6 @@ static int unwind_user_next_fp(struct unwind_user_state *state)
 		ARCH_INIT_USER_FP_FRAME(state->ws)
 	};
 	return unwind_user_next_common(state, &fp_frame);
-#else
-	return -EINVAL;
-#endif
 }
 
 static int unwind_user_next(struct unwind_user_state *state)
-- 
2.51.0


^ permalink raw reply related
* [PATCH v4 0/4] unwind_user: Cleanups
From: Jens Remus @ 2025-12-08 16:03 UTC (permalink / raw)
  To: linux-kernel, linux-trace-kernel, x86, Steven Rostedt,
	Peter Zijlstra
  Cc: Jens Remus, Josh Poimboeuf, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, Mathieu Desnoyers,
	Indu Bhagat, Jose E. Marchesi, Heiko Carstens, Vasily Gorbik,
	Ilya Leoshkevich, Linus Torvalds

This patch series applies on top of Peter Zijlstras' latest unwind user
enhancements (and perf deferred callchain support) on his tip perf/core
branch:

  git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git perf/core

Which has already been merged to tip/master and linux-next/master.

Patch 1 enhances a few comments in unwind_user_next_common().

Patch 2 gets rid of an ifdef in unwind_user_next_fp() by moving it to
linux/unwind_user.h.  Additionally it provides a common fallback for
unwind_user_at_function_start().

Patch 3 ensures the x86 unwind_user_word_size() implementation is
available whenever config option UNWIND_USER is enabled, as it is
required by unwind user in general and is not specific to its FP
unwind method.

Patch 4 (new in v4) simplifies unwind_user_word_size().

Regards,
Jens

Jens Remus (4):
  unwind_user: Enhance comments on get CFA, FP, and RA
  unwind_user/fp: Use dummies instead of ifdef
  x86/unwind_user: Guard unwind_user_word_size() by UNWIND_USER
  x86/unwind_user: Simplify unwind_user_word_size()

 arch/x86/include/asm/unwind_user.h | 27 ++++++++++++++-------------
 include/linux/unwind_user.h        | 18 ++++++++++++++++--
 kernel/unwind/user.c               | 12 ++++--------
 3 files changed, 34 insertions(+), 23 deletions(-)

-- 
2.51.0


^ permalink raw reply
* [PATCH v4 3/4] x86/unwind_user: Guard unwind_user_word_size() by UNWIND_USER
From: Jens Remus @ 2025-12-08 16:03 UTC (permalink / raw)
  To: linux-kernel, linux-trace-kernel, x86, Steven Rostedt,
	Peter Zijlstra
  Cc: Jens Remus, Josh Poimboeuf, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, Dave Hansen, H. Peter Anvin, Mathieu Desnoyers,
	Indu Bhagat, Jose E. Marchesi, Heiko Carstens, Vasily Gorbik,
	Ilya Leoshkevich, Linus Torvalds
In-Reply-To: <20251208160352.1363040-1-jremus@linux.ibm.com>

The unwind user framework in general requires an architecture-specific
implementation of unwind_user_word_size() to be present for any unwind
method, whether that is fp or a future other method, such as potentially
sframe.

Guard unwind_user_word_size() by the availability of the UNWIND_USER
framework instead of the specific HAVE_UNWIND_USER_FP method.

This facilitates to selectively disable HAVE_UNWIND_USER_FP on x86
(e.g. for test purposes) once a new unwind method is added to unwind
user.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---

Notes (jremus):
    Changes in v3:
    - Move includes into more common UNWIND_USER guard at the top of the
      source.  asm/ptrace.h is required for struct pt_regs.

 arch/x86/include/asm/unwind_user.h | 30 +++++++++++++++++-------------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/arch/x86/include/asm/unwind_user.h b/arch/x86/include/asm/unwind_user.h
index 971ffe937d50..7f1229b33d06 100644
--- a/arch/x86/include/asm/unwind_user.h
+++ b/arch/x86/include/asm/unwind_user.h
@@ -2,23 +2,11 @@
 #ifndef _ASM_X86_UNWIND_USER_H
 #define _ASM_X86_UNWIND_USER_H
 
-#ifdef CONFIG_HAVE_UNWIND_USER_FP
+#ifdef CONFIG_UNWIND_USER
 
 #include <asm/ptrace.h>
 #include <asm/uprobes.h>
 
-#define ARCH_INIT_USER_FP_FRAME(ws)			\
-	.cfa_off	=  2*(ws),			\
-	.ra_off		= -1*(ws),			\
-	.fp_off		= -2*(ws),			\
-	.use_fp		= true,
-
-#define ARCH_INIT_USER_FP_ENTRY_FRAME(ws)		\
-	.cfa_off	=  1*(ws),			\
-	.ra_off		= -1*(ws),			\
-	.fp_off		= 0,				\
-	.use_fp		= false,
-
 static inline int unwind_user_word_size(struct pt_regs *regs)
 {
 	/* We can't unwind VM86 stacks */
@@ -31,6 +19,22 @@ static inline int unwind_user_word_size(struct pt_regs *regs)
 	return sizeof(long);
 }
 
+#endif /* CONFIG_UNWIND_USER */
+
+#ifdef CONFIG_HAVE_UNWIND_USER_FP
+
+#define ARCH_INIT_USER_FP_FRAME(ws)			\
+	.cfa_off	=  2*(ws),			\
+	.ra_off		= -1*(ws),			\
+	.fp_off		= -2*(ws),			\
+	.use_fp		= true,
+
+#define ARCH_INIT_USER_FP_ENTRY_FRAME(ws)		\
+	.cfa_off	=  1*(ws),			\
+	.ra_off		= -1*(ws),			\
+	.fp_off		= 0,				\
+	.use_fp		= false,
+
 static inline bool unwind_user_at_function_start(struct pt_regs *regs)
 {
 	return is_uprobe_at_func_entry(regs);
-- 
2.51.0


^ permalink raw reply related
* Re: [PATCH 1/3] powerpc64: make clang cross-build friendly
From: Hari Bathini @ 2025-12-08 16:02 UTC (permalink / raw)
  To: Naveen N Rao
  Cc: linuxppc-dev, Madhavan Srinivasan, Christophe Leroy,
	Michael Ellerman, linux-trace-kernel, Mark Rutland,
	Steven Rostedt, Masami Hiramatsu, llvm, Maryam Moghadas
In-Reply-To: <rtnn6dd6y3refo7myc4crzmrunbuyabr2ewlyk7oe4yx3f2rl2@wzoyvsucfeyf>

Thanks for the review, Naveen.

On 24/11/25 11:19 am, Naveen N Rao wrote:
> On Sun, Nov 09, 2025 at 02:34:03AM +0530, Hari Bathini wrote:
>> ARCH_USING_PATCHABLE_FUNCTION_ENTRY depends on toolchain support for
>> -fpatchable-function-entry option. The current script that checks
>> for this support only handles GCC. Rename the script and extend it
>> to detect support for -fpatchable-function-entry with Clang as well,
>> allowing clean cross-compilation with Clang toolchains.
>>
>> Signed-off-by: Hari Bathini <hbathini@linux.ibm.com>
>> ---
>>   arch/powerpc/Kconfig                                         | 5 +++--
>>   ...-function-entry.sh => check-fpatchable-function-entry.sh} | 0
>>   2 files changed, 3 insertions(+), 2 deletions(-)
>>   rename arch/powerpc/tools/{gcc-check-fpatchable-function-entry.sh => check-fpatchable-function-entry.sh} (100%)
>>
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index 325c1171894d..dfb62e211c92 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -568,8 +568,9 @@ config ARCH_USING_PATCHABLE_FUNCTION_ENTRY
>>   	depends on FUNCTION_TRACER && (PPC32 || PPC64_ELF_ABI_V2)
>>   	depends on $(cc-option,-fpatchable-function-entry=2)
>>   	def_bool y if PPC32
>> -	def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-fpatchable-function-entry.sh $(CC) -mlittle-endian) if PPC64 && CPU_LITTLE_ENDIAN
>> -	def_bool $(success,$(srctree)/arch/powerpc/tools/gcc-check-fpatchable-function-entry.sh $(CC) -mbig-endian) if PPC64 && CPU_BIG_ENDIAN
>> +	def_bool $(success,$(srctree)/arch/powerpc/tools/check-fpatchable-function-entry.sh $(CC) -mlittle-endian) if PPC64 && CPU_LITTLE_ENDIAN && CC_IS_GCC
>> +	def_bool $(success,$(srctree)/arch/powerpc/tools/check-fpatchable-function-entry.sh $(CC) -target ppc64le -mlittle-endian) if PPC64 && CPU_LITTLE_ENDIAN && CC_IS_CLANG
> 
> Can you instead pass $(CLANG_FLAGS) to retain the same command across
> gcc/clang?

Should work, I guess.
But do I need to test for any additional clang flags that
may interfere with what we are trying to check here?

- Hari

^ permalink raw reply
* Re: [PATCH v1 1/1] : Mark BPF printing functions with __printf() attribute
From: Andy Shevchenko @ 2025-12-08 15:50 UTC (permalink / raw)
  To: Alexei Starovoitov, bpf, linux-kernel, linux-trace-kernel
  Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Matt Bobrowski,
	Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Alan Maguire,
	Mirsad Todorovac
In-Reply-To: <20251208154733.2901633-1-andriy.shevchenko@linux.intel.com>

On Mon, Dec 08, 2025 at 04:47:33PM +0100, Andy Shevchenko wrote:
> The printing functions in BPF code are using printf() type of format,
> and compiler is not happy about them as is:

Sorry, this is noise that has to not be sent, it's WIP.
I will send it later when I provide a proper commit message.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply
* [PATCH v1 1/1] : Mark BPF printing functions with __printf() attribute
From: Andy Shevchenko @ 2025-12-08 15:47 UTC (permalink / raw)
  To: Alexei Starovoitov, bpf, linux-kernel, linux-trace-kernel
  Cc: Daniel Borkmann, Andrii Nakryiko, Martin KaFai Lau,
	Eduard Zingerman, Song Liu, Yonghong Song, John Fastabend,
	KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa, Matt Bobrowski,
	Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers, Alan Maguire,
	Mirsad Todorovac, Andy Shevchenko

The printing functions in BPF code are using printf() type of format,
and compiler is not happy about them as is:

kernel/bpf/helpers.c:1069:9: error: function ‘____bpf_snprintf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
 1069 |         err = bstr_printf(str, str_size, fmt, data.bin_args);
      |         ^~~

kernel/trace/bpf_trace.c:377:9: error: function ‘____bpf_trace_printk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
  377 |         ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
      |         ^~~

kernel/trace/bpf_trace.c:433:9: error: function ‘____bpf_trace_vprintk’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
  433 |         ret = bstr_printf(data.buf, MAX_BPRINTF_BUF, fmt, data.bin_args);
      |         ^~~

kernel/trace/bpf_trace.c:475:9: error: function ‘____bpf_seq_printf’ might be a candidate for ‘gnu_printf’ format attribute [-Werror=suggest-attribute=format]
  475 |         seq_bprintf(m, fmt, data.bin_args);
      |         ^~~~~~~~~~~

Fix the compilation errors by adding __printf() attribute. For that
we need to pass it down to the BPF_CALL_x() and wrap into PRINTF_BPF_CALL_*()
to make code neater.

| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202512061425.x0qTt9ww-lkp@intel.com/
| Closes: https://lore.kernel.org/oe-kbuild-all/202512061640.9hKTnB8p-lkp@intel.com/
| Closes: https://lore.kernel.org/oe-kbuild-all/202512081321.2h9ThWTg-lkp@intel.com/

Fixes:
Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 include/linux/filter.h   | 24 +++++++++++++++---------
 kernel/bpf/helpers.c     |  2 +-
 kernel/trace/bpf_trace.c |  6 +++---
 3 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index fd54fed8f95f..31034a74af22 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -607,9 +607,9 @@ static inline bool insn_is_cast_user(const struct bpf_insn *insn)
 	__BPF_MAP(n, __BPF_DECL_ARGS, __BPF_N, u64, __ur_1, u64, __ur_2,       \
 		  u64, __ur_3, u64, __ur_4, u64, __ur_5)
 
-#define BPF_CALL_x(x, attr, name, ...)					       \
+#define BPF_CALL_x(x, __attr, attr, name, ...)				       \
 	static __always_inline						       \
-	u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));   \
+	__attr u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__));     \
 	typedef u64 (*btf_##name)(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__)); \
 	attr u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__));    \
 	attr u64 name(__BPF_REG(x, __BPF_DECL_REGS, __BPF_N, __VA_ARGS__))     \
@@ -620,14 +620,20 @@ static inline bool insn_is_cast_user(const struct bpf_insn *insn)
 	u64 ____##name(__BPF_MAP(x, __BPF_DECL_ARGS, __BPF_V, __VA_ARGS__))
 
 #define __NOATTR
-#define BPF_CALL_0(name, ...)	BPF_CALL_x(0, __NOATTR, name, __VA_ARGS__)
-#define BPF_CALL_1(name, ...)	BPF_CALL_x(1, __NOATTR, name, __VA_ARGS__)
-#define BPF_CALL_2(name, ...)	BPF_CALL_x(2, __NOATTR, name, __VA_ARGS__)
-#define BPF_CALL_3(name, ...)	BPF_CALL_x(3, __NOATTR, name, __VA_ARGS__)
-#define BPF_CALL_4(name, ...)	BPF_CALL_x(4, __NOATTR, name, __VA_ARGS__)
-#define BPF_CALL_5(name, ...)	BPF_CALL_x(5, __NOATTR, name, __VA_ARGS__)
+#define BPF_CALL_0(name, ...)	BPF_CALL_x(0, __NOATTR, __NOATTR, name, __VA_ARGS__)
+#define BPF_CALL_1(name, ...)	BPF_CALL_x(1, __NOATTR, __NOATTR, name, __VA_ARGS__)
+#define BPF_CALL_2(name, ...)	BPF_CALL_x(2, __NOATTR, __NOATTR, name, __VA_ARGS__)
+#define BPF_CALL_3(name, ...)	BPF_CALL_x(3, __NOATTR, __NOATTR, name, __VA_ARGS__)
+#define BPF_CALL_4(name, ...)	BPF_CALL_x(4, __NOATTR, __NOATTR, name, __VA_ARGS__)
+#define BPF_CALL_5(name, ...)	BPF_CALL_x(5, __NOATTR, __NOATTR, name, __VA_ARGS__)
 
-#define NOTRACE_BPF_CALL_1(name, ...)	BPF_CALL_x(1, notrace, name, __VA_ARGS__)
+#define PRINTF_BPF_CALL_x(p, name, x, ...)				       \
+	BPF_CALL_x(x, __printf(p, 0), __NOATTR, name, __VA_ARGS__)
+
+#define PRINTF_BPF_CALL_4(p, name, ...)		PRINTF_BPF_CALL_x(p, name, 4, __VA_ARGS__)
+#define PRINTF_BPF_CALL_5(p, name, ...)		PRINTF_BPF_CALL_x(p, name, 5, __VA_ARGS__)
+
+#define NOTRACE_BPF_CALL_1(name, ...)	BPF_CALL_x(1, __NOATTR, notrace, name, __VA_ARGS__)
 
 #define bpf_ctx_range(TYPE, MEMBER)						\
 	offsetof(TYPE, MEMBER) ... offsetofend(TYPE, MEMBER) - 1
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index db72b96f9c8c..cbc66865e3dc 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1046,7 +1046,7 @@ int bpf_bprintf_prepare(const char *fmt, u32 fmt_size, const u64 *raw_args,
 	return err;
 }
 
-BPF_CALL_5(bpf_snprintf, char *, str, u32, str_size, char *, fmt,
+PRINTF_BPF_CALL_5(3, bpf_snprintf, char *, str, u32, str_size, char *, fmt,
 	   const void *, args, u32, data_len)
 {
 	struct bpf_bprintf_data data = {
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d57727abaade..5fd46b4bcf48 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -359,7 +359,7 @@ static const struct bpf_func_proto bpf_probe_write_user_proto = {
 #define MAX_TRACE_PRINTK_VARARGS	3
 #define BPF_TRACE_PRINTK_SIZE		1024
 
-BPF_CALL_5(bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
+PRINTF_BPF_CALL_5(1, bpf_trace_printk, char *, fmt, u32, fmt_size, u64, arg1,
 	   u64, arg2, u64, arg3)
 {
 	u64 args[MAX_TRACE_PRINTK_VARARGS] = { arg1, arg2, arg3 };
@@ -412,7 +412,7 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void)
 	return &bpf_trace_printk_proto;
 }
 
-BPF_CALL_4(bpf_trace_vprintk, char *, fmt, u32, fmt_size, const void *, args,
+PRINTF_BPF_CALL_4(1, bpf_trace_vprintk, char *, fmt, u32, fmt_size, const void *, args,
 	   u32, data_len)
 {
 	struct bpf_bprintf_data data = {
@@ -455,7 +455,7 @@ const struct bpf_func_proto *bpf_get_trace_vprintk_proto(void)
 	return &bpf_trace_vprintk_proto;
 }
 
-BPF_CALL_5(bpf_seq_printf, struct seq_file *, m, char *, fmt, u32, fmt_size,
+PRINTF_BPF_CALL_5(2, bpf_seq_printf, struct seq_file *, m, char *, fmt, u32, fmt_size,
 	   const void *, args, u32, data_len)
 {
 	struct bpf_bprintf_data data = {
-- 
2.50.1


^ permalink raw reply related
* Re: [RFC PATCH v2 14/15] unwind_user/backchain: Introduce back chain user space unwinding
From: Jens Remus @ 2025-12-08 15:38 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: linux-kernel, linux-trace-kernel, linux-s390, bpf, x86,
	Steven Rostedt, Heiko Carstens, Vasily Gorbik, Ilya Leoshkevich,
	Masami Hiramatsu, Mathieu Desnoyers, Peter Zijlstra, Ingo Molnar,
	Jiri Olsa, Arnaldo Carvalho de Melo, Namhyung Kim,
	Thomas Gleixner, Andrii Nakryiko, Indu Bhagat, Jose E. Marchesi,
	Beau Belgrave, Linus Torvalds, Andrew Morton, Florian Weimer,
	Kees Cook, Carlos O'Donell, Sam James, Dylan Hatch
In-Reply-To: <iidpbjmxnjf3zu4fa3atiubgb365yonv4gymaj76l6jvuxy67s@2y5o4txs4vhr>

Hello Josh,

thank you for your feedback!

On 12/7/2025 4:10 PM, Josh Poimboeuf wrote:
> On Fri, Dec 05, 2025 at 06:14:45PM +0100, Jens Remus wrote:
>> @@ -159,6 +165,10 @@ static int unwind_user_next(struct unwind_user_state *state)
>>  			if (!unwind_user_next_fp(state))
>>  				return 0;
>>  			continue;
>> +		case UNWIND_USER_TYPE_BACKCHAIN:
>> +			if (!unwind_user_next_backchain(state))
>> +				return 0;
>> +			continue;		/* Try next method. */
>>  		default:
>>  			WARN_ONCE(1, "Undefined unwind bit %d", bit);
>>  			break;
>> @@ -187,6 +197,8 @@ static int unwind_user_start(struct unwind_user_state *state)
>>  		state->available_types |= UNWIND_USER_TYPE_SFRAME;
>>  	if (IS_ENABLED(CONFIG_HAVE_UNWIND_USER_FP))
>>  		state->available_types |= UNWIND_USER_TYPE_FP;
>> +	if (IS_ENABLED(CONFIG_HAVE_UNWIND_USER_BACKCHAIN))
>> +		state->available_types |= UNWIND_USER_TYPE_BACKCHAIN;
> 
> Any reason not to just use the existing CONFIG_HAVE_UNWIND_USER_FP hook
> here rather than create the new BACKCHAIN one?

At first I thought this would not be a good idea, as my unwind user
backchain implementation relies on being standalone without using
unwind_user_next_common().  Mainly because s390 back chain unwinding
does not have fixed CFA, FP, and RA offsets/locations.  But then I gave
it a try and it does not look that bad actually.

I'll send a RFC v3 soon.

Regards,
Jens
-- 
Jens Remus
Linux on Z Development (D3303)
+49-7031-16-1128 Office
jremus@de.ibm.com

IBM

IBM Deutschland Research & Development GmbH; Vorsitzender des Aufsichtsrats: Wolfgang Wendt; Geschäftsführung: David Faller; Sitz der Gesellschaft: Böblingen; Registergericht: Amtsgericht Stuttgart, HRB 243294
IBM Data Privacy Statement: https://www.ibm.com/privacy/


^ permalink raw reply
* Re: PATCH] tracing: Fix unused tracepoints when module uses only exported ones
From: Mathieu Desnoyers @ 2025-12-08 14:22 UTC (permalink / raw)
  To: Steven Rostedt, LKML, Linux trace kernel
  Cc: Masami Hiramatsu, Masahiro Yamada
In-Reply-To: <20251208085336.6658743c@debian>

On 2025-12-08 08:53, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>

Missing bracket for [PATCH] in subject.

> 
> Building the KVM intel module failed to build with UT=1:

failed -> fails (present ?)

> 
> no __tracepoint_strings in file: arch/x86/kvm/kvm-intel.o
> make[3]: *** [/work/git/test-linux.git/scripts/Makefile.modfinal:62: arch/x86/kvm/kvm-intel.ko] Error 1
> 
> The reason is that the module only uses the tracepoints defined and
> exported by the main kvm module. The tracepoint-udpate.c code fails the

I guess you mean "tracepoint-update.c" ?

> build if a tracepoint is used, but there's no tracepoints defined. But

tracepoint

> this is acceptable in modules if the tracepoints is defined in the vmlinux

tracepoint

> proper or another module and exported.
> 
> Do not fail to build if a tracepoint is used but no tracepoints are

tracepoint .. is

Thanks,

Mathieu

> defined if the code is a module. This should still never happen for the
> vmlinux itself.
> 
> Fixes: e30f8e61e2518 ("tracing: Add a tracepoint verification check at build time")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>   scripts/tracepoint-update.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/scripts/tracepoint-update.c b/scripts/tracepoint-update.c
> index 7f7d90df14ce..90046aedc97b 100644
> --- a/scripts/tracepoint-update.c
> +++ b/scripts/tracepoint-update.c
> @@ -210,6 +210,9 @@ static int process_tracepoints(bool mod, void *addr, const char *fname)
>   	}
>   
>   	if (!tracepoint_data_sec) {
> +		/* A module may reference only exported tracepoints */
> +		if (mod)
> +			return 0;
>   		fprintf(stderr,	"no __tracepoint_strings in file: %s\n", fname);
>   		return -1;
>   	}


-- 
Mathieu Desnoyers
EfficiOS Inc.
https://www.efficios.com

^ permalink raw reply
* PATCH] tracing: Fix unused tracepoints when module uses only exported ones
From: Steven Rostedt @ 2025-12-08 13:53 UTC (permalink / raw)
  To: LKML, Linux trace kernel
  Cc: Masami Hiramatsu, Mathieu Desnoyers, Masahiro Yamada

From: Steven Rostedt <rostedt@goodmis.org>

Building the KVM intel module failed to build with UT=1:

no __tracepoint_strings in file: arch/x86/kvm/kvm-intel.o
make[3]: *** [/work/git/test-linux.git/scripts/Makefile.modfinal:62: arch/x86/kvm/kvm-intel.ko] Error 1

The reason is that the module only uses the tracepoints defined and
exported by the main kvm module. The tracepoint-udpate.c code fails the
build if a tracepoint is used, but there's no tracepoints defined. But
this is acceptable in modules if the tracepoints is defined in the vmlinux
proper or another module and exported.

Do not fail to build if a tracepoint is used but no tracepoints are
defined if the code is a module. This should still never happen for the
vmlinux itself.

Fixes: e30f8e61e2518 ("tracing: Add a tracepoint verification check at build time")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 scripts/tracepoint-update.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/tracepoint-update.c b/scripts/tracepoint-update.c
index 7f7d90df14ce..90046aedc97b 100644
--- a/scripts/tracepoint-update.c
+++ b/scripts/tracepoint-update.c
@@ -210,6 +210,9 @@ static int process_tracepoints(bool mod, void *addr, const char *fname)
 	}
 
 	if (!tracepoint_data_sec) {
+		/* A module may reference only exported tracepoints */
+		if (mod)
+			return 0;
 		fprintf(stderr,	"no __tracepoint_strings in file: %s\n", fname);
 		return -1;
 	}
-- 
2.51.0


^ permalink raw reply related
* Re: [RFC 1/2] Bluetooth: hci_conn: Add tracepoints for debugging
From: Paul Menzel @ 2025-12-08 13:24 UTC (permalink / raw)
  To: Wang Yaxin
  Cc: marcel, johan.hedberg, luiz.dentz, linux-bluetooth, xu.xin16,
	yang.yang29, qiu.yutan, chen.junlin, jiang.kun2,
	linux-trace-kernel
In-Reply-To: <20251208203245047U3UW5v2aGUumhmvjLl96E@zte.com.cn>

[Cc: +linux-trace-kernel@vger.kernel.org]

Dear Yutan,


Thank you for your patch.

Am 08.12.25 um 13:32 schrieb wang.yaxin@zte.com.cn:
> From: Qiu Yutan <qiu.yutan@zte.com.cn>
> 
> Add tracepoints for tracking hci_conn_hold, hci_conn_drop, and hci_conn_del
> to facilitate debugging and viewing call stacks.

I’d add a blank line between paragraphs.

> The existing Bluetooth debugging method, BT_DBG, cannot trace call stacks.

It’d be great if you added an example output, how to use hte new trace 
points.

> Signed-off-by: Qiu Yutan <qiu.yutan@zte.com.cn>
> Signed-off-by: Wang Yaxin <wang.yaxin@zte.com.cn>
> Signed-off-by: xu xin <xu.xin16@zte.com.cn>

Please start each name with a capital letter.

> Signed-off-by: Chen Junlin <chen.junlin@zte.com.cn>
> ---
>   include/net/bluetooth/hci_core.h |  3 ++
>   include/trace/events/bluetooth.h | 80 ++++++++++++++++++++++++++++++++
>   net/bluetooth/hci_conn.c         |  4 ++
>   3 files changed, 87 insertions(+)
>   create mode 100644 include/trace/events/bluetooth.h
> 
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 2b261e74e2c4..5e01e6c501c1 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -29,6 +29,7 @@
>   #include <linux/idr.h>
>   #include <linux/leds.h>
>   #include <linux/rculist.h>
> +#include <trace/events/bluetooth.h>
> 
>   #include <net/bluetooth/hci.h>
>   #include <net/bluetooth/hci_drv.h>
> @@ -1620,6 +1621,7 @@ static inline void hci_conn_put(struct hci_conn *conn)
> 
>   static inline struct hci_conn *hci_conn_hold(struct hci_conn *conn)
>   {
> +	trace_hci_conn_hold(conn);
>   	BT_DBG("hcon %p orig refcnt %d", conn, atomic_read(&conn->refcnt));
> 
>   	atomic_inc(&conn->refcnt);
> @@ -1630,6 +1632,7 @@ static inline struct hci_conn *hci_conn_hold(struct hci_conn *conn)
> 
>   static inline void hci_conn_drop(struct hci_conn *conn)
>   {
> +	trace_hci_conn_drop(conn);
>   	BT_DBG("hcon %p orig refcnt %d", conn, atomic_read(&conn->refcnt));
> 
>   	if (atomic_dec_and_test(&conn->refcnt)) {
> diff --git a/include/trace/events/bluetooth.h b/include/trace/events/bluetooth.h
> new file mode 100644
> index 000000000000..dd6446263e83
> --- /dev/null
> +++ b/include/trace/events/bluetooth.h
> @@ -0,0 +1,80 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM bluetooth
> +
> +#if !defined(_TRACE_BLUETOOTH_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_BLUETOOTH_H
> +
> +#include <linux/tracepoint.h>
> +#include <net/bluetooth/hci_core.h>
> +
> +TRACE_EVENT(hci_conn_hold,
> +
> +	TP_PROTO(void *conn_ptr),
> +
> +	TP_ARGS(conn_ptr),
> +
> +	TP_STRUCT__entry(
> +		__field(void *, conn_addr)
> +		__field(int, refcnt)
> +	),
> +
> +	TP_fast_assign(
> +		struct hci_conn *conn = (struct hci_conn *)conn_ptr;
> +
> +		__entry->conn_addr = conn;
> +		__entry->refcnt = atomic_read(&conn->refcnt);
> +	),
> +
> +	TP_printk("conn_addr=%p, orig refcnt=%d",
> +			__entry->conn_addr, __entry->refcnt)
> +)
> +
> +TRACE_EVENT(hci_conn_drop,
> +
> +	TP_PROTO(void *conn_ptr),
> +
> +	TP_ARGS(conn_ptr),
> +
> +	TP_STRUCT__entry(
> +		__field(void *, conn_addr)
> +		__field(int, refcnt)
> +	),
> +
> +	TP_fast_assign(
> +		struct hci_conn *conn = (struct hci_conn *)conn_ptr;
> +
> +		__entry->conn_addr = conn;
> +		__entry->refcnt = atomic_read(&conn->refcnt);
> +	),
> +
> +	TP_printk("conn_addr=%p, orig refcnt=%d",
> +			__entry->conn_addr, __entry->refcnt)
> +)
> +
> +TRACE_EVENT(hci_conn_del,
> +
> +	TP_PROTO(void *conn_ptr),
> +
> +	TP_ARGS(conn_ptr),
> +
> +	TP_STRUCT__entry(
> +		__field(void *, conn_addr)
> +		 __field(int, refcnt)
> +	),
> +
> +	TP_fast_assign(
> +		struct hci_conn *conn = (struct hci_conn *)conn_ptr;
> +
> +		__entry->conn_addr = conn;
> +		__entry->refcnt = atomic_read(&conn->refcnt);
> +	),
> +
> +	TP_printk("conn_addr=%p, orig refcnt=%d",
> +			__entry->conn_addr, __entry->refcnt)
> +)

All definitions look the same besides the name. Any idea, how the 
duplication can be avoided?

> +
> +#endif /* _TRACE_BLUETOOTH_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 99efeed6a766..74a02cf7ba14 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -35,6 +35,9 @@
>   #include <net/bluetooth/iso.h>
>   #include <net/bluetooth/mgmt.h>
> 
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/bluetooth.h>
> +
>   #include "smp.h"
>   #include "eir.h"
> 
> @@ -1129,6 +1132,7 @@ static void hci_conn_unlink(struct hci_conn *conn)
> 
>   void hci_conn_del(struct hci_conn *conn)
>   {
> +	trace_hci_conn_del(conn);
>   	struct hci_dev *hdev = conn->hdev;
> 
>   	BT_DBG("%s hcon %p handle %d", hdev->name, conn, conn->handle);

Thank you for working on that. I’t great to see.


Kind regards,

Paul

^ permalink raw reply
* [PATCH v2 2/2] tracing: Update funcgraph-retval documentation
From: Donglin Peng @ 2025-12-08 13:19 UTC (permalink / raw)
  To: rostedt
  Cc: linux-trace-kernel, linux-kernel, pengdonglin, Masami Hiramatsu,
	Xiaoqin Zhang
In-Reply-To: <20251208131917.2444620-1-dolinux.peng@gmail.com>

From: pengdonglin <pengdonglin@xiaomi.com>

The existing documentation for funcgraph-retval is outdated and partially
incorrect, as it describes limitations that have now been resolved.

Recent changes (e.g., using BTF to obtain function return types) have
addressed key issues:
1. Return values are now printed only for non-void functions.
2. Values are trimmed to the correct width of the return type, avoiding
   garbage data from high bits.

Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Xiaoqin Zhang <zhangxiaoqin@xiaomi.com>
Signed-off-by: pengdonglin <pengdonglin@xiaomi.com>
---
 Documentation/trace/ftrace.rst | 76 +++++++++++++++++++---------------
 1 file changed, 43 insertions(+), 33 deletions(-)

diff --git a/Documentation/trace/ftrace.rst b/Documentation/trace/ftrace.rst
index d1f313a5f4ad..d2edd58f04c3 100644
--- a/Documentation/trace/ftrace.rst
+++ b/Documentation/trace/ftrace.rst
@@ -1454,6 +1454,10 @@ Options for function_graph tracer:
 	printed in hexadecimal format. By default, this option
 	is off.
 
+  funcgraph-retaddr
+	When set, the return address will always be printed.
+	By default, this option is off.
+
   sleep-time
 	When running function graph tracer, to include
 	the time a task schedules out in its function.
@@ -2800,7 +2804,7 @@ It is default disabled.
     0)   2.861 us    |      } /* putname() */
 
 The return value of each traced function can be displayed after
-an equal sign "=". When encountering system call failures, it
+an equal sign "ret =". When encountering system call failures, it
 can be very helpful to quickly locate the function that first
 returns an error code.
 
@@ -2810,16 +2814,16 @@ returns an error code.
   Example with funcgraph-retval::
 
     1)               |    cgroup_migrate() {
-    1)   0.651 us    |      cgroup_migrate_add_task(); /* = 0xffff93fcfd346c00 */
+    1)   0.651 us    |      cgroup_migrate_add_task(); /* ret=0xffff93fcfd346c00 */
     1)               |      cgroup_migrate_execute() {
     1)               |        cpu_cgroup_can_attach() {
     1)               |          cgroup_taskset_first() {
-    1)   0.732 us    |            cgroup_taskset_next(); /* = 0xffff93fc8fb20000 */
-    1)   1.232 us    |          } /* cgroup_taskset_first = 0xffff93fc8fb20000 */
-    1)   0.380 us    |          sched_rt_can_attach(); /* = 0x0 */
-    1)   2.335 us    |        } /* cpu_cgroup_can_attach = -22 */
-    1)   4.369 us    |      } /* cgroup_migrate_execute = -22 */
-    1)   7.143 us    |    } /* cgroup_migrate = -22 */
+    1)   0.732 us    |            cgroup_taskset_next(); /* ret=0xffff93fc8fb20000 */
+    1)   1.232 us    |          } /* cgroup_taskset_first ret=0xffff93fc8fb20000 */
+    1)   0.380 us    |          sched_rt_can_attach(); /* ret=0x0 */
+    1)   2.335 us    |        } /* cpu_cgroup_can_attach ret=-22 */
+    1)   4.369 us    |      } /* cgroup_migrate_execute ret=-22 */
+    1)   7.143 us    |    } /* cgroup_migrate ret=-22 */
 
 The above example shows that the function cpu_cgroup_can_attach
 returned the error code -22 firstly, then we can read the code
@@ -2836,37 +2840,39 @@ printed in hexadecimal format.
   Example with funcgraph-retval-hex::
 
     1)               |      cgroup_migrate() {
-    1)   0.651 us    |        cgroup_migrate_add_task(); /* = 0xffff93fcfd346c00 */
+    1)   0.651 us    |        cgroup_migrate_add_task(); /* ret=0xffff93fcfd346c00 */
     1)               |        cgroup_migrate_execute() {
     1)               |          cpu_cgroup_can_attach() {
     1)               |            cgroup_taskset_first() {
-    1)   0.732 us    |              cgroup_taskset_next(); /* = 0xffff93fc8fb20000 */
-    1)   1.232 us    |            } /* cgroup_taskset_first = 0xffff93fc8fb20000 */
-    1)   0.380 us    |            sched_rt_can_attach(); /* = 0x0 */
-    1)   2.335 us    |          } /* cpu_cgroup_can_attach = 0xffffffea */
-    1)   4.369 us    |        } /* cgroup_migrate_execute = 0xffffffea */
+    1)   0.732 us    |              cgroup_taskset_next(); /* ret=0xffff93fc8fb20000 */
+    1)   1.232 us    |            } /* cgroup_taskset_first ret=0xffff93fc8fb20000 */
+    1)   0.380 us    |            sched_rt_can_attach(); /* ret=0x0 */
+    1)   2.335 us    |          } /* cpu_cgroup_can_attach ret=0xffffffea */
+    1)   4.369 us    |        } /* cgroup_migrate_execute ret=0xffffffea */
     1)   7.143 us    |      } /* cgroup_migrate = 0xffffffea */
 
-At present, there are some limitations when using the funcgraph-retval
-option, and these limitations will be eliminated in the future:
+Note that there are some limitations when using the funcgraph-retval
+option:
+
+- If CONFIG_DEBUG_INFO_BTF is disabled (n), a return value is printed even for
+  functions with a void return type. When CONFIG_DEBUG_INFO_BTF is enabled (y),
+  the return value is printed only for non-void functions.
 
-- Even if the function return type is void, a return value will still
-  be printed, and you can just ignore it.
+- If a return value occupies multiple registers, only the value in the first
+  register is recorded and printed. For example, on the x86 architecture, a
+  64-bit return value is stored across eax (lower 32 bits) and edx (upper 32 bits),
+  but only the contents of eax are captured.
 
-- Even if return values are stored in multiple registers, only the
-  value contained in the first register will be recorded and printed.
-  To illustrate, in the x86 architecture, eax and edx are used to store
-  a 64-bit return value, with the lower 32 bits saved in eax and the
-  upper 32 bits saved in edx. However, only the value stored in eax
-  will be recorded and printed.
+- Under certain procedure-call standards (e.g., arm64's AAPCS64), when the return
+  type is smaller than a general-purpose register (GPR), the caller is responsible
+  for narrowing the value; the upper bits of the register may contain undefined data.
+  For instance, when a u8 is returned in 64-bit GPR, bits [63:8] can hold arbitrary
+  values, especially when larger types are truncated (explicitly or implicitly). It
+  is therefore advisable to inspect the code in such cases. If CONFIG_DEBUG_INFO_BTF
+  is enabled (y), the return value is automatically trimmed to the width of the return
+  type.
 
-- In certain procedure call standards, such as arm64's AAPCS64, when a
-  type is smaller than a GPR, it is the responsibility of the consumer
-  to perform the narrowing, and the upper bits may contain UNKNOWN values.
-  Therefore, it is advisable to check the code for such cases. For instance,
-  when using a u8 in a 64-bit GPR, bits [63:8] may contain arbitrary values,
-  especially when larger types are truncated, whether explicitly or implicitly.
-  Here are some specific cases to illustrate this point:
+  The following examples illustrate the behavior:
 
   **Case One**:
 
@@ -2885,7 +2891,9 @@ option, and these limitations will be eliminated in the future:
 		RET
 
   If you pass 0x123456789abcdef to this function and want to narrow it,
-  it may be recorded as 0x123456789abcdef instead of 0xef.
+  it may be recorded as 0x123456789abcdef instead of 0xef. When
+  CONFIG_DEBUG_INFO_BTF is enabled, the value will be correctly truncated
+  to 0xef based on the size constraints of the u8 type.
 
   **Case Two**:
 
@@ -2910,7 +2918,9 @@ option, and these limitations will be eliminated in the future:
 		RET
 
   When passing 0x2_0000_0000 to it, the return value may be recorded as
-  0x2_0000_0000 instead of 0.
+  0x2_0000_0000 instead of 0. When CONFIG_DEBUG_INFO_BTF is enabled, the
+  value will be correctly truncated to 0 based on the size constraints of
+  the int type.
 
 You can put some comments on specific functions by using
 trace_printk() For example, if you want to put a comment inside
-- 
2.34.1


^ permalink raw reply related
* [PATCH v2 1/2] fgraph: Use BTF to trim and filter return values
From: Donglin Peng @ 2025-12-08 13:19 UTC (permalink / raw)
  To: rostedt
  Cc: linux-trace-kernel, linux-kernel, pengdonglin, Masami Hiramatsu,
	Xiaoqin Zhang
In-Reply-To: <20251208131917.2444620-1-dolinux.peng@gmail.com>

From: pengdonglin <pengdonglin@xiaomi.com>

The current funcgraph-retval implementation has two limitations:

1. It prints a return value even when the traced function returns void.
2. When the return type is narrower than a register, the printed value may
   be incorrect because high bits can contain undefined data.

Both issues are addressed by using BTF to obtain the precise return type
of each traced function:

- Return values are now printed only for functions whose return type is
  not void.
- The value is truncated to the actual width of the return type, ensuring
  correct representation.

These changes make the funcgraph-retval output more accurate and remove
noise from void functions.

Cc: Steven Rostedt (Google) <rostedt@goodmis.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Xiaoqin Zhang <zhangxiaoqin@xiaomi.com>
Signed-off-by: pengdonglin <pengdonglin@xiaomi.com>
---
 kernel/trace/trace_functions_graph.c | 64 +++++++++++++++++++++++-----
 1 file changed, 54 insertions(+), 10 deletions(-)

diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c
index 17c75cf2348e..9e63665c81e2 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -15,6 +15,7 @@
 
 #include "trace.h"
 #include "trace_output.h"
+#include "trace_btf.h"
 
 /* When set, irq functions might be ignored */
 static int ftrace_graph_skip_irqs;
@@ -865,6 +866,46 @@ static void print_graph_retaddr(struct trace_seq *s, struct fgraph_retaddr_ent_e
 
 #if defined(CONFIG_FUNCTION_GRAPH_RETVAL) || defined(CONFIG_FUNCTION_GRAPH_RETADDR)
 
+static void trim_retval(unsigned long func, unsigned long *retval, bool *print_retval)
+{
+	const struct btf_type *t;
+	char name[KSYM_NAME_LEN];
+	struct btf *btf;
+	u32 v, msb;
+
+	if (!IS_ENABLED(CONFIG_DEBUG_INFO_BTF))
+		return;
+
+	if (lookup_symbol_name(func, name))
+		return;
+
+	t = btf_find_func_proto(name, &btf);
+	if (IS_ERR_OR_NULL(t))
+		return;
+
+	t = btf_type_skip_modifiers(btf, t->type, NULL);
+	switch (t ? BTF_INFO_KIND(t->info) : BTF_KIND_UNKN) {
+	case BTF_KIND_UNKN:
+		*print_retval = false;
+		break;
+	case BTF_KIND_ENUM:
+	case BTF_KIND_ENUM64:
+		msb = BITS_PER_BYTE * t->size - 1;
+		*retval &= GENMASK(msb, 0);
+		break;
+	case BTF_KIND_INT:
+		v = *(u32 *)(t + 1);
+		if (BTF_INT_ENCODING(v) == BTF_INT_BOOL)
+			msb = 0;
+		else
+			msb = BTF_INT_BITS(v) - 1;
+		*retval &= GENMASK(msb, 0);
+		break;
+	default:
+		break;
+	}
+}
+
 static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entry *entry,
 				struct ftrace_graph_ret *graph_ret, void *func,
 				u32 opt_flags, u32 trace_flags, int args_size)
@@ -884,17 +925,20 @@ static void print_graph_retval(struct trace_seq *s, struct ftrace_graph_ent_entr
 	print_retaddr = !!(opt_flags & TRACE_GRAPH_PRINT_RETADDR);
 #endif
 
-	if (print_retval && retval && !hex_format) {
-		/* Check if the return value matches the negative format */
-		if (IS_ENABLED(CONFIG_64BIT) && (retval & BIT(31)) &&
-			(((u64)retval) >> 32) == 0) {
-			err_code = sign_extend64(retval, 31);
-		} else {
-			err_code = retval;
+	if (print_retval) {
+		trim_retval((unsigned long)func, &retval, &print_retval);
+		if (print_retval && retval && !hex_format) {
+			/* Check if the return value matches the negative format */
+			if (IS_ENABLED(CONFIG_64BIT) && (retval & BIT(31)) &&
+				(((u64)retval) >> 32) == 0) {
+				err_code = sign_extend64(retval, 31);
+			} else {
+				err_code = retval;
+			}
+
+			if (!IS_ERR_VALUE(err_code))
+				err_code = 0;
 		}
-
-		if (!IS_ERR_VALUE(err_code))
-			err_code = 0;
 	}
 
 	if (entry) {
-- 
2.34.1


^ permalink raw reply related
* [PATCH v2 0/2] Use BTF to trim return values
From: Donglin Peng @ 2025-12-08 13:19 UTC (permalink / raw)
  To: rostedt; +Cc: linux-trace-kernel, linux-kernel, pengdonglin

From: pengdonglin <pengdonglin@xiaomi.com>

This patch series addresses two limitations of the funcgraph-retval feature:

1. Previously, return values were printed even for void functions.
2. For return types narrower than a register, high-bit corruption could
   lead to incorrect values.

By leveraging BTF to obtain precise return type information, we now:

1. Suppress output for void functions (eliminating noise)
2. Properly truncate values to match the actual return type width (ensuring
  correctness)

These improvements make funcgraph-retval more accurate and useful.

Before:
 # perf ftrace -G vfs_read --graph-opts retval
 ...
 1)               |    _raw_spin_unlock() {
 1)   0.074 us    |      do_raw_spin_unlock(); /* ret=0x1 */
 1)   0.072 us    |      preempt_count_sub(); /* ret=0x0 */
 1)   0.373 us    |    } /* _raw_spin_unlock ret=0x80000000 */
 1)               |    down_read() {
 1)   0.070 us    |      __cond_resched(); /* ret=0x0 */
 1)   0.069 us    |      preempt_count_add(); /* ret=0x0 */
 1)   0.071 us    |      preempt_count_sub(); /* ret=0x0 */
 1)   0.612 us    |    } /* down_read ret=0x80000000 */
 1)               |    down_write() {
 1)   0.070 us    |      __cond_resched(); /* ret=0x0 */
 1)   0.071 us    |      preempt_count_add(); /* ret=0x0 */
 1)   0.070 us    |      preempt_count_sub(); /* ret=0x0 */
 1)   0.605 us    |    } /* down_write ret=0x80000000 */

After:
 # perf ftrace -G vfs_read --graph-opts retval
 ...
 0)               |    _raw_spin_unlock() {
 0)   0.075 us    |      do_raw_spin_unlock();
 0)   0.075 us    |      preempt_count_sub();
 0)   0.380 us    |    } /* _raw_spin_unlock */
 0)               |    down_read() {
 0)   0.070 us    |      __cond_resched(); /* ret=0x0 */
 0)   0.073 us    |      preempt_count_add();
 0)   0.072 us    |      preempt_count_sub();
 0)   0.586 us    |    } /* down_read */
 0)               |    down_write() {
 0)   0.070 us    |      __cond_resched(); /* ret=0x0 */
 0)   0.072 us    |      preempt_count_add();
 0)   0.072 us    |      preempt_count_sub();
 0)   0.676 us    |    } /* down_write */

Changelog:
v2:
- Update the funcgraph-retval documentation
- Revise the cover letter

v1:
- Link: https://lore.kernel.org/all/20251207142742.229924-1-dolinux.peng@gmail.com/

pengdonglin (2):
  fgraph: Use BTF to trim and filter return values
  tracing: Update funcgraph-retval documentation

 Documentation/trace/ftrace.rst       | 76 ++++++++++++++++------------
 kernel/trace/trace_functions_graph.c | 64 +++++++++++++++++++----
 2 files changed, 97 insertions(+), 43 deletions(-)

-- 
2.34.1


^ permalink raw reply
* Re: [RFC PATCH 0/8] uprobe/x86: Add support to optimize prologue
From: Oleg Nesterov @ 2025-12-08 10:29 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Jiri Olsa, Peter Zijlstra, Andrii Nakryiko, bpf, linux-kernel,
	linux-trace-kernel, x86, Song Liu, Yonghong Song, John Fastabend,
	Steven Rostedt, Ingo Molnar, David Laight
In-Reply-To: <20251208153056.3a4e9cd3511ccf00dc12e265@kernel.org>

On 12/08, Masami Hiramatsu wrote:
>
> On Mon, 24 Nov 2025 19:12:42 +0100
> Oleg Nesterov <oleg@redhat.com> wrote:
>
> > On 11/17, Jiri Olsa wrote:
> > >
> > > There's an additional issue that single instruction replacement does
> > > not have and it's the possibility of the user space code to jump in the
> > > middle of those 5 bytes. I think it's unlikely to happen at the function
> > > prologue, but uprobe could be placed anywhere. I'm not sure how to
> > > mitigate this other than having some enable/disable switch or config
> > > option, which is unfortunate.
> >
> > plus this breaks single-stepping... Although perhaps we don't really care.
>
> Yeah, and I think we can stop optimization if post_handler is set.

Hmm, why? This doesn't depend on whether ->ret_handler is set or not...

Oleg.


^ permalink raw reply
* Re: [PATCH] kprobes: Call check_ftrace_location() on CONFIG_KPROBES_ON_FTRACE
From: qingwei hu @ 2025-12-08  8:59 UTC (permalink / raw)
  To: Masami Hiramatsu (Google)
  Cc: davem, linux-kernel, linux-trace-kernel, naveen, qingwei.hu,
	rostedt
In-Reply-To: <20251208161955.54e2fef0fc793092a93ce88e@kernel.org>


> On Dec 8, 2025, at 15:19, Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:
> 
> On Mon, 8 Dec 2025 14:54:33 +0800
> qingwei hu <huqingwei.kernel@gmail.com> wrote:
> 
>> 
>> 
>>> 2025年12月5日 23:08，Steven Rostedt <rostedt@goodmis.org> 写道：
>>> 
>>> On Fri,  5 Dec 2025 17:29:33 +0800
>>> "qingwei.hu" <qingwei.hu@bytedance.com> wrote:
>>> 
>>>> From: Qingwei Hu <qingwei.hu@bytedance.com>
>>>> 
>>>> There is a possible configuration dependency:
>>>> 
>>>> KPROBES_ON_FTRACE [=n]
>>>>      ^----- KPROBES [=y]
>>>>        |--- HAVE_KPROBES_ON_FTRACE [=n]
>>>>        |--- DYNAMIC_FTRACE_WITH_REGS [=n]
>>>>               ^----- FTRACE [=y]
>>>>                 |--- DYNAMIC_FTRACE [=y]
>>>>                 |--- HAVE_DYNAMIC_FTRACE_WITH_REGS [=n]
>>>> 
>>>> With DYNAMIC_FTRACE=y, ftrace_location() is meaningful and may
>>>> return the same address as the probe target.
>>>> 
>>>> However, when KPROBES_ON_FTRACE=n, the current implementation
>>>> returns -EINVAL after calling check_ftrace_location(), causing
>>>> the validation to fail.
>>> 
>>> This is a feature not a bug.
>>> 
>>> The reason is if you put a kprobe on a ftrace location, it can cause ftrace
>>> to trigger a bug, as kprobes will modify the location and ftrace will see
>>> something it doesn't expect and think the system is corrupted. We don't want
>>> that either.
>>> 
>>> If you say "KPROBES_ON_FTRACE=n" and place a kprobe on a location that is
>>> controlled by ftrace, it had better fail!
>>> 
>>> NAK
>>> 
>>> -- Steve
>> 
>> Thanks for your clear explanation. I will look into other approaches
>> that work with this configuration.
>> 
> 
> Can you check the code under arch/<your machine>/ implements the
> kprobe_ftrace_handler() correctly? if so, it should be enabled automatically.
> 
> Thank you,
> 
> 
> -- 
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

I am working on the risc-v that have removed support for KPROBE_ON_FTRACE and
probe_ftrace_hander() since 7caa9765465f60 and 3308172276db5d.

As 7caa9765465f60 commit say KPROBES_ON_FTRACE will be supplanted by FPROBES,
I have tested and verified this feature's usability. In this situation, may
be we should use fprobe instead of kprobe.

But in the latest version, risc-v adjusts ftrace location to an offset 4 byte
from the function symbol. So it will kprobe successfully in the beginning of
the symbol while not support KPROBE_ON_FTRACE. I am not sure whether this
arrangement is consistent with Steve’s explanation. If the adjustment is valid,
I can temporarily backport the adjust function in order to support kprobe.


^ permalink raw reply
* Re: [PATCH] kprobes: Call check_ftrace_location() on CONFIG_KPROBES_ON_FTRACE
From: Masami Hiramatsu @ 2025-12-08  7:19 UTC (permalink / raw)
  To: qingwei hu
  Cc: Steven Rostedt, davem, linux-kernel, linux-trace-kernel, mhiramat,
	naveen, qingwei.hu
In-Reply-To: <2E248E7C-773B-4998-B059-A576163B36B0@bytedance.com>

On Mon, 8 Dec 2025 14:54:33 +0800
qingwei hu <huqingwei.kernel@gmail.com> wrote:

> 
> 
> > 2025年12月5日 23:08，Steven Rostedt <rostedt@goodmis.org> 写道：
> > 
> > On Fri,  5 Dec 2025 17:29:33 +0800
> > "qingwei.hu" <qingwei.hu@bytedance.com> wrote:
> > 
> >> From: Qingwei Hu <qingwei.hu@bytedance.com>
> >> 
> >> There is a possible configuration dependency:
> >> 
> >>  KPROBES_ON_FTRACE [=n]
> >>       ^----- KPROBES [=y]
> >>         |--- HAVE_KPROBES_ON_FTRACE [=n]
> >>         |--- DYNAMIC_FTRACE_WITH_REGS [=n]
> >>                ^----- FTRACE [=y]
> >>                  |--- DYNAMIC_FTRACE [=y]
> >>                  |--- HAVE_DYNAMIC_FTRACE_WITH_REGS [=n]
> >> 
> >> With DYNAMIC_FTRACE=y, ftrace_location() is meaningful and may
> >> return the same address as the probe target.
> >> 
> >> However, when KPROBES_ON_FTRACE=n, the current implementation
> >> returns -EINVAL after calling check_ftrace_location(), causing
> >> the validation to fail.
> > 
> > This is a feature not a bug.
> > 
> > The reason is if you put a kprobe on a ftrace location, it can cause ftrace
> > to trigger a bug, as kprobes will modify the location and ftrace will see
> > something it doesn't expect and think the system is corrupted. We don't want
> > that either.
> > 
> > If you say "KPROBES_ON_FTRACE=n" and place a kprobe on a location that is
> > controlled by ftrace, it had better fail!
> > 
> > NAK
> > 
> > -- Steve
> 
> Thanks for your clear explanation. I will look into other approaches
> that work with this configuration.
> 

Can you check the code under arch/<your machine>/ implements the
kprobe_ftrace_handler() correctly? if so, it should be enabled automatically.

Thank you,


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply
* Re: [PATCH] kprobes: Call check_ftrace_location() on CONFIG_KPROBES_ON_FTRACE
From: qingwei hu @ 2025-12-08  6:54 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: davem, linux-kernel, linux-trace-kernel, mhiramat, naveen,
	qingwei.hu
In-Reply-To: <20251205100823.28aa8ffd@gandalf.local.home>



> 2025年12月5日 23:08，Steven Rostedt <rostedt@goodmis.org> 写道：
> 
> On Fri,  5 Dec 2025 17:29:33 +0800
> "qingwei.hu" <qingwei.hu@bytedance.com> wrote:
> 
>> From: Qingwei Hu <qingwei.hu@bytedance.com>
>> 
>> There is a possible configuration dependency:
>> 
>>  KPROBES_ON_FTRACE [=n]
>>       ^----- KPROBES [=y]
>>         |--- HAVE_KPROBES_ON_FTRACE [=n]
>>         |--- DYNAMIC_FTRACE_WITH_REGS [=n]
>>                ^----- FTRACE [=y]
>>                  |--- DYNAMIC_FTRACE [=y]
>>                  |--- HAVE_DYNAMIC_FTRACE_WITH_REGS [=n]
>> 
>> With DYNAMIC_FTRACE=y, ftrace_location() is meaningful and may
>> return the same address as the probe target.
>> 
>> However, when KPROBES_ON_FTRACE=n, the current implementation
>> returns -EINVAL after calling check_ftrace_location(), causing
>> the validation to fail.
> 
> This is a feature not a bug.
> 
> The reason is if you put a kprobe on a ftrace location, it can cause ftrace
> to trigger a bug, as kprobes will modify the location and ftrace will see
> something it doesn't expect and think the system is corrupted. We don't want
> that either.
> 
> If you say "KPROBES_ON_FTRACE=n" and place a kprobe on a location that is
> controlled by ftrace, it had better fail!
> 
> NAK
> 
> -- Steve

Thanks for your clear explanation. I will look into other approaches
that work with this configuration.


^ permalink raw reply
* Re: [RFC PATCH 0/8] uprobe/x86: Add support to optimize prologue
From: Masami Hiramatsu @ 2025-12-08  6:30 UTC (permalink / raw)
  To: Oleg Nesterov
  Cc: Jiri Olsa, Masami Hiramatsu, Peter Zijlstra, Andrii Nakryiko, bpf,
	linux-kernel, linux-trace-kernel, x86, Song Liu, Yonghong Song,
	John Fastabend, Steven Rostedt, Ingo Molnar, David Laight
In-Reply-To: <aSSgGu8X04XoYN8D@redhat.com>

On Mon, 24 Nov 2025 19:12:42 +0100
Oleg Nesterov <oleg@redhat.com> wrote:

> On 11/17, Jiri Olsa wrote:
> >
> > This patchset adds support to optimize uprobe on top of instruction
> > that could be emulated and also adds support to emulate particular
> > versions of mov and sub instructions to cover some of the user space
> > functions prologues, like:
> >
> >   pushq %rbp
> >   movq  %rsp,%rbp
> >   subq  $0xb0,%rsp
> 
> ...
> 
> > There's an additional issue that single instruction replacement does
> > not have and it's the possibility of the user space code to jump in the
> > middle of those 5 bytes. I think it's unlikely to happen at the function
> > prologue, but uprobe could be placed anywhere. I'm not sure how to
> > mitigate this other than having some enable/disable switch or config
> > option, which is unfortunate.
> 
> plus this breaks single-stepping... Although perhaps we don't really care.

Yeah, and I think we can stop optimization if post_handler is set.

Thanks,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox