On 08/16, Ananth N Mavinakayanahalli wrote:
On Thu, Aug 16, 2012 at 07:41:53AM +1000, Benjamin Herrenschmidt wrote:
quoted
On Wed, 2012-08-15 at 18:59 +0200, Oleg Nesterov wrote:
quoted
On 07/26, Ananth N Mavinakayanahalli wrote:
quoted
From: Ananth N Mavinakayanahalli <redacted>
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!
quoted
quoted
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.