* [PATCH 0/6] [RFC v3] ftrace/kprobes: Setting up ftrace for kprobes
@ 2012-07-02 20:03 Steven Rostedt
2012-07-02 20:03 ` [PATCH 1/6] ftrace: Pass ftrace_ops as third parameter to function trace callback Steven Rostedt
` (5 more replies)
0 siblings, 6 replies; 18+ messages in thread
From: Steven Rostedt @ 2012-07-02 20:03 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
Masami Hiramatsu, H. Peter Anvin
This is the (hopefully) final round of RFCs, for having kprobes use ftrace
as optimization. This round, I did not include Masami's patches that are
on top of this, simply because I posted them twice before with no changes.
I have them still on top of this series, but I'm only posting my set.
The final pull request will also include Masami's.
If you are interested in Masami's changes, you can find them here:
http://marc.info/?l=linux-kernel&m=133954203218870&w=2
Patches 7 - 13 are mostly Masami's and some are mine. But none were changed
in this release. I may post them again after these are reviewed.
This round, I only did the updates that Masami recommended. I did not
modify the ones that he gave his 'Reviewed-by' tag, except to change
the change log to have that tag.
What was changed were:
Patch 3:
Moved out the x86 changes into patch 5
Added a SAVED_REGS_IF_SUPPORTED flag, that can be set if the callback
can handle the regs parameter being NULL. If the ftrace_ops only has
SAVED_REGS set, it will fail the registering if the arch does not
support saving regs.
Patch 5:
Incorporated x86 changes from the previous patch 3.
Patch 6:
Have i386 restore flags on return of a function trace that saves regs.
Masami,
Can you review those patches (I believe the rest have your Reviewed-by
already). Then I'll add that tag and push for a pull request.
Thanks!
-- Steve
Steven Rostedt (6):
ftrace: Pass ftrace_ops as third parameter to function trace callback
ftrace: Consolidate arch dependent functions with 'list' function
ftrace: Return pt_regs to function trace callback
ftrace/x86_32: Push ftrace_ops in as 3rd parameter to function tracer
ftrace/x86: Add separate function to save regs
ftrace/x86: Add save_regs for i386 function calls
----
arch/x86/include/asm/ftrace.h | 47 +++++---
arch/x86/kernel/entry_32.S | 56 +++++++++
arch/x86/kernel/entry_64.S | 89 ++++++++++++--
arch/x86/kernel/ftrace.c | 73 +++++++++++-
include/linux/ftrace.h | 138 ++++++++++++++++++++--
kernel/trace/ftrace.c | 236 +++++++++++++++++++++++++------------
kernel/trace/trace_event_perf.c | 3 +-
kernel/trace/trace_events.c | 3 +-
kernel/trace/trace_functions.c | 10 +-
kernel/trace/trace_irqsoff.c | 3 +-
kernel/trace/trace_sched_wakeup.c | 3 +-
kernel/trace/trace_selftest.c | 20 +++-
kernel/trace/trace_stack.c | 3 +-
13 files changed, 557 insertions(+), 127 deletions(-)
^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 1/6] ftrace: Pass ftrace_ops as third parameter to function trace callback
2012-07-02 20:03 [PATCH 0/6] [RFC v3] ftrace/kprobes: Setting up ftrace for kprobes Steven Rostedt
@ 2012-07-02 20:03 ` Steven Rostedt
2012-07-02 20:03 ` [PATCH 2/6] ftrace: Consolidate arch dependent functions with list function Steven Rostedt
` (4 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2012-07-02 20:03 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
Masami Hiramatsu, H. Peter Anvin
[-- Attachment #1: 0001-ftrace-Pass-ftrace_ops-as-third-parameter-to-functio.patch --]
[-- Type: text/plain, Size: 15993 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
Currently the function trace callback receives only the ip and parent_ip
of the function that it traced. It would be more powerful to also return
the ops that registered the function as well. This allows the same function
to act differently depending on what ftrace_ops registered it.
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
arch/x86/include/asm/ftrace.h | 4 ++
arch/x86/kernel/entry_64.S | 1 +
include/linux/ftrace.h | 16 +++++-
kernel/trace/ftrace.c | 101 +++++++++++++++++++++++++------------
kernel/trace/trace_event_perf.c | 3 +-
kernel/trace/trace_events.c | 3 +-
kernel/trace/trace_functions.c | 9 ++--
kernel/trace/trace_irqsoff.c | 3 +-
kernel/trace/trace_sched_wakeup.c | 2 +-
kernel/trace/trace_selftest.c | 15 ++++--
kernel/trace/trace_stack.c | 2 +-
11 files changed, 113 insertions(+), 46 deletions(-)
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index b0767bc..783b107 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -32,6 +32,10 @@
#define MCOUNT_ADDR ((long)(mcount))
#define MCOUNT_INSN_SIZE 5 /* sizeof mcount call */
+#if defined(CONFIG_DYNAMIC_FTRACE) && defined(CONFIG_X86_64)
+#define ARCH_SUPPORTS_FTRACE_OPS 1
+#endif
+
#ifndef __ASSEMBLY__
extern void mcount(void);
extern atomic_t modifying_ftrace_code;
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 7d65133..2b4f94c 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -79,6 +79,7 @@ ENTRY(ftrace_caller)
MCOUNT_SAVE_FRAME
+ leaq function_trace_op, %rdx
movq 0x38(%rsp), %rdi
movq 8(%rbp), %rsi
subq $MCOUNT_INSN_SIZE, %rdi
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 55e6d63..2d59641 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -18,6 +18,15 @@
#include <asm/ftrace.h>
+/*
+ * If the arch supports passing the variable contents of
+ * function_trace_op as the third parameter back from the
+ * mcount call, then the arch should define this as 1.
+ */
+#ifndef ARCH_SUPPORTS_FTRACE_OPS
+#define ARCH_SUPPORTS_FTRACE_OPS 0
+#endif
+
struct module;
struct ftrace_hash;
@@ -29,7 +38,10 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,
void __user *buffer, size_t *lenp,
loff_t *ppos);
-typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip);
+struct ftrace_ops;
+
+typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op);
/*
* FTRACE_OPS_FL_* bits denote the state of ftrace_ops struct and are
@@ -163,7 +175,7 @@ static inline int ftrace_function_local_disabled(struct ftrace_ops *ops)
return *this_cpu_ptr(ops->disabled);
}
-extern void ftrace_stub(unsigned long a0, unsigned long a1);
+extern void ftrace_stub(unsigned long a0, unsigned long a1, struct ftrace_ops *op);
#else /* !CONFIG_FUNCTION_TRACER */
/*
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index b4f20fb..4f2ab93 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -64,12 +64,19 @@
#define FL_GLOBAL_CONTROL_MASK (FTRACE_OPS_FL_GLOBAL | FTRACE_OPS_FL_CONTROL)
+static struct ftrace_ops ftrace_list_end __read_mostly = {
+ .func = ftrace_stub,
+};
+
/* ftrace_enabled is a method to turn ftrace on or off */
int ftrace_enabled __read_mostly;
static int last_ftrace_enabled;
/* Quick disabling of function tracer. */
-int function_trace_stop;
+int function_trace_stop __read_mostly;
+
+/* Current function tracing op */
+struct ftrace_ops *function_trace_op __read_mostly = &ftrace_list_end;
/* List for set_ftrace_pid's pids. */
LIST_HEAD(ftrace_pids);
@@ -86,10 +93,6 @@ static int ftrace_disabled __read_mostly;
static DEFINE_MUTEX(ftrace_lock);
-static struct ftrace_ops ftrace_list_end __read_mostly = {
- .func = ftrace_stub,
-};
-
static struct ftrace_ops *ftrace_global_list __read_mostly = &ftrace_list_end;
static struct ftrace_ops *ftrace_control_list __read_mostly = &ftrace_list_end;
static struct ftrace_ops *ftrace_ops_list __read_mostly = &ftrace_list_end;
@@ -100,8 +103,14 @@ ftrace_func_t ftrace_pid_function __read_mostly = ftrace_stub;
static struct ftrace_ops global_ops;
static struct ftrace_ops control_ops;
-static void
-ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip);
+#if ARCH_SUPPORTS_FTRACE_OPS
+static void ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op);
+#else
+/* See comment below, where ftrace_ops_list_func is defined */
+static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip);
+#define ftrace_ops_list_func ((ftrace_func_t)ftrace_ops_no_ops)
+#endif
/*
* Traverse the ftrace_global_list, invoking all entries. The reason that we
@@ -112,29 +121,29 @@ ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip);
*
* Silly Alpha and silly pointer-speculation compiler optimizations!
*/
-static void ftrace_global_list_func(unsigned long ip,
- unsigned long parent_ip)
+static void
+ftrace_global_list_func(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op)
{
- struct ftrace_ops *op;
-
if (unlikely(trace_recursion_test(TRACE_GLOBAL_BIT)))
return;
trace_recursion_set(TRACE_GLOBAL_BIT);
op = rcu_dereference_raw(ftrace_global_list); /*see above*/
while (op != &ftrace_list_end) {
- op->func(ip, parent_ip);
+ op->func(ip, parent_ip, op);
op = rcu_dereference_raw(op->next); /*see above*/
};
trace_recursion_clear(TRACE_GLOBAL_BIT);
}
-static void ftrace_pid_func(unsigned long ip, unsigned long parent_ip)
+static void ftrace_pid_func(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op)
{
if (!test_tsk_trace_trace(current))
return;
- ftrace_pid_function(ip, parent_ip);
+ ftrace_pid_function(ip, parent_ip, op);
}
static void set_ftrace_pid_function(ftrace_func_t func)
@@ -163,12 +172,13 @@ void clear_ftrace_function(void)
* For those archs that do not test ftrace_trace_stop in their
* mcount call site, we need to do it from C.
*/
-static void ftrace_test_stop_func(unsigned long ip, unsigned long parent_ip)
+static void ftrace_test_stop_func(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op)
{
if (function_trace_stop)
return;
- __ftrace_trace_function(ip, parent_ip);
+ __ftrace_trace_function(ip, parent_ip, op);
}
#endif
@@ -230,15 +240,24 @@ static void update_ftrace_function(void)
/*
* If we are at the end of the list and this ops is
- * not dynamic, then have the mcount trampoline call
- * the function directly
+ * not dynamic and the arch supports passing ops, then have the
+ * mcount trampoline call the function directly.
*/
if (ftrace_ops_list == &ftrace_list_end ||
(ftrace_ops_list->next == &ftrace_list_end &&
- !(ftrace_ops_list->flags & FTRACE_OPS_FL_DYNAMIC)))
+ !(ftrace_ops_list->flags & FTRACE_OPS_FL_DYNAMIC) &&
+ ARCH_SUPPORTS_FTRACE_OPS)) {
+ /* Set the ftrace_ops that the arch callback uses */
+ if (ftrace_ops_list == &global_ops)
+ function_trace_op = ftrace_global_list;
+ else
+ function_trace_op = ftrace_ops_list;
func = ftrace_ops_list->func;
- else
+ } else {
+ /* Just use the default ftrace_ops */
+ function_trace_op = &ftrace_list_end;
func = ftrace_ops_list_func;
+ }
#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
ftrace_trace_function = func;
@@ -773,7 +792,8 @@ ftrace_profile_alloc(struct ftrace_profile_stat *stat, unsigned long ip)
}
static void
-function_profile_call(unsigned long ip, unsigned long parent_ip)
+function_profile_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *ops)
{
struct ftrace_profile_stat *stat;
struct ftrace_profile *rec;
@@ -803,7 +823,7 @@ function_profile_call(unsigned long ip, unsigned long parent_ip)
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
static int profile_graph_entry(struct ftrace_graph_ent *trace)
{
- function_profile_call(trace->func, 0);
+ function_profile_call(trace->func, 0, NULL);
return 1;
}
@@ -2790,8 +2810,8 @@ static int __init ftrace_mod_cmd_init(void)
}
device_initcall(ftrace_mod_cmd_init);
-static void
-function_trace_probe_call(unsigned long ip, unsigned long parent_ip)
+static void function_trace_probe_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op)
{
struct ftrace_func_probe *entry;
struct hlist_head *hhd;
@@ -3942,10 +3962,9 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip)
#endif /* CONFIG_DYNAMIC_FTRACE */
static void
-ftrace_ops_control_func(unsigned long ip, unsigned long parent_ip)
+ftrace_ops_control_func(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op)
{
- struct ftrace_ops *op;
-
if (unlikely(trace_recursion_test(TRACE_CONTROL_BIT)))
return;
@@ -3959,7 +3978,7 @@ ftrace_ops_control_func(unsigned long ip, unsigned long parent_ip)
while (op != &ftrace_list_end) {
if (!ftrace_function_local_disabled(op) &&
ftrace_ops_test(op, ip))
- op->func(ip, parent_ip);
+ op->func(ip, parent_ip, op);
op = rcu_dereference_raw(op->next);
};
@@ -3971,8 +3990,9 @@ static struct ftrace_ops control_ops = {
.func = ftrace_ops_control_func,
};
-static void
-ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip)
+static inline void
+__ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *ignored)
{
struct ftrace_ops *op;
@@ -3988,13 +4008,32 @@ ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip)
op = rcu_dereference_raw(ftrace_ops_list);
while (op != &ftrace_list_end) {
if (ftrace_ops_test(op, ip))
- op->func(ip, parent_ip);
+ op->func(ip, parent_ip, op);
op = rcu_dereference_raw(op->next);
};
preempt_enable_notrace();
trace_recursion_clear(TRACE_INTERNAL_BIT);
}
+/*
+ * Some archs only support passing ip and parent_ip. Even though
+ * the list function ignores the op parameter, we do not want any
+ * C side effects, where a function is called without the caller
+ * sending a third parameter.
+ */
+#if ARCH_SUPPORTS_FTRACE_OPS
+static void ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op)
+{
+ __ftrace_ops_list_func(ip, parent_ip, NULL);
+}
+#else
+static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip)
+{
+ __ftrace_ops_list_func(ip, parent_ip, NULL);
+}
+#endif
+
static void clear_ftrace_swapper(void)
{
struct task_struct *p;
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index fee3752..a872a9a 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -258,7 +258,8 @@ EXPORT_SYMBOL_GPL(perf_trace_buf_prepare);
#ifdef CONFIG_FUNCTION_TRACER
static void
-perf_ftrace_function_call(unsigned long ip, unsigned long parent_ip)
+perf_ftrace_function_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *ops)
{
struct ftrace_entry *entry;
struct hlist_head *head;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 29111da..88daa51 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1681,7 +1681,8 @@ static __init void event_trace_self_tests(void)
static DEFINE_PER_CPU(atomic_t, ftrace_test_event_disable);
static void
-function_test_events_call(unsigned long ip, unsigned long parent_ip)
+function_test_events_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op)
{
struct ring_buffer_event *event;
struct ring_buffer *buffer;
diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index c7b0c6a..fceb7a9 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c
@@ -48,7 +48,8 @@ static void function_trace_start(struct trace_array *tr)
}
static void
-function_trace_call_preempt_only(unsigned long ip, unsigned long parent_ip)
+function_trace_call_preempt_only(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op)
{
struct trace_array *tr = func_trace;
struct trace_array_cpu *data;
@@ -75,7 +76,8 @@ function_trace_call_preempt_only(unsigned long ip, unsigned long parent_ip)
}
static void
-function_trace_call(unsigned long ip, unsigned long parent_ip)
+function_trace_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op)
{
struct trace_array *tr = func_trace;
struct trace_array_cpu *data;
@@ -106,7 +108,8 @@ function_trace_call(unsigned long ip, unsigned long parent_ip)
}
static void
-function_stack_trace_call(unsigned long ip, unsigned long parent_ip)
+function_stack_trace_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op)
{
struct trace_array *tr = func_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index 99d20e9..2862c77 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -136,7 +136,8 @@ static int func_prolog_dec(struct trace_array *tr,
* irqsoff uses its own tracer function to keep the overhead down:
*/
static void
-irqsoff_tracer_call(unsigned long ip, unsigned long parent_ip)
+irqsoff_tracer_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op)
{
struct trace_array *tr = irqsoff_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
index ff791ea..0caf4f5 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -108,7 +108,7 @@ out_enable:
* wakeup uses its own tracer function to keep the overhead down:
*/
static void
-wakeup_tracer_call(unsigned long ip, unsigned long parent_ip)
+wakeup_tracer_call(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op)
{
struct trace_array *tr = wakeup_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index 288541f..9ae40c8 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -103,35 +103,40 @@ static inline void warn_failed_init_tracer(struct tracer *trace, int init_ret)
static int trace_selftest_test_probe1_cnt;
static void trace_selftest_test_probe1_func(unsigned long ip,
- unsigned long pip)
+ unsigned long pip,
+ struct ftrace_ops *op)
{
trace_selftest_test_probe1_cnt++;
}
static int trace_selftest_test_probe2_cnt;
static void trace_selftest_test_probe2_func(unsigned long ip,
- unsigned long pip)
+ unsigned long pip,
+ struct ftrace_ops *op)
{
trace_selftest_test_probe2_cnt++;
}
static int trace_selftest_test_probe3_cnt;
static void trace_selftest_test_probe3_func(unsigned long ip,
- unsigned long pip)
+ unsigned long pip,
+ struct ftrace_ops *op)
{
trace_selftest_test_probe3_cnt++;
}
static int trace_selftest_test_global_cnt;
static void trace_selftest_test_global_func(unsigned long ip,
- unsigned long pip)
+ unsigned long pip,
+ struct ftrace_ops *op)
{
trace_selftest_test_global_cnt++;
}
static int trace_selftest_test_dyn_cnt;
static void trace_selftest_test_dyn_func(unsigned long ip,
- unsigned long pip)
+ unsigned long pip,
+ struct ftrace_ops *op)
{
trace_selftest_test_dyn_cnt++;
}
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index d4545f4..e20006d 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -111,7 +111,7 @@ static inline void check_stack(void)
}
static void
-stack_trace_call(unsigned long ip, unsigned long parent_ip)
+stack_trace_call(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op)
{
int cpu;
--
1.7.10
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 2/6] ftrace: Consolidate arch dependent functions with list function
2012-07-02 20:03 [PATCH 0/6] [RFC v3] ftrace/kprobes: Setting up ftrace for kprobes Steven Rostedt
2012-07-02 20:03 ` [PATCH 1/6] ftrace: Pass ftrace_ops as third parameter to function trace callback Steven Rostedt
@ 2012-07-02 20:03 ` Steven Rostedt
2012-07-02 20:03 ` [PATCH 3/6] ftrace: Return pt_regs to function trace callback Steven Rostedt
` (3 subsequent siblings)
5 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2012-07-02 20:03 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
Masami Hiramatsu, H. Peter Anvin
[-- Attachment #1: 0002-ftrace-Consolidate-arch-dependent-functions-with-lis.patch --]
[-- Type: text/plain, Size: 5415 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
As the function tracer starts to get more features, the support for
theses features will spread out throughout the different architectures
over time. These features boil down to what each arch does in the
mcount trampoline (the ftrace_caller).
Currently there's two features that are not the same throughout the
archs.
1) Support to stop function tracing before the callback
2) passing of the ftrace ops
Both of these require placing an indirect function to support the
features if the mcount trampoline does not.
On a side note, for all architectures, when more than one callback
is registered to the function tracer, an intermediate 'list' function
is called by the mcount trampoline to iterate through the callbacks
that are registered.
Instead of making a separate function for each of these features,
and requiring several indirect calls, just use the single 'list' function
as the intermediate, to handle all cases. If an arch does not support
the 'stop function tracing' or the passing of ftrace ops, just force
it to use the list function that will handle the features required.
This makes the code cleaner and simpler and removes a lot of
#ifdefs in the code.
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace.h | 13 +++++++++++++
kernel/trace/ftrace.c | 45 ++++-----------------------------------------
2 files changed, 17 insertions(+), 41 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 2d59641..3651fdc 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -27,6 +27,19 @@
#define ARCH_SUPPORTS_FTRACE_OPS 0
#endif
+/*
+ * If the arch's mcount caller does not support all of ftrace's
+ * features, then it must call an indirect function that
+ * does. Or at least does enough to prevent any unwelcomed side effects.
+ */
+#if !defined(CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST) || \
+ !ARCH_SUPPORTS_FTRACE_OPS
+# define FTRACE_FORCE_LIST_FUNC 1
+#else
+# define FTRACE_FORCE_LIST_FUNC 0
+#endif
+
+
struct module;
struct ftrace_hash;
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 4f2ab93..4cbca2e 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -97,8 +97,6 @@ static struct ftrace_ops *ftrace_global_list __read_mostly = &ftrace_list_end;
static struct ftrace_ops *ftrace_control_list __read_mostly = &ftrace_list_end;
static struct ftrace_ops *ftrace_ops_list __read_mostly = &ftrace_list_end;
ftrace_func_t ftrace_trace_function __read_mostly = ftrace_stub;
-static ftrace_func_t __ftrace_trace_function_delay __read_mostly = ftrace_stub;
-ftrace_func_t __ftrace_trace_function __read_mostly = ftrace_stub;
ftrace_func_t ftrace_pid_function __read_mostly = ftrace_stub;
static struct ftrace_ops global_ops;
static struct ftrace_ops control_ops;
@@ -162,26 +160,9 @@ static void set_ftrace_pid_function(ftrace_func_t func)
void clear_ftrace_function(void)
{
ftrace_trace_function = ftrace_stub;
- __ftrace_trace_function = ftrace_stub;
- __ftrace_trace_function_delay = ftrace_stub;
ftrace_pid_function = ftrace_stub;
}
-#ifndef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
-/*
- * For those archs that do not test ftrace_trace_stop in their
- * mcount call site, we need to do it from C.
- */
-static void ftrace_test_stop_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
-{
- if (function_trace_stop)
- return;
-
- __ftrace_trace_function(ip, parent_ip, op);
-}
-#endif
-
static void control_ops_disable_all(struct ftrace_ops *ops)
{
int cpu;
@@ -246,7 +227,7 @@ static void update_ftrace_function(void)
if (ftrace_ops_list == &ftrace_list_end ||
(ftrace_ops_list->next == &ftrace_list_end &&
!(ftrace_ops_list->flags & FTRACE_OPS_FL_DYNAMIC) &&
- ARCH_SUPPORTS_FTRACE_OPS)) {
+ !FTRACE_FORCE_LIST_FUNC)) {
/* Set the ftrace_ops that the arch callback uses */
if (ftrace_ops_list == &global_ops)
function_trace_op = ftrace_global_list;
@@ -259,18 +240,7 @@ static void update_ftrace_function(void)
func = ftrace_ops_list_func;
}
-#ifdef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
ftrace_trace_function = func;
-#else
-#ifdef CONFIG_DYNAMIC_FTRACE
- /* do not update till all functions have been modified */
- __ftrace_trace_function_delay = func;
-#else
- __ftrace_trace_function = func;
-#endif
- ftrace_trace_function =
- (func == ftrace_stub) ? func : ftrace_test_stop_func;
-#endif
}
static void add_ftrace_ops(struct ftrace_ops **list, struct ftrace_ops *ops)
@@ -1902,16 +1872,6 @@ static void ftrace_run_update_code(int command)
*/
arch_ftrace_update_code(command);
-#ifndef CONFIG_HAVE_FUNCTION_TRACE_MCOUNT_TEST
- /*
- * For archs that call ftrace_test_stop_func(), we must
- * wait till after we update all the function callers
- * before we update the callback. This keeps different
- * ops that record different functions from corrupting
- * each other.
- */
- __ftrace_trace_function = __ftrace_trace_function_delay;
-#endif
function_trace_stop--;
ret = ftrace_arch_code_modify_post_process();
@@ -3996,6 +3956,9 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
{
struct ftrace_ops *op;
+ if (function_trace_stop)
+ return;
+
if (unlikely(trace_recursion_test(TRACE_INTERNAL_BIT)))
return;
--
1.7.10
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 3/6] ftrace: Return pt_regs to function trace callback
2012-07-02 20:03 [PATCH 0/6] [RFC v3] ftrace/kprobes: Setting up ftrace for kprobes Steven Rostedt
2012-07-02 20:03 ` [PATCH 1/6] ftrace: Pass ftrace_ops as third parameter to function trace callback Steven Rostedt
2012-07-02 20:03 ` [PATCH 2/6] ftrace: Consolidate arch dependent functions with list function Steven Rostedt
@ 2012-07-02 20:03 ` Steven Rostedt
2012-07-03 5:19 ` Masami Hiramatsu
2012-08-21 15:00 ` [tip:perf/core] " tip-bot for Steven Rostedt
2012-07-02 20:03 ` [PATCH 4/6] ftrace/x86_32: Push ftrace_ops in as 3rd parameter to function tracer Steven Rostedt
` (2 subsequent siblings)
5 siblings, 2 replies; 18+ messages in thread
From: Steven Rostedt @ 2012-07-02 20:03 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
Masami Hiramatsu, H. Peter Anvin
[-- Attachment #1: 0003-ftrace-Return-pt_regs-to-function-trace-callback.patch --]
[-- Type: text/plain, Size: 12867 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
Return as the 4th paramater to the function tracer callback the pt_regs.
Later patches that implement regs passing for the architectures will require
having the ftrace_ops set the SAVE_REGS flag, which will tell the arch
to take the time to pass a full set of pt_regs to the ftrace_ops callback
function. If the arch does not support it then it should pass NULL.
If an arch can pass full regs, then it should define:
ARCH_SUPPORTS_FTRACE_SAVE_REGS to 1
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace.h | 6 ++++--
kernel/trace/ftrace.c | 37 ++++++++++++++++++++++---------------
kernel/trace/trace_event_perf.c | 2 +-
kernel/trace/trace_events.c | 2 +-
kernel/trace/trace_functions.c | 7 ++++---
kernel/trace/trace_irqsoff.c | 2 +-
kernel/trace/trace_sched_wakeup.c | 3 ++-
kernel/trace/trace_selftest.c | 15 ++++++++++-----
kernel/trace/trace_stack.c | 3 ++-
9 files changed, 47 insertions(+), 30 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 3651fdc..e420288 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -10,6 +10,7 @@
#include <linux/kallsyms.h>
#include <linux/linkage.h>
#include <linux/bitops.h>
+#include <linux/ptrace.h>
#include <linux/ktime.h>
#include <linux/sched.h>
#include <linux/types.h>
@@ -54,7 +55,7 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,
struct ftrace_ops;
typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op);
+ struct ftrace_ops *op, struct pt_regs *regs);
/*
* FTRACE_OPS_FL_* bits denote the state of ftrace_ops struct and are
@@ -188,7 +189,8 @@ static inline int ftrace_function_local_disabled(struct ftrace_ops *ops)
return *this_cpu_ptr(ops->disabled);
}
-extern void ftrace_stub(unsigned long a0, unsigned long a1, struct ftrace_ops *op);
+extern void ftrace_stub(unsigned long a0, unsigned long a1,
+ struct ftrace_ops *op, struct pt_regs *regs);
#else /* !CONFIG_FUNCTION_TRACER */
/*
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 4cbca2e..6ff07ad 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -103,7 +103,7 @@ static struct ftrace_ops control_ops;
#if ARCH_SUPPORTS_FTRACE_OPS
static void ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op);
+ struct ftrace_ops *op, struct pt_regs *regs);
#else
/* See comment below, where ftrace_ops_list_func is defined */
static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip);
@@ -121,7 +121,7 @@ static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip);
*/
static void
ftrace_global_list_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *regs)
{
if (unlikely(trace_recursion_test(TRACE_GLOBAL_BIT)))
return;
@@ -129,19 +129,19 @@ ftrace_global_list_func(unsigned long ip, unsigned long parent_ip,
trace_recursion_set(TRACE_GLOBAL_BIT);
op = rcu_dereference_raw(ftrace_global_list); /*see above*/
while (op != &ftrace_list_end) {
- op->func(ip, parent_ip, op);
+ op->func(ip, parent_ip, op, regs);
op = rcu_dereference_raw(op->next); /*see above*/
};
trace_recursion_clear(TRACE_GLOBAL_BIT);
}
static void ftrace_pid_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *regs)
{
if (!test_tsk_trace_trace(current))
return;
- ftrace_pid_function(ip, parent_ip, op);
+ ftrace_pid_function(ip, parent_ip, op, regs);
}
static void set_ftrace_pid_function(ftrace_func_t func)
@@ -763,7 +763,7 @@ ftrace_profile_alloc(struct ftrace_profile_stat *stat, unsigned long ip)
static void
function_profile_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *ops)
+ struct ftrace_ops *ops, struct pt_regs *regs)
{
struct ftrace_profile_stat *stat;
struct ftrace_profile *rec;
@@ -793,7 +793,7 @@ function_profile_call(unsigned long ip, unsigned long parent_ip,
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
static int profile_graph_entry(struct ftrace_graph_ent *trace)
{
- function_profile_call(trace->func, 0, NULL);
+ function_profile_call(trace->func, 0, NULL, NULL);
return 1;
}
@@ -2771,7 +2771,7 @@ static int __init ftrace_mod_cmd_init(void)
device_initcall(ftrace_mod_cmd_init);
static void function_trace_probe_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
struct ftrace_func_probe *entry;
struct hlist_head *hhd;
@@ -3923,7 +3923,7 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip)
static void
ftrace_ops_control_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *regs)
{
if (unlikely(trace_recursion_test(TRACE_CONTROL_BIT)))
return;
@@ -3938,7 +3938,7 @@ ftrace_ops_control_func(unsigned long ip, unsigned long parent_ip,
while (op != &ftrace_list_end) {
if (!ftrace_function_local_disabled(op) &&
ftrace_ops_test(op, ip))
- op->func(ip, parent_ip, op);
+ op->func(ip, parent_ip, op, regs);
op = rcu_dereference_raw(op->next);
};
@@ -3952,7 +3952,7 @@ static struct ftrace_ops control_ops = {
static inline void
__ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *ignored)
+ struct ftrace_ops *ignored, struct pt_regs *regs)
{
struct ftrace_ops *op;
@@ -3971,7 +3971,7 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
op = rcu_dereference_raw(ftrace_ops_list);
while (op != &ftrace_list_end) {
if (ftrace_ops_test(op, ip))
- op->func(ip, parent_ip, op);
+ op->func(ip, parent_ip, op, regs);
op = rcu_dereference_raw(op->next);
};
preempt_enable_notrace();
@@ -3983,17 +3983,24 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
* the list function ignores the op parameter, we do not want any
* C side effects, where a function is called without the caller
* sending a third parameter.
+ * Archs are to support both the regs and ftrace_ops at the same time.
+ * If they support ftrace_ops, it is assumed they support regs.
+ * If call backs want to use regs, they must either check for regs
+ * being NULL, or ARCH_SUPPORTS_FTRACE_SAVE_REGS.
+ * Note, ARCH_SUPPORT_SAVE_REGS expects a full regs to be saved.
+ * An architecture can pass partial regs with ftrace_ops and still
+ * set the ARCH_SUPPORT_FTARCE_OPS.
*/
#if ARCH_SUPPORTS_FTRACE_OPS
static void ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *regs)
{
- __ftrace_ops_list_func(ip, parent_ip, NULL);
+ __ftrace_ops_list_func(ip, parent_ip, NULL, regs);
}
#else
static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip)
{
- __ftrace_ops_list_func(ip, parent_ip, NULL);
+ __ftrace_ops_list_func(ip, parent_ip, NULL, NULL);
}
#endif
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index a872a9a..9824419 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -259,7 +259,7 @@ EXPORT_SYMBOL_GPL(perf_trace_buf_prepare);
#ifdef CONFIG_FUNCTION_TRACER
static void
perf_ftrace_function_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *ops)
+ struct ftrace_ops *ops, struct pt_regs *pt_regs)
{
struct ftrace_entry *entry;
struct hlist_head *head;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 88daa51..8c66968 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1682,7 +1682,7 @@ static DEFINE_PER_CPU(atomic_t, ftrace_test_event_disable);
static void
function_test_events_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
struct ring_buffer_event *event;
struct ring_buffer *buffer;
diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index fceb7a9..5675ebd 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c
@@ -49,7 +49,7 @@ static void function_trace_start(struct trace_array *tr)
static void
function_trace_call_preempt_only(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
struct trace_array *tr = func_trace;
struct trace_array_cpu *data;
@@ -77,7 +77,8 @@ function_trace_call_preempt_only(unsigned long ip, unsigned long parent_ip,
static void
function_trace_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
+
{
struct trace_array *tr = func_trace;
struct trace_array_cpu *data;
@@ -109,7 +110,7 @@ function_trace_call(unsigned long ip, unsigned long parent_ip,
static void
function_stack_trace_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
struct trace_array *tr = func_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index 2862c77..c7a9ba9 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -137,7 +137,7 @@ static int func_prolog_dec(struct trace_array *tr,
*/
static void
irqsoff_tracer_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
struct trace_array *tr = irqsoff_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
index 0caf4f5..7547e36 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -108,7 +108,8 @@ out_enable:
* wakeup uses its own tracer function to keep the overhead down:
*/
static void
-wakeup_tracer_call(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op)
+wakeup_tracer_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
struct trace_array *tr = wakeup_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index 9ae40c8..add37e0 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -104,7 +104,8 @@ static inline void warn_failed_init_tracer(struct tracer *trace, int init_ret)
static int trace_selftest_test_probe1_cnt;
static void trace_selftest_test_probe1_func(unsigned long ip,
unsigned long pip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op,
+ struct pt_regs *pt_regs)
{
trace_selftest_test_probe1_cnt++;
}
@@ -112,7 +113,8 @@ static void trace_selftest_test_probe1_func(unsigned long ip,
static int trace_selftest_test_probe2_cnt;
static void trace_selftest_test_probe2_func(unsigned long ip,
unsigned long pip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op,
+ struct pt_regs *pt_regs)
{
trace_selftest_test_probe2_cnt++;
}
@@ -120,7 +122,8 @@ static void trace_selftest_test_probe2_func(unsigned long ip,
static int trace_selftest_test_probe3_cnt;
static void trace_selftest_test_probe3_func(unsigned long ip,
unsigned long pip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op,
+ struct pt_regs *pt_regs)
{
trace_selftest_test_probe3_cnt++;
}
@@ -128,7 +131,8 @@ static void trace_selftest_test_probe3_func(unsigned long ip,
static int trace_selftest_test_global_cnt;
static void trace_selftest_test_global_func(unsigned long ip,
unsigned long pip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op,
+ struct pt_regs *pt_regs)
{
trace_selftest_test_global_cnt++;
}
@@ -136,7 +140,8 @@ static void trace_selftest_test_global_func(unsigned long ip,
static int trace_selftest_test_dyn_cnt;
static void trace_selftest_test_dyn_func(unsigned long ip,
unsigned long pip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op,
+ struct pt_regs *pt_regs)
{
trace_selftest_test_dyn_cnt++;
}
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index e20006d..2fa5328 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -111,7 +111,8 @@ static inline void check_stack(void)
}
static void
-stack_trace_call(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op)
+stack_trace_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
int cpu;
--
1.7.10
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 4/6] ftrace/x86_32: Push ftrace_ops in as 3rd parameter to function tracer
2012-07-02 20:03 [PATCH 0/6] [RFC v3] ftrace/kprobes: Setting up ftrace for kprobes Steven Rostedt
` (2 preceding siblings ...)
2012-07-02 20:03 ` [PATCH 3/6] ftrace: Return pt_regs to function trace callback Steven Rostedt
@ 2012-07-02 20:03 ` Steven Rostedt
2012-07-02 20:03 ` [PATCH 5/6] ftrace/x86: Add separate function to save regs Steven Rostedt
2012-07-02 20:03 ` [PATCH 6/6] ftrace/x86: Add save_regs for i386 function calls Steven Rostedt
5 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2012-07-02 20:03 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
Masami Hiramatsu, H. Peter Anvin
[-- Attachment #1: 0004-ftrace-x86_32-Push-ftrace_ops-in-as-3rd-parameter-to.patch --]
[-- Type: text/plain, Size: 1196 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
Add support of passing the current ftrace_ops into the 3rd parameter
of the callback to the function tracer.
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
arch/x86/include/asm/ftrace.h | 2 +-
arch/x86/kernel/entry_32.S | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index 783b107..b3bb1f3 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -32,7 +32,7 @@
#define MCOUNT_ADDR ((long)(mcount))
#define MCOUNT_INSN_SIZE 5 /* sizeof mcount call */
-#if defined(CONFIG_DYNAMIC_FTRACE) && defined(CONFIG_X86_64)
+#ifdef CONFIG_DYNAMIC_FTRACE
#define ARCH_SUPPORTS_FTRACE_OPS 1
#endif
diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index 623f288..e3e17a0 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -1111,6 +1111,7 @@ ENTRY(ftrace_caller)
pushl %edx
movl 0xc(%esp), %eax
movl 0x4(%ebp), %edx
+ leal function_trace_op, %ecx
subl $MCOUNT_INSN_SIZE, %eax
.globl ftrace_call
--
1.7.10
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 5/6] ftrace/x86: Add separate function to save regs
2012-07-02 20:03 [PATCH 0/6] [RFC v3] ftrace/kprobes: Setting up ftrace for kprobes Steven Rostedt
` (3 preceding siblings ...)
2012-07-02 20:03 ` [PATCH 4/6] ftrace/x86_32: Push ftrace_ops in as 3rd parameter to function tracer Steven Rostedt
@ 2012-07-02 20:03 ` Steven Rostedt
2012-07-03 8:29 ` Masami Hiramatsu
2012-07-03 16:54 ` Alexander van Heukelum
2012-07-02 20:03 ` [PATCH 6/6] ftrace/x86: Add save_regs for i386 function calls Steven Rostedt
5 siblings, 2 replies; 18+ messages in thread
From: Steven Rostedt @ 2012-07-02 20:03 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
Masami Hiramatsu, H. Peter Anvin
[-- Attachment #1: 0005-ftrace-x86-Add-separate-function-to-save-regs.patch --]
[-- Type: text/plain, Size: 22234 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
Add a way to have different functions calling different trampolines.
If a ftrace_ops wants regs saved on the return, then have only the
functions with ops registered to save regs. Functions registered by
other ops would not be affected, unless the functions overlap.
If one ftrace_ops registered functions A, B and C and another ops
registered fucntions to save regs on A, and D, then only functions
A and D would be saving regs. Function B and C would work as normal.
Although A is registered by both ops: normal and saves regs; this is fine
as saving the regs is needed to satisfy one of the ops that calls it
but the regs are ignored by the other ops function.
x86_64 implements the full regs saving, and i386 just passes a NULL
for regs to satisfy the ftrace_ops passing. Where an arch must supply
both regs and ftrace_ops parameters, even if regs is just NULL.
It is OK for an arch to pass NULL regs. All function trace users that
require regs passing must add the flag FTRACE_OPS_FL_SAVE_REGS when
registering the ftrace_ops. If the arch does not support saving regs
then the ftrace_ops will fail to register. The flag
FTRACE_OPS_FL_SAVE_REGS_IF_SUPPORTED may be set that will prevent the
ftrace_ops from failing to register. In this case, the handler may
either check if regs is not NULL or check if ARCH_SUPPORTS_FTRACE_SAVE_REGS.
If the arch supports passing regs it will set this macro and pass regs
for ops that request them. All other archs will just pass NULL.
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
arch/x86/include/asm/ftrace.h | 45 ++++++++++-------
arch/x86/kernel/entry_32.S | 2 +
arch/x86/kernel/entry_64.S | 90 +++++++++++++++++++++++++++++++---
arch/x86/kernel/ftrace.c | 77 +++++++++++++++++++++++++++--
include/linux/ftrace.h | 107 ++++++++++++++++++++++++++++++++++++++---
kernel/trace/ftrace.c | 91 ++++++++++++++++++++++++++++++++---
6 files changed, 367 insertions(+), 45 deletions(-)
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index b3bb1f3..fd10faf 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -3,27 +3,33 @@
#ifdef __ASSEMBLY__
- .macro MCOUNT_SAVE_FRAME
- /* taken from glibc */
- subq $0x38, %rsp
- movq %rax, (%rsp)
- movq %rcx, 8(%rsp)
- movq %rdx, 16(%rsp)
- movq %rsi, 24(%rsp)
- movq %rdi, 32(%rsp)
- movq %r8, 40(%rsp)
- movq %r9, 48(%rsp)
+ /* skip is set if the stack was already partially adjusted */
+ .macro MCOUNT_SAVE_FRAME skip=0
+ /*
+ * We add enough stack to save all regs.
+ */
+ subq $(SS+8-\skip), %rsp
+ movq %rax, RAX(%rsp)
+ movq %rcx, RCX(%rsp)
+ movq %rdx, RDX(%rsp)
+ movq %rsi, RSI(%rsp)
+ movq %rdi, RDI(%rsp)
+ movq %r8, R8(%rsp)
+ movq %r9, R9(%rsp)
+ /* Move RIP to its proper location */
+ movq SS+8(%rsp), %rdx
+ movq %rdx, RIP(%rsp)
.endm
.macro MCOUNT_RESTORE_FRAME
- movq 48(%rsp), %r9
- movq 40(%rsp), %r8
- movq 32(%rsp), %rdi
- movq 24(%rsp), %rsi
- movq 16(%rsp), %rdx
- movq 8(%rsp), %rcx
- movq (%rsp), %rax
- addq $0x38, %rsp
+ movq R9(%rsp), %r9
+ movq R8(%rsp), %r8
+ movq RDI(%rsp), %rdi
+ movq RSI(%rsp), %rsi
+ movq RDX(%rsp), %rdx
+ movq RCX(%rsp), %rcx
+ movq RAX(%rsp), %rax
+ addq $(SS+8), %rsp
.endm
#endif
@@ -34,6 +40,9 @@
#ifdef CONFIG_DYNAMIC_FTRACE
#define ARCH_SUPPORTS_FTRACE_OPS 1
+#ifdef CONFIG_X86_64
+#define ARCH_SUPPORTS_FTRACE_SAVE_REGS
+#endif
#endif
#ifndef __ASSEMBLY__
diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index e3e17a0..acd4963 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -1109,6 +1109,7 @@ ENTRY(ftrace_caller)
pushl %eax
pushl %ecx
pushl %edx
+ pushl $0 /* Pass NULL as regs pointer */
movl 0xc(%esp), %eax
movl 0x4(%ebp), %edx
leal function_trace_op, %ecx
@@ -1118,6 +1119,7 @@ ENTRY(ftrace_caller)
ftrace_call:
call ftrace_stub
+ addl $4,%esp /* skip NULL pointer */
popl %edx
popl %ecx
popl %eax
diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 2b4f94c..63d43bc 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -73,21 +73,34 @@ ENTRY(mcount)
retq
END(mcount)
+/* skip is set if stack has been adjusted */
+.macro ftrace_caller_setup skip=0
+ MCOUNT_SAVE_FRAME \skip
+
+ /* Load the ftrace_ops into the 3rd parameter */
+ leaq function_trace_op, %rdx
+
+ /* Load ip into the first parameter */
+ movq RIP(%rsp), %rdi
+ subq $MCOUNT_INSN_SIZE, %rdi
+ /* Load the parent_ip into the second parameter */
+ movq 8(%rbp), %rsi
+.endm
+
ENTRY(ftrace_caller)
+ /* Check if tracing was disabled (quick check) */
cmpl $0, function_trace_stop
jne ftrace_stub
- MCOUNT_SAVE_FRAME
-
- leaq function_trace_op, %rdx
- movq 0x38(%rsp), %rdi
- movq 8(%rbp), %rsi
- subq $MCOUNT_INSN_SIZE, %rdi
+ ftrace_caller_setup
+ /* regs go into 4th parameter (but make it NULL) */
+ movq $0, %rcx
GLOBAL(ftrace_call)
call ftrace_stub
MCOUNT_RESTORE_FRAME
+ftrace_return:
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
GLOBAL(ftrace_graph_call)
@@ -98,6 +111,67 @@ GLOBAL(ftrace_stub)
retq
END(ftrace_caller)
+ENTRY(ftrace_regs_caller)
+ /* Save the current flags before compare (in SS location)*/
+ pushfq
+
+ /* Check if tracing was disabled (quick check) */
+ cmpl $0, function_trace_stop
+ jne ftrace_restore_flags
+
+ /* skip=8 to skip flags saved in SS */
+ ftrace_caller_setup 8
+
+ /* Save the rest of pt_regs */
+ movq %r15, R15(%rsp)
+ movq %r14, R14(%rsp)
+ movq %r13, R13(%rsp)
+ movq %r12, R12(%rsp)
+ movq %r11, R11(%rsp)
+ movq %r10, R10(%rsp)
+ movq %rbp, RBP(%rsp)
+ movq %rbx, RBX(%rsp)
+ /* Copy saved flags */
+ movq SS(%rsp), %rcx
+ movq %rcx, EFLAGS(%rsp)
+ /* Kernel segments */
+ movq $__KERNEL_DS, %rcx
+ movq %rcx, SS(%rsp)
+ movq $__KERNEL_CS, %rcx
+ movq %rcx, CS(%rsp)
+ /* Stack - skipping return address */
+ leaq SS+16(%rsp), %rcx
+ movq %rcx, RSP(%rsp)
+
+ /* regs go into 4th parameter */
+ leaq (%rsp), %rcx
+
+GLOBAL(ftrace_regs_call)
+ call ftrace_stub
+
+ /* restore the rest of pt_regs */
+ movq R15(%rsp), %r15
+ movq R14(%rsp), %r14
+ movq R13(%rsp), %r13
+ movq R12(%rsp), %r12
+ movq R10(%rsp), %r10
+ movq RBP(%rsp), %rbp
+ movq RBX(%rsp), %rbx
+
+ /* Restore flags */
+ pushq EFLAGS(%rsp)
+ popfq
+
+ MCOUNT_RESTORE_FRAME
+
+ jmp ftrace_return
+ftrace_restore_flags:
+ popfq
+ jmp ftrace_stub
+
+END(ftrace_regs_caller)
+
+
#else /* ! CONFIG_DYNAMIC_FTRACE */
ENTRY(mcount)
cmpl $0, function_trace_stop
@@ -120,7 +194,7 @@ GLOBAL(ftrace_stub)
trace:
MCOUNT_SAVE_FRAME
- movq 0x38(%rsp), %rdi
+ movq RIP(%rsp), %rdi
movq 8(%rbp), %rsi
subq $MCOUNT_INSN_SIZE, %rdi
@@ -141,7 +215,7 @@ ENTRY(ftrace_graph_caller)
MCOUNT_SAVE_FRAME
leaq 8(%rbp), %rdi
- movq 0x38(%rsp), %rsi
+ movq RIP(%rsp), %rsi
movq (%rbp), %rdx
subq $MCOUNT_INSN_SIZE, %rsi
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index c3a7cb4..b90eb1a 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -206,6 +206,23 @@ static int
ftrace_modify_code(unsigned long ip, unsigned const char *old_code,
unsigned const char *new_code);
+#ifdef ARCH_SUPPORTS_FTRACE_SAVE_REGS
+/*
+ * Should never be called:
+ * As it is only called by __ftrace_replace_code() which is called by
+ * ftrace_replace_code() that x86 overrides, and by ftrace_update_code()
+ * which is called to turn mcount into nops or nops into function calls
+ * but not to convert a function from not using regs to one that uses
+ * regs, which ftrace_modify_call() is for.
+ */
+int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
+ unsigned long addr)
+{
+ WARN_ON(1);
+ return -EINVAL;
+}
+#endif
+
int ftrace_update_ftrace_func(ftrace_func_t func)
{
unsigned long ip = (unsigned long)(&ftrace_call);
@@ -220,6 +237,16 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
ret = ftrace_modify_code(ip, old, new);
+#ifdef ARCH_SUPPORTS_FTRACE_SAVE_REGS
+ /* Also update the regs callback function */
+ if (!ret) {
+ ip = (unsigned long)(&ftrace_regs_call);
+ memcpy(old, &ftrace_regs_call, MCOUNT_INSN_SIZE);
+ new = ftrace_call_replace(ip, (unsigned long)func);
+ ret = ftrace_modify_code(ip, old, new);
+ }
+#endif
+
atomic_dec(&modifying_ftrace_code);
return ret;
@@ -299,6 +326,32 @@ static int add_brk_on_nop(struct dyn_ftrace *rec)
return add_break(rec->ip, old);
}
+/*
+ * If the record has the FTRACE_FL_REGS set, that means that it
+ * wants to convert to a callback that saves all regs. If FTRACE_FL_REGS
+ * is not not set, then it wants to convert to the normal callback.
+ */
+static unsigned long get_ftrace_addr(struct dyn_ftrace *rec)
+{
+ if (rec->flags & FTRACE_FL_REGS)
+ return (unsigned long)FTRACE_REGS_ADDR;
+ else
+ return (unsigned long)FTRACE_ADDR;
+}
+
+/*
+ * The FTRACE_FL_REGS_EN is set when the record already points to
+ * a function that saves all the regs. Basically the '_EN' version
+ * represents the current state of the function.
+ */
+static unsigned long get_ftrace_old_addr(struct dyn_ftrace *rec)
+{
+ if (rec->flags & FTRACE_FL_REGS_EN)
+ return (unsigned long)FTRACE_REGS_ADDR;
+ else
+ return (unsigned long)FTRACE_ADDR;
+}
+
static int add_breakpoints(struct dyn_ftrace *rec, int enable)
{
unsigned long ftrace_addr;
@@ -306,7 +359,7 @@ static int add_breakpoints(struct dyn_ftrace *rec, int enable)
ret = ftrace_test_record(rec, enable);
- ftrace_addr = (unsigned long)FTRACE_ADDR;
+ ftrace_addr = get_ftrace_addr(rec);
switch (ret) {
case FTRACE_UPDATE_IGNORE:
@@ -316,6 +369,10 @@ static int add_breakpoints(struct dyn_ftrace *rec, int enable)
/* converting nop to call */
return add_brk_on_nop(rec);
+ case FTRACE_UPDATE_MODIFY_CALL_REGS:
+ case FTRACE_UPDATE_MODIFY_CALL:
+ ftrace_addr = get_ftrace_old_addr(rec);
+ /* fall through */
case FTRACE_UPDATE_MAKE_NOP:
/* converting a call to a nop */
return add_brk_on_call(rec, ftrace_addr);
@@ -360,13 +417,21 @@ static int remove_breakpoint(struct dyn_ftrace *rec)
* If not, don't touch the breakpoint, we make just create
* a disaster.
*/
- ftrace_addr = (unsigned long)FTRACE_ADDR;
+ ftrace_addr = get_ftrace_addr(rec);
+ nop = ftrace_call_replace(ip, ftrace_addr);
+
+ if (memcmp(&ins[1], &nop[1], MCOUNT_INSN_SIZE - 1) == 0)
+ goto update;
+
+ /* Check both ftrace_addr and ftrace_old_addr */
+ ftrace_addr = get_ftrace_old_addr(rec);
nop = ftrace_call_replace(ip, ftrace_addr);
if (memcmp(&ins[1], &nop[1], MCOUNT_INSN_SIZE - 1) != 0)
return -EINVAL;
}
+ update:
return probe_kernel_write((void *)ip, &nop[0], 1);
}
@@ -405,12 +470,14 @@ static int add_update(struct dyn_ftrace *rec, int enable)
ret = ftrace_test_record(rec, enable);
- ftrace_addr = (unsigned long)FTRACE_ADDR;
+ ftrace_addr = get_ftrace_addr(rec);
switch (ret) {
case FTRACE_UPDATE_IGNORE:
return 0;
+ case FTRACE_UPDATE_MODIFY_CALL_REGS:
+ case FTRACE_UPDATE_MODIFY_CALL:
case FTRACE_UPDATE_MAKE_CALL:
/* converting nop to call */
return add_update_call(rec, ftrace_addr);
@@ -455,12 +522,14 @@ static int finish_update(struct dyn_ftrace *rec, int enable)
ret = ftrace_update_record(rec, enable);
- ftrace_addr = (unsigned long)FTRACE_ADDR;
+ ftrace_addr = get_ftrace_addr(rec);
switch (ret) {
case FTRACE_UPDATE_IGNORE:
return 0;
+ case FTRACE_UPDATE_MODIFY_CALL_REGS:
+ case FTRACE_UPDATE_MODIFY_CALL:
case FTRACE_UPDATE_MAKE_CALL:
/* converting nop to call */
return finish_update_call(rec, ftrace_addr);
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index e420288..ab39990 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -71,12 +71,28 @@ typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
* could be controled by following calls:
* ftrace_function_local_enable
* ftrace_function_local_disable
+ * SAVE_REGS - The ftrace_ops wants regs saved at each function called
+ * and passed to the callback. If this flag is set, but the
+ * architecture does not support passing regs
+ * (ARCH_SUPPORTS_FTRACE_SAVE_REGS is not defined), then the
+ * ftrace_ops will fail to register, unless the next flag
+ * is set.
+ * SAVE_REGS_IF_SUPPORTED - This is the same as SAVE_REGS, but if the
+ * handler can handle an arch that does not save regs
+ * (the handler tests if regs == NULL), then it can set
+ * this flag instead. It will not fail registering the ftrace_ops
+ * but, the regs field will be NULL if the arch does not support
+ * passing regs to the handler.
+ * Note, if this flag is set, the SAVE_REGS flag will automatically
+ * get set upon registering the ftrace_ops, if the arch supports it.
*/
enum {
- FTRACE_OPS_FL_ENABLED = 1 << 0,
- FTRACE_OPS_FL_GLOBAL = 1 << 1,
- FTRACE_OPS_FL_DYNAMIC = 1 << 2,
- FTRACE_OPS_FL_CONTROL = 1 << 3,
+ FTRACE_OPS_FL_ENABLED = 1 << 0,
+ FTRACE_OPS_FL_GLOBAL = 1 << 1,
+ FTRACE_OPS_FL_DYNAMIC = 1 << 2,
+ FTRACE_OPS_FL_CONTROL = 1 << 3,
+ FTRACE_OPS_FL_SAVE_REGS = 1 << 4,
+ FTRACE_OPS_FL_SAVE_REGS_IF_SUPPORTED = 1 << 5,
};
struct ftrace_ops {
@@ -254,12 +270,31 @@ extern void unregister_ftrace_function_probe_all(char *glob);
extern int ftrace_text_reserved(void *start, void *end);
+/*
+ * The dyn_ftrace record's flags field is split into two parts.
+ * the first part which is '0-FTRACE_REF_MAX' is a counter of
+ * the number of callbacks that have registered the function that
+ * the dyn_ftrace descriptor represents.
+ *
+ * The second part is a mask:
+ * ENABLED - the function is being traced
+ * REGS - the record wants the function to save regs
+ * REGS_EN - the function is set up to save regs.
+ *
+ * When a new ftrace_ops is registered and wants a function to save
+ * pt_regs, the rec->flag REGS is set. When the function has been
+ * set up to save regs, the REG_EN flag is set. Once a function
+ * starts saving regs it will do so until all ftrace_ops are removed
+ * from tracing that function.
+ */
enum {
- FTRACE_FL_ENABLED = (1 << 30),
+ FTRACE_FL_ENABLED = (1UL << 29),
+ FTRACE_FL_REGS = (1UL << 30),
+ FTRACE_FL_REGS_EN = (1UL << 31)
};
-#define FTRACE_FL_MASK (0x3UL << 30)
-#define FTRACE_REF_MAX ((1 << 30) - 1)
+#define FTRACE_FL_MASK (0x7UL << 29)
+#define FTRACE_REF_MAX ((1UL << 29) - 1)
struct dyn_ftrace {
union {
@@ -290,9 +325,23 @@ enum {
FTRACE_STOP_FUNC_RET = (1 << 4),
};
+/*
+ * The FTRACE_UPDATE_* enum is used to pass information back
+ * from the ftrace_update_record() and ftrace_test_record()
+ * functions. These are called by the code update routines
+ * to find out what is to be done for a given function.
+ *
+ * IGNORE - The function is already what we want it to be
+ * MAKE_CALL - Start tracing the function
+ * MODIFY_CALL - Stop saving regs for the function
+ * MODIFY_CALL_REGS - Start saving regs for the function
+ * MAKE_NOP - Stop tracing the function
+ */
enum {
FTRACE_UPDATE_IGNORE,
FTRACE_UPDATE_MAKE_CALL,
+ FTRACE_UPDATE_MODIFY_CALL,
+ FTRACE_UPDATE_MODIFY_CALL_REGS,
FTRACE_UPDATE_MAKE_NOP,
};
@@ -344,7 +393,9 @@ extern int ftrace_dyn_arch_init(void *data);
extern void ftrace_replace_code(int enable);
extern int ftrace_update_ftrace_func(ftrace_func_t func);
extern void ftrace_caller(void);
+extern void ftrace_regs_caller(void);
extern void ftrace_call(void);
+extern void ftrace_regs_call(void);
extern void mcount_call(void);
void ftrace_modify_all_code(int command);
@@ -352,6 +403,15 @@ void ftrace_modify_all_code(int command);
#ifndef FTRACE_ADDR
#define FTRACE_ADDR ((unsigned long)ftrace_caller)
#endif
+
+#ifndef FTRACE_REGS_ADDR
+#ifdef ARCH_SUPPORTS_FTRACE_SAVE_REGS
+# define FTRACE_REGS_ADDR ((unsigned long)ftrace_regs_caller)
+#else
+# define FTRACE_REGS_ADDR FTRACE_ADDR
+#endif
+#endif
+
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
extern void ftrace_graph_caller(void);
extern int ftrace_enable_ftrace_graph_caller(void);
@@ -407,6 +467,39 @@ extern int ftrace_make_nop(struct module *mod,
*/
extern int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr);
+#ifdef ARCH_SUPPORTS_FTRACE_SAVE_REGS
+/**
+ * ftrace_modify_call - convert from one addr to another (no nop)
+ * @rec: the mcount call site record
+ * @old_addr: the address expected to be currently called to
+ * @addr: the address to change to
+ *
+ * This is a very sensitive operation and great care needs
+ * to be taken by the arch. The operation should carefully
+ * read the location, check to see if what is read is indeed
+ * what we expect it to be, and then on success of the compare,
+ * it should write to the location.
+ *
+ * The code segment at @rec->ip should be a caller to @old_addr
+ *
+ * Return must be:
+ * 0 on success
+ * -EFAULT on error reading the location
+ * -EINVAL on a failed compare of the contents
+ * -EPERM on error writing to the location
+ * Any other value will be considered a failure.
+ */
+extern int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
+ unsigned long addr);
+#else
+/* Should never be called */
+static inline int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
+ unsigned long addr)
+{
+ return -EINVAL;
+}
+#endif
+
/* May be defined in arch */
extern int ftrace_arch_read_dyn_info(char *buf, int size);
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 6ff07ad..c55f7e2 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -314,6 +314,20 @@ static int __register_ftrace_function(struct ftrace_ops *ops)
if ((ops->flags & FL_GLOBAL_CONTROL_MASK) == FL_GLOBAL_CONTROL_MASK)
return -EINVAL;
+#ifndef ARCH_SUPPORTS_FTRACE_SAVE_REGS
+ /*
+ * If the ftrace_ops specifies SAVE_REGS, then it only can be used
+ * if the arch supports it, or SAVE_REGS_IF_SUPPORTED is also set.
+ * Setting SAVE_REGS_IF_SUPPORTED makes SAVE_REGS irrelevant.
+ */
+ if (ops->flags & FTRACE_OPS_FL_SAVE_REGS &&
+ !(ops->flags & FTRACE_OPS_FL_SAVE_REGS_IF_SUPPORTED))
+ return -EINVAL;
+
+ if (ops->flags & FTRACE_OPS_FL_SAVE_REGS_IF_SUPPORTED)
+ ops->flags |= FTRACE_OPS_FL_SAVE_REGS;
+#endif
+
if (!core_kernel_data((unsigned long)ops))
ops->flags |= FTRACE_OPS_FL_DYNAMIC;
@@ -1515,6 +1529,12 @@ static void __ftrace_hash_rec_update(struct ftrace_ops *ops,
rec->flags++;
if (FTRACE_WARN_ON((rec->flags & ~FTRACE_FL_MASK) == FTRACE_REF_MAX))
return;
+ /*
+ * If any ops wants regs saved for this function
+ * then all ops will get saved regs.
+ */
+ if (ops->flags & FTRACE_OPS_FL_SAVE_REGS)
+ rec->flags |= FTRACE_FL_REGS;
} else {
if (FTRACE_WARN_ON((rec->flags & ~FTRACE_FL_MASK) == 0))
return;
@@ -1606,18 +1626,59 @@ static int ftrace_check_record(struct dyn_ftrace *rec, int enable, int update)
if (enable && (rec->flags & ~FTRACE_FL_MASK))
flag = FTRACE_FL_ENABLED;
+ /*
+ * If enabling and the REGS flag does not match the REGS_EN, then
+ * do not ignore this record. Set flags to fail the compare against
+ * ENABLED.
+ */
+ if (flag &&
+ (!(rec->flags & FTRACE_FL_REGS) != !(rec->flags & FTRACE_FL_REGS_EN)))
+ flag |= FTRACE_FL_REGS;
+
/* If the state of this record hasn't changed, then do nothing */
if ((rec->flags & FTRACE_FL_ENABLED) == flag)
return FTRACE_UPDATE_IGNORE;
if (flag) {
- if (update)
+ /* Save off if rec is being enabled (for return value) */
+ flag ^= rec->flags & FTRACE_FL_ENABLED;
+
+ if (update) {
rec->flags |= FTRACE_FL_ENABLED;
- return FTRACE_UPDATE_MAKE_CALL;
+ if (flag & FTRACE_FL_REGS) {
+ if (rec->flags & FTRACE_FL_REGS)
+ rec->flags |= FTRACE_FL_REGS_EN;
+ else
+ rec->flags &= ~FTRACE_FL_REGS_EN;
+ }
+ }
+
+ /*
+ * If this record is being updated from a nop, then
+ * return UPDATE_MAKE_CALL.
+ * Otherwise, if the EN flag is set, then return
+ * UPDATE_MODIFY_CALL_REGS to tell the caller to convert
+ * from the non-save regs, to a save regs function.
+ * Otherwise,
+ * return UPDATE_MODIFY_CALL to tell the caller to convert
+ * from the save regs, to a non-save regs function.
+ */
+ if (flag & FTRACE_FL_ENABLED)
+ return FTRACE_UPDATE_MAKE_CALL;
+ else if (rec->flags & FTRACE_FL_REGS_EN)
+ return FTRACE_UPDATE_MODIFY_CALL_REGS;
+ else
+ return FTRACE_UPDATE_MODIFY_CALL;
}
- if (update)
- rec->flags &= ~FTRACE_FL_ENABLED;
+ if (update) {
+ /* If there's no more users, clear all flags */
+ if (!(rec->flags & ~FTRACE_FL_MASK))
+ rec->flags = 0;
+ else
+ /* Just disable the record (keep REGS state) */
+ rec->flags &= ~FTRACE_FL_ENABLED;
+ }
return FTRACE_UPDATE_MAKE_NOP;
}
@@ -1652,13 +1713,17 @@ int ftrace_test_record(struct dyn_ftrace *rec, int enable)
static int
__ftrace_replace_code(struct dyn_ftrace *rec, int enable)
{
+ unsigned long ftrace_old_addr;
unsigned long ftrace_addr;
int ret;
- ftrace_addr = (unsigned long)FTRACE_ADDR;
-
ret = ftrace_update_record(rec, enable);
+ if (rec->flags & FTRACE_FL_REGS)
+ ftrace_addr = (unsigned long)FTRACE_REGS_ADDR;
+ else
+ ftrace_addr = (unsigned long)FTRACE_ADDR;
+
switch (ret) {
case FTRACE_UPDATE_IGNORE:
return 0;
@@ -1668,6 +1733,15 @@ __ftrace_replace_code(struct dyn_ftrace *rec, int enable)
case FTRACE_UPDATE_MAKE_NOP:
return ftrace_make_nop(NULL, rec, ftrace_addr);
+
+ case FTRACE_UPDATE_MODIFY_CALL_REGS:
+ case FTRACE_UPDATE_MODIFY_CALL:
+ if (rec->flags & FTRACE_FL_REGS)
+ ftrace_old_addr = (unsigned long)FTRACE_ADDR;
+ else
+ ftrace_old_addr = (unsigned long)FTRACE_REGS_ADDR;
+
+ return ftrace_modify_call(rec, ftrace_old_addr, ftrace_addr);
}
return -1; /* unknow ftrace bug */
@@ -2421,8 +2495,9 @@ static int t_show(struct seq_file *m, void *v)
seq_printf(m, "%ps", (void *)rec->ip);
if (iter->flags & FTRACE_ITER_ENABLED)
- seq_printf(m, " (%ld)",
- rec->flags & ~FTRACE_FL_MASK);
+ seq_printf(m, " (%ld)%s",
+ rec->flags & ~FTRACE_FL_MASK,
+ rec->flags & FTRACE_FL_REGS ? " R" : "");
seq_printf(m, "\n");
return 0;
--
1.7.10
^ permalink raw reply related [flat|nested] 18+ messages in thread
* [PATCH 6/6] ftrace/x86: Add save_regs for i386 function calls
2012-07-02 20:03 [PATCH 0/6] [RFC v3] ftrace/kprobes: Setting up ftrace for kprobes Steven Rostedt
` (4 preceding siblings ...)
2012-07-02 20:03 ` [PATCH 5/6] ftrace/x86: Add separate function to save regs Steven Rostedt
@ 2012-07-02 20:03 ` Steven Rostedt
2012-07-03 5:27 ` Masami Hiramatsu
5 siblings, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2012-07-02 20:03 UTC (permalink / raw)
To: linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
Masami Hiramatsu, H. Peter Anvin
[-- Attachment #1: 0006-ftrace-x86-Add-save_regs-for-i386-function-calls.patch --]
[-- Type: text/plain, Size: 3544 bytes --]
From: Steven Rostedt <srostedt@redhat.com>
Add saving full regs for function tracing on i386.
The saving of regs was influenced by patches sent out by
Masami Hiramatsu.
Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
arch/x86/include/asm/ftrace.h | 2 --
arch/x86/kernel/entry_32.S | 53 +++++++++++++++++++++++++++++++++++++++++
arch/x86/kernel/ftrace.c | 4 ----
3 files changed, 53 insertions(+), 6 deletions(-)
diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
index fd10faf..2b396cf 100644
--- a/arch/x86/include/asm/ftrace.h
+++ b/arch/x86/include/asm/ftrace.h
@@ -40,10 +40,8 @@
#ifdef CONFIG_DYNAMIC_FTRACE
#define ARCH_SUPPORTS_FTRACE_OPS 1
-#ifdef CONFIG_X86_64
#define ARCH_SUPPORTS_FTRACE_SAVE_REGS
#endif
-#endif
#ifndef __ASSEMBLY__
extern void mcount(void);
diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
index acd4963..e18ea33 100644
--- a/arch/x86/kernel/entry_32.S
+++ b/arch/x86/kernel/entry_32.S
@@ -1123,6 +1123,7 @@ ftrace_call:
popl %edx
popl %ecx
popl %eax
+ftrace_ret:
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
.globl ftrace_graph_call
ftrace_graph_call:
@@ -1134,6 +1135,58 @@ ftrace_stub:
ret
END(ftrace_caller)
+ENTRY(ftrace_regs_caller)
+ pushf /* push flags before compare */
+ cmpl $0, function_trace_stop
+ jne ftrace_exit
+
+ subl $8, %esp /* skip ip and orig_ax */
+ pushl %gs
+ pushl %fs
+ pushl %es
+ pushl %ds
+ pushl %eax
+ pushl %ebp
+ pushl %edi
+ pushl %esi
+ pushl %edx
+ pushl %ecx
+ pushl %ebx
+ movl 14*4(%esp), %eax /* Load return address */
+ pushl %eax /* Save return address (+4) */
+ subl $MCOUNT_INSN_SIZE, %eax
+ movl %eax, 12*4+4(%esp) /* Store IP */
+ movl 13*4+4(%esp), %edx /* Load flags */
+ movl %edx, 14*4+4(%esp) /* Store flags */
+ movl $__KERNEL_CS, %edx
+ movl %edx, 13*4+4(%esp) /* Store CS */
+
+ movl 0x4(%ebp), %edx
+ lea 4(%esp), %ecx
+ pushl %ecx /* Save pt_regs as 4th parameter */
+ leal function_trace_op, %ecx
+
+GLOBAL(ftrace_regs_call)
+ call ftrace_stub
+
+ addl $4,%esp /* Skip pt_regs */
+ popl %eax
+ movl %eax, 14*4(%esp) /* Restore return address */
+ popl %ebx
+ popl %ecx
+ popl %edx
+ popl %esi
+ popl %edi
+ popl %ebp
+ popl %eax
+ popl %ds
+ popl %es
+ popl %fs
+ popl %gs
+ addl $8, %esp
+ftrace_exit:
+ popf
+ jmp ftrace_ret
#else /* ! CONFIG_DYNAMIC_FTRACE */
ENTRY(mcount)
diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index b90eb1a..1d41402 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -206,7 +206,6 @@ static int
ftrace_modify_code(unsigned long ip, unsigned const char *old_code,
unsigned const char *new_code);
-#ifdef ARCH_SUPPORTS_FTRACE_SAVE_REGS
/*
* Should never be called:
* As it is only called by __ftrace_replace_code() which is called by
@@ -221,7 +220,6 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
WARN_ON(1);
return -EINVAL;
}
-#endif
int ftrace_update_ftrace_func(ftrace_func_t func)
{
@@ -237,7 +235,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
ret = ftrace_modify_code(ip, old, new);
-#ifdef ARCH_SUPPORTS_FTRACE_SAVE_REGS
/* Also update the regs callback function */
if (!ret) {
ip = (unsigned long)(&ftrace_regs_call);
@@ -245,7 +242,6 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
new = ftrace_call_replace(ip, (unsigned long)func);
ret = ftrace_modify_code(ip, old, new);
}
-#endif
atomic_dec(&modifying_ftrace_code);
--
1.7.10
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 3/6] ftrace: Return pt_regs to function trace callback
2012-07-02 20:03 ` [PATCH 3/6] ftrace: Return pt_regs to function trace callback Steven Rostedt
@ 2012-07-03 5:19 ` Masami Hiramatsu
2012-07-11 15:28 ` Steven Rostedt
2012-08-21 15:00 ` [tip:perf/core] " tip-bot for Steven Rostedt
1 sibling, 1 reply; 18+ messages in thread
From: Masami Hiramatsu @ 2012-07-03 5:19 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Frederic Weisbecker, H. Peter Anvin, yrl.pp-manager.tt
(2012/07/03 5:03), Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
>
> Return as the 4th paramater to the function tracer callback the pt_regs.
>
> Later patches that implement regs passing for the architectures will require
> having the ftrace_ops set the SAVE_REGS flag, which will tell the arch
> to take the time to pass a full set of pt_regs to the ftrace_ops callback
> function. If the arch does not support it then it should pass NULL.
>
This looks good for me:)
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> If an arch can pass full regs, then it should define:
> ARCH_SUPPORTS_FTRACE_SAVE_REGS to 1
I just think this would better be commented on 5/6, since actual
arch-independent parts of that feature is not implemented yet on
this patch.(just interface is changed)
>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> include/linux/ftrace.h | 6 ++++--
> kernel/trace/ftrace.c | 37 ++++++++++++++++++++++---------------
> kernel/trace/trace_event_perf.c | 2 +-
> kernel/trace/trace_events.c | 2 +-
> kernel/trace/trace_functions.c | 7 ++++---
> kernel/trace/trace_irqsoff.c | 2 +-
> kernel/trace/trace_sched_wakeup.c | 3 ++-
> kernel/trace/trace_selftest.c | 15 ++++++++++-----
> kernel/trace/trace_stack.c | 3 ++-
> 9 files changed, 47 insertions(+), 30 deletions(-)
>
> diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
> index 3651fdc..e420288 100644
> --- a/include/linux/ftrace.h
> +++ b/include/linux/ftrace.h
> @@ -10,6 +10,7 @@
> #include <linux/kallsyms.h>
> #include <linux/linkage.h>
> #include <linux/bitops.h>
> +#include <linux/ptrace.h>
> #include <linux/ktime.h>
> #include <linux/sched.h>
> #include <linux/types.h>
> @@ -54,7 +55,7 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,
> struct ftrace_ops;
>
> typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *op);
> + struct ftrace_ops *op, struct pt_regs *regs);
>
> /*
> * FTRACE_OPS_FL_* bits denote the state of ftrace_ops struct and are
> @@ -188,7 +189,8 @@ static inline int ftrace_function_local_disabled(struct ftrace_ops *ops)
> return *this_cpu_ptr(ops->disabled);
> }
>
> -extern void ftrace_stub(unsigned long a0, unsigned long a1, struct ftrace_ops *op);
> +extern void ftrace_stub(unsigned long a0, unsigned long a1,
> + struct ftrace_ops *op, struct pt_regs *regs);
>
> #else /* !CONFIG_FUNCTION_TRACER */
> /*
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index 4cbca2e..6ff07ad 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -103,7 +103,7 @@ static struct ftrace_ops control_ops;
>
> #if ARCH_SUPPORTS_FTRACE_OPS
> static void ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *op);
> + struct ftrace_ops *op, struct pt_regs *regs);
> #else
> /* See comment below, where ftrace_ops_list_func is defined */
> static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip);
> @@ -121,7 +121,7 @@ static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip);
> */
> static void
> ftrace_global_list_func(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op, struct pt_regs *regs)
> {
> if (unlikely(trace_recursion_test(TRACE_GLOBAL_BIT)))
> return;
> @@ -129,19 +129,19 @@ ftrace_global_list_func(unsigned long ip, unsigned long parent_ip,
> trace_recursion_set(TRACE_GLOBAL_BIT);
> op = rcu_dereference_raw(ftrace_global_list); /*see above*/
> while (op != &ftrace_list_end) {
> - op->func(ip, parent_ip, op);
> + op->func(ip, parent_ip, op, regs);
> op = rcu_dereference_raw(op->next); /*see above*/
> };
> trace_recursion_clear(TRACE_GLOBAL_BIT);
> }
>
> static void ftrace_pid_func(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op, struct pt_regs *regs)
> {
> if (!test_tsk_trace_trace(current))
> return;
>
> - ftrace_pid_function(ip, parent_ip, op);
> + ftrace_pid_function(ip, parent_ip, op, regs);
> }
>
> static void set_ftrace_pid_function(ftrace_func_t func)
> @@ -763,7 +763,7 @@ ftrace_profile_alloc(struct ftrace_profile_stat *stat, unsigned long ip)
>
> static void
> function_profile_call(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *ops)
> + struct ftrace_ops *ops, struct pt_regs *regs)
> {
> struct ftrace_profile_stat *stat;
> struct ftrace_profile *rec;
> @@ -793,7 +793,7 @@ function_profile_call(unsigned long ip, unsigned long parent_ip,
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> static int profile_graph_entry(struct ftrace_graph_ent *trace)
> {
> - function_profile_call(trace->func, 0, NULL);
> + function_profile_call(trace->func, 0, NULL, NULL);
> return 1;
> }
>
> @@ -2771,7 +2771,7 @@ static int __init ftrace_mod_cmd_init(void)
> device_initcall(ftrace_mod_cmd_init);
>
> static void function_trace_probe_call(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op, struct pt_regs *pt_regs)
> {
> struct ftrace_func_probe *entry;
> struct hlist_head *hhd;
> @@ -3923,7 +3923,7 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip)
>
> static void
> ftrace_ops_control_func(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op, struct pt_regs *regs)
> {
> if (unlikely(trace_recursion_test(TRACE_CONTROL_BIT)))
> return;
> @@ -3938,7 +3938,7 @@ ftrace_ops_control_func(unsigned long ip, unsigned long parent_ip,
> while (op != &ftrace_list_end) {
> if (!ftrace_function_local_disabled(op) &&
> ftrace_ops_test(op, ip))
> - op->func(ip, parent_ip, op);
> + op->func(ip, parent_ip, op, regs);
>
> op = rcu_dereference_raw(op->next);
> };
> @@ -3952,7 +3952,7 @@ static struct ftrace_ops control_ops = {
>
> static inline void
> __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *ignored)
> + struct ftrace_ops *ignored, struct pt_regs *regs)
> {
> struct ftrace_ops *op;
>
> @@ -3971,7 +3971,7 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
> op = rcu_dereference_raw(ftrace_ops_list);
> while (op != &ftrace_list_end) {
> if (ftrace_ops_test(op, ip))
> - op->func(ip, parent_ip, op);
> + op->func(ip, parent_ip, op, regs);
> op = rcu_dereference_raw(op->next);
> };
> preempt_enable_notrace();
> @@ -3983,17 +3983,24 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
> * the list function ignores the op parameter, we do not want any
> * C side effects, where a function is called without the caller
> * sending a third parameter.
> + * Archs are to support both the regs and ftrace_ops at the same time.
> + * If they support ftrace_ops, it is assumed they support regs.
> + * If call backs want to use regs, they must either check for regs
> + * being NULL, or ARCH_SUPPORTS_FTRACE_SAVE_REGS.
> + * Note, ARCH_SUPPORT_SAVE_REGS expects a full regs to be saved.
> + * An architecture can pass partial regs with ftrace_ops and still
> + * set the ARCH_SUPPORT_FTARCE_OPS.
> */
> #if ARCH_SUPPORTS_FTRACE_OPS
> static void ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op, struct pt_regs *regs)
> {
> - __ftrace_ops_list_func(ip, parent_ip, NULL);
> + __ftrace_ops_list_func(ip, parent_ip, NULL, regs);
> }
> #else
> static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip)
> {
> - __ftrace_ops_list_func(ip, parent_ip, NULL);
> + __ftrace_ops_list_func(ip, parent_ip, NULL, NULL);
> }
> #endif
>
> diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
> index a872a9a..9824419 100644
> --- a/kernel/trace/trace_event_perf.c
> +++ b/kernel/trace/trace_event_perf.c
> @@ -259,7 +259,7 @@ EXPORT_SYMBOL_GPL(perf_trace_buf_prepare);
> #ifdef CONFIG_FUNCTION_TRACER
> static void
> perf_ftrace_function_call(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *ops)
> + struct ftrace_ops *ops, struct pt_regs *pt_regs)
> {
> struct ftrace_entry *entry;
> struct hlist_head *head;
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 88daa51..8c66968 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1682,7 +1682,7 @@ static DEFINE_PER_CPU(atomic_t, ftrace_test_event_disable);
>
> static void
> function_test_events_call(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op, struct pt_regs *pt_regs)
> {
> struct ring_buffer_event *event;
> struct ring_buffer *buffer;
> diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
> index fceb7a9..5675ebd 100644
> --- a/kernel/trace/trace_functions.c
> +++ b/kernel/trace/trace_functions.c
> @@ -49,7 +49,7 @@ static void function_trace_start(struct trace_array *tr)
>
> static void
> function_trace_call_preempt_only(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op, struct pt_regs *pt_regs)
> {
> struct trace_array *tr = func_trace;
> struct trace_array_cpu *data;
> @@ -77,7 +77,8 @@ function_trace_call_preempt_only(unsigned long ip, unsigned long parent_ip,
>
> static void
> function_trace_call(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op, struct pt_regs *pt_regs)
> +
> {
> struct trace_array *tr = func_trace;
> struct trace_array_cpu *data;
> @@ -109,7 +110,7 @@ function_trace_call(unsigned long ip, unsigned long parent_ip,
>
> static void
> function_stack_trace_call(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op, struct pt_regs *pt_regs)
> {
> struct trace_array *tr = func_trace;
> struct trace_array_cpu *data;
> diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
> index 2862c77..c7a9ba9 100644
> --- a/kernel/trace/trace_irqsoff.c
> +++ b/kernel/trace/trace_irqsoff.c
> @@ -137,7 +137,7 @@ static int func_prolog_dec(struct trace_array *tr,
> */
> static void
> irqsoff_tracer_call(unsigned long ip, unsigned long parent_ip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op, struct pt_regs *pt_regs)
> {
> struct trace_array *tr = irqsoff_trace;
> struct trace_array_cpu *data;
> diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
> index 0caf4f5..7547e36 100644
> --- a/kernel/trace/trace_sched_wakeup.c
> +++ b/kernel/trace/trace_sched_wakeup.c
> @@ -108,7 +108,8 @@ out_enable:
> * wakeup uses its own tracer function to keep the overhead down:
> */
> static void
> -wakeup_tracer_call(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op)
> +wakeup_tracer_call(unsigned long ip, unsigned long parent_ip,
> + struct ftrace_ops *op, struct pt_regs *pt_regs)
> {
> struct trace_array *tr = wakeup_trace;
> struct trace_array_cpu *data;
> diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
> index 9ae40c8..add37e0 100644
> --- a/kernel/trace/trace_selftest.c
> +++ b/kernel/trace/trace_selftest.c
> @@ -104,7 +104,8 @@ static inline void warn_failed_init_tracer(struct tracer *trace, int init_ret)
> static int trace_selftest_test_probe1_cnt;
> static void trace_selftest_test_probe1_func(unsigned long ip,
> unsigned long pip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op,
> + struct pt_regs *pt_regs)
> {
> trace_selftest_test_probe1_cnt++;
> }
> @@ -112,7 +113,8 @@ static void trace_selftest_test_probe1_func(unsigned long ip,
> static int trace_selftest_test_probe2_cnt;
> static void trace_selftest_test_probe2_func(unsigned long ip,
> unsigned long pip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op,
> + struct pt_regs *pt_regs)
> {
> trace_selftest_test_probe2_cnt++;
> }
> @@ -120,7 +122,8 @@ static void trace_selftest_test_probe2_func(unsigned long ip,
> static int trace_selftest_test_probe3_cnt;
> static void trace_selftest_test_probe3_func(unsigned long ip,
> unsigned long pip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op,
> + struct pt_regs *pt_regs)
> {
> trace_selftest_test_probe3_cnt++;
> }
> @@ -128,7 +131,8 @@ static void trace_selftest_test_probe3_func(unsigned long ip,
> static int trace_selftest_test_global_cnt;
> static void trace_selftest_test_global_func(unsigned long ip,
> unsigned long pip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op,
> + struct pt_regs *pt_regs)
> {
> trace_selftest_test_global_cnt++;
> }
> @@ -136,7 +140,8 @@ static void trace_selftest_test_global_func(unsigned long ip,
> static int trace_selftest_test_dyn_cnt;
> static void trace_selftest_test_dyn_func(unsigned long ip,
> unsigned long pip,
> - struct ftrace_ops *op)
> + struct ftrace_ops *op,
> + struct pt_regs *pt_regs)
> {
> trace_selftest_test_dyn_cnt++;
> }
> diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
> index e20006d..2fa5328 100644
> --- a/kernel/trace/trace_stack.c
> +++ b/kernel/trace/trace_stack.c
> @@ -111,7 +111,8 @@ static inline void check_stack(void)
> }
>
> static void
> -stack_trace_call(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op)
> +stack_trace_call(unsigned long ip, unsigned long parent_ip,
> + struct ftrace_ops *op, struct pt_regs *pt_regs)
> {
> int cpu;
>
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] ftrace/x86: Add save_regs for i386 function calls
2012-07-02 20:03 ` [PATCH 6/6] ftrace/x86: Add save_regs for i386 function calls Steven Rostedt
@ 2012-07-03 5:27 ` Masami Hiramatsu
2012-07-03 11:56 ` Steven Rostedt
0 siblings, 1 reply; 18+ messages in thread
From: Masami Hiramatsu @ 2012-07-03 5:27 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Frederic Weisbecker, H. Peter Anvin
(2012/07/03 5:03), Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
>
> Add saving full regs for function tracing on i386.
> The saving of regs was influenced by patches sent out by
> Masami Hiramatsu.
>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
> ---
> arch/x86/include/asm/ftrace.h | 2 --
> arch/x86/kernel/entry_32.S | 53 +++++++++++++++++++++++++++++++++++++++++
> arch/x86/kernel/ftrace.c | 4 ----
> 3 files changed, 53 insertions(+), 6 deletions(-)
>
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index fd10faf..2b396cf 100644
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -40,10 +40,8 @@
>
> #ifdef CONFIG_DYNAMIC_FTRACE
> #define ARCH_SUPPORTS_FTRACE_OPS 1
> -#ifdef CONFIG_X86_64
> #define ARCH_SUPPORTS_FTRACE_SAVE_REGS
> #endif
> -#endif
>
> #ifndef __ASSEMBLY__
> extern void mcount(void);
> diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
> index acd4963..e18ea33 100644
> --- a/arch/x86/kernel/entry_32.S
> +++ b/arch/x86/kernel/entry_32.S
> @@ -1123,6 +1123,7 @@ ftrace_call:
> popl %edx
> popl %ecx
> popl %eax
> +ftrace_ret:
> #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> .globl ftrace_graph_call
> ftrace_graph_call:
> @@ -1134,6 +1135,58 @@ ftrace_stub:
> ret
> END(ftrace_caller)
>
> +ENTRY(ftrace_regs_caller)
> + pushf /* push flags before compare */
> + cmpl $0, function_trace_stop
> + jne ftrace_exit
> +
> + subl $8, %esp /* skip ip and orig_ax */
> + pushl %gs
> + pushl %fs
> + pushl %es
> + pushl %ds
> + pushl %eax
> + pushl %ebp
> + pushl %edi
> + pushl %esi
> + pushl %edx
> + pushl %ecx
> + pushl %ebx
> + movl 14*4(%esp), %eax /* Load return address */
> + pushl %eax /* Save return address (+4) */
> + subl $MCOUNT_INSN_SIZE, %eax
> + movl %eax, 12*4+4(%esp) /* Store IP */
> + movl 13*4+4(%esp), %edx /* Load flags */
> + movl %edx, 14*4+4(%esp) /* Store flags */
> + movl $__KERNEL_CS, %edx
> + movl %edx, 13*4+4(%esp) /* Store CS */
> +
> + movl 0x4(%ebp), %edx
> + lea 4(%esp), %ecx
> + pushl %ecx /* Save pt_regs as 4th parameter */
> + leal function_trace_op, %ecx
> +
> +GLOBAL(ftrace_regs_call)
> + call ftrace_stub
> +
> + addl $4,%esp /* Skip pt_regs */
Hmm, you need to restoring flags here, because
original flags is clobbered by KERNEL_CS.
movl 14*4+4(%esp), %edx
movl %edx, 13*4+4(%esp)
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] ftrace/x86: Add separate function to save regs
2012-07-02 20:03 ` [PATCH 5/6] ftrace/x86: Add separate function to save regs Steven Rostedt
@ 2012-07-03 8:29 ` Masami Hiramatsu
2012-07-11 16:22 ` Steven Rostedt
2012-07-03 16:54 ` Alexander van Heukelum
1 sibling, 1 reply; 18+ messages in thread
From: Masami Hiramatsu @ 2012-07-03 8:29 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Frederic Weisbecker, H. Peter Anvin, yrl.pp-manager.tt
(2012/07/03 5:03), Steven Rostedt wrote:
> From: Steven Rostedt <srostedt@redhat.com>
>
> Add a way to have different functions calling different trampolines.
> If a ftrace_ops wants regs saved on the return, then have only the
> functions with ops registered to save regs. Functions registered by
> other ops would not be affected, unless the functions overlap.
>
> If one ftrace_ops registered functions A, B and C and another ops
> registered fucntions to save regs on A, and D, then only functions
> A and D would be saving regs. Function B and C would work as normal.
> Although A is registered by both ops: normal and saves regs; this is fine
> as saving the regs is needed to satisfy one of the ops that calls it
> but the regs are ignored by the other ops function.
>
> x86_64 implements the full regs saving, and i386 just passes a NULL
> for regs to satisfy the ftrace_ops passing. Where an arch must supply
> both regs and ftrace_ops parameters, even if regs is just NULL.
>
> It is OK for an arch to pass NULL regs. All function trace users that
> require regs passing must add the flag FTRACE_OPS_FL_SAVE_REGS when
> registering the ftrace_ops. If the arch does not support saving regs
> then the ftrace_ops will fail to register. The flag
> FTRACE_OPS_FL_SAVE_REGS_IF_SUPPORTED may be set that will prevent the
> ftrace_ops from failing to register. In this case, the handler may
> either check if regs is not NULL or check if ARCH_SUPPORTS_FTRACE_SAVE_REGS.
> If the arch supports passing regs it will set this macro and pass regs
> for ops that request them. All other archs will just pass NULL.
This is nice for me :)
It helps me to maintain kprobes on ftrace.
[...]
> @@ -98,6 +111,67 @@ GLOBAL(ftrace_stub)
> retq
> END(ftrace_caller)
>
> +ENTRY(ftrace_regs_caller)
> + /* Save the current flags before compare (in SS location)*/
> + pushfq
> +
> + /* Check if tracing was disabled (quick check) */
> + cmpl $0, function_trace_stop
> + jne ftrace_restore_flags
> +
> + /* skip=8 to skip flags saved in SS */
> + ftrace_caller_setup 8
> +
> + /* Save the rest of pt_regs */
> + movq %r15, R15(%rsp)
> + movq %r14, R14(%rsp)
> + movq %r13, R13(%rsp)
> + movq %r12, R12(%rsp)
> + movq %r11, R11(%rsp)
> + movq %r10, R10(%rsp)
> + movq %rbp, RBP(%rsp)
> + movq %rbx, RBX(%rsp)
> + /* Copy saved flags */
> + movq SS(%rsp), %rcx
> + movq %rcx, EFLAGS(%rsp)
> + /* Kernel segments */
> + movq $__KERNEL_DS, %rcx
> + movq %rcx, SS(%rsp)
> + movq $__KERNEL_CS, %rcx
> + movq %rcx, CS(%rsp)
> + /* Stack - skipping return address */
> + leaq SS+16(%rsp), %rcx
> + movq %rcx, RSP(%rsp)
> +
> + /* regs go into 4th parameter */
> + leaq (%rsp), %rcx
> +
> +GLOBAL(ftrace_regs_call)
> + call ftrace_stub
> +
> + /* restore the rest of pt_regs */
> + movq R15(%rsp), %r15
> + movq R14(%rsp), %r14
> + movq R13(%rsp), %r13
> + movq R12(%rsp), %r12
> + movq R10(%rsp), %r10
> + movq RBP(%rsp), %rbp
> + movq RBX(%rsp), %rbx
> +
> + /* Restore flags */
> + pushq EFLAGS(%rsp)
> + popfq
> +
> + MCOUNT_RESTORE_FRAME
Here, if MCOUNT_RESTORE_FRAME has skip too, I think you don't
need to restore flags before restoring other registers, like
below;
MCOUNT_RESTORE_FRAME 8
popfq
And also, this will prevent to modify flags before return by
addq in MCOUNT_RESTORE_FRAME.
> +
> + jmp ftrace_return
> +ftrace_restore_flags:
> + popfq
> + jmp ftrace_stub
> +
> +END(ftrace_regs_caller)
> +
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 6/6] ftrace/x86: Add save_regs for i386 function calls
2012-07-03 5:27 ` Masami Hiramatsu
@ 2012-07-03 11:56 ` Steven Rostedt
0 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2012-07-03 11:56 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Frederic Weisbecker, H. Peter Anvin
On Tue, 2012-07-03 at 14:27 +0900, Masami Hiramatsu wrote:
> (2012/07/03 5:03), Steven Rostedt wrote:
> > +GLOBAL(ftrace_regs_call)
> > + call ftrace_stub
> > +
> > + addl $4,%esp /* Skip pt_regs */
>
> Hmm, you need to restoring flags here, because
> original flags is clobbered by KERNEL_CS.
>
> movl 14*4+4(%esp), %edx
> movl %edx, 13*4+4(%esp)
Ah, you're right. I'll update it. I need to make changes anyway, because
I found that if I have a kprobe using ftrace and enable function graph
tracer, the machine triple faults. I'm working on fixing that now.
Thanks!
-- Steve
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] ftrace/x86: Add separate function to save regs
2012-07-02 20:03 ` [PATCH 5/6] ftrace/x86: Add separate function to save regs Steven Rostedt
2012-07-03 8:29 ` Masami Hiramatsu
@ 2012-07-03 16:54 ` Alexander van Heukelum
2012-07-05 20:37 ` Steven Rostedt
1 sibling, 1 reply; 18+ messages in thread
From: Alexander van Heukelum @ 2012-07-03 16:54 UTC (permalink / raw)
To: Steven Rostedt, linux-kernel
Cc: Ingo Molnar, Andrew Morton, Thomas Gleixner, Frederic Weisbecker,
Masami Hiramatsu, H. Peter Anvin
On Mon, Jul 2, 2012, at 16:03, Steven Rostedt wrote:
> Email had 1 attachment:
> + 0005-ftrace-x86-Add-separate-function-to-save-regs.patch
> 23k (text/plain)
Hi Steven,
One thing that caught my eye...
> diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
> index e3e17a0..acd4963 100644
> --- a/arch/x86/kernel/entry_32.S
> +++ b/arch/x86/kernel/entry_32.S
> @@ -1109,6 +1109,7 @@ ENTRY(ftrace_caller)
> pushl %eax
> pushl %ecx
> pushl %edx
> + pushl $0 /* Pass NULL as regs pointer */
> movl 0xc(%esp), %eax
This should now be changed to "0x10(%esp)", right?
Greetings,
Alexander
> movl 0x4(%ebp), %edx
> leal function_trace_op, %ecx
> @@ -1118,6 +1119,7 @@ ENTRY(ftrace_caller)
> ftrace_call:
> call ftrace_stub
>
> + addl $4,%esp /* skip NULL pointer */
> popl %edx
> popl %ecx
> popl %eax
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] ftrace/x86: Add separate function to save regs
2012-07-03 16:54 ` Alexander van Heukelum
@ 2012-07-05 20:37 ` Steven Rostedt
0 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2012-07-05 20:37 UTC (permalink / raw)
To: Alexander van Heukelum
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Frederic Weisbecker, Masami Hiramatsu, H. Peter Anvin
On Tue, 2012-07-03 at 18:54 +0200, Alexander van Heukelum wrote:
> On Mon, Jul 2, 2012, at 16:03, Steven Rostedt wrote:
> > Email had 1 attachment:
> > + 0005-ftrace-x86-Add-separate-function-to-save-regs.patch
> > 23k (text/plain)
>
> Hi Steven,
>
> One thing that caught my eye...
>
> > diff --git a/arch/x86/kernel/entry_32.S b/arch/x86/kernel/entry_32.S
> > index e3e17a0..acd4963 100644
> > --- a/arch/x86/kernel/entry_32.S
> > +++ b/arch/x86/kernel/entry_32.S
> > @@ -1109,6 +1109,7 @@ ENTRY(ftrace_caller)
> > pushl %eax
> > pushl %ecx
> > pushl %edx
> > + pushl $0 /* Pass NULL as regs pointer */
> > movl 0xc(%esp), %eax
>
> This should now be changed to "0x10(%esp)", right?
Yeah, I think you're right. Actually, I think I originally had it as:
movl 0xc(%esp), %eax
+ pushl $0
But after a few other changes, it went to this. I haven't finished
testing against x86_64, and haven't started the i386 tests. I'm sure
this would have blown up then ;-)
Thanks,
-- Steve
>
> Greetings,
> Alexander
>
> > movl 0x4(%ebp), %edx
> > leal function_trace_op, %ecx
> > @@ -1118,6 +1119,7 @@ ENTRY(ftrace_caller)
> > ftrace_call:
> > call ftrace_stub
> >
> > + addl $4,%esp /* skip NULL pointer */
> > popl %edx
> > popl %ecx
> > popl %eax
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 3/6] ftrace: Return pt_regs to function trace callback
2012-07-03 5:19 ` Masami Hiramatsu
@ 2012-07-11 15:28 ` Steven Rostedt
0 siblings, 0 replies; 18+ messages in thread
From: Steven Rostedt @ 2012-07-11 15:28 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Frederic Weisbecker, H. Peter Anvin, yrl.pp-manager.tt
On Tue, 2012-07-03 at 14:19 +0900, Masami Hiramatsu wrote:
> (2012/07/03 5:03), Steven Rostedt wrote:
> > From: Steven Rostedt <srostedt@redhat.com>
> >
> > Return as the 4th paramater to the function tracer callback the pt_regs.
> >
> > Later patches that implement regs passing for the architectures will require
> > having the ftrace_ops set the SAVE_REGS flag, which will tell the arch
> > to take the time to pass a full set of pt_regs to the ftrace_ops callback
> > function. If the arch does not support it then it should pass NULL.
> >
>
> This looks good for me:)
>
> Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>
> > If an arch can pass full regs, then it should define:
> > ARCH_SUPPORTS_FTRACE_SAVE_REGS to 1
>
> I just think this would better be commented on 5/6, since actual
> arch-independent parts of that feature is not implemented yet on
> this patch.(just interface is changed)
>
Sure, but this variable is referenced in this patch. Yes, no arch code
is implemented, but documenting that it needs to be set for this change
here does help. Other arch developers that look at these changes and
want to know about it can see this change and investigate further.
But this is just the change log, I'm assuming the patch is fine as you
gave your reviewed by tag.
Thanks,
-- Steve
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] ftrace/x86: Add separate function to save regs
2012-07-03 8:29 ` Masami Hiramatsu
@ 2012-07-11 16:22 ` Steven Rostedt
2012-07-11 16:28 ` Steven Rostedt
0 siblings, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2012-07-11 16:22 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Frederic Weisbecker, H. Peter Anvin, yrl.pp-manager.tt
On Tue, 2012-07-03 at 17:29 +0900, Masami Hiramatsu wrote:
>
> > + /* Restore flags */
> > + pushq EFLAGS(%rsp)
> > + popfq
> > +
> > + MCOUNT_RESTORE_FRAME
>
> Here, if MCOUNT_RESTORE_FRAME has skip too, I think you don't
> need to restore flags before restoring other registers, like
> below;
>
> MCOUNT_RESTORE_FRAME 8
> popfq
>
> And also, this will prevent to modify flags before return by
> addq in MCOUNT_RESTORE_FRAME.
Ah, because the addq will modify flags :-/
Grumble, I guess I should implement this, even though it will make it a
little more complex. I thought it was better to restore flags
explicitly, but that's not the case.
OK, I'll make the update. As MCOUNT_SAVE_FRAME has the skip, I guess
it's not so bad to have restore have it too.
-- Steve
>
> > +
> > + jmp ftrace_return
> > +ftrace_restore_flags:
> > + popfq
> > + jmp ftrace_stub
> > +
> > +END(ftrace_regs_caller)
> > +
>
> Thank you,
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] ftrace/x86: Add separate function to save regs
2012-07-11 16:22 ` Steven Rostedt
@ 2012-07-11 16:28 ` Steven Rostedt
2012-07-12 2:08 ` Masami Hiramatsu
0 siblings, 1 reply; 18+ messages in thread
From: Steven Rostedt @ 2012-07-11 16:28 UTC (permalink / raw)
To: Masami Hiramatsu
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Frederic Weisbecker, H. Peter Anvin, yrl.pp-manager.tt
On Wed, 2012-07-11 at 12:22 -0400, Steven Rostedt wrote:
> On Tue, 2012-07-03 at 17:29 +0900, Masami Hiramatsu wrote:
>
>
> >
> > > + /* Restore flags */
> > > + pushq EFLAGS(%rsp)
> > > + popfq
> > > +
> > > + MCOUNT_RESTORE_FRAME
> >
> > Here, if MCOUNT_RESTORE_FRAME has skip too, I think you don't
> > need to restore flags before restoring other registers, like
> > below;
> >
> > MCOUNT_RESTORE_FRAME 8
> > popfq
> >
> > And also, this will prevent to modify flags before return by
> > addq in MCOUNT_RESTORE_FRAME.
>
> Ah, because the addq will modify flags :-/
>
> Grumble, I guess I should implement this, even though it will make it a
> little more complex. I thought it was better to restore flags
> explicitly, but that's not the case.
>
I know why I did this. Do you want kprobes to be able to modify flags?
If so, then I need to add, before the restore:
movq EFLAGS(%rsp), %rax
movq %rax, SS(%rsp)
-- Steve
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 5/6] ftrace/x86: Add separate function to save regs
2012-07-11 16:28 ` Steven Rostedt
@ 2012-07-12 2:08 ` Masami Hiramatsu
0 siblings, 0 replies; 18+ messages in thread
From: Masami Hiramatsu @ 2012-07-12 2:08 UTC (permalink / raw)
To: Steven Rostedt
Cc: linux-kernel, Ingo Molnar, Andrew Morton, Thomas Gleixner,
Frederic Weisbecker, H. Peter Anvin, yrl.pp-manager.tt
(2012/07/12 1:28), Steven Rostedt wrote:
> On Wed, 2012-07-11 at 12:22 -0400, Steven Rostedt wrote:
>> On Tue, 2012-07-03 at 17:29 +0900, Masami Hiramatsu wrote:
>>
>>
>>>
>>>> + /* Restore flags */
>>>> + pushq EFLAGS(%rsp)
>>>> + popfq
>>>> +
>>>> + MCOUNT_RESTORE_FRAME
>>>
>>> Here, if MCOUNT_RESTORE_FRAME has skip too, I think you don't
>>> need to restore flags before restoring other registers, like
>>> below;
>>>
>>> MCOUNT_RESTORE_FRAME 8
>>> popfq
>>>
>>> And also, this will prevent to modify flags before return by
>>> addq in MCOUNT_RESTORE_FRAME.
>>
>> Ah, because the addq will modify flags :-/
>>
>> Grumble, I guess I should implement this, even though it will make it a
>> little more complex. I thought it was better to restore flags
>> explicitly, but that's not the case.
>>
>
> I know why I did this. Do you want kprobes to be able to modify flags?
> If so, then I need to add, before the restore:
>
> movq EFLAGS(%rsp), %rax
> movq %rax, SS(%rsp)
Yes, kprobes might be used for modifying flags, so, please :)
Thank you,
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 18+ messages in thread
* [tip:perf/core] ftrace: Return pt_regs to function trace callback
2012-07-02 20:03 ` [PATCH 3/6] ftrace: Return pt_regs to function trace callback Steven Rostedt
2012-07-03 5:19 ` Masami Hiramatsu
@ 2012-08-21 15:00 ` tip-bot for Steven Rostedt
1 sibling, 0 replies; 18+ messages in thread
From: tip-bot for Steven Rostedt @ 2012-08-21 15:00 UTC (permalink / raw)
To: linux-tip-commits
Cc: linux-kernel, hpa, mingo, masami.hiramatsu.pt, rostedt, srostedt, tglx
Commit-ID: a1e2e31d175a1349274eba3465d17616c6725f8c
Gitweb: http://git.kernel.org/tip/a1e2e31d175a1349274eba3465d17616c6725f8c
Author: Steven Rostedt <srostedt@redhat.com>
AuthorDate: Tue, 9 Aug 2011 12:50:46 -0400
Committer: Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 19 Jul 2012 13:18:49 -0400
ftrace: Return pt_regs to function trace callback
Return as the 4th paramater to the function tracer callback the pt_regs.
Later patches that implement regs passing for the architectures will require
having the ftrace_ops set the SAVE_REGS flag, which will tell the arch
to take the time to pass a full set of pt_regs to the ftrace_ops callback
function. If the arch does not support it then it should pass NULL.
If an arch can pass full regs, then it should define:
ARCH_SUPPORTS_FTRACE_SAVE_REGS to 1
Link: http://lkml.kernel.org/r/20120702201821.019966811@goodmis.org
Reviewed-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/ftrace.h | 6 ++++--
kernel/trace/ftrace.c | 37 ++++++++++++++++++++++---------------
kernel/trace/trace_event_perf.c | 2 +-
kernel/trace/trace_events.c | 2 +-
kernel/trace/trace_functions.c | 7 ++++---
kernel/trace/trace_irqsoff.c | 2 +-
kernel/trace/trace_sched_wakeup.c | 3 ++-
kernel/trace/trace_selftest.c | 15 ++++++++++-----
kernel/trace/trace_stack.c | 3 ++-
9 files changed, 47 insertions(+), 30 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 3651fdc..e420288 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -10,6 +10,7 @@
#include <linux/kallsyms.h>
#include <linux/linkage.h>
#include <linux/bitops.h>
+#include <linux/ptrace.h>
#include <linux/ktime.h>
#include <linux/sched.h>
#include <linux/types.h>
@@ -54,7 +55,7 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,
struct ftrace_ops;
typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op);
+ struct ftrace_ops *op, struct pt_regs *regs);
/*
* FTRACE_OPS_FL_* bits denote the state of ftrace_ops struct and are
@@ -188,7 +189,8 @@ static inline int ftrace_function_local_disabled(struct ftrace_ops *ops)
return *this_cpu_ptr(ops->disabled);
}
-extern void ftrace_stub(unsigned long a0, unsigned long a1, struct ftrace_ops *op);
+extern void ftrace_stub(unsigned long a0, unsigned long a1,
+ struct ftrace_ops *op, struct pt_regs *regs);
#else /* !CONFIG_FUNCTION_TRACER */
/*
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 4cbca2e..6ff07ad 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -103,7 +103,7 @@ static struct ftrace_ops control_ops;
#if ARCH_SUPPORTS_FTRACE_OPS
static void ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op);
+ struct ftrace_ops *op, struct pt_regs *regs);
#else
/* See comment below, where ftrace_ops_list_func is defined */
static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip);
@@ -121,7 +121,7 @@ static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip);
*/
static void
ftrace_global_list_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *regs)
{
if (unlikely(trace_recursion_test(TRACE_GLOBAL_BIT)))
return;
@@ -129,19 +129,19 @@ ftrace_global_list_func(unsigned long ip, unsigned long parent_ip,
trace_recursion_set(TRACE_GLOBAL_BIT);
op = rcu_dereference_raw(ftrace_global_list); /*see above*/
while (op != &ftrace_list_end) {
- op->func(ip, parent_ip, op);
+ op->func(ip, parent_ip, op, regs);
op = rcu_dereference_raw(op->next); /*see above*/
};
trace_recursion_clear(TRACE_GLOBAL_BIT);
}
static void ftrace_pid_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *regs)
{
if (!test_tsk_trace_trace(current))
return;
- ftrace_pid_function(ip, parent_ip, op);
+ ftrace_pid_function(ip, parent_ip, op, regs);
}
static void set_ftrace_pid_function(ftrace_func_t func)
@@ -763,7 +763,7 @@ ftrace_profile_alloc(struct ftrace_profile_stat *stat, unsigned long ip)
static void
function_profile_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *ops)
+ struct ftrace_ops *ops, struct pt_regs *regs)
{
struct ftrace_profile_stat *stat;
struct ftrace_profile *rec;
@@ -793,7 +793,7 @@ function_profile_call(unsigned long ip, unsigned long parent_ip,
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
static int profile_graph_entry(struct ftrace_graph_ent *trace)
{
- function_profile_call(trace->func, 0, NULL);
+ function_profile_call(trace->func, 0, NULL, NULL);
return 1;
}
@@ -2771,7 +2771,7 @@ static int __init ftrace_mod_cmd_init(void)
device_initcall(ftrace_mod_cmd_init);
static void function_trace_probe_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
struct ftrace_func_probe *entry;
struct hlist_head *hhd;
@@ -3923,7 +3923,7 @@ ftrace_ops_test(struct ftrace_ops *ops, unsigned long ip)
static void
ftrace_ops_control_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *regs)
{
if (unlikely(trace_recursion_test(TRACE_CONTROL_BIT)))
return;
@@ -3938,7 +3938,7 @@ ftrace_ops_control_func(unsigned long ip, unsigned long parent_ip,
while (op != &ftrace_list_end) {
if (!ftrace_function_local_disabled(op) &&
ftrace_ops_test(op, ip))
- op->func(ip, parent_ip, op);
+ op->func(ip, parent_ip, op, regs);
op = rcu_dereference_raw(op->next);
};
@@ -3952,7 +3952,7 @@ static struct ftrace_ops control_ops = {
static inline void
__ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *ignored)
+ struct ftrace_ops *ignored, struct pt_regs *regs)
{
struct ftrace_ops *op;
@@ -3971,7 +3971,7 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
op = rcu_dereference_raw(ftrace_ops_list);
while (op != &ftrace_list_end) {
if (ftrace_ops_test(op, ip))
- op->func(ip, parent_ip, op);
+ op->func(ip, parent_ip, op, regs);
op = rcu_dereference_raw(op->next);
};
preempt_enable_notrace();
@@ -3983,17 +3983,24 @@ __ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
* the list function ignores the op parameter, we do not want any
* C side effects, where a function is called without the caller
* sending a third parameter.
+ * Archs are to support both the regs and ftrace_ops at the same time.
+ * If they support ftrace_ops, it is assumed they support regs.
+ * If call backs want to use regs, they must either check for regs
+ * being NULL, or ARCH_SUPPORTS_FTRACE_SAVE_REGS.
+ * Note, ARCH_SUPPORT_SAVE_REGS expects a full regs to be saved.
+ * An architecture can pass partial regs with ftrace_ops and still
+ * set the ARCH_SUPPORT_FTARCE_OPS.
*/
#if ARCH_SUPPORTS_FTRACE_OPS
static void ftrace_ops_list_func(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *regs)
{
- __ftrace_ops_list_func(ip, parent_ip, NULL);
+ __ftrace_ops_list_func(ip, parent_ip, NULL, regs);
}
#else
static void ftrace_ops_no_ops(unsigned long ip, unsigned long parent_ip)
{
- __ftrace_ops_list_func(ip, parent_ip, NULL);
+ __ftrace_ops_list_func(ip, parent_ip, NULL, NULL);
}
#endif
diff --git a/kernel/trace/trace_event_perf.c b/kernel/trace/trace_event_perf.c
index a872a9a..9824419 100644
--- a/kernel/trace/trace_event_perf.c
+++ b/kernel/trace/trace_event_perf.c
@@ -259,7 +259,7 @@ EXPORT_SYMBOL_GPL(perf_trace_buf_prepare);
#ifdef CONFIG_FUNCTION_TRACER
static void
perf_ftrace_function_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *ops)
+ struct ftrace_ops *ops, struct pt_regs *pt_regs)
{
struct ftrace_entry *entry;
struct hlist_head *head;
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 88daa51..8c66968 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -1682,7 +1682,7 @@ static DEFINE_PER_CPU(atomic_t, ftrace_test_event_disable);
static void
function_test_events_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
struct ring_buffer_event *event;
struct ring_buffer *buffer;
diff --git a/kernel/trace/trace_functions.c b/kernel/trace/trace_functions.c
index fceb7a9..5675ebd 100644
--- a/kernel/trace/trace_functions.c
+++ b/kernel/trace/trace_functions.c
@@ -49,7 +49,7 @@ static void function_trace_start(struct trace_array *tr)
static void
function_trace_call_preempt_only(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
struct trace_array *tr = func_trace;
struct trace_array_cpu *data;
@@ -77,7 +77,8 @@ function_trace_call_preempt_only(unsigned long ip, unsigned long parent_ip,
static void
function_trace_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
+
{
struct trace_array *tr = func_trace;
struct trace_array_cpu *data;
@@ -109,7 +110,7 @@ function_trace_call(unsigned long ip, unsigned long parent_ip,
static void
function_stack_trace_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
struct trace_array *tr = func_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_irqsoff.c b/kernel/trace/trace_irqsoff.c
index 2862c77..c7a9ba9 100644
--- a/kernel/trace/trace_irqsoff.c
+++ b/kernel/trace/trace_irqsoff.c
@@ -137,7 +137,7 @@ static int func_prolog_dec(struct trace_array *tr,
*/
static void
irqsoff_tracer_call(unsigned long ip, unsigned long parent_ip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
struct trace_array *tr = irqsoff_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_sched_wakeup.c b/kernel/trace/trace_sched_wakeup.c
index 0caf4f5..7547e36 100644
--- a/kernel/trace/trace_sched_wakeup.c
+++ b/kernel/trace/trace_sched_wakeup.c
@@ -108,7 +108,8 @@ out_enable:
* wakeup uses its own tracer function to keep the overhead down:
*/
static void
-wakeup_tracer_call(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op)
+wakeup_tracer_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
struct trace_array *tr = wakeup_trace;
struct trace_array_cpu *data;
diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c
index 9ae40c8..add37e0 100644
--- a/kernel/trace/trace_selftest.c
+++ b/kernel/trace/trace_selftest.c
@@ -104,7 +104,8 @@ static inline void warn_failed_init_tracer(struct tracer *trace, int init_ret)
static int trace_selftest_test_probe1_cnt;
static void trace_selftest_test_probe1_func(unsigned long ip,
unsigned long pip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op,
+ struct pt_regs *pt_regs)
{
trace_selftest_test_probe1_cnt++;
}
@@ -112,7 +113,8 @@ static void trace_selftest_test_probe1_func(unsigned long ip,
static int trace_selftest_test_probe2_cnt;
static void trace_selftest_test_probe2_func(unsigned long ip,
unsigned long pip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op,
+ struct pt_regs *pt_regs)
{
trace_selftest_test_probe2_cnt++;
}
@@ -120,7 +122,8 @@ static void trace_selftest_test_probe2_func(unsigned long ip,
static int trace_selftest_test_probe3_cnt;
static void trace_selftest_test_probe3_func(unsigned long ip,
unsigned long pip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op,
+ struct pt_regs *pt_regs)
{
trace_selftest_test_probe3_cnt++;
}
@@ -128,7 +131,8 @@ static void trace_selftest_test_probe3_func(unsigned long ip,
static int trace_selftest_test_global_cnt;
static void trace_selftest_test_global_func(unsigned long ip,
unsigned long pip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op,
+ struct pt_regs *pt_regs)
{
trace_selftest_test_global_cnt++;
}
@@ -136,7 +140,8 @@ static void trace_selftest_test_global_func(unsigned long ip,
static int trace_selftest_test_dyn_cnt;
static void trace_selftest_test_dyn_func(unsigned long ip,
unsigned long pip,
- struct ftrace_ops *op)
+ struct ftrace_ops *op,
+ struct pt_regs *pt_regs)
{
trace_selftest_test_dyn_cnt++;
}
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index e20006d..2fa5328 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -111,7 +111,8 @@ static inline void check_stack(void)
}
static void
-stack_trace_call(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op)
+stack_trace_call(unsigned long ip, unsigned long parent_ip,
+ struct ftrace_ops *op, struct pt_regs *pt_regs)
{
int cpu;
^ permalink raw reply related [flat|nested] 18+ messages in thread
end of thread, other threads:[~2012-08-21 15:06 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-07-02 20:03 [PATCH 0/6] [RFC v3] ftrace/kprobes: Setting up ftrace for kprobes Steven Rostedt
2012-07-02 20:03 ` [PATCH 1/6] ftrace: Pass ftrace_ops as third parameter to function trace callback Steven Rostedt
2012-07-02 20:03 ` [PATCH 2/6] ftrace: Consolidate arch dependent functions with list function Steven Rostedt
2012-07-02 20:03 ` [PATCH 3/6] ftrace: Return pt_regs to function trace callback Steven Rostedt
2012-07-03 5:19 ` Masami Hiramatsu
2012-07-11 15:28 ` Steven Rostedt
2012-08-21 15:00 ` [tip:perf/core] " tip-bot for Steven Rostedt
2012-07-02 20:03 ` [PATCH 4/6] ftrace/x86_32: Push ftrace_ops in as 3rd parameter to function tracer Steven Rostedt
2012-07-02 20:03 ` [PATCH 5/6] ftrace/x86: Add separate function to save regs Steven Rostedt
2012-07-03 8:29 ` Masami Hiramatsu
2012-07-11 16:22 ` Steven Rostedt
2012-07-11 16:28 ` Steven Rostedt
2012-07-12 2:08 ` Masami Hiramatsu
2012-07-03 16:54 ` Alexander van Heukelum
2012-07-05 20:37 ` Steven Rostedt
2012-07-02 20:03 ` [PATCH 6/6] ftrace/x86: Add save_regs for i386 function calls Steven Rostedt
2012-07-03 5:27 ` Masami Hiramatsu
2012-07-03 11:56 ` Steven Rostedt
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).