Re: [PATCH v4 2/2] powerpc: Uprobes port to powerpc

[Oleg Nesterov <oleg@redhat.com>]

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 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.

But probably I misunderstood you.

Oleg.