Skip to contentSkip to conversation

ftrace,kprobes: Ftrace-based kprobe optimization

PATCH In review Masami Hiramatsu 5 participants 25 messages Jun 5, 2012 – Aug 21, 2012

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

v2 cover ftrace,kprobes: Ftrace-based kprobe optimization

Hi Steven,

Here is the version 2 patches for kprobe-ftrace. Since I didn't touch ftrace part in this version, old gcc bug still be there. So, if you have another implementation, prease replace former 3 patches in this series with your patch.

In this version, I've fixed kprobe-ftrace recursion problem by initializing ftrace_ops filter correctly and registering/ unregistering on-demand.

This series of patches which allows kprobes to use ftrace for optimizing probing path if the probe on ftrace (mcount call). The optimization is transparently done by kprobes.

Only if kprobe.break_handler is set, that probe can not be 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). In this series, ftrace doesn't allow to change regs->ip, since I don't want to make any non-essential trouble ;)

After deep consideration, I've decided to remove "real_addr" method at this time, since it is hard to achieve complete transparency with the combination of jprobe and aggregated kprobes.
And also, at least on x86, ftrace-based kprobes is available.

Anyway, I can say jprobes is an out-dated probe because we already has kprobe-tracer and perf-probe which allows us to get function arguments directly from kprobes. :)

On the other hand, since kprobes has self-recursion check, it is safe if a probe is recursively hit, or hit a fault and call fault_handler.

For using ftrace from kprobes, this series introduces two new interface for ftrace;

- ftrace_ops.regs_func, which is a callback handler invoked with pt_regs as third argument. For enable this feature, ftrace_ops.flags must set FTRACE_OPS_FL_SAVE_REGS bit.
- ftrace_set_filter_ip(), which allows to set new address-based filter instead of glob pattern.

ftrace_ops.regs_func is still under discussion because older gcc doesn't support initalizing unnamed union member.

In this version, I've removed notrace dependency from __kprobes. Instead, it just set filter correctly before adding ftrace_ops not to trace all functions.

Changes in v2:
- Fix ftrace_ops registering right after setting its filter.
- Unregister ftrace_ops if there is no kprobe useing.
- Remove notrace dependency from __kprobes macro.

Thank you,

---

Masami Hiramatsu (8):
kprobes/x86: ftrace based optimization for x86
kprobes: introduce ftrace based optimization
kprobes: Move locks into appropriate functions
kprobes: cleanup to separate probe-able check
ftrace: add ftrace_set_filter_ip() for address based filter ftrace/x86: Support SAVE_REGS feature on i386 ftrace/x86-64: support SAVE_REGS feature on x86-64
ftrace: Add pt_regs acceptable trace callback

Steven Rostedt (1):
kprobes: Inverse taking of module_mutex with kprobe_mutex

 arch/x86/include/asm/ftrace.h  |    4 +
 arch/x86/include/asm/kprobes.h |    1 
 arch/x86/kernel/entry_32.S     |   64 +++++++++-
 arch/x86/kernel/entry_64.S     |   38 +++++-
 arch/x86/kernel/kprobes.c      |   48 ++++++++
 include/linux/ftrace.h         |   24 ++++
 include/linux/kprobes.h        |   27 ++++
 kernel/kprobes.c               |  250 +++++++++++++++++++++++++++++-----------
 kernel/trace/ftrace.c          |  103 ++++++++++++++--
 9 files changed, 458 insertions(+), 101 deletions(-)
Signature · 4 lines
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
#1 lore raw

PATCH v2 1/9 ftrace: Add pt_regs acceptable trace callback

Add a new callback interface regs_func to ftrace_ops which is an anonymous union to share the pointer with func member. So, current ftrace user doesn't need to change their callbacks.
This callback must be used with FTRACE_OPS_FL_SAVE_REGS.

---

 include/linux/ftrace.h |   21 ++++++++++++++++++++-
 kernel/trace/ftrace.c  |   44 +++++++++++++++++++++++++++++++-------------
 2 files changed, 51 insertions(+), 14 deletions(-)
