On 08/16, Ananth N Mavinakayanahalli wrote: > > On Thu, Aug 16, 2012 at 07:41:53AM +1000, Benjamin Herrenschmidt wrote: > > On Wed, 2012-08-15 at 18:59 +0200, Oleg Nesterov wrote: > > > On 07/26, Ananth N Mavinakayanahalli wrote: > > > > > > > > From: Ananth N Mavinakayanahalli <ana...@in.ibm.com> > > > > > > > > This is the port of uprobes to powerpc. Usage is similar to x86. > > > > > > I am just curious why this series was ignored by powerpc maintainers... > > > > Because it arrived too late for the previous merge window considering my > > limited bandwidth for reviewing things and that nobody else seems to > > have reviewed it :-) > > > > It's still on track for the next one, and I'm hoping to dedicate most of > > next week going through patches & doing a powerpc -next. > > Thanks Ben!
Great! > > > Just one question... Shouldn't arch_uprobe_pre_xol() forbid to probe > > > UPROBE_SWBP_INSN (at least) ? > > > > > > (I assume that emulate_step() can't handle this case but of course I > > > do not understand arch/powerpc/lib/sstep.c) > > > > > > Note that uprobe_pre_sstep_notifier() sets utask->state = UTASK_BP_HIT > > > without any checks. This doesn't look right if it was UTASK_SSTEP... > > > > > > But again, I do not know what powepc will actually do if we try to > > > single-step over UPROBE_SWBP_INSN. > > > > Ananth ? > > set_swbp() will return -EEXIST to install_breakpoint if we are trying to > put a breakpoint on UPROBE_SWBP_INSN. not really, this -EEXIST (already removed by recent changes) means that bp was already installed. But this doesn't matter, > So, the arch agnostic code itself > takes care of this case... Yes. I forgot about install_breakpoint()->is_swbp_insn() check which returns -ENOTSUPP, somehow I thought arch_uprobe_analyze_insn() does this. > or am I missing something? No, it is me. > However, I see that we need a powerpc specific is_swbp_insn() > implementation since we will have to take care of all the trap variants. Hmm, I am not sure. is_swbp_insn(insn), as it is used in the arch agnostic code, should only return true if insn == UPROBE_SWBP_INSN (just in case, this logic needs more fixes but this is offtopic). If powerpc has another insn(s) which can trigger powerpc's do_int3() counterpart, they should be rejected by arch_uprobe_analyze_insn(). I think. > I will need to update the patches based on changes being made by Oleg > and Sebastien for the single-step issues. Perhaps you can do this in a separate change? We need some (simple) changes in the arch agnostic code first, they should not break poweppc. These changes are still under discussion. Once we have "__weak arch_uprobe_step*" you can reimplement these hooks and fix the problems with single-stepping. Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
