public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	Florent Revest <revest@chromium.org>,
	linux-trace-kernel@vger.kernel.org,
	LKML <linux-kernel@vger.kernel.org>,
	Martin KaFai Lau <martin.lau@linux.dev>,
	bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
	Jiri Olsa <jolsa@kernel.org>,
	Alan Maguire <alan.maguire@oracle.com>,
	Mark Rutland <mark.rutland@arm.com>,
	linux-arch@vger.kernel.org, Heiko Carstens <hca@linux.ibm.com>,
	Will Deacon <will@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Huacai Chen <chenhuacai@kernel.org>,
	WANG Xuerui <kernel@xen0n.name>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Alexander Gordeev <agordeev@linux.ibm.com>,
	Christian Borntraeger <borntraeger@linux.ibm.com>,
	Sven Schnelle <svens@linux.ibm.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	x86@kernel.org, "H. Peter Anvin" <hpa@zytor.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Subject: Re: [PATCH v21 03/20] fgraph: Replace fgraph_ret_regs with ftrace_regs
Date: Wed, 25 Dec 2024 22:33:00 +0900	[thread overview]
Message-ID: <20241225223300.68299ea8a2836c6947fe9d1c@kernel.org> (raw)
In-Reply-To: <20241223163956.44245b4b@gandalf.local.home>

Hi,

On Mon, 23 Dec 2024 16:39:56 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> On Tue, 10 Dec 2024 11:09:26 +0900
> "Masami Hiramatsu (Google)" <mhiramat@kernel.org> wrote:
> 
> > From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > 
> > Use ftrace_regs instead of fgraph_ret_regs for tracing return value
> > on function_graph tracer because of simplifying the callback interface.
> > 
> > The CONFIG_HAVE_FUNCTION_GRAPH_RETVAL is also replaced by
> > CONFIG_HAVE_FUNCTION_GRAPH_FREGS.
> > 
> > Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> > Acked-by: Heiko Carstens <hca@linux.ibm.com>
> 
> My x86-32 test failed on this patch with:
> 
> [    8.387985] Testing tracer preemptoff: PASSED
> [    9.603053] Testing tracer preemptirqsoff: PASSED
> [   10.820200] Testing tracer wakeup: PASSED
> [   12.030489] Testing tracer wakeup_rt: PASSED
> [   13.237925] Testing tracer wakeup_dl: PASSED
> [   14.440146] Testing tracer function_graph:
> [   14.611021] ------------[ cut here ]------------
> [   14.614997] WARNING: CPU: 0 PID: 1 at kernel/sched/deadline.c:1519 update_curr_dl_se+0x205/0x270
> [   14.614997] Modules linked in:
> [   14.614997] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 <89>E<EC><E8>^Q~A<FF>d<8B>5<80><96>wɅ<C0><B8>`<FC>9<C9>^OE<F8><8B><86><94>^D 6.13.0-rc4-test-00003-g50b6e5b87e20-dirty #811
> [   14.614997] PSh<E5>^L5<C9><E8><99><FC>^Y<FF><83><C4>^L<EB><C3>.<8D><B4>&
> [   14.614997] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [   14.614997] EIP: update_curr_dl_se+0x205/0x270
> [   14.614997] Code: 51 10 89 43 34 0f b7 43 48 89 53 38 66 25 40 01 66 83 f8 40 75 04 80 4b 48 81 89 d8 e8 14 5b ff ff 83 f8 01 0f 84 d4 fe ff ff <0f> 0b e9 cd fe ff ff 8d 74 26 00 ba 20 00 00 00 89 d8 e8 c4 ab ff
> [   14.614997] EAX: c81eed9c EBX: f6f896e0 ECX: c81eed9c EDX: 00000001
> [   14.614997] ESI: f6f89100 EDI: fffffffe EBP: c11cdaac ESP: c11cda94
> [   14.614997] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010082
> [   14.614997] CR0: 80050033 CR2: ff9ff000 CR3: 0978c000 CR4: 000006f0
> [   14.614997] Call Trace:
> [   14.614997] irq event stamp: 11132292
> [   14.614997] hardirqs last  enabled at (11132291): [<c82fac0c>] trace_graph_entry+0x23c/0x3d0
> [   14.614997] hardirqs last disabled at (11132292): [<c8e9f0bc>] sysvec_apic_timer_interrupt+0xc/0x40
> [   14.614997] softirqs last  enabled at (11059360): [<c81678ac>] return_to_handler+0x0/0x34
> [   14.614997] softirqs last disabled at (11059355): [<c81678ac>] return_to_handler+0x0/0x34
> [   14.614997] ---[ end trace 0000000000000000 ]---
> [    7.652636] ------------[ cut here ]------------
> [    7.652636] DEBUG_LOCKS_WARN_ON(1)
> [    7.652636] WARNING: CPU: 0 PID: 1 at kernel/locking/lockdep.c:232 __lock_acquire+0xf42/0x25c0
> [    7.652636] Modules linked in:
> [    7.652636] CPU: 0 UID: 0 PID: 1 Comm: swapper/0 <89>E<EC><E8>^Q~A<FF>d<8B>5<80><96>wɅ<C0><B8>`<FC>9<C9>^OE<F8><8B><86><94>^D 6.13.0-rc4-test-00003-g50b6e5b87e20-dirty #811
> [    7.652636] PSh<E5>^L5<C9><E8><99><FC>^Y<FF><83><C4>^L<EB><C3>.<8D><B4>&
> [    7.652636] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> [    7.652636] EIP: __lock_acquire+0xf42/0x25c0
> [    7.652636] Code: e8 63 ba 57 00 85 c0 0f 84 19 f9 ff ff 8b 0d a8 59 62 c9 85 c9 0f 85 0b f9 ff ff 68 c4 8c 34 c9 68 0b 23 32 c9 e8 be ce f7 ff <0f> 0b 58 31 c0 5a e9 61 f2 ff ff 8d 76 00 b9 05 00 00 00 64 a1 80
> [    7.652636] EAX: c820bf72 EBX: 00000000 ECX: c820bf72 EDX: 00000001
> [    7.652636] ESI: 00000000 EDI: c11a2e38 EBP: c1143e7c ESP: c1143de4
> [    7.652636] DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068 EFLAGS: 00010086
> [    7.652636] CR0: 80050033 CR2: ff9ff000 CR3: 0978c000 CR4: 000006f0
> [    7.652636] Call Trace:
> [    7.652636] WARNING: stack recursion on stack type 3
> [    7.652636] irq event stamp: 11215629
> [    7.652636] hardirqs last  enabled at (11215628): [<c81917c9>] handle_softirqs+0x99/0x3b0
> [    7.652636] hardirqs last disabled at (11215629): [<c8eaf5cd>] _raw_spin_lock_irq+0x4d/0x50
> [    7.652636] softirqs last  enabled at (11059360): [<c81678ac>] return_to_handler+0x0/0x34
> [    7.652636] softirqs last disabled at (11215627): [<c81678ac>] return_to_handler+0x0/0x34
> [    7.652636] ---[ end trace 0000000000000000 ]---
> [    7.652636] ------------[ cut here ]------------
> [    7.652636] kernel BUG at arch/x86/mm/extable.c:373!
> [    7.652636] ------------[ cut here ]------------
> [    7.652636] kernel BUG at arch/x86/mm/extable.c:373!
> [    7.652636] ------------[ cut here ]------------
> [    7.652636] kernel BUG at arch/x86/mm/extable.c:373!
> [    7.652636] ------------[ cut here ]------------
> [    7.652636] kernel BUG at arch/x86/mm/extable.c:373!
> [    7.652636] ------------[ cut here ]------------
> [    7.652636] kernel BUG at arch/x86/mm/extable.c:373!
> [    7.652636] ------------[ cut here ]------------
> [    7.652636] kernel BUG at arch/x86/mm/extable.c:373!
> [    7.652636] ------------[ cut here ]------------
> [...]
> 
> Config attached.