include/linux/ftrace.h +20 -1
index 55e6d63..11abe4e 100644
@@ -10,6 +10,8 @@
 #include <linux/kallsyms.h>
 #include <linux/linkage.h>
 #include <linux/bitops.h>
+#include <linux/module.h>
+#include <linux/ptrace.h>
 #include <linux/ktime.h>
 #include <linux/sched.h>
 #include <linux/types.h>
@@ -18,6 +20,14 @@
 
 #include <asm/ftrace.h>
 
+/*
+ * If the arch supports saving the regs to the ftrace_ops
+ * then it should set this to 1.
+ */
+#ifndef ARCH_SUPPORTS_FTRACE_SAVE_REGS
+#define ARCH_SUPPORTS_FTRACE_SAVE_REGS 0
+#endif
+
 struct module;
 struct ftrace_hash;
 
@@ -30,6 +40,8 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,
 		     loff_t *ppos);
 
 typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip);
+typedef void (*ftrace_regs_func_t)(unsigned long ip, unsigned long parent_ip,
+				   struct pt_regs *regs);
 
 /*
  * FTRACE_OPS_FL_* bits denote the state of ftrace_ops struct and are
@@ -45,16 +57,22 @@ 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 - set manualy by ftrace_ops user to denote the ftrace_ops
+ *             requests to pass pt_regs to callback.
  */
 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_SAVE_REGS		= 1 << 4,
 };
 
 struct ftrace_ops {
-	ftrace_func_t			func;
+	union {
+		ftrace_func_t			func;
+		ftrace_regs_func_t		regs_func;
+	};
 	struct ftrace_ops		*next;
 	unsigned long			flags;
 	int __percpu			*disabled;
@@ -164,6 +182,7 @@ static inline int ftrace_function_local_disabled(struct ftrace_ops *ops)
 }
 
 extern void ftrace_stub(unsigned long a0, unsigned long a1);
+#define ftrace_regs_stub	(ftrace_regs_func_t)(ftrace_stub)
 
 #else /* !CONFIG_FUNCTION_TRACER */
 /*
kernel/trace/ftrace.c +31 -13

119 lines not on this page · raw message

#2 lore raw

PATCH v2 2/9 ftrace/x86-64: support SAVE_REGS feature on x86-64

In reply to Masami Hiramatsu

Add register saving/restoring sequence in ftrace_caller on x86-64 for enabling SAVE_REGS feature.

---

 arch/x86/include/asm/ftrace.h |    4 ++++
 arch/x86/kernel/entry_64.S    |   38 +++++++++++++++++++++++++++++++++-----
 2 files changed, 37 insertions(+), 5 deletions(-)
arch/x86/include/asm/ftrace.h +4 -0
index 18d9005..ffb9564 100644
@@ -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_SAVE_REGS 1
+#endif
+
 #ifndef __ASSEMBLY__
 extern void mcount(void);
 extern int modifying_ftrace_code;
arch/x86/kernel/entry_64.S +33 -5
index 320852d..6d0545b 100644
@@ -73,21 +73,44 @@ ENTRY(mcount)
 	retq
 END(mcount)
 
+	.macro MCOUNT_SAVE_ALL
+	SAVE_ARGS (RSP-ORIG_RAX)
+	SAVE_REST
+	/* Adjust eflags, rsp and rip */
+	movq RSP(%rsp), %rdx
+	movq %rdx, EFLAGS(%rsp)
+	leaq SS+8(%rsp), %rdx
+	movq %rdx, RSP(%rsp)
+	movq SS(%rsp), %rdi
+	subq $MCOUNT_INSN_SIZE, %rdi
+	movq %rdi, RIP(%rsp)
+	.endm
+
+	.macro MCOUNT_RESTORE_ALL
+	/* store eflags into regs->rsp */
+	movq EFLAGS(%rsp), %rax
+	movq %rax, RSP(%rsp)
+	RESTORE_REST
+	RESTORE_ARGS 1, (RSP-ORIG_RAX)
+	.endm
+
 ENTRY(ftrace_caller)
+	CFI_STARTPROC	simple
+	pushfq_cfi
 	cmpl $0, function_trace_stop
-	jne  ftrace_stub
+	jne  ftrace_exit
 
