linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jiri Olsa <jolsa@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@kernel.org>,
	Alexei Starovoitov <alexei.starovoitov@gmail.com>
Subject: [RFC][PATCH 1/2] ftrace/x86: Allow for arguments to be passed in to REGS by default
Date: Wed, 28 Oct 2020 09:15:43 -0400	[thread overview]
Message-ID: <20201028131909.738751907@goodmis.org> (raw)
In-Reply-To: 20201028131542.963014814@goodmis.org

From: "Steven Rostedt (VMware)" <rostedt@goodmis.org>

Currently, the only way to get access to the registers of a function via a
ftrace callback is to set the "FL_FLAGS" bit in the ftrace_ops. But as this
saves all regs as if a breakpoint were to trigger (for use with kprobes), it
is expensive.

The regs are already saved on the stack for the default ftrace callbacks, as
that is required otherwise a function being traced will get the wrong
arguments and possibly crash. And on x86, the argumentes are already stored
where they would be on a pt_regs structure to use that code for both the
regs version of a callback, it makes sense to pass that information always
to all functions.

If an architecture does this (as x86 now does), it is to set
HAVE_DYNAMIC_FTRACE_WITH_ARGS, and this will let the generic code that it
could use the regs without having to set the flags if it wants to access the
arguments from the regs pointer.

The stack pointer is also saved in the regs, and this could be useful for
having the function graph tracer not require its own trampoline!

Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 arch/x86/Kconfig            |  1 +
 arch/x86/kernel/ftrace_64.S | 12 +++++++-----
 kernel/trace/Kconfig        |  9 +++++++++
 3 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 7101ac64bb20..b4d2b1fcfd09 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -167,6 +167,7 @@ config X86
 	select HAVE_DMA_CONTIGUOUS
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS
+	select HAVE_DYNAMIC_FTRACE_WITH_ARGS	if X86_64
 	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 	select HAVE_EBPF_JIT
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS
diff --git a/arch/x86/kernel/ftrace_64.S b/arch/x86/kernel/ftrace_64.S
index ac3d5f22fe64..77c512334b4c 100644
--- a/arch/x86/kernel/ftrace_64.S
+++ b/arch/x86/kernel/ftrace_64.S
@@ -86,6 +86,11 @@
 	movq %r8, R8(%rsp)
 	movq %r9, R9(%rsp)
 	movq $0, ORIG_RAX(%rsp)
+
+	/* Stack - skipping return address and flags */
+	leaq MCOUNT_REG_SIZE+\added+8(%rsp), %rcx
+	movq %rcx, RSP(%rsp)
+
 	/*
 	 * Save the original RBP. Even though the mcount ABI does not
 	 * require this, it helps out callers.
@@ -144,8 +149,8 @@ SYM_INNER_LABEL(ftrace_caller_op_ptr, SYM_L_GLOBAL)
 	/* Load the ftrace_ops into the 3rd parameter */
 	movq function_trace_op(%rip), %rdx
 
-	/* regs go into 4th parameter (but make it NULL) */
-	movq $0, %rcx
+	/* regs go into 4th parameter */
+	leaq (%rsp), %rcx
 
 SYM_INNER_LABEL(ftrace_call, SYM_L_GLOBAL)
 	call ftrace_stub
@@ -204,9 +209,6 @@ SYM_INNER_LABEL(ftrace_regs_caller_op_ptr, SYM_L_GLOBAL)
 	movq %rcx, SS(%rsp)
 	movq $__KERNEL_CS, %rcx
 	movq %rcx, CS(%rsp)
-	/* Stack - skipping return address and flags */
-	leaq MCOUNT_REG_SIZE+8*2(%rsp), %rcx
-	movq %rcx, RSP(%rsp)
 
 	ENCODE_FRAME_POINTER
 
diff --git a/kernel/trace/Kconfig b/kernel/trace/Kconfig
index a4020c0b4508..6a5b7a818d7d 100644
--- a/kernel/trace/Kconfig
+++ b/kernel/trace/Kconfig
@@ -31,6 +31,15 @@ config HAVE_DYNAMIC_FTRACE_WITH_REGS
 config HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 	bool
 
+config HAVE_DYNAMIC_FTRACE_WITH_ARGS
+	bool
+	help
+	 If this is set, then arguments and stack can be found from
+	 the pt_regs passed into the function callback regs parameter
+	 by default, even without setting the REGS flag in the ftrace_ops.
+	 This allows for use of regs_get_kernel_argument() and
+	 kernel_stack_pointer().
+
 config HAVE_FTRACE_MCOUNT_RECORD
 	bool
 	help
-- 
2.28.0



  reply	other threads:[~2020-10-29  0:59 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-28 13:15 [RFC][PATCH 0/2] ftrace: Add access to function arguments for all callbacks Steven Rostedt
2020-10-28 13:15 ` Steven Rostedt [this message]
2020-10-28 14:25   ` [RFC][PATCH 1/2 v2] ftrace/x86: Allow for arguments to be passed in to REGS by default Steven Rostedt
2020-10-28 15:29     ` Steven Rostedt
2020-10-28 20:36       ` Steven Rostedt
2020-10-28 21:14         ` Steven Rostedt
2020-10-29  8:07       ` Masami Hiramatsu
2020-10-28 13:15 ` [RFC][PATCH 2/2] ftrace: Test arguments by adding trace_printk in function tracer Steven Rostedt
2020-10-28 21:33 ` [RFC][PATCH 0/2] ftrace: Add access to function arguments for all callbacks Alexei Starovoitov
2020-10-28 22:07   ` Steven Rostedt
2020-10-28 23:03     ` Alexei Starovoitov
2020-10-28 23: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=20201028131909.738751907@goodmis.org \
    --to=rostedt@goodmis.org \
    --cc=akpm@linux-foundation.org \
    --cc=alexei.starovoitov@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=tglx@linutronix.de \
    /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;
as well as URLs for NNTP newsgroup(s).