Re: [RFC 2/2] dyn ftrace porting of IA64

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2008-12-18 at 11:17 +0800, Shaohua Li wrote:
> This is the IA64 port of dyn ftrace. In IA64, there are some issues we
> must solve.
> 1. kernel compile has different option against module compile, so I
> extend the Makefile.build/recordmcount.pl script to distinct the two
> cases.
> 2. in a module, each routine's mcount call will call a PLT stub, which
> will call kernel mcount. We can't simply make the mcount call call into
> kernel mcount, as kernel and mocule have different gp and the
> instruction just supports 25bit offset. So I introduced a new PLT stub,
> which will call into kernel ftrace_caller. When module loading, all
> mcount call will be converted to nop. When the nop is converted to call,
> we make the call to the new PLT stub instead of old mcount PLT stub.

Have you taken a look at how powerpc64 does it?

You can look at Ingo's tip tree under the branch tip/tracing/powerpc, or
my own tree under tip/ftrace/ppc.

My tree is at:

git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git




> 
> Signed-off-by: Shaohua Li <shaohua.li@xxxxxxxxx>
> ---
>  arch/ia64/Kconfig              |    2 
>  arch/ia64/include/asm/ftrace.h |   12 ++
>  arch/ia64/include/asm/module.h |    4 
>  arch/ia64/kernel/Makefile      |    5 +
>  arch/ia64/kernel/entry.S       |   37 +++++++++
>  arch/ia64/kernel/ftrace.c      |  165 +++++++++++++++++++++++++++++++++++++++++
>  arch/ia64/kernel/module.c      |   15 +++
>  kernel/trace/ftrace.c          |   12 --
>  scripts/Makefile.build         |    8 +
>  scripts/recordmcount.pl        |   20 ++++
>  10 files changed, 267 insertions(+), 13 deletions(-)
> 
> Index: linux/arch/ia64/Kconfig
> ===================================================================
> --- linux.orig/arch/ia64/Kconfig	2008-12-17 17:14:49.000000000 +0800
> +++ linux/arch/ia64/Kconfig	2008-12-17 17:15:10.000000000 +0800
> @@ -21,6 +21,8 @@ config IA64
>  	select HAVE_OPROFILE
>  	select HAVE_KPROBES
>  	select HAVE_KRETPROBES
> +	select HAVE_FTRACE_MCOUNT_RECORD
> +	select HAVE_DYNAMIC_FTRACE
>  	select HAVE_FUNCTION_TRACER
>  	select HAVE_DMA_ATTRS
>  	select HAVE_KVM
> Index: linux/arch/ia64/kernel/Makefile
> ===================================================================
> --- linux.orig/arch/ia64/kernel/Makefile	2008-12-17 17:14:49.000000000 +0800
> +++ linux/arch/ia64/kernel/Makefile	2008-12-17 17:15:10.000000000 +0800
> @@ -2,6 +2,10 @@
>  # Makefile for the linux kernel.
>  #
>  
> +ifdef CONFIG_DYNAMIC_FTRACE
> +CFLAGS_REMOVE_ftrace.o = -pg
> +endif
> +
>  extra-y	:= head.o init_task.o vmlinux.lds
>  
>  obj-y := acpi.o entry.o efi.o efi_stub.o gate-data.o fsys.o ia64_ksyms.o irq.o irq_ia64.o	\
> @@ -28,6 +32,7 @@ obj-$(CONFIG_IA64_CYCLONE)	+= cyclone.o
>  obj-$(CONFIG_CPU_FREQ)		+= cpufreq/
>  obj-$(CONFIG_IA64_MCA_RECOVERY)	+= mca_recovery.o
>  obj-$(CONFIG_KPROBES)		+= kprobes.o jprobes.o
> +obj-$(CONFIG_DYNAMIC_FTRACE)	+= ftrace.o
>  obj-$(CONFIG_KEXEC)		+= machine_kexec.o relocate_kernel.o crash.o
>  obj-$(CONFIG_CRASH_DUMP)	+= crash_dump.o
>  obj-$(CONFIG_IA64_UNCACHED_ALLOCATOR)	+= uncached.o
> Index: linux/arch/ia64/kernel/ftrace.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ linux/arch/ia64/kernel/ftrace.c	2008-12-18 10:46:50.000000000 +0800
> @@ -0,0 +1,165 @@
> +/*
> + * Dynamic function tracing support.
> + *
> + * Copyright (C) 2008 Shaohua Li <shaohua.li@xxxxxxxxx>
> + *
> + * For licencing details, see COPYING.
> + *
> + * Defines low-level handling of mcount calls when the kernel
> + * is compiled with the -pg flag. When using dynamic ftrace, the
> + * mcount call-sites get patched lazily with NOP till they are
> + * enabled. All code mutation routines here take effect atomically.
> + */
> +
> +#include <linux/uaccess.h>
> +#include <linux/ftrace.h>
> +
> +#include <asm/cacheflush.h>
> +#include <asm/ftrace.h>
> +
> +static unsigned char ftrace_nop_code[MCOUNT_INSN_SIZE] = {
> +	0x00, 0x00, 0x00, 0x00, 0x01, 0x00,	/* nop.m 0x0 */
> +	0x00, 0x00, 0x00, 0x02, 0x00, 0x00,	/* nop.i 0x0 */
> +	0x00, 0x00, 0x04, 0x00,			/* nop.i 0x0 */
> +	0x01, 0x00, 0x00, 0x00, 0x01, 0x00,	/* nop.m 0x0 */
> +	0x00, 0x00, 0x00, 0x02, 0x00, 0x00,	/* nop.i 0x0 */
> +	0x00, 0x00, 0x04, 0x00			/* nop.i 0x0;; */
> +};

