[PATCH 0/3] unwind_user: Cleanups

[PATCH 0/3] unwind_user: Cleanups

[Jens Remus]

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.

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.

Regards,
Jens

Jens Remus (3):
  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

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

-- 
2.51.0

[PATCH 1/3] unwind_user: Enhance comments on get CFA, FP, and RA

[Jens Remus]

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

[PATCH 2/3] unwind_user/fp: Use dummies instead of ifdef

[Jens Remus]

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.

Note that enabling the config option HAVE_UNWIND_USER_FP without
defining ARCH_INIT_USER_FP_FRAME, ARCH_INIT_USER_FP_ENTRY_FRAME, and
unwind_user_at_function_start() will result in a compile error, which
is helpful when implementing support for unwind user fp in an
architecture.

Signed-off-by: Jens Remus <jremus@linux.ibm.com>
---
 include/linux/unwind_user.h | 14 +++++++++++---
 kernel/unwind/user.c        |  4 ----
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h
index 7f7282516bf5..c3ff690a43e2 100644
--- a/include/linux/unwind_user.h
+++ b/include/linux/unwind_user.h
@@ -5,9 +5,17 @@
 #include <linux/unwind_user_types.h>
 #include <asm/unwind_user.h>
 
-#ifndef ARCH_INIT_USER_FP_FRAME
- #define ARCH_INIT_USER_FP_FRAME
-#endif
+#ifndef CONFIG_HAVE_UNWIND_USER_FP
+
+#define ARCH_INIT_USER_FP_FRAME
+#define ARCH_INIT_USER_FP_ENTRY_FRAME
+
+static inline bool unwind_user_at_function_start(struct pt_regs *regs)
+{
+	return false;
+}
+
+#endif /* !CONFIG_HAVE_UNWIND_USER_FP */
 
 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

[PATCH 3/3] x86/unwind_user: Guard unwind_user_word_size() by UNWIND_USER

[Jens Remus]

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>
---
 arch/x86/include/asm/unwind_user.h | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/arch/x86/include/asm/unwind_user.h b/arch/x86/include/asm/unwind_user.h
index 12064284bc4e..77d776c7fca9 100644
--- a/arch/x86/include/asm/unwind_user.h
+++ b/arch/x86/include/asm/unwind_user.h
@@ -2,6 +2,22 @@
 #ifndef _ASM_X86_UNWIND_USER_H
 #define _ASM_X86_UNWIND_USER_H
 
+#ifdef CONFIG_UNWIND_USER
+
+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);
+}
+
+#endif /* CONFIG_UNWIND_USER */
+
 #ifdef CONFIG_HAVE_UNWIND_USER_FP
 
 #include <asm/ptrace.h>
@@ -19,18 +35,6 @@
 	.fp_off		= 0,				\
 	.use_fp		= false,
 
