From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26343 invoked by alias); 17 Feb 2006 12:31:51 -0000 Received: (qmail 26335 invoked by uid 22791); 17 Feb 2006 12:31:50 -0000 X-Spam-Status: No, hits=-2.4 required=5.0 tests=AWL,BAYES_00,UNPARSEABLE_RELAY X-Spam-Check-By: sourceware.org Received: from mail7.hitachi.co.jp (HELO mail7.hitachi.co.jp) (133.145.228.42) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 17 Feb 2006 12:31:46 +0000 Received: from mlsv3.hitachi.co.jp by mail7.hitachi.co.jp (8.9.3p3/3.7W-mail7) id VAA26326; Fri, 17 Feb 2006 21:31:43 +0900 Received: from mfilter-s1.hitachi.co.jp by mlsv3.hitachi.co.jp (8.12.10/8.12.10) id k1HCVfYV021652; Fri, 17 Feb 2006 21:31:42 +0900 Received: from vshuts1.hitachi.co.jp (unverified) by mfilter-s1.hitachi.co.jp (Content Technologies SMTPRS 4.3.17) with SMTP id ; Fri, 17 Feb 2006 21:31:41 +0900 Received: from hsdlgw92.sdl.hitachi.co.jp ([133.144.7.20]) by vshuts1.hitachi.co.jp with SMTP id M2006021721314130018 ; Fri, 17 Feb 2006 21:31:41 +0900 Received: from vgate2.sdl.hitachi.co.jp by hsdlgw92.sdl.hitachi.co.jp (8.9.3/3.7W01100113) id VAA14383; Fri, 17 Feb 2006 21:31:40 +0900 Received: from maila.sdl.hitachi.co.jp ([133.144.14.196]) by vgate2.sdl.hitachi.co.jp (SAVSMTP 3.1.1.32) with SMTP id M2006021721314013708 ; Fri, 17 Feb 2006 21:31:40 +0900 Received: from [192.168.16.226] ([192.168.16.226]) by maila.sdl.hitachi.co.jp (8.13.1/3.7W04031011) with ESMTP id k1HCVeuN008914; Fri, 17 Feb 2006 21:31:40 +0900 Message-ID: <43F5C22C.5050702@sdl.hitachi.co.jp> Date: Fri, 17 Feb 2006 12:31:00 -0000 From: Masami Hiramatsu User-Agent: Thunderbird 1.5 (Windows/20051201) MIME-Version: 1.0 To: prasanna@in.ibm.com CC: systemtap@sources.redhat.com, Ananth N Mavinakayanahalli , "Keshavamurthy, Anil S" , Jim Keniston , Satoshi Oshima , Yumiko Sugita , Hideo Aoki , Maneesh Soni Subject: Re: [PATCH] kprobe-booster: boosting multi-probe References: <43F59215.5040300@sdl.hitachi.co.jp> <20060217095256.GB6660@in.ibm.com> In-Reply-To: <20060217095256.GB6660@in.ibm.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org X-SW-Source: 2006-q1/txt/msg00545.txt.bz2 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 = {