Re: [PATCH v1 07/22] powerpc/ftrace: Use patch_instruction() return directly

["Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>]

Christophe Leroy wrote:
> Instead of returning -EPERM when patch_instruction() fails,
> just return what patch_instruction returns.
> 
> That simplifies ftrace_modify_code():
> 
> 	   0:	94 21 ff c0 	stwu    r1,-64(r1)
> 	   4:	93 e1 00 3c 	stw     r31,60(r1)
> 	   8:	7c 7f 1b 79 	mr.     r31,r3
> 	   c:	40 80 00 30 	bge     3c <ftrace_modify_code+0x3c>
> 	  10:	93 c1 00 38 	stw     r30,56(r1)
> 	  14:	7c 9e 23 78 	mr      r30,r4
> 	  18:	7c a4 2b 78 	mr      r4,r5
> 	  1c:	80 bf 00 00 	lwz     r5,0(r31)
> 	  20:	7c 1e 28 40 	cmplw   r30,r5
> 	  24:	40 82 00 34 	bne     58 <ftrace_modify_code+0x58>
> 	  28:	83 c1 00 38 	lwz     r30,56(r1)
> 	  2c:	7f e3 fb 78 	mr      r3,r31
> 	  30:	83 e1 00 3c 	lwz     r31,60(r1)
> 	  34:	38 21 00 40 	addi    r1,r1,64
> 	  38:	48 00 00 00 	b       38 <ftrace_modify_code+0x38>
> 				38: R_PPC_REL24	patch_instruction
> 
> Before:
> 
> 	   0:	94 21 ff c0 	stwu    r1,-64(r1)
> 	   4:	93 e1 00 3c 	stw     r31,60(r1)
> 	   8:	7c 7f 1b 79 	mr.     r31,r3
> 	   c:	40 80 00 4c 	bge     58 <ftrace_modify_code+0x58>
> 	  10:	93 c1 00 38 	stw     r30,56(r1)
> 	  14:	7c 9e 23 78 	mr      r30,r4
> 	  18:	7c a4 2b 78 	mr      r4,r5
> 	  1c:	80 bf 00 00 	lwz     r5,0(r31)
> 	  20:	7c 08 02 a6 	mflr    r0
> 	  24:	90 01 00 44 	stw     r0,68(r1)
> 	  28:	7c 1e 28 40 	cmplw   r30,r5
> 	  2c:	40 82 00 48 	bne     74 <ftrace_modify_code+0x74>
> 	  30:	7f e3 fb 78 	mr      r3,r31
> 	  34:	48 00 00 01 	bl      34 <ftrace_modify_code+0x34>
> 				34: R_PPC_REL24	patch_instruction
> 	  38:	80 01 00 44 	lwz     r0,68(r1)
> 	  3c:	20 63 00 00 	subfic  r3,r3,0
> 	  40:	83 c1 00 38 	lwz     r30,56(r1)
> 	  44:	7c 63 19 10 	subfe   r3,r3,r3
> 	  48:	7c 08 03 a6 	mtlr    r0
> 	  4c:	83 e1 00 3c 	lwz     r31,60(r1)
> 	  50:	38 21 00 40 	addi    r1,r1,64
> 	  54:	4e 80 00 20 	blr
> 
> It improves ftrace activation/deactivation duration by about 3%.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
>  arch/powerpc/kernel/trace/ftrace.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/trace/ftrace.c b/arch/powerpc/kernel/trace/ftrace.c
> index 98e82fa4980f..1b05d33f96c6 100644
> --- a/arch/powerpc/kernel/trace/ftrace.c
> +++ b/arch/powerpc/kernel/trace/ftrace.c
> @@ -78,10 +78,7 @@ ftrace_modify_code(unsigned long ip, ppc_inst_t old, ppc_inst_t new)
>  	}
> 
>  	/* replace the text with the new text */
> -	if (patch_instruction((u32 *)ip, new))
> -		return -EPERM;
> -
> -	return 0;
> +	return patch_instruction((u32 *)ip, new);

I think the reason we were returning -EPERM is so that ftrace_bug() can 
throw the right error message. That will change due to this patch, 
though I'm not sure how much it matters. -EFAULT and -EPERM seem to 
print almost the same error message.

- Naveen