Is that an atomic operation?  That is, will it all get executed at once?
If not, the task can be preempted in the middle of the nop and if the
change happens then, when that task starts again, it may crash.

Another option to nop, is a jump over the rest of the code.

> +
> +unsigned char *ftrace_nop_replace(void)
> +{
> +	return ftrace_nop_code;
> +}
> +
> +/* In IA64, each function will be added below two bundles with -pg option */
> +static unsigned char __attribute__((aligned(8)))
> +ftrace_call_code[MCOUNT_INSN_SIZE] = {
> +	0x02, 0x40, 0x31, 0x10, 0x80, 0x05, /* alloc r40=ar.pfs,12,8,0 */
> +	0xb0, 0x02, 0x00, 0x00, 0x42, 0x40, /* mov r43=r0;; */
> +	0x05, 0x00, 0xc4, 0x00,             /* mov r42=b0 */
> +	0x11, 0x48, 0x01, 0x02, 0x00, 0x21, /* mov r41=r1 */
> +	0x00, 0x00, 0x00, 0x02, 0x00, 0x00, /* nop.i 0x0 */
> +	0x08, 0x00, 0x00, 0x50              /* br.call.sptk.many b0 = _mcount;; */
> +};

ia64 adds that much code with -pg??  This may be a problem with respect
to preemption. I think the way we solved this with powerpc, was just to
change the first line into a jump.

You can also look at the powerpc patches I did here:

  http://lkml.org/lkml/2008/11/20/356

and

  http://lkml.org/lkml/2008/11/26/434
  

> +
> +struct ftrace_call_insn {
> +	u64 dummy1, dummy2, dummy3;
> +	u64 dummy4:64-41+13;
> +	u64 imm20:20;
> +	u64 dummy5:3;
> +	u64 sign:1;
> +	u64 dummy6:4;
> +};
> +
> +static void calculate_offset(unsigned long ip, unsigned long addr,
> +	unsigned long *offset, int *sign)
> +{
> +	long of = addr - (ip + 0x10);
> +	if (of < 0)
> +		*sign = 1;
> +	else
> +		*sign = 0;
> +	*offset = of >> 4;
> +}
> +
> +unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr)
> +{
> +	struct ftrace_call_insn *code = (void *)ftrace_call_code;
> +	unsigned long offset;
> +	int sign;
> +
> +	calculate_offset(ip, addr, &offset, &sign);
> +	code->sign = sign;
> +	code->imm20 = offset;
> +
> +	return ftrace_call_code;
> +}
> +
> +#define CODE_IS_INVALID 0
> +#define CODE_IS_NOP 1
> +#define CODE_IS_CALL 2
> +int ftrace_modify_code(unsigned long ip, unsigned char *old_code,
> +		       unsigned char *new_code)
> +{
> +	unsigned char __attribute__((aligned(8))) replaced[MCOUNT_INSN_SIZE];
> +	struct ftrace_call_insn *call_insn = (void *)ftrace_call_code, *tmp_call;
> +	int old_code_type = CODE_IS_INVALID;
> +	struct module *mod = module_text_address(ip);
> +	int is_kernel_address = core_kernel_text(ip);
> +
> +	/*
> +	 * Note: Due to modules and __init, code can
> +	 *  disappear and change, we need to protect against faulting
> +	 *  as well as code changing. We do this by using the
> +	 *  probe_kernel_* functions.
> +	 *
> +	 * No real locking needed, this code is run through
> +	 * kstop_machine, or before SMP starts.
> +	 */
> +
> +	/* read the text we want to modify */
> +	if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE))
> +		return -EFAULT;
> +	/*
> +	 * the routine could be called before the module add into the module
> +	 * list. At that time, even the ip is a module address,
> +	 * module_text_address() will return wrong. Foutunately, this only
> +	 * happens on convert call insn to nop. In this case, we don't need
> +	 * module, so we can workaround the issue.
> +	 * FIXME: make the routine accepts a 'struct module' parameter
> +	 * */
> +	if (!mod && is_kernel_address) {
> +		/* Make sure it is what we expect it to be */
> +		if (memcmp(replaced, old_code, MCOUNT_INSN_SIZE) != 0)
> +			return -EINVAL;
> +		goto do_replace;
> +	}
> +
> +	/* for a mcount call in a module, the mcount will call a PLT entry */
> +	old_code_type = CODE_IS_NOP;
> +
> +	tmp_call = (void *)replaced;
> +	call_insn->sign = tmp_call->sign;
> +	call_insn->imm20 = tmp_call->imm20;
> +	/* is original code call? */
> +	if (memcmp(replaced, ftrace_call_code, MCOUNT_INSN_SIZE) == 0)
> +		old_code_type = CODE_IS_CALL;
> +
> +	if (!mod && old_code_type != CODE_IS_CALL)
> +		return -EINVAL;
> +
> +	/* from nop to call */
> +	if (old_code_type == CODE_IS_NOP) {
> +		unsigned long offset;
> +		int sign;
> +
> +		if (memcmp(replaced, ftrace_nop_code, MCOUNT_INSN_SIZE) != 0)
> +			return -EINVAL;
> +		calculate_offset(ip, mod->arch.ftrace_caller_plt, &offset, &sign);
> +		call_insn->sign = sign;
> +		call_insn->imm20 = offset;
> +		new_code = ftrace_call_code;
> +	}
> +
> +do_replace:
> +	/* replace the text with the new text */
> +	if (probe_kernel_write((void *)ip, new_code, MCOUNT_INSN_SIZE))
> +		return -EPERM;
> +
> +	flush_icache_range(ip, ip + MCOUNT_INSN_SIZE);
> +	return 0;
> +}
> +
> +/* we don't support update ftrace_func */
> +int ftrace_update_ftrace_func(ftrace_func_t func)
> +{
> +	return 0;
> +}
> +
> +/* run from kstop_machine */
> +int __init ftrace_dyn_arch_init(void *data)
> +{
> +	*(unsigned long *)data = 0;
> +
> +	return 0;
> +}
> Index: linux/scripts/recordmcount.pl
> ===================================================================
> --- linux.orig/scripts/recordmcount.pl	2008-12-17 17:14:49.000000000 +0800
> +++ linux/scripts/recordmcount.pl	2008-12-17 17:15:10.000000000 +0800
> @@ -100,14 +100,19 @@ $P =~ s@.*/@@g;
>  
>  my $V = '0.1';
>  
> -if ($#ARGV < 6) {
> -	print "usage: $P arch objdump objcopy cc ld nm rm mv inputfile\n";
> +if ($#ARGV < 7) {
> +	print "usage: $P arch objdump objcopy cc ld nm rm mv is_module inputfile\n";
>  	print "version: $V\n";
>  	exit(1);
>  }
>  
>  my ($arch, $bits, $objdump, $objcopy, $cc,
> -    $ld, $nm, $rm, $mv, $inputfile) = @ARGV;
> +    $ld, $nm, $rm, $mv, $is_module, $inputfile) = @ARGV;
> +
> +# This file refers to mcount, so let's ignore it
> +if ($inputfile eq "kernel/trace/ftrace.o") {
> +	exit(0);
> +}
>  
>  # Acceptable sections to record.
>  my %text_sections = (
> @@ -167,6 +172,15 @@ if ($arch eq "x86_64") {
>      $objcopy .= " -O elf32-i386";
>      $cc .= " -m32";
>  
> +} elsif ($arch eq "ia64") {
> +    $section_regex = "Disassembly of section\\s+(\\S+):";
> +    $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
> +    $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s_mcount\$";
> +    $type = "data8";
> +
> +    if ($is_module eq "0") {
> +        $cc .= " -mconstant-gp";

Again, take a look at the powerpc code to handle the 24 bit jumps, and
see if you can solve this with that.

> +    }
>  } else {
>      die "Arch $arch is not supported with CONFIG_FTRACE_MCOUNT_RECORD";
>  }
> Index: linux/arch/ia64/include/asm/ftrace.h
> ===================================================================
> --- linux.orig/arch/ia64/include/asm/ftrace.h	2008-12-17 17:14:49.000000000 +0800
> +++ linux/arch/ia64/include/asm/ftrace.h	2008-12-17 17:15:10.000000000 +0800
> @@ -7,6 +7,18 @@
>  #ifndef __ASSEMBLY__
>  extern void _mcount(unsigned long pfs, unsigned long r1, unsigned long b0, unsigned long r0);
>  #define mcount _mcount
> +#define ftrace_call ftrace_caller
> +
> +#include <asm/kprobes.h>
> +/* In IA64, MCOUNT_ADDR is set in link time, so it's not a constant at compile time */
> +#define MCOUNT_ADDR (((struct fnptr *)mcount)->ip)

Again, powerpc64 does pretty much the same thing. I'm thinking you may
be able to avoid this.

> +#define FTRACE_ADDR (((struct fnptr *)ftrace_caller)->ip)
> +
> +static inline unsigned long ftrace_call_adjust(unsigned long addr)
> +{
> +	/* second bundle, insn 2 */
> +	return addr - 0x12;
> +}
>  
>  #endif
>  
> Index: linux/kernel/trace/ftrace.c
> ===================================================================
> --- linux.orig/kernel/trace/ftrace.c	2008-12-17 17:14:49.000000000 +0800
> +++ linux/kernel/trace/ftrace.c	2008-12-17 17:15:10.000000000 +0800
> @@ -168,14 +168,6 @@ static int __unregister_ftrace_function(
>  # error Dynamic ftrace depends on MCOUNT_RECORD
>  #endif
>  
> -/*
> - * Since MCOUNT_ADDR may point to mcount itself, we do not want
> - * to get it confused by reading a reference in the code as we
> - * are parsing on objcopy output of text. Use a variable for
> - * it instead.
> - */
> -static unsigned long mcount_addr = MCOUNT_ADDR;
> -
>  enum {
>  	FTRACE_ENABLE_CALLS		= (1 << 0),
>  	FTRACE_DISABLE_CALLS		= (1 << 1),
> @@ -322,7 +314,9 @@ ftrace_record_ip(unsigned long ip)
>  	return rec;
>  }
>  
> +#ifndef FTRACE_ADDR
>  #define FTRACE_ADDR ((long)(ftrace_caller))
> +#endif
>  
>  static int
>  __ftrace_replace_code(struct dyn_ftrace *rec,
> @@ -458,7 +452,7 @@ ftrace_code_disable(struct dyn_ftrace *r
>  	ip = rec->ip;
>  
>  	nop = ftrace_nop_replace();
> -	call = ftrace_call_replace(ip, mcount_addr);
> +	call = ftrace_call_replace(ip, MCOUNT_ADDR);
>  
>  	ret = ftrace_modify_code(ip, call, nop);
>  	if (ret) {
> Index: linux/scripts/Makefile.build
> ===================================================================
> --- linux.orig/scripts/Makefile.build	2008-12-17 17:14:50.000000000 +0800
> +++ linux/scripts/Makefile.build	2008-12-17 17:15:10.000000000 +0800
> @@ -114,6 +114,7 @@ endif
>  # Default is built-in, unless we know otherwise
>  modkern_cflags := $(CFLAGS_KERNEL)
>  quiet_modtag := $(empty)   $(empty)
> +is_module := 0
>  
>  $(real-objs-m)        : modkern_cflags := $(CFLAGS_MODULE)
>  $(real-objs-m:.o=.i)  : modkern_cflags := $(CFLAGS_MODULE)
> @@ -125,6 +126,11 @@ $(real-objs-m:.o=.i)  : quiet_modtag := 
>  $(real-objs-m:.o=.s)  : quiet_modtag := [M]
>  $(real-objs-m:.o=.lst): quiet_modtag := [M]
>  
> +$(real-objs-m)        : is_module := 1
> +$(real-objs-m:.o=.i)  : is_module := 1
> +$(real-objs-m:.o=.s)  : is_module := 1
> +$(real-objs-m:.o=.lst): is_module := 1
> +
>  $(obj-m)              : quiet_modtag := [M]
>  
>  # Default for not multi-part modules
> @@ -207,7 +213,7 @@ endif
>  ifdef CONFIG_FTRACE_MCOUNT_RECORD
>  cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl \
>  	"$(ARCH)" "$(arch_bits)" "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" \
> -	"$(NM)" "$(RM)" "$(MV)" "$(@)";
> +	"$(NM)" "$(RM)" "$(MV)" "$(is_module)" "$(@)";
>  endif
>  
>  define rule_cc_o_c
> Index: linux/arch/ia64/include/asm/module.h
> ===================================================================
> --- linux.orig/arch/ia64/include/asm/module.h	2008-12-17 17:14:49.000000000 +0800
> +++ linux/arch/ia64/include/asm/module.h	2008-12-17 17:15:10.000000000 +0800
> @@ -21,6 +21,10 @@ struct mod_arch_specific {
>  	void *core_unw_table;		/* core unwind-table cookie returned by unwinder */
>  	void *init_unw_table;		/* init unwind-table cookie returned by unwinder */
>  	unsigned int next_got_entry;	/* index of next available got entry */
> +
> +#ifdef CONFIG_DYNAMIC_FTRACE
> +	uint64_t ftrace_caller_plt;
> +#endif
>  };
>  
>  #define Elf_Shdr	Elf64_Shdr
> Index: linux/arch/ia64/kernel/module.c
> ===================================================================
> --- linux.orig/arch/ia64/kernel/module.c	2008-12-17 17:14:49.000000000 +0800
> +++ linux/arch/ia64/kernel/module.c	2008-12-17 17:15:10.000000000 +0800
> @@ -32,6 +32,7 @@
>  #include <linux/moduleloader.h>
>  #include <linux/string.h>
>  #include <linux/vmalloc.h>
> +#include <linux/ftrace.h>
>  
>  #include <asm/patch.h>
>  #include <asm/unaligned.h>
> @@ -468,6 +469,9 @@ module_frob_arch_sections (Elf_Ehdr *ehd
>  			core_plts += count_plts(rels, numrels);
>  	}
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE
> +	core_plts++;
> +#endif
>  	mod->arch.core_plt->sh_type = SHT_NOBITS;
>  	mod->arch.core_plt->sh_flags = SHF_EXECINSTR | SHF_ALLOC;
>  	mod->arch.core_plt->sh_addralign = 16;
> @@ -839,6 +843,17 @@ apply_relocate_add (Elf64_Shdr *sechdrs,
>  		if (ret < 0)
>  			return ret;
>  	}
> +#ifdef CONFIG_DYNAMIC_FTRACE
> +	if (!mod->arch.ftrace_caller_plt) {
> +		int ok = 1;
> +		/* fake a 'struct insn' for get_plt, which is in module core */
> +		uint64_t fake_insn = (uint64_t)mod->module_init + mod->init_size;
> +		mod->arch.ftrace_caller_plt = get_plt(mod,
> +			(struct insn *)fake_insn, (uint64_t)ftrace_caller, &ok);
> +		if (!ok)
> +			mod->arch.ftrace_caller_plt = 0;
> +	}
> +#endif
>  	return 0;
>  }
>  
> Index: linux/arch/ia64/kernel/entry.S
> ===================================================================
> --- linux.orig/arch/ia64/kernel/entry.S	2008-12-17 17:15:20.000000000 +0800
> +++ linux/arch/ia64/kernel/entry.S	2008-12-17 17:16:54.000000000 +0800
> @@ -1406,6 +1406,42 @@ GLOBAL_ENTRY(unw_init_running)
>  END(unw_init_running)
>  
>  #ifdef CONFIG_FUNCTION_TRACER
> +#ifdef CONFIG_DYNAMIC_FTRACE
> +GLOBAL_ENTRY(_mcount)
> +	br ftrace_stub
> +END(_mcount)
> +
> +GLOBAL_ENTRY(ftrace_caller)
> +	movl r2 = ftrace_stub
> +	movl r3 = ftrace_trace_function;;
> +	ld8 r3 = [r3];;
> +	ld8 r3 = [r3];;
> +	cmp.eq p7,p0 = r2, r3
> +(p7)	br.sptk.many ftrace_stub
> +	;;
> +
> +	alloc loc0 = ar.pfs, 4, 4, 2, 0
> +	;;
> +	mov loc1 = b0
> +	mov out0 = b0
> +	mov loc2 = r8
> +	mov loc3 = r15
> +	;;
> +	adds out0 = -MCOUNT_INSN_SIZE, out0
> +	mov out1 = in2
> +	mov b6 = r3
> +
> +	br.call.sptk.many b0 = b6
> +	;;
> +	mov ar.pfs = loc0
> +	mov b0 = loc1
> +	mov r8 = loc2
> +	mov r15 = loc3
> +	br ftrace_stub
> +	;;
> +END(ftrace_caller)
> +
> +#else
>  GLOBAL_ENTRY(_mcount)
>  	movl r2 = ftrace_stub
>  	movl r3 = ftrace_trace_function;;
> @@ -1435,6 +1471,7 @@ GLOBAL_ENTRY(_mcount)
>  	br ftrace_stub
>  	;;
>  END(_mcount)
> +#endif
>  
>  GLOBAL_ENTRY(ftrace_stub)
>  	mov r3 = b0
> 
> 

Thanks for taking the time to do this.

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel]     [Sparc Linux]     [DCCP]     [Linux ARM]     [Yosemite News]     [Linux SCSI]     [Linux x86_64]     [Linux for Ham Radio]

  Powered by Linux