public inbox for systemtap@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] kprobes patch
@ 2006-10-27 18:58 Jun Koi
  2006-10-27 20:45 ` Keshavamurthy, Anil S
  0 siblings, 1 reply; 6+ messages in thread
From: Jun Koi @ 2006-10-27 18:58 UTC (permalink / raw)
  To: prasanna, jkenisto, hien, systemtap

Hello,

I am not sure if this is a right place to send patch for kprobes, but
here it is.  This patch moves the reset command of kprobe instance
into the loop. (against 2.6.18)


Signed-off-by: Jun Koi <junkoi2004@gmail.com>


--- a/kernel/kprobes.c    2006-09-20 12:42:06.000000000 +0900
+++ b/kernel/kprobes.c        2006-10-28 03:52:27.000000000 +0900
@@ -204,8 +204,8 @@ static int __kprobes aggr_pre_handler(st
                        set_kprobe_instance(kp);
                        if (kp->pre_handler(kp, regs))
                                return 1;
+                       reset_kprobe_instance();
                }
-               reset_kprobe_instance();
        }
        return 0;
 }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kprobes patch
  2006-10-27 18:58 [PATCH] kprobes patch Jun Koi
@ 2006-10-27 20:45 ` Keshavamurthy, Anil S
  2006-10-28  1:21   ` Jun Koi
  0 siblings, 1 reply; 6+ messages in thread
From: Keshavamurthy, Anil S @ 2006-10-27 20:45 UTC (permalink / raw)
  To: Jun Koi; +Cc: prasanna, jkenisto, hien, systemtap

On Sat, Oct 28, 2006 at 03:58:18AM +0900, Jun Koi wrote:
> Hello,
> 
> I am not sure if this is a right place to send patch for kprobes, but
> here it is.  This patch moves the reset command of kprobe instance
> into the loop. (against 2.6.18)

Hi jun,
Yes, you have sent it to the right place. thanks.
BTW, is is helpful to know what problem did you encounter due to which 
you had come up with this patch. Or did you catch this bug by just code review?

> 
> 
> Signed-off-by: Jun Koi <junkoi2004@gmail.com>
> 
> 
> --- a/kernel/kprobes.c    2006-09-20 12:42:06.000000000 +0900
> +++ b/kernel/kprobes.c        2006-10-28 03:52:27.000000000 +0900
> @@ -204,8 +204,8 @@ static int __kprobes aggr_pre_handler(st
>                        set_kprobe_instance(kp);
>                        if (kp->pre_handler(kp, regs))
>                                return 1;
> +                       reset_kprobe_instance();
>                }
> -               reset_kprobe_instance();
>        }
>        return 0;
> }

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kprobes patch
  2006-10-27 20:45 ` Keshavamurthy, Anil S
@ 2006-10-28  1:21   ` Jun Koi
  2006-10-29 18:22     ` Vara Prasad
  2006-10-30 18:45     ` Keshavamurthy, Anil S
  0 siblings, 2 replies; 6+ messages in thread
From: Jun Koi @ 2006-10-28  1:21 UTC (permalink / raw)
  To: Keshavamurthy, Anil S; +Cc: prasanna, jkenisto, hien, systemtap

Hi Keshavanmurthy,

On 10/28/06, Keshavamurthy@bambi.jf.intel.com
<Keshavamurthy@bambi.jf.intel.com> wrote:
> On Sat, Oct 28, 2006 at 03:58:18AM +0900, Jun Koi wrote:
> > Hello,
> >
> > I am not sure if this is a right place to send patch for kprobes, but
> > here it is.  This patch moves the reset command of kprobe instance
> > into the loop. (against 2.6.18)
>
> Hi jun,
> Yes, you have sent it to the right place. thanks.
> BTW, is is helpful to know what problem did you encounter due to which
> you had come up with this patch. Or did you catch this bug by just code review?
>

I had no problem at al. You are right, the patch is a result of
reviewing the code.

Best,
Jun

> >
> > Signed-off-by: Jun Koi <junkoi2004@gmail.com>
> >
> >
> > --- a/kernel/kprobes.c    2006-09-20 12:42:06.000000000 +0900
> > +++ b/kernel/kprobes.c        2006-10-28 03:52:27.000000000 +0900
> > @@ -204,8 +204,8 @@ static int __kprobes aggr_pre_handler(st
> >                        set_kprobe_instance(kp);
> >                        if (kp->pre_handler(kp, regs))
> >                                return 1;
> > +                       reset_kprobe_instance();
> >                }
> > -               reset_kprobe_instance();
> >        }
> >        return 0;
> > }
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kprobes patch
  2006-10-28  1:21   ` Jun Koi
@ 2006-10-29 18:22     ` Vara Prasad
  2006-10-30 18:45     ` Keshavamurthy, Anil S
  1 sibling, 0 replies; 6+ messages in thread
From: Vara Prasad @ 2006-10-29 18:22 UTC (permalink / raw)
  To: Jun Koi; +Cc: Keshavamurthy, Anil S, prasanna, jkenisto, hien, systemtap

Jun Koi wrote:

> Hi Keshavanmurthy,
>
> On 10/28/06, Keshavamurthy@bambi.jf.intel.com
> <Keshavamurthy@bambi.jf.intel.com> wrote:
>
>> On Sat, Oct 28, 2006 at 03:58:18AM +0900, Jun Koi wrote:
>> > Hello,
>> >
>> > I am not sure if this is a right place to send patch for kprobes, but
>> > here it is.  This patch moves the reset command of kprobe instance
>> > into the loop. (against 2.6.18)
>>
>> Hi jun,
>> Yes, you have sent it to the right place. thanks.
>> BTW, is is helpful to know what problem did you encounter due to which
>> you had come up with this patch. Or did you catch this bug by just 
>> code review?
>>
>
> I had no problem at al. You are right, the patch is a result of
> reviewing the code.

