* Oleg Nesterov [off-list ref] [2012-08-23 11:02:09]:
On 08/23, Benjamin Herrenschmidt wrote:
quoted
On Thu, 2012-08-23 at 11:02 +0530, Srikar Dronamraju wrote:
quoted
quoted
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?