-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);
-}
-
 static inline bool unwind_user_at_function_start(struct pt_regs *regs)
 {
 	return is_uprobe_at_func_entry(regs);
-- 
2.51.0

Re: [PATCH 2/3] unwind_user/fp: Use dummies instead of ifdef

[Jens Remus]

On 11/25/2025 5:43 PM, Jens Remus wrote:
> 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.
> 
> Note that enabling the config option HAVE_UNWIND_USER_FP without
> defining ARCH_INIT_USER_FP_FRAME, ARCH_INIT_USER_FP_ENTRY_FRAME, and
> unwind_user_at_function_start() will result in a compile error, which
> is helpful when implementing support for unwind user fp in an
> architecture.
> 
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>

> diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h

> @@ -5,9 +5,17 @@
>  #include <linux/unwind_user_types.h>
>  #include <asm/unwind_user.h>
>  
> -#ifndef ARCH_INIT_USER_FP_FRAME
> - #define ARCH_INIT_USER_FP_FRAME
> -#endif
> +#ifndef CONFIG_HAVE_UNWIND_USER_FP
> +
> +#define ARCH_INIT_USER_FP_FRAME
> +#define ARCH_INIT_USER_FP_ENTRY_FRAME

Will fix this as follows in the next version:

#define ARCH_INIT_USER_FP_FRAME(ws)
#define ARCH_INIT_USER_FP_ENTRY_FRAME(ws)

> +
> +static inline bool unwind_user_at_function_start(struct pt_regs *regs)
> +{
> +	return false;
> +}

Would it be better to provide a generic dummy implementation (see below)
or should each arch implement that if it cannot tell whether the topmost
frame is at function start? If so, would it move from linux/unwind_user.h
to asm-generic/unwind_user.h?  Either way it would need to be outside of
the !CONFIG_HAVE_UNWIND_USER_FP guard.

#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

If doing so ARCH_INIT_USER_FP_ENTRY_FRAME should be handled similar, so
that archs do not need to provide their own dummy either:

#ifndef ARCH_INIT_USER_FP_ENTRY_FRAME
#define ARCH_INIT_USER_FP_ENTRY_FRAME(ws)
#endif

In that case only ARCH_INIT_USER_FP_FRAME would remain guarded by
!CONFIG_HAVE_UNWIND_USER_FP, so that compile would fail, if enabling
CONFIG_HAVE_UNWIND_USER_FP without providing ARCH_INIT_USER_FP_FRAME:

#ifndef CONFIG_HAVE_UNWIND_USER_FP

#define ARCH_INIT_USER_FP_FRAME(ws)

#endif /* !CONFIG_HAVE_UNWIND_USER_FP */

> +
> +#endif /* !CONFIG_HAVE_UNWIND_USER_FP */
>  
>  int unwind_user(struct unwind_stacktrace *trace, unsigned int max_entries);
>  
Thanks and 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/

Re: [PATCH 2/3] unwind_user/fp: Use dummies instead of ifdef

[Peter Zijlstra]

On Thu, Nov 27, 2025 at 05:51:47PM +0100, Jens Remus wrote:
> On 11/25/2025 5:43 PM, Jens Remus wrote:
> > 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.
> > 
> > Note that enabling the config option HAVE_UNWIND_USER_FP without
> > defining ARCH_INIT_USER_FP_FRAME, ARCH_INIT_USER_FP_ENTRY_FRAME, and
> > unwind_user_at_function_start() will result in a compile error, which
> > is helpful when implementing support for unwind user fp in an
> > architecture.
> > 
> > Signed-off-by: Jens Remus <jremus@linux.ibm.com>
> 
> > diff --git a/include/linux/unwind_user.h b/include/linux/unwind_user.h
> 
> > @@ -5,9 +5,17 @@
> >  #include <linux/unwind_user_types.h>
> >  #include <asm/unwind_user.h>
> >  
> > -#ifndef ARCH_INIT_USER_FP_FRAME
> > - #define ARCH_INIT_USER_FP_FRAME
> > -#endif
> > +#ifndef CONFIG_HAVE_UNWIND_USER_FP
> > +
> > +#define ARCH_INIT_USER_FP_FRAME
> > +#define ARCH_INIT_USER_FP_ENTRY_FRAME
> 
> Will fix this as follows in the next version:
> 
> #define ARCH_INIT_USER_FP_FRAME(ws)
> #define ARCH_INIT_USER_FP_ENTRY_FRAME(ws)
> 
> > +
> > +static inline bool unwind_user_at_function_start(struct pt_regs *regs)
> > +{
> > +	return false;
> > +}
> 
> Would it be better to provide a generic dummy implementation (see below)
> or should each arch implement that if it cannot tell whether the topmost
> frame is at function start? If so, would it move from linux/unwind_user.h
> to asm-generic/unwind_user.h?  Either way it would need to be outside of
> the !CONFIG_HAVE_UNWIND_USER_FP guard.

I suppose a common fallback would be fine.