Ananth, Srikar, I think the patch is correct and I am sorry for nit-picking, this is really minor.
But, On 06/08, Ananth N Mavinakayanahalli wrote: > > Changes in V2: > Pass (unsigned long)addr Well, IMHO, this is confusing. First of all, why do we have this "addr" or even "vaddr"? It should not exists. We pass it to copy_insn(), but for what?? copy_insn() should simply use uprobe->offset, the virtual address for this particular mapping does not matter at all. I am going to send the cleanup. Note also that we should move this !UPROBE_COPY_INSN from install_breakpoint() to somewhere near alloc_uprobe(). This code is called only once, it looks a bit strange to use the "random" mm (the first mm vma_prio_tree_foreach() finds) and its mapping to verify the insn. In fact this is simply not correct and should be fixed, note that on x86 arch_uprobe_analyze_insn() checks mm->context.ia32_compat. IOW, Perhaps uprobe->offset makes more sense? > --- linux-3.5-rc1.orig/kernel/events/uprobes.c > +++ linux-3.5-rc1/kernel/events/uprobes.c > @@ -697,7 +697,7 @@ install_breakpoint(struct uprobe *uprobe > if (is_swbp_insn((uprobe_opcode_t *)uprobe->arch.insn)) > return -EEXIST; > > - ret = arch_uprobe_analyze_insn(&uprobe->arch, mm); > + ret = arch_uprobe_analyze_insn(&uprobe->arch, mm, addr); Just fyi, this conflicts with "[PATCH 1/3] uprobes: install_breakpoint() should fail if is_swbp_insn() == T" I sent, but the conflict is trivial. Oleg. _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
