dle-develop — Dependable Linux Effort mailing list for developers

[Dle-develop] [PATCH -tip v3 0/5] kprobes: batch (un)optimization support

[Masami H. <mhi...@re...>]

Hi,

Here is batch optimizing patch series version 3.

Since current kprobes jump optimization calls stop_machine() for each
probe, it can make a lot latency noise when (un)registering a lot of
probes (~1000) at once. For example, on 4 core Xeon, 256 probes
optimization takes 770us in average (max 3.3ms).
This patch series introduces batch (un)optimization which modifies
code with just one stop_machine(), and it improves optimization time
to 90us in average (max 330us).

- Introduce text_poke_smp_batch() which modifies multiple
  codes with one stop_machine().
- Limit how many probes can be (un)optimized at once.
- Introduce delayed unoptimization for batch processing.

text_poke_smp_batch() also helps Jason's Jump label to reduce
its overhead coming from text_poke_smp().

Changes in v3:
- Set kp.addr = NULL according to Ananth's comment.

Changes in v2:
- Add kprobes selftest bugfix patch.
- Add some comments about locks according to Mathieu's comment.
- Allocate working buffers when initializing kprobes, instead of
  using static arraies.
- Merge max optimization limit patch into batch optimizing patch.

Thank you,

---

Masami Hiramatsu (5):
      kprobes: Support delayed unoptimization
      kprobes/x86: Use text_poke_smp_batch
      x86: Introduce text_poke_smp_batch() for batch-code modifying
      [CLEANUP] kprobes: Remove redundant text_mutex lock in optimize
      [BUGFIX] kprobes: Fix selftest to clear flags field for reusing probes


 arch/x86/include/asm/alternative.h |    7 +
 arch/x86/include/asm/kprobes.h     |    4 
 arch/x86/kernel/alternative.c      |   49 +++-
 arch/x86/kernel/kprobes.c          |  114 +++++++++
 include/linux/kprobes.h            |    4 
 kernel/kprobes.c                   |  442 ++++++++++++++++++++++++------------
 kernel/test_kprobes.c              |   12 +
 7 files changed, 464 insertions(+), 168 deletions(-)

-- 
Masami Hiramatsu
e-mail: mhi...@re...

Re: [Dle-develop] [PATCH -tip v2 1/5] [BUGFIX] kprobes: Fix selftest to clear flags field for reusing probes

[Ananth N M. <an...@in...>]

On Tue, May 18, 2010 at 04:12:51PM -0400, Masami Hiramatsu wrote:
 
>  kernel/test_kprobes.c |   12 +++++++++---
>  1 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/test_kprobes.c b/kernel/test_kprobes.c
> index 4f10451..ede20f4 100644
> --- a/kernel/test_kprobes.c
> +++ b/kernel/test_kprobes.c
> @@ -115,7 +115,9 @@ static int test_kprobes(void)
>  	int ret;
>  	struct kprobe *kps[2] = {&kp, &kp2};
> 
> -	kp.addr = 0; /* addr should be cleard for reusing kprobe. */
> +	/* addr and flags should be cleard for reusing kprobe. */
> +	kp.addr = 0;

kp.addr = NULL;

