This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [PATCH] kprobe-booster: boosting multi-probe
- From: "bibo,mao" <bibo dot mao at intel dot com>
- To: Masami Hiramatsu <hiramatu at sdl dot hitachi dot co dot jp>
- Cc: prasanna at in dot ibm dot com, systemtap at sources dot redhat dot com, Ananth N Mavinakayanahalli <ananth at in dot ibm dot com>, "Keshavamurthy, Anil S" <anil dot s dot keshavamurthy at intel dot com>, Jim Keniston <jkenisto at us dot ibm dot com>, Satoshi Oshima <soshima at redhat dot com>, Yumiko Sugita <sugita at sdl dot hitachi dot co dot jp>, Hideo Aoki <haoki at redhat dot com>, Maneesh Soni <maneesh at in dot ibm dot com>
- Date: Mon, 13 Mar 2006 14:29:10 +0800
- Subject: Re: [PATCH] kprobe-booster: boosting multi-probe
- References: <43F5C22C.5050702@sdl.hitachi.co.jp>
Hi Marami,
I refresh kprobe boost patch as follows, in this patch post_handler is
seperated from break_handler. And I think booster can be enabled even
when preempt is disabled.
--- 2.6.16-rc6-mm1.org/kernel/kprobes.c 2006-03-13 12:25:18.000000000 +0800
+++ 2.6.16-rc6-mm1/kernel/kprobes.c 2006-03-13 12:57:28.000000000 +0800
@@ -368,16 +368,15 @@ static inline void copy_kprobe(struct kp
*/
static int __kprobes add_new_kprobe(struct kprobe *old_p, struct
kprobe *p)
{
- struct kprobe *kp;
-
if (p->break_handler) {
- list_for_each_entry_rcu(kp, &old_p->list, list) {
- if (kp->break_handler)
- return -EEXIST;
- }
+ if (old_p->break_handler)
+ return -EEXIST;
list_add_tail_rcu(&p->list, &old_p->list);
+ old_p->break_handler = aggr_break_handler;
} else
list_add_rcu(&p->list, &old_p->list);
+ if (p->post_handler && !old_p->post_handler)
+ old_p->post_handler = aggr_post_handler;
return 0;
}
@@ -390,9 +389,12 @@ static inline void add_aggr_kprobe(struc
copy_kprobe(p, ap);
ap->addr = p->addr;
ap->pre_handler = aggr_pre_handler;
- ap->post_handler = aggr_post_handler;
ap->fault_handler = aggr_fault_handler;
- ap->break_handler = aggr_break_handler;
+ if (p->post_handler)
+ ap->post_handler = aggr_post_handler;
+ if (p->break_handler)
+ ap->break_handler = aggr_break_handler;
+
INIT_LIST_HEAD(&ap->list);
list_add_rcu(&p->list, &ap->list);
@@ -537,6 +539,18 @@ valid_p:
}
arch_remove_kprobe(p);
}
+ else{
+ mutex_lock(&kprobe_mutex);
+ if (p->break_handler)
+ old_p->break_handler = NULL;
+ if (p->post_handler){
+ list_for_each_entry_rcu(list_p, &old_p->list, list){
+ if (list_p->post_handler) break;
+ old_p->post_handler = NULL;
+ }
+ }
+ mutex_unlock(&kprobe_mutex);
+ }
}
static struct notifier_block kprobe_exceptions_nb = {
--- 2.6.16-rc6-mm1.org/arch/i386/kernel/kprobes.c 2006-03-13
12:41:01.000000000 +0800
+++ 2.6.16-rc6-mm1/arch/i386/kernel/kprobes.c 2006-03-13
13:00:21.000000000 +0800
@@ -205,9 +205,6 @@ static int __kprobes kprobe_handler(stru
int ret = 0;
kprobe_opcode_t *addr;
struct kprobe_ctlblk *kcb;
-#ifdef CONFIG_PREEMPT
- unsigned pre_preempt_count = preempt_count();
-#endif /* CONFIG_PREEMPT */
addr = (kprobe_opcode_t *)(regs->eip - sizeof(kprobe_opcode_t));
@@ -293,22 +290,14 @@ static int __kprobes kprobe_handler(stru
/* handler has already set things up, so skip ss setup */
return 1;
- if (p->ainsn.boostable == 1 &&
-#ifdef CONFIG_PREEMPT
- !(pre_preempt_count) && /*
- * This enables booster when the direct
- * execution path aren't preempted.
- */
-#endif /* CONFIG_PREEMPT */
- !p->post_handler && !p->break_handler ) {
+ss_probe:
+ if (p->ainsn.boostable == 1 && !p->post_handler) {
/* Boost up -- we can execute copied instructions directly */
reset_current_kprobe();
regs->eip = (unsigned long)p->ainsn.insn;
preempt_enable_no_resched();
return 1;
}
-
-ss_probe:
prepare_singlestep(p, regs);
kcb->kprobe_status = KPROBE_HIT_SS;
return 1;
Masami Hiramatsu wrote:
Hi, Prasanna
Prasanna S Panchamukhi wrote:
> Masami,
>
> Please see few coding style issues inline below marked with "^^^".
Thank you. I fixed those issues.
> Thanks
> Prasanna
>> +static inline void boost_aggr_kprobe(struct kprobe *ap)
>> +{
>> + struct kprobe *kp;
>> + if (ap->post_handler || ap->break_handler) {
> ^^^^^^^^^^^
> Could you please, leave a line after local variables as shown below
> struct kprobe *kp;
>
> if (ap->post_handler || ap->break_handler) {
>
>
>> kfree(old_p);
>> }
>> arch_remove_kprobe(p);
>> + } else {
>> + boost_aggr_kprobe(old_p);
>> }
> ^^^^^^^^^
> This does not look good, could you please remove the "{" for else
> condition, since it is just a single line, as shown below
> else
> boost_aggr_kprobe(old_p);
--
Masami HIRAMATSU
2nd Research Dept.
Hitachi, Ltd., Systems Development Laboratory
E-mail: hiramatu@sdl.hitachi.co.jp
kprobes.c | 28 ++++++++++++++++++++++++----
1 files changed, 24 insertions(+), 4 deletions(-)
diff -Narup a/kernel/kprobes.c b/kernel/kprobes.c
--- a/kernel/kprobes.c 2006-02-15 10:48:32.000000000 +0900
+++ b/kernel/kprobes.c 2006-02-17 19:03:21.000000000 +0900
@@ -370,6 +370,11 @@ static int __kprobes add_new_kprobe(stru
{
struct kprobe *kp;
+ if (p->post_handler || p->break_handler) { /* For more
boostability */
+ old_p->post_handler = aggr_post_handler;
+ old_p->break_handler = aggr_break_handler;
+ }
+
if (p->break_handler) {
list_for_each_entry_rcu(kp, &old_p->list, list) {
if (kp->break_handler)
@@ -390,16 +395,30 @@ static inline void add_aggr_kprobe(struc
copy_kprobe(p, ap);
ap->addr = p->addr;
ap->pre_handler = aggr_pre_handler;
- ap->post_handler = aggr_post_handler;
ap->fault_handler = aggr_fault_handler;
- ap->break_handler = aggr_break_handler;
-
+ if (p->post_handler || p->break_handler) { /* For more
boostability */
+ ap->post_handler = aggr_post_handler;
+ ap->break_handler = aggr_break_handler;
+ }
INIT_LIST_HEAD(&ap->list);
list_add_rcu(&p->list, &ap->list);
hlist_replace_rcu(&p->hlist, &ap->hlist);
}
+static inline void boost_aggr_kprobe(struct kprobe *ap)
+{
+ struct kprobe *kp;
+ if (ap->post_handler || ap->break_handler) {
+ list_for_each_entry_rcu(kp, &ap->list, list) {
+ if (kp->post_handler || kp->break_handler)
+ return;
+ }
+ }
+ ap->post_handler = NULL;
+ ap->break_handler = NULL;
+}
+
/*
* This is the second or subsequent kprobe at the address - handle
* the intricacies
@@ -536,7 +555,8 @@ valid_p:
kfree(old_p);
}
arch_remove_kprobe(p);
- }
+ } else
+ boost_aggr_kprobe(old_p);
}
static struct notifier_block kprobe_exceptions_nb = {