Re: [PATCH v2] powerpc: split ftrace bits into a separate file

[Michael Ellerman]

"Naveen N. Rao"  writes:

> entry_*.S now includes a lot more than just kernel entry/exit code. As a
> first step at cleaning this up, let's split out the ftrace bits into
> separate files.
>
> No functional changes.
>
> Suggested-by: Michael Ellerman 
> Signed-off-by: Naveen N. Rao 
> ---
> v2: updated commit description.
>
>  arch/powerpc/kernel/Makefile|   2 +
>  arch/powerpc/kernel/entry_32.S  | 107 ---
>  arch/powerpc/kernel/entry_64.S  | 380 --
>  arch/powerpc/kernel/ftrace_32.S | 118 
>  arch/powerpc/kernel/ftrace_64.S | 391 
> 

Thanks for having a crack at this.

I think I'd actually like to go the whole way, and move all the tracing
code into arch/powerpc/trace (or ftrace?).

There's the 32 and 64-bit asm, and ftrace.c, and trace_clock.c. And then
possibly later some of the ftrace module code could move in there too.

And as part of moving the asm, I think we can come up with a better way
to organise the code. If you look at the ftrace code in entry_64.S for
example the ifdefs look like:

#ifdef CONFIG_FUNCTION_TRACER
#ifdef CONFIG_DYNAMIC_FTRACE
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
#endif
#else /* CC_USING_MPROFILE_KERNEL */
#ifdef CONFIG_LIVEPATCH
#endif
#ifdef CONFIG_LIVEPATCH
#endif
#ifdef CONFIG_LIVEPATCH
#endif
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
#endif
#endif /* CC_USING_MPROFILE_KERNEL */
#ifdef CONFIG_LIVEPATCH
#endif
#else
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
#endif
#endif /* CONFIG_DYNAMIC_FTRACE */
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
#else /* CC_USING_MPROFILE_KERNEL */
#endif /* CC_USING_MPROFILE_KERNEL */
#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
#endif /* CONFIG_FUNCTION_TRACER */


Which is not easy to follow.

I think the main axis is CC_USING_MPROFILE_KERNEL y/n, so perhaps we split
it into two .S files based on that?

cheers

Re: [PATCH v2] powerpc: split ftrace bits into a separate file

[Steven Rostedt]

On Tue, 22 Nov 2016 22:04:39 +0530
"Naveen N. Rao"  wrote:

> entry_*.S now includes a lot more than just kernel entry/exit code. As a
> first step at cleaning this up, let's split out the ftrace bits into
> separate files.
> 
> No functional changes.
> 
> Suggested-by: Michael Ellerman 
> Signed-off-by: Naveen N. Rao 

Looks good to me.

Acked-by: Steven Rostedt 

-- Steve

[PATCH v2] powerpc: split ftrace bits into a separate file

[Naveen N. Rao]

entry_*.S now includes a lot more than just kernel entry/exit code. As a
first step at cleaning this up, let's split out the ftrace bits into
separate files.

No functional changes.

Suggested-by: Michael Ellerman 
Signed-off-by: Naveen N. Rao 
---
v2: updated commit description.

 arch/powerpc/kernel/Makefile|   2 +
 arch/powerpc/kernel/entry_32.S  | 107 ---
 arch/powerpc/kernel/entry_64.S  | 380 --
 arch/powerpc/kernel/ftrace_32.S | 118 
 arch/powerpc/kernel/ftrace_64.S | 391 
 5 files changed, 511 insertions(+), 487 deletions(-)
 create mode 100644 arch/powerpc/kernel/ftrace_32.S
 create mode 100644 arch/powerpc/kernel/ftrace_64.S

diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
index 26b5a5e..298c98d 100644
--- a/arch/powerpc/kernel/Makefile
+++ b/arch/powerpc/kernel/Makefile
@@ -112,6 +112,8 @@ obj64-$(CONFIG_AUDIT)   += compat_audit.o
 
 obj-$(CONFIG_PPC_IO_WORKAROUNDS)   += io-workarounds.o
 
+obj32-$(CONFIG_FUNCTION_TRACER)+= ftrace_32.o
+obj64-$(CONFIG_FUNCTION_TRACER)+= ftrace_64.o
 obj-$(CONFIG_DYNAMIC_FTRACE)   += ftrace.o
 obj-$(CONFIG_FUNCTION_GRAPH_TRACER)+= ftrace.o
 obj-$(CONFIG_FTRACE_SYSCALLS)  += ftrace.o
diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
index 3841d74..5b30eb9 100644
--- a/arch/powerpc/kernel/entry_32.S
+++ b/arch/powerpc/kernel/entry_32.S
@@ -31,7 +31,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
@@ -1304,109 +1303,3 @@ machine_check_in_rtas:
/* XXX load up BATs and panic */
 
 #endif /* CONFIG_PPC_RTAS */
-
-#ifdef CONFIG_FUNCTION_TRACER
-#ifdef CONFIG_DYNAMIC_FTRACE
-_GLOBAL(mcount)
-_GLOBAL(_mcount)
-   /*
-* It is required that _mcount on PPC32 must preserve the
-* link register. But we have r0 to play with. We use r0
-* to push the return address back to the caller of mcount
-* into the ctr register, restore the link register and
-* then jump back using the ctr register.
-*/
-   mflrr0
-   mtctr   r0
-   lwz r0, 4(r1)
-   mtlrr0
-   bctr
-
-_GLOBAL(ftrace_caller)
-   MCOUNT_SAVE_FRAME
-   /* r3 ends up with link register */
-   subir3, r3, MCOUNT_INSN_SIZE
-.globl ftrace_call
-ftrace_call:
-   bl  ftrace_stub
-   nop
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-.globl ftrace_graph_call
-ftrace_graph_call:
-   b   ftrace_graph_stub
-_GLOBAL(ftrace_graph_stub)
-#endif
-   MCOUNT_RESTORE_FRAME
-   /* old link register ends up in ctr reg */
-   bctr
-#else
-_GLOBAL(mcount)
-_GLOBAL(_mcount)
-
-   MCOUNT_SAVE_FRAME
-
-   subir3, r3, MCOUNT_INSN_SIZE
-   LOAD_REG_ADDR(r5, ftrace_trace_function)
-   lwz r5,0(r5)
-
-   mtctr   r5
-   bctrl
-   nop
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-   b   ftrace_graph_caller
-#endif
-   MCOUNT_RESTORE_FRAME
-   bctr
-#endif
-EXPORT_SYMBOL(_mcount)
-
-_GLOBAL(ftrace_stub)
-   blr
-
-#ifdef CONFIG_FUNCTION_GRAPH_TRACER
-_GLOBAL(ftrace_graph_caller)
-   /* load r4 with local address */
-   lwz r4, 44(r1)
-   subir4, r4, MCOUNT_INSN_SIZE
-
-   /* Grab the LR out of the caller stack frame */
-   lwz r3,52(r1)
-
-   bl  prepare_ftrace_return
-   nop
-
-/*
- * prepare_ftrace_return gives us the address we divert to.
- * Change the LR in the callers stack frame to this.
- */
-   stw r3,52(r1)
-
-   MCOUNT_RESTORE_FRAME
-   /* old link register ends up in ctr reg */
-   bctr
-
-_GLOBAL(return_to_handler)
-   /* need to save return values */
-   stwur1, -32(r1)
-   stw r3, 20(r1)
-   stw r4, 16(r1)
-   stw r31, 12(r1)
-   mr  r31, r1
-
-   bl  ftrace_return_to_handler
-   nop
-
-   /* return value has real return address */
-   mtlrr3
-
-   lwz r3, 20(r1)
-   lwz r4, 16(r1)
-   lwz r31,12(r1)
-   lwz r1, 0(r1)
-
-   /* Jump back to real return address */
-   blr
-#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
-
-#endif /* CONFIG_FUNCTION_TRACER */
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 6432d4b..9b541d2 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -20,7 +20,6 @@
 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -33,7 +32,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -1173,381 +1171,3 @@ _GLOBAL(enter_prom)
ld  r0,16(r1)
mtlrr0
 blr
-
-#ifdef CONFIG_FUNCTION_TRACER
-#ifdef CONFIG_DYNAMIC_FTRACE
-_GLOBAL(mcount)
-_GLOBAL(_mcount)
-EXPORT_SYMBOL(_mcount)
-   mflrr12
-   mtctr   r12
-   mtlrr0
-   bctr
-