| Message ID | 20130320124301.GA30887@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wed, Mar 20, 2013 at 01:43:01PM +0100, Oleg Nesterov wrote: > On 03/20, Oleg Nesterov wrote: > > > > But we did not install UPROBE_SWBP_INSN. Is it fine? I hope yes, just to > > verify. If not, we need 2 definitions. is_uprobe_insn() should still check > > insns == UPROBE_SWBP_INSN, and is_swbp_insn() should check is_trap(). > > > > And I am just curious, could you explain how X and UPROBE_SWBP_INSN > > differ? > > IOW, if I wasn't clear... Lets forget about gdb/etc for the moment. > Suppose we apply the patch below. Will uprobes on powerpc work? > > If yes, then your patch should be fine. If not, we probably need more > changes. Yes, it will work fine. > And, forgot to mention. If you change is_swbp_insn(), you can remove > is_trap() from arch_uprobe_analyze_insn(). Yeah, that check is no longer needed. I'll send a separate cleanup after this patch gets applied. Ananth
On 03/20, Ananth N Mavinakayanahalli wrote: > > On Wed, Mar 20, 2013 at 01:43:01PM +0100, Oleg Nesterov wrote: > > On 03/20, Oleg Nesterov wrote: > > > > > > But we did not install UPROBE_SWBP_INSN. Is it fine? I hope yes, just to > > > verify. If not, we need 2 definitions. is_uprobe_insn() should still check > > > insns == UPROBE_SWBP_INSN, and is_swbp_insn() should check is_trap(). > > > > > > And I am just curious, could you explain how X and UPROBE_SWBP_INSN > > > differ? > > > > IOW, if I wasn't clear... Lets forget about gdb/etc for the moment. > > Suppose we apply the patch below. Will uprobes on powerpc work? > > > > If yes, then your patch should be fine. If not, we probably need more > > changes. > > Yes, it will work fine. Even if this new insn is conditional? Oleg.
On Wed, Mar 20, 2013 at 05:07:28PM +0100, Oleg Nesterov wrote: > On 03/20, Ananth N Mavinakayanahalli wrote: > > > > On Wed, Mar 20, 2013 at 01:43:01PM +0100, Oleg Nesterov wrote: > > > On 03/20, Oleg Nesterov wrote: > > > > > > > > But we did not install UPROBE_SWBP_INSN. Is it fine? I hope yes, just to > > > > verify. If not, we need 2 definitions. is_uprobe_insn() should still check > > > > insns == UPROBE_SWBP_INSN, and is_swbp_insn() should check is_trap(). > > > > > > > > And I am just curious, could you explain how X and UPROBE_SWBP_INSN > > > > differ? > > > > > > IOW, if I wasn't clear... Lets forget about gdb/etc for the moment. > > > Suppose we apply the patch below. Will uprobes on powerpc work? > > > > > > If yes, then your patch should be fine. If not, we probably need more > > > changes. > > > > Yes, it will work fine. > > Even if this new insn is conditional? Yes.
On 03/21, Ananth N Mavinakayanahalli wrote: ? > On Wed, Mar 20, 2013 at 05:07:28PM +0100, Oleg Nesterov wrote: > > On 03/20, Ananth N Mavinakayanahalli wrote: > > > > > > On Wed, Mar 20, 2013 at 01:43:01PM +0100, Oleg Nesterov wrote: > > > > On 03/20, Oleg Nesterov wrote: > > > > > > > > > IOW, if I wasn't clear... Lets forget about gdb/etc for the moment. > > > > Suppose we apply the patch below. Will uprobes on powerpc work? > > > > > > > > If yes, then your patch should be fine. If not, we probably need more > > > > changes. > > > > > > Yes, it will work fine. > > > > Even if this new insn is conditional? > > Yes. But this can't be true. If it is conditional, it won't always generate a trap, this means uprobe won't actually work. Oleg.
On Thu, Mar 21, 2013 at 05:00:02PM +0100, Oleg Nesterov wrote: > On 03/21, Ananth N Mavinakayanahalli wrote: > ? > > On Wed, Mar 20, 2013 at 05:07:28PM +0100, Oleg Nesterov wrote: > > > On 03/20, Ananth N Mavinakayanahalli wrote: > > > > > > > > On Wed, Mar 20, 2013 at 01:43:01PM +0100, Oleg Nesterov wrote: > > > > > On 03/20, Oleg Nesterov wrote: > > > > > > > > > > > IOW, if I wasn't clear... Lets forget about gdb/etc for the moment. > > > > > Suppose we apply the patch below. Will uprobes on powerpc work? > > > > > > > > > > If yes, then your patch should be fine. If not, we probably need more > > > > > changes. > > > > > > > > Yes, it will work fine. > > > > > > Even if this new insn is conditional? > > > > Yes. > > But this can't be true. If it is conditional, it won't always generate a > trap, this means uprobe won't actually work. I meant to say, uprobes if we use a conditional trap instruction as long as the condition is met. Ananth
--- x/arch/powerpc/include/asm/uprobes.h +++ x/arch/powerpc/include/asm/uprobes.h @@ -31,7 +31,7 @@ typedef ppc_opcode_t uprobe_opcode_t; #define UPROBE_XOL_SLOT_BYTES (MAX_UINSN_BYTES) /* The following alias is needed for reference from arch-agnostic code */ -#define UPROBE_SWBP_INSN BREAKPOINT_INSTRUCTION +#define UPROBE_SWBP_INSN TRAP_INSN_USED_BY_GDB #define UPROBE_SWBP_INSN_SIZE 4 /* swbp insn size in bytes */ struct arch_uprobe {