> +	kp.flags = 0;
>  	ret = register_kprobes(kps, 2);
>  	if (ret < 0) {
>  		printk(KERN_ERR "Kprobe smoke test failed: "
> @@ -210,7 +212,9 @@ static int test_jprobes(void)
>  	int ret;
>  	struct jprobe *jps[2] = {&jp, &jp2};
> 
> -	jp.kp.addr = 0; /* addr should be cleard for reusing kprobe. */
> +	/* addr and flags should be cleard for reusing kprobe. */
> +	jp.kp.addr = 0;

Same here...

> +	jp.kp.flags = 0;
>  	ret = register_jprobes(jps, 2);
>  	if (ret < 0) {
>  		printk(KERN_ERR "Kprobe smoke test failed: "
> @@ -323,7 +327,9 @@ static int test_kretprobes(void)
>  	int ret;
>  	struct kretprobe *rps[2] = {&rp, &rp2};
> 
> -	rp.kp.addr = 0; /* addr should be cleard for reusing kprobe. */
> +	/* addr and flags should be cleard for reusing kprobe. */
> +	rp.kp.addr = 0;

Here too.

Ananth

[Dle-develop] [PATCH -tip v2 4/5] kprobes/x86: Use text_poke_smp_batch

[Masami H. <mhi...@re...>]

Use text_poke_smp_batch() in optimization path for reducing
the number of stop_machine() issues. If the number of optimizing
probes is more than MAX_OPTIMIZE_PROBES(=256), kprobes optimizes
first MAX_OPTIMIZE_PROBES probes and kicks optimizer for remaining
probes.

Changes in v2:
- Allocate code buffer and parameters in arch_init_kprobes()
  instead of using static arraies.
- Merge previous max optimization limit patch into this patch.
  So, this patch introduces upper limit of optimization at
  once.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Ingo Molnar <mi...@el...>
Cc: Jim Keniston <jke...@us...>
Cc: Jason Baron <jb...@re...>
Cc: Mathieu Desnoyers <mat...@ef...>
---

 arch/x86/kernel/kprobes.c |   69 ++++++++++++++++++++++++++++++++++++++++-----
 include/linux/kprobes.h   |    2 +
 kernel/kprobes.c          |   11 ++-----
 3 files changed, 65 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index 345a4b1..50cf5d4 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -1385,10 +1385,16 @@ int __kprobes arch_prepare_optimized_kprobe(struct optimized_kprobe *op)
 	return 0;
 }
 
-/* Replace a breakpoint (int3) with a relative jump.  */
-int __kprobes arch_optimize_kprobe(struct optimized_kprobe *op)
+#define MAX_OPTIMIZE_PROBES 256
+static struct text_poke_param *jump_poke_params;
+static struct jump_poke_buffer {
+	u8 buf[RELATIVEJUMP_SIZE];
+} *jump_poke_bufs;
+
+static void __kprobes setup_optimize_kprobe(struct text_poke_param *tprm,
+					    u8 *insn_buf,
+					    struct optimized_kprobe *op)
 {
-	unsigned char jmp_code[RELATIVEJUMP_SIZE];
 	s32 rel = (s32)((long)op->optinsn.insn -
 			((long)op->kp.addr + RELATIVEJUMP_SIZE));
 
@@ -1396,16 +1402,39 @@ int __kprobes arch_optimize_kprobe(struct optimized_kprobe *op)
 	memcpy(op->optinsn.copied_insn, op->kp.addr + INT3_SIZE,
 	       RELATIVE_ADDR_SIZE);
 
-	jmp_code[0] = RELATIVEJUMP_OPCODE;
-	*(s32 *)(&jmp_code[1]) = rel;
+	insn_buf[0] = RELATIVEJUMP_OPCODE;
+	*(s32 *)(&insn_buf[1]) = rel;
+
+	tprm->addr = op->kp.addr;
+	tprm->opcode = insn_buf;
+	tprm->len = RELATIVEJUMP_SIZE;
+}
+
+/*
+ * Replace breakpoints (int3) with relative jumps.
+ * Caller must call with locking kprobe_mutex.
+ */
+void __kprobes arch_optimize_kprobes(struct list_head *oplist)
+{
+	struct optimized_kprobe *op, *tmp;
+	int c = 0;
+
+	list_for_each_entry_safe(op, tmp, oplist, list) {
+		WARN_ON(kprobe_disabled(&op->kp));
+		/* Setup param */
+		setup_optimize_kprobe(&jump_poke_params[c],
+				      jump_poke_bufs[c].buf, op);
+		list_del_init(&op->list);
+		if (++c >= MAX_OPTIMIZE_PROBES)
+			break;
+	}
 
 	/*
 	 * text_poke_smp doesn't support NMI/MCE code modifying.
 	 * However, since kprobes itself also doesn't support NMI/MCE
 	 * code probing, it's not a problem.
 	 */
-	text_poke_smp(op->kp.addr, jmp_code, RELATIVEJUMP_SIZE);
-	return 0;
+	text_poke_smp_batch(jump_poke_params, c);
 }
 
 /* Replace a relative jump with a breakpoint (int3).  */
@@ -1437,11 +1466,35 @@ static int  __kprobes setup_detour_execution(struct kprobe *p,
 	}
 	return 0;
 }
+
+static int __kprobes init_poke_params(void)
+{
+	/* Allocate code buffer and parameter array */
+	jump_poke_bufs = kmalloc(sizeof(struct jump_poke_buffer) *
+				 MAX_OPTIMIZE_PROBES, GFP_KERNEL);
+	if (!jump_poke_bufs)
+		return -ENOMEM;
+
+	jump_poke_params = kmalloc(sizeof(struct text_poke_param) *
+				   MAX_OPTIMIZE_PROBES, GFP_KERNEL);
+	if (!jump_poke_params) {
+		kfree(jump_poke_bufs);
+		jump_poke_bufs = NULL;
+		return -ENOMEM;
+	}
+
+	return 0;
+}
+#else	/* !CONFIG_OPTPROBES */
+static int __kprobes init_poke_params(void)
+{
+	return 0;
+}
 #endif
 
 int __init arch_init_kprobes(void)
 {
-	return 0;
+	return init_poke_params();
 }
 
 int __kprobes arch_trampoline_kprobe(struct kprobe *p)
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index e7d1b2e..fe157ba 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -275,7 +275,7 @@ extern int arch_prepared_optinsn(struct arch_optimized_insn *optinsn);
 extern int arch_check_optimized_kprobe(struct optimized_kprobe *op);
 extern int arch_prepare_optimized_kprobe(struct optimized_kprobe *op);
 extern void arch_remove_optimized_kprobe(struct optimized_kprobe *op);
-extern int  arch_optimize_kprobe(struct optimized_kprobe *op);
+extern void arch_optimize_kprobes(struct list_head *oplist);
 extern void arch_unoptimize_kprobe(struct optimized_kprobe *op);
 extern kprobe_opcode_t *get_optinsn_slot(void);
 extern void free_optinsn_slot(kprobe_opcode_t *slot, int dirty);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index f1351f2..6bd2e53 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -429,8 +429,6 @@ static DECLARE_DELAYED_WORK(optimizing_work, kprobe_optimizer);
 /* Kprobe jump optimizer */
 static __kprobes void kprobe_optimizer(struct work_struct *work)
 {
-	struct optimized_kprobe *op, *tmp;
-
 	/* Lock modules while optimizing kprobes */
 	mutex_lock(&module_mutex);
 	mutex_lock(&kprobe_mutex);
@@ -458,14 +456,11 @@ static __kprobes void kprobe_optimizer(struct work_struct *work)
 	 */
 	get_online_cpus();
 	mutex_lock(&text_mutex);
-	list_for_each_entry_safe(op, tmp, &optimizing_list, list) {
-		WARN_ON(kprobe_disabled(&op->kp));
-		if (arch_optimize_kprobe(op) < 0)
-			op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
-		list_del_init(&op->list);
-	}
+	arch_optimize_kprobes(&optimizing_list);
 	mutex_unlock(&text_mutex);
 	put_online_cpus();
+	if (!list_empty(&optimizing_list))
+		schedule_delayed_work(&optimizing_work, OPTIMIZE_DELAY);
 end:
 	mutex_unlock(&kprobe_mutex);
 	mutex_unlock(&module_mutex);


-- 
Masami Hiramatsu
e-mail: mhi...@re...

[Dle-develop] [PATCH -tip v2 5/5] kprobes: Support delayed unoptimization

[Masami H. <mhi...@re...>]

Unoptimization occurs when a probe is unregistered or disabled, and
is heavy because it recovers instructions by using stop_machine().
This patch delays unoptimization operations and unoptimize several
probes at once by using text_poke_smp_batch().
This can avoid unexpected system slowdown coming from stop_machine().

Changes in v2:
- Use dynamic allocated buffers and params.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Ingo Molnar <mi...@el...>
Cc: Jim Keniston <jke...@us...>
Cc: Jason Baron <jb...@re...>
Cc: Mathieu Desnoyers <mat...@ef...>
---

 arch/x86/include/asm/kprobes.h |    4 
 arch/x86/kernel/kprobes.c      |   45 ++++
 include/linux/kprobes.h        |    2 
 kernel/kprobes.c               |  434 +++++++++++++++++++++++++++-------------
 4 files changed, 344 insertions(+), 141 deletions(-)

diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h
index 5478825..e8a864c 100644
--- a/arch/x86/include/asm/kprobes.h
+++ b/arch/x86/include/asm/kprobes.h
@@ -79,12 +79,12 @@ struct arch_specific_insn {
 };
 
 struct arch_optimized_insn {
-	/* copy of the original instructions */
-	kprobe_opcode_t copied_insn[RELATIVE_ADDR_SIZE];
 	/* detour code buffer */
 	kprobe_opcode_t *insn;
 	/* the size of instructions copied to detour code buffer */
 	size_t size;
+	/* copy of the original instructions */
+	kprobe_opcode_t copied_insn[RELATIVE_ADDR_SIZE];
 };
 
 /* Return true (!0) if optinsn is prepared for optimization. */
diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index 50cf5d4..92a7114 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -1167,6 +1167,10 @@ static void __kprobes optimized_callback(struct optimized_kprobe *op,
 {
 	struct kprobe_ctlblk *kcb = get_kprobe_ctlblk();
 
+	/* This is possible if op is under delayed unoptimizing */
+	if (kprobe_disabled(&op->kp))
+		return;
+
 	preempt_disable();
 	if (kprobe_running()) {
 		kprobes_inc_nmissed_count(&op->kp);
@@ -1437,6 +1441,47 @@ void __kprobes arch_optimize_kprobes(struct list_head *oplist)
 	text_poke_smp_batch(jump_poke_params, c);
 }
 
+static void __kprobes setup_unoptimize_kprobe(struct text_poke_param *tprm,
+					      u8 *insn_buf,
+					      struct optimized_kprobe *op)
+{
+	/* Set int3 to first byte for kprobes */
+	insn_buf[0] = BREAKPOINT_INSTRUCTION;
+	memcpy(insn_buf + 1, op->optinsn.copied_insn, RELATIVE_ADDR_SIZE);
+
+	tprm->addr = op->kp.addr;
+	tprm->opcode = insn_buf;
+	tprm->len = RELATIVEJUMP_SIZE;
+}
+
+/*
+ * Recover original instructions and breakpoints from relative jumps.
+ * Caller must call with locking kprobe_mutex.
+ */
+extern void arch_unoptimize_kprobes(struct list_head *oplist,
+				    struct list_head *done_list)
+{
+	struct optimized_kprobe *op, *tmp;
+	int c = 0;
+
+	list_for_each_entry_safe(op, tmp, oplist, list) {
+		/* Setup param */
+		setup_unoptimize_kprobe(&jump_poke_params[c],
+					jump_poke_bufs[c].buf, op);
+		list_del_init(&op->list);
+		list_add(&op->list, done_list);
+		if (++c >= MAX_OPTIMIZE_PROBES)
+			break;
+	}
+
+	/*
+	 * text_poke_smp doesn't support NMI/MCE code modifying.
+	 * However, since kprobes itself also doesn't support NMI/MCE
+	 * code probing, it's not a problem.
+	 */
+	text_poke_smp_batch(jump_poke_params, c);
+}
+
 /* Replace a relative jump with a breakpoint (int3).  */
 void __kprobes arch_unoptimize_kprobe(struct optimized_kprobe *op)
 {
diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
index fe157ba..b78edb5 100644
--- a/include/linux/kprobes.h
+++ b/include/linux/kprobes.h
@@ -276,6 +276,8 @@ extern int arch_check_optimized_kprobe(struct optimized_kprobe *op);
 extern int arch_prepare_optimized_kprobe(struct optimized_kprobe *op);
 extern void arch_remove_optimized_kprobe(struct optimized_kprobe *op);
 extern void arch_optimize_kprobes(struct list_head *oplist);
+extern void arch_unoptimize_kprobes(struct list_head *oplist,
+				    struct list_head *done_list);
 extern void arch_unoptimize_kprobe(struct optimized_kprobe *op);
 extern kprobe_opcode_t *get_optinsn_slot(void);
 extern void free_optinsn_slot(kprobe_opcode_t *slot, int dirty);
diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 6bd2e53..ec57d08 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -353,6 +353,13 @@ static inline int kprobe_aggrprobe(struct kprobe *p)
 	return p->pre_handler == aggr_pre_handler;
 }
 
+/* Return true(!0) if the kprobe is unused */
+static inline int kprobe_unused(struct kprobe *p)
+{
+	return kprobe_aggrprobe(p) && kprobe_disabled(p) &&
+	       list_empty(&p->list);
+}
+
 /*
  * Keep all fields in the kprobe consistent
  */
@@ -383,6 +390,17 @@ void __kprobes opt_pre_handler(struct kprobe *p, struct pt_regs *regs)
 	}
 }
 
+/* Free optimized instructions and optimized_kprobe */
+static __kprobes void free_aggr_kprobe(struct kprobe *p)
+{
+	struct optimized_kprobe *op;
+
+	op = container_of(p, struct optimized_kprobe, kp);
+	arch_remove_optimized_kprobe(op);
+	arch_remove_kprobe(p);
+	kfree(op);
+}
+
 /* Return true(!0) if the kprobe is ready for optimization. */
 static inline int kprobe_optready(struct kprobe *p)
 {
@@ -396,6 +414,20 @@ static inline int kprobe_optready(struct kprobe *p)
 	return 0;
 }
 
+/* Return true(!0) if the kprobe is disarmed. Note: p must be on hash list */
+static inline int kprobe_disarmed(struct kprobe *p)
+{
+	struct optimized_kprobe *op;
+
+	/* If kprobe is not aggr/opt probe, just return kprobe is disabled */
+	if (!kprobe_aggrprobe(p))
+		return kprobe_disabled(p);
+
+	op = container_of(p, struct optimized_kprobe, kp);
+
+	return kprobe_disabled(p) && list_empty(&op->list);
+}
+
 /*
  * Return an optimized kprobe whose optimizing code replaces
  * instructions including addr (exclude breakpoint).
@@ -421,29 +453,19 @@ struct kprobe *__kprobes get_optimized_kprobe(unsigned long addr)
 
 /* Optimization staging list, protected by kprobe_mutex */
 static LIST_HEAD(optimizing_list);
+static LIST_HEAD(unoptimizing_list);
 
 static void kprobe_optimizer(struct work_struct *work);
 static DECLARE_DELAYED_WORK(optimizing_work, kprobe_optimizer);
 #define OPTIMIZE_DELAY 5
+static DECLARE_COMPLETION(optimizer_comp);
 
-/* Kprobe jump optimizer */
-static __kprobes void kprobe_optimizer(struct work_struct *work)
+static __kprobes void do_unoptimize_kprobes(struct list_head *free_list)
 {
-	/* Lock modules while optimizing kprobes */
-	mutex_lock(&module_mutex);
-	mutex_lock(&kprobe_mutex);
-	if (kprobes_all_disarmed || !kprobes_allow_optimization)
-		goto end;
-
-	/*
-	 * Wait for quiesence period to ensure all running interrupts
-	 * are done. Because optprobe may modify multiple instructions
-	 * there is a chance that Nth instruction is interrupted. In that
-	 * case, running interrupt can return to 2nd-Nth byte of jump
-	 * instruction. This wait is for avoiding it.
-	 */
-	synchronize_sched();
+	struct optimized_kprobe *op, *tmp;
 
+	if (list_empty(&unoptimizing_list))
+		return;
 	/*
 	 * The optimization/unoptimization refers online_cpus via
 	 * stop_machine() and cpu-hotplug modifies online_cpus.
@@ -456,14 +478,93 @@ static __kprobes void kprobe_optimizer(struct work_struct *work)
 	 */
 	get_online_cpus();
 	mutex_lock(&text_mutex);
+	/* Do batch unoptimization */
+	arch_unoptimize_kprobes(&unoptimizing_list, free_list);
+	mutex_unlock(&text_mutex);
+	put_online_cpus();
+
+	list_for_each_entry_safe(op, tmp, free_list, list) {
+		/* Disarm probes if disabled */
+		if (kprobe_disabled(&op->kp))
+			arch_disarm_kprobe(&op->kp);
+		/* leave only unused probes on free_list */
+		if (!kprobe_unused(&op->kp))
+			list_del_init(&op->list);
+		else
+			/*
+			 * Remove unused probe from hash list. After waiting
+			 * for synchronization, this probe can be freed safely
+			 * (which will be freed by do_free_unused_kprobes.)
+			 */
+			hlist_del_rcu(&op->kp.hlist);
+	}
+}
+
+static __kprobes void do_free_unused_kprobes(struct list_head *free_list)
+{
+	struct optimized_kprobe *op, *tmp;
+
+	list_for_each_entry_safe(op, tmp, free_list, list) {
+		list_del_init(&op->list);
+		free_aggr_kprobe(&op->kp);
+	}
+}
+
+static __kprobes void do_optimize_kprobes(void)
+{
+	/* Optimization never allowed when disarmed */
+	if (kprobes_all_disarmed || !kprobes_allow_optimization ||
+	    list_empty(&optimizing_list))
+		return;
+
+	get_online_cpus();
+	mutex_lock(&text_mutex);
 	arch_optimize_kprobes(&optimizing_list);
 	mutex_unlock(&text_mutex);
 	put_online_cpus();
-	if (!list_empty(&optimizing_list))
-		schedule_delayed_work(&optimizing_work, OPTIMIZE_DELAY);
-end:
+}
+
+/* Kprobe jump optimize/unoptimize and collect unused aggrprobes */
+static __kprobes void kprobe_optimizer(struct work_struct *work)
+{
+	LIST_HEAD(free_list);
+
+	/* Lock modules while optimizing kprobes */
+	mutex_lock(&module_mutex);
+	mutex_lock(&kprobe_mutex);
+
+	do_unoptimize_kprobes(&free_list);
+
+	/*
+	 * Wait for quiesence period to ensure all running interrupts
+	 * are done. Because optprobe may modify multiple instructions
+	 * there is a chance that Nth instruction is interrupted. In that
+	 * case, running interrupt can return to 2nd-Nth byte of jump
+	 * instruction. This wait is for avoiding it.
+	 */
+	synchronize_sched();
+
+	do_optimize_kprobes();
+
+	/* Release all releasable probes */
+	do_free_unused_kprobes(&free_list);
+
 	mutex_unlock(&kprobe_mutex);
 	mutex_unlock(&module_mutex);
+
+	if (!list_empty(&optimizing_list) || !list_empty(&unoptimizing_list)) {
+		if (!delayed_work_pending(&optimizing_work))
+			schedule_delayed_work(&optimizing_work, OPTIMIZE_DELAY);
+	} else
+		/* Wake up all waiters */
+		complete_all(&optimizer_comp);
+}
+
+/* Wait for completion */
+static __kprobes void wait_for_optimizer(void)
+{
+	if (delayed_work_pending(&optimizing_work))
+		wait_for_completion(&optimizer_comp);
 }
 
 /* Optimize kprobe if p is ready to be optimized */
@@ -476,6 +577,10 @@ static __kprobes void optimize_kprobe(struct kprobe *p)
 	    (kprobe_disabled(p) || kprobes_all_disarmed))
 		return;
 
+	/* Check if it is already optimized. */
+	if (kprobe_optimized(p))
+		return;
+
 	/* Both of break_handler and post_handler are not supported. */
 	if (p->break_handler || p->post_handler)
 		return;
@@ -486,45 +591,96 @@ static __kprobes void optimize_kprobe(struct kprobe *p)
 	if (arch_check_optimized_kprobe(op) < 0)
 		return;
 
-	/* Check if it is already optimized. */
-	if (op->kp.flags & KPROBE_FLAG_OPTIMIZED)
-		return;
-
 	op->kp.flags |= KPROBE_FLAG_OPTIMIZED;
-	list_add(&op->list, &optimizing_list);
-	if (!delayed_work_pending(&optimizing_work))
-		schedule_delayed_work(&optimizing_work, OPTIMIZE_DELAY);
+	if (!list_empty(&op->list))
+		/* Unoptimizing probe: dequeue probe */
+		list_del_init(&op->list);
+	else {
+		list_add(&op->list, &optimizing_list);
+		if (!delayed_work_pending(&optimizing_work))
+			schedule_delayed_work(&optimizing_work, OPTIMIZE_DELAY);
+	}
 }
 
-/* Unoptimize a kprobe if p is optimized */
-static __kprobes void unoptimize_kprobe(struct kprobe *p)
+static __kprobes void force_unoptimize_kprobe(struct optimized_kprobe *op)
+{
+	get_online_cpus();
+	arch_unoptimize_kprobe(op);
+	put_online_cpus();
+	if (kprobe_disabled(&op->kp))
+		arch_disarm_kprobe(&op->kp);
+}
+
+/* Unoptimize a kprobe if p is optimized. */
+static __kprobes void unoptimize_kprobe(struct kprobe *p, int force)
 {
 	struct optimized_kprobe *op;
 
-	if ((p->flags & KPROBE_FLAG_OPTIMIZED) && kprobe_aggrprobe(p)) {
-		op = container_of(p, struct optimized_kprobe, kp);
-		if (!list_empty(&op->list))
-			/* Dequeue from the optimization queue */
+	if (!kprobe_aggrprobe(p) || kprobe_disarmed(p))
+		return;	/* This is not an optprobe nor optimized */
+
+	op = container_of(p, struct optimized_kprobe, kp);
+	if (!kprobe_optimized(p)) {
+		/* Unoptimized or unoptimizing */
+		if (force && !list_empty(&op->list)) {
+			/*
+			 * If unoptimizing probe and forced option, forcibly
+			 * unoptimize it.
+			*/
 			list_del_init(&op->list);
-		else
-			/* Replace jump with break */
-			arch_unoptimize_kprobe(op);
-		op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
+			force_unoptimize_kprobe(op);
+		}
+		return;
+	}
+
+	op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
+	if (!list_empty(&op->list)) {
+		/* Dequeue from the optimization queue */
+		list_del_init(&op->list);
+		return;
+	}
+
+	if (force)	/* Forcibly update the code: this is a special case */
+		force_unoptimize_kprobe(op);
+	else {
+		list_add(&op->list, &unoptimizing_list);
+		if (!delayed_work_pending(&optimizing_work))
+			schedule_delayed_work(&optimizing_work, OPTIMIZE_DELAY);
 	}
 }
 
+/* Cancel unoptimizing for reusing */
+static void reuse_unused_kprobe(struct kprobe *ap)
+{
+	struct optimized_kprobe *op;
+
+	BUG_ON(!kprobe_unused(ap));
+	/*
+	 * Unused kprobe MUST be on the way of delayed unoptimizing (means
+	 * there is still a relative jump) and disabled.
+	 */
+	op = container_of(ap, struct optimized_kprobe, kp);
+	if (unlikely(list_empty(&op->list)))
+		printk(KERN_WARNING "Warning: found a stray unused "
+			"aggrprobe@%p\n", ap->addr);
+	/* Enable the probe again */
+	ap->flags &= ~KPROBE_FLAG_DISABLED;
+	/* Optimize it again (remove from op->list) */
+	BUG_ON(!kprobe_optready(ap));
+	optimize_kprobe(ap);
+}
+
 /* Remove optimized instructions */
 static void __kprobes kill_optimized_kprobe(struct kprobe *p)
 {
 	struct optimized_kprobe *op;
 
 	op = container_of(p, struct optimized_kprobe, kp);
-	if (!list_empty(&op->list)) {
-		/* Dequeue from the optimization queue */
+	if (!list_empty(&op->list))
+		/* Dequeue from the (un)optimization queue */
 		list_del_init(&op->list);
-		op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
-	}
-	/* Don't unoptimize, because the target code will be freed. */
+	op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
+	/* Don't unoptimize, because the target code is already freed. */
 	arch_remove_optimized_kprobe(op);
 }
 
@@ -537,16 +693,6 @@ static __kprobes void prepare_optimized_kprobe(struct kprobe *p)
 	arch_prepare_optimized_kprobe(op);
 }
 
-/* Free optimized instructions and optimized_kprobe */
-static __kprobes void free_aggr_kprobe(struct kprobe *p)
-{
-	struct optimized_kprobe *op;
-
-	op = container_of(p, struct optimized_kprobe, kp);
-	arch_remove_optimized_kprobe(op);
-	kfree(op);
-}
-
 /* Allocate new optimized_kprobe and try to prepare optimized instructions */
 static __kprobes struct kprobe *alloc_aggr_kprobe(struct kprobe *p)
 {
@@ -626,20 +772,15 @@ static void __kprobes unoptimize_all_kprobes(void)
 
 	kprobes_allow_optimization = false;
 	printk(KERN_INFO "Kprobes globally unoptimized\n");
-	get_online_cpus();	/* For avoiding text_mutex deadlock */
-	mutex_lock(&text_mutex);
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
 		hlist_for_each_entry_rcu(p, node, head, hlist) {
 			if (!kprobe_disabled(p))
-				unoptimize_kprobe(p);
+				unoptimize_kprobe(p, 0);
 		}
 	}
-
-	mutex_unlock(&text_mutex);
-	put_online_cpus();
-	/* Allow all currently running kprobes to complete */
-	synchronize_sched();
+	/* Wait for unoptimizing completion */
+	wait_for_optimizer();
 }
 
 int sysctl_kprobes_optimization;
@@ -670,37 +811,64 @@ static void __kprobes __arm_kprobe(struct kprobe *p)
 	/* Check collision with other optimized kprobes */
 	old_p = get_optimized_kprobe((unsigned long)p->addr);
 	if (unlikely(old_p))
-		unoptimize_kprobe(old_p); /* Fallback to unoptimized kprobe */
+		unoptimize_kprobe(old_p, 1);/* Fallback to unoptimized kprobe */
 
 	arch_arm_kprobe(p);
 	optimize_kprobe(p);	/* Try to optimize (add kprobe to a list) */
 }
 
+/* Return true(!0) if the probe is queued on (un)optimizing lists */
+static int __kprobes kprobe_queued(struct kprobe *p)
+{
+	struct optimized_kprobe *op;
+
+	if (kprobe_aggrprobe(p)) {
+		op = container_of(p, struct optimized_kprobe, kp);
+		if (!list_empty(&op->list))
+			return 1;
+	}
+	return 0;
+}
+
 static void __kprobes __disarm_kprobe(struct kprobe *p)
 {
 	struct kprobe *old_p;
 
-	unoptimize_kprobe(p);	/* Try to unoptimize */
-	arch_disarm_kprobe(p);
+	/* Delayed unoptimizing if needed (add kprobe to a list) */
+	if (kprobe_optimized(p))
+		unoptimize_kprobe(p, 0);
 
-	/* If another kprobe was blocked, optimize it. */
-	old_p = get_optimized_kprobe((unsigned long)p->addr);
-	if (unlikely(old_p))
-		optimize_kprobe(old_p);
+	if (!kprobe_queued(p)) {
+		arch_disarm_kprobe(p);
+		/* If another kprobe was blocked, optimize it. */
+		old_p = get_optimized_kprobe((unsigned long)p->addr);
+		if (unlikely(old_p))
+			optimize_kprobe(old_p);
+	}
 }
 
 #else /* !CONFIG_OPTPROBES */
 
 #define optimize_kprobe(p)			do {} while (0)