Can you think of a test case that exercises this part of the code so we 
can make sure this change doesn't adversely affect existing functionality.

>
> Best,
> Jun



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kprobes patch
  2006-10-28  1:21   ` Jun Koi
  2006-10-29 18:22     ` Vara Prasad
@ 2006-10-30 18:45     ` Keshavamurthy, Anil S
  2006-10-30 18:53       ` Keshavamurthy, Anil S
  1 sibling, 1 reply; 6+ messages in thread
From: Keshavamurthy, Anil S @ 2006-10-30 18:45 UTC (permalink / raw)
  To: Jun Koi; +Cc: Keshavamurthy, Anil S, prasanna, jkenisto, hien, systemtap

On Sat, Oct 28, 2006 at 10:21:05AM +0900, Jun Koi wrote:
> Hi Keshavanmurthy,
> 
> On 10/28/06, Keshavamurthy@bambi.jf.intel.com
> <Keshavamurthy@bambi.jf.intel.com> wrote:
> >On Sat, Oct 28, 2006 at 03:58:18AM +0900, Jun Koi wrote:
> >> Hello,
> >>
> >> I am not sure if this is a right place to send patch for kprobes, but
> >> here it is.  This patch moves the reset command of kprobe instance
> >> into the loop. (against 2.6.18)
> >
> >Hi jun,
> >Yes, you have sent it to the right place. thanks.
> >BTW, is is helpful to know what problem did you encounter due to which
> >you had come up with this patch. Or did you catch this bug by just code 
> >review?
> >
> 
> I had no problem at al. You are right, the patch is a result of
> reviewing the code.

Okay. Looked at your patch more carefully and found that there is
no bug as such. Please see my inline comments.

> 
> Best,
> Jun
> 
> >>
> >> Signed-off-by: Jun Koi <junkoi2004@gmail.com>
> >>
> >>
> >> --- a/kernel/kprobes.c    2006-09-20 12:42:06.000000000 +0900
> >> +++ b/kernel/kprobes.c        2006-10-28 03:52:27.000000000 +0900
> >> @@ -204,8 +204,8 @@ static int __kprobes aggr_pre_handler(st
> >>                        set_kprobe_instance(kp);
> >>                        if (kp->pre_handler(kp, regs))
> >>                                return 1;
> >> +                       reset_kprobe_instance();
> >>                }
> >> -               reset_kprobe_instance();

The reason that reset_kprobe_instance() is outside the loop is a kind of 
optimization.  You can call reset_kprobe_instance() once you are
out of the loop instead of having to call everytime inside the loop
followed  by immediate call to set_kprobe_instance().

> >>        }
> >>        return 0;
> >> }
> >

thanks,
Anil Keshavamurthy

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] kprobes patch
  2006-10-30 18:45     ` Keshavamurthy, Anil S
@ 2006-10-30 18:53       ` Keshavamurthy, Anil S
  0 siblings, 0 replies; 6+ messages in thread
From: Keshavamurthy, Anil S @ 2006-10-30 18:53 UTC (permalink / raw)
  To: Jun Koi; +Cc: anil.s.keshavamurthy, prasanna, jkenisto, hien, systemtap

On Sat, Oct 28, 2006 at 10:21:05AM +0900, Jun Koi wrote:
> Hi Keshavanmurthy,
> 
> On 10/28/06, Keshavamurthy@bambi.jf.intel.com
> <Keshavamurthy@bambi.jf.intel.com> wrote:
> >On Sat, Oct 28, 2006 at 03:58:18AM +0900, Jun Koi wrote:
> >> Hello,
> >>
> >> I am not sure if this is a right place to send patch for kprobes, but
> >> here it is.  This patch moves the reset command of kprobe instance
> >> into the loop. (against 2.6.18)
> >
> >Hi jun,
> >Yes, you have sent it to the right place. thanks.
> >BTW, is is helpful to know what problem did you encounter due to which
> >you had come up with this patch. Or did you catch this bug by just code 
> >review?
> >
> 
> I had no problem at al. You are right, the patch is a result of
> reviewing the code.

Okay. Looked at your patch more carefully and found that there is
no bug as such. Please see my inline comments.

> 
> Best,
> Jun
> 
> >>
> >> Signed-off-by: Jun Koi <junkoi2004@gmail.com>
> >>
> >>
> >> --- a/kernel/kprobes.c    2006-09-20 12:42:06.000000000 +0900
> >> +++ b/kernel/kprobes.c        2006-10-28 03:52:27.000000000 +0900
> >> @@ -204,8 +204,8 @@ static int __kprobes aggr_pre_handler(st
> >>                        set_kprobe_instance(kp);
> >>                        if (kp->pre_handler(kp, regs))
> >>                                return 1;
> >> +                       reset_kprobe_instance();
> >>                }
> >> -               reset_kprobe_instance();

The reason that reset_kprobe_instance() is outside the loop is a kind of 
optimization.  You can call reset_kprobe_instance() once you are
out of the loop instead of having to call everytime inside the loop
followed  by immediate call to set_kprobe_instance().

> >>        }
> >>        return 0;
> >> }
> >

thanks,
Anil Keshavamurthy

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2006-10-30 18:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-10-27 18:58 [PATCH] kprobes patch Jun Koi
2006-10-27 20:45 ` Keshavamurthy, Anil S
2006-10-28  1:21   ` Jun Koi
2006-10-29 18:22     ` Vara Prasad
2006-10-30 18:45     ` Keshavamurthy, Anil S
2006-10-30 18:53       ` Keshavamurthy, Anil S

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).