-	MCOUNT_SAVE_FRAME
+	MCOUNT_SAVE_ALL
 
-	movq 0x38(%rsp), %rdi
 	movq 8(%rbp), %rsi
-	subq $MCOUNT_INSN_SIZE, %rdi
+	movq %rsp, %rdx
 
 GLOBAL(ftrace_call)
 	call ftrace_stub
 
-	MCOUNT_RESTORE_FRAME
+	MCOUNT_RESTORE_ALL
 
+	popfq_cfi
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 GLOBAL(ftrace_graph_call)
 	jmp ftrace_stub
@@ -95,6 +118,11 @@ GLOBAL(ftrace_graph_call)
 
 GLOBAL(ftrace_stub)
 	retq
+
+ftrace_exit:
+	popfq_cfi
+	retq
+	CFI_ENDPROC
 END(ftrace_caller)
 
 #else /* ! CONFIG_DYNAMIC_FTRACE */
#3 lore raw

PATCH v2 3/9 ftrace/x86: Support SAVE_REGS feature on i386

In reply to Masami Hiramatsu

Add register saving/restoring sequence in ftrace_caller on i386 for enabling SAVE_REGS feature.

---

 arch/x86/include/asm/ftrace.h |    2 +
 arch/x86/kernel/entry_32.S    |   64 +++++++++++++++++++++++++++++++++++------
 2 files changed, 56 insertions(+), 10 deletions(-)
arch/x86/include/asm/ftrace.h +1 -1
index ffb9564..e2f7269 100644
@@ -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)
+#if defined(CONFIG_DYNAMIC_FTRACE)
 #define ARCH_SUPPORTS_FTRACE_SAVE_REGS 1
 #endif
 
arch/x86/kernel/entry_32.S +55 -9

85 lines not on this page · raw message

maintainer
#4 lore raw

Re: [PATCH -tip v2 3/9] ftrace/x86: Support SAVE_REGS feature on i386

Quoted 51 lines · Masami Hiramatsu

Add register saving/restoring sequence in ftrace_caller on i386 for enabling SAVE_REGS feature.

---

 arch/x86/include/asm/ftrace.h |    2 +
 arch/x86/kernel/entry_32.S    |   64 +++++++++++++++++++++++++++++++++++------
 2 files changed, 56 insertions(+), 10 deletions(-)
arch/x86/include/asm/ftrace.h +1 -1
index ffb9564..e2f7269 100644
@@ -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)
+#if defined(CONFIG_DYNAMIC_FTRACE)
 #define ARCH_SUPPORTS_FTRACE_SAVE_REGS 1
 #endif
 
arch/x86/kernel/entry_32.S +19 -0
index 01ccf9b..79c45a2 100644
@@ -1095,24 +1095,66 @@ ENTRY(mcount)
 	ret
 END(mcount)
 
+	.macro FTRACE_SAVE_ALL
+	/* eflags is saved on cs */
+	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 */

arch/x86/kernel/entry_32.S ENTRY(mcount) · line 1126

Quoted 10 lines
arch/x86/kernel/entry_32.S +10 -0
+	movl %edx, 14*4+4(%esp)	/* Store flags */
+	movl $__KERNEL_CS, %edx
+	movl %edx, 13*4+4(%esp)
+	.endm
+
+	.macro FTRACE_RESTORE_ALL
+	movl 14*4+4(%esp), %eax	/* Load flags */
+	movl %eax, 13*4+4(%esp)	/* Restore flags */
+	popl %eax
+	movl %eax, 14*4(%esp)	/* Restore return address */

I'm not sure we really need to restore all the regs. I'll keep this for now, but for an optimization, we can just restore the ones that mcount mcount needs to.

Or do you expect kprobes to change any of theses?

Quoted 30 lines
arch/x86/kernel/entry_32.S +19 -6
+	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
+	.endm
+
 ENTRY(ftrace_caller)
+	pushf	/* flags on regs->cs */
 	cmpl $0, function_trace_stop
-	jne  ftrace_stub
+	jne  ftrace_exit
+
+	FTRACE_SAVE_ALL
 
-	pushl %eax
-	pushl %ecx
-	pushl %edx
-	movl 0xc(%esp), %eax
 	movl 0x4(%ebp), %edx
