Under the help of the -mlong-calls option of gcc, This revision comes with the
module tracing support. and there are also lots of cleanups and fixes.
therefore, it is ready to upstream.
Currently, function graph tracer support have some overhead for searching the
stack address of the return address(ra). we hope the PROFILE_BEFORE_PROLOGUE
macro will be enabled by default in the next version of gcc, and then the
overhead can be removed via hijacking the ra register directly for both
non-leaf and leaf function.
Thanks very much to the people in the CC list for their feedbacks, all the
lastest changes goes to this git repo:
git://dev.lemote.com/rt4ls.git linux-mips/dev/ftrace-upstream
Welcome to play with it and give more feedbacks.
Thanks & Regards,
Wu Zhangjin
Wu Zhangjin (11):
tracing: convert trace_clock_local() as weak function
MIPS: add mips_timecounter_read() to get high precision timestamp
tracing: add MIPS specific trace_clock_local()
tracing: add static function tracer support for MIPS
tracing: enable HAVE_FUNCTION_TRACE_MCOUNT_TEST for MIPS
tracing: add an endian argument to scripts/recordmcount.pl
tracing: add dynamic function tracer support for MIPS
tracing: not trace mips_timecounter_init() in MIPS
tracing: add IRQENTRY_EXIT for MIPS
tracing: add function graph tracer support for MIPS
tracing: add dynamic function graph tracer for MIPS
arch/mips/Kconfig | 5 +
arch/mips/Makefile | 4 +
arch/mips/include/asm/ftrace.h | 37 ++++++++-
arch/mips/include/asm/time.h | 19 ++++
arch/mips/kernel/Makefile | 9 ++
arch/mips/kernel/csrc-r4k.c | 41 +++++++++
arch/mips/kernel/ftrace.c | 190 ++++++++++++++++++++++++++++++++++++++++
arch/mips/kernel/mcount.S | 177 +++++++++++++++++++++++++++++++++++++
arch/mips/kernel/mips_ksyms.c | 5 +
arch/mips/kernel/time.c | 2 +
arch/mips/kernel/trace_clock.c | 33 +++++++
arch/mips/kernel/vmlinux.lds.S | 1 +
include/linux/clocksource.h | 2 +-
kernel/time/clocksource.c | 4 +-
kernel/trace/trace_clock.c | 2 +-
scripts/Makefile.build | 1 +
scripts/recordmcount.pl | 44 +++++++++-
17 files changed, 568 insertions(+), 8 deletions(-)
create mode 100644 arch/mips/kernel/ftrace.c
create mode 100644 arch/mips/kernel/mcount.S
create mode 100644 arch/mips/kernel/trace_clock.c
trace_clock_local() is based on the arch-specific sched_clock(), in X86,
it is tsc(64bit) based, which can give very high precision(about 1ns
with 1GHz). but in MIPS, the sched_clock() is jiffies based, which can
give only 10ms precison with 1000 HZ. which is not enough for tracing,
especially for Real Time system.
so, we need to implement a MIPS specific sched_clock() to get higher
precision. There is a tsc like clock counter register in MIPS, whose
frequency is half of the processor, so, if the cpu frequency is 800MHz,
the time precision reaches 2.5ns, which is very good for tracing, even
for Real Time system.
but 'Cause it is only 32bit long, which will rollover quickly, so, such
a sched_clock() will bring with extra load, which is not good for the
whole system. so, we only need to implement a arch-specific
trace_clock_local() for tracing. as a preparation, we convert it as a
weak function.
The MIPS specific trace_clock_local() is coming in the next two patches.
Signed-off-by: Wu Zhangjin <[email protected]>
---
kernel/trace/trace_clock.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/trace/trace_clock.c b/kernel/trace/trace_clock.c
index 20c5f92..06b8cd2 100644
--- a/kernel/trace/trace_clock.c
+++ b/kernel/trace/trace_clock.c
@@ -26,7 +26,7 @@
* Useful for tracing that does not cross to other CPUs nor
* does it go through idle events.
*/
-u64 notrace trace_clock_local(void)
+u64 __weak notrace trace_clock_local(void)
{
unsigned long flags;
u64 clock;
--
1.6.2.1
This patch implement a mips_timecounter_read(), which can be used to get
high precision timestamp without extra lock.
It is based on the clock counter register and the
timecounter/cyclecounter abstraction layer of kernel.
Signed-off-by: Wu Zhangjin <[email protected]>
---
arch/mips/include/asm/time.h | 19 +++++++++++++++++++
arch/mips/kernel/csrc-r4k.c | 41 +++++++++++++++++++++++++++++++++++++++++
arch/mips/kernel/time.c | 2 ++
3 files changed, 62 insertions(+), 0 deletions(-)
diff --git a/arch/mips/include/asm/time.h b/arch/mips/include/asm/time.h
index df6a430..b8af7fa 100644
--- a/arch/mips/include/asm/time.h
+++ b/arch/mips/include/asm/time.h
@@ -73,8 +73,18 @@ static inline int mips_clockevent_init(void)
*/
#ifdef CONFIG_CSRC_R4K_LIB
extern int init_r4k_clocksource(void);
+extern int init_r4k_timecounter(void);
+extern u64 r4k_timecounter_read(void);
#endif
+static inline u64 mips_timecounter_read(void)
+{
+#ifdef CONFIG_CSRC_R4K
+ return r4k_timecounter_read();
+#else
+ return sched_clock();
+#endif
+}
static inline int init_mips_clocksource(void)
{
#ifdef CONFIG_CSRC_R4K
@@ -84,6 +94,15 @@ static inline int init_mips_clocksource(void)
#endif
}
+static inline int init_mips_timecounter(void)
+{
+#ifdef CONFIG_CSRC_R4K
+ return init_r4k_timecounter();
+#else
+ return 0;
+#endif
+}
+
extern void clocksource_set_clock(struct clocksource *cs, unsigned int clock);
extern void clockevent_set_clock(struct clock_event_device *cd,
unsigned int clock);
diff --git a/arch/mips/kernel/csrc-r4k.c b/arch/mips/kernel/csrc-r4k.c
index e95a3cd..4e7705f 100644
--- a/arch/mips/kernel/csrc-r4k.c
+++ b/arch/mips/kernel/csrc-r4k.c
@@ -7,6 +7,7 @@
*/
#include <linux/clocksource.h>
#include <linux/init.h>
+#include <linux/sched.h>
#include <asm/time.h>
@@ -36,3 +37,43 @@ int __init init_r4k_clocksource(void)
return 0;
}
+
+static struct timecounter r4k_tc = {
+ .cc = NULL,
+};
+
+static cycle_t r4k_cc_read(const struct cyclecounter *cc)
+{
+ return read_c0_count();
+}
+
+static struct cyclecounter r4k_cc = {
+ .read = r4k_cc_read,
+ .mask = CLOCKSOURCE_MASK(32),
+ .shift = 32,
+};
+
+int __init init_r4k_timecounter(void)
+{
+ if (!cpu_has_counter || !mips_hpt_frequency)
+ return -ENXIO;
+
+ r4k_cc.mult = div_sc((unsigned long)mips_hpt_frequency, NSEC_PER_SEC,
+ 32);
+
+ timecounter_init(&r4k_tc, &r4k_cc, sched_clock());
+
+ return 0;
+}
+
+u64 r4k_timecounter_read(void)
+{
+ u64 clock;
+
+ if (r4k_tc.cc != NULL)
+ clock = timecounter_read(&r4k_tc);
+ else
+ clock = sched_clock();
+
+ return clock;
+}
diff --git a/arch/mips/kernel/time.c b/arch/mips/kernel/time.c
index 1f467d5..e38cca1 100644
--- a/arch/mips/kernel/time.c
+++ b/arch/mips/kernel/time.c
@@ -156,4 +156,6 @@ void __init time_init(void)
if (!mips_clockevent_init() || !cpu_has_mfc0_count_bug())
init_mips_clocksource();
+ if (!cpu_has_mfc0_count_bug())
+ init_mips_timecounter();
}
--
1.6.2.1
This patch add the mips_timecounter_read() based high precision version
of trace_clock_local().
Signed-off-by: Wu Zhangjin <[email protected]>
---
arch/mips/kernel/Makefile | 6 ++++++
arch/mips/kernel/trace_clock.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 39 insertions(+), 0 deletions(-)
create mode 100644 arch/mips/kernel/trace_clock.c
diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
index 8e26e9c..f228868 100644
--- a/arch/mips/kernel/Makefile
+++ b/arch/mips/kernel/Makefile
@@ -8,6 +8,10 @@ obj-y += cpu-probe.o branch.o entry.o genex.o irq.o process.o \
ptrace.o reset.o setup.o signal.o syscall.o \
time.o topology.o traps.o unaligned.o watch.o
+ifdef CONFIG_FUNCTION_TRACER
+CFLAGS_REMOVE_trace_clock.o = -pg
+endif
+
obj-$(CONFIG_CEVT_BCM1480) += cevt-bcm1480.o
obj-$(CONFIG_CEVT_R4K_LIB) += cevt-r4k.o
obj-$(CONFIG_MIPS_MT_SMTC) += cevt-smtc.o
@@ -24,6 +28,8 @@ obj-$(CONFIG_SYNC_R4K) += sync-r4k.o
obj-$(CONFIG_STACKTRACE) += stacktrace.o
obj-$(CONFIG_MODULES) += mips_ksyms.o module.o
+obj-$(CONFIG_FTRACE) += trace_clock.o
+
obj-$(CONFIG_CPU_LOONGSON2) += r4k_fpu.o r4k_switch.o
obj-$(CONFIG_CPU_MIPS32) += r4k_fpu.o r4k_switch.o
obj-$(CONFIG_CPU_MIPS64) += r4k_fpu.o r4k_switch.o
diff --git a/arch/mips/kernel/trace_clock.c b/arch/mips/kernel/trace_clock.c
new file mode 100644
index 0000000..0fe85ea
--- /dev/null
+++ b/arch/mips/kernel/trace_clock.c
@@ -0,0 +1,33 @@
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive for
+ * more details.
+ *
+ * Copyright (C) 2009 Lemote Inc. & DSLab, Lanzhou University, China
+ * Author: Wu Zhangjin <[email protected]>
+ */
+
+#include <linux/clocksource.h>
+#include <linux/sched.h>
+
+#include <asm/time.h>
+
+/*
+ * trace_clock_local(): the simplest and least coherent tracing clock.
+ *
+ * Useful for tracing that does not cross to other CPUs nor
+ * does it go through idle events.
+ */
+u64 trace_clock_local(void)
+{
+ unsigned long flags;
+ u64 clock;
+
+ raw_local_irq_save(flags);
+
+ clock = mips_timecounter_read();
+
+ raw_local_irq_restore(flags);
+
+ return clock;
+}
--
1.6.2.1
If -pg of gcc is enabled with CONFIG_FUNCTION_TRACER=y. a calling to
_mcount will be inserted into each kernel function. so, there is a
possibility to trace the kernel functions in _mcount.
This patch add the MIPS specific _mcount support for static function
tracing. by deafult, ftrace_trace_function is initialized as
ftrace_stub(an empty function), so, the default _mcount will introduce
very little overhead. after enabling ftrace in user-space, it will jump
to a real tracing function and do static function tracing for us.
And to support module tracing, we need to enable -mlong-calls for the
long call from modules space to kernel space. -mlong-calls load the
address of _mcount to a register and then jump to it, so, the address is
allowed to be 32bit long, but without -mlong-calls, for the instruction
"jal _mcount" only left 26bit for the address of _mcount, which is not
enough for jumping from the module space to kernel space.
Reviewed-by: Steven Rostedt <[email protected]>
Signed-off-by: Wu Zhangjin <[email protected]>
---
arch/mips/Kconfig | 1 +
arch/mips/Makefile | 4 ++
arch/mips/include/asm/ftrace.h | 25 +++++++++++-
arch/mips/kernel/Makefile | 2 +
arch/mips/kernel/mcount.S | 89 ++++++++++++++++++++++++++++++++++++++++
arch/mips/kernel/mips_ksyms.c | 5 ++
6 files changed, 125 insertions(+), 1 deletions(-)
create mode 100644 arch/mips/kernel/mcount.S
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 03bd56a..6b33e88 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -4,6 +4,7 @@ config MIPS
select HAVE_IDE
select HAVE_OPROFILE
select HAVE_ARCH_KGDB
+ select HAVE_FUNCTION_TRACER
# Horrible source of confusion. Die, die, die ...
select EMBEDDED
select RTC_LIB if !LEMOTE_FULOONG2E
diff --git a/arch/mips/Makefile b/arch/mips/Makefile
index 77f5021..041deca 100644
--- a/arch/mips/Makefile
+++ b/arch/mips/Makefile
@@ -48,7 +48,11 @@ ifneq ($(SUBARCH),$(ARCH))
endif
endif
+ifndef CONFIG_FUNCTION_TRACER
cflags-y := -ffunction-sections
+else
+cflags-y := -mlong-calls
+endif
cflags-y += $(call cc-option, -mno-check-zero-division)
ifdef CONFIG_32BIT
diff --git a/arch/mips/include/asm/ftrace.h b/arch/mips/include/asm/ftrace.h
index 40a8c17..5f8ebcf 100644
--- a/arch/mips/include/asm/ftrace.h
+++ b/arch/mips/include/asm/ftrace.h
@@ -1 +1,24 @@
-/* empty */
+/*
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive for
+ * more details.
+ *
+ * Copyright (C) 2009 DSLab, Lanzhou University, China
+ * Author: Wu Zhangjin <[email protected]>
+ */
+
+#ifndef _ASM_MIPS_FTRACE_H
+#define _ASM_MIPS_FTRACE_H
+
+#ifdef CONFIG_FUNCTION_TRACER
+
+#define MCOUNT_ADDR ((unsigned long)(_mcount))
+#define MCOUNT_INSN_SIZE 4 /* sizeof mcount call */
+
+#ifndef __ASSEMBLY__
+extern void _mcount(void);
+#define mcount _mcount
+
+#endif /* __ASSEMBLY__ */
+#endif /* CONFIG_FUNCTION_TRACER */
+#endif /* _ASM_MIPS_FTRACE_H */
diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
index f228868..38e704a 100644
--- a/arch/mips/kernel/Makefile
+++ b/arch/mips/kernel/Makefile
@@ -10,6 +10,7 @@ obj-y += cpu-probe.o branch.o entry.o genex.o irq.o process.o \
ifdef CONFIG_FUNCTION_TRACER
CFLAGS_REMOVE_trace_clock.o = -pg
+CFLAGS_REMOVE_early_printk.o = -pg
endif
obj-$(CONFIG_CEVT_BCM1480) += cevt-bcm1480.o
@@ -29,6 +30,7 @@ obj-$(CONFIG_STACKTRACE) += stacktrace.o
obj-$(CONFIG_MODULES) += mips_ksyms.o module.o
obj-$(CONFIG_FTRACE) += trace_clock.o
+obj-$(CONFIG_FUNCTION_TRACER) += mcount.o
obj-$(CONFIG_CPU_LOONGSON2) += r4k_fpu.o r4k_switch.o
obj-$(CONFIG_CPU_MIPS32) += r4k_fpu.o r4k_switch.o
diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
new file mode 100644
index 0000000..0c39bc8
--- /dev/null
+++ b/arch/mips/kernel/mcount.S
@@ -0,0 +1,89 @@
+/*
+ * MIPS specific _mcount support
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive for
+ * more details.
+ *
+ * Copyright (C) 2009 Lemote Inc. & DSLab, Lanzhou University, China
+ * Author: Wu Zhangjin <[email protected]>
+ */
+
+#include <asm/regdef.h>
+#include <asm/stackframe.h>
+#include <asm/ftrace.h>
+
+ .text
+ .set noreorder
+ .set noat
+
+ .macro MCOUNT_SAVE_REGS
+ PTR_SUBU sp, PT_SIZE
+ PTR_S ra, PT_R31(sp)
+ PTR_S AT, PT_R1(sp)
+ PTR_S a0, PT_R4(sp)
+ PTR_S a1, PT_R5(sp)
+ PTR_S a2, PT_R6(sp)
+ PTR_S a3, PT_R7(sp)
+#ifdef CONFIG_64BIT
+ PTR_S a4, PT_R8(sp)
+ PTR_S a5, PT_R9(sp)
+ PTR_S a6, PT_R10(sp)
+ PTR_S a7, PT_R11(sp)
+#endif
+ .endm
+
+ .macro MCOUNT_RESTORE_REGS
+ PTR_L ra, PT_R31(sp)
+ PTR_L AT, PT_R1(sp)
+ PTR_L a0, PT_R4(sp)
+ PTR_L a1, PT_R5(sp)
+ PTR_L a2, PT_R6(sp)
+ PTR_L a3, PT_R7(sp)
+#ifdef CONFIG_64BIT
+ PTR_L a4, PT_R8(sp)
+ PTR_L a5, PT_R9(sp)
+ PTR_L a6, PT_R10(sp)
+ PTR_L a7, PT_R11(sp)
+#endif
+ PTR_ADDIU sp, PT_SIZE
+.endm
+
+ .macro MCOUNT_SET_ARGS
+ move a0, ra /* arg1: next ip, selfaddr */
+ move a1, AT /* arg2: the caller's next ip, parent */
+ PTR_SUBU a0, MCOUNT_INSN_SIZE
+ .endm
+
+ .macro RETURN_BACK
+#ifdef CONFIG_32BIT
+ PTR_ADDIU sp, 8
+#endif
+ jr ra
+ move ra, AT
+ .endm
+
+NESTED(_mcount, PT_SIZE, ra)
+ PTR_LA t0, ftrace_stub
+ PTR_L t1, ftrace_trace_function /* Prepare t1 for (1) */
+ bne t0, t1, static_trace
+ nop
+
+ j ftrace_stub
+ nop
+
+static_trace:
+ MCOUNT_SAVE_REGS
+
+ MCOUNT_SET_ARGS /* call *ftrace_trace_function */
+ jalr t1 /* (1) */
+ nop
+
+ MCOUNT_RESTORE_REGS
+ .globl ftrace_stub
+ftrace_stub:
+ RETURN_BACK
+ END(_mcount)
+
+ .set at
+ .set reorder
diff --git a/arch/mips/kernel/mips_ksyms.c b/arch/mips/kernel/mips_ksyms.c
index 225755d..1d04807 100644
--- a/arch/mips/kernel/mips_ksyms.c
+++ b/arch/mips/kernel/mips_ksyms.c
@@ -13,6 +13,7 @@
#include <asm/checksum.h>
#include <asm/pgtable.h>
#include <asm/uaccess.h>
+#include <asm/ftrace.h>
extern void *__bzero(void *__s, size_t __count);
extern long __strncpy_from_user_nocheck_asm(char *__to,
@@ -51,3 +52,7 @@ EXPORT_SYMBOL(csum_partial_copy_nocheck);
EXPORT_SYMBOL(__csum_partial_copy_user);
EXPORT_SYMBOL(invalid_pte_table);
+#ifdef CONFIG_FUNCTION_TRACER
+/* _mcount is defined in arch/mips/kernel/mcount.S */
+EXPORT_SYMBOL(_mcount);
+#endif
--
1.6.2.1
There is an exisiting common ftrace_test_stop_func() in
kernel/trace/ftrace.c, which is used to check the global variable
ftrace_trace_stop to determine whether stop the function tracing.
This patch implepment the MIPS specific one to speedup the procedure.
Thanks goes to Zhang Le for Cleaning it up.
Signed-off-by: Wu Zhangjin <[email protected]>
---
arch/mips/Kconfig | 1 +
arch/mips/kernel/mcount.S | 4 ++++
2 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 6b33e88..a9bd992 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -5,6 +5,7 @@ config MIPS
select HAVE_OPROFILE
select HAVE_ARCH_KGDB
select HAVE_FUNCTION_TRACER
+ select HAVE_FUNCTION_TRACE_MCOUNT_TEST
# Horrible source of confusion. Die, die, die ...
select EMBEDDED
select RTC_LIB if !LEMOTE_FULOONG2E
diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
index 0c39bc8..5dfaca8 100644
--- a/arch/mips/kernel/mcount.S
+++ b/arch/mips/kernel/mcount.S
@@ -64,6 +64,10 @@
.endm
NESTED(_mcount, PT_SIZE, ra)
+ lw t0, function_trace_stop
+ bnez t0, ftrace_stub
+ nop
+
PTR_LA t0, ftrace_stub
PTR_L t1, ftrace_trace_function /* Prepare t1 for (1) */
bne t0, t1, static_trace
--
1.6.2.1
MIPS and some other architectures need this argument to handle
big/little endian respectively.
Signed-off-by: Wu Zhangjin <[email protected]>
---
scripts/Makefile.build | 1 +
scripts/recordmcount.pl | 6 +++---
2 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 341b589..0b94d2f 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -207,6 +207,7 @@ endif
ifdef CONFIG_FTRACE_MCOUNT_RECORD
cmd_record_mcount = set -e ; perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
+ "$(if $(CONFIG_CPU_BIG_ENDIAN),big,little)" \
"$(if $(CONFIG_64BIT),64,32)" \
"$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" "$(NM)" "$(RM)" "$(MV)" \
"$(if $(part-of-module),1,0)" "$(@)";
diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 090d300..daee038 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -99,13 +99,13 @@ $P =~ s@.*/@@g;
my $V = '0.1';
-if ($#ARGV < 7) {
- print "usage: $P arch bits objdump objcopy cc ld nm rm mv is_module inputfile\n";
+if ($#ARGV < 8) {
+ print "usage: $P arch endian bits objdump objcopy cc ld nm rm mv is_module inputfile\n";
print "version: $V\n";
exit(1);
}
-my ($arch, $bits, $objdump, $objcopy, $cc,
+my ($arch, $endian, $bits, $objdump, $objcopy, $cc,
$ld, $nm, $rm, $mv, $is_module, $inputfile) = @ARGV;
# This file refers to mcount and shouldn't be ftraced, so lets' ignore it
--
1.6.2.1
With dynamic function tracer, by default, _mcount is defined as an
"empty" function, it just return without any more action, just like the
static function tracer does. When enabling it in user-space, it will
jump to a real tracing function(ftrace_caller), and do some real job for
us.
Differ from the static function tracer, dynamic function tracer provides
two functions ftrace_make_call()/ftrace_make_nop() to enable/disable the
tracing of some indicated kernel functions(set_ftrace_filter).
In the -v4 version, the implementation of this support is basically the same as
X86 version does: _mcount is implemented as an empty function and ftrace_caller
is implemented as a real tracing function respectively.
But in this version, to support module tracing with the help of
-mlong-calls. the stuff becomes a little more complex, let's have a look
at the new calling to _mcount with -mlong-calls(result of "objdump -hdr
vmlinux").
c: 3c030000 lui v1,0x0
c: R_MIPS_HI16 _mcount
c: R_MIPS_NONE *ABS*
c: R_MIPS_NONE *ABS*
10: 64630000 daddiu v1,v1,0
10: R_MIPS_LO16 _mcount
10: R_MIPS_NONE *ABS*
10: R_MIPS_NONE *ABS*
14: 03e0082d move at,ra
18: 0060f809 jalr v1
and the old version without -mlong-calls looks like this:
108: 03e0082d move at,ra
10c: 0c000000 jal 0 <fpcsr_pending>
10c: R_MIPS_26 _mcount
10c: R_MIPS_NONE *ABS*
10c: R_MIPS_NONE *ABS*
110: 00020021 nop
In the old version, there is only one "_mcount" string for every kernel
function, so, we just need to match this one in mcount_regex of
scripts/recordmcount.pl, but in the new version, we need to choose one
of the two to match. Herein, I choose the first one with "R_MIPS_HI16
_mcount".
and In the old verion, without module tracing support, we just need to replace
"jal _mcount" by "jal ftrace_caller" to do real tracing, and filter the tracing
of some kernel functions via replacing it by a nop instruction.
but as we have described before, the instruction "jal ftrace_caller" only left
32bit length for the address of ftrace_caller, it will fail when calling from
the module space. so, herein, we must replace something else.
the basic idea is loading the address of ftrace_caller to v1 via changing these
two instructions:
lui v1,0x0
addiu v1,v1,0
If we want to enable the tracing, we need to replace the above instructions to:
lui v1, HI_16BIT_ftrace_caller
addiu v1, v1, LOW_16BIT_ftrace_caller
If we want to stop the tracing of the indicated kernel functions, we
just need to replace the "jalr v1" to a nop instruction. but we need to
replace two instructions and encode the above two instructions
oursevles.
Is there a simpler solution, Yes! Here it is, in this version, we put _mcount
and ftrace_caller together, which means the address of _mcount and
ftrace_caller is the same:
_mcount:
ftrace_caller:
j ftrace_stub
nop
...(do real tracing here)...
ftrace_stub:
jr ra
move ra, at
By default, the kernel functions call _mcount, and then jump to ftrace_stub and
return. and when we want to do real tracing, we just need to remove that "j
ftrace_stub", and it will run through the two "nop" instructions and then do
the real tracing job.
what about filtering job? we just need to switching the "jalr v1" and "nop"
instruction.
In linux-mips64, there will be some local symbols, whose name are
prefixed by $L, which need to be filtered. thanks goes to Steven for
writing the mips64-specific function_regex.
In a conclusion, with RISC, things becomes easier with such a "stupid"
trick, RISC is something like K.I.S.S, and also, there are lots of
"simple" tricks in the whole ftrace support, thanks goes to Steven and
the other folks for providing such a wonderful tracing framework!
Signed-off-by: Wu Zhangjin <[email protected]>
---
arch/mips/Kconfig | 2 +
arch/mips/include/asm/ftrace.h | 12 ++++++
arch/mips/kernel/Makefile | 3 +-
arch/mips/kernel/ftrace.c | 76 ++++++++++++++++++++++++++++++++++++++++
arch/mips/kernel/mcount.S | 29 +++++++++++++++
scripts/recordmcount.pl | 38 ++++++++++++++++++++
6 files changed, 159 insertions(+), 1 deletions(-)
create mode 100644 arch/mips/kernel/ftrace.c
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index a9bd992..147fbbc 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -6,6 +6,8 @@ config MIPS
select HAVE_ARCH_KGDB
select HAVE_FUNCTION_TRACER
select HAVE_FUNCTION_TRACE_MCOUNT_TEST
+ select HAVE_DYNAMIC_FTRACE
+ select HAVE_FTRACE_MCOUNT_RECORD
# Horrible source of confusion. Die, die, die ...
select EMBEDDED
select RTC_LIB if !LEMOTE_FULOONG2E
diff --git a/arch/mips/include/asm/ftrace.h b/arch/mips/include/asm/ftrace.h
index 5f8ebcf..d5771e8 100644
--- a/arch/mips/include/asm/ftrace.h
+++ b/arch/mips/include/asm/ftrace.h
@@ -19,6 +19,18 @@
extern void _mcount(void);
#define mcount _mcount
+#ifdef CONFIG_DYNAMIC_FTRACE
+static inline unsigned long ftrace_call_adjust(unsigned long addr)
+{
+ /* 12 is the offset of "jalr v1" from the first loading instruction
+ * "lui v1, HI_16BIT", please get more information from
+ * scripts/recordmcount.pl */
+ return addr + 12;
+}
+
+struct dyn_arch_ftrace {
+};
+#endif /* CONFIG_DYNAMIC_FTRACE */
#endif /* __ASSEMBLY__ */
#endif /* CONFIG_FUNCTION_TRACER */
#endif /* _ASM_MIPS_FTRACE_H */
diff --git a/arch/mips/kernel/Makefile b/arch/mips/kernel/Makefile
index 38e704a..3cda3ab 100644
--- a/arch/mips/kernel/Makefile
+++ b/arch/mips/kernel/Makefile
@@ -10,6 +10,7 @@ obj-y += cpu-probe.o branch.o entry.o genex.o irq.o process.o \
ifdef CONFIG_FUNCTION_TRACER
CFLAGS_REMOVE_trace_clock.o = -pg
+CFLAGS_REMOVE_ftrace.o = -pg
CFLAGS_REMOVE_early_printk.o = -pg
endif
@@ -30,7 +31,7 @@ obj-$(CONFIG_STACKTRACE) += stacktrace.o
obj-$(CONFIG_MODULES) += mips_ksyms.o module.o
obj-$(CONFIG_FTRACE) += trace_clock.o
-obj-$(CONFIG_FUNCTION_TRACER) += mcount.o
+obj-$(CONFIG_FUNCTION_TRACER) += mcount.o ftrace.o
obj-$(CONFIG_CPU_LOONGSON2) += r4k_fpu.o r4k_switch.o
obj-$(CONFIG_CPU_MIPS32) += r4k_fpu.o r4k_switch.o
diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
new file mode 100644
index 0000000..0be30cf
--- /dev/null
+++ b/arch/mips/kernel/ftrace.c
@@ -0,0 +1,76 @@
+/*
+ * Code for replacing ftrace calls with jumps.
+ *
+ * Copyright (C) 2007-2008 Steven Rostedt <[email protected]>
+ * Copyright (C) 2009 DSLab, Lanzhou University, China
+ * Author: Wu Zhangjin <[email protected]>
+ *
+ * Thanks goes to Steven Rostedt for writing the original x86 version.
+ */
+
+#include <linux/uaccess.h>
+#include <linux/init.h>
+#include <linux/ftrace.h>
+
+#include <asm/cacheflush.h>
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+
+#define JAL 0x0c000000 /* jump & link: ip --> ra, jump to target */
+#define ADDR_MASK 0x03ffffff /* op_code|addr : 31...26|25 ....0 */
+
+#ifdef CONFIG_CPU_LOONGSON2F
+static unsigned int ftrace_nop = 0x00020021;
+#else
+static unsigned int ftrace_nop = 0x00000000;
+#endif
+
+static int ftrace_modify_code(unsigned long ip, unsigned int new_code)
+{
+ *(unsigned int *)ip = new_code;
+
+ flush_icache_range(ip, ip + 8);
+
+ return 0;
+}
+
+int ftrace_make_nop(struct module *mod,
+ struct dyn_ftrace *rec, unsigned long addr)
+{
+ return ftrace_modify_code(rec->ip, ftrace_nop);
+}
+
+static int modified; /* initialized as 0 by default */
+#define JALR_V1 0x0060f809
+
+int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
+{
+ /* We just need to remove the "j ftrace_stub" at the fist time! */
+ if (modified == 0) {
+ modified = 1;
+ ftrace_modify_code(addr, ftrace_nop);
+ }
+ return ftrace_modify_code(rec->ip, JALR_V1);
+}
+
+#define FTRACE_CALL_IP ((unsigned long)(&ftrace_call))
+#define jump_insn_encode(op_code, addr) \
+ ((unsigned int)((op_code) | (((addr) >> 2) & ADDR_MASK)))
+
+int ftrace_update_ftrace_func(ftrace_func_t func)
+{
+ unsigned int new;
+
+ new = jump_insn_encode(JAL, (unsigned long)func);
+
+ return ftrace_modify_code(FTRACE_CALL_IP, new);
+}
+
+int __init ftrace_dyn_arch_init(void *data)
+{
+ /* The return code is retured via data */
+ *(unsigned long *)data = 0;
+
+ return 0;
+}
+#endif /* CONFIG_DYNAMIC_FTRACE */
diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
index 5dfaca8..389be7b 100644
--- a/arch/mips/kernel/mcount.S
+++ b/arch/mips/kernel/mcount.S
@@ -63,6 +63,33 @@
move ra, AT
.endm
+#ifdef CONFIG_DYNAMIC_FTRACE
+
+NESTED(ftrace_caller, PT_SIZE, ra)
+ .globl _mcount
+_mcount:
+ j ftrace_stub
+ nop
+ lw t0, function_trace_stop
+ bnez t0, ftrace_stub
+ nop
+
+ MCOUNT_SAVE_REGS
+
+ MCOUNT_SET_ARGS
+ .globl ftrace_call
+ftrace_call:
+ nop /* a placeholder for the call to a real tracing function */
+ nop /* Do not touch me, I'm in the dealy slot of "jal func" */
+
+ MCOUNT_RESTORE_REGS
+ .globl ftrace_stub
+ftrace_stub:
+ RETURN_BACK
+ END(ftrace_caller)
+
+#else /* ! CONFIG_DYNAMIC_FTRACE */
+
NESTED(_mcount, PT_SIZE, ra)
lw t0, function_trace_stop
bnez t0, ftrace_stub
@@ -89,5 +116,7 @@ ftrace_stub:
RETURN_BACK
END(_mcount)
+#endif /* ! CONFIG_DYNAMIC_FTRACE */
+
.set at
.set reorder
diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index daee038..aefe777 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -245,6 +245,44 @@ if ($arch eq "x86_64") {
$ld .= " -m elf64_sparc";
$cc .= " -m64";
$objcopy .= " -O elf64-sparc";
+
+} elsif ($arch eq "mips") {
+ # To enable module support, we need to enable the -mlong-calls option
+ # of gcc for MIPS, after using this option, we can not get the real
+ # offset of the calling to _mcount, but the offset of the first loading
+ # instruction or the second one, here we get the first one, and then,
+ # we plus it with 12 in arch/mips/kernel/asm/ftrace.h.
+ #
+ # c: 3c030000 lui v1,0x0
+ # c: R_MIPS_HI16 _mcount
+ # c: R_MIPS_NONE *ABS*
+ # c: R_MIPS_NONE *ABS*
+ # 10: 64630000 daddiu v1,v1,0
+ # 10: R_MIPS_LO16 _mcount
+ # 10: R_MIPS_NONE *ABS*
+ # 10: R_MIPS_NONE *ABS*
+ # 14: 03e0082d move at,ra
+ # 18: 0060f809 jalr v1
+ $mcount_regex = "^\\s*([0-9a-fA-F]+): R_MIPS_HI16\\s+_mcount\$";
+ $objdump .= " -Melf-trad".$endian."mips ";
+
+ if ($endian eq "big") {
+ $endian = " -EB ";
+ $ld .= " -melf".$bits."btsmip";
+ } else {
+ $endian = " -EL ";
+ $ld .= " -melf".$bits."ltsmip";
+ }
+
+ $cc .= " -mno-abicalls -fno-pic -mabi=" . $bits . $endian;
+ $ld .= $endian;
+
+ if ($bits == 64) {
+ $function_regex =
+ "^([0-9a-fA-F]+)\\s+<(.|[^\$]L.*?|\$[^L].*?|[^\$][^L].*?)>:";
+ $type = ".dword";
+ }
+
} else {
die "Arch $arch is not supported with CONFIG_FTRACE_MCOUNT_RECORD";
}
--
1.6.2.1
We use mips_timecounter_init() to get the timestamp in MIPS, so, it's
better to not trace it, otherwise, it will goto recursion(hang) when
using function graph tracer.
but this patch have touched the common part in kernel/time/clocksource.c
and include/linux/clocksource.h, so it is not good, perhaps the better
solution is moving the whole mips_timecounter_init() implementation into
arch/mips, but that will introduce source code duplication. any other
solution?
Signed-off-by: Wu Zhangjin <[email protected]>
---
arch/mips/include/asm/time.h | 2 +-
arch/mips/kernel/csrc-r4k.c | 4 ++--
include/linux/clocksource.h | 2 +-
kernel/time/clocksource.c | 4 ++--
4 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/mips/include/asm/time.h b/arch/mips/include/asm/time.h
index b8af7fa..e1c660f 100644
--- a/arch/mips/include/asm/time.h
+++ b/arch/mips/include/asm/time.h
@@ -77,7 +77,7 @@ extern int init_r4k_timecounter(void);
extern u64 r4k_timecounter_read(void);
#endif
-static inline u64 mips_timecounter_read(void)
+static inline u64 notrace mips_timecounter_read(void)
{
#ifdef CONFIG_CSRC_R4K
return r4k_timecounter_read();
diff --git a/arch/mips/kernel/csrc-r4k.c b/arch/mips/kernel/csrc-r4k.c
index 4e7705f..0690bea 100644
--- a/arch/mips/kernel/csrc-r4k.c
+++ b/arch/mips/kernel/csrc-r4k.c
@@ -42,7 +42,7 @@ static struct timecounter r4k_tc = {
.cc = NULL,
};
-static cycle_t r4k_cc_read(const struct cyclecounter *cc)
+static cycle_t notrace r4k_cc_read(const struct cyclecounter *cc)
{
return read_c0_count();
}
@@ -66,7 +66,7 @@ int __init init_r4k_timecounter(void)
return 0;
}
-u64 r4k_timecounter_read(void)
+u64 notrace r4k_timecounter_read(void)
{
u64 clock;
diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
index 83d2fbd..2a02992 100644
--- a/include/linux/clocksource.h
+++ b/include/linux/clocksource.h
@@ -73,7 +73,7 @@ struct timecounter {
* XXX - This could use some mult_lxl_ll() asm optimization. Same code
* as in cyc2ns, but with unsigned result.
*/
-static inline u64 cyclecounter_cyc2ns(const struct cyclecounter *cc,
+static inline u64 notrace cyclecounter_cyc2ns(const struct cyclecounter *cc,
cycle_t cycles)
{
u64 ret = (u64)cycles;
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 5e18c6a..9ce9d02 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -52,7 +52,7 @@ EXPORT_SYMBOL(timecounter_init);
* The first call to this function for a new time counter initializes
* the time tracking and returns an undefined result.
*/
-static u64 timecounter_read_delta(struct timecounter *tc)
+static u64 notrace timecounter_read_delta(struct timecounter *tc)
{
cycle_t cycle_now, cycle_delta;
u64 ns_offset;
@@ -72,7 +72,7 @@ static u64 timecounter_read_delta(struct timecounter *tc)
return ns_offset;
}
-u64 timecounter_read(struct timecounter *tc)
+u64 notrace timecounter_read(struct timecounter *tc)
{
u64 nsec;
--
1.6.2.1
This patch fix the following error with FUNCTION_GRAPH_TRACER=y:
kernel/built-in.o: In function `print_graph_irq':
trace_functions_graph.c:(.text+0x6dba0): undefined reference to `__irqentry_text_start'
trace_functions_graph.c:(.text+0x6dba8): undefined reference to `__irqentry_text_start'
trace_functions_graph.c:(.text+0x6dbd0): undefined reference to `__irqentry_text_end'
trace_functions_graph.c:(.text+0x6dbd4): undefined reference to `__irqentry_text_end'
(This patch is need to support function graph tracer of MIPS)
Signed-off-by: Wu Zhangjin <[email protected]>
---
arch/mips/kernel/vmlinux.lds.S | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/arch/mips/kernel/vmlinux.lds.S b/arch/mips/kernel/vmlinux.lds.S
index 162b299..f25df73 100644
--- a/arch/mips/kernel/vmlinux.lds.S
+++ b/arch/mips/kernel/vmlinux.lds.S
@@ -46,6 +46,7 @@ SECTIONS
SCHED_TEXT
LOCK_TEXT
KPROBES_TEXT
+ IRQENTRY_TEXT
*(.text.*)
*(.fixup)
*(.gnu.warning)
--
1.6.2.1
The implementation of function graph tracer for MIPS is a little
different from X86.
in MIPS, gcc(with -pg) only transfer the caller's return address(at) and
the _mcount's return address(ra) to us.
move at, ra
jal _mcount
if the function is a leaf, it will no save the return address(ra):
ffffffff80101298 <au1k_wait>:
ffffffff80101298: 67bdfff0 daddiu sp,sp,-16
ffffffff8010129c: ffbe0008 sd s8,8(sp)
ffffffff801012a0: 03a0f02d move s8,sp
ffffffff801012a4: 03e0082d move at,ra
ffffffff801012a8: 0c042930 jal ffffffff8010a4c0 <_mcount>
ffffffff801012ac: 00020021 nop
so, we can hijack it directly in _mcount, but if the function is non-leaf, the
return address is saved in the stack.
ffffffff80133030 <copy_process>:
ffffffff80133030: 67bdff50 daddiu sp,sp,-176
ffffffff80133034: ffbe00a0 sd s8,160(sp)
ffffffff80133038: 03a0f02d move s8,sp
ffffffff8013303c: ffbf00a8 sd ra,168(sp)
ffffffff80133040: ffb70098 sd s7,152(sp)
ffffffff80133044: ffb60090 sd s6,144(sp)
ffffffff80133048: ffb50088 sd s5,136(sp)
ffffffff8013304c: ffb40080 sd s4,128(sp)
ffffffff80133050: ffb30078 sd s3,120(sp)
ffffffff80133054: ffb20070 sd s2,112(sp)
ffffffff80133058: ffb10068 sd s1,104(sp)
ffffffff8013305c: ffb00060 sd s0,96(sp)
ffffffff80133060: 03e0082d move at,ra
ffffffff80133064: 0c042930 jal ffffffff8010a4c0 <_mcount>
ffffffff80133068: 00020021 nop
but we can not get the exact stack address(which saved ra) directly in
_mcount, we need to search the content of at register in the stack space
or search the "s{d,w} ra, offset(sp)" instruction in the text. 'Cause we
can not prove there is only a match in the stack space, so, we search
the text instead.
as we can see, if the first instruction above "move at, ra" is "move
s8(fp), sp"(only available with -fno-omit-frame-pointer which is enabled
by CONFIG_FRAME_POINTER), it is a leaf function, so we hijack the at
register directly via putting &return_to_handler into it, otherwise, we
search the "s{d,w} ra, offset(sp)" instruction to get the stack offset,
and then the stack address. we use the above copy_process() as an
example, we at last find "ffbf00a8", 0xa8 is the stack offset, we plus
it with s8(fp), that is the stack address, we hijack the content via
writing the &return_to_handler in.
(As pajko <[email protected]> recommend, if we enable
PROFILE_BEFORE_PROLOGUE in gcc/config/mips/mips.h, there will be no
difference between non-leaf and leaf function, we hope
PROFILE_BEFORE_PROLOGUE will be enabled by default in the future
version of gcc, so, we can remove ftrace_get_parent_addr() directly. )
Signed-off-by: Wu Zhangjin <[email protected]>
---
arch/mips/Kconfig | 1 +
arch/mips/kernel/ftrace.c | 93 +++++++++++++++++++++++++++++++++++++++++++++
arch/mips/kernel/mcount.S | 47 ++++++++++++++++++++++-
3 files changed, 140 insertions(+), 1 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 147fbbc..de690fd 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -8,6 +8,7 @@ config MIPS
select HAVE_FUNCTION_TRACE_MCOUNT_TEST
select HAVE_DYNAMIC_FTRACE
select HAVE_FTRACE_MCOUNT_RECORD
+ select HAVE_FUNCTION_GRAPH_TRACER
# Horrible source of confusion. Die, die, die ...
select EMBEDDED
select RTC_LIB if !LEMOTE_FULOONG2E
diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index 0be30cf..4cf11f5 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -13,6 +13,8 @@
#include <linux/ftrace.h>
#include <asm/cacheflush.h>
+#include <asm/asm.h>
+#include <asm/asm-offsets.h>
#ifdef CONFIG_DYNAMIC_FTRACE
@@ -74,3 +76,94 @@ int __init ftrace_dyn_arch_init(void *data)
return 0;
}
#endif /* CONFIG_DYNAMIC_FTRACE */
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+
+#define S_RA (0x2fbf << 16) /* 32bit: afbf, 64bit: ffbf */
+#define MOV_FP_SP 0x03a0f021 /* 32bit: 0x03a0f021, 64bit: 0x03a0f02d */
+#define STACK_OFFSET_MASK 0xfff /* stack offset range: 0 ~ PT_SIZE(304) */
+
+unsigned long ftrace_get_parent_addr(unsigned long self_addr,
+ unsigned long parent,
+ unsigned long parent_addr,
+ unsigned long fp)
+{
+ unsigned long sp, ip, ra;
+ unsigned int code;
+
+ /* move to the instruction "move ra, at" */
+ ip = self_addr - 8;
+
+ /* search the text until finding the "move s8, sp" instruction or
+ * "s{d,w} ra, offset(sp)" instruction */
+ do {
+ ip -= 4;
+
+ /* get the code at "ip" */
+ code = *(unsigned int *)ip;
+
+ /* If we hit the "move s8(fp), sp" instruction before finding
+ * where the ra is stored, then this is a leaf function and it
+ * does not store the ra on the stack. */
+ if ((code & MOV_FP_SP) == MOV_FP_SP)
+ return parent_addr;
+ } while (((code & S_RA) != S_RA));
+
+ sp = fp + (code & STACK_OFFSET_MASK);
+ ra = *(unsigned long *)sp;
+
+ if (ra == parent)
+ return sp;
+
+ ftrace_graph_stop();
+ WARN_ON(1);
+ return parent_addr;
+}
+
+/*
+ * Hook the return address and push it in the stack of return addrs
+ * in current thread info.
+ */
+void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
+ unsigned long fp)
+{
+ unsigned long old;
+ struct ftrace_graph_ent trace;
+ unsigned long return_hooker = (unsigned long)
+ &return_to_handler;
+
+ if (unlikely(atomic_read(¤t->tracing_graph_pause)))
+ return;
+
+ /* "parent" is the stack address saved the return address of the caller
+ * of _mcount, for a leaf function not save the return address in the
+ * stack address, so, we "emulate" one in _mcount's stack space, and
+ * hijack it directly, but for a non-leaf function, it will save the
+ * return address to the its stack space, so, we can not hijack the
+ * "parent" directly, but need to find the real stack address,
+ * ftrace_get_parent_addr() does it!
+ */
+
+ old = *parent;
+
+ parent = (unsigned long *)ftrace_get_parent_addr(self_addr, old,
+ (unsigned long)parent,
+ fp);
+
+ *parent = return_hooker;
+
+ if (ftrace_push_return_trace(old, self_addr, &trace.depth, fp) ==
+ -EBUSY) {
+ *parent = old;
+ return;
+ }
+
+ trace.func = self_addr;
+
+ /* Only trace if the calling function expects to */
+ if (!ftrace_graph_entry(&trace)) {
+ current->curr_ret_stack--;
+ *parent = old;
+ }
+}
+#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
index 389be7b..a9ba888 100644
--- a/arch/mips/kernel/mcount.S
+++ b/arch/mips/kernel/mcount.S
@@ -99,7 +99,15 @@ NESTED(_mcount, PT_SIZE, ra)
PTR_L t1, ftrace_trace_function /* Prepare t1 for (1) */
bne t0, t1, static_trace
nop
-
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ PTR_L t2, ftrace_graph_return
+ bne t0, t2, ftrace_graph_caller
+ nop
+ PTR_LA t0, ftrace_graph_entry_stub
+ PTR_L t2, ftrace_graph_entry
+ bne t0, t2, ftrace_graph_caller
+ nop
+#endif
j ftrace_stub
nop
@@ -118,5 +126,42 @@ ftrace_stub:
#endif /* ! CONFIG_DYNAMIC_FTRACE */
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+
+NESTED(ftrace_graph_caller, PT_SIZE, ra)
+ MCOUNT_SAVE_REGS
+
+ PTR_LA a0, PT_R1(sp) /* arg1: &AT -> a0 */
+ move a1, ra /* arg2: next ip, selfaddr */
+ PTR_SUBU a1, MCOUNT_INSN_SIZE
+ move a2, fp /* arg3: frame pointer */
+ jal prepare_ftrace_return
+ nop
+
+ MCOUNT_RESTORE_REGS
+ RETURN_BACK
+ END(ftrace_graph_caller)
+
+ .align 2
+ .globl return_to_handler
+return_to_handler:
+ PTR_SUBU sp, PT_SIZE
+ PTR_S v0, PT_R2(sp)
+ PTR_S v1, PT_R3(sp)
+
+ jal ftrace_return_to_handler
+ nop
+
+ /* restore the real parent address: v0 -> ra */
+ move ra, v0
+
+ PTR_L v0, PT_R2(sp)
+ PTR_L v1, PT_R3(sp)
+ PTR_ADDIU sp, PT_SIZE
+
+ jr ra
+ nop
+#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
+
.set at
.set reorder
--
1.6.2.1
This patch make function graph tracer work with dynamic function tracer.
To share the source code of dynamic function tracer(MCOUNT_SAVE_REGS),
and avoid restoring the whole saved registers, we need to restore the ra
register from the stack.
Signed-off-by: Wu Zhangjin <[email protected]>
---
arch/mips/kernel/ftrace.c | 21 +++++++++++++++++++++
arch/mips/kernel/mcount.S | 14 ++++++++++++--
2 files changed, 33 insertions(+), 2 deletions(-)
diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index 4cf11f5..1c02e65 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -79,6 +79,27 @@ int __init ftrace_dyn_arch_init(void *data)
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+#ifdef CONFIG_DYNAMIC_FTRACE
+
+extern void ftrace_graph_call(void);
+#define JMP 0x08000000 /* jump to target directly */
+#define CALL_FTRACE_GRAPH_CALLER \
+ jump_insn_encode(JMP, (unsigned long)(&ftrace_graph_caller))
+#define FTRACE_GRAPH_CALL_IP ((unsigned long)(&ftrace_graph_call))
+
+int ftrace_enable_ftrace_graph_caller(void)
+{
+ return ftrace_modify_code(FTRACE_GRAPH_CALL_IP,
+ CALL_FTRACE_GRAPH_CALLER);
+}
+
+int ftrace_disable_ftrace_graph_caller(void)
+{
+ return ftrace_modify_code(FTRACE_GRAPH_CALL_IP, ftrace_nop);
+}
+
+#endif /* !CONFIG_DYNAMIC_FTRACE */
+
#define S_RA (0x2fbf << 16) /* 32bit: afbf, 64bit: ffbf */
#define MOV_FP_SP 0x03a0f021 /* 32bit: 0x03a0f021, 64bit: 0x03a0f02d */
#define STACK_OFFSET_MASK 0xfff /* stack offset range: 0 ~ PT_SIZE(304) */
diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
index a9ba888..260719d 100644
--- a/arch/mips/kernel/mcount.S
+++ b/arch/mips/kernel/mcount.S
@@ -82,6 +82,13 @@ ftrace_call:
nop /* a placeholder for the call to a real tracing function */
nop /* Do not touch me, I'm in the dealy slot of "jal func" */
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ .globl ftrace_graph_call
+ftrace_graph_call:
+ nop
+ nop
+#endif
+
MCOUNT_RESTORE_REGS
.globl ftrace_stub
ftrace_stub:
@@ -129,10 +136,13 @@ ftrace_stub:
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
NESTED(ftrace_graph_caller, PT_SIZE, ra)
+#ifdef CONFIG_DYNAMIC_FTRACE
+ PTR_L a1, PT_R31(sp) /* load the original ra from the stack */
+#else
MCOUNT_SAVE_REGS
-
- PTR_LA a0, PT_R1(sp) /* arg1: &AT -> a0 */
move a1, ra /* arg2: next ip, selfaddr */
+#endif
+ PTR_LA a0, PT_R1(sp) /* arg1: &AT -> a0 */
PTR_SUBU a1, MCOUNT_INSN_SIZE
move a2, fp /* arg3: frame pointer */
jal prepare_ftrace_return
--
1.6.2.1
2009/10/25 Wu Zhangjin <[email protected]>:
> -static inline u64 mips_timecounter_read(void)
> +static inline u64 notrace mips_timecounter_read(void)
You don't need to set notrace functions, unless their addresses
are referenced somewhere, which unfortunately might happen
for some functions but this is rare.
> ?{
> ?#ifdef CONFIG_CSRC_R4K
> ? ? ? ?return r4k_timecounter_read();
> diff --git a/arch/mips/kernel/csrc-r4k.c b/arch/mips/kernel/csrc-r4k.c
> index 4e7705f..0690bea 100644
> --- a/arch/mips/kernel/csrc-r4k.c
> +++ b/arch/mips/kernel/csrc-r4k.c
> @@ -42,7 +42,7 @@ static struct timecounter r4k_tc = {
> ? ? ? ?.cc = NULL,
> ?};
>
> -static cycle_t r4k_cc_read(const struct cyclecounter *cc)
> +static cycle_t notrace r4k_cc_read(const struct cyclecounter *cc)
> ?{
> ? ? ? ?return read_c0_count();
> ?}
> @@ -66,7 +66,7 @@ int __init init_r4k_timecounter(void)
> ? ? ? ?return 0;
> ?}
>
> -u64 r4k_timecounter_read(void)
> +u64 notrace r4k_timecounter_read(void)
> ?{
> ? ? ? ?u64 clock;
>
> diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> index 83d2fbd..2a02992 100644
> --- a/include/linux/clocksource.h
> +++ b/include/linux/clocksource.h
> @@ -73,7 +73,7 @@ struct timecounter {
> ?* XXX - This could use some mult_lxl_ll() asm optimization. Same code
> ?* as in cyc2ns, but with unsigned result.
> ?*/
> -static inline u64 cyclecounter_cyc2ns(const struct cyclecounter *cc,
> +static inline u64 notrace cyclecounter_cyc2ns(const struct cyclecounter
ditto here.
*cc,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?cycle_t cycles)
> ?{
> ? ? ? ?u64 ret = (u64)cycles;
> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
> index 5e18c6a..9ce9d02 100644
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -52,7 +52,7 @@ EXPORT_SYMBOL(timecounter_init);
> ?* The first call to this function for a new time counter initializes
> ?* the time tracking and returns an undefined result.
> ?*/
> -static u64 timecounter_read_delta(struct timecounter *tc)
> +static u64 notrace timecounter_read_delta(struct timecounter *tc)
> ?{
> ? ? ? ?cycle_t cycle_now, cycle_delta;
> ? ? ? ?u64 ns_offset;
> @@ -72,7 +72,7 @@ static u64 timecounter_read_delta(struct timecounter
Hmm yeah this is not very nice to do that in core functions because
of a specific arch problem.
At least you have __notrace_funcgraph, this is a notrace
that only applies if CONFIG_FUNCTION_GRAPH_TRACER
so that it's still traceable by the function tracer in this case.
But I would rather see a __mips_notrace on these two core functions.
2009/10/25 Wu Zhangjin <[email protected]>:
> This patch fix the following error with FUNCTION_GRAPH_TRACER=y:
>
> kernel/built-in.o: In function `print_graph_irq':
> trace_functions_graph.c:(.text+0x6dba0): undefined reference to `__irqentry_text_start'
> trace_functions_graph.c:(.text+0x6dba8): undefined reference to `__irqentry_text_start'
> trace_functions_graph.c:(.text+0x6dbd0): undefined reference to `__irqentry_text_end'
> trace_functions_graph.c:(.text+0x6dbd4): undefined reference to `__irqentry_text_end'
>
> (This patch is need to support function graph tracer of MIPS)
If you want to enjoy this section, you'd need to tag the
mips irq entry functions with "__irq_entry" :)
I guess there is a do_IRQ() in mips that is waiting for that (and
probably some others).
The effect is that interrupt areas are cut with a pair of arrows
in the trace, so that you more easily spot interrupts in the traces
May be I missed this part in another patch in this series though...
2009/10/25 Wu Zhangjin <[email protected]>:
> Under the help of the -mlong-calls option of gcc, This revision comes with the
> module tracing support. and there are also lots of cleanups and fixes.
> therefore, it is ready to upstream.
>
> Currently, function graph tracer support have some overhead for searching the
> stack address of the return address(ra). we hope the PROFILE_BEFORE_PROLOGUE
> macro will be enabled by default in the next version of gcc, and then the
> overhead can be removed via hijacking the ra register directly for both
> non-leaf and leaf function.
>
> Thanks very much to the people in the CC list for their feedbacks, all the
> lastest changes goes to this git repo:
>
> git://dev.lemote.com/rt4ls.git ?linux-mips/dev/ftrace-upstream
>
> Welcome to play with it and give more feedbacks.
>
> Thanks & Regards,
> ? ? ? Wu Zhangjin
>
> Wu Zhangjin (11):
> ?tracing: convert trace_clock_local() as weak function
> ?MIPS: add mips_timecounter_read() to get high precision timestamp
> ?tracing: add MIPS specific trace_clock_local()
> ?tracing: add static function tracer support for MIPS
> ?tracing: enable HAVE_FUNCTION_TRACE_MCOUNT_TEST for MIPS
> ?tracing: add an endian argument to scripts/recordmcount.pl
> ?tracing: add dynamic function tracer support for MIPS
> ?tracing: not trace mips_timecounter_init() in MIPS
> ?tracing: add IRQENTRY_EXIT for MIPS
> ?tracing: add function graph tracer support for MIPS
I can't find this one in the series. Did you forget it or am I somehow
missing it?
Thanks.
(Add Frederic Weisbecker <[email protected]> to the CC list)
The implementation of function graph tracer for MIPS is a little
different from X86.
in MIPS, gcc(with -pg) only transfer the caller's return address(at) and
the _mcount's return address(ra) to us.
move at, ra
jal _mcount
if the function is a leaf, it will no save the return address(ra):
ffffffff80101298 <au1k_wait>:
ffffffff80101298: 67bdfff0 daddiu sp,sp,-16
ffffffff8010129c: ffbe0008 sd s8,8(sp)
ffffffff801012a0: 03a0f02d move s8,sp
ffffffff801012a4: 03e0082d move at,ra
ffffffff801012a8: 0c042930 jal ffffffff8010a4c0 <_mcount>
ffffffff801012ac: 00020021 nop
so, we can hijack it directly in _mcount, but if the function is non-leaf, the
return address is saved in the stack.
ffffffff80133030 <copy_process>:
ffffffff80133030: 67bdff50 daddiu sp,sp,-176
ffffffff80133034: ffbe00a0 sd s8,160(sp)
ffffffff80133038: 03a0f02d move s8,sp
ffffffff8013303c: ffbf00a8 sd ra,168(sp)
ffffffff80133040: ffb70098 sd s7,152(sp)
ffffffff80133044: ffb60090 sd s6,144(sp)
ffffffff80133048: ffb50088 sd s5,136(sp)
ffffffff8013304c: ffb40080 sd s4,128(sp)
ffffffff80133050: ffb30078 sd s3,120(sp)
ffffffff80133054: ffb20070 sd s2,112(sp)
ffffffff80133058: ffb10068 sd s1,104(sp)
ffffffff8013305c: ffb00060 sd s0,96(sp)
ffffffff80133060: 03e0082d move at,ra
ffffffff80133064: 0c042930 jal ffffffff8010a4c0 <_mcount>
ffffffff80133068: 00020021 nop
but we can not get the exact stack address(which saved ra) directly in
_mcount, we need to search the content of at register in the stack space
or search the "s{d,w} ra, offset(sp)" instruction in the text. 'Cause we
can not prove there is only a match in the stack space, so, we search
the text instead.
as we can see, if the first instruction above "move at, ra" is "move
s8(fp), sp"(only available with -fno-omit-frame-pointer which is enabled
by CONFIG_FRAME_POINTER), it is a leaf function, so we hijack the at
register directly via putting &return_to_handler into it, otherwise, we
search the "s{d,w} ra, offset(sp)" instruction to get the stack offset,
and then the stack address. we use the above copy_process() as an
example, we at last find "ffbf00a8", 0xa8 is the stack offset, we plus
it with s8(fp), that is the stack address, we hijack the content via
writing the &return_to_handler in.
(As pajko <[email protected]> recommend, if we enable
PROFILE_BEFORE_PROLOGUE in gcc/config/mips/mips.h, there will be no
difference between non-leaf and leaf function, we hope
PROFILE_BEFORE_PROLOGUE will be enabled by default in the future
version of gcc, so, we can remove ftrace_get_parent_addr() directly. )
Signed-off-by: Wu Zhangjin <[email protected]>
---
arch/mips/Kconfig | 1 +
arch/mips/kernel/ftrace.c | 93 +++++++++++++++++++++++++++++++++++++++++++++
arch/mips/kernel/mcount.S | 47 ++++++++++++++++++++++-
3 files changed, 140 insertions(+), 1 deletions(-)
diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 147fbbc..de690fd 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -8,6 +8,7 @@ config MIPS
select HAVE_FUNCTION_TRACE_MCOUNT_TEST
select HAVE_DYNAMIC_FTRACE
select HAVE_FTRACE_MCOUNT_RECORD
+ select HAVE_FUNCTION_GRAPH_TRACER
# Horrible source of confusion. Die, die, die ...
select EMBEDDED
select RTC_LIB if !LEMOTE_FULOONG2E
diff --git a/arch/mips/kernel/ftrace.c b/arch/mips/kernel/ftrace.c
index 0be30cf..4cf11f5 100644
--- a/arch/mips/kernel/ftrace.c
+++ b/arch/mips/kernel/ftrace.c
@@ -13,6 +13,8 @@
#include <linux/ftrace.h>
#include <asm/cacheflush.h>
+#include <asm/asm.h>
+#include <asm/asm-offsets.h>
#ifdef CONFIG_DYNAMIC_FTRACE
@@ -74,3 +76,94 @@ int __init ftrace_dyn_arch_init(void *data)
return 0;
}
#endif /* CONFIG_DYNAMIC_FTRACE */
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+
+#define S_RA (0x2fbf << 16) /* 32bit: afbf, 64bit: ffbf */
+#define MOV_FP_SP 0x03a0f021 /* 32bit: 0x03a0f021, 64bit: 0x03a0f02d */
+#define STACK_OFFSET_MASK 0xfff /* stack offset range: 0 ~ PT_SIZE(304) */
+
+unsigned long ftrace_get_parent_addr(unsigned long self_addr,
+ unsigned long parent,
+ unsigned long parent_addr,
+ unsigned long fp)
+{
+ unsigned long sp, ip, ra;
+ unsigned int code;
+
+ /* move to the instruction "move ra, at" */
+ ip = self_addr - 8;
+
+ /* search the text until finding the "move s8, sp" instruction or
+ * "s{d,w} ra, offset(sp)" instruction */
+ do {
+ ip -= 4;
+
+ /* get the code at "ip" */
+ code = *(unsigned int *)ip;
+
+ /* If we hit the "move s8(fp), sp" instruction before finding
+ * where the ra is stored, then this is a leaf function and it
+ * does not store the ra on the stack. */
+ if ((code & MOV_FP_SP) == MOV_FP_SP)
+ return parent_addr;
+ } while (((code & S_RA) != S_RA));
+
+ sp = fp + (code & STACK_OFFSET_MASK);
+ ra = *(unsigned long *)sp;
+
+ if (ra == parent)
+ return sp;
+
+ ftrace_graph_stop();
+ WARN_ON(1);
+ return parent_addr;
+}
+
+/*
+ * Hook the return address and push it in the stack of return addrs
+ * in current thread info.
+ */
+void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
+ unsigned long fp)
+{
+ unsigned long old;
+ struct ftrace_graph_ent trace;
+ unsigned long return_hooker = (unsigned long)
+ &return_to_handler;
+
+ if (unlikely(atomic_read(¤t->tracing_graph_pause)))
+ return;
+
+ /* "parent" is the stack address saved the return address of the caller
+ * of _mcount, for a leaf function not save the return address in the
+ * stack address, so, we "emulate" one in _mcount's stack space, and
+ * hijack it directly, but for a non-leaf function, it will save the
+ * return address to the its stack space, so, we can not hijack the
+ * "parent" directly, but need to find the real stack address,
+ * ftrace_get_parent_addr() does it!
+ */
+
+ old = *parent;
+
+ parent = (unsigned long *)ftrace_get_parent_addr(self_addr, old,
+ (unsigned long)parent,
+ fp);
+
+ *parent = return_hooker;
+
+ if (ftrace_push_return_trace(old, self_addr, &trace.depth, fp) ==
+ -EBUSY) {
+ *parent = old;
+ return;
+ }
+
+ trace.func = self_addr;
+
+ /* Only trace if the calling function expects to */
+ if (!ftrace_graph_entry(&trace)) {
+ current->curr_ret_stack--;
+ *parent = old;
+ }
+}
+#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
diff --git a/arch/mips/kernel/mcount.S b/arch/mips/kernel/mcount.S
index 389be7b..a9ba888 100644
--- a/arch/mips/kernel/mcount.S
+++ b/arch/mips/kernel/mcount.S
@@ -99,7 +99,15 @@ NESTED(_mcount, PT_SIZE, ra)
PTR_L t1, ftrace_trace_function /* Prepare t1 for (1) */
bne t0, t1, static_trace
nop
-
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+ PTR_L t2, ftrace_graph_return
+ bne t0, t2, ftrace_graph_caller
+ nop
+ PTR_LA t0, ftrace_graph_entry_stub
+ PTR_L t2, ftrace_graph_entry
+ bne t0, t2, ftrace_graph_caller
+ nop
+#endif
j ftrace_stub
nop
@@ -118,5 +126,42 @@ ftrace_stub:
#endif /* ! CONFIG_DYNAMIC_FTRACE */
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+
+NESTED(ftrace_graph_caller, PT_SIZE, ra)
+ MCOUNT_SAVE_REGS
+
+ PTR_LA a0, PT_R1(sp) /* arg1: &AT -> a0 */
+ move a1, ra /* arg2: next ip, selfaddr */
+ PTR_SUBU a1, MCOUNT_INSN_SIZE
+ move a2, fp /* arg3: frame pointer */
+ jal prepare_ftrace_return
+ nop
+
+ MCOUNT_RESTORE_REGS
+ RETURN_BACK
+ END(ftrace_graph_caller)
+
+ .align 2
+ .globl return_to_handler
+return_to_handler:
+ PTR_SUBU sp, PT_SIZE
+ PTR_S v0, PT_R2(sp)
+ PTR_S v1, PT_R3(sp)
+
+ jal ftrace_return_to_handler
+ nop
+
+ /* restore the real parent address: v0 -> ra */
+ move ra, v0
+
+ PTR_L v0, PT_R2(sp)
+ PTR_L v1, PT_R3(sp)
+ PTR_ADDIU sp, PT_SIZE
+
+ jr ra
+ nop
+#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
+
.set at
.set reorder
--
1.6.2.1
Hi,
On Mon, 2009-10-26 at 01:36 +0100, Frederic Weisbecker wrote:
> 2009/10/25 Wu Zhangjin <[email protected]>:
> > This patch fix the following error with FUNCTION_GRAPH_TRACER=y:
> >
> > kernel/built-in.o: In function `print_graph_irq':
> > trace_functions_graph.c:(.text+0x6dba0): undefined reference to `__irqentry_text_start'
> > trace_functions_graph.c:(.text+0x6dba8): undefined reference to `__irqentry_text_start'
> > trace_functions_graph.c:(.text+0x6dbd0): undefined reference to `__irqentry_text_end'
> > trace_functions_graph.c:(.text+0x6dbd4): undefined reference to `__irqentry_text_end'
> >
> > (This patch is need to support function graph tracer of MIPS)
>
>
> If you want to enjoy this section, you'd need to tag the
> mips irq entry functions with "__irq_entry" :)
>
> I guess there is a do_IRQ() in mips that is waiting for that (and
> probably some others).
> The effect is that interrupt areas are cut with a pair of arrows
> in the trace, so that you more easily spot interrupts in the traces
>
> May be I missed this part in another patch in this series though...
ooh, Sorry, only this patch added(I stopped after fixing the compiling
errors, no more check! so lazy a guy!).
Just checked the source code of MIPS, the do_IRQ() is defined as a
macro, so, I must move the macro to a C file, and also, there is a
irq_enter...irq_exit block in a "big" function, I need to split it out.
[...]
/*
* do_IRQ handles all normal device IRQ's (the special
* SMP cross-CPU interrupts have their own specific
* handlers).
*
* Ideally there should be away to get this into kernel/irq/handle.c to
* avoid the overhead of a call for just a tiny function ...
*/
#define do_IRQ(irq)
\
do {
\
irq_enter();
\
__DO_IRQ_SMTC_HOOK(irq);
\
generic_handle_irq(irq);
\
irq_exit();
\
} while (0)
[...]
But The comment told us: do not make this tiny function be a standalone
function, so???
the same to do_IRQ_no_affinity.
and, about the following function, I need to split the
irq_enter()...irq_exit() block out.
void ipi_decode(struct smtc_ipi *pipi)
{
[...]
switch (type_copy) {
case SMTC_CLOCK_TICK:
irq_enter();
kstat_incr_irqs_this_cpu(irq, irq_to_desc(irq));
cd = &per_cpu(mips_clockevent_device, cpu);
cd->event_handler(cd);
irq_exit();
break;
case LINUX_SMP_IPI:
switch ((int)arg_copy) {
case SMP_RESCHEDULE_YOURSELF:
ipi_resched_interrupt();
break;
case SMP_CALL_FUNCTION:
ipi_call_interrupt();
break;
[...]
Regards,
Wu Zhangjin
On Mon, 2009-10-26 at 01:27 +0100, Frederic Weisbecker wrote:
> 2009/10/25 Wu Zhangjin <[email protected]>:
> > -static inline u64 mips_timecounter_read(void)
> > +static inline u64 notrace mips_timecounter_read(void)
>
>
> You don't need to set notrace functions, unless their addresses
> are referenced somewhere, which unfortunately might happen
> for some functions but this is rare.
>
Okay, Will remove it.
>
> > {
> > #ifdef CONFIG_CSRC_R4K
> > return r4k_timecounter_read();
> > diff --git a/arch/mips/kernel/csrc-r4k.c b/arch/mips/kernel/csrc-r4k.c
> > index 4e7705f..0690bea 100644
> > --- a/arch/mips/kernel/csrc-r4k.c
> > +++ b/arch/mips/kernel/csrc-r4k.c
> > @@ -42,7 +42,7 @@ static struct timecounter r4k_tc = {
> > .cc = NULL,
> > };
> >
> > -static cycle_t r4k_cc_read(const struct cyclecounter *cc)
> > +static cycle_t notrace r4k_cc_read(const struct cyclecounter *cc)
> > {
> > return read_c0_count();
> > }
> > @@ -66,7 +66,7 @@ int __init init_r4k_timecounter(void)
> > return 0;
> > }
> >
> > -u64 r4k_timecounter_read(void)
> > +u64 notrace r4k_timecounter_read(void)
> > {
> > u64 clock;
> >
> > diff --git a/include/linux/clocksource.h b/include/linux/clocksource.h
> > index 83d2fbd..2a02992 100644
> > --- a/include/linux/clocksource.h
> > +++ b/include/linux/clocksource.h
> > @@ -73,7 +73,7 @@ struct timecounter {
> > * XXX - This could use some mult_lxl_ll() asm optimization. Same code
> > * as in cyc2ns, but with unsigned result.
> > */
> > -static inline u64 cyclecounter_cyc2ns(const struct cyclecounter *cc,
> > +static inline u64 notrace cyclecounter_cyc2ns(const struct cyclecounter
>
> ditto here.
>
Will remove it too.
>
> *cc,
> > cycle_t cycles)
> > {
> > u64 ret = (u64)cycles;
> > diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
> > index 5e18c6a..9ce9d02 100644
> > --- a/kernel/time/clocksource.c
> > +++ b/kernel/time/clocksource.c
> > @@ -52,7 +52,7 @@ EXPORT_SYMBOL(timecounter_init);
> > * The first call to this function for a new time counter initializes
> > * the time tracking and returns an undefined result.
> > */
> > -static u64 timecounter_read_delta(struct timecounter *tc)
> > +static u64 notrace timecounter_read_delta(struct timecounter *tc)
> > {
> > cycle_t cycle_now, cycle_delta;
> > u64 ns_offset;
> > @@ -72,7 +72,7 @@ static u64 timecounter_read_delta(struct timecounter
>
>
> Hmm yeah this is not very nice to do that in core functions because
> of a specific arch problem.
> At least you have __notrace_funcgraph, this is a notrace
> that only applies if CONFIG_FUNCTION_GRAPH_TRACER
> so that it's still traceable by the function tracer in this case.
>
> But I would rather see a __mips_notrace on these two core functions.
What about this: __arch_notrace? If the arch need this, define it,
otherwise, ignore it! if only graph tracer need it, define it in "#ifdef
CONFIG_FUNCTION_GRAPH_TRACER ... #endif".
diff --git a/arch/mips/include/asm/ftrace.h
b/arch/mips/include/asm/ftrace.h
index d5771e8..eeacd51 100644
--- a/arch/mips/include/asm/ftrace.h
+++ b/arch/mips/include/asm/ftrace.h
@@ -31,6 +31,11 @@ static inline unsigned long
ftrace_call_adjust(unsigned long addr)
struct dyn_arch_ftrace {
};
#endif /* CONFIG_DYNAMIC_FTRACE */
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+#define __arch_notrace
+#endif
+
#endif /* __ASSEMBLY__ */
#endif /* CONFIG_FUNCTION_TRACER */
#endif /* _ASM_MIPS_FTRACE_H */
[...]
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 0b4f97d..959c8b3 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -511,4 +511,12 @@ static inline void trace_hw_branch_oops(void) {}
#endif /* CONFIG_HW_BRANCH_TRACER */
+/* arch specific notrace */
+#ifndef __arch_notrace
+#define __arch_notrace
+#else
+#undef __arch_notrace
+#define __arch_notrace notrace
+#endif
+
#endif /* _LINUX_FTRACE_H */
[...]
diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
index 9ce9d02..91acdf7 100644
--- a/kernel/time/clocksource.c
+++ b/kernel/time/clocksource.c
@@ -30,6 +30,7 @@
#include <linux/sched.h> /* for spin_unlock_irq() using preempt_count()
m68k */
#include <linux/tick.h>
#include <linux/kthread.h>
+#include <linux/ftrace.h>
void timecounter_init(struct timecounter *tc,
const struct cyclecounter *cc,
@@ -52,7 +53,7 @@ EXPORT_SYMBOL(timecounter_init);
* The first call to this function for a new time counter initializes
* the time tracking and returns an undefined result.
*/
-static u64 notrace timecounter_read_delta(struct timecounter *tc)
+static u64 __arch_notrace timecounter_read_delta(struct timecounter
*tc)
{
cycle_t cycle_now, cycle_delta;
u64 ns_offset;
@@ -72,7 +73,7 @@ static u64 notrace timecounter_read_delta(struct
timecounter *tc)
return ns_offset;
}
-u64 notrace timecounter_read(struct timecounter *tc)
+u64 __arch_notrace timecounter_read(struct timecounter *tc)
{
u64 nsec;
Regards,
Wu Zhangjin
On Sun, 2009-10-25 at 23:16 +0800, Wu Zhangjin wrote:
> This patch implement a mips_timecounter_read(), which can be used to get
> high precision timestamp without extra lock.
>
> It is based on the clock counter register and the
> timecounter/cyclecounter abstraction layer of kernel.
>
> Signed-off-by: Wu Zhangjin <[email protected]>
> ---
> arch/mips/include/asm/time.h | 19 +++++++++++++++++++
> arch/mips/kernel/csrc-r4k.c | 41 +++++++++++++++++++++++++++++++++++++++++
> arch/mips/kernel/time.c | 2 ++
Some patches touch core tracing code, and some are arch specific. Now
the question is how do we go. I prefer that we go the path of the
tracing tree (easier for me to test). But every patch that touches MIPS
arch, needs an Acked-by from the MIPS maintainer. Which I see is Ralf
(on the Cc of this patch set.)
Thanks,
-- Steve
Hi,
On Mon, 2009-10-26 at 10:01 -0400, Steven Rostedt wrote:
[...]
> Some patches touch core tracing code, and some are arch specific. Now
> the question is how do we go. I prefer that we go the path of the
> tracing tree (easier for me to test).
Just coped with the feedbacks from Frederic Weisbecker.
I will rebase the whole patches to your git repo(the following one?) and
send them out as the -v6 revision:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git tip/tracing/core
> But every patch that touches MIPS
> arch, needs an Acked-by from the MIPS maintainer. Which I see is Ralf
> (on the Cc of this patch set.)
>
Looking forward to the feedback from Ralf, Seems he is a little busy.
and also looking forward to the testing result from the other MIPS
developers, so, we can ensure ftrace for MIPS really function!
Welcome to clone this branch and test it:
git://dev.lemote.com/rt4ls.git linux-mips/dev/ftrace-upstream
And this document will tell you how to play with it:
Documentation/trace/ftrace.txt
Thanks & Regards,
Wu Zhangjin
On Mon, 2009-10-26 at 22:25 +0800, Wu Zhangjin wrote:
> Hi,
>
> On Mon, 2009-10-26 at 10:01 -0400, Steven Rostedt wrote:
> [...]
> > Some patches touch core tracing code, and some are arch specific. Now
> > the question is how do we go. I prefer that we go the path of the
> > tracing tree (easier for me to test).
>
> Just coped with the feedbacks from Frederic Weisbecker.
>
> I will rebase the whole patches to your git repo(the following one?) and
> send them out as the -v6 revision:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git tip/tracing/core
Actually, I always base off of tip itself. Don't use mine. Use this one:
git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git tracing/core
Then we will stay in sync.
>
> > But every patch that touches MIPS
> > arch, needs an Acked-by from the MIPS maintainer. Which I see is Ralf
> > (on the Cc of this patch set.)
> >
>
> Looking forward to the feedback from Ralf, Seems he is a little busy.
> and also looking forward to the testing result from the other MIPS
> developers, so, we can ensure ftrace for MIPS really function!
>
> Welcome to clone this branch and test it:
>
> git://dev.lemote.com/rt4ls.git linux-mips/dev/ftrace-upstream
I already have your repo as a remote ;-)
>
> And this document will tell you how to play with it:
> Documentation/trace/ftrace.txt
Did you add to it?
Thanks,
-- Steve
Hi,
On Mon, 2009-10-26 at 10:34 -0400, Steven Rostedt wrote:
> On Mon, 2009-10-26 at 22:25 +0800, Wu Zhangjin wrote:
> > Hi,
> >
> > On Mon, 2009-10-26 at 10:01 -0400, Steven Rostedt wrote:
> > [...]
> > > Some patches touch core tracing code, and some are arch specific. Now
> > > the question is how do we go. I prefer that we go the path of the
> > > tracing tree (easier for me to test).
> >
> > Just coped with the feedbacks from Frederic Weisbecker.
> >
> > I will rebase the whole patches to your git repo(the following one?) and
> > send them out as the -v6 revision:
> >
> > git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git tip/tracing/core
>
> Actually, I always base off of tip itself. Don't use mine. Use this one:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git tracing/core
>
> Then we will stay in sync.
>
Okay, pulling it...
> >
> > > But every patch that touches MIPS
> > > arch, needs an Acked-by from the MIPS maintainer. Which I see is Ralf
> > > (on the Cc of this patch set.)
> > >
> >
> > Looking forward to the feedback from Ralf, Seems he is a little busy.
> > and also looking forward to the testing result from the other MIPS
> > developers, so, we can ensure ftrace for MIPS really function!
> >
> > Welcome to clone this branch and test it:
> >
> > git://dev.lemote.com/rt4ls.git linux-mips/dev/ftrace-upstream
>
> I already have your repo as a remote ;-)
>
Thanks :-)
> >
> > And this document will tell you how to play with it:
> > Documentation/trace/ftrace.txt
>
> Did you add to it?
I have never touched the file, just hope some newbies(like me) can
follow it and help to test it :-)
Thanks & Regards,
Wu Zhangjin
On Sun, 2009-10-25 at 23:17 +0800, Wu Zhangjin wrote:
> +
> +unsigned long ftrace_get_parent_addr(unsigned long self_addr,
> + unsigned long parent,
> + unsigned long parent_addr,
> + unsigned long fp)
> +{
> + unsigned long sp, ip, ra;
> + unsigned int code;
> +
> + /* move to the instruction "move ra, at" */
> + ip = self_addr - 8;
> +
> + /* search the text until finding the "move s8, sp" instruction or
> + * "s{d,w} ra, offset(sp)" instruction */
> + do {
> + ip -= 4;
> +
> + /* get the code at "ip" */
> + code = *(unsigned int *)ip;
Probably want to put the above in an asm with exception handling.
> +
> + /* If we hit the "move s8(fp), sp" instruction before finding
> + * where the ra is stored, then this is a leaf function and it
> + * does not store the ra on the stack. */
> + if ((code & MOV_FP_SP) == MOV_FP_SP)
> + return parent_addr;
> + } while (((code & S_RA) != S_RA));
Hmm, that condition also looks worrisome. Should we just always search
for s{d,w} R,X(sp)?
Since there should only be stores of registers into the sp above the
jump to mcount. The break out loop is a check for move. I think it would
be safer to have the break out loop is a check for non storing of a
register into SP.
> +
> + sp = fp + (code & STACK_OFFSET_MASK);
> + ra = *(unsigned long *)sp;
Also might want to make the above into a asm with exception handling.
> +
> + if (ra == parent)
> + return sp;
> +
> + ftrace_graph_stop();
> + WARN_ON(1);
> + return parent_addr;
Hmm, may need to do more than this. See below.
> +}
> +
> +/*
> + * Hook the return address and push it in the stack of return addrs
> + * in current thread info.
> + */
> +void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
> + unsigned long fp)
> +{
> + unsigned long old;
> + struct ftrace_graph_ent trace;
> + unsigned long return_hooker = (unsigned long)
> + &return_to_handler;
> +
> + if (unlikely(atomic_read(¤t->tracing_graph_pause)))
> + return;
> +
> + /* "parent" is the stack address saved the return address of the caller
> + * of _mcount, for a leaf function not save the return address in the
> + * stack address, so, we "emulate" one in _mcount's stack space, and
> + * hijack it directly, but for a non-leaf function, it will save the
> + * return address to the its stack space, so, we can not hijack the
> + * "parent" directly, but need to find the real stack address,
> + * ftrace_get_parent_addr() does it!
> + */
> +
> + old = *parent;
> +
> + parent = (unsigned long *)ftrace_get_parent_addr(self_addr, old,
> + (unsigned long)parent,
> + fp);
> +
> + *parent = return_hooker;
Although you may have turned off fgraph tracer in
ftrace_get_parent_addr, nothing stops the below from messing with the
stack. The return stack may get off sync and break later. If you fail
the above, you should not be calling the push function below.
-- Steve
> +
> + if (ftrace_push_return_trace(old, self_addr, &trace.depth, fp) ==
> + -EBUSY) {
> + *parent = old;
> + return;
> + }
> +
> + trace.func = self_addr;
> +
> + /* Only trace if the calling function expects to */
> + if (!ftrace_graph_entry(&trace)) {
> + current->curr_ret_stack--;
> + *parent = old;
> + }
> +}
> +#endif /* CONFIG_FUNCTION_GRAPH_TRACER */
Hi,
On Mon, 2009-10-26 at 11:13 -0400, Steven Rostedt wrote:
[...]
> > +
> > + /* get the code at "ip" */
> > + code = *(unsigned int *)ip;
>
> Probably want to put the above in an asm with exception handling.
>
Seems that exception handling in an asm is really "awful"(un-readable)
and the above ip is what we have got from the ftrace_graph_caller, it
should be okay. but if exception handling is necessary, I will send a
new patch for the places(including the following one) which need it.
> > +
> > + /* If we hit the "move s8(fp), sp" instruction before finding
> > + * where the ra is stored, then this is a leaf function and it
> > + * does not store the ra on the stack. */
> > + if ((code & MOV_FP_SP) == MOV_FP_SP)
> > + return parent_addr;
> > + } while (((code & S_RA) != S_RA));
>
> Hmm, that condition also looks worrisome. Should we just always search
> for s{d,w} R,X(sp)?
>
> Since there should only be stores of registers into the sp above the
> jump to mcount. The break out loop is a check for move. I think it would
> be safer to have the break out loop is a check for non storing of a
> register into SP.
Okay, let's look at this with -mlong-calls,
leaf function:
ffffffff80243cd8 <oops_may_print>:
ffffffff80243cd8: 67bdfff0 daddiu sp,sp,-16
ffffffff80243cdc: ffbe0008 sd s8,8(sp)
ffffffff80243ce0: 03a0f02d move s8,sp
ffffffff80243ce4: 3c038021 lui v1,0x8021
ffffffff80243ce8: 646316b0 daddiu v1,v1,5808
ffffffff80243cec: 03e0082d move at,ra
ffffffff80243cf0: 0060f809 jalr v1
ffffffff80243cf4: 00020021 nop
non-leaf function:
ffffffff802414c0 <copy_process>:
ffffffff802414c0: 67bdff40 daddiu sp,sp,-192
ffffffff802414c4: ffbe00b0 sd s8,176(sp)
ffffffff802414c8: 03a0f02d move s8,sp
ffffffff802414cc: ffbf00b8 sd ra,184(sp)
ffffffff802414d0: ffb700a8 sd s7,168(sp)
ffffffff802414d4: ffb600a0 sd s6,160(sp)
ffffffff802414d8: ffb50098 sd s5,152(sp)
ffffffff802414dc: ffb40090 sd s4,144(sp)
ffffffff802414e0: ffb30088 sd s3,136(sp)
ffffffff802414e4: ffb20080 sd s2,128(sp)
ffffffff802414e8: ffb10078 sd s1,120(sp)
ffffffff802414ec: ffb00070 sd s0,112(sp)
ffffffff802414f0: 3c038021 lui v1,0x8021
ffffffff802414f4: 646316b0 daddiu v1,v1,5808
ffffffff802414f8: 03e0082d move at,ra
ffffffff802414fc: 0060f809 jalr v1
ffffffff80241500: 00020021 nop
ip -->
At first, we move to "lui, v1, HI_16BIT_OF_MCOUNT", ip = ip - 12(not 8
when without -mlong-calls, i need to update the source code later).
and then, we check whether there is a "Store" instruction, if it's not a
"Store" instruction, the function should be a leaf? otherwise, we
continue the searching until finding the "s{d,w} ra, offset(sp)"
instruction, get the offset, calculate the stack address, and finish?
So, we just need to replace this:
if ((code & MOV_FP_SP) == MOV_FP_SP)
return parent_addr;
by
#define S_INSN (0xafb0 << 16)
if ((code & S_INSN) != S_INSN)
return parent_addr;
>
> > +
> > + sp = fp + (code & STACK_OFFSET_MASK);
> > + ra = *(unsigned long *)sp;
>
> Also might want to make the above into a asm with exception handling.
>
> > +
> > + if (ra == parent)
> > + return sp;
> > +
> > + ftrace_graph_stop();
> > + WARN_ON(1);
> > + return parent_addr;
>
> Hmm, may need to do more than this. See below.
>
> > +}
> > +
> > +/*
> > + * Hook the return address and push it in the stack of return addrs
> > + * in current thread info.
> > + */
> > +void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
> > + unsigned long fp)
> > +{
> > + unsigned long old;
> > + struct ftrace_graph_ent trace;
> > + unsigned long return_hooker = (unsigned long)
> > + &return_to_handler;
> > +
> > + if (unlikely(atomic_read(¤t->tracing_graph_pause)))
> > + return;
> > +
> > + /* "parent" is the stack address saved the return address of the caller
> > + * of _mcount, for a leaf function not save the return address in the
> > + * stack address, so, we "emulate" one in _mcount's stack space, and
> > + * hijack it directly, but for a non-leaf function, it will save the
> > + * return address to the its stack space, so, we can not hijack the
> > + * "parent" directly, but need to find the real stack address,
> > + * ftrace_get_parent_addr() does it!
> > + */
> > +
> > + old = *parent;
> > +
> > + parent = (unsigned long *)ftrace_get_parent_addr(self_addr, old,
> > + (unsigned long)parent,
> > + fp);
> > +
> > + *parent = return_hooker;
>
> Although you may have turned off fgraph tracer in
> ftrace_get_parent_addr, nothing stops the below from messing with the
> stack. The return stack may get off sync and break later. If you fail
> the above, you should not be calling the push function below.
>
We need to really stop before ftrace_push_return_trace to avoid messing
with the stack :-) but if we have stopped the tracer, is it important to
mess with the stack or not?
Regards,
Wu Zhangjin
On Tue, 2009-10-27 at 00:11 +0800, Wu Zhangjin wrote:
> Hi,
>
> On Mon, 2009-10-26 at 11:13 -0400, Steven Rostedt wrote:
> [...]
> > > +
> > > + /* get the code at "ip" */
> > > + code = *(unsigned int *)ip;
> >
> > Probably want to put the above in an asm with exception handling.
> >
>
> Seems that exception handling in an asm is really "awful"(un-readable)
> and the above ip is what we have got from the ftrace_graph_caller, it
> should be okay. but if exception handling is necessary, I will send a
> new patch for the places(including the following one) which need it.
Yeah, and probably not as important in the mips world, as it is used
more with embedded devices than desktops. We must always take the
"paranoid" approach for tracing. At least in PPC and x86, we assume
everything is broken ;-) And we want to be as robust as possible. If
something goes wrong, we want to detect it ASAP and report it. And keep
the system from crashing.
At least with MIPS we don't need to worry about crashing Linus's
desktop. With is the #1 priority we have on x86 ... "Don't crash Linus's
desktop!".
If Linus sees a warning, he'll bitch at us. If we crash his box, and he
was to lose any information, he'll strip out our code!
>
> > > +
> > > + /* If we hit the "move s8(fp), sp" instruction before finding
> > > + * where the ra is stored, then this is a leaf function and it
> > > + * does not store the ra on the stack. */
> > > + if ((code & MOV_FP_SP) == MOV_FP_SP)
> > > + return parent_addr;
> > > + } while (((code & S_RA) != S_RA));
> >
> > Hmm, that condition also looks worrisome. Should we just always search
> > for s{d,w} R,X(sp)?
> >
> > Since there should only be stores of registers into the sp above the
> > jump to mcount. The break out loop is a check for move. I think it would
> > be safer to have the break out loop is a check for non storing of a
> > register into SP.
>
>
> Okay, let's look at this with -mlong-calls,
>
> leaf function:
>
> ffffffff80243cd8 <oops_may_print>:
> ffffffff80243cd8: 67bdfff0 daddiu sp,sp,-16
> ffffffff80243cdc: ffbe0008 sd s8,8(sp)
> ffffffff80243ce0: 03a0f02d move s8,sp
> ffffffff80243ce4: 3c038021 lui v1,0x8021
> ffffffff80243ce8: 646316b0 daddiu v1,v1,5808
> ffffffff80243cec: 03e0082d move at,ra
> ffffffff80243cf0: 0060f809 jalr v1
> ffffffff80243cf4: 00020021 nop
>
> non-leaf function:
>
> ffffffff802414c0 <copy_process>:
> ffffffff802414c0: 67bdff40 daddiu sp,sp,-192
> ffffffff802414c4: ffbe00b0 sd s8,176(sp)
> ffffffff802414c8: 03a0f02d move s8,sp
> ffffffff802414cc: ffbf00b8 sd ra,184(sp)
> ffffffff802414d0: ffb700a8 sd s7,168(sp)
> ffffffff802414d4: ffb600a0 sd s6,160(sp)
> ffffffff802414d8: ffb50098 sd s5,152(sp)
> ffffffff802414dc: ffb40090 sd s4,144(sp)
> ffffffff802414e0: ffb30088 sd s3,136(sp)
> ffffffff802414e4: ffb20080 sd s2,128(sp)
> ffffffff802414e8: ffb10078 sd s1,120(sp)
> ffffffff802414ec: ffb00070 sd s0,112(sp)
> ffffffff802414f0: 3c038021 lui v1,0x8021
> ffffffff802414f4: 646316b0 daddiu v1,v1,5808
> ffffffff802414f8: 03e0082d move at,ra
> ffffffff802414fc: 0060f809 jalr v1
> ffffffff80241500: 00020021 nop
> ip -->
>
> At first, we move to "lui, v1, HI_16BIT_OF_MCOUNT", ip = ip - 12(not 8
> when without -mlong-calls, i need to update the source code later).
Yes with -mlong-calls you must jump pass the setting up of the jalr.
>
> and then, we check whether there is a "Store" instruction, if it's not a
> "Store" instruction, the function should be a leaf? otherwise, we
> continue the searching until finding the "s{d,w} ra, offset(sp)"
> instruction, get the offset, calculate the stack address, and finish?
Note, you are commenting different than your code. Your code matches
more what I want than your comments ;-) You keep saying "if the
instruction just after the jump to mcount (and preparation for) is not a
store than it is a leaf", where I'm saying (and the code matches),
search above the jump to mcount and if we don't find the store of ra,
then it is a leaf.
>
> So, we just need to replace this:
>
> if ((code & MOV_FP_SP) == MOV_FP_SP)
> return parent_addr;
>
> by
>
> #define S_INSN (0xafb0 << 16)
>
> if ((code & S_INSN) != S_INSN)
> return parent_addr;
I would be even more paranoid, and make sure each of those stores, store
into sp.
>
> >
> > > +
> > > + sp = fp + (code & STACK_OFFSET_MASK);
> > > + ra = *(unsigned long *)sp;
> >
> > Also might want to make the above into a asm with exception handling.
> >
> > > +
> > > + if (ra == parent)
> > > + return sp;
> > > +
> > > + ftrace_graph_stop();
> > > + WARN_ON(1);
> > > + return parent_addr;
> >
> > Hmm, may need to do more than this. See below.
> >
> > > +}
> > > +
> > > +/*
> > > + * Hook the return address and push it in the stack of return addrs
> > > + * in current thread info.
> > > + */
> > > +void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
> > > + unsigned long fp)
> > > +{
> > > + unsigned long old;
> > > + struct ftrace_graph_ent trace;
> > > + unsigned long return_hooker = (unsigned long)
> > > + &return_to_handler;
> > > +
> > > + if (unlikely(atomic_read(¤t->tracing_graph_pause)))
> > > + return;
> > > +
> > > + /* "parent" is the stack address saved the return address of the caller
> > > + * of _mcount, for a leaf function not save the return address in the
> > > + * stack address, so, we "emulate" one in _mcount's stack space, and
> > > + * hijack it directly, but for a non-leaf function, it will save the
> > > + * return address to the its stack space, so, we can not hijack the
> > > + * "parent" directly, but need to find the real stack address,
> > > + * ftrace_get_parent_addr() does it!
> > > + */
> > > +
> > > + old = *parent;
> > > +
> > > + parent = (unsigned long *)ftrace_get_parent_addr(self_addr, old,
> > > + (unsigned long)parent,
> > > + fp);
> > > +
> > > + *parent = return_hooker;
> >
> > Although you may have turned off fgraph tracer in
> > ftrace_get_parent_addr, nothing stops the below from messing with the
> > stack. The return stack may get off sync and break later. If you fail
> > the above, you should not be calling the push function below.
> >
>
> We need to really stop before ftrace_push_return_trace to avoid messing
> with the stack :-) but if we have stopped the tracer, is it important to
> mess with the stack or not?
The ftrace_push_return_trace does not test if the trace stopped, that is
expected to be done by the caller. If you mess with the stack set up,
you will crash the box. Remember, before the failure, you could have
already replaced return jumps. Those will still be falling back to the
return_to_handler. If you mess with the stack, but don't update the
return, the other returns will be out of sync and call the wrong return
address.
-- Steve
Hi,
[...]
>
> Yeah, and probably not as important in the mips world, as it is used
> more with embedded devices than desktops. We must always take the
> "paranoid" approach for tracing. At least in PPC and x86, we assume
> everything is broken ;-) And we want to be as robust as possible. If
> something goes wrong, we want to detect it ASAP and report it. And keep
> the system from crashing.
>
> At least with MIPS we don't need to worry about crashing Linus's
> desktop. With is the #1 priority we have on x86 ... "Don't crash Linus's
> desktop!".
>
> If Linus sees a warning, he'll bitch at us. If we crash his box, and he
> was to lose any information, he'll strip out our code!
>
Okay, a new patch for all of the exception handling will go into -v7.
>
> >
> > So, we just need to replace this:
> >
> > if ((code & MOV_FP_SP) == MOV_FP_SP)
> > return parent_addr;
> >
> > by
> >
> > #define S_INSN (0xafb0 << 16)
> >
> > if ((code & S_INSN) != S_INSN)
> > return parent_addr;
>
> I would be even more paranoid, and make sure each of those stores, store
> into sp.
get it :-)
(I need to be more paranoid too, otherwise, Steven will not accept my
patches!)
>
> >
> > >
> > > > +
> > > > + sp = fp + (code & STACK_OFFSET_MASK);
> > > > + ra = *(unsigned long *)sp;
> > >
> > > Also might want to make the above into a asm with exception handling.
> > >
> > > > +
> > > > + if (ra == parent)
> > > > + return sp;
> > > > +
> > > > + ftrace_graph_stop();
> > > > + WARN_ON(1);
> > > > + return parent_addr;
> > >
> > > Hmm, may need to do more than this. See below.
> > >
[...]
> > > > +
> > > > + old = *parent;
> > > > +
> > > > + parent = (unsigned long *)ftrace_get_parent_addr(self_addr, old,
> > > > + (unsigned long)parent,
> > > > + fp);
> > > > +
> > > > + *parent = return_hooker;
> > >
> > > Although you may have turned off fgraph tracer in
> > > ftrace_get_parent_addr, nothing stops the below from messing with the
> > > stack. The return stack may get off sync and break later. If you fail
> > > the above, you should not be calling the push function below.
> > >
> >
> > We need to really stop before ftrace_push_return_trace to avoid messing
> > with the stack :-) but if we have stopped the tracer, is it important to
> > mess with the stack or not?
>
> The ftrace_push_return_trace does not test if the trace stopped, that is
> expected to be done by the caller. If you mess with the stack set up,
> you will crash the box. Remember, before the failure, you could have
> already replaced return jumps. Those will still be falling back to the
> return_to_handler. If you mess with the stack, but don't update the
> return, the other returns will be out of sync and call the wrong return
> address.
>
As you can see, after stopping the function graph tracer(here the function is non-leaf)
with ftrace_graph_stop() in ftrace_get_parent_addr(), I return the old parent_addr,
this is only the stack address in the stack space of ftrace_graph_caller, which means
that, I never touch the real stack address of the non-leaf function, and it will not trap
into the return_to_handler hooker 'Cause the non-leaf function will load it's own normal
return address from it's own stack, and then just return back normally.
-- This is another trick :-)
Regards,
Wu Zhangjin
On Tue, 2009-10-27 at 00:57 +0800, Wu Zhangjin wrote:
> > I would be even more paranoid, and make sure each of those stores, store
> > into sp.
>
> get it :-)
>
> (I need to be more paranoid too, otherwise, Steven will not accept my
> patches!)
Sure I would accept them. I don't know of any MIPS boxes that Linus
runs. So I'm not afraid of crashing his boxes with these patches ;-)
> > >
> > > We need to really stop before ftrace_push_return_trace to avoid messing
> > > with the stack :-) but if we have stopped the tracer, is it important to
> > > mess with the stack or not?
> >
> > The ftrace_push_return_trace does not test if the trace stopped, that is
> > expected to be done by the caller. If you mess with the stack set up,
> > you will crash the box. Remember, before the failure, you could have
> > already replaced return jumps. Those will still be falling back to the
> > return_to_handler. If you mess with the stack, but don't update the
> > return, the other returns will be out of sync and call the wrong return
> > address.
> >
>
> As you can see, after stopping the function graph tracer(here the function is non-leaf)
> with ftrace_graph_stop() in ftrace_get_parent_addr(), I return the old parent_addr,
> this is only the stack address in the stack space of ftrace_graph_caller, which means
> that, I never touch the real stack address of the non-leaf function, and it will not trap
> into the return_to_handler hooker 'Cause the non-leaf function will load it's own normal
> return address from it's own stack, and then just return back normally.
But then you should not be calling the push function. That will still
push onto the graph stack.
The function graph tracer keeps a separate return stack
(current->ret_stack). This is what holds the return addresses.
(normal operation)
func1
jalr _mcount
push ra onto ret_stack
replace ra with return_to_handler
jr ra --> return_to_handler
return_to_handler
pop ret_stack, have original ra
jr original_ra
Now what happens if we fail a call but still push to ret_stack
func1
jalr _mcount
(success)
push ra onto ret_stack
replace ra with return_to_handler
jalr func2
func2
jalr _mcount
(failed)
push ra onto ret_stack <<-- this is wrong!
keep original ra
jr ra << does not call tracer function!!!
jr ra << goes to return_to_handler
return_to_handler
pop ra from ret_stack <<--- has func2 ra not func1 ra!!
jr func1_ra
**** CRASH ****
Make sense?
-- Steve
2009/10/26 Wu Zhangjin <[email protected]>:
> ooh, Sorry, only this patch added(I stopped after fixing the compiling
> errors, no more check! so lazy a guy!).
>
> Just checked the source code of MIPS, the do_IRQ() is defined as a
> macro, so, I must move the macro to a C file, and also, there is a
> irq_enter...irq_exit block in a "big" function, I need to split it out.
>
> [...]
> /*
> ?* do_IRQ handles all normal device IRQ's (the special
> ?* SMP cross-CPU interrupts have their own specific
> ?* handlers).
> ?*
> ?* Ideally there should be away to get this into kernel/irq/handle.c to
> ?* avoid the overhead of a call for just a tiny function ...
> ?*/
> #define do_IRQ(irq)
> \
> do {
> \
> ? ? ? ?irq_enter();
> \
> ? ? ? ?__DO_IRQ_SMTC_HOOK(irq);
> \
> ? ? ? ?generic_handle_irq(irq);
> \
> ? ? ? ?irq_exit();
> \
> } while (0)
> [...]
>
> But The comment told us: do not make this tiny function be a standalone
> function, so???
I can't check that currently. But may be the caller of this m
>
> the same to do_IRQ_no_affinity.
>
> and, about the following function, I need to split the
> irq_enter()...irq_exit() block out.
>
> void ipi_decode(struct smtc_ipi *pipi)
> {
> ? ? ? ?[...]
> ? ? ? ?switch (type_copy) {
> ? ? ? ?case SMTC_CLOCK_TICK:
> ? ? ? ? ? ? ? ?irq_enter();
> ? ? ? ? ? ? ? ?kstat_incr_irqs_this_cpu(irq, irq_to_desc(irq));
> ? ? ? ? ? ? ? ?cd = &per_cpu(mips_clockevent_device, cpu);
> ? ? ? ? ? ? ? ?cd->event_handler(cd);
> ? ? ? ? ? ? ? ?irq_exit();
> ? ? ? ? ? ? ? ?break;
>
> ? ? ? ?case LINUX_SMP_IPI:
> ? ? ? ? ? ? ? ?switch ((int)arg_copy) {
> ? ? ? ? ? ? ? ?case SMP_RESCHEDULE_YOURSELF:
> ? ? ? ? ? ? ? ? ? ? ? ?ipi_resched_interrupt();
> ? ? ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ? ? ? ? ?case SMP_CALL_FUNCTION:
> ? ? ? ? ? ? ? ? ? ? ? ?ipi_call_interrupt();
> ? ? ? ? ? ? ? ? ? ? ? ?break;
> ? ? ? ?[...]
>
> Regards,
> ? ? ? ?Wu Zhangjin
>
>
2009/10/27 Frederic Weisbecker <[email protected]>:
> 2009/10/26 Wu Zhangjin <[email protected]>:
>> ooh, Sorry, only this patch added(I stopped after fixing the compiling
>> errors, no more check! so lazy a guy!).
>>
>> Just checked the source code of MIPS, the do_IRQ() is defined as a
>> macro, so, I must move the macro to a C file, and also, there is a
>> irq_enter...irq_exit block in a "big" function, I need to split it out.
>>
>> [...]
>> /*
>> ?* do_IRQ handles all normal device IRQ's (the special
>> ?* SMP cross-CPU interrupts have their own specific
>> ?* handlers).
>> ?*
>> ?* Ideally there should be away to get this into kernel/irq/handle.c to
>> ?* avoid the overhead of a call for just a tiny function ...
>> ?*/
>> #define do_IRQ(irq)
>> \
>> do {
>> \
>> ? ? ? ?irq_enter();
>> \
>> ? ? ? ?__DO_IRQ_SMTC_HOOK(irq);
>> \
>> ? ? ? ?generic_handle_irq(irq);
>> \
>> ? ? ? ?irq_exit();
>> \
>> } while (0)
>> [...]
>>
>> But The comment told us: do not make this tiny function be a standalone
>> function, so???
>
>
> I can't check that currently. But may be the caller of this m
Sorry, my message has been sent in the middle. I'm dealing with a
strange keyboard where
I am (and also with my strange hands).
So, may be the caller of this macro can take the irqentry tag?
>
>
>>
>> the same to do_IRQ_no_affinity.
>>
>> and, about the following function, I need to split the
>> irq_enter()...irq_exit() block out.
>>
>> void ipi_decode(struct smtc_ipi *pipi)
>> {
>> ? ? ? ?[...]
>> ? ? ? ?switch (type_copy) {
>> ? ? ? ?case SMTC_CLOCK_TICK:
>> ? ? ? ? ? ? ? ?irq_enter();
>> ? ? ? ? ? ? ? ?kstat_incr_irqs_this_cpu(irq, irq_to_desc(irq));
>> ? ? ? ? ? ? ? ?cd = &per_cpu(mips_clockevent_device, cpu);
>> ? ? ? ? ? ? ? ?cd->event_handler(cd);
>> ? ? ? ? ? ? ? ?irq_exit();
>> ? ? ? ? ? ? ? ?break;
>>
>> ? ? ? ?case LINUX_SMP_IPI:
>> ? ? ? ? ? ? ? ?switch ((int)arg_copy) {
>> ? ? ? ? ? ? ? ?case SMP_RESCHEDULE_YOURSELF:
>> ? ? ? ? ? ? ? ? ? ? ? ?ipi_resched_interrupt();
>> ? ? ? ? ? ? ? ? ? ? ? ?break;
>> ? ? ? ? ? ? ? ?case SMP_CALL_FUNCTION:
>> ? ? ? ? ? ? ? ? ? ? ? ?ipi_call_interrupt();
>> ? ? ? ? ? ? ? ? ? ? ? ?break;
>> ? ? ? ?[...]
>>
Oh right, this one is more tricky. Well, if that's important for
someone, we can deal with that later.
On Mon, Oct 26, 2009 at 05:42:36PM +0800, Wu Zhangjin wrote:
> On Mon, 2009-10-26 at 01:27 +0100, Frederic Weisbecker wrote:
> > 2009/10/25 Wu Zhangjin <[email protected]>:
> > > -static inline u64 mips_timecounter_read(void)
> > > +static inline u64 notrace mips_timecounter_read(void)
> >
> >
> > You don't need to set notrace functions, unless their addresses
> > are referenced somewhere, which unfortunately might happen
> > for some functions but this is rare.
> >
>
> Okay, Will remove it.
Oops, a word has escaped from my above sentence. I wanted to say:
"You don't need to set notrace to inline functions" :)
> > Hmm yeah this is not very nice to do that in core functions because
> > of a specific arch problem.
> > At least you have __notrace_funcgraph, this is a notrace
> > that only applies if CONFIG_FUNCTION_GRAPH_TRACER
> > so that it's still traceable by the function tracer in this case.
> >
> > But I would rather see a __mips_notrace on these two core functions.
>
> What about this: __arch_notrace? If the arch need this, define it,
> otherwise, ignore it! if only graph tracer need it, define it in "#ifdef
> CONFIG_FUNCTION_GRAPH_TRACER ... #endif".
The problem is that archs may want to disable tracing on different
places.
For example mips wants to disable tracing in timecounter_read_delta,
but another arch may want to disable tracing somewhere else.
We'll then have several unrelated __arch_notrace. One that is relevant
for mips, another that is relevant for arch_foo, but all of them will
apply for all arch that have defined a __arch_notrace.
It's true that __mips_notrace is not very elegant as it looks like
a specific arch annotation intruder.
But at least that gives us a per arch filter granularity.
If only static ftrace could disappear, we could keep only dynamic
ftrace and we would then be able to filter dynamically.
But I'm not sure it's a good idea for archs integration.
Hi,
On Mon, 2009-11-02 at 22:43 +0100, Frederic Weisbecker wrote:
[...]
> > > > -static inline u64 mips_timecounter_read(void)
> > > > +static inline u64 notrace mips_timecounter_read(void)
> > >
> > >
> > > You don't need to set notrace functions, unless their addresses
> > > are referenced somewhere, which unfortunately might happen
> > > for some functions but this is rare.
> > >
> >
> > Okay, Will remove it.
>
>
>
> Oops, a word has escaped from my above sentence. I wanted to say:
>
> "You don't need to set notrace to inline functions" :)
>
>
Thanks ;)
I have got your meaning at that time, and have removed them with inline
functions.
> > > But I would rather see a __mips_notrace on these two core functions.
> >
> > What about this: __arch_notrace? If the arch need this, define it,
> > otherwise, ignore it! if only graph tracer need it, define it in "#ifdef
> > CONFIG_FUNCTION_GRAPH_TRACER ... #endif".
>
> The problem is that archs may want to disable tracing on different
> places.
> For example mips wants to disable tracing in timecounter_read_delta,
> but another arch may want to disable tracing somewhere else.
>
> We'll then have several unrelated __arch_notrace. One that is relevant
> for mips, another that is relevant for arch_foo, but all of them will
> apply for all arch that have defined a __arch_notrace.
>
> It's true that __mips_notrace is not very elegant as it looks like
> a specific arch annotation intruder.
>
> But at least that gives us a per arch filter granularity.
>
> If only static ftrace could disappear, we could keep only dynamic
> ftrace and we would then be able to filter dynamically.
> But I'm not sure it's a good idea for archs integration.
>
Got it.
Thanks & Regards,
Wu Zhangjin
Hi,
On Mon, 2009-11-02 at 22:43 +0100, Frederic Weisbecker wrote:
[...]
> > >
> > > But I would rather see a __mips_notrace on these two core functions.
> >
> > What about this: __arch_notrace? If the arch need this, define it,
> > otherwise, ignore it! if only graph tracer need it, define it in "#ifdef
> > CONFIG_FUNCTION_GRAPH_TRACER ... #endif".
>
>
>
> The problem is that archs may want to disable tracing on different
> places.
> For example mips wants to disable tracing in timecounter_read_delta,
> but another arch may want to disable tracing somewhere else.
>
> We'll then have several unrelated __arch_notrace. One that is relevant
> for mips, another that is relevant for arch_foo, but all of them will
> apply for all arch that have defined a __arch_notrace.
>
> It's true that __mips_notrace is not very elegant as it looks like
> a specific arch annotation intruder.
>
>
> But at least that gives us a per arch filter granularity.
>
> If only static ftrace could disappear, we could keep only dynamic
> ftrace and we would then be able to filter dynamically.
> But I'm not sure it's a good idea for archs integration.
>
I think if we use something like __mips_notrace here, we may get lots of
__ARCH_notraces here too, 'Cause some other platforms(at least, as I
know, Microblaze will do it too) may also need to add one here, it will
become:
__mips_notrace __ARCH1_notrace __ARCH2_notrace .... foo() {...}
A little ugly ;)
and If a new platform need it's __ARCH_notrace, they need to touch the
common part of ftrace, more side-effects!
but with __arch_notrace, the archs only need to touch it's own part,
Although there is a side-effect as you mentioned above ;)
So, what should we do?
Regards,
Wu Zhangjin
On Mon, Nov 09, 2009 at 12:31:12PM +0800, Wu Zhangjin wrote:
> I think if we use something like __mips_notrace here, we may get lots of
> __ARCH_notraces here too, 'Cause some other platforms(at least, as I
> know, Microblaze will do it too) may also need to add one here, it will
> become:
>
> __mips_notrace __ARCH1_notrace __ARCH2_notrace .... foo() {...}
>
> A little ugly ;)
Yeah :)
I thought Mips would be the only one to do that.
> and If a new platform need it's __ARCH_notrace, they need to touch the
> common part of ftrace, more side-effects!
>
> but with __arch_notrace, the archs only need to touch it's own part,
> Although there is a side-effect as you mentioned above ;)
>
> So, what should we do?
>
> Regards,
> Wu Zhangjin
>
Why not __time ?
As it's normal that such few functions that are used to read the timecounter
have fair chances to be __no_trace on archs like MIPS. Interested
archs would just need to override a default stub __time.
On Mon, 2009-11-09 at 12:53 +0100, Frederic Weisbecker wrote:
> On Mon, Nov 09, 2009 at 12:31:12PM +0800, Wu Zhangjin wrote:
> > I think if we use something like __mips_notrace here, we may get lots of
> > __ARCH_notraces here too, 'Cause some other platforms(at least, as I
> > know, Microblaze will do it too) may also need to add one here, it will
> > become:
> >
> > __mips_notrace __ARCH1_notrace __ARCH2_notrace .... foo() {...}
> >
> > A little ugly ;)
>
>
> Yeah :)
> I thought Mips would be the only one to do that.
>
>
> > and If a new platform need it's __ARCH_notrace, they need to touch the
> > common part of ftrace, more side-effects!
> >
> > but with __arch_notrace, the archs only need to touch it's own part,
> > Although there is a side-effect as you mentioned above ;)
> >
> > So, what should we do?
> >
> > Regards,
> > Wu Zhangjin
> >
>
> Why not __time ?
> As it's normal that such few functions that are used to read the timecounter
> have fair chances to be __no_trace on archs like MIPS. Interested
> archs would just need to override a default stub __time.
>
Good pointer, will apply it ;)
Regards,
Wu Zhangjin
On Mon, 2009-11-09 at 12:31 +0800, Wu Zhangjin wrote:
> I think if we use something like __mips_notrace here, we may get lots of
> __ARCH_notraces here too, 'Cause some other platforms(at least, as I
> know, Microblaze will do it too) may also need to add one here, it will
> become:
>
> __mips_notrace __ARCH1_notrace __ARCH2_notrace .... foo() {...}
>
> A little ugly ;)
I agree, that is ugly.
>
> and If a new platform need it's __ARCH_notrace, they need to touch the
> common part of ftrace, more side-effects!
>
> but with __arch_notrace, the archs only need to touch it's own part,
> Although there is a side-effect as you mentioned above ;)
>
> So, what should we do?
Just do it in the Makefile. We can add __arch_notrace, and then in the
Makefile define it with the arch.
ifeq ($(ARCH), MIPS)
CFLAGS_foo.o = -D__arch_notrace=notrace
endif
And we can simply define __arch_notrace in a header:
#ifndef __arch_notrace
# define __arch_notrace
#endif
I much rather uglify the Makefile than the code.
-- Steve
Hi,
On Mon, 2009-11-09 at 07:54 -0500, Steven Rostedt wrote:
[...]
>
> Just do it in the Makefile. We can add __arch_notrace, and then in the
> Makefile define it with the arch.
>
> ifeq ($(ARCH), MIPS)
> CFLAGS_foo.o = -D__arch_notrace=notrace
> endif
>
> And we can simply define __arch_notrace in a header:
>
> #ifndef __arch_notrace
> # define __arch_notrace
> #endif
>
> I much rather uglify the Makefile than the code.
>
Seems can not totally avoid the problem mentioned by Frederic, that is
if there are two many functions in the file, and different platforms
care about different functions ;)
what about Frederic's __time, just replace that __arch_notrace by
__time_notrace, and only consider the time relative functions currently?
Seems this will really make the stuff simpler.
Regards,
Wu Zhangjin