Re: [PATCH 1/2] MIPS: ftrace.h: Fix the MCOUNT_INSN_SIZE definition

From: David Daney
Date: Mon Sep 22 2014 - 12:55:18 EST

On 09/22/2014 06:32 AM, Markos Chandras wrote:
The MCOUNT_INSN_SIZE is meant to be used to denote the overall
size of the mcount() call. Since a jal instruction is used to
call mcount() the delay slot should be taken into consideration
as well.
This also replaces the MCOUNT_INSN_SIZE usage with the real size
of a single MIPS instruction since, as described above, the
MCOUNT_INSN_SIZE is used to denote the total overhead of the
mcount() call.

Are you seeing errors with the existing code? If so please state what they are.

By changing this, we can no longer atomically replace the instruction. So I think shouldn't be changing this stuff unless there is a real bug we are fixing.

In conclusion: NAK unless the patch fixes a bug, in which case the change log *must* state what the bug is, and how the patch addresses the problem.

David Daney

Cc: Steven Rostedt <rostedt@xxxxxxxxxxx>
Cc: Ingo Molnar <mingo@xxxxxxxxxx>
Cc: linux-kernel@xxxxxxxxxxxxxxx
Signed-off-by: Markos Chandras <markos.chandras@xxxxxxxxxx>
arch/mips/include/asm/ftrace.h | 2 +-
arch/mips/kernel/ftrace.c | 4 +++-
2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/ftrace.h b/arch/mips/include/asm/ftrace.h
index 992aaba603b5..70d4a35fb560 100644
--- a/arch/mips/include/asm/ftrace.h
+++ b/arch/mips/include/asm/ftrace.h
@@ -13,7 +13,7 @@

#define MCOUNT_ADDR ((unsigned long)(_mcount))
-#define MCOUNT_INSN_SIZE 4 /* sizeof mcount call */
+#define MCOUNT_INSN_SIZE 8 /* sizeof mcount call + delay slot */

#ifndef __ASSEMBLY__
extern void _mcount(void);
diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index 937c54bc8ccc..211460d4617d 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -28,6 +28,8 @@

+#define FTRACE_MIPS_INSN_SIZE 4 /* Size of single MIPS instruction */

/* Arch override because MIPS doesn't need to run this from stop_machine() */
@@ -395,7 +397,7 @@ void prepare_ftrace_return(unsigned long *parent_ra_addr, unsigned long self_ra,

insns = in_kernel_space(self_ra) ? 2 : MCOUNT_OFFSET_INSNS + 1;
- trace.func = self_ra - (MCOUNT_INSN_SIZE * insns);
+ trace.func = self_ra - (FTRACE_MIPS_INSN_SIZE * insns);

/* Only trace if the calling function expects to */
if (!ftrace_graph_entry(&trace)) {

To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at
Please read the FAQ at