-	subl $MCOUNT_INSN_SIZE, %eax
+	lea  4(%esp), %ecx
 

arch/x86/kernel/entry_32.S ENTRY(mcount) · line 1157

Quoted 10 lines
arch/x86/kernel/entry_32.S +3 -3
 .globl ftrace_call
 ftrace_call:
 	call ftrace_stub
 
-	popl %edx
-	popl %ecx
-	popl %eax
+	FTRACE_RESTORE_ALL
+
+	popf

Flags do not need to be restored. mcount calls give no guarantee that flags will be persistent.

-- Steve

Quoted 14 lines
arch/x86/kernel/entry_32.S +4 -0
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 .globl ftrace_graph_call
 ftrace_graph_call:
@@ -1122,6 +1164,10 @@ ftrace_graph_call:
 .globl ftrace_stub
 ftrace_stub:
 	ret
+
+ftrace_exit:
+	popf
+	ret
 END(ftrace_caller)
 
 #else /* ! CONFIG_DYNAMIC_FTRACE */
#5 lore raw

Re: [PATCH -tip v2 3/9] ftrace/x86: Support SAVE_REGS feature on i386

Hi -

Quoted 6 lines · Steven Rostedt

[...]
I'm not sure we really need to restore all the regs. I'll keep this for now, but for an optimization, we can just restore the ones that mcount mcount needs to.

Or do you expect kprobes to change any of theses?

That would be the way for a kprobe to modify variables/values that happen to be in the registers. In systemtap, for example:

# stap -g -e 'probe kernel.function("foo") { $bar = 1 }'

- FChE

maintainer
#6 lore raw

Re: [PATCH -tip v2 3/9] ftrace/x86: Support SAVE_REGS feature on i386

Quoted 8 lines · Frank Ch. Eigler

Hi -

Quoted 5 lines · Steven Rostedt

[...]
I'm not sure we really need to restore all the regs. I'll keep this for now, but for an optimization, we can just restore the ones that mcount mcount needs to.

Quoted 7 lines
Quoted 1 line

Or do you expect kprobes to change any of theses?

That would be the way for a kprobe to modify variables/values that happen to be in the registers. In systemtap, for example:

# stap -g -e 'probe kernel.function("foo") { $bar = 1 }'

And why would we want to allow this?

Modifying variables with probes is another way to lead to disaster. If the system did not intend for a variable to be a certain value, why let someone change it?

What real world example leads to external sources modifying internal core variables? With the obvious exception of rootkits.

-- Steve

#7 lore raw

Re: [PATCH -tip v2 3/9] ftrace/x86: Support SAVE_REGS feature on i386

Hi -

Quoted 8 lines
Quoted 3 lines

That would be the way for a kprobe to modify variables/values that happen to be in the registers. In systemtap, for example:

# stap -g -e 'probe kernel.function("foo") { $bar = 1 }'

And why would we want to allow this?
Modifying variables with probes is another way to lead to disaster. [...] What real world example leads to external sources modifying internal core variables? With the obvious exception of rootkits.

Among others, systemtap has been successfully used for fault injection for development/testing, as well as band-aids for kernel security vulnerabilities, where a small change of state can improve the state of the system. Obviously, this functionality is restricted to highly privileged users.

- FChE

#8 lore raw

Re: [PATCH -tip v2 3/9] ftrace/x86: Support SAVE_REGS feature on i386

Quoted 11 lines · Frank Ch. Eigler

Hi -

Quoted 8 lines
Quoted 3 lines

That would be the way for a kprobe to modify variables/values that happen to be in the registers. In systemtap, for example:

# stap -g -e 'probe kernel.function("foo") { $bar = 1 }'

And why would we want to allow this?
Modifying variables with probes is another way to lead to disaster. [...] What real world example leads to external sources modifying internal core variables? With the obvious exception of rootkits.

Quoted 5 lines

Among others, systemtap has been successfully used for fault injection for development/testing, as well as band-aids for kernel security vulnerabilities, where a small change of state can improve the state of the system. Obviously, this functionality is restricted to highly privileged users.

