mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: Steven Rostedt <rostedt@kernel.org>
Cc: linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	x86@kernel.org, Masami Hiramatsu <mhiramat@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Josh Poimboeuf <jpoimboe@kernel.org>
Subject: Re: [PATCH v2 2/2] unwind deferred/x86: Do not defer stack tracing for compat tasks
Date: Tue, 23 Sep 2025 12:45:15 +0200	[thread overview]
Message-ID: <20250923104515.GF3419281@noisy.programming.kicks-ass.net> (raw)
In-Reply-To: <20250827193828.514833400@kernel.org>

On Wed, Aug 27, 2025 at 03:36:46PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
> 
> Currently compat tasks are not supported. If a deferred user space stack
> trace is requested on a compat task, it should fail and return an error so
> that the profiler can use an alternative approach (whatever it uses
> today).
> 
> Add a arch_unwind_can_defer() macro that is called in
> unwind_deferred_request(). Have x86 define it to a function that makes
> sure that the current task is running in 64bit mode, and if it is not, it
> returns false. This will cause unwind_deferred_request() to error out and
> the caller can use the current method of user space stack tracing.

Changelog seems to forget mentioning *why* we can't unwind compat.

I'm sure I've seen compat FP unwind support at some point in this
series. Did that go missing somewhere?

Also, these two patches are in the wrong order, first you enable things,
including compat tasks, and then you go 'whoopsie, no compats'.

> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> Changes since v1: https://lore.kernel.org/20250820190639.843442474@kernel.org
> 
> - Added #include <linux/compat.h> to use user_64bit_mode() in
>   unwind_user.h. When adding this to the ftrace code, it failed to build
>   due to the missing header.
> 
>  arch/x86/include/asm/unwind_user.h | 12 ++++++++++++
>  include/linux/unwind_deferred.h    |  5 +++++
>  kernel/unwind/deferred.c           |  3 +++
>  3 files changed, 20 insertions(+)
> 
> diff --git a/arch/x86/include/asm/unwind_user.h b/arch/x86/include/asm/unwind_user.h
> index 8597857bf896..0cef2e3b08c8 100644
> --- a/arch/x86/include/asm/unwind_user.h
> +++ b/arch/x86/include/asm/unwind_user.h
> @@ -2,6 +2,18 @@
>  #ifndef _ASM_X86_UNWIND_USER_H
>  #define _ASM_X86_UNWIND_USER_H
>  
> +#ifdef CONFIG_IA32_EMULATION
> +#include <linux/compat.h>
> +/* Currently compat mode is not supported for deferred stack trace */
> +static inline bool arch_unwind_can_defer(void)
> +{
> +	struct pt_regs *regs = task_pt_regs(current);
> +
> +	return user_64bit_mode(regs);
> +}
> +# define arch_unwind_can_defer	arch_unwind_can_defer
> +#endif /* CONFIG_IA32_EMULATION */
> +
>  #define ARCH_INIT_USER_FP_FRAME							\
>  	.cfa_off	= (s32)sizeof(long) *  2,				\
>  	.ra_off		= (s32)sizeof(long) * -1,				\
> diff --git a/include/linux/unwind_deferred.h b/include/linux/unwind_deferred.h
> index 26122d00708a..0124865aaab4 100644
> --- a/include/linux/unwind_deferred.h
> +++ b/include/linux/unwind_deferred.h
> @@ -16,6 +16,11 @@ struct unwind_work {
>  	int				bit;
>  };
>  
> +/* Architectures can add a test to not defer unwinding */
> +#ifndef arch_unwind_can_defer
> +# define arch_unwind_can_defer()	(true)
> +#endif
> +
>  #ifdef CONFIG_UNWIND_USER
>  
>  enum {
> diff --git a/kernel/unwind/deferred.c b/kernel/unwind/deferred.c
> index dc6040aae3ee..3601b2efe48d 100644
> --- a/kernel/unwind/deferred.c
> +++ b/kernel/unwind/deferred.c
> @@ -237,6 +237,9 @@ int unwind_deferred_request(struct unwind_work *work, u64 *cookie)
>  
>  	*cookie = 0;
>  
> +	if (!arch_unwind_can_defer())
> +		return -EINVAL;
> +
>  	if ((current->flags & (PF_KTHREAD | PF_EXITING)) ||
>  	    !user_mode(task_pt_regs(current)))
>  		return -EINVAL;

Should not this arch hook be after all the 'normal' early exits?

  reply	other threads:[~2025-09-23 10:45 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-08-27 19:36 [PATCH v2 0/2] x86: Enable the deferred unwinding infrastructure Steven Rostedt
2025-08-27 19:36 ` [PATCH v2 1/2] unwind_user/x86: Enable frame pointer unwinding on x86 Steven Rostedt
2025-09-23 10:51   ` Peter Zijlstra
2025-09-23 12:20     ` Steven Rostedt
2025-10-29  9:36   ` [tip: perf/core] " tip-bot2 for Josh Poimboeuf
2025-08-27 19:36 ` [PATCH v2 2/2] unwind deferred/x86: Do not defer stack tracing for compat tasks Steven Rostedt
2025-09-23 10:45   ` Peter Zijlstra [this message]
2025-09-23 11:42     ` Peter Zijlstra
2025-09-23 12:16     ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250923104515.GF3419281@noisy.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=jpoimboe@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mhiramat@kernel.org \
    --cc=mingo@redhat.com \
    --cc=rostedt@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®