Thanks for reporting!

diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 58d9ed50fe61..8e1a27d2c1eb 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -187,14 +187,15 @@ SYM_CODE_END(ftrace_graph_caller)
 
 .globl return_to_handler
 return_to_handler:
-	pushl	$0
-	pushl	%edx
-	pushl	%eax
+	subl	$(PTREGS_SIZE), %esp
+	movl	$0, PT_EBP(%esp)
+	movl	%edx, PT_EDX(%esp)
+	movl	%eax, PT_EAX(%esp)
 	movl	%esp, %eax
 	call	ftrace_return_to_handler
 	movl	%eax, %ecx
-	popl	%eax
-	popl	%edx
-	addl	$4, %esp		# skip ebp
+	movl	%eax, PT_EAX(%esp)
+	movl	%edx, PT_EDX(%esp)

Aah, my bad! These should recover registers from stack...

	movl	PT_EAX(%esp), %eax
	movl	PT_EDX(%esp), %edx


+	addl	$(PTREGS_SIZE), %esp
 	JMP_NOSPEC ecx
 #endif

Can you try below change?

Thank you!


diff --git a/arch/x86/kernel/ftrace_32.S b/arch/x86/kernel/ftrace_32.S
index 8e1a27d2c1eb..f4e0c3361234 100644
--- a/arch/x86/kernel/ftrace_32.S
+++ b/arch/x86/kernel/ftrace_32.S
@@ -194,8 +194,8 @@ return_to_handler:
 	movl	%esp, %eax
 	call	ftrace_return_to_handler
 	movl	%eax, %ecx