I agree with Frank. Register restoring should be done as far as the ftrace is used for kprobes. Of course, one reason is for the fault injection, which is very useful for debugging system failure. And another technical reason is that we should do "optimization" transparently. IMHO, kprobes works normally doing something, optimized kprobes also should do so.

But if you introduce FTRACE_OPS_FL_RSTR_REGS flag for restoring registers, it could be possible to provide corresponding flag from kprobes side. (perhaps KPROBE_FLAG_NOMODREGS? :))

Thank you,

Signature · 4 lines
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
maintainer
#9 lore raw

Re: [PATCH -tip v2 3/9] ftrace/x86: Support SAVE_REGS feature on i386

Quoted 10 lines · Masami Hiramatsu

I agree with Frank. Register restoring should be done as far as the ftrace is used for kprobes. Of course, one reason is for the fault injection, which is very useful for debugging system failure. And another technical reason is that we should do "optimization" transparently. IMHO, kprobes works normally doing something, optimized kprobes also should do so.

But if you introduce FTRACE_OPS_FL_RSTR_REGS flag for restoring registers, it could be possible to provide corresponding flag from kprobes side. (perhaps KPROBE_FLAG_NOMODREGS? :))

That's fine, my patch set restores the regs anyway.

-- Steve

#10 lore raw

Re: [PATCH -tip v2 3/9] ftrace/x86: Support SAVE_REGS feature on i386

In reply to Masami Hiramatsu

arch/x86/kernel/entry_32.S ENTRY(mcount) · line 1102

Quoted 6 lines · Masami Hiramatsu
arch/x86/kernel/entry_32.S +5 -0
+	.macro FTRACE_SAVE_ALL
+	/* eflags is saved on cs */
+	subl $8, %esp		/* skip ip and orig_ax */
+	pushl %gs
+	pushl %fs

For pure in kernel use you don't need to save/restore fs/gs This is only needed on kernel/user space boundaries.

And for this usage probably also not flags

-Andi

Signature · 1 line
[email protected] -- Speaking for myself only
#11 lore raw

Re: [PATCH -tip v2 3/9] ftrace/x86: Support SAVE_REGS feature on i386

Quoted 11 lines · Andi Kleen

arch/x86/kernel/entry_32.S ENTRY(mcount) · line 1102

Quoted 6 lines · Masami Hiramatsu
arch/x86/kernel/entry_32.S +5 -0
+	.macro FTRACE_SAVE_ALL
+	/* eflags is saved on cs */
+	subl $8, %esp		/* skip ip and orig_ax */
+	pushl %gs
+	pushl %fs

For pure in kernel use you don't need to save/restore fs/gs This is only needed on kernel/user space boundaries.

Quoted 1 line

And for this usage probably also not flags

I see, most of the case, user doesn't really need it. But in some case, it can be easily imagined that pt_regs is used for dumping all registers. Suppose that this ftrace-based kprobe optimization is done transparently, they may see that the results of dumping registers are different even if the probe moves just one instruction ahead. I'd like to avoid this kind of differences.

Thank you,

Signature · 4 lines
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
#12 lore raw

PATCH v2 4/9 ftrace: add ftrace_set_filter_ip() for address based filter

In reply to Masami Hiramatsu

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
index 11abe4e..f3c5487 100644
@@ -263,6 +263,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,
@@ -432,6 +434,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

#13 lore raw

[tip:perf/core] ftrace: add ftrace_set_filter_ip() for address based filter

Commit-ID:  647664eaf4033501739ac1f42dd52ce8c9266ccc
Gitweb:     http://git.kernel.org/tip/647664eaf4033501739ac1f42dd52ce8c9266ccc
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Tue, 5 Jun 2012 19:28:08 +0900
Committer:  Steven Rostedt <[email protected]>
CommitDate: Tue, 31 Jul 2012 10:29:55 -0400

ftrace: add ftrace_set_filter_ip() for address based filter

Add a new filter update interface ftrace_set_filter_ip() to set ftrace filter by ip address, not only glob pattern.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Ananth N Mavinakayanahalli <[email protected]>
Cc "Frank Ch. Eigler" <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Frederic Weisbecker <[email protected]>
---
 include/linux/ftrace.h |    3 ++
 kernel/trace/ftrace.c  |   59 ++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 2 deletions(-)
