Re: [PATCH] kprobe-booster: boosting multi-probe

[Masami Hiramatsu <hiramatu@sdl.hitachi.co.jp>]

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 = {