-	movl	%eax, PT_EAX(%esp)
-	movl	%edx, PT_EDX(%esp)
+	movl	PT_EAX(%esp), %eax
+	movl	PT_EDX(%esp), %edx
 	addl	$(PTREGS_SIZE), %esp
 	JMP_NOSPEC ecx
 #endif


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

  reply	other threads:[~2024-12-25 13:33 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-10  2:08 [PATCH v21 00/20] tracing: fprobe: function_graph: Multi-function graph and fprobe on fgraph Masami Hiramatsu (Google)
2024-12-10  2:08 ` [PATCH v21 01/20] fgraph: Get ftrace recursion lock in function_graph_enter Masami Hiramatsu (Google)
2024-12-10  2:09 ` [PATCH v21 02/20] fgraph: Pass ftrace_regs to entryfunc Masami Hiramatsu (Google)
2024-12-10  2:09 ` [PATCH v21 03/20] fgraph: Replace fgraph_ret_regs with ftrace_regs Masami Hiramatsu (Google)
2024-12-19 21:34   ` Steven Rostedt
2024-12-20 14:51     ` Heiko Carstens
2024-12-20 15:08       ` Steven Rostedt
2024-12-23 21:39   ` Steven Rostedt
2024-12-25 13:33     ` Masami Hiramatsu [this message]
2024-12-26 15:42       ` Steven Rostedt
2024-12-10  2:09 ` [PATCH v21 04/20] fgraph: Pass ftrace_regs to retfunc Masami Hiramatsu (Google)
2024-12-10  2:09 ` [PATCH v21 05/20] fprobe: Use ftrace_regs in fprobe entry handler Masami Hiramatsu (Google)
2024-12-23 21:42   ` Steven Rostedt
2024-12-26  2:08     ` Masami Hiramatsu
2024-12-10  2:10 ` [PATCH v21 06/20] fprobe: Use ftrace_regs in fprobe exit handler Masami Hiramatsu (Google)
2024-12-10  2:10 ` [PATCH v21 07/20] tracing: Add ftrace_partial_regs() for converting ftrace_regs to pt_regs Masami Hiramatsu (Google)
2024-12-10  2:10 ` [PATCH v21 08/20] tracing: Add ftrace_fill_perf_regs() for perf event Masami Hiramatsu (Google)
2024-12-10  2:10 ` [PATCH v21 09/20] tracing/fprobe: Enable fprobe events with CONFIG_DYNAMIC_FTRACE_WITH_ARGS Masami Hiramatsu (Google)
2024-12-10  2:10 ` [PATCH v21 10/20] bpf: Enable kprobe_multi feature if CONFIG_FPROBE is enabled Masami Hiramatsu (Google)
2024-12-10  2:11 ` [PATCH v21 11/20] ftrace: Add CONFIG_HAVE_FTRACE_GRAPH_FUNC Masami Hiramatsu (Google)
2024-12-10  2:11 ` [PATCH v21 12/20] s390/tracing: Enable HAVE_FTRACE_GRAPH_FUNC Masami Hiramatsu (Google)
2024-12-10  2:11 ` [PATCH v21 13/20] fprobe: Rewrite fprobe on function-graph tracer Masami Hiramatsu (Google)
2024-12-10  2:11 ` [PATCH v21 14/20] fprobe: Add fprobe_header encoding feature Masami Hiramatsu (Google)
2024-12-10  2:12 ` [PATCH v21 15/20] tracing/fprobe: Remove nr_maxactive from fprobe Masami Hiramatsu (Google)
2024-12-10  2:12 ` [PATCH v21 16/20] selftests: ftrace: Remove obsolate maxactive syntax check Masami Hiramatsu (Google)
2024-12-10  2:12 ` [PATCH v21 17/20] selftests/ftrace: Add a test case for repeating register/unregister fprobe Masami Hiramatsu (Google)
2024-12-10  2:12 ` [PATCH v21 18/20] Documentation: probes: Update fprobe on function-graph tracer Masami Hiramatsu (Google)
2024-12-10  2:12 ` [PATCH v21 19/20] ftrace: Add ftrace_get_symaddr to convert fentry_ip to symaddr Masami Hiramatsu (Google)
2024-12-10  2:12 ` [PATCH v21 20/20] bpf: Use ftrace_get_symaddr() for kprobe_multi probes Masami Hiramatsu (Google)

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=20241225223300.68299ea8a2836c6947fe9d1c@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=agordeev@linux.ibm.com \
    --cc=alan.maguire@oracle.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=aou@eecs.berkeley.edu \
    --cc=ast@kernel.org \
    --cc=borntraeger@linux.ibm.com \
    --cc=bp@alien8.de \
    --cc=bpf@vger.kernel.org \
    --cc=catalin.marinas@arm.com \
    --cc=chenhuacai@kernel.org \
    --cc=dave.hansen@linux.intel.com \
    --cc=gor@linux.ibm.com \
    --cc=hca@linux.ibm.com \
    --cc=hpa@zytor.com \
    --cc=jolsa@kernel.org \
    --cc=kernel@xen0n.name \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=martin.lau@linux.dev \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@redhat.com \
    --cc=palmer@dabbelt.com \
    --cc=paul.walmsley@sifive.com \
    --cc=revest@chromium.org \
    --cc=rostedt@goodmis.org \
    --cc=svens@linux.ibm.com \
    --cc=tglx@linutronix.de \
    --cc=will@kernel.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