include/linux/ftrace.h +3 -0
index 9962e95..3e71112 100644
@@ -317,6 +317,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,
@@ -544,6 +546,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

#14 lore raw

PATCH v2 5/9 kprobes: Inverse taking of module_mutex with kprobe_mutex

In reply to Masami Hiramatsu

From: 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.

Cc Masami Hiramatsu <[email protected]>

---

 kernel/kprobes.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
kernel/kprobes.c +2 -2
index c62b854..7a8a122 100644
@@ -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))
#15 lore raw

[tip:perf/core] kprobes: Inverse taking of module_mutex with kprobe_mutex

Commit-ID:  72ef3794c5cd5f5f0e6355c24a529224c449cd14
Gitweb:     http://git.kernel.org/tip/72ef3794c5cd5f5f0e6355c24a529224c449cd14
Author:     Steven Rostedt <[email protected]>
AuthorDate: Tue, 5 Jun 2012 19:28:14 +0900
Committer:  Steven Rostedt <[email protected]>
CommitDate: Tue, 31 Jul 2012 10:29:55 -0400

kprobes: Inverse taking of module_mutex with kprobe_mutex

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.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Ananth N Mavinakayanahalli <[email protected]>
Cc "Frank Ch. Eigler" <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Cc Masami Hiramatsu <[email protected]>
---
 kernel/kprobes.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
kernel/kprobes.c +2 -2
index c62b854..7a8a122 100644
@@ -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))
#16 lore raw

PATCH v2 6/9 kprobes: cleanup to separate probe-able check

In reply to Masami Hiramatsu

Separate probe-able address checking code from
register_kprobe().

---

 kernel/kprobes.c |   82 ++++++++++++++++++++++++++++++------------------------
 1 files changed, 45 insertions(+), 37 deletions(-)
kernel/kprobes.c +45 -37
index 7a8a122..6137fe3 100644
@@ -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);
 
#17 lore raw

[tip:perf/core] kprobes: cleanup to separate probe-able check

Commit-ID:  f7fa6ef0ded995aad68650a877198f70e44b7621
Gitweb:     http://git.kernel.org/tip/f7fa6ef0ded995aad68650a877198f70e44b7621
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Tue, 5 Jun 2012 19:28:20 +0900
Committer:  Steven Rostedt <[email protected]>
CommitDate: Tue, 31 Jul 2012 10:29:56 -0400

kprobes: cleanup to separate probe-able check

Separate probe-able address checking code from register_kprobe().

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Ananth N Mavinakayanahalli <[email protected]>
Cc "Frank Ch. Eigler" <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Frederic Weisbecker <[email protected]>
---
 kernel/kprobes.c |   82 +++++++++++++++++++++++++++++------------------------
 1 files changed, 45 insertions(+), 37 deletions(-)
kernel/kprobes.c +45 -37
index 7a8a122..6137fe3 100644
@@ -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);
 
#18 lore raw

PATCH v2 7/9 kprobes: Move locks into appropriate functions

In reply to Masami Hiramatsu

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 files changed, 42 insertions(+), 21 deletions(-)
kernel/kprobes.c +42 -21
index 6137fe3..9e47f44 100644
@@ -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)
#19 lore raw

[tip:perf/core] kprobes: Move locks into appropriate functions

Commit-ID:  25764288d8dc4792f0f487baf043ccfee5d8c2ba
Gitweb:     http://git.kernel.org/tip/25764288d8dc4792f0f487baf043ccfee5d8c2ba
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Tue, 5 Jun 2012 19:28:26 +0900
Committer:  Steven Rostedt <[email protected]>
CommitDate: Tue, 31 Jul 2012 10:29:57 -0400

kprobes: Move locks into appropriate functions

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.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Ananth N Mavinakayanahalli <[email protected]>
Cc "Frank Ch. Eigler" <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Frederic Weisbecker <[email protected]>
---
 kernel/kprobes.c |   63 ++++++++++++++++++++++++++++++++++++------------------
 1 files changed, 42 insertions(+), 21 deletions(-)
kernel/kprobes.c +42 -21
index 6137fe3..9e47f44 100644
@@ -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)
#20 lore raw