-#define unoptimize_kprobe(p)			do {} while (0)
+#define unoptimize_kprobe(p, f)			do {} while (0)
 #define kill_optimized_kprobe(p)		do {} while (0)
 #define prepare_optimized_kprobe(p)		do {} while (0)
 #define try_to_optimize_kprobe(p)		do {} while (0)
 #define __arm_kprobe(p)				arch_arm_kprobe(p)
 #define __disarm_kprobe(p)			arch_disarm_kprobe(p)
+#define kprobe_disarmed(p)			kprobe_disabled(p)
+#define wait_for_optimizer()			do {} while (0)
+
+/* There should be no unused kprobes can be reused without optimization */
+static void reuse_unused_kprobe(struct kprobe *ap)
+{
+	printk(KERN_ERR "Error: There should be no unused kprobe here.\n");
+	BUG_ON(kprobe_unused(ap));
+}
 
 static __kprobes void free_aggr_kprobe(struct kprobe *p)
 {
+	arch_remove_kprobe(ap);
 	kfree(p);
 }
 
@@ -723,16 +891,6 @@ static void __kprobes arm_kprobe(struct kprobe *kp)
 	mutex_unlock(&text_mutex);
 }
 
-/* Disarm a kprobe with text_mutex */
-static void __kprobes disarm_kprobe(struct kprobe *kp)
-{
-	get_online_cpus();	/* For avoiding text_mutex deadlock */
-	mutex_lock(&text_mutex);
-	__disarm_kprobe(kp);
-	mutex_unlock(&text_mutex);
-	put_online_cpus();
-}
-
 /*
  * Aggregate handlers for multiple kprobes support - these handlers
  * take care of invoking the individual kprobe handlers on p->list
@@ -931,7 +1089,7 @@ static int __kprobes add_new_kprobe(struct kprobe *ap, struct kprobe *p)
 	BUG_ON(kprobe_gone(ap) || kprobe_gone(p));
 
 	if (p->break_handler || p->post_handler)
-		unoptimize_kprobe(ap);	/* Fall back to normal kprobe */
+		unoptimize_kprobe(ap, 1);/* Fall back to normal kprobe */
 
 	if (p->break_handler) {
 		if (ap->break_handler)
@@ -994,7 +1152,9 @@ static int __kprobes register_aggr_kprobe(struct kprobe *old_p,
 		if (!ap)
 			return -ENOMEM;
 		init_aggr_kprobe(ap, old_p);
-	}
+	} else if (kprobe_unused(ap))
+		/* This probe is going to die. Rescue it */
+		reuse_unused_kprobe(ap);
 
 	if (kprobe_gone(ap)) {
 		/*
@@ -1028,23 +1188,6 @@ static int __kprobes register_aggr_kprobe(struct kprobe *old_p,
 	return add_new_kprobe(ap, p);
 }
 
-/* Try to disable aggr_kprobe, and return 1 if succeeded.*/
-static int __kprobes try_to_disable_aggr_kprobe(struct kprobe *p)
-{
-	struct kprobe *kp;
-
-	list_for_each_entry_rcu(kp, &p->list, list) {
-		if (!kprobe_disabled(kp))
-			/*
-			 * There is an active probe on the list.
-			 * We can't disable aggr_kprobe.
-			 */
-			return 0;
-	}
-	p->flags |= KPROBE_FLAG_DISABLED;
-	return 1;
-}
-
 static int __kprobes in_kprobes_functions(unsigned long addr)
 {
 	struct kprobe_blackpoint *kb;
@@ -1211,6 +1354,45 @@ out:
 }
 EXPORT_SYMBOL_GPL(register_kprobe);
 
+/* Check if all probes on the aggrprobe are disabled */
+static int __kprobes aggr_kprobe_disabled(struct kprobe *ap)
+{
+	struct kprobe *kp;
+
+	list_for_each_entry_rcu(kp, &ap->list, list) {
+		if (!kprobe_disabled(kp))
+			/*
+			 * There is an active probe on the list.
+			 * We can't disable aggr_kprobe.
+			 */
+			return 0;
+	}
+	return 1;
+}
+
+/* Disable one kprobe: Make sure called under kprobe_mutex is locked */
+static struct kprobe *__kprobes __disable_kprobe(struct kprobe *p)
+{
+	struct kprobe *orig_p;
+
+	/* Check whether specified probe is valid, and get real probe */
+	orig_p = __get_valid_kprobe(p);
+	if (unlikely(orig_p == NULL))
+		return NULL;
+
+	if (!kprobe_disabled(p)) {
+		if (p != orig_p)
+			p->flags |= KPROBE_FLAG_DISABLED;
+
+		if (p == orig_p || aggr_kprobe_disabled(orig_p)) {
+			__disarm_kprobe(orig_p);
+			orig_p->flags |= KPROBE_FLAG_DISABLED;
+		}
+	}
+
+	return orig_p;
+}
+
 /*
  * Unregister a kprobe without a scheduler synchronization.
  */
@@ -1218,22 +1400,18 @@ static int __kprobes __unregister_kprobe_top(struct kprobe *p)
 {
 	struct kprobe *old_p, *list_p;
 
-	old_p = __get_valid_kprobe(p);
+	/* Disable kprobe. This will disarm it if needed. */
+	old_p = __disable_kprobe(p);
 	if (old_p == NULL)
 		return -EINVAL;
 
 	if (old_p == p ||
 	    (kprobe_aggrprobe(old_p) &&
-	     list_is_singular(&old_p->list))) {
-		/*
-		 * Only probe on the hash list. Disarm only if kprobes are
-		 * enabled and not gone - otherwise, the breakpoint would
-		 * already have been removed. We save on flushing icache.
-		 */
-		if (!kprobes_all_disarmed && !kprobe_disabled(old_p))
-			disarm_kprobe(old_p);
+	     list_is_singular(&old_p->list) && kprobe_disarmed(old_p)))
+		/* Only disarmed probe on the hash list or on aggr_probe */
 		hlist_del_rcu(&old_p->hlist);
-	} else {
+	else {
+		/* This probe is on an aggr_probe */
 		if (p->break_handler && !kprobe_gone(p))
 			old_p->break_handler = NULL;
 		if (p->post_handler && !kprobe_gone(p)) {
@@ -1245,16 +1423,12 @@ static int __kprobes __unregister_kprobe_top(struct kprobe *p)
 		}
 noclean:
 		list_del_rcu(&p->list);
-		if (!kprobe_disabled(old_p)) {
-			try_to_disable_aggr_kprobe(old_p);
-			if (!kprobes_all_disarmed) {
-				if (kprobe_disabled(old_p))
-					disarm_kprobe(old_p);
-				else
-					/* Try to optimize this probe again */
-					optimize_kprobe(old_p);
-			}
-		}
+		if (!kprobe_disabled(old_p) && !kprobes_all_disarmed)
+			/*
+			 * Try to optimize this probe again,
+			 * because post hander may be changed.
+			 */
+			optimize_kprobe(old_p);
 	}
 	return 0;
 }
@@ -1263,13 +1437,13 @@ static void __kprobes __unregister_kprobe_bottom(struct kprobe *p)
 {
 	struct kprobe *old_p;
 
+	/* If this probe was removed from hlist, remove kprobes insn buffer */
 	if (list_empty(&p->list))
 		arch_remove_kprobe(p);
 	else if (list_is_singular(&p->list)) {
 		/* "p" is the last child of an aggr_kprobe */
 		old_p = list_entry(p->list.next, struct kprobe, list);
 		list_del(&p->list);
-		arch_remove_kprobe(old_p);
 		free_aggr_kprobe(old_p);
 	}
 }
@@ -1311,6 +1485,7 @@ void __kprobes unregister_kprobes(struct kprobe **kps, int num)
 	mutex_unlock(&kprobe_mutex);
 
 	synchronize_sched();
+
 	for (i = 0; i < num; i++)
 		if (kps[i]->addr)
 			__unregister_kprobe_bottom(kps[i]);
@@ -1588,29 +1763,10 @@ static void __kprobes kill_kprobe(struct kprobe *p)
 int __kprobes disable_kprobe(struct kprobe *kp)
 {
 	int ret = 0;
-	struct kprobe *p;
 
 	mutex_lock(&kprobe_mutex);
-
-	/* Check whether specified probe is valid. */
-	p = __get_valid_kprobe(kp);
-	if (unlikely(p == NULL)) {
+	if (__disable_kprobe(kp) == NULL)
 		ret = -EINVAL;
-		goto out;
-	}
-
-	/* If the probe is already disabled (or gone), just return */
-	if (kprobe_disabled(kp))
-		goto out;
-
-	kp->flags |= KPROBE_FLAG_DISABLED;
-	if (p != kp)
-		/* When kp != p, p is always enabled. */
-		try_to_disable_aggr_kprobe(p);
-
-	if (!kprobes_all_disarmed && kprobe_disabled(p))
-		disarm_kprobe(p);
-out:
 	mutex_unlock(&kprobe_mutex);
 	return ret;
 }
@@ -1931,8 +2087,8 @@ static void __kprobes disarm_all_kprobes(void)
 	mutex_unlock(&text_mutex);
 	put_online_cpus();
 	mutex_unlock(&kprobe_mutex);
-	/* Allow all currently running kprobes to complete */
-	synchronize_sched();
+
+	wait_for_optimizer();
 	return;
 
 already_disabled:


-- 
Masami Hiramatsu
e-mail: mhi...@re...

[Dle-develop] [PATCH -tip v2 3/5] x86: Introduce text_poke_smp_batch() for batch-code modifying

[Masami H. <mhi...@re...>]

Introduce text_poke_smp_batch(). This function modifies several
text areas with one stop_machine() on SMPr. Because calling
stop_machine() is heavy task, it is better to aggregate text_poke
requests.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Ingo Molnar <mi...@el...>
Cc: Jim Keniston <jke...@us...>
Cc: Jason Baron <jb...@re...>
Cc: Mathieu Desnoyers <mat...@ef...>
---

 arch/x86/include/asm/alternative.h |    7 +++++
 arch/x86/kernel/alternative.c      |   49 +++++++++++++++++++++++++++++-------
 2 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h
index 03b6bb5..9a08773 100644
--- a/arch/x86/include/asm/alternative.h
+++ b/arch/x86/include/asm/alternative.h
@@ -176,7 +176,14 @@ static inline void apply_paravirt(struct paravirt_patch_site *start,
  * On the local CPU you need to be protected again NMI or MCE handlers seeing an
  * inconsistent instruction while you patch.
  */
+struct text_poke_param {
+	void *addr;
+	const void *opcode;
+	size_t len;
+};
+
 extern void *text_poke(void *addr, const void *opcode, size_t len);
 extern void *text_poke_smp(void *addr, const void *opcode, size_t len);
+extern void text_poke_smp_batch(struct text_poke_param *params, int n);
 
 #endif /* _ASM_X86_ALTERNATIVE_H */
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 7023773..7256800 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -590,17 +590,21 @@ static atomic_t stop_machine_first;
 static int wrote_text;
 
 struct text_poke_params {
-	void *addr;
-	const void *opcode;
-	size_t len;
+	struct text_poke_param *params;
+	int nparams;
 };
 
 static int __kprobes stop_machine_text_poke(void *data)
 {
 	struct text_poke_params *tpp = data;
+	struct text_poke_param *p;
+	int i;
 
 	if (atomic_dec_and_test(&stop_machine_first)) {
-		text_poke(tpp->addr, tpp->opcode, tpp->len);
+		for (i = 0; i < tpp->nparams; i++) {
+			p = &tpp->params[i];
+			text_poke(p->addr, p->opcode, p->len);
+		}
 		smp_wmb();	/* Make sure other cpus see that this has run */
 		wrote_text = 1;
 	} else {
@@ -609,8 +613,12 @@ static int __kprobes stop_machine_text_poke(void *data)
 		smp_mb();	/* Load wrote_text before following execution */
 	}
 
-	flush_icache_range((unsigned long)tpp->addr,
-			   (unsigned long)tpp->addr + tpp->len);
+	for (i = 0; i < tpp->nparams; i++) {
+		p = &tpp->params[i];
+		flush_icache_range((unsigned long)p->addr,
+				   (unsigned long)p->addr + p->len);
+	}
+
 	return 0;
 }
 
@@ -630,13 +638,36 @@ static int __kprobes stop_machine_text_poke(void *data)
 void *__kprobes text_poke_smp(void *addr, const void *opcode, size_t len)
 {
 	struct text_poke_params tpp;
+	struct text_poke_param p;
 
-	tpp.addr = addr;
-	tpp.opcode = opcode;
-	tpp.len = len;
+	p.addr = addr;
+	p.opcode = opcode;
+	p.len = len;
+	tpp.params = &p;
+	tpp.nparams = 1;
 	atomic_set(&stop_machine_first, 1);
 	wrote_text = 0;
 	stop_machine(stop_machine_text_poke, (void *)&tpp, NULL);
 	return addr;
 }
 
+/**
+ * text_poke_smp_batch - Update instructions on a live kernel on SMP
+ * @params: an array of text_poke parameters
+ * @n: the number of elements in params.
+ *
+ * Modify multi-byte instruction by using stop_machine() on SMP. Since the
+ * stop_machine() is heavy task, it is better to aggregate text_poke requests
+ * and do it once if possible.
+ *
+ * Note: Must be called under get_online_cpus() and text_mutex.
+ */
+void __kprobes text_poke_smp_batch(struct text_poke_param *params, int n)
+{
+	struct text_poke_params tpp = {.params = params, .nparams = n};
+
+	atomic_set(&stop_machine_first, 1);
+	wrote_text = 0;
+	stop_machine(stop_machine_text_poke, (void *)&tpp, NULL);
+}
+


-- 
Masami Hiramatsu
e-mail: mhi...@re...

[Dle-develop] [PATCH -tip v2 2/5] [CLEANUP] kprobes: Remove redundant text_mutex lock in optimize

[Masami H. <mhi...@re...>]

Remove text_mutex locking in optimize_all_kprobes, because
this function doesn't modify text. It simply queues probes on
optimization list for kprobe_optimizer worker thread.

Changes in v2:
 - Update changelog.
 - Add comments for (un)optimize_all_kprobes.
 - Update kprobe_mutex comment.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Ingo Molnar <mi...@el...>
Cc: Jim Keniston <jke...@us...>
Cc: Jason Baron <jb...@re...>
Cc: Mathieu Desnoyers <mat...@ef...>
---

 kernel/kprobes.c |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 282035f..f1351f2 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -73,7 +73,8 @@ static struct hlist_head kretprobe_inst_table[KPROBE_TABLE_SIZE];
 /* NOTE: change this value only with kprobe_mutex held */
 static bool kprobes_all_disarmed;
 
-static DEFINE_MUTEX(kprobe_mutex);	/* Protects kprobe_table */
+/* This protects kprobe_table and optimizing_list */
+static DEFINE_MUTEX(kprobe_mutex);
 static DEFINE_PER_CPU(struct kprobe *, kprobe_instance) = NULL;
 static struct {
 	spinlock_t lock ____cacheline_aligned_in_smp;
@@ -594,6 +595,7 @@ static __kprobes void try_to_optimize_kprobe(struct kprobe *p)
 }
 
 #ifdef CONFIG_SYSCTL
+/* This should be called with kprobe_mutex locked */
 static void __kprobes optimize_all_kprobes(void)
 {
 	struct hlist_head *head;
@@ -606,17 +608,16 @@ static void __kprobes optimize_all_kprobes(void)
 		return;
 
 	kprobes_allow_optimization = true;
-	mutex_lock(&text_mutex);
 	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
 		head = &kprobe_table[i];
 		hlist_for_each_entry_rcu(p, node, head, hlist)
 			if (!kprobe_disabled(p))
 				optimize_kprobe(p);
 	}
-	mutex_unlock(&text_mutex);
 	printk(KERN_INFO "Kprobes globally optimized\n");
 }
 
