kprobes/ftrace: Making ftrace usable for kprobes
Steven Rostedt
maintainerv2 cover kprobes/ftrace: Making ftrace usable for kprobes
This is an RFC of patches that allow ftrace to be used directly by kprobes.
The first set of patches modify the function tracer to:
1) have ftrace_ops passed to all functions
2) allow regs to be passed to all functions when requested
The first change is not required by kprobes, but is a nice feature that people have asked for. You can now pass data to different functions. Well, you can pass the ftrace_ops that registered the function such that if two ftrace_ops register the same callback, the call back can do different things depending on what registered it.
The second change adds a second mcount trampoline. That is, if you request to have pt_regs returned, it will call a different function that saves those registers. If nothing asks for regs, then the old way is performed, and there's no slow down in the performance, as not many functions would ever require regs passed to it.
If an arch supports passing of ftrace ops, it must also pass regs. But it does not need to support passing regs. By default an arch can just pass NULL. If it supports regs, then it can allow tools like kprobes to ask for regs. Otherwise the regs parameter should just be NULL.
The second set is Masami's patches ported on top of these changes.
This is v2, and I've cleaned the patches up a bit and added a lot more comments. This is a more serious RFC as this is the current way I plan on pushing it to mainline. But I'm posting first in case someone spots something that I missed. I'll also be doing more testing on it and if someting comes up I'll obviously will fix it before pushing.
Some change since v1:
Added fix for undefined ftrace_location() when compiling with !DYNAMIC_FTRACE.
Added missing regs passed for x86_64 (segment regs, and r11).
Saved flags for when regs are being saved. The cmp in the mcount trampoline that checks if function tracing is disabled will save flags before the compare, and restore flags before returning back to the function.
Decided that passing partial regs is a bad idea. Either the arch should pass all regs or NULL. Even though partial regs are saved for calling the function, it does't make sense to give that to the callback. By making regs be full pt_regs or NULL, then the callbacks can simply check if regs is NULL and if it isn't it can do more functionality, but if it is NULL then it just does limited work. Having 'partial regs' would just complicate that for no good reason. If in the future we want partial regs, we can add another flag to request it.
Note, code can still check if ARCH_SUPPORTS_FTRACE_SAVE_REGS is defined, and if it is, if the ftrace_ops specifies the SAVE_REGS flag, then the arch will pass the full pt_regs to it.
These patches are in git and can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
rfc/kprobes/ftrace-v5
Head SHA1: 3cbbedd93e39d1394251825775d913be10d16e4e
Masami Hiramatsu (5):
ftrace: add ftrace_set_filter_ip() for address based filter
kprobes: cleanup to separate probe-able check
kprobes: Move locks into appropriate functions
kprobes: introduce ftrace based optimization kprobes/x86: ftrace based optimization for x86
Steven Rostedt (8):
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
kprobes: Inverse taking of module_mutex with kprobe_mutex
ftrace: Make ftrace_location() a nop on !DYNAMIC_FTRACE
----
arch/x86/include/asm/ftrace.h | 47 ++++--- arch/x86/include/asm/kprobes.h | 1 + arch/x86/kernel/entry_32.S | 56 ++++++++ arch/x86/kernel/entry_64.S | 89 +++++++++++- arch/x86/kernel/ftrace.c | 73 +++++++++- arch/x86/kernel/kprobes.c | 48 +++++++ include/linux/ftrace.h | 130 ++++++++++++++++- include/linux/kprobes.h | 27 ++++ kernel/kprobes.c | 250 +++++++++++++++++++++++---------- kernel/trace/ftrace.c | 281 +++++++++++++++++++++++++++---------- 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 +- 17 files changed, 850 insertions(+), 197 deletions(-)
Steven Rostedt
maintainerRFC PATCH v2 1/13 ftrace: Pass ftrace_ops as third parameter to function trace callback
From: Steven Rostedt <[email protected]>
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.
--- 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(-)
arch/x86/include/asm/ftrace.h +4 -0
@@ -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;
arch/x86/kernel/entry_64.S +1 -0
@@ -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
include/linux/ftrace.h +14 -2
@@ -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 */ /*
kernel/trace/ftrace.c +70 -31
216 lines not on this page · raw message
kernel/trace/trace_event_perf.c +2 -1
@@ -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;
kernel/trace/trace_events.c +2 -1
@@ -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;
kernel/trace/trace_functions.c +6 -3
@@ -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;
kernel/trace/trace_irqsoff.c +2 -1
@@ -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;
kernel/trace/trace_sched_wakeup.c +1 -1
@@ -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;
kernel/trace/trace_selftest.c +10 -5
@@ -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++; }
kernel/trace/trace_stack.c +1 -1
@@ -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;
Signature · 1 line
1.7.10
Re: [RFC][PATCH 01/13 v2] ftrace: Pass ftrace_ops as third parameter to function trace callback
Quoted 8 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
Currently the function trace callback receives only the ip and
parent_ipof 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 whatftrace_opsregistered it.Signed-off-by Steven Rostedt <[email protected]>
looks good to me :)
Thank you,
Signature · 4 lines
Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected]
[tip:perf/core] ftrace: Pass ftrace_ops as third parameter to function trace callback
In reply to Steven RostedtCommit-ID: 2f5f6ad9390c1ebbf738d130dbfe80b60eaa167e Gitweb: http://git.kernel.org/tip/2f5f6ad9390c1ebbf738d130dbfe80b60eaa167e Author: Steven Rostedt <[email protected]> AuthorDate: Mon, 8 Aug 2011 16:57:47 -0400 Committer: Steven Rostedt <[email protected]> CommitDate: Thu, 19 Jul 2012 13:17:35 -0400
ftrace: Pass ftrace_ops as third parameter to function trace callback
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.
--- 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(-)
arch/x86/include/asm/ftrace.h +4 -0
@@ -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;
arch/x86/kernel/entry_64.S +1 -0
@@ -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
include/linux/ftrace.h +14 -2
@@ -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 */ /*
kernel/trace/ftrace.c +70 -31
216 lines not on this page · raw message
kernel/trace/trace_event_perf.c +2 -1
@@ -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;
kernel/trace/trace_events.c +2 -1
@@ -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;
kernel/trace/trace_functions.c +6 -3
@@ -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;
kernel/trace/trace_irqsoff.c +2 -1
@@ -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;
kernel/trace/trace_sched_wakeup.c +1 -1
@@ -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;
kernel/trace/trace_selftest.c +10 -5
@@ -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++; }
kernel/trace/trace_stack.c +1 -1
@@ -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;
Steven Rostedt
maintainerRFC PATCH v2 2/13 ftrace: Consolidate arch dependent functions with list function
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
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.
--- include/linux/ftrace.h | 13 +++++++++++++ kernel/trace/ftrace.c | 45 ++++----------------------------------------- 2 files changed, 17 insertions(+), 41 deletions(-)
include/linux/ftrace.h +13 -0
@@ -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 ;
kernel/trace/ftrace.c +4 -41
85 lines not on this page · raw message
Signature · 1 line
1.7.10
Re: [RFC][PATCH 02/13 v2] ftrace: Consolidate arch dependent functions with list function
Quoted 21 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
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 opsBoth 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.
Quoted 5 lines
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.
Ah, I see. so even if the arch doesn't support passing ftrace_ops in mcount trampoline, this ensures that the correct ftrace_ops should be passed to the handlers.
Thank you,
Signature · 4 lines
Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected]
[tip:perf/core] ftrace: Consolidate arch dependent functions with 'list' function
In reply to Steven RostedtCommit-ID: ccf3672d530170c98c734dfc5db07d64bcbad2ad Gitweb: http://git.kernel.org/tip/ccf3672d530170c98c734dfc5db07d64bcbad2ad Author: Steven Rostedt <[email protected]> AuthorDate: Tue, 5 Jun 2012 09:44:25 -0400 Committer: Steven Rostedt <[email protected]> CommitDate: Thu, 19 Jul 2012 13:18:22 -0400
ftrace: Consolidate arch dependent functions with 'list' function
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.
--- include/linux/ftrace.h | 13 +++++++++++++ kernel/trace/ftrace.c | 45 ++++----------------------------------------- 2 files changed, 17 insertions(+), 41 deletions(-)
include/linux/ftrace.h +13 -0
@@ -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 ;
kernel/trace/ftrace.c +4 -41
85 lines not on this page · raw message
Steven Rostedt
maintainerRFC PATCH v2 3/13 ftrace: Return pt_regs to function trace callback
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Return as the 4th paramater to the function tracer callback the pt_regs.
Currently x86_64 just passes NULL as the regs arguement. Later patches that implement regs passing 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.
A ftrace_ops call back can either check if the macro ARCH_SUPPORTS_FTRACE_SAVE_REGS is defined, or it can check if regs is NULL. As it will be NULL if
it is not supported by the arch even if the SAVE_REGS flag is set.
If an arch can pass full regs, then it should define:
ARCH_SUPPORTS_FTRACE_SAVE_REGS to 1
--- arch/x86/include/asm/ftrace.h | 39 +++++++++++++++++++++---------------- arch/x86/kernel/entry_64.S | 10 +++++++--- 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 ++- 11 files changed, 76 insertions(+), 50 deletions(-)
arch/x86/include/asm/ftrace.h +22 -17
@@ -4,26 +4,31 @@#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) +/* +* We add enough stack to save all regs. + */ + subq $(SS+8), %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
arch/x86/kernel/entry_64.S +7 -3
@@ -80,7 +80,11 @@ ENTRY(ftrace_caller) MCOUNT_SAVE_FRAME leaq function_trace_op, %rdx - movq 0x38(%rsp), %rdi + + /* regs go into 4th parameter (but make it NULL) */ + movq $0, %rcx + + movq RIP(%rsp), %rdi movq 8(%rbp), %rsi subq $MCOUNT_INSN_SIZE, %rdi
@@ -120,7 +124,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 +145,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
include/linux/ftrace.h +4 -2
@@ -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 */ /*
kernel/trace/ftrace.c +22 -15
@@ -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
kernel/trace/trace_event_perf.c +1 -1
@@ -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;
kernel/trace/trace_events.c +1 -1
@@ -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;
kernel/trace/trace_functions.c +4 -3
@@ -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;
kernel/trace/trace_irqsoff.c +1 -1
@@ -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;
kernel/trace/trace_sched_wakeup.c +2 -1
@@ -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;
kernel/trace/trace_selftest.c +10 -5
@@ -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++; }
kernel/trace/trace_stack.c +2 -1
@@ -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;
Signature · 1 line
1.7.10
Re: [RFC][PATCH 03/13 v2] ftrace: Return pt_regs to function trace callback
Quoted 9 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
Return as the 4th paramater to the function tracer callback the
pt_regs.Currently
x86_64just passes NULL as the regs arguement. Later patches that implement regs passing will require having theftrace_opsset theSAVE_REGSflag, which will tell the arch to take the time to pass a full set ofpt_regsto theftrace_opscallback function. If the arch does not support it then it should pass NULL.
Hmm, I think the x86-64 part of this patch would be better to be separated with x86-64 part of [5/13], so that I can easily review the change... Another reason is that this patch doesn't define ARCH_SUPPORTS_FTRACE_SAVE_REGS on x86_64 too...
Quoted 8 lines
A
ftrace_opscall back can either check if the macroARCH_SUPPORTS_FTRACE_SAVE_REGSis defined, or it can check if regs is NULL. As it will be NULL if
it is not supported by the arch even if theSAVE_REGSflag is set.If an arch can pass full regs, then it should define:
ARCH_SUPPORTS_FTRACE_SAVE_REGSto 1Signed-off-by Steven Rostedt <[email protected]>
[...]
arch/x86/kernel/entry_64.S ENTRY(ftrace_caller) · line 85
Quoted 12 lines
arch/x86/kernel/entry_64.S +3
-1@@ -80,7 +80,11 @@ ENTRY(ftrace_caller) MCOUNT_SAVE_FRAME leaq function_trace_op, %rdx - movq 0x38(%rsp), %rdi + + /* regs go into 4th parameter (but make it NULL) */ + movq $0, %rcx
There is no ARCH_SUPPORTS_FTRACE_SAVE_REGS, so I think you don't need to clear rcx at least this time.
Another generic ftrace part of this patch is good to me :)
Thank you,
Signature · 4 lines
Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected]
Steven Rostedt
maintainerRe: [RFC][PATCH 03/13 v2] ftrace: Return pt_regs to function trace callback
Quoted 11 lines · Masami Hiramatsu
Quoted 9 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
Return as the 4th paramater to the function tracer callback the
pt_regs.Currently
x86_64just passes NULL as the regs arguement. Later patches that implement regs passing will require having theftrace_opsset theSAVE_REGSflag, which will tell the arch to take the time to pass a full set ofpt_regsto theftrace_opscallback function. If the arch does not support it then it should pass NULL.
Quoted 4 lines
Hmm, I think the x86-64 part of this patch would be better to be separated with x86-64 part of [5/13], so that I can easily review the change... Another reason is that this patch doesn't define
ARCH_SUPPORTS_FTRACE_SAVE_REGSonx86_64too...
I agree. This was more left over from the 'PARTIAL_REGS' saving, which I scrapped. I'll just fold the x86 bits of this patch into patch 5 as you suggested, and then just keep this patch as the 'added API' change.
Quoted 18 lines
Quoted 10 lines
A
ftrace_opscall back can either check if the macroARCH_SUPPORTS_FTRACE_SAVE_REGSis defined, or it can check if regs is NULL. As it will be NULL if
it is not supported by the arch even if theSAVE_REGSflag is set.If an arch can pass full regs, then it should define:
ARCH_SUPPORTS_FTRACE_SAVE_REGSto 1Signed-off-by Steven Rostedt <[email protected]>[...]
Quoted 7 lines
arch/x86/kernel/entry_64.S +0
-0@@ -80,7 +80,11 @@ ENTRY(ftrace_caller) MCOUNT_SAVE_FRAME
Quoted 8 lines
arch/x86/kernel/entry_64.S ENTRY(ftrace_caller) · line 85
Quoted 5 lines
arch/x86/kernel/entry_64.S +3
-1leaq function_trace_op, %rdx - movq 0x38(%rsp), %rdi + + /* regs go into 4th parameter (but make it NULL) */ + movq $0, %rcxThere is no
ARCH_SUPPORTS_FTRACE_SAVE_REGS, so I think you don't need to clear rcx at least this time.
Agreed, I'll fold this in then.
Quoted 4 lines
Another generic ftrace part of this patch is good to me :)
Reviewed-by Masami Hiramatsu <[email protected]>
Thanks for the review, but I'll modify 3 and 5 and hopefully, you can review it again. I'll do this tomorrow as it's bed time for me now ;-)
-- Steve
Steven Rostedt
maintainerRFC PATCH v2 4/13 ftrace/x86_32: Push ftrace_ops in as 3rd parameter to function tracer
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Add support of passing the current ftrace_ops into the 3rd parameter of the callback to the function tracer.
--- arch/x86/include/asm/ftrace.h | 2 +- arch/x86/kernel/entry_32.S | 1 + 2 files changed, 2 insertions(+), 1 deletion(-)
arch/x86/include/asm/ftrace.h +1 -1
@@ -37,7 +37,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
arch/x86/kernel/entry_32.S +1 -0
@@ -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
Signature · 1 line
1.7.10
Re: [RFC][PATCH 04/13 v2] ftrace/x86_32: Push ftrace_ops in as 3rd parameter to function tracer
Quoted 6 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
Add support of passing the current
ftrace_opsinto the 3rd parameter of the callback to the function tracer.Signed-off-by Steven Rostedt <[email protected]>
Thanks!
Signature · 4 lines
Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected]
[tip:perf/core] ftrace/x86_32: Push ftrace_ops in as 3rd parameter to function tracer
In reply to Steven RostedtCommit-ID: 28fb5dfa783c25dbeeb25a72663f8066a3a517f5 Gitweb: http://git.kernel.org/tip/28fb5dfa783c25dbeeb25a72663f8066a3a517f5 Author: Steven Rostedt <[email protected]> AuthorDate: Wed, 10 Aug 2011 22:00:55 -0400 Committer: Steven Rostedt <[email protected]> CommitDate: Thu, 19 Jul 2012 13:19:27 -0400
ftrace/x86_32: Push ftrace_ops in as 3rd parameter to function tracer
Add support of passing the current ftrace_ops into the 3rd parameter of the callback to the function tracer.
--- arch/x86/include/asm/ftrace.h | 2 +- arch/x86/kernel/entry_32.S | 1 + 2 files changed, 2 insertions(+), 1 deletions(-)
arch/x86/include/asm/ftrace.h +1 -1
@@ -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
arch/x86/kernel/entry_32.S +1 -0
@@ -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
Steven Rostedt
maintainerRFC PATCH v2 5/13 ftrace/x86: Add separate function to save regs
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
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 and 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.
--- arch/x86/include/asm/ftrace.h | 8 +++- arch/x86/kernel/entry_32.S | 2 + arch/x86/kernel/entry_64.S | 86 +++++++++++++++++++++++++++++++++++---- arch/x86/kernel/ftrace.c | 77 +++++++++++++++++++++++++++++++++-- include/linux/ftrace.h | 90 +++++++++++++++++++++++++++++++++++++++-- kernel/trace/ftrace.c | 77 +++++++++++++++++++++++++++++++---- 6 files changed, 315 insertions(+), 25 deletions(-)
arch/x86/include/asm/ftrace.h +6 -2
@@ -3,11 +3,12 @@#ifdef __ASSEMBLY__ - .macro MCOUNT_SAVE_FRAME +/* 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), %rsp + subq $(SS+8-\skip), %rsp movq %rax, RAX(%rsp) movq %rcx, RCX(%rsp) movq %rdx, RDX(%rsp)
@@ -39,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__
arch/x86/kernel/entry_32.S +2 -0
@@ -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
arch/x86/kernel/entry_64.S +78 -8
109 lines not on this page · raw message
arch/x86/kernel/ftrace.c +73 -4
141 lines not on this page · raw message
include/linux/ftrace.h +87 -3
139 lines not on this page · raw message
kernel/trace/ftrace.c +69 -8
120 lines not on this page · raw message
Signature · 1 line
1.7.10
Re: [RFC][PATCH 05/13 v2] ftrace/x86: Add separate function to save regs
Quoted 18 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
Add a way to have different functions calling different trampolines. If a
ftrace_opswants 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_opsregistered 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_64implements the full regs saving, and i386 just passes a NULL for regs to satisfy theftrace_opspassing. Where an arch must supply both regs andftrace_opsparameters, even if regs is just NULL.
Quoted 6 lines
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_REGSwhen registering theftrace_opsand either check if regs is not NULL or check ifARCH_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.
Hmm, so would you mean that user is responsible for checking whether the arch supports save_regs or not?
I would rather like ftrace to check it as my patch has done. I think ARCH_SUPPORTS_FTRACE_SAVE_REGS macro checking in all handler code is something like odd...
Thank you,
Signature · 4 lines
Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected]
Steven Rostedt
maintainerRe: [RFC][PATCH 05/13 v2] ftrace/x86: Add separate function to save regs
Quoted 7 lines · Masami Hiramatsu
Quoted 6 lines
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_REGSwhen registering theftrace_opsand either check if regs is not NULL or check ifARCH_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.
Quoted 5 lines
Hmm, so would you mean that user is responsible for checking whether the arch supports
save_regsor not?
I would rather like ftrace to check it as my patch has done. I thinkARCH_SUPPORTS_FTRACE_SAVE_REGSmacro checking in all handler code is something like odd...
I was thinking of routines that may or may not use regs. Actually, I was thinking about perf in general, that could use regs if supported, or get its own set.
But I agree that it may not be the best for those that must have regs.
Perhaps we could add another flag:
FTRACE_OPS_FL_SAVE_REGS_IF_SUPPORTED
Where it wont error out if you have this set. But if you just pass in FTRACE_OPS_FL_SAVE_REGS (as kprobes does) it will fail.
How's that sound?
-- Steve
Re: [RFC][PATCH 05/13 v2] ftrace/x86: Add separate function to save regs
Quoted 21 lines · Steven Rostedt
Quoted 7 lines · Masami Hiramatsu
Quoted 6 lines
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_REGSwhen registering theftrace_opsand either check if regs is not NULL or check ifARCH_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.Quoted 5 lines
Hmm, so would you mean that user is responsible for checking whether the arch supports
save_regsor not?
I would rather like ftrace to check it as my patch has done. I thinkARCH_SUPPORTS_FTRACE_SAVE_REGSmacro checking in all handler code is something like odd...I was thinking of routines that may or may not use regs. Actually, I was thinking about perf in general, that could use regs if supported, or get its own set.
But I agree that it may not be the best for those that must have regs.
Quoted 8 lines
Perhaps we could add another flag:
FTRACE_OPS_FL_SAVE_REGS_IF_SUPPORTEDWhere it wont error out if you have this set. But if you just pass in
FTRACE_OPS_FL_SAVE_REGS(as kprobes does) it will fail.How's that sound?
Yeah, that's good for me. :)
Thank you,
Signature · 4 lines
Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected]
Steven Rostedt
maintainerRFC PATCH v2 6/13 ftrace/x86: Add save_regs for i386 function calls
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Add saving full regs for function tracing on i386. The saving of regs was influenced by patches sent out by Masami Hiramatsu.
--- 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(-)
arch/x86/include/asm/ftrace.h +0 -2
@@ -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 );
arch/x86/kernel/entry_32.S +53 -0
65 lines not on this page · raw message
arch/x86/kernel/ftrace.c +0 -4
@@ -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);
Signature · 1 line
1.7.10
Re: [RFC][PATCH 06/13 v2] ftrace/x86: Add save_regs for i386 function calls
Quoted 86 lines · Steven Rostedt
From: Steven Rostedt <[email protected]>
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 <[email protected]>Signed-off-by Steven Rostedt <[email protected]>--- 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(-)arch/x86/include/asm/ftrace.h +0
-2@@ -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 );arch/x86/kernel/entry_32.S +42
-0@@ -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
arch/x86/kernel/entry_32.S ftrace_stub: · line 1188
Quoted 10 lines
arch/x86/kernel/entry_32.S +10
-0+ popl %edi + popl %ebp + popl %eax + popl %ds + popl %es + popl %fs + popl %gs + addl $8, %esp +ftrace_exit: + addl $4, %esp /* Skip eflags */
Hmm, why don't you recover eflags, as x86-64 has done? IMHO, it should be recovered if ftrace-based kprobe is a transparent acceleration.
Thank you,
Signature · 4 lines
Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected]
Steven Rostedt
maintainerRe: [RFC][PATCH 06/13 v2] ftrace/x86: Add save_regs for i386 function calls
Quoted 18 lines · Masami Hiramatsu
Quoted 16 lines · Steven Rostedt
arch/x86/kernel/entry_32.S +16
-0+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
Quoted 8 lines
arch/x86/kernel/entry_32.S ftrace_stub: · line 1188
Quoted 4 lines
arch/x86/kernel/entry_32.S +4
-0+ popl %gs + addl $8, %esp +ftrace_exit: + addl $4, %esp /* Skip eflags */Hmm, why don't you recover eflags, as x86-64 has done? IMHO, it should be recovered if ftrace-based kprobe is a transparent acceleration.
As I copied this mostly from your patch I probably was thinking that it saved it already ;-)
That is, we originally debated the usefulness of restoring eflags at the start of the function call, as function calls give no guarantees to them. I modified this patch to not restore flags. But later, I agreed that I would keep kprobes the way it was, even though flags are pretty meaningless here (unless you are going to have the kprobe modify the enabling of interrupts), and decided to add back the flags. I simply forgot to restore back to your original patch.
Will fix,
Thanks,
-- Steve
Steven Rostedt
maintainerRFC PATCH v2 7/13 ftrace: add ftrace_set_filter_ip() for address based filter
In reply to Steven RostedtFrom: Masami Hiramatsu <[email protected]>
Add a new filter update interface ftrace_set_filter_ip() to set ftrace filter by ip address, not only glob pattern.
--- include/linux/ftrace.h | 3 +++ kernel/trace/ftrace.c | 59 ++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 2 deletions(-)
include/linux/ftrace.h +3 -0
@@ -297,6 +297,8 @@ struct dyn_ftrace { };int ftrace_force_update (void ); +int ftrace_set_filter_ip (struct ftrace_ops *ops,unsigned long ip, +int remove,int reset);int ftrace_set_filter (struct ftrace_ops *ops,unsigned char *buf,int len,int reset); int ftrace_set_notrace(struct ftrace_ops *ops, unsigned char *buf,
@@ -524,6 +526,7 @@ static inline int ftrace_text_reserved(void *start, void *end)*/ #define ftrace_regex_open (ops, flag, inod, file) ({ -ENODEV; })#define ftrace_set_early_filter (ops, buf, enable) do { } while (0) +#define ftrace_set_filter_ip (ops, ip, remove, reset) ({ -ENODEV; })#define ftrace_set_filter (ops, buf, len, reset) ({ -ENODEV; })#define ftrace_set_notrace (ops, buf, len, reset) ({ -ENODEV; })#define ftrace_free_filter (ops) do { } while (0)
kernel/trace/ftrace.c +57 -2
77 lines not on this page · raw message
Signature · 1 line
1.7.10
Steven Rostedt
maintainerRFC PATCH v2 8/13 kprobes: Inverse taking of module_mutex with kprobe_mutex
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
Currently module_mutex is taken before kprobe_mutex, but this can cause issues when we have kprobes register ftrace, as the ftrace mutex is taken before enabling a tracepoint, which currently takes the module mutex.
If module_mutex is taken before kprobe_mutex, then we can not have kprobes use the ftrace infrastructure.
There seems to be no reason that the kprobe_mutex can't be taken before the module_mutex. Running lockdep shows that it is safe among the kernels I've run.
--- kernel/kprobes.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/kprobes.c +2 -2
@@ -561,9 +561,9 @@ static __kprobes void kprobe_optimizer(struct work_struct *work) {LIST_HEAD (free_list); +mutex_lock (&kprobe_mutex);/* Lock modules while optimizing kprobes */ mutex_lock (&module_mutex); -mutex_lock (&kprobe_mutex);/* * Step 1: Unoptimize kprobes and collect cleaned (unused and disarmed)
@@ -586,8 +586,8 @@ static __kprobes void kprobe_optimizer(struct work_struct *work)/* Step 4: Free cleaned kprobes after quiesence period */ do_free_cleaned_kprobes (&free_list); -mutex_unlock (&kprobe_mutex);mutex_unlock (&module_mutex); +mutex_unlock (&kprobe_mutex);/* Step 5: Kick optimizer again if needed */ if (!list_empty (&optimizing_list) || !list_empty (&unoptimizing_list))
Signature · 1 line
1.7.10
Steven Rostedt
maintainerRFC PATCH v2 9/13 kprobes: cleanup to separate probe-able check
In reply to Steven RostedtFrom: Masami Hiramatsu <[email protected]>
Separate probe-able address checking code fromregister_kprobe().
--- kernel/kprobes.c | 82 ++++++++++++++++++++++++++++++------------------------ 1 file changed, 45 insertions(+), 37 deletions(-)
kernel/kprobes.c +45 -37
@@ -1313,67 +1313,80 @@ static inline int check_kprobe_rereg(struct kprobe *p)return ret; } -int __kprobes register_kprobe (struct kprobe *p) +static __kprobes int check_kprobe_address_safe (struct kprobe *p, +struct module **probed_mod) {int ret =0 ; -struct kprobe *old_p; -struct module *probed_mod; -kprobe_opcode_t *addr; - - addr =kprobe_addr (p); -if (IS_ERR (addr)) -return PTR_ERR (addr); - p->addr = addr; - - ret =check_kprobe_rereg (p); -if (ret) -return ret;jump_label_lock ();preempt_disable (); + +/* Ensure it is not in reserved area nor out of text */ if (!kernel_text_address ((unsigned long ) p->addr) ||in_kprobes_functions ((unsigned long ) p->addr) ||ftrace_text_reserved (p->addr, p->addr) ||jump_label_text_reserved (p->addr, p->addr)) { ret = -EINVAL ; - goto cannot_probe; + goto out; } -/* User can pass only KPROBE_FLAG_DISABLED to register_kprobe */ - p->flags &=KPROBE_FLAG_DISABLED ; - -/* -* Check if are we probing a module. -*/ - probed_mod =__module_text_address ((unsigned long ) p->addr); -if (probed_mod) { -/* Return -ENOENT if fail. */ - ret = -ENOENT ; +/* Check if are we probing a module */ + *probed_mod =__module_text_address ((unsigned long ) p->addr); +if (*probed_mod) {/* * We must hold a refcount of the probed module while updating * its code to prohibit unexpected unloading. */ -if (unlikely (!try_module_get (probed_mod))) - goto cannot_probe; +if (unlikely (!try_module_get (*probed_mod))) { + ret = -ENOENT ; + goto out; + }/* * If the module freed .init.text, we couldn't insert * kprobes in there. */ -if (within_module_init ((unsigned long )p->addr, probed_mod) && - probed_mod->state !=MODULE_STATE_COMING ) { -module_put (probed_mod); - goto cannot_probe; +if (within_module_init ((unsigned long )p->addr, *probed_mod) && + (*probed_mod)->state !=MODULE_STATE_COMING ) { +module_put (*probed_mod); + *probed_mod =NULL ; + ret = -ENOENT ; } -/* ret will be updated by following code */ } +out:preempt_enable ();jump_label_unlock (); +return ret; +} + +int __kprobes register_kprobe (struct kprobe *p) +{ +int ret; +struct kprobe *old_p; +struct module *probed_mod; +kprobe_opcode_t *addr; + +/* Adjust probe address from symbol */ + addr =kprobe_addr (p); +if (IS_ERR (addr)) +return PTR_ERR (addr); + p->addr = addr; + + ret =check_kprobe_rereg (p); +if (ret) +return ret; + +/* User can pass only KPROBE_FLAG_DISABLED to register_kprobe */ + p->flags &=KPROBE_FLAG_DISABLED ; p->nmissed =0 ;INIT_LIST_HEAD (&p->list); -mutex_lock (&kprobe_mutex); + ret =check_kprobe_address_safe (p, &probed_mod); +if (ret) +return ret; + +mutex_lock (&kprobe_mutex);jump_label_lock ();/* needed to call jump_label_text_reserved() */ get_online_cpus ();/* For avoiding text_mutex deadlock. */
@@ -1410,11 +1423,6 @@ out:module_put (probed_mod);return ret; - -cannot_probe: -preempt_enable (); -jump_label_unlock (); -return ret; }EXPORT_SYMBOL_GPL (register_kprobe);
Signature · 1 line
1.7.10
Steven Rostedt
maintainerRFC PATCH v2 10/13 kprobes: Move locks into appropriate functions
In reply to Steven RostedtFrom: Masami Hiramatsu <[email protected]>
Break a big critical region into fine-grained pieces at registering kprobe path. This helps us to solve circular locking dependency when introducing ftrace-based kprobes.
--- kernel/kprobes.c | 63 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 21 deletions(-)
kernel/kprobes.c +42 -21
@@ -759,20 +759,28 @@ static __kprobes void try_to_optimize_kprobe(struct kprobe *p)struct kprobe *ap;struct optimized_kprobe *op; +/* For preparing optimization, jump_label_text_reserved() is called */ +jump_label_lock (); +mutex_lock (&text_mutex); + ap =alloc_aggr_kprobe (p);if (!ap) -return ; + goto out; op = container_of(ap, struct optimized_kprobe, kp);if (!arch_prepared_optinsn (&op->optinsn)) {/* If failed to setup optimizing, fallback to kprobe */ arch_remove_optimized_kprobe (op);kfree (op); -return ; + goto out; }init_aggr_kprobe (ap, p); -optimize_kprobe (ap); +optimize_kprobe (ap);/* This just kicks optimizer thread */ + +out: +mutex_unlock (&text_mutex); +jump_label_unlock (); }#ifdef CONFIG_SYSCTL
@@ -1144,12 +1152,6 @@ static int __kprobes add_new_kprobe(struct kprobe *ap, struct kprobe *p)if (p->post_handler && !ap->post_handler) ap->post_handler = aggr_post_handler; -if (kprobe_disabled (ap) && !kprobe_disabled (p)) { - ap->flags &= ~KPROBE_FLAG_DISABLED ; -if (!kprobes_all_disarmed) -/* Arm the breakpoint again. */ -__arm_kprobe (ap); - }return 0 ; }
@@ -1189,11 +1191,22 @@ static int __kprobes register_aggr_kprobe(struct kprobe *orig_p,int ret =0 ;struct kprobe *ap = orig_p; +/* For preparing optimization, jump_label_text_reserved() is called */ +jump_label_lock (); +/* +* Get online CPUs to avoid text_mutex deadlock.with stop machine, +* which is invoked by unoptimize_kprobe() in add_new_kprobe() +*/ +get_online_cpus (); +mutex_lock (&text_mutex); +if (!kprobe_aggrprobe (orig_p)) {/* If orig_p is not an aggr_kprobe, create new aggr_kprobe. */ ap =alloc_aggr_kprobe (orig_p); -if (!ap) -return -ENOMEM ; +if (!ap) { + ret = -ENOMEM ; + goto out; + }init_aggr_kprobe (ap, orig_p); }else if (kprobe_unused (ap))/* This probe is going to die. Rescue it */
@@ -1213,7 +1226,7 @@ static int __kprobes register_aggr_kprobe(struct kprobe *orig_p,* free aggr_probe. It will be used next time, or * freed by unregister_kprobe. */ -return ret; + goto out;/* Prepare optimized instructions if possible. */ prepare_optimized_kprobe (ap);
@@ -1228,7 +1241,20 @@ static int __kprobes register_aggr_kprobe(struct kprobe *orig_p,/* Copy ap's insn slot to p */ copy_kprobe (ap, p); -return add_new_kprobe (ap, p); + ret =add_new_kprobe (ap, p); + +out: +mutex_unlock (&text_mutex); +put_online_cpus (); +jump_label_unlock (); + +if (ret ==0 &&kprobe_disabled (ap) && !kprobe_disabled (p)) { + ap->flags &= ~KPROBE_FLAG_DISABLED ; +if (!kprobes_all_disarmed) +/* Arm the breakpoint again. */ +arm_kprobe (ap); + } +return ret; }static int __kprobes in_kprobes_functions (unsigned long addr)
@@ -1387,10 +1413,6 @@ int __kprobes register_kprobe(struct kprobe *p)return ret;mutex_lock (&kprobe_mutex); -jump_label_lock ();/* needed to call jump_label_text_reserved() */ - -get_online_cpus ();/* For avoiding text_mutex deadlock. */ -mutex_lock (&text_mutex); old_p =get_kprobe (p->addr);if (old_p) {
@@ -1399,7 +1421,9 @@ int __kprobes register_kprobe(struct kprobe *p) goto out; } +mutex_lock (&text_mutex);/* Avoiding text modification */ ret =arch_prepare_kprobe (p); +mutex_unlock (&text_mutex);if (ret) goto out;
@@ -1408,15 +1432,12 @@ int __kprobes register_kprobe(struct kprobe *p) &kprobe_table[hash_ptr(p->addr, KPROBE_HASH_BITS)]);if (!kprobes_all_disarmed && !kprobe_disabled (p)) -__arm_kprobe (p); +arm_kprobe (p);/* Try to optimize kprobe */ try_to_optimize_kprobe (p); out: -mutex_unlock (&text_mutex); -put_online_cpus (); -jump_label_unlock ();mutex_unlock (&kprobe_mutex);if (probed_mod)
Signature · 1 line
1.7.10
Steven Rostedt
maintainerRFC PATCH v2 11/13 ftrace: Make ftrace_location() a nop on !DYNAMIC_FTRACE
In reply to Steven RostedtFrom: Steven Rostedt <[email protected]>
When CONFIG_DYNAMIC_FTRACE is not set, ftrace_location() is not defined. If a user (like kprobes) references this function, it will break the compile when CONFIG_DYNAMIC_FTRACE is not set.
Add ftrace_location() as a nop (return 0) when DYNAMIC_FTRACE is not defined.
--- include/linux/ftrace.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
include/linux/ftrace.h +5 -1
@@ -500,7 +500,7 @@ extern int skip_trace(unsigned long ip);extern void ftrace_disable_daemon (void );extern void ftrace_enable_daemon (void ); -#else +#else /* CONFIG_DYNAMIC_FTRACE */ static inline int skip_trace (unsigned long ip) {return 0 ; }static inline int ftrace_force_update (void ) {return 0 ; }static inline void ftrace_disable_daemon (void ) { }
@@ -518,6 +518,10 @@ static inline int ftrace_text_reserved(void *start, void *end) {return 0 ; } +static inline unsigned long ftrace_location (unsigned long ip) +{ +return 0 ; +}/* * Again users of functions that have ftrace_ops may not
Signature · 1 line
1.7.10
[tip:perf/core] ftrace: Make ftrace_location() a nop on !DYNAMIC_FTRACE
Commit-ID: 4dc936769e8a6382a4cc12375e8a4daa2b829fda Gitweb: http://git.kernel.org/tip/4dc936769e8a6382a4cc12375e8a4daa2b829fda Author: Steven Rostedt <[email protected]> AuthorDate: Wed, 6 Jun 2012 13:45:31 -0400 Committer: Steven Rostedt <[email protected]> CommitDate: Tue, 31 Jul 2012 10:29:57 -0400
ftrace: Make ftrace_location() a nop on !DYNAMIC_FTRACE
When CONFIG_DYNAMIC_FTRACE is not set, ftrace_location() is not defined. If a user (like kprobes) references this function, it will break the compile when CONFIG_DYNAMIC_FTRACE is not set.
Add ftrace_location() as a nop (return 0) when DYNAMIC_FTRACE is not defined.
--- include/linux/ftrace.h | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-)
include/linux/ftrace.h +5 -1
@@ -520,7 +520,7 @@ extern int skip_trace(unsigned long ip);extern void ftrace_disable_daemon (void );extern void ftrace_enable_daemon (void ); -#else +#else /* CONFIG_DYNAMIC_FTRACE */ static inline int skip_trace (unsigned long ip) {return 0 ; }static inline int ftrace_force_update (void ) {return 0 ; }static inline void ftrace_disable_daemon (void ) { }
@@ -538,6 +538,10 @@ static inline int ftrace_text_reserved(void *start, void *end) {return 0 ; } +static inline unsigned long ftrace_location (unsigned long ip) +{ +return 0 ; +}/* * Again users of functions that have ftrace_ops may not
Steven Rostedt
maintainerRFC PATCH v2 12/13 kprobes: introduce ftrace based optimization
In reply to Steven RostedtFrom: Masami Hiramatsu <[email protected]>
Introduce function trace based kprobes optimization.
With using ftrace optimization, kprobes on the mcount calling address, use ftrace's mcount call instead of breakpoint. Furthermore, this optimization works with preemptive kernel not like as current jump-based optimization. Of cource, this feature works only if the probe is on mcount call.
Only if kprobe.break_handler is set, that probe is not optimized with ftrace (nor put on ftrace). The reason why this limitation comes is that this break_handler may be used only from jprobes which changes ip address (for fetching the function arguments), but function tracer ignores modified ip address.
Changes in v2:
- Fix ftrace_ops registering right after setting its filter.
- Unregister ftrace_ops if there is no kprobe using.
- Remove notrace dependency from __kprobes macro.
--- include/linux/kprobes.h | 27 ++++++++++++ kernel/kprobes.c | 105 +++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 119 insertions(+), 13 deletions(-)
include/linux/kprobes.h +27 -0
@@ -38,6 +38,7 @@#include <linux/spinlock.h> #include <linux/rcupdate.h> #include <linux/mutex.h> +#include <linux/ftrace.h> #ifdef CONFIG_KPROBES #include <asm/kprobes.h>
@@ -48,14 +49,26 @@#define KPROBE_REENTER 0x00000004#define KPROBE_HIT_SSDONE 0x00000008 +/* +* If function tracer is enabled and the arch supports full +* passing of pt_regs to function tracing, then kprobes can +* optimize on top of function tracing. +*/ +#if defined(CONFIG_FUNCTION_TRACER ) && defined(ARCH_SUPPORTS_FTRACE_SAVE_REGS ) \ + && defined(ARCH_SUPPORTS_KPROBES_ON_FTRACE ) +# define KPROBES_CAN_USE_FTRACE +#endif +/* Attach to insert probes on any functions which should be ignored*/ #define __kprobes __attribute__((__section__(".kprobes.text"))) +#else /* CONFIG_KPROBES */ typedef int kprobe_opcode_t ;struct arch_specific_insn {int dummy; };#define __kprobes +#endif /* CONFIG_KPROBES */ struct kprobe ;
@@ -128,6 +141,7 @@ struct kprobe {* NOTE: * this flag is only for optimized_kprobe. */ +#define KPROBE_FLAG_FTRACE 8/* probe is using ftrace */ /* Has this kprobe gone ? */ static inline int kprobe_gone (struct kprobe *p)
@@ -146,6 +160,13 @@ static inline int kprobe_optimized(struct kprobe *p) {return p->flags &KPROBE_FLAG_OPTIMIZED ; } + +/* Is this kprobe uses ftrace ? */ +static inline int kprobe_ftrace (struct kprobe *p) +{ +return p->flags &KPROBE_FLAG_FTRACE ; +} +/* * Special probe type that uses setjmp-longjmp type tricks to resume * execution at a specified entry with a matching prototype corresponding
@@ -295,6 +316,12 @@ extern int proc_kprobes_optimization_handler(struct ctl_table *table,#endif #endif /* CONFIG_OPTPROBES */ +#ifdef KPROBES_CAN_USE_FTRACE +extern void kprobe_ftrace_handler (unsigned long ip,unsigned long parent_ip, +struct pt_regs *regs); +extern int arch_prepare_kprobe_ftrace (struct kprobe *p); +#endif +/* Get the kprobe at this addr (if any) - called with preemption disabled */ struct kprobe *get_kprobe (void *addr);
kernel/kprobes.c +92 -13
184 lines not on this page · raw message
Signature · 1 line
1.7.10
Steven Rostedt
maintainerRFC PATCH v2 13/13 kprobes/x86: ftrace based optimization for x86
In reply to Steven RostedtFrom: Masami Hiramatsu <[email protected]>
Add function tracer based kprobe optimization support handlers on x86. This allows kprobes to use function tracer for probing on mcount call.
[ Updated to new port of ftrace save regs functions ]
--- arch/x86/include/asm/kprobes.h | 1 + arch/x86/kernel/kprobes.c | 48 ++++++++++++++++++++++++++++++++++++++++ include/linux/kprobes.h | 2 +- kernel/kprobes.c | 2 +- 4 files changed, 51 insertions(+), 2 deletions(-)
arch/x86/include/asm/kprobes.h +1 -0
@@ -27,6 +27,7 @@#include <asm/insn.h> #define __ARCH_WANT_KPROBES_INSN_SLOT +#define ARCH_SUPPORTS_KPROBES_ON_FTRACE struct pt_regs ;struct kprobe ;
arch/x86/kernel/kprobes.c +48 -0
54 lines not on this page · raw message
include/linux/kprobes.h +1 -1
@@ -318,7 +318,7 @@ extern int proc_kprobes_optimization_handler(struct ctl_table *table,#endif /* CONFIG_OPTPROBES */ #ifdef KPROBES_CAN_USE_FTRACE extern void kprobe_ftrace_handler (unsigned long ip,unsigned long parent_ip, -struct pt_regs *regs); +struct ftrace_ops *ops,struct pt_regs *regs);extern int arch_prepare_kprobe_ftrace (struct kprobe *p);#endif
kernel/kprobes.c +1 -1
@@ -921,7 +921,7 @@ static __kprobes struct kprobe *alloc_aggr_kprobe(struct kprobe *p)#ifdef KPROBES_CAN_USE_FTRACE static struct ftrace_ops kprobe_ftrace_ops__read_mostly = { - .regs_func = kprobe_ftrace_handler, + .func = kprobe_ftrace_handler, .flags =FTRACE_OPS_FL_SAVE_REGS , };static int kprobe_ftrace_enabled;
Signature · 1 line
1.7.10
Quoted 5 lines · Steven Rostedt
If an arch supports passing of ftrace ops, it must also pass regs. But it does not need to support passing regs. By default an arch can just pass NULL. If it supports regs, then it can allow tools like kprobes to ask for regs. Otherwise the regs parameter should just be NULL.
Hmm, by default, will the ftrace_ops be also NULL? or NULL only if the arch doesn't support passing ftrace ops?
I mean, should the generic handler always check if ftrace_ops isn't NULL before using it?
Quoted 10 lines
The second set is Masami's patches ported on top of these changes.
This is v2, and I've cleaned the patches up a bit and added a lot more comments. This is a more serious RFC as this is the current way I plan on pushing it to mainline. But I'm posting first in case someone spots something that I missed. I'll also be doing more testing on it and if someting comes up I'll obviously will fix it before pushing.
Some change since v1:
Quoted 7 lines
Added fix for undefined
ftrace_location()when compiling with !DYNAMIC_FTRACE.Added missing regs passed for
x86_64(segment regs, and r11).Saved flags for when regs are being saved. The cmp in the mcount trampoline that checks if function tracing is disabled will save flags before the compare, and restore flags before returning back to the function.
Nice! :) this is what I hope in previous thread.
Quoted 8 lines
Decided that passing partial regs is a bad idea. Either the arch should pass all regs or NULL. Even though partial regs are saved for calling the function, it does't make sense to give that to the callback. By making regs be full
pt_regsor NULL, then the callbacks can simply check if regs is NULL and if it isn't it can do more functionality, but if it is NULL then it just does limited work. Having 'partial regs' would just complicate that for no good reason. If in the future we want partial regs, we can add another flag to request it.
Agreed.
Thank you,
Signature · 4 lines
Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected]
Steven Rostedt
maintainerQuoted 11 lines · Masami Hiramatsu
Quoted 5 lines · Steven Rostedt
If an arch supports passing of ftrace ops, it must also pass regs. But it does not need to support passing regs. By default an arch can just pass NULL. If it supports regs, then it can allow tools like kprobes to ask for regs. Otherwise the regs parameter should just be NULL.
Hmm, by default, will the
ftrace_opsbe also NULL? or NULL only if the arch doesn't support passing ftrace ops?
I mean, should the generic handler always check ifftrace_opsisn't NULL before using it?
Nope, ftrace_ops will always be set with the ops that registered it for the callback. As suppose to regs, ftrace_ops is created by the generic core code and is arch agnostic. With regs, that's very arch specific and if an arch does not support passing regs, there's nothing that the core code can do about it but pass a NULL pointer telling the callback "Sorry!".
Currently there's a few things that an arch needs to do in the mcount handler to support full dynamic ftrace. One is to check the function_trace_stop variable is not set before calling the callback. If an arch does not support this, then ftrace will only let the callback be a generic function that does the check on behalf of the arch and then calls the normal routine.
Now the arch also needs to support sending in the 3rd parameter the ftrace_ops. If it does not support this ftrace generic code will. Luckily, it already has code to do this. If more than one ftrace_ops is registered (even if they register to different functions), a generic handler is called that iterates through all the registered ftrace_ops to call their callbacks. It also checks if the ftrace_ops has registered the current function before calling it.
Now if an arch does not pass the ftrace ops we simply call this list function instead. The list function already has access to what ftrace_ops is being used and passes that to the callback. Again, this is what must be done anyway if more than one ftrace_ops is registered to the function tracer.
As this is another indirect call that must be made, I removed the old helper function that does the check against function_trace_stop variable and made the arch call the list code instead. The list code now does the check. If the arch does not support checking function_trace_stop, it most likely doesn't support passing ftrace_ops either.
-- Steve