lkml.org 
[lkml]   [2012]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v4 2/2] powerpc: Uprobes port to powerpc
    * Oleg Nesterov <oleg@redhat.com> [2012-08-23 11:02:09]:

    > On 08/23, Benjamin Herrenschmidt wrote:
    > >
    > > On Thu, 2012-08-23 at 11:02 +0530, Srikar Dronamraju wrote:
    > > > >
    > > >
    > > > insn is updated/accessed in the arch independent code. Size of
    > > > uprobe_opcode_t could be different for different archs.
    > > > uprobe_opcode_t
    > > > represents the size of the smallest breakpoint instruction for an
    > > > arch.
    > > >
    > > > Hence u8 works out the best. I know we could still use uprobe_opcode_t
    > > > and achieve the same. In which case, we would have to interpret
    > > > MAX_UINSN_BYTES differently. Do you see any advantages of using
    > > > uprobe_opcode_t instead of u8 across archs?
    > >
    > > But don't you actively rely on the fact that on powerpc, unlike x86, you
    > > -can- atomically replace an instruction with a single 32-bit store ?
    >
    > I must have missed something...
    >
    > But powerpc does not replace an instruction, the arch independent code
    > does this and it assumes that uprobe->arch.insn is u8[MAX_UINSN_BYTES].
    >
    > Perhaps you meant that on powerpc it is "safe" to replace the insn
    > even if this can race with some CPU executing this code? But uprobes
    > has to replace the original page anyway, we should not write to
    > ->vm_file.

    I think Ben is referring to the fact that because we use an array we
    endup using memcpy to copy the original instruction from the ->vm_file.

    >
    > I agree that memcpy() in arch_uprobe_analyze_insn() and
    > arch_uprobe_skip_sstep() looks a bit strange. May be powerpc can do
    >
    > struct arch_uprobe {
    > union {
    > u8 insn[MAX_UINSN_BYTES];
    > u32 ainsn;
    > };
    > };
    >
    > and use auprobe->ainsn directly, I dunno.

    I think this should work.

    Ben would this suffice?



    \
     
     \ /
      Last update: 2012-08-23 18:41    [W:0.025 / U:63.052 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site