On 1/4/08, Masami Hiramatsu <[EMAIL PROTECTED]> wrote:
> > + case KPROBE_HIT_SS:
> > + if (*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
> > + regs->flags &= ~TF_MASK;
> > + regs->flags |= kcb->kprobe_saved_flags;
> > + retur
On 1/4/08, Masami Hiramatsu <[EMAIL PROTECTED]> wrote:
> I could understand what the original code did at last.
> If a kprobe is inserted on a breakpoint which other debugger inserts,
> it single step inline instead of out-of-line.(this is done in
> prepare_singlestep)
> In this case, (p && kprobe
Hi Abhishek,
Masami Hiramatsu wrote:
>> Hmm, I can not agree, because it is possible to insert a kprobe
>> into kprobe's instruction buffer. If it should be a bug, we must
>> check it when registering the kprobe.
>
> I discussed it with other maintainers and knew that current kprobes
> does not a
Abhishek Sagar wrote:
> Masami Hiramatsu wrote:
...
> Done. You should find the desired changed in this patch.
Well done!
This cleans it up very well.
I have just one more comment.
> @@ -463,14 +487,26 @@ static int __kprobes reenter_kprobe(struct kprobe *p,
> struct pt_regs *regs,
>
Masami Hiramatsu wrote:
>>> As a result, this function just setups re-entrance.
>> As you've also pointed out in your previous reply, this case is
>> peculiar and therefore I believe it should be marked as a BUG(). I've
>> left the original case, if (kcb->kprobe_status==KPROBE_HIT_SS) &&
>> (*p->ai
Hi Abhishek,
Masami Hiramatsu wrote:
...
+ case KPROBE_HIT_SS:
+ if (*p->ainsn.insn == BREAKPOINT_INSTRUCTION) {
+ regs->flags &= ~TF_MASK;
+ regs->flags |= kcb->kprobe_saved_flags;
+ } else {
+
Hi Abhishek,
Abhishek Sagar wrote:
> On 1/2/08, Masami Hiramatsu <[EMAIL PROTECTED]> wrote:
>> I think setup_singlestep() in your first patch is better, because it avoided
>> code duplication(*).
>
> Will retain it then.
Thank you very much.
>>> static int __kprobes reenter_kprobe(struct kprob
* Abhishek Sagar <[EMAIL PROTECTED]> wrote:
> > > + default:
> > > + /* impossible cases */
> > > + BUG();
> >
> > I think no need to check that here.
>
> It may not get hit at runtime but is quite informative.
sidenote: please use WARN_ON(1) instead. In the case of
On 1/2/08, Masami Hiramatsu <[EMAIL PROTECTED]> wrote:
> I think setup_singlestep() in your first patch is better, because it avoided
> code duplication(*).
Will retain it then.
> > static int __kprobes reenter_kprobe(struct kprobe *p, struct pt_regs *regs,
> >
Hi Abhishek,
Thank you for good work.
Abhishek Sagar wrote:
> @@ -441,6 +441,26 @@ void __kprobes arch_prepare_kretprobe(struct
> kretprobe_instance *ri,
> /* Replace the return addr with trampoline addr */
> *sara = (unsigned long) &kretprobe_trampoline;
> }
> +
> +#if !defined(CON
Hi,
Abhishek Sagar wrote:
>>> static int __kprobes kprobe_handler(struct pt_regs *regs)
>>> {
>>> - struct kprobe *p;
>>> int ret = 0;
>>> kprobe_opcode_t *addr;
>>> + struct kprobe *p, *cur;
>>> struct kprobe_ctlblk *kcb;
>>>
>>> addr = (kprobe_opcode_t *)(regs->
On 1/2/08, Harvey Harrison <[EMAIL PROTECTED]> wrote:
> > +#if !defined(CONFIG_PREEMPT) || defined(CONFIG_PM)
> > +static __always_inline int setup_boost(struct kprobe *p, struct pt_regs
> > *regs)
> > +{
> > + if (p->ainsn.boostable == 1 && !p->post_handler) {
> > + /* Boost up --
On 1/1/08, Masami Hiramatsu <[EMAIL PROTECTED]> wrote:
> Could you separate changing logic from cleanup and explain
> why the logic should be changed?
The major portion of logical changes is re-routing of code flow by
removing gotos. Hopefully all the cases have been covered. There are a
couple o
Just a few nitpicks. I need to look closer at the reenter_kprobe
changes, but it looks like this should lead to clearer flow than
before. The whole !p/kprobe_running() differences were pretty
twisty before.
On Wed, 2008-01-02 at 01:10 +0530, Abhishek Sagar wrote:
> Thanks for pointing me to the
Ingo Molnar wrote:
> hm, this patch does not apply to x86.git#mm, due to the fixes,
> unifications and cleanups done there. Could you send a patch against -mm
> or against x86.git? (see the tree-fetching instructions below) Thanks,
>
> Ingo
>
> --{ x86.git instructions }--
Hello Abhishek,
Abhishek Sagar wrote:
> Harvey Harrison wrote:
>> Make the control flow of kprobe_handler more obvious.
>>
>> Collapse the separate if blocks/gotos with if/else blocks
>> this unifies the duplication of the check for a breakpoint
>> instruction race with another cpu.
>
> This is a
* Abhishek Sagar <[EMAIL PROTECTED]> wrote:
> Harvey Harrison wrote:
> > Make the control flow of kprobe_handler more obvious.
> >
> > Collapse the separate if blocks/gotos with if/else blocks
> > this unifies the duplication of the check for a breakpoint
> > instruction race with another cpu.
>
Harvey Harrison wrote:
> Make the control flow of kprobe_handler more obvious.
>
> Collapse the separate if blocks/gotos with if/else blocks
> this unifies the duplication of the check for a breakpoint
> instruction race with another cpu.
This is a patch derived from kprobe_handler of the ARM kpr
* Masami Hiramatsu <[EMAIL PROTECTED]> wrote:
> And also, I prefer "return 1" to "{ret = 1; goto out;}" for
> simplicity.
it seems that the code already uses 'ret', and sets it to 1 in certain
cases - in that light it's a tiny bit cleaner to just have a single
return code flow.
Ingo
Hello Harvey,
Thank you for your great works!
Harvey Harrison wrote:
> Make the control flow of kprobe_handler more obvious.
>
> Collapse the separate if blocks/gotos with if/else blocks
> this unifies the duplication of the check for a breakpoint
> instruction race with another cpu.
I agree it
Make the control flow of kprobe_handler more obvious.
Collapse the separate if blocks/gotos with if/else blocks
this unifies the duplication of the check for a breakpoint
instruction race with another cpu.
Create two jump targets:
preempt_out: re-enables preemption before returning ret
Make the control flow of kprobe_handler more obvious.
Collapse the separate if blocks/gotos with if/else blocks
this unifies the duplication of the check for a breakpoint
instruction race with another cpu.
Signed-off-by: Harvey Harrison <[EMAIL PROTECTED]>
---
Masami, please have a look at this,
22 matches
Mail list logo