+/* This should be called with kprobe_mutex locked */
 static void __kprobes unoptimize_all_kprobes(void)
 {
 	struct hlist_head *head;


-- 
Masami Hiramatsu
e-mail: mhi...@re...

[Dle-develop] [PATCH -tip v2 1/5] [BUGFIX] kprobes: Fix selftest to clear flags field for reusing probes

[Masami H. <mhi...@re...>]

Fix selftest to clear flags field for reusing probes
because the flags field can be modified by Kprobes.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Ananth N Mavinakayanahalli <an...@in...>
Cc: Ingo Molnar <mi...@el...>
Cc: Jim Keniston <jke...@us...>
Cc: Jason Baron <jb...@re...>
Cc: Mathieu Desnoyers <mat...@ef...>
---

 kernel/test_kprobes.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/kernel/test_kprobes.c b/kernel/test_kprobes.c
index 4f10451..ede20f4 100644
--- a/kernel/test_kprobes.c
+++ b/kernel/test_kprobes.c
@@ -115,7 +115,9 @@ static int test_kprobes(void)
 	int ret;
 	struct kprobe *kps[2] = {&kp, &kp2};
 
-	kp.addr = 0; /* addr should be cleard for reusing kprobe. */
+	/* addr and flags should be cleard for reusing kprobe. */
+	kp.addr = 0;
+	kp.flags = 0;
 	ret = register_kprobes(kps, 2);
 	if (ret < 0) {
 		printk(KERN_ERR "Kprobe smoke test failed: "
@@ -210,7 +212,9 @@ static int test_jprobes(void)
 	int ret;
 	struct jprobe *jps[2] = {&jp, &jp2};
 
-	jp.kp.addr = 0; /* addr should be cleard for reusing kprobe. */
+	/* addr and flags should be cleard for reusing kprobe. */
+	jp.kp.addr = 0;
+	jp.kp.flags = 0;
 	ret = register_jprobes(jps, 2);
 	if (ret < 0) {
 		printk(KERN_ERR "Kprobe smoke test failed: "
@@ -323,7 +327,9 @@ static int test_kretprobes(void)
 	int ret;
 	struct kretprobe *rps[2] = {&rp, &rp2};
 
-	rp.kp.addr = 0; /* addr should be cleard for reusing kprobe. */
+	/* addr and flags should be cleard for reusing kprobe. */
+	rp.kp.addr = 0;
+	rp.kp.flags = 0;
 	ret = register_kretprobes(rps, 2);
 	if (ret < 0) {
 		printk(KERN_ERR "Kprobe smoke test failed: "


-- 
Masami Hiramatsu
e-mail: mhi...@re...

[Dle-develop] [PATCH -tip v2 0/5] kprobes: batch (un)optimization support

[Masami H. <mhi...@re...>]

Hi,

Here is the 2nd version of batch optimizing patch series.

Since current kprobes jump optimization calls stop_machine() for each
probe, it can make a lot latency noise when (un)registering a lot of
probes (~1000) at once. For example, on 4 core Xeon, 256 probes
optimization takes 770us in average (max 3.3ms).
This patch series introduces batch (un)optimization which modifies
code with just one stop_machine(), and it improves optimization time
to 90us in average (max 330us).

- Introduce text_poke_smp_batch() which modifies multiple
  codes with one stop_machine().
- Limit how many probes can be (un)optimized at once.
- Introduce delayed unoptimization for batch processing.

text_poke_smp_batch() also helps Jason's Jump label to reduce
its overhead coming from text_poke_smp().

Changes in v2:
- Add kprobes selftest bugfix patch.
- Add some comments about locks according to Mathieu's comment.
- Allocate working buffers when initializing kprobes, instead of
  using static arraies.
- Merge max optimization limit patch into batch optimizing patch.

Thank you,

---

Masami Hiramatsu (5):
      kprobes: Support delayed unoptimization
      kprobes/x86: Use text_poke_smp_batch
      x86: Introduce text_poke_smp_batch() for batch-code modifying
      [CLEANUP] kprobes: Remove redundant text_mutex lock in optimize
      [BUGFIX] kprobes: Fix selftest to clear flags field for reusing probes


 arch/x86/include/asm/alternative.h |    7 +
 arch/x86/include/asm/kprobes.h     |    4 
 arch/x86/kernel/alternative.c      |   49 +++-
 arch/x86/kernel/kprobes.c          |  114 +++++++++
 include/linux/kprobes.h            |    4 
 kernel/kprobes.c                   |  442 ++++++++++++++++++++++++------------
 kernel/test_kprobes.c              |   12 +
 7 files changed, 464 insertions(+), 168 deletions(-)

-- 
Masami Hiramatsu
e-mail: mhi...@re...

Re: [Dle-develop] [PATCH -tip 0/4] perf-probe updates: string support, etc.

[Masami H. <mhi...@re...>]

Masami Hiramatsu wrote:
> 
> Hi,
> 
> Here is a series of patches which improves perf probe to
> add below features.

Oops, I've found some bugs. I'll fix it and send again.

Thank you,

-- 
Masami Hiramatsu
e-mail: mhi...@re...

[Dle-develop] [PATCH -tip 1/4] tracing/kprobes: Support "string" type

[Masami H. <mhi...@re...>]

Support string type tracing and printing in kprobe-tracer.
This allows user to trace string data in kernel including
__user data. Note that sometimes __user data may not be
accessed if it is paged-out (sorry, but kprobes operation
should be done in atomic, we can not wait for page-in).

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Cc: Paul Mackerras <pa...@sa...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
Cc: Peter Zijlstra <pe...@in...>
Cc: Mike Galbraith <ef...@gm...>
Cc: Frederic Weisbecker <fwe...@gm...>
---

 Documentation/trace/kprobetrace.txt |    2 
 kernel/trace/trace_kprobe.c         |  356 +++++++++++++++++++++++++++--------
 2 files changed, 279 insertions(+), 79 deletions(-)

diff --git a/Documentation/trace/kprobetrace.txt b/Documentation/trace/kprobetrace.txt
index ec94748..5f77d94 100644
--- a/Documentation/trace/kprobetrace.txt
+++ b/Documentation/trace/kprobetrace.txt
@@ -42,7 +42,7 @@ Synopsis of kprobe_events
   +|-offs(FETCHARG) : Fetch memory at FETCHARG +|- offs address.(**)
   NAME=FETCHARG : Set NAME as the argument name of FETCHARG.
   FETCHARG:TYPE : Set TYPE as the type of FETCHARG. Currently, basic types
-		  (u8/u16/u32/u64/s8/s16/s32/s64) are supported.
+		  (u8/u16/u32/u64/s8/s16/s32/s64) and string are supported.
 
   (*) only for return probe.
   (**) this is useful for fetching a field of data structures.
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index a751432..43bcdf3 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -30,6 +30,8 @@
 #include <linux/ptrace.h>
 #include <linux/perf_event.h>
 #include <linux/stringify.h>
+#include <linux/limits.h>
+#include <linux/uaccess.h>
 #include <asm/bitsperlong.h>
 
 #include "trace.h"
@@ -38,6 +40,7 @@
 #define MAX_TRACE_ARGS 128
 #define MAX_ARGSTR_LEN 63
 #define MAX_EVENT_NAME_LEN 64
+#define MAX_STRING_SIZE PATH_MAX
 #define KPROBE_EVENT_SYSTEM "kprobes"
 
 /* Reserved field names */
@@ -58,14 +61,16 @@ const char *reserved_field_names[] = {
 };
 
 /* Printing function type */
-typedef int (*print_type_func_t)(struct trace_seq *, const char *, void *);
+typedef int (*print_type_func_t)(struct trace_seq *, const char *, void *,
+				 void *);
 #define PRINT_TYPE_FUNC_NAME(type)	print_type_##type
 #define PRINT_TYPE_FMT_NAME(type)	print_type_format_##type
 
 /* Printing  in basic type function template */
 #define DEFINE_BASIC_PRINT_TYPE_FUNC(type, fmt, cast)			\
 static __kprobes int PRINT_TYPE_FUNC_NAME(type)(struct trace_seq *s,	\
-						const char *name, void *data)\
+						const char *name,	\
+						void *data, void *ent)\
 {									\
 	return trace_seq_printf(s, " %s=" fmt, name, (cast)*(type *)data);\
 }									\
@@ -80,6 +85,41 @@ DEFINE_BASIC_PRINT_TYPE_FUNC(s16, "%d", int)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s32, "%ld", long)
 DEFINE_BASIC_PRINT_TYPE_FUNC(s64, "%lld", long long)
 
+typedef u32 data_rloc_t;	/* Data relative location */
+#define make_data_rloc(len, roffs)	\
+	(data_rloc_t)(((u32)(len) << 16) | ((u32)(roffs) & 0xffff))
+#define get_rloc_len(dl)	((data_rloc_t)(dl) >> 16)
+#define get_rloc_offs(dl)	((data_rloc_t)(dl) & 0xffff)
+static inline void *get_rloc_data(data_rloc_t *dl)
+{
+	return (u8 *)dl + get_rloc_offs(*dl);
+}
+/* For data_loc conversion */
+static inline void *get_loc_data(u32 *dl, void *ent)
+{
+	return (u8 *)ent + get_rloc_offs(*dl);
+}
+#define convert_rloc_to_loc(dl, offs)	((data_rloc_t)(dl) + (offs))
+
+/* For defining macros */
+typedef data_rloc_t string;
+typedef data_rloc_t string_size;
+
+/* Print type function for string type */
+static __kprobes int PRINT_TYPE_FUNC_NAME(string)(struct trace_seq *s,
+						  const char *name,
+						  void *data, void *ent)
+{
+	int len = *(u32 *)data >> 16;
+
+	if (!len)
+		return trace_seq_printf(s, " %s=(fault)", name);
+	else
+		return trace_seq_printf(s, " %s=\"%s\"", name,
+					(const char *)get_loc_data(data, ent));
+}
+static const char PRINT_TYPE_FMT_NAME(string)[] = "\\\"%s\\\"";
+
 /* Data fetch function type */
 typedef	void (*fetch_func_t)(struct pt_regs *, void *, void *);
 
@@ -94,32 +134,38 @@ static __kprobes void call_fetch(struct fetch_param *fprm,
 	return fprm->fn(regs, fprm->data, dest);
 }
 
-#define FETCH_FUNC_NAME(kind, type)	fetch_##kind##_##type
+#define FETCH_FUNC_NAME(method, type)	fetch_##method##_##type
 /*
  * Define macro for basic types - we don't need to define s* types, because
  * we have to care only about bitwidth at recording time.
  */
-#define DEFINE_BASIC_FETCH_FUNCS(kind)  \
-DEFINE_FETCH_##kind(u8)			\
-DEFINE_FETCH_##kind(u16)		\
-DEFINE_FETCH_##kind(u32)		\
-DEFINE_FETCH_##kind(u64)
-
-#define CHECK_BASIC_FETCH_FUNCS(kind, fn)	\
-	((FETCH_FUNC_NAME(kind, u8) == fn) ||	\
-	 (FETCH_FUNC_NAME(kind, u16) == fn) ||	\
-	 (FETCH_FUNC_NAME(kind, u32) == fn) ||	\
-	 (FETCH_FUNC_NAME(kind, u64) == fn))
+#define DEFINE_BASIC_FETCH_FUNCS(method) \
+DEFINE_FETCH_##method(u8)		\
+DEFINE_FETCH_##method(u16)		\
+DEFINE_FETCH_##method(u32)		\
+DEFINE_FETCH_##method(u64)
+
+#define CHECK_FETCH_FUNCS(method, fn)			\
+	(((FETCH_FUNC_NAME(method, u8) == fn) ||	\
+	  (FETCH_FUNC_NAME(method, u16) == fn) ||	\
+	  (FETCH_FUNC_NAME(method, u32) == fn) ||	\
+	  (FETCH_FUNC_NAME(method, u64) == fn) ||	\
+	  (FETCH_FUNC_NAME(method, string) == fn) ||	\
+	  (FETCH_FUNC_NAME(method, string_size) == fn)) \
+	 && (fn != NULL))
 
 /* Data fetch function templates */
 #define DEFINE_FETCH_reg(type)						\
 static __kprobes void FETCH_FUNC_NAME(reg, type)(struct pt_regs *regs,	\
-					  void *offset, void *dest)	\
+					void *offset, void *dest)	\
 {									\
 	*(type *)dest = (type)regs_get_register(regs,			\
 				(unsigned int)((unsigned long)offset));	\
 }
 DEFINE_BASIC_FETCH_FUNCS(reg)
+/* No string on the register */
+#define fetch_reg_string NULL
+#define fetch_reg_string_size NULL
 
 #define DEFINE_FETCH_stack(type)					\
 static __kprobes void FETCH_FUNC_NAME(stack, type)(struct pt_regs *regs,\
@@ -129,6 +175,9 @@ static __kprobes void FETCH_FUNC_NAME(stack, type)(struct pt_regs *regs,\
 				(unsigned int)((unsigned long)offset));	\
 }
 DEFINE_BASIC_FETCH_FUNCS(stack)
+/* No string on the stack entry */
+#define fetch_stack_string NULL
+#define fetch_stack_string_size NULL
 
 #define DEFINE_FETCH_retval(type)					\
 static __kprobes void FETCH_FUNC_NAME(retval, type)(struct pt_regs *regs,\
@@ -137,6 +186,9 @@ static __kprobes void FETCH_FUNC_NAME(retval, type)(struct pt_regs *regs,\
 	*(type *)dest = (type)regs_return_value(regs);			\
 }
 DEFINE_BASIC_FETCH_FUNCS(retval)
+/* No string on the retval */
+#define fetch_retval_string NULL
+#define fetch_retval_string_size NULL
 
 #define DEFINE_FETCH_memory(type)					\
 static __kprobes void FETCH_FUNC_NAME(memory, type)(struct pt_regs *regs,\
@@ -149,6 +201,63 @@ static __kprobes void FETCH_FUNC_NAME(memory, type)(struct pt_regs *regs,\
 		*(type *)dest = retval;					\
 }
 DEFINE_BASIC_FETCH_FUNCS(memory)
+/*
+ * Fetch a null-terminated string. Caller MUST set *(u32 *)dest with max
+ * length and relative data location.
+ */
+static __kprobes void FETCH_FUNC_NAME(memory, string)(struct pt_regs *regs,
+						      void *addr, void *dest)
+{
+	long ret;
+	int maxlen = get_rloc_len(*(data_rloc_t *)dest);
+	u8 *dst = get_rloc_data(dest);
+	u8 *src = addr;
+	mm_segment_t old_fs = get_fs();
+	if (!maxlen)
+		return;
+	/*
+	 * Try to get string again, since the string can be changed while
+	 * probing.
+	 */
+	set_fs(KERNEL_DS);
+	pagefault_disable();
+	do
+		ret = __copy_from_user_inatomic(dst++, src++, 1);
+	while (dst[-1] && ret == 0 && src - (u8 *)addr < maxlen);
+	dst[-1] = '\0';
+	pagefault_enable();
+	set_fs(old_fs);
+
+	if (ret < 0) {	/* Failed to fetch string */
+		((u8 *)get_rloc_data(dest))[0] = '\0';
+		*(data_rloc_t *)dest = make_data_rloc(0,
+					get_rloc_offs(*(data_rloc_t *)dest));
+	} else
+		*(data_rloc_t *)dest = make_data_rloc(src - (u8 *)addr,
+					get_rloc_offs(*(data_rloc_t *)dest));
+}
+/* Return the length of string -- including null terminal byte */
+static __kprobes void FETCH_FUNC_NAME(memory, string_size)(struct pt_regs *regs,
+							void *addr, void *dest)
+{
+	int ret, len = 0;
+	u8 c;
+	mm_segment_t old_fs = get_fs();
+
+	set_fs(KERNEL_DS);
+	pagefault_disable();
+	do {
+		ret = __copy_from_user_inatomic(&c, (u8 *)addr + len, 1);
+		len++;
+	} while (c && ret == 0 && len < MAX_STRING_SIZE);
+	pagefault_enable();
+	set_fs(old_fs);
+
+	if (ret < 0)	/* Failed to check the length */
+		*(u32 *)dest = 0;
+	else
+		*(u32 *)dest = len;
+}
 
 /* Memory fetching by symbol */
 struct symbol_cache {
@@ -203,6 +312,8 @@ static __kprobes void FETCH_FUNC_NAME(symbol, type)(struct pt_regs *regs,\
 		*(type *)dest = 0;					\
 }
 DEFINE_BASIC_FETCH_FUNCS(symbol)
+DEFINE_FETCH_symbol(string)
+DEFINE_FETCH_symbol(string_size)
 
 /* Dereference memory access function */
 struct deref_fetch_param {
@@ -224,12 +335,14 @@ static __kprobes void FETCH_FUNC_NAME(deref, type)(struct pt_regs *regs,\
 		*(type *)dest = 0;					\
 }
 DEFINE_BASIC_FETCH_FUNCS(deref)
+DEFINE_FETCH_deref(string)
+DEFINE_FETCH_deref(string_size)
 
 static __kprobes void free_deref_fetch_param(struct deref_fetch_param *data)
 {
-	if (CHECK_BASIC_FETCH_FUNCS(deref, data->orig.fn))
+	if (CHECK_FETCH_FUNCS(deref, data->orig.fn))
 		free_deref_fetch_param(data->orig.data);
-	else if (CHECK_BASIC_FETCH_FUNCS(symbol, data->orig.fn))
+	else if (CHECK_FETCH_FUNCS(symbol, data->orig.fn))
 		free_symbol_cache(data->orig.data);
 	kfree(data);
 }
@@ -240,23 +353,43 @@ static __kprobes void free_deref_fetch_param(struct deref_fetch_param *data)
 #define DEFAULT_FETCH_TYPE _DEFAULT_FETCH_TYPE(BITS_PER_LONG)
 #define DEFAULT_FETCH_TYPE_STR __stringify(DEFAULT_FETCH_TYPE)
 
-#define ASSIGN_FETCH_FUNC(kind, type)	\
-	.kind = FETCH_FUNC_NAME(kind, type)
-
-#define ASSIGN_FETCH_TYPE(ptype, ftype, sign)	\
-	{.name = #ptype,			\
-	 .size = sizeof(ftype),			\
-	 .is_signed = sign,			\
-	 .print = PRINT_TYPE_FUNC_NAME(ptype),	\
-	 .fmt = PRINT_TYPE_FMT_NAME(ptype),	\
-ASSIGN_FETCH_FUNC(reg, ftype),			\
-ASSIGN_FETCH_FUNC(stack, ftype),		\
-ASSIGN_FETCH_FUNC(retval, ftype),		\
-ASSIGN_FETCH_FUNC(memory, ftype),		\
-ASSIGN_FETCH_FUNC(symbol, ftype),		\
-ASSIGN_FETCH_FUNC(deref, ftype),		\
+/* Fetch types */
+enum {
+	FETCH_MTD_reg = 0,
+	FETCH_MTD_stack,
+	FETCH_MTD_retval,
+	FETCH_MTD_memory,
+	FETCH_MTD_symbol,
+	FETCH_MTD_deref,
+	FETCH_MTD_END,
+};
+
+#define ASSIGN_FETCH_FUNC(method, type)	\
+	[FETCH_MTD_##method] = FETCH_FUNC_NAME(method, type)
+
+#define __ASSIGN_FETCH_TYPE(_name, ptype, ftype, _size, sign, _fmttype)	\
+	{.name = _name,				\
+	 .size = _size,					\
+	 .is_signed = sign,				\
+	 .print = PRINT_TYPE_FUNC_NAME(ptype),		\
+	 .fmt = PRINT_TYPE_FMT_NAME(ptype),		\
+	 .fmttype = _fmttype,				\
+	 .fetch = {					\
+ASSIGN_FETCH_FUNC(reg, ftype),				\
+ASSIGN_FETCH_FUNC(stack, ftype),			\
+ASSIGN_FETCH_FUNC(retval, ftype),			\
+ASSIGN_FETCH_FUNC(memory, ftype),			\
+ASSIGN_FETCH_FUNC(symbol, ftype),			\
+ASSIGN_FETCH_FUNC(deref, ftype),			\
+	  }						\
 	}
 
+#define ASSIGN_FETCH_TYPE(ptype, ftype, sign)			\
+	__ASSIGN_FETCH_TYPE(#ptype, ptype, ftype, sizeof(ftype), sign, #ptype)
+
+#define FETCH_TYPE_STRING 0
+#define FETCH_TYPE_STRSIZE 1
+
 /* Fetch type information table */
 static const struct fetch_type {
 	const char	*name;		/* Name of type */
@@ -264,14 +397,16 @@ static const struct fetch_type {
 	int		is_signed;	/* Signed flag */
 	print_type_func_t	print;	/* Print functions */
 	const char	*fmt;		/* Fromat string */
+	const char	*fmttype;	/* Name in format file */
 	/* Fetch functions */
-	fetch_func_t	reg;
-	fetch_func_t	stack;
-	fetch_func_t	retval;
-	fetch_func_t	memory;
-	fetch_func_t	symbol;
-	fetch_func_t	deref;
+	fetch_func_t	fetch[FETCH_MTD_END];
 } fetch_type_table[] = {
+	/* Special types */
+	[FETCH_TYPE_STRING] = __ASSIGN_FETCH_TYPE("string", string, string,
+					sizeof(u32), 1, "__data_loc char[]"),
+	[FETCH_TYPE_STRSIZE] = __ASSIGN_FETCH_TYPE("string_size", u32,
+					string_size, sizeof(u32), 0, "u32"),
+	/* Basic types */
 	ASSIGN_FETCH_TYPE(u8,  u8,  0),
 	ASSIGN_FETCH_TYPE(u16, u16, 0),
 	ASSIGN_FETCH_TYPE(u32, u32, 0),
@@ -302,12 +437,28 @@ static __kprobes void fetch_stack_address(struct pt_regs *regs,
 	*(unsigned long *)dest = kernel_stack_pointer(regs);
 }
 
+static fetch_func_t get_fetch_size_function(const struct fetch_type *type,
+					    fetch_func_t orig_fn)
+{
+	int i;
+
+	if (type != &fetch_type_table[FETCH_TYPE_STRING])
+		return NULL;	/* Only string type needs size function */
+	for (i = 0; i < FETCH_MTD_END; i++)
+		if (type->fetch[i] == orig_fn)
+			return fetch_type_table[FETCH_TYPE_STRSIZE].fetch[i];
+
+	WARN_ON(1);	/* This should not happen */
+	return NULL;
+}
+
 /**
  * Kprobe event core functions
  */
 
 struct probe_arg {
 	struct fetch_param	fetch;
+	struct fetch_param	fetch_size;
 	unsigned int		offset;	/* Offset from argument entry */
 	const char		*name;	/* Name of this argument */
 	const char		*comm;	/* Command of this argument */
@@ -428,9 +579,9 @@ error:
 
 static void free_probe_arg(struct probe_arg *arg)
 {
-	if (CHECK_BASIC_FETCH_FUNCS(deref, arg->fetch.fn))
+	if (CHECK_FETCH_FUNCS(deref, arg->fetch.fn))
 		free_deref_fetch_param(arg->fetch.data);
-	else if (CHECK_BASIC_FETCH_FUNCS(symbol, arg->fetch.fn))
+	else if (CHECK_FETCH_FUNCS(symbol, arg->fetch.fn))
 		free_symbol_cache(arg->fetch.data);
 	kfree(arg->name);
 	kfree(arg->comm);
@@ -547,7 +698,7 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
 
 	if (strcmp(arg, "retval") == 0) {
 		if (is_return)
-			f->fn = t->retval;
+			f->fn = t->fetch[FETCH_MTD_retval];
 		else
 			ret = -EINVAL;
 	} else if (strncmp(arg, "stack", 5) == 0) {
@@ -561,7 +712,7 @@ static int parse_probe_vars(char *arg, const struct fetch_type *t,
 			if (ret || param > PARAM_MAX_STACK)
 				ret = -EINVAL;
 			else {
-				f->fn = t->stack;
+				f->fn = t->fetch[FETCH_MTD_stack];
 				f->data = (void *)param;
 			}
 		} else
@@ -587,7 +738,7 @@ static int __parse_probe_arg(char *arg, const struct fetch_type *t,
 	case '%':	/* named register */
 		ret = regs_query_register_offset(arg + 1);
 		if (ret >= 0) {
-			f->fn = t->reg;
+			f->fn = t->fetch[FETCH_MTD_reg];
 			f->data = (void *)(unsigned long)ret;
 			ret = 0;
 		}
@@ -597,7 +748,7 @@ static int __parse_probe_arg(char *arg, const struct fetch_type *t,
 			ret = strict_strtoul(arg + 1, 0, &param);
 			if (ret)
 				break;
-			f->fn = t->memory;
+			f->fn = t->fetch[FETCH_MTD_memory];
 			f->data = (void *)param;
 		} else {
 			ret = split_symbol_offset(arg + 1, &offset);
@@ -605,7 +756,7 @@ static int __parse_probe_arg(char *arg, const struct fetch_type *t,
 				break;
 			f->data = alloc_symbol_cache(arg + 1, offset);
 			if (f->data)
-				f->fn = t->symbol;
+				f->fn = t->fetch[FETCH_MTD_symbol];
 		}
 		break;
 	case '+':	/* deref memory */
@@ -635,14 +786,17 @@ static int __parse_probe_arg(char *arg, const struct fetch_type *t,
 			if (ret)
 				kfree(dprm);
 			else {
-				f->fn = t->deref;
+				f->fn = t->fetch[FETCH_MTD_deref];
 				f->data = (void *)dprm;
 			}
 		}
 		break;
 	}
-	if (!ret && !f->fn)
+	if (!ret && !f->fn) {	/* Parsed, but do not find fetch method */
+		pr_info("%s type has no corresponding fetch method.\n",
+			t->name);
 		ret = -EINVAL;
+	}
 	return ret;
 }
 
@@ -651,6 +805,7 @@ static int parse_probe_arg(char *arg, struct trace_probe *tp,
 			   struct probe_arg *parg, int is_return)
 {
 	const char *t;
+	int ret;
 
 	if (strlen(arg) > MAX_ARGSTR_LEN) {
 		pr_info("Argument is too long.: %s\n",  arg);
@@ -673,7 +828,13 @@ static int parse_probe_arg(char *arg, struct trace_probe *tp,
 	}
 	parg->offset = tp->size;
 	tp->size += parg->type->size;
-	return __parse_probe_arg(arg, parg->type, &parg->fetch, is_return);
+	ret = __parse_probe_arg(arg, parg->type, &parg->fetch, is_return);
+	if (ret >= 0) {
+		parg->fetch_size.fn = get_fetch_size_function(parg->type,
+							      parg->fetch.fn);
+		parg->fetch_size.data = parg->fetch.data;
+	}
+	return ret;
 }
 
 /* Return 1 if name is reserved or already used by another argument */
@@ -1042,6 +1203,47 @@ static const struct file_operations kprobe_profile_ops = {
 	.release        = seq_release,
 };
 
+/* Sum up total data length for dynamic arraies (strings) */
+static __kprobes int __get_data_size(struct trace_probe *tp,
+				     struct pt_regs *regs)
+{
+	int i, ret = 0;
+	u32 len;
+
+	for (i = 0; i < tp->nr_args; i++)
+		if (unlikely(tp->args[i].fetch_size.fn)) {
+			call_fetch(&tp->args[i].fetch_size, regs, &len);
+			ret += len;
+		}
+
+	return ret;
+}
+
+/* Store the value of each argument */
+static __kprobes void store_trace_args(int ent_size, struct trace_probe *tp,
+				       struct pt_regs *regs,
+				       u8 *data, int maxlen)
+{
+	int i;
+	u32 end = tp->size;
+	data_rloc_t *dl;
+
+	for (i = 0; i < tp->nr_args; i++) {
+		if (unlikely(tp->args[i].fetch_size.fn)) {
+			dl = (data_rloc_t *)(data + tp->args[i].offset);
+			*dl = make_data_rloc(maxlen, end - tp->args[i].offset);
+			call_fetch(&tp->args[i].fetch, regs, dl);
+			end += get_rloc_len(*dl);
+			maxlen -= get_rloc_len(*dl);
+			/* Trick here, convert data_rloc to data_loc */
+			*dl = convert_rloc_to_loc(*dl,
+				 ent_size + tp->args[i].offset);
+		} else
+			call_fetch(&tp->args[i].fetch, regs,
+				   data + tp->args[i].offset);
+	}
+}
+
 /* Kprobe handler */
 static __kprobes void kprobe_trace_func(struct kprobe *kp, struct pt_regs *regs)
 {
@@ -1049,8 +1251,7 @@ static __kprobes void kprobe_trace_func(struct kprobe *kp, struct pt_regs *regs)
 	struct kprobe_trace_entry_head *entry;
 	struct ring_buffer_event *event;
 	struct ring_buffer *buffer;
-	u8 *data;
-	int size, i, pc;
+	int size, dsize, pc;
 	unsigned long irq_flags;
 	struct ftrace_event_call *call = &tp->call;
 
@@ -1059,7 +1260,8 @@ static __kprobes void kprobe_trace_func(struct kprobe *kp, struct pt_regs *regs)
 	local_save_flags(irq_flags);
 	pc = preempt_count();
 
-	size = sizeof(*entry) + tp->size;
+	dsize = __get_data_size(tp, regs);
+	size = sizeof(*entry) + tp->size + dsize;
 
 	event = trace_current_buffer_lock_reserve(&buffer, call->id, size,
 						  irq_flags, pc);
@@ -1068,9 +1270,7 @@ static __kprobes void kprobe_trace_func(struct kprobe *kp, struct pt_regs *regs)
 
 	entry = ring_buffer_event_data(event);
 	entry->ip = (unsigned long)kp->addr;
-	data = (u8 *)&entry[1];
-	for (i = 0; i < tp->nr_args; i++)
-		call_fetch(&tp->args[i].fetch, regs, data + tp->args[i].offset);
+	store_trace_args(sizeof(*entry), tp, regs, (u8 *)&entry[1], dsize);
 
 	if (!filter_current_check_discard(buffer, call, entry, event))
 		trace_nowake_buffer_unlock_commit(buffer, event, irq_flags, pc);
@@ -1084,15 +1284,15 @@ static __kprobes void kretprobe_trace_func(struct kretprobe_instance *ri,
 	struct kretprobe_trace_entry_head *entry;
 	struct ring_buffer_event *event;
 	struct ring_buffer *buffer;
-	u8 *data;
-	int size, i, pc;
+	int size, pc, dsize;
 	unsigned long irq_flags;
 	struct ftrace_event_call *call = &tp->call;
 
 	local_save_flags(irq_flags);
 	pc = preempt_count();
 
-	size = sizeof(*entry) + tp->size;
+	dsize = __get_data_size(tp, regs);
+	size = sizeof(*entry) + tp->size + dsize;
 
 	event = trace_current_buffer_lock_reserve(&buffer, call->id, size,
 						  irq_flags, pc);
@@ -1102,9 +1302,7 @@ static __kprobes void kretprobe_trace_func(struct kretprobe_instance *ri,
 	entry = ring_buffer_event_data(event);
 	entry->func = (unsigned long)tp->rp.kp.addr;
 	entry->ret_ip = (unsigned long)ri->ret_addr;
-	data = (u8 *)&entry[1];
-	for (i = 0; i < tp->nr_args; i++)
-		call_fetch(&tp->args[i].fetch, regs, data + tp->args[i].offset);
+	store_trace_args(sizeof(*entry), tp, regs, (u8 *)&entry[1], dsize);
 
 	if (!filter_current_check_discard(buffer, call, entry, event))
 		trace_nowake_buffer_unlock_commit(buffer, event, irq_flags, pc);
@@ -1137,7 +1335,7 @@ print_kprobe_event(struct trace_iterator *iter, int flags)
 	data = (u8 *)&field[1];
 	for (i = 0; i < tp->nr_args; i++)
 		if (!tp->args[i].type->print(s, tp->args[i].name,
-					     data + tp->args[i].offset))
+					     data + tp->args[i].offset, field))
 			goto partial;
 
 	if (!trace_seq_puts(s, "\n"))
@@ -1180,7 +1378,7 @@ print_kretprobe_event(struct trace_iterator *iter, int flags)
 	data = (u8 *)&field[1];
 	for (i = 0; i < tp->nr_args; i++)
 		if (!tp->args[i].type->print(s, tp->args[i].name,
-					     data + tp->args[i].offset))
+					     data + tp->args[i].offset, field))
 			goto partial;
 
 	if (!trace_seq_puts(s, "\n"))
@@ -1242,7 +1440,7 @@ static int kprobe_event_define_fields(struct ftrace_event_call *event_call)
 	DEFINE_FIELD(unsigned long, ip, FIELD_STRING_IP, 0);
 	/* Set argument names as fields */
 	for (i = 0; i < tp->nr_args; i++) {
-		ret = trace_define_field(event_call, tp->args[i].type->name,
+		ret = trace_define_field(event_call, tp->args[i].type->fmttype,
 					 tp->args[i].name,
 					 sizeof(field) + tp->args[i].offset,
 					 tp->args[i].type->size,
@@ -1264,7 +1462,7 @@ static int kretprobe_event_define_fields(struct ftrace_event_call *event_call)
 	DEFINE_FIELD(unsigned long, ret_ip, FIELD_STRING_RETIP, 0);
 	/* Set argument names as fields */
 	for (i = 0; i < tp->nr_args; i++) {
-		ret = trace_define_field(event_call, tp->args[i].type->name,
+		ret = trace_define_field(event_call, tp->args[i].type->fmttype,
 					 tp->args[i].name,
 					 sizeof(field) + tp->args[i].offset,
 					 tp->args[i].type->size,
@@ -1304,8 +1502,13 @@ static int __set_print_fmt(struct trace_probe *tp, char *buf, int len)
 	pos += snprintf(buf + pos, LEN_OR_ZERO, "\", %s", arg);
 
 	for (i = 0; i < tp->nr_args; i++) {
-		pos += snprintf(buf + pos, LEN_OR_ZERO, ", REC->%s",
-				tp->args[i].name);
+		if (strcmp(tp->args[i].type->name, "string") == 0)
+			pos += snprintf(buf + pos, LEN_OR_ZERO,
+					", __get_str(%s)",
+					tp->args[i].name);
+		else
+			pos += snprintf(buf + pos, LEN_OR_ZERO, ", REC->%s",
+					tp->args[i].name);
 	}
 
 #undef LEN_OR_ZERO
@@ -1341,12 +1544,12 @@ static __kprobes void kprobe_perf_func(struct kprobe *kp,
 	struct trace_probe *tp = container_of(kp, struct trace_probe, rp.kp);
 	struct ftrace_event_call *call = &tp->call;
 	struct kprobe_trace_entry_head *entry;
-	u8 *data;
-	int size, __size, i;
+	int size, __size, dsize;
 	unsigned long irq_flags;
 	int rctx;
 
-	__size = sizeof(*entry) + tp->size;
+	dsize = __get_data_size(tp, regs);
+	__size = sizeof(*entry) + tp->size + dsize;
 	size = ALIGN(__size + sizeof(u32), sizeof(u64));
 	size -= sizeof(u32);
 	if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE,
@@ -1358,9 +1561,8 @@ static __kprobes void kprobe_perf_func(struct kprobe *kp,
 		return;
 
 	entry->ip = (unsigned long)kp->addr;
-	data = (u8 *)&entry[1];
-	for (i = 0; i < tp->nr_args; i++)
-		call_fetch(&tp->args[i].fetch, regs, data + tp->args[i].offset);
+	memset(&entry[1], 0, dsize);
+	store_trace_args(sizeof(*entry), tp, regs, (u8 *)&entry[1], dsize);
 
 	perf_trace_buf_submit(entry, size, rctx, entry->ip, 1, irq_flags, regs);
 }
@@ -1372,12 +1574,12 @@ static __kprobes void kretprobe_perf_func(struct kretprobe_instance *ri,
 	struct trace_probe *tp = container_of(ri->rp, struct trace_probe, rp);
 	struct ftrace_event_call *call = &tp->call;
 	struct kretprobe_trace_entry_head *entry;
-	u8 *data;
-	int size, __size, i;
+	int size, __size, dsize;
 	unsigned long irq_flags;
 	int rctx;
 
-	__size = sizeof(*entry) + tp->size;
+	dsize = __get_data_size(tp, regs);
+	__size = sizeof(*entry) + tp->size + dsize;
 	size = ALIGN(__size + sizeof(u32), sizeof(u64));
 	size -= sizeof(u32);
 	if (WARN_ONCE(size > PERF_MAX_TRACE_SIZE,
@@ -1390,9 +1592,7 @@ static __kprobes void kretprobe_perf_func(struct kretprobe_instance *ri,
 
 	entry->func = (unsigned long)tp->rp.kp.addr;
 	entry->ret_ip = (unsigned long)ri->ret_addr;
-	data = (u8 *)&entry[1];
-	for (i = 0; i < tp->nr_args; i++)
-		call_fetch(&tp->args[i].fetch, regs, data + tp->args[i].offset);
+	store_trace_args(sizeof(*entry), tp, regs, (u8 *)&entry[1], dsize);
 
 	perf_trace_buf_submit(entry, size, rctx, entry->ret_ip, 1,
 			       irq_flags, regs);


-- 
Masami Hiramatsu
e-mail: mhi...@re...

[Dle-develop] [PATCH -tip 0/4] perf-probe updates: string support, etc.

[Masami H. <mhi...@re...>]

Hi,

Here is a series of patches which improves perf probe to
add below features.

- String support, which enables dynamic events to trace string
  variables instead of its address.

- Fixed-index array entry support, which allows user to trace
  an array entry with digit index, e.g. array[0].

- Global/static variable support, which allows user to trace
  global or static variables, as same as local one.


TODOs (possible features):
  - Support dynamic array-indexing (var[var2])
  - Support force type-casting ((type)var)
  - Show what deta-structure member is assigned to each argument.
  - Better support for probes on modules
  - More debugger like enhancements(%next, --disasm, etc.)

Thank you,

---

Masami Hiramatsu (4):
      perf probe: Support static and global variables
      perf probe: Support tracing an entry of array
      perf probe: Support "string" type
      tracing/kprobes: Support "string" type


 Documentation/trace/kprobetrace.txt     |    2 
 kernel/trace/trace_kprobe.c             |  356 ++++++++++++++++++++++++-------
 tools/perf/Documentation/perf-probe.txt |    4 
 tools/perf/util/probe-event.c           |   71 ++++--
 tools/perf/util/probe-event.h           |    1 
 tools/perf/util/probe-finder.c          |  192 +++++++++++++----
 6 files changed, 484 insertions(+), 142 deletions(-)

-- 
Masami Hiramatsu
e-mail: mhi...@re...

Re: [Dle-develop] [PATCH -tip 4/5] kprobes/x86: Use text_poke_smp_batch

[Mathieu D. <mat...@ef...>]

* Masami Hiramatsu (mhi...@re...) wrote:
> Mathieu Desnoyers wrote:
> > * Masami Hiramatsu (mhi...@re...) wrote:
> >> Mathieu Desnoyers wrote:
> >>> * Masami Hiramatsu (mhi...@re...) wrote:
> >>>> Use text_poke_smp_batch() in optimization path for reducing
> >>>> the number of stop_machine() issues.
> >>>>
> >>>> Signed-off-by: Masami Hiramatsu <mhi...@re...>
> >>>> Cc: Ananth N Mavinakayanahalli <an...@in...>
> >>>> Cc: Ingo Molnar <mi...@el...>
> >>>> Cc: Jim Keniston <jke...@us...>
> >>>> Cc: Jason Baron <jb...@re...>
> >>>> Cc: Mathieu Desnoyers <mat...@ef...>
> >>>> ---
> >>>>
> >>>>  arch/x86/kernel/kprobes.c |   37 ++++++++++++++++++++++++++++++-------
> >>>>  include/linux/kprobes.h   |    2 +-
> >>>>  kernel/kprobes.c          |   13 +------------
> >>>>  3 files changed, 32 insertions(+), 20 deletions(-)
> >>>>
> >>>> diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
> >>>> index 345a4b1..63a5c24 100644
> >>>> --- a/arch/x86/kernel/kprobes.c
> >>>> +++ b/arch/x86/kernel/kprobes.c
> >>>> @@ -1385,10 +1385,14 @@ int __kprobes arch_prepare_optimized_kprobe(struct optimized_kprobe *op)
> >>>>  	return 0;
> >>>>  }
> >>>>  
> >>>> -/* Replace a breakpoint (int3) with a relative jump.  */
> >>>> -int __kprobes arch_optimize_kprobe(struct optimized_kprobe *op)
> >>>> +#define MAX_OPTIMIZE_PROBES 256
> >>>
> >>> So what kind of interrupt latency does a 256-probes batch generate on the
> >>> system ?  Are we talking about a few milliseconds, a few seconds ?
> >>
> >> From my experiment on kvm/4cpu, it took about 3 seconds in average.
> > 
> > That's 3 seconds for multiple calls to stop_machine(). So we can expect
> > latencies in the area of few microseconds for each call, right ?
> 
> Sorry, my bad. Non tuned kvm guest is so slow...
> I've tried to check it again on *bare machine* (4core Xeon 2.33GHz, 4cpu).
> I found that even without this patch, optimizing 256 probes took 770us in
> average (min 150us, max 3.3ms.)
> With this patch, it went down to 90us in average (min 14us, max 324us!)
> 
> Isn't it enough low latency? :)
> 
> >> With this patch, it went down to 30ms. (x100 faster :))
> > 
> > This is beefing up the latency from few microseconds to 30ms. It sounds like a
> > regression rather than a gain to me.
> 
> so, it just takes 90us. I hope it is acceptable.

Yes, this is far below the scheduler tick, which is much more acceptable.

Thanks,

Mathieu

> 
> Thank you,
> 
> 
> -- 
> Masami Hiramatsu
> e-mail: mhi...@re...

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [Dle-develop] [PATCH -tip 4/5] kprobes/x86: Use text_poke_smp_batch

[Masami H. <mhi...@re...>]

Mathieu Desnoyers wrote:
> * Masami Hiramatsu (mhi...@re...) wrote:
>> Mathieu Desnoyers wrote:
>>> * Masami Hiramatsu (mhi...@re...) wrote:
>>>> Use text_poke_smp_batch() in optimization path for reducing
>>>> the number of stop_machine() issues.
>>>>
>>>> Signed-off-by: Masami Hiramatsu <mhi...@re...>
>>>> Cc: Ananth N Mavinakayanahalli <an...@in...>
>>>> Cc: Ingo Molnar <mi...@el...>
>>>> Cc: Jim Keniston <jke...@us...>
>>>> Cc: Jason Baron <jb...@re...>
>>>> Cc: Mathieu Desnoyers <mat...@ef...>
>>>> ---
>>>>
>>>>  arch/x86/kernel/kprobes.c |   37 ++++++++++++++++++++++++++++++-------
>>>>  include/linux/kprobes.h   |    2 +-
>>>>  kernel/kprobes.c          |   13 +------------
>>>>  3 files changed, 32 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
>>>> index 345a4b1..63a5c24 100644
>>>> --- a/arch/x86/kernel/kprobes.c
>>>> +++ b/arch/x86/kernel/kprobes.c
>>>> @@ -1385,10 +1385,14 @@ int __kprobes arch_prepare_optimized_kprobe(struct optimized_kprobe *op)
>>>>  	return 0;
>>>>  }
>>>>  
>>>> -/* Replace a breakpoint (int3) with a relative jump.  */
>>>> -int __kprobes arch_optimize_kprobe(struct optimized_kprobe *op)
>>>> +#define MAX_OPTIMIZE_PROBES 256
>>>
>>> So what kind of interrupt latency does a 256-probes batch generate on the
>>> system ?  Are we talking about a few milliseconds, a few seconds ?
>>
>> From my experiment on kvm/4cpu, it took about 3 seconds in average.
> 
> That's 3 seconds for multiple calls to stop_machine(). So we can expect
> latencies in the area of few microseconds for each call, right ?

Sorry, my bad. Non tuned kvm guest is so slow...
I've tried to check it again on *bare machine* (4core Xeon 2.33GHz, 4cpu).
I found that even without this patch, optimizing 256 probes took 770us in
average (min 150us, max 3.3ms.)
With this patch, it went down to 90us in average (min 14us, max 324us!)

Isn't it enough low latency? :)

>> With this patch, it went down to 30ms. (x100 faster :))
> 
> This is beefing up the latency from few microseconds to 30ms. It sounds like a
> regression rather than a gain to me.

so, it just takes 90us. I hope it is acceptable.

Thank you,


-- 
Masami Hiramatsu
e-mail: mhi...@re...

Re: [Dle-develop] [PATCH -tip 4/5] kprobes/x86: Use text_poke_smp_batch

[Masami H. <mhi...@re...>]

Mathieu Desnoyers wrote:
> * Masami Hiramatsu (mhi...@re...) wrote:
>> Mathieu Desnoyers wrote:
>>> * Masami Hiramatsu (mhi...@re...) wrote:
>>>> Mathieu Desnoyers wrote:
>>>>> * Masami Hiramatsu (mhi...@re...) wrote:
>>>>>> Use text_poke_smp_batch() in optimization path for reducing
>>>>>> the number of stop_machine() issues.
>>>>>>
>>>>>> Signed-off-by: Masami Hiramatsu <mhi...@re...>
>>>>>> Cc: Ananth N Mavinakayanahalli <an...@in...>
>>>>>> Cc: Ingo Molnar <mi...@el...>
>>>>>> Cc: Jim Keniston <jke...@us...>
>>>>>> Cc: Jason Baron <jb...@re...>
>>>>>> Cc: Mathieu Desnoyers <mat...@ef...>
>>>>>> ---
>>>>>>
>>>>>>  arch/x86/kernel/kprobes.c |   37 ++++++++++++++++++++++++++++++-------
>>>>>>  include/linux/kprobes.h   |    2 +-
>>>>>>  kernel/kprobes.c          |   13 +------------
>>>>>>  3 files changed, 32 insertions(+), 20 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
>>>>>> index 345a4b1..63a5c24 100644
>>>>>> --- a/arch/x86/kernel/kprobes.c
>>>>>> +++ b/arch/x86/kernel/kprobes.c
>>>>>> @@ -1385,10 +1385,14 @@ int __kprobes arch_prepare_optimized_kprobe(struct optimized_kprobe *op)
>>>>>>  	return 0;
>>>>>>  }
>>>>>>  
>>>>>> -/* Replace a breakpoint (int3) with a relative jump.  */
>>>>>> -int __kprobes arch_optimize_kprobe(struct optimized_kprobe *op)
>>>>>> +#define MAX_OPTIMIZE_PROBES 256
>>>>>
>>>>> So what kind of interrupt latency does a 256-probes batch generate on the
>>>>> system ?  Are we talking about a few milliseconds, a few seconds ?
>>>>
>>>> From my experiment on kvm/4cpu, it took about 3 seconds in average.
>>>
>>> That's 3 seconds for multiple calls to stop_machine(). So we can expect
>>> latencies in the area of few microseconds for each call, right ?
>>
>> Theoretically yes.
>> But if we register more than 1000 probes at once, it's hard to do
>> anything except optimizing a while(more than 10 sec), because
>> it stops machine so frequently.
>>
>>>> With this patch, it went down to 30ms. (x100 faster :))
>>>
>>> This is beefing up the latency from few microseconds to 30ms. It sounds like a
>>> regression rather than a gain to me.
>>
>> If it is not acceptable, I can add a knob for control how many probes
>> optimize/unoptimize at once. Anyway, it is expectable latency (after
>> registering/unregistering probes) and it will be small if we put a few probes.
>> (30ms is the worst case)
>> And if you want, it can be disabled by sysctl.
> 
> I think we are starting to see the stop_machine() approach is really limiting
> our ability to do even relatively small amount of work without hurting
> responsiveness significantly.
> 
> What's the current showstopper with the breakpoint-bypass-ipi approach that
> solves this issue properly and makes this batching approach unnecessary ?

We still do not have any official answer from chip vendors.
As you know, basic implementation has been done.

Thank you,

-- 
Masami Hiramatsu
e-mail: mhi...@re...

Re: [Dle-develop] [PATCH -tip 4/5] kprobes/x86: Use text_poke_smp_batch

[Mathieu D. <mat...@ef...>]

* Masami Hiramatsu (mhi...@re...) wrote:
> Mathieu Desnoyers wrote:
> > * Masami Hiramatsu (mhi...@re...) wrote:
> >> Mathieu Desnoyers wrote:
> >>> * Masami Hiramatsu (mhi...@re...) wrote:
> >>>> Use text_poke_smp_batch() in optimization path for reducing
> >>>> the number of stop_machine() issues.
> >>>>
> >>>> Signed-off-by: Masami Hiramatsu <mhi...@re...>
> >>>> Cc: Ananth N Mavinakayanahalli <an...@in...>
> >>>> Cc: Ingo Molnar <mi...@el...>
> >>>> Cc: Jim Keniston <jke...@us...>
> >>>> Cc: Jason Baron <jb...@re...>
> >>>> Cc: Mathieu Desnoyers <mat...@ef...>
> >>>> ---
> >>>>
> >>>>  arch/x86/kernel/kprobes.c |   37 ++++++++++++++++++++++++++++++-------
> >>>>  include/linux/kprobes.h   |    2 +-
> >>>>  kernel/kprobes.c          |   13 +------------
> >>>>  3 files changed, 32 insertions(+), 20 deletions(-)
> >>>>
> >>>> diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
> >>>> index 345a4b1..63a5c24 100644
> >>>> --- a/arch/x86/kernel/kprobes.c
> >>>> +++ b/arch/x86/kernel/kprobes.c
> >>>> @@ -1385,10 +1385,14 @@ int __kprobes arch_prepare_optimized_kprobe(struct optimized_kprobe *op)
> >>>>  	return 0;
> >>>>  }
> >>>>  
> >>>> -/* Replace a breakpoint (int3) with a relative jump.  */
> >>>> -int __kprobes arch_optimize_kprobe(struct optimized_kprobe *op)
> >>>> +#define MAX_OPTIMIZE_PROBES 256
> >>>
> >>> So what kind of interrupt latency does a 256-probes batch generate on the
> >>> system ?  Are we talking about a few milliseconds, a few seconds ?
> >>
> >> From my experiment on kvm/4cpu, it took about 3 seconds in average.
> > 
> > That's 3 seconds for multiple calls to stop_machine(). So we can expect
> > latencies in the area of few microseconds for each call, right ?
> 
> Theoretically yes.
> But if we register more than 1000 probes at once, it's hard to do
> anything except optimizing a while(more than 10 sec), because
> it stops machine so frequently.
> 
> >> With this patch, it went down to 30ms. (x100 faster :))
> > 
> > This is beefing up the latency from few microseconds to 30ms. It sounds like a
> > regression rather than a gain to me.
> 
> If it is not acceptable, I can add a knob for control how many probes
> optimize/unoptimize at once. Anyway, it is expectable latency (after
> registering/unregistering probes) and it will be small if we put a few probes.
> (30ms is the worst case)
> And if you want, it can be disabled by sysctl.

I think we are starting to see the stop_machine() approach is really limiting
our ability to do even relatively small amount of work without hurting
responsiveness significantly.

What's the current showstopper with the breakpoint-bypass-ipi approach that
solves this issue properly and makes this batching approach unnecessary ?

Thanks,

Mathieu

> 
> Thank you,
> 
> -- 
> Masami Hiramatsu
> e-mail: mhi...@re...

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [Dle-develop] [PATCH -tip 4/5] kprobes/x86: Use text_poke_smp_batch

[Masami H. <mhi...@re...>]

Mathieu Desnoyers wrote:
> * Masami Hiramatsu (mhi...@re...) wrote:
>> Mathieu Desnoyers wrote:
>>> * Masami Hiramatsu (mhi...@re...) wrote:
>>>> Use text_poke_smp_batch() in optimization path for reducing
>>>> the number of stop_machine() issues.
>>>>
>>>> Signed-off-by: Masami Hiramatsu <mhi...@re...>
>>>> Cc: Ananth N Mavinakayanahalli <an...@in...>
>>>> Cc: Ingo Molnar <mi...@el...>
>>>> Cc: Jim Keniston <jke...@us...>
>>>> Cc: Jason Baron <jb...@re...>
>>>> Cc: Mathieu Desnoyers <mat...@ef...>
>>>> ---
>>>>
>>>>  arch/x86/kernel/kprobes.c |   37 ++++++++++++++++++++++++++++++-------
>>>>  include/linux/kprobes.h   |    2 +-
>>>>  kernel/kprobes.c          |   13 +------------
>>>>  3 files changed, 32 insertions(+), 20 deletions(-)
>>>>
>>>> diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
>>>> index 345a4b1..63a5c24 100644
>>>> --- a/arch/x86/kernel/kprobes.c
>>>> +++ b/arch/x86/kernel/kprobes.c
>>>> @@ -1385,10 +1385,14 @@ int __kprobes arch_prepare_optimized_kprobe(struct optimized_kprobe *op)
>>>>  	return 0;
>>>>  }
>>>>  
>>>> -/* Replace a breakpoint (int3) with a relative jump.  */
>>>> -int __kprobes arch_optimize_kprobe(struct optimized_kprobe *op)
>>>> +#define MAX_OPTIMIZE_PROBES 256
>>>
>>> So what kind of interrupt latency does a 256-probes batch generate on the
>>> system ?  Are we talking about a few milliseconds, a few seconds ?
>>
>> From my experiment on kvm/4cpu, it took about 3 seconds in average.
> 
> That's 3 seconds for multiple calls to stop_machine(). So we can expect
> latencies in the area of few microseconds for each call, right ?

Theoretically yes.
But if we register more than 1000 probes at once, it's hard to do
anything except optimizing a while(more than 10 sec), because
it stops machine so frequently.

>> With this patch, it went down to 30ms. (x100 faster :))
> 
> This is beefing up the latency from few microseconds to 30ms. It sounds like a
> regression rather than a gain to me.

If it is not acceptable, I can add a knob for control how many probes
optimize/unoptimize at once. Anyway, it is expectable latency (after
registering/unregistering probes) and it will be small if we put a few probes.
(30ms is the worst case)
And if you want, it can be disabled by sysctl.

Thank you,

-- 
Masami Hiramatsu
e-mail: mhi...@re...

Re: [Dle-develop] [PATCH -tip 4/5] kprobes/x86: Use text_poke_smp_batch

[Mathieu D. <mat...@ef...>]

* Masami Hiramatsu (mhi...@re...) wrote:
> Mathieu Desnoyers wrote:
> > * Masami Hiramatsu (mhi...@re...) wrote:
> >> Use text_poke_smp_batch() in optimization path for reducing
> >> the number of stop_machine() issues.
> >>
> >> Signed-off-by: Masami Hiramatsu <mhi...@re...>
> >> Cc: Ananth N Mavinakayanahalli <an...@in...>
> >> Cc: Ingo Molnar <mi...@el...>
> >> Cc: Jim Keniston <jke...@us...>
> >> Cc: Jason Baron <jb...@re...>
> >> Cc: Mathieu Desnoyers <mat...@ef...>
> >> ---
> >>
> >>  arch/x86/kernel/kprobes.c |   37 ++++++++++++++++++++++++++++++-------
> >>  include/linux/kprobes.h   |    2 +-
> >>  kernel/kprobes.c          |   13 +------------
> >>  3 files changed, 32 insertions(+), 20 deletions(-)
> >>
> >> diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
> >> index 345a4b1..63a5c24 100644
> >> --- a/arch/x86/kernel/kprobes.c
> >> +++ b/arch/x86/kernel/kprobes.c
> >> @@ -1385,10 +1385,14 @@ int __kprobes arch_prepare_optimized_kprobe(struct optimized_kprobe *op)
> >>  	return 0;
> >>  }
> >>  
> >> -/* Replace a breakpoint (int3) with a relative jump.  */
> >> -int __kprobes arch_optimize_kprobe(struct optimized_kprobe *op)
> >> +#define MAX_OPTIMIZE_PROBES 256
> > 
> > So what kind of interrupt latency does a 256-probes batch generate on the
> > system ?  Are we talking about a few milliseconds, a few seconds ?
> 
> From my experiment on kvm/4cpu, it took about 3 seconds in average.

That's 3 seconds for multiple calls to stop_machine(). So we can expect
latencies in the area of few microseconds for each call, right ?

> With this patch, it went down to 30ms. (x100 faster :))

This is beefing up the latency from few microseconds to 30ms. It sounds like a
regression rather than a gain to me.

Thanks,

Mathieu

> 
> Thank you,
> -- 
> Masami Hiramatsu
> e-mail: mhi...@re...
> 

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [Dle-develop] [PATCH -tip 4/5] kprobes/x86: Use text_poke_smp_batch

[Masami H. <mhi...@re...>]

Mathieu Desnoyers wrote:
> * Masami Hiramatsu (mhi...@re...) wrote:
>> Use text_poke_smp_batch() in optimization path for reducing
>> the number of stop_machine() issues.
>>
>> Signed-off-by: Masami Hiramatsu <mhi...@re...>
>> Cc: Ananth N Mavinakayanahalli <an...@in...>
>> Cc: Ingo Molnar <mi...@el...>
>> Cc: Jim Keniston <jke...@us...>
>> Cc: Jason Baron <jb...@re...>
>> Cc: Mathieu Desnoyers <mat...@ef...>
>> ---
>>
>>  arch/x86/kernel/kprobes.c |   37 ++++++++++++++++++++++++++++++-------
>>  include/linux/kprobes.h   |    2 +-
>>  kernel/kprobes.c          |   13 +------------
>>  3 files changed, 32 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
>> index 345a4b1..63a5c24 100644
>> --- a/arch/x86/kernel/kprobes.c
>> +++ b/arch/x86/kernel/kprobes.c
>> @@ -1385,10 +1385,14 @@ int __kprobes arch_prepare_optimized_kprobe(struct optimized_kprobe *op)
>>  	return 0;
>>  }
>>  
>> -/* Replace a breakpoint (int3) with a relative jump.  */
>> -int __kprobes arch_optimize_kprobe(struct optimized_kprobe *op)
>> +#define MAX_OPTIMIZE_PROBES 256
> 
> So what kind of interrupt latency does a 256-probes batch generate on the
> system ?  Are we talking about a few milliseconds, a few seconds ?

>From my experiment on kvm/4cpu, it took about 3 seconds in average.
With this patch, it went down to 30ms. (x100 faster :))

Thank you,
-- 
Masami Hiramatsu
e-mail: mhi...@re...

Re: [Dle-develop] [PATCH -tip 1/5] [CLEANUP] kprobes: Remove redundant text_mutex lock in optimize

[Masami H. <mhi...@re...>]

Hi Mathieu,

Mathieu Desnoyers wrote:
> * Masami Hiramatsu (mhi...@re...) wrote:
>> Remove text_mutex locking in optimize_all_kprobes, because
>> that function doesn't modify text but just order optimization
>> to worker.
> 
> Hi Masami,
> 
> A few comments:
> 
> 1) optimize_all_kprobes/unoptimize_all_kprobes should have comments saying that
> they are always called with kprobe_mutex held.
> 
> 2) The sentence above in the changelog could be changed into:
> 
>  ..."because this function doesn't modify text. It simply queues optimizations
> for the kprobe_optimizer worker thread."
> 
> 3)
> 
> static DEFINE_MUTEX(kprobe_mutex);      /* Protects kprobe_table */
> 
> .. should also state that it protects optimizing_list.
> 

Thanks! all comments are good to me!

Thank you again,




-- 
Masami Hiramatsu
e-mail: mhi...@re...

Re: [Dle-develop] [PATCH -tip 4/5] kprobes/x86: Use text_poke_smp_batch

[Mathieu D. <mat...@ef...>]

* Masami Hiramatsu (mhi...@re...) wrote:
> Use text_poke_smp_batch() in optimization path for reducing
> the number of stop_machine() issues.
> 
> Signed-off-by: Masami Hiramatsu <mhi...@re...>
> Cc: Ananth N Mavinakayanahalli <an...@in...>
> Cc: Ingo Molnar <mi...@el...>
> Cc: Jim Keniston <jke...@us...>
> Cc: Jason Baron <jb...@re...>
> Cc: Mathieu Desnoyers <mat...@ef...>
> ---
> 
>  arch/x86/kernel/kprobes.c |   37 ++++++++++++++++++++++++++++++-------
>  include/linux/kprobes.h   |    2 +-
>  kernel/kprobes.c          |   13 +------------
>  3 files changed, 32 insertions(+), 20 deletions(-)
> 
> diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
> index 345a4b1..63a5c24 100644
> --- a/arch/x86/kernel/kprobes.c
> +++ b/arch/x86/kernel/kprobes.c
> @@ -1385,10 +1385,14 @@ int __kprobes arch_prepare_optimized_kprobe(struct optimized_kprobe *op)
>  	return 0;
>  }
>  
> -/* Replace a breakpoint (int3) with a relative jump.  */
> -int __kprobes arch_optimize_kprobe(struct optimized_kprobe *op)
> +#define MAX_OPTIMIZE_PROBES 256

So what kind of interrupt latency does a 256-probes batch generate on the
system ?  Are we talking about a few milliseconds, a few seconds ?

Thanks,

Mathieu

> +static struct text_poke_param jump_params[MAX_OPTIMIZE_PROBES];
> +static char jump_code_buf[MAX_OPTIMIZE_PROBES][RELATIVEJUMP_SIZE];
> +
> +static void __kprobes setup_optimize_kprobe(struct text_poke_param *tprm,
> +					    char *insn_buf,
> +					    struct optimized_kprobe *op)
>  {
> -	unsigned char jmp_code[RELATIVEJUMP_SIZE];
>  	s32 rel = (s32)((long)op->optinsn.insn -
>  			((long)op->kp.addr + RELATIVEJUMP_SIZE));
>  
> @@ -1396,16 +1400,35 @@ int __kprobes arch_optimize_kprobe(struct optimized_kprobe *op)
>  	memcpy(op->optinsn.copied_insn, op->kp.addr + INT3_SIZE,
>  	       RELATIVE_ADDR_SIZE);
>  
> -	jmp_code[0] = RELATIVEJUMP_OPCODE;
> -	*(s32 *)(&jmp_code[1]) = rel;
> +	insn_buf[0] = RELATIVEJUMP_OPCODE;
> +	*(s32 *)(&insn_buf[1]) = rel;
> +
> +	tprm->addr = op->kp.addr;
> +	tprm->opcode = insn_buf;
> +	tprm->len = RELATIVEJUMP_SIZE;
> +}
> +
> +/* Replace a breakpoint (int3) with a relative jump.  */
> +void __kprobes arch_optimize_kprobes(struct list_head *oplist)
> +{
> +	struct optimized_kprobe *op, *tmp;
> +	int c = 0;
> +
> +	list_for_each_entry_safe(op, tmp, oplist, list) {
> +		WARN_ON(kprobe_disabled(&op->kp));
> +		/* Setup param */
> +		setup_optimize_kprobe(&jump_params[c], jump_code_buf[c], op);
> +		list_del_init(&op->list);
> +		if (++c >= MAX_OPTIMIZE_PROBES)
> +			break;
> +	}
>  
>  	/*
>  	 * text_poke_smp doesn't support NMI/MCE code modifying.
>  	 * However, since kprobes itself also doesn't support NMI/MCE
>  	 * code probing, it's not a problem.
>  	 */
> -	text_poke_smp(op->kp.addr, jmp_code, RELATIVEJUMP_SIZE);
> -	return 0;
> +	text_poke_smp_batch(jump_params, c);
>  }
>  
>  /* Replace a relative jump with a breakpoint (int3).  */
> diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h
> index e7d1b2e..fe157ba 100644
> --- a/include/linux/kprobes.h
> +++ b/include/linux/kprobes.h
> @@ -275,7 +275,7 @@ extern int arch_prepared_optinsn(struct arch_optimized_insn *optinsn);
>  extern int arch_check_optimized_kprobe(struct optimized_kprobe *op);
>  extern int arch_prepare_optimized_kprobe(struct optimized_kprobe *op);
>  extern void arch_remove_optimized_kprobe(struct optimized_kprobe *op);
> -extern int  arch_optimize_kprobe(struct optimized_kprobe *op);
> +extern void arch_optimize_kprobes(struct list_head *oplist);
>  extern void arch_unoptimize_kprobe(struct optimized_kprobe *op);
>  extern kprobe_opcode_t *get_optinsn_slot(void);
>  extern void free_optinsn_slot(kprobe_opcode_t *slot, int dirty);
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index aae368a..c824c23 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -424,14 +424,10 @@ static LIST_HEAD(optimizing_list);
>  static void kprobe_optimizer(struct work_struct *work);
>  static DECLARE_DELAYED_WORK(optimizing_work, kprobe_optimizer);
>  #define OPTIMIZE_DELAY 5
> -#define MAX_OPTIMIZE_PROBES 64
>  
>  /* Kprobe jump optimizer */
>  static __kprobes void kprobe_optimizer(struct work_struct *work)
>  {
> -	struct optimized_kprobe *op, *tmp;
> -	int c = 0;
> -
>  	/* Lock modules while optimizing kprobes */
>  	mutex_lock(&module_mutex);
>  	mutex_lock(&kprobe_mutex);
> @@ -459,14 +455,7 @@ static __kprobes void kprobe_optimizer(struct work_struct *work)
>  	 */
>  	get_online_cpus();
>  	mutex_lock(&text_mutex);
> -	list_for_each_entry_safe(op, tmp, &optimizing_list, list) {
> -		WARN_ON(kprobe_disabled(&op->kp));
> -		if (arch_optimize_kprobe(op) < 0)
> -			op->kp.flags &= ~KPROBE_FLAG_OPTIMIZED;
> -		list_del_init(&op->list);
> -		if (++c >= MAX_OPTIMIZE_PROBES)
> -			break;
> -	}
> +	arch_optimize_kprobes(&optimizing_list);
>  	mutex_unlock(&text_mutex);
>  	put_online_cpus();
>  	if (!list_empty(&optimizing_list))
> 
> 
> -- 
> Masami Hiramatsu
> e-mail: mhi...@re...

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [Dle-develop] [PATCH -tip 1/5] [CLEANUP] kprobes: Remove redundant text_mutex lock in optimize

[Mathieu D. <mat...@ef...>]

* Masami Hiramatsu (mhi...@re...) wrote:
> Remove text_mutex locking in optimize_all_kprobes, because
> that function doesn't modify text but just order optimization
> to worker.

Hi Masami,

A few comments:

1) optimize_all_kprobes/unoptimize_all_kprobes should have comments saying that
they are always called with kprobe_mutex held.

2) The sentence above in the changelog could be changed into:

 ..."because this function doesn't modify text. It simply queues optimizations
for the kprobe_optimizer worker thread."

3)

static DEFINE_MUTEX(kprobe_mutex);      /* Protects kprobe_table */

.. should also state that it protects optimizing_list.

Thanks,

Mathieu

> 
> Signed-off-by: Masami Hiramatsu <mhi...@re...>
> Cc: Ananth N Mavinakayanahalli <an...@in...>
> Cc: Ingo Molnar <mi...@el...>
> Cc: Jim Keniston <jke...@us...>
> Cc: Jason Baron <jb...@re...>
> Cc: Mathieu Desnoyers <mat...@ef...>
> ---
> 
>  kernel/kprobes.c |    2 --
>  1 files changed, 0 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 282035f..1d34eef 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -606,14 +606,12 @@ static void __kprobes optimize_all_kprobes(void)
>  		return;
>  
>  	kprobes_allow_optimization = true;
> -	mutex_lock(&text_mutex);
>  	for (i = 0; i < KPROBE_TABLE_SIZE; i++) {
>  		head = &kprobe_table[i];
>  		hlist_for_each_entry_rcu(p, node, head, hlist)
>  			if (!kprobe_disabled(p))
>  				optimize_kprobe(p);
>  	}
> -	mutex_unlock(&text_mutex);
>  	printk(KERN_INFO "Kprobes globally optimized\n");
>  }
>  
> 
> 
> -- 
> Masami Hiramatsu
> e-mail: mhi...@re...

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com

Re: [Dle-develop] [PATCH -tip] perf probe: Check older elfutils and set NO_DWARF

[Robert R. <rob...@am...>]

On 11.05.10 00:59:53, Masami Hiramatsu wrote:
> Check whether elfutils is older than 0.138 (from which version checking
> routine has been introduced). And if so, set NO_DWARF because it is hard
> to check the API dependency without version checking.

This works for me, now the DWARF_SUPPORT macro is undefined and a
warning is generated:

Makefile:510: No libdw.h found or old libdw.h found or elfutils is older than 0.138, disables dwarf support. Please install new elfutils-devel/libdw-dev

Thanks,

-Robert

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: rob...@am...

Re: [Dle-develop] [PATCH -tip] perf probe: Don't compile CFI related code if elfutils is old

[Mark W. <mj...@re...>]

On Mon, 2010-05-10 at 15:19 -0400, Masami Hiramatsu wrote:
> Hm, I'd like to add just '#ifndef _ELFUTILS_PREREQ;#error;#endif' to
> NO_DWARF checking in Makefile, because it's easier to expand to another
> API dependency.

You might want to check systemtap/dwarf-wrappers.h which handles missing
_ELFUTILS_PREREQ, so you can use it as if it is always defined. That
said, 0.138 is pretty old (latest is 0.147), so just requiring something
fresh might not be a bad thing.

Cheers,

Mark

[Dle-develop] [tip:perf/urgent] kprobes/x86: Fix removed int3 checking order

[tip-bot f. M. H. <mhi...@re...>]

Commit-ID:  829e92458532b1dbfeb972435d45bb060cdbf5a3
Gitweb:     http://git.kernel.org/tip/829e92458532b1dbfeb972435d45bb060cdbf5a3
Author:     Masami Hiramatsu <mhi...@re...>
AuthorDate: Tue, 27 Apr 2010 18:33:49 -0400
Committer:  Ingo Molnar <mi...@el...>
CommitDate: Tue, 11 May 2010 09:14:25 +0200

kprobes/x86: Fix removed int3 checking order

Fix kprobe/x86 to check removed int3 when failing to get kprobe
from hlist. Since we have a time window between checking int3
exists on probed address and getting kprobe on that address,
we can have following scenario:

 -------
 CPU1                     CPU2
 hit int3
 check int3 exists
                          remove int3
                          remove kprobe from hlist
 get kprobe from hlist
 no kprobe->OOPS!
 -------

This patch moves int3 checking if there is no kprobe on that
address for fixing this problem as follows:

 ------
 CPU1                     CPU2
 hit int3
                          remove int3
                          remove kprobe from hlist
 get kprobe from hlist
 no kprobe->check int3 exists
          ->rollback&retry
 ------

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Acked-by: Ananth N Mavinakayanahalli <an...@in...>
Cc: systemtap <sys...@so...>
Cc: DLE <dle...@li...>
Cc: Dave Anderson <and...@re...>
Cc: Peter Zijlstra <a.p...@ch...>
Cc: Mike Galbraith <ef...@gm...>
Cc: Paul Mackerras <pa...@sa...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
Cc: Frederic Weisbecker <fwe...@gm...>
LKML-Reference: <20100427223348.2322.9112.stgit@localhost6.localdomain6>
Signed-off-by: Ingo Molnar <mi...@el...>
---
 arch/x86/kernel/kprobes.c |   27 +++++++++++++--------------
 1 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index b43bbae..1658efd 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -534,20 +534,6 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
 	struct kprobe_ctlblk *kcb;
 
 	addr = (kprobe_opcode_t *)(regs->ip - sizeof(kprobe_opcode_t));
-	if (*addr != BREAKPOINT_INSTRUCTION) {
-		/*
-		 * The breakpoint instruction was removed right
-		 * after we hit it.  Another cpu has removed
-		 * either a probepoint or a debugger breakpoint
-		 * at this address.  In either case, no further
-		 * handling of this interrupt is appropriate.
-		 * Back up over the (now missing) int3 and run
-		 * the original instruction.
-		 */
-		regs->ip = (unsigned long)addr;
-		return 1;
-	}
-
 	/*
 	 * We don't want to be preempted for the entire
 	 * duration of kprobe processing. We conditionally
@@ -579,6 +565,19 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
 				setup_singlestep(p, regs, kcb, 0);
 			return 1;
 		}
+	} else if (*addr != BREAKPOINT_INSTRUCTION) {
+		/*
+		 * The breakpoint instruction was removed right
+		 * after we hit it.  Another cpu has removed
+		 * either a probepoint or a debugger breakpoint
+		 * at this address.  In either case, no further
+		 * handling of this interrupt is appropriate.
+		 * Back up over the (now missing) int3 and run
+		 * the original instruction.
+		 */
+		regs->ip = (unsigned long)addr;
+		preempt_enable_no_resched();
+		return 1;
 	} else if (kprobe_running()) {
 		p = __get_cpu_var(current_kprobe);
 		if (p->break_handler && p->break_handler(p, regs)) {

[Dle-develop] [PATCH -tip] perf probe: Check older elfutils and set NO_DWARF

[Masami H. <mhi...@re...>]

Check whether elfutils is older than 0.138 (from which version checking
routine has been introduced). And if so, set NO_DWARF because it is hard
to check the API dependency without version checking.

Signed-off-by: Masami Hiramatsu <mhi...@re...>
Reported-by: Robert Richter <rob...@am...>
Cc: Arnaldo Carvalho de Melo <ac...@re...>
Cc: Ingo Molnar <mi...@el...>
---

 tools/perf/Makefile |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 0ef5cfe..b28bb73 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -506,8 +506,8 @@ PERFLIBS = $(LIB_FILE)
 -include config.mak
 
 ifndef NO_DWARF
-ifneq ($(shell sh -c "(echo '\#include <dwarf.h>'; echo '\#include <libdw.h>'; echo 'int main(void) { Dwarf *dbg; dbg = dwarf_begin(0, DWARF_C_READ); return (long)dbg; }') | $(CC) -x c - $(ALL_CFLAGS) -I/usr/include/elfutils -ldw -lelf -o $(BITBUCKET) $(ALL_LDFLAGS) $(EXTLIBS) "$(QUIET_STDERR)" && echo y"), y)
-	msg := $(warning No libdw.h found or old libdw.h found, disables dwarf support. Please install elfutils-devel/libdw-dev);
+ifneq ($(shell sh -c "(echo '\#include <dwarf.h>'; echo '\#include <libdw.h>'; echo '\#include <version.h>'; echo '\#ifndef _ELFUTILS_PREREQ'; echo '\#error'; echo '\#endif'; echo 'int main(void) { Dwarf *dbg; dbg = dwarf_begin(0, DWARF_C_READ); return (long)dbg; }') | $(CC) -x c - $(ALL_CFLAGS) -I/usr/include/elfutils -ldw -lelf -o $(BITBUCKET) $(ALL_LDFLAGS) $(EXTLIBS) "$(QUIET_STDERR)" && echo y"), y)
+	msg := $(warning No libdw.h found or old libdw.h found or elfutils is older than 0.138, disables dwarf support. Please install new elfutils-devel/libdw-dev);
 	NO_DWARF := 1
 endif # Dwarf support
 endif # NO_DWARF


-- 
Masami Hiramatsu
e-mail: mhi...@re...