diff mbox series

[v6,3/5] samples: ftrace: Save required argument registers in sample trampolines

Message ID 20230405180250.2046566-4-revest@chromium.org (mailing list archive)
State Not Applicable
Headers show
Series Add ftrace direct call for arm64 | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch
bpf/vmtest-bpf-PR success PR summary
bpf/vmtest-bpf-VM_Test-1 success Logs for ${{ matrix.test }} on ${{ matrix.arch }} with ${{ matrix.toolchain_full }}
bpf/vmtest-bpf-VM_Test-2 success Logs for ShellCheck
bpf/vmtest-bpf-VM_Test-3 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-VM_Test-4 success Logs for build for aarch64 with llvm-16
bpf/vmtest-bpf-VM_Test-5 fail Logs for build for s390x with gcc
bpf/vmtest-bpf-VM_Test-6 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-VM_Test-7 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-VM_Test-8 success Logs for set-matrix

Commit Message

Florent Revest April 5, 2023, 6:02 p.m. UTC
The ftrace-direct-too sample traces the handle_mm_fault function whose
signature changed since the introduction of the sample. Since:
commit bce617edecad ("mm: do page fault accounting in handle_mm_fault")
handle_mm_fault now has 4 arguments. Therefore, the sample trampoline
should save 4 argument registers.

s390 saves all argument registers already so it does not need a change
but x86_64 needs an extra push and pop.

This also evolves the signature of the tracing function to make it
mirror the signature of the traced function.

Signed-off-by: Florent Revest <revest@chromium.org>
---
 samples/ftrace/ftrace-direct-too.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

Comments

Steven Rostedt April 5, 2023, 8:40 p.m. UTC | #1
On Wed,  5 Apr 2023 20:02:48 +0200
Florent Revest <revest@chromium.org> wrote:

> The ftrace-direct-too sample traces the handle_mm_fault function whose
> signature changed since the introduction of the sample. Since:
> commit bce617edecad ("mm: do page fault accounting in handle_mm_fault")
> handle_mm_fault now has 4 arguments. Therefore, the sample trampoline
> should save 4 argument registers.
> 
> s390 saves all argument registers already so it does not need a change
> but x86_64 needs an extra push and pop.
> 
> This also evolves the signature of the tracing function to make it
> mirror the signature of the traced function.
>

Should probably add:

Cc: stable@vger.kernel.org
Fixes: bce617edecad ("mm: do page fault accounting in handle_mm_fault")

Reviewed-by: Steven Rostedt (Google) <rostedt@goodmis.org>

-- Steve
Mark Rutland April 6, 2023, 10:22 a.m. UTC | #2
On Wed, Apr 05, 2023 at 08:02:48PM +0200, Florent Revest wrote:
> The ftrace-direct-too sample traces the handle_mm_fault function whose
> signature changed since the introduction of the sample. Since:
> commit bce617edecad ("mm: do page fault accounting in handle_mm_fault")
> handle_mm_fault now has 4 arguments. Therefore, the sample trampoline
> should save 4 argument registers.
> 
> s390 saves all argument registers already so it does not need a change
> but x86_64 needs an extra push and pop.
> 
> This also evolves the signature of the tracing function to make it
> mirror the signature of the traced function.
> 
> Signed-off-by: Florent Revest <revest@chromium.org>

Reviewed-by: Mark Rutland <mark.rutland@arm.com>

Thanks for this!

Mark.

> ---
>  samples/ftrace/ftrace-direct-too.c | 14 ++++++++------
>  1 file changed, 8 insertions(+), 6 deletions(-)
> 
> diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c
> index f28e7b99840f..71ed4ee8cb4a 100644
> --- a/samples/ftrace/ftrace-direct-too.c
> +++ b/samples/ftrace/ftrace-direct-too.c
> @@ -5,14 +5,14 @@
>  #include <linux/ftrace.h>
>  #include <asm/asm-offsets.h>
>  
> -extern void my_direct_func(struct vm_area_struct *vma,
> -			   unsigned long address, unsigned int flags);
> +extern void my_direct_func(struct vm_area_struct *vma, unsigned long address,
> +			   unsigned int flags, struct pt_regs *regs);
>  
> -void my_direct_func(struct vm_area_struct *vma,
> -			unsigned long address, unsigned int flags)
> +void my_direct_func(struct vm_area_struct *vma, unsigned long address,
> +		    unsigned int flags, struct pt_regs *regs)
>  {
> -	trace_printk("handle mm fault vma=%p address=%lx flags=%x\n",
> -		     vma, address, flags);
> +	trace_printk("handle mm fault vma=%p address=%lx flags=%x regs=%p\n",
> +		     vma, address, flags, regs);
>  }
>  
>  extern void my_tramp(void *);
> @@ -34,7 +34,9 @@ asm (
>  "	pushq %rdi\n"
>  "	pushq %rsi\n"
>  "	pushq %rdx\n"
> +"	pushq %rcx\n"
>  "	call my_direct_func\n"
> +"	popq %rcx\n"
>  "	popq %rdx\n"
>  "	popq %rsi\n"
>  "	popq %rdi\n"
> -- 
> 2.40.0.577.gac1e443424-goog
>
diff mbox series

Patch

diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c
index f28e7b99840f..71ed4ee8cb4a 100644
--- a/samples/ftrace/ftrace-direct-too.c
+++ b/samples/ftrace/ftrace-direct-too.c
@@ -5,14 +5,14 @@ 
 #include <linux/ftrace.h>
 #include <asm/asm-offsets.h>
 
-extern void my_direct_func(struct vm_area_struct *vma,
-			   unsigned long address, unsigned int flags);
+extern void my_direct_func(struct vm_area_struct *vma, unsigned long address,
+			   unsigned int flags, struct pt_regs *regs);
 
-void my_direct_func(struct vm_area_struct *vma,
-			unsigned long address, unsigned int flags)
+void my_direct_func(struct vm_area_struct *vma, unsigned long address,
+		    unsigned int flags, struct pt_regs *regs)
 {
-	trace_printk("handle mm fault vma=%p address=%lx flags=%x\n",
-		     vma, address, flags);
+	trace_printk("handle mm fault vma=%p address=%lx flags=%x regs=%p\n",
+		     vma, address, flags, regs);
 }
 
 extern void my_tramp(void *);
@@ -34,7 +34,9 @@  asm (
 "	pushq %rdi\n"
 "	pushq %rsi\n"
 "	pushq %rdx\n"
+"	pushq %rcx\n"
 "	call my_direct_func\n"
+"	popq %rcx\n"
 "	popq %rdx\n"
 "	popq %rsi\n"
 "	popq %rdi\n"