PATCH v2 8/9 kprobes: introduce ftrace based optimization

In reply to Masami Hiramatsu

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
index b6e1f8c..aa0d05e 100644
@@ -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

#21 lore raw

[tip:perf/core] kprobes: introduce ftrace based optimization

Commit-ID:  ae6aa16fdc163afe6b04b6c073ad4ddd4663c03b
Gitweb:     http://git.kernel.org/tip/ae6aa16fdc163afe6b04b6c073ad4ddd4663c03b
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Tue, 5 Jun 2012 19:28:32 +0900
Committer:  Steven Rostedt <[email protected]>
CommitDate: Tue, 31 Jul 2012 10:29:58 -0400

kprobes: introduce ftrace based optimization

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.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Ananth N Mavinakayanahalli <[email protected]>
Cc "Frank Ch. Eigler" <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Frederic Weisbecker <[email protected]>
---
 include/linux/kprobes.h |   27 ++++++++++++
 kernel/kprobes.c        |  105 +++++++++++++++++++++++++++++++++++++++++------
 2 files changed, 119 insertions(+), 13 deletions(-)
include/linux/kprobes.h +27 -0
index b6e1f8c..aa0d05e 100644
@@ -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

#22 lore raw

PATCH v2 9/9 kprobes/x86: ftrace based optimization for x86

In reply to Masami Hiramatsu

Add function tracer based kprobe optimization support handlers on x86. This allows kprobes to use function tracer for probing on mcount call.

---

 arch/x86/include/asm/kprobes.h |    1 +
 arch/x86/kernel/kprobes.c      |   48 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 49 insertions(+), 0 deletions(-)
arch/x86/include/asm/kprobes.h +1 -0
index 5478825..d3ddd17 100644
@@ -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

#23 lore raw

[tip:perf/core] kprobes/x86: ftrace based optimization for x86

Commit-ID:  e52538965119319447c0800c534da73142c27be2
Gitweb:     http://git.kernel.org/tip/e52538965119319447c0800c534da73142c27be2
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Tue, 5 Jun 2012 19:28:38 +0900
Committer:  Steven Rostedt <[email protected]>
CommitDate: Tue, 31 Jul 2012 10:29:59 -0400

kprobes/x86: ftrace based optimization for x86

Add function tracer based kprobe optimization support handlers on x86. This allows kprobes to use function tracer for probing on mcount call.

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Cc Ananth N Mavinakayanahalli <[email protected]>
Cc "Frank Ch. Eigler" <[email protected]>
Cc Andrew Morton <[email protected]>
Cc Frederic Weisbecker <[email protected]>

[ 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
index 5478825..d3ddd17 100644
@@ -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
index aa0d05e..23755ba 100644
@@ -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
index 69c16ef..35b4315 100644
@@ -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;
maintainer
#24 lore raw
In reply to Masami Hiramatsu
Quoted 7 lines · Masami Hiramatsu

Hi Steven,

Here is the version 2 patches for kprobe-ftrace. Since I didn't touch ftrace part in this version, old gcc bug still be there. So, if you have another implementation, prease replace former 3 patches in this series with your patch.

Thanks Masami,

I'll be incorporating these patches into my own set, and see how they fit. I would have had more done yesterday, but I'm also testing against PPC and discovered a bug in the PPC function tracing and was side tracked in fixing it (ended up being a new function that had to be tagged with 'notrace').

Quoted 2 lines

ftrace_ops.regs_func is still under discussion because older gcc doesn't support initalizing unnamed union member.

Right, I working on adding the ops and regs parameters for all users, and working on making sure this work for other archs. I'll be testing against i386, x86_64, PPC64, PPC32, and ARM (snowball board, and this is the perfect excuse for me to get my panda board running too :-)

Thanks!

-- Steve

Quoted 10 lines

In this version, I've removed notrace dependency from __kprobes. Instead, it just set filter correctly before adding ftrace_ops not to trace all functions.

Changes in v2:
- Fix ftrace_ops registering right after setting its filter.
- Unregister ftrace_ops if there is no kprobe useing.
- Remove notrace dependency from __kprobes macro.