On Thu, 2008-12-18 at 13:19 +0800, Steven Rostedt wrote: > 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 Thanks for your time. Is my implementation wrong or just not matches with ppc's? Adding a new PLT entry is easy and nice solution for ia64 to me. I had quick look at your patch, it appears you are using ftrace_make_call and something, are there any generic ftrace code changes related them? > > + > > +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. No, this isn't an atomic operation, but why it should be atomic? As far as I know, the code modification is called in stop_machine. Preempt is impossible. Actually x86's mcount is 5 bytes, and not an atomic too. > > +/* 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 Yes, these are what ia64 added. A simple jump doesn't work, because we must save some registers. > > > > +} 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. this flag isn't related with the jump. Instead, ia64 requires all modules build without the option. > > + } > > } 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. I know nothing about ppc64, but in ia64, a function pointer actually is a structure (struct fnptr), which has two members. One is gp (GOT pointer) the other is the function address. Thanks, Shaohua -- 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
![]() |