linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Add C version of recordmcount for MIPS
@ 2010-10-27 10:59 Wu Zhangjin
  2010-10-27 10:59 ` [PATCH 1/3] ftrace/MIPS: Add MIPS64 support for C version of recordmcount Wu Zhangjin
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Wu Zhangjin @ 2010-10-27 10:59 UTC (permalink / raw)
  To: rostedt, Ralf Baechle, linux-mips, linux-kernel
  Cc: John Reiser, Maciej W. Rozycki, David Daney, Wu Zhangjin

From: Wu Zhangjin <wuzhangjin@gmail.com>

This patchset adds the C Version of recordmcount for MIPS, it includes the
necessary fixups for the MIPS64 support and module support of the old
recordmcount.c.

To add MIPS64 support, John has introduced function pointers which can be
overriden by specific e_machine(.e.g. EM_MIPS, EM_xx) in do_file() of
scripts/recordmcount.c. this method helps a lot, when migrating from the old
Perl recordmcount, the left Archs may possibly apply this method to add their
specific support. Maciej has simplified the MIPS specific ELF64_Rel.r_info and
the related functions.

The module support for MIPS is such a good application of John's method, it
adds MIPS_is_fake_mcount() to filter one of the two _mcount symbol of the long
mcount call, a function pointer: is_fake_mcount() points to the default 'empty'
fn_is_fake_mcount(), we overrides it by MIPS_is_fake_mcount() for EM_MIPS in
do_file().

At last, HAVE_C_RECORDMCOUNT is selected for MIPS to enable the C version of
recordmcount.

The whole support has been tested on my YeeLoong laptop(MIPS, Litten Endian),
including: 32bit kernel and moduels, 64bit kernel and modules.

Here is part of the testing log:

-------

root@yeeloong:~# lsmod
Module                  Size  Used by
yeeloong_laptop        16142  0 
sparse_keymap           3803  1 yeeloong_laptop
hwmon                   1841  1 yeeloong_laptop
backlight               5627  1 yeeloong_laptop
power_supply            9512  1 yeeloong_laptop
output                  2524  1 yeeloong_laptop

1. 32bit kernel and module

root@yeeloong:~# uname -a
Linux yeeloong 2.6.36-ftrace+ #71 PREEMPT Wed Oct 27 15:03:02 CST 2010 mips GNU/Linux
root@yeeloong:~# mount -t debugfs nodev /debug
root@yeeloong:~# echo function > /debug/tracing/current_tracer
root@yeeloong:~# echo 1 > /debug/tracing/tracing_enabled
root@yeeloong:~# sleep 1
root@yeeloong:~# echo 0 > /debug/tracing/tracing_enabled
root@yeeloong:~# head -20 /debug/tracing/trace
# tracer: function
#
#	    TASK-PID	CPU#	TIMESTAMP  FUNCTION
#	       | |	 |	    |	      |
	    Xorg-1443  [000]   132.196336: remove_wait_queue <-poll_freewait
	    Xorg-1443  [000]   132.196337: fput <-poll_freewait
	    Xorg-1443  [000]   132.196338: remove_wait_queue <-poll_freewait
	    Xorg-1443  [000]   132.196339: fput <-poll_freewait
	    Xorg-1443  [000]   132.196340: remove_wait_queue <-poll_freewait
	    Xorg-1443  [000]   132.196341: fput <-poll_freewait
	    Xorg-1443  [000]   132.196342: remove_wait_queue <-poll_freewait
	    Xorg-1443  [000]   132.196343: fput <-poll_freewait
	    Xorg-1443  [000]   132.196344: remove_wait_queue <-poll_freewait
	    Xorg-1443  [000]   132.196345: fput <-poll_freewait
	    Xorg-1443  [000]   132.196346: remove_wait_queue <-poll_freewait
	    Xorg-1443  [000]   132.196347: fput <-poll_freewait
	    Xorg-1443  [000]   132.196348: remove_wait_queue <-poll_freewait
	    Xorg-1443  [000]   132.196349: fput <-poll_freewait
	    Xorg-1443  [000]   132.196351: poll_select_copy_remaining <-sys_select
	    Xorg-1443  [000]   132.196352: ktime_get_ts <-poll_select_copy_remaining
root@yeeloong:~# echo *yeeloong* > /debug/tracing/setKset_ftrace_filter
root@yeeloong:~# echo 1 > /debug/tracing/tracing_enabled
root@yeeloong:~# (Press Fn + Up/Down to trigger the functions in yeeloong_laptop module) 
root@yeeloong:~# echo 0 > /tracing/tracing_enabled
root@yeeloong:~# head -20 /debug/tracing/trace
# tracer: function
#
#	    TASK-PID	CPU#	TIMESTAMP  FUNCTION
#	       | |	 |	    |	      |
 hald-addon-gene-1397  [000]   168.904096: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   170.414572: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   170.625428: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   170.806029: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   170.977210: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   171.262519: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   171.271132: yeeloong_set_brightness <-backlight_store_brightness
 hald-addon-gene-1397  [000]   171.545363: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   171.679358: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   171.825652: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   171.996121: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   172.246240: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   172.383859: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   172.532963: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   172.844048: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1397  [000]   172.851410: yeeloong_set_brightness <-backlight_store_brightness
root@yeeloong:~# exit

2. 64bit kernel and module

root@yeeloong:~# uname -a
Linux yeeloong 2.6.36-ftrace+ #75 PREEMPT Wed Oct 27 16:12:20 CST 2010 mips64 GNU/Linux
root@yeeloong:~# mount -t debugfs nodev /debug
root@yeeloong:~# echo function > /debug/tracing/current_tracer 
root@yeeloong:~# echo 1 > /debug/tracing/tracing_enabled 
root@yeeloong:~# ls
[snip]
root@yeeloong:~# echo 0 > /debug/tracing/tracing_enabled 
root@yeeloong:~# cat /debug/tracing/trace | head -20
# tracer: function
#
#           TASK-PID    CPU#    TIMESTAMP  FUNCTION
#              | |       |          |         |
          <idle>-0     [000]   389.507465: complete <-usb_stor_blocking_completion
          <idle>-0     [000]   389.507466: default_wake_function <-complete
          <idle>-0     [000]   389.507467: try_to_wake_up <-default_wake_function
          <idle>-0     [000]   389.507468: enqueue_task_fair <-try_to_wake_up
          <idle>-0     [000]   389.507469: T.1160 <-enqueue_task_fair
          <idle>-0     [000]   389.507470: check_preempt_curr_idle <-try_to_wake_up
          <idle>-0     [000]   389.507472: usb_free_urb <-usb_hcd_giveback_urb
          <idle>-0     [000]   389.507473: dma_pool_free <-qh_completions
          <idle>-0     [000]   389.507475: mod_timer <-scan_async
          <idle>-0     [000]   389.507476: usb_hcd_irq <-handle_IRQ_event
          <idle>-0     [000]   389.507477: ohci_irq <-usb_hcd_irq
          <idle>-0     [000]   389.507481: note_interrupt <-handle_level_irq
          <idle>-0     [000]   389.507482: compat_irq_unmask <-handle_level_irq
          <idle>-0     [000]   389.507483: enable_8259A_irq <-compat_irq_unmask
          <idle>-0     [000]   389.507484: irq_exit <-do_IRQ
          <idle>-0     [000]   389.507485: rcu_irq_exit <-irq_exit
root@yeeloong:~# echo *yeeloong* > /debug/tracing/set_ftrace_filter 
root@yeeloong:~# echo 1 > /debug/tracing/tracing_enabled 
root@yeeloong:~# (Press Fn + Up/Down here to trigger the kernel functions called in yeeloong_laptop module) 
root@yeeloong:~# echo 0 > /debug/tracing/tracing_enabled 
root@yeeloong:~# cat /debug/tracing/trace | head -20
# tracer: function
#
#           TASK-PID    CPU#    TIMESTAMP  FUNCTION
#              | |       |          |         |
            hald-1378  [000]   414.479887: yeeloong_get_bat_props <-power_supply_show_property
            hald-1378  [000]   414.480143: yeeloong_get_bat_props <-power_supply_show_property
            hald-1378  [000]   414.480327: yeeloong_get_bat_props <-power_supply_show_property
            hald-1378  [000]   414.480486: yeeloong_get_bat_props <-power_supply_show_property
            hald-1378  [000]   414.480602: yeeloong_get_bat_props <-power_supply_show_property
            hald-1378  [000]   414.480741: yeeloong_get_bat_props <-power_supply_show_property
            hald-1378  [000]   414.480860: yeeloong_get_bat_props <-power_supply_show_property
         upowerd-1656  [000]   414.481609: yeeloong_get_bat_props <-power_supply_show_property
         upowerd-1656  [000]   414.482058: yeeloong_get_bat_props <-power_supply_show_property
         upowerd-1656  [000]   414.482185: yeeloong_get_bat_props <-power_supply_show_property
         upowerd-1656  [000]   414.482257: yeeloong_get_bat_props <-power_supply_show_property
         upowerd-1656  [000]   414.482333: yeeloong_get_bat_props <-power_supply_show_property
         upowerd-1656  [000]   414.482414: yeeloong_get_bat_props <-power_supply_show_property
 hald-addon-gene-1409  [000]   415.412442: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1409  [000]   416.135571: yeeloong_get_brightness <-backlight_show_actual_brightness
 hald-addon-gene-1409  [000]   416.381292: yeeloong_get_brightness <-backlight_show_actual_brightness

--------

Thanks all.

Best Regards,
	Wu Zhangjin

Wu Zhangjin (3):
  ftrace/MIPS: Add MIPS64 support for C version of recordmcount
  ftrace/MIPS: Add module support for C version of recordmcount
  ftrace/MIPS: Enable C Version of recordmcount

 arch/mips/Kconfig      |    1 +
 scripts/recordmcount.c |   44 ++++++++++++++++++++++++
 scripts/recordmcount.h |   86 +++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 126 insertions(+), 5 deletions(-)


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] ftrace/MIPS: Add MIPS64 support for C version of recordmcount
  2010-10-27 10:59 [PATCH 0/3] Add C version of recordmcount for MIPS Wu Zhangjin
@ 2010-10-27 10:59 ` Wu Zhangjin
  2010-10-27 13:32   ` Steven Rostedt
  2010-10-27 10:59 ` [PATCH 2/3] ftrace/MIPS: Add module " Wu Zhangjin
  2010-10-27 10:59 ` [PATCH 3/3] ftrace/MIPS: Enable C Version " Wu Zhangjin
  2 siblings, 1 reply; 11+ messages in thread
From: Wu Zhangjin @ 2010-10-27 10:59 UTC (permalink / raw)
  To: rostedt, Ralf Baechle, linux-mips, linux-kernel
  Cc: John Reiser, Maciej W. Rozycki, David Daney, Wu Zhangjin, John Reiser

From: Wu Zhangjin <wuzhangjin@gmail.com>

MIPS64 has 'weird' Elf64_Rel.r_info[1,2], which must be used instead of
the generic Elf64_Rel.r_info, otherwise, the C version of recordmcount
will not work for "segmentation fault".

----
[1] http://techpubs.sgi.com/library/manuals/4000/007-4658-001/pdf/007-4658-001.pdf
[2] arch/mips/include/asm/module.h

Signed-off-by: John Reiser <jreiser@BitWagon.com>
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
Tested-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 scripts/recordmcount.c |   41 +++++++++++++++++++++++++++++++++++++++++
 scripts/recordmcount.h |   34 ++++++++++++++++++++++++++++++----
 2 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 26e1271..2d32b9c 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -217,6 +217,39 @@ is_mcounted_section_name(char const *const txtname)
 #define RECORD_MCOUNT_64
 #include "recordmcount.h"
 
+/* 64-bit EM_MIPS has weird ELF64_Rela.r_info.
+ * http://techpubs.sgi.com/library/manuals/4000/007-4658-001/pdf/007-4658-001.pdf
+ * We interpret Table 29 Relocation Operation (Elf64_Rel, Elf64_Rela) [p.40]
+ * to imply the order of the members; the spec does not say so.
+ *	typedef unsigned char Elf64_Byte;
+ * fails on MIPS64 because their <elf.h> already has it!
+ */
+
+typedef uint8_t myElf64_Byte;		/* Type for a 8-bit quantity.  */
+
+union mips_r_info {
+	Elf64_Xword r_info;
+	struct {
+		Elf64_Word r_sym;		/* Symbol index.  */
+		myElf64_Byte r_ssym;		/* Special symbol.  */
+		myElf64_Byte r_type3;		/* Third relocation.  */
+		myElf64_Byte r_type2;		/* Second relocation.  */
+		myElf64_Byte r_type;		/* First relocation.  */
+	} r_mips;
+};
+
+static uint64_t MIPS64_r_sym(Elf64_Rel const *rp)
+{
+	return w(((union mips_r_info){ .r_info = rp->r_info }).r_mips.r_sym);
+}
+
+static void MIPS64_r_info(Elf64_Rel *const rp, unsigned sym, unsigned type)
+{
+	rp->r_info = ((union mips_r_info){
+		.r_mips = { .r_sym = w(sym), .r_type = type }
+	}).r_info;
+}
+
 static void
 do_file(char const *const fname)
 {
@@ -268,6 +301,7 @@ do_file(char const *const fname)
 	case EM_386:	 reltype = R_386_32;                   break;
 	case EM_ARM:	 reltype = R_ARM_ABS32;                break;
 	case EM_IA_64:	 reltype = R_IA64_IMM64;   gpfx = '_'; break;
+	case EM_MIPS:	 /* reltype: e_class    */ gpfx = '_'; break;
 	case EM_PPC:	 reltype = R_PPC_ADDR32;   gpfx = '_'; break;
 	case EM_PPC64:	 reltype = R_PPC64_ADDR64; gpfx = '_'; break;
 	case EM_S390:    /* reltype: e_class    */ gpfx = '_'; break;
@@ -291,6 +325,8 @@ do_file(char const *const fname)
 		}
 		if (EM_S390 == w2(ehdr->e_machine))
 			reltype = R_390_32;
+		if (EM_MIPS == w2(ehdr->e_machine))
+			reltype = R_MIPS_32;
 		do32(ehdr, fname, reltype);
 	} break;
 	case ELFCLASS64: {
@@ -303,6 +339,11 @@ do_file(char const *const fname)
 		}
 		if (EM_S390 == w2(ghdr->e_machine))
 			reltype = R_390_64;
+		if (EM_MIPS == w2(ghdr->e_machine)) {
+			reltype = R_MIPS_64;
+			Elf64_r_sym = MIPS64_r_sym;
+			Elf64_r_info = MIPS64_r_info;
+		}
 		do64(ghdr, fname, reltype);
 	} break;
 	}  /* end switch */
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 7f39d09..190fd18 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -31,8 +31,12 @@
 #undef Elf_Rela
 #undef Elf_Sym
 #undef ELF_R_SYM
+#undef Elf_r_sym
 #undef ELF_R_INFO
+#undef Elf_r_info
 #undef ELF_ST_BIND
+#undef fn_ELF_R_SYM
+#undef fn_ELF_R_INFO
 #undef uint_t
 #undef _w
 #undef _align
@@ -52,8 +56,12 @@
 # define Elf_Rela		Elf64_Rela
 # define Elf_Sym		Elf64_Sym
 # define ELF_R_SYM		ELF64_R_SYM
+# define Elf_r_sym		Elf64_r_sym
 # define ELF_R_INFO		ELF64_R_INFO
+# define Elf_r_info		Elf64_r_info
 # define ELF_ST_BIND		ELF64_ST_BIND
+# define fn_ELF_R_SYM		fn_ELF64_R_SYM
+# define fn_ELF_R_INFO		fn_ELF64_R_INFO
 # define uint_t			uint64_t
 # define _w			w8
 # define _align			7u
@@ -72,14 +80,32 @@
 # define Elf_Rela		Elf32_Rela
 # define Elf_Sym		Elf32_Sym
 # define ELF_R_SYM		ELF32_R_SYM
+# define Elf_r_sym		Elf32_r_sym
 # define ELF_R_INFO		ELF32_R_INFO
+# define Elf_r_info		Elf32_r_info
 # define ELF_ST_BIND		ELF32_ST_BIND
+# define fn_ELF_R_SYM		fn_ELF32_R_SYM
+# define fn_ELF_R_INFO		fn_ELF32_R_INFO
 # define uint_t			uint32_t
 # define _w			w
 # define _align			3u
 # define _size			4
 #endif
 
+/* Functions and pointers that 64-bit EM_MIPS can override. */
+static uint_t fn_ELF_R_SYM(Elf_Rel const *rp)
+{
+	return ELF_R_SYM(_w(rp->r_info));
+}
+static uint_t (*Elf_r_sym)(Elf_Rel const *rp) = fn_ELF_R_SYM;
+
+static void fn_ELF_R_INFO(Elf_Rel *const rp, unsigned sym, unsigned type)
+{
+	rp->r_info = ELF_R_INFO(sym, type);
+}
+static void (*Elf_r_info)(Elf_Rel *const rp, unsigned sym, unsigned type) = fn_ELF_R_INFO;
+
+
 /* Append the new shstrtab, Elf_Shdr[], __mcount_loc and its relocations. */
 static void append_func(Elf_Ehdr *const ehdr,
 			Elf_Shdr *const shstr,
@@ -197,22 +223,22 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
 	for (t = nrel; t; --t) {
 		if (!mcountsym) {
 			Elf_Sym const *const symp =
-				&sym0[ELF_R_SYM(_w(relp->r_info))];
+				&sym0[Elf_r_sym(relp)];
 			char const *symname = &str0[w(symp->st_name)];
 
 			if ('.' == symname[0])
 				++symname;  /* ppc64 hack */
 			if (0 == strcmp((('_' == gpfx) ? "_mcount" : "mcount"),
 					symname))
-				mcountsym = ELF_R_SYM(_w(relp->r_info));
+				mcountsym = Elf_r_sym(relp);
 		}
 
-		if (mcountsym == ELF_R_SYM(_w(relp->r_info))) {
+		if (mcountsym == Elf_r_sym(relp)) {
 			uint_t const addend = _w(_w(relp->r_offset) - recval);
 
 			mrelp->r_offset = _w(offbase
 				+ ((void *)mlocp - (void *)mloc0));
-			mrelp->r_info = _w(ELF_R_INFO(recsym, reltype));
+			Elf_r_info(mrelp, recsym, reltype);
 			if (sizeof(Elf_Rela) == rel_entsize) {
 				((Elf_Rela *)mrelp)->r_addend = addend;
 				*mlocp++ = 0;
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] ftrace/MIPS: Add module support for C version of recordmcount
  2010-10-27 10:59 [PATCH 0/3] Add C version of recordmcount for MIPS Wu Zhangjin
  2010-10-27 10:59 ` [PATCH 1/3] ftrace/MIPS: Add MIPS64 support for C version of recordmcount Wu Zhangjin
@ 2010-10-27 10:59 ` Wu Zhangjin
  2010-10-27 10:59 ` [PATCH 3/3] ftrace/MIPS: Enable C Version " Wu Zhangjin
  2 siblings, 0 replies; 11+ messages in thread
From: Wu Zhangjin @ 2010-10-27 10:59 UTC (permalink / raw)
  To: rostedt, Ralf Baechle, linux-mips, linux-kernel
  Cc: John Reiser, Maciej W. Rozycki, David Daney, Wu Zhangjin

From: Wu Zhangjin <wuzhangjin@gmail.com>

Since MIPS modules' address space differs from the core kernel space, to access
the _mcount in the core kernel, the kernel functions in modules must use long
call (-mlong-calls): load the _mcount address into one register and jump to the
address stored by the register:

 c:  3c030000        lui     v1,0x0  <-------->  b label
           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
label:

In the old Perl version of recordmcount, we only need to record the position of
the 1st R_MIPS_HI16 type of _mcount, and later, in ftrace_make_nop(), replace
the instruction in this position by a "b label" and in ftrace_make_call(),
replace it back.

But, the default C version of recordmcount records all of the _mcount symbols,
so, we must filter the 2nd _mcount like the Perl version of recordmcount does.

The C version of recordmcount copes with the symbols before they are linked, So
It doesn't know the type of the symbols and therefore can not filter the
symbols as the Perl version of recordmcount does. But as we can see above, the
2nd _mcount symbols of the long call alawys follows the 1st _mcount symbol of
the same long call, which means the offset from the 1st to the 2nd is fixed, it
is 0x10-0xc = 4 here, 4 is the length of the 1st load instruciton, for MIPS has
fixed length of instructions, this offset is always 4.

And as we know, the _mcount is inserted into the entry of every kernel
function, the offset between the other _mcount's is expected to be always
bigger than 4. So, to filter the 2ns _mcount symbol of the long call, we can
simply check the offset between two _mcount symbols, If it is 4, then, filter
the 2nd _mcount symbol.

To avoid touching too much code, an 'empty' function fn_is_fake_mcount() is
added for all of the archs, and the specific archs can override it via chaning
the function pointer: is_fake_mcount in do_file() with the e_machine. e.g. This
patch adds MIPS_is_fake_mcount() to override the default fn_is_fake_mcount()
pointed by is_fake_mcount.

This fn_is_fake_mcount() checks if the _mcount symbol is fake, e.g. the 2nd
_mcount symbol of the long call is fake, for there are 2 _mcount symbols mapped
to one real mcount call, so, one of them is fake and must be filtered.

This fn_is_fake_mcount() is called in sift_rel_mcount() after finding the
_mcount symbols and before adding the _mcount symbol into mrelp, so, it can
prevent the fake mcount symbol going into the last __mcount_loc table.

Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 scripts/recordmcount.c |    5 +++-
 scripts/recordmcount.h |   56 +++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 57 insertions(+), 4 deletions(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 2d32b9c..f2f32ee 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -325,8 +325,10 @@ do_file(char const *const fname)
 		}
 		if (EM_S390 == w2(ehdr->e_machine))
 			reltype = R_390_32;
-		if (EM_MIPS == w2(ehdr->e_machine))
+		if (EM_MIPS == w2(ehdr->e_machine)) {
 			reltype = R_MIPS_32;
+			is_fake_mcount32 = MIPS32_is_fake_mcount;
+		}
 		do32(ehdr, fname, reltype);
 	} break;
 	case ELFCLASS64: {
@@ -343,6 +345,7 @@ do_file(char const *const fname)
 			reltype = R_MIPS_64;
 			Elf64_r_sym = MIPS64_r_sym;
 			Elf64_r_info = MIPS64_r_info;
+			is_fake_mcount64 = MIPS64_is_fake_mcount;
 		}
 		do64(ghdr, fname, reltype);
 	} break;
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 190fd18..58e933a 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -19,12 +19,16 @@
  * Licensed under the GNU General Public License, version 2 (GPLv2).
  */
 #undef append_func
+#undef is_fake_mcount
+#undef fn_is_fake_mcount
+#undef MIPS_is_fake_mcount
 #undef sift_rel_mcount
 #undef find_secsym_ndx
 #undef __has_rel_mcount
 #undef has_rel_mcount
 #undef tot_relsize
 #undef do_func
+#undef Elf_Addr
 #undef Elf_Ehdr
 #undef Elf_Shdr
 #undef Elf_Rel
@@ -50,6 +54,10 @@
 # define has_rel_mcount		has64_rel_mcount
 # define tot_relsize		tot64_relsize
 # define do_func		do64
+# define is_fake_mcount		is_fake_mcount64
+# define fn_is_fake_mcount	fn_is_fake_mcount64
+# define MIPS_is_fake_mcount	MIPS64_is_fake_mcount
+# define Elf_Addr		Elf64_Addr
 # define Elf_Ehdr		Elf64_Ehdr
 # define Elf_Shdr		Elf64_Shdr
 # define Elf_Rel		Elf64_Rel
@@ -74,6 +82,10 @@
 # define has_rel_mcount		has32_rel_mcount
 # define tot_relsize		tot32_relsize
 # define do_func		do32
+# define is_fake_mcount		is_fake_mcount32
+# define fn_is_fake_mcount	fn_is_fake_mcount32
+# define MIPS_is_fake_mcount	MIPS32_is_fake_mcount
+# define Elf_Addr		Elf32_Addr
 # define Elf_Ehdr		Elf32_Ehdr
 # define Elf_Shdr		Elf32_Shdr
 # define Elf_Rel		Elf32_Rel
@@ -92,7 +104,13 @@
 # define _size			4
 #endif
 
-/* Functions and pointers that 64-bit EM_MIPS can override. */
+/* Functions and pointers that do_file() may override for specific e_machine. */
+static int fn_is_fake_mcount(Elf_Rel const *rp)
+{
+	return 0;
+}
+static int (*is_fake_mcount)(Elf_Rel const *rp) = fn_is_fake_mcount;
+
 static uint_t fn_ELF_R_SYM(Elf_Rel const *rp)
 {
 	return ELF_R_SYM(_w(rp->r_info));
@@ -105,6 +123,39 @@ static void fn_ELF_R_INFO(Elf_Rel *const rp, unsigned sym, unsigned type)
 }
 static void (*Elf_r_info)(Elf_Rel *const rp, unsigned sym, unsigned type) = fn_ELF_R_INFO;
 
+/*
+ * MIPS mcount long call has 2 _mcount symbols, only the position of the 1st
+ * _mcount symbol is needed for dynamic function tracer, with it, to disable
+ * tracing(ftrace_make_nop), the instruction in the position is replaced with
+ * the "b label" instruction, to enable tracing(ftrace_make_call), replace the
+ * instruction back. So, here, we set the 2nd one as fake and filter it.
+ *
+ * c:	3c030000	lui	v1,0x0		<-->	b	label
+ *		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
+ * label:
+ */
+#define MIPS_FAKEMCOUNT_OFFSET	4
+
+static int MIPS_is_fake_mcount(Elf_Rel const *rp)
+{
+	static Elf_Addr old_r_offset;
+	Elf_Addr current_r_offset = _w(rp->r_offset);
+	int is_fake;
+
+	is_fake = old_r_offset &&
+		(current_r_offset - old_r_offset == MIPS_FAKEMCOUNT_OFFSET);
+	old_r_offset = current_r_offset;
+
+	return is_fake;
+}
 
 /* Append the new shstrtab, Elf_Shdr[], __mcount_loc and its relocations. */
 static void append_func(Elf_Ehdr *const ehdr,
@@ -183,7 +234,6 @@ static void append_func(Elf_Ehdr *const ehdr,
 	uwrite(fd_map, ehdr, sizeof(*ehdr));
 }
 
-
 /*
  * Look at the relocations in order to find the calls to mcount.
  * Accumulate the section offsets that are found, and their relocation info,
@@ -233,7 +283,7 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
 				mcountsym = Elf_r_sym(relp);
 		}
 
-		if (mcountsym == Elf_r_sym(relp)) {
+		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
 			uint_t const addend = _w(_w(relp->r_offset) - recval);
 
 			mrelp->r_offset = _w(offbase
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] ftrace/MIPS: Enable C Version of recordmcount
  2010-10-27 10:59 [PATCH 0/3] Add C version of recordmcount for MIPS Wu Zhangjin
  2010-10-27 10:59 ` [PATCH 1/3] ftrace/MIPS: Add MIPS64 support for C version of recordmcount Wu Zhangjin
  2010-10-27 10:59 ` [PATCH 2/3] ftrace/MIPS: Add module " Wu Zhangjin
@ 2010-10-27 10:59 ` Wu Zhangjin
  2010-10-27 13:36   ` Steven Rostedt
  2 siblings, 1 reply; 11+ messages in thread
From: Wu Zhangjin @ 2010-10-27 10:59 UTC (permalink / raw)
  To: rostedt, Ralf Baechle, linux-mips, linux-kernel
  Cc: John Reiser, Maciej W. Rozycki, David Daney, Wu Zhangjin

From: Wu Zhangjin <wuzhangjin@gmail.com>

Selects HAVE_C_RECORDMCOUNT to use the C version of the recordmcount
intead of the old Perl Version of recordmcount.

Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
---
 arch/mips/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 46cae2b..144e4b3 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -9,6 +9,7 @@ config MIPS
 	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE_MCOUNT_RECORD
+	select HAVE_C_RECORDMCOUNT
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_KPROBES
 	select HAVE_KRETPROBES
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] ftrace/MIPS: Add MIPS64 support for C version of recordmcount
  2010-10-27 10:59 ` [PATCH 1/3] ftrace/MIPS: Add MIPS64 support for C version of recordmcount Wu Zhangjin
@ 2010-10-27 13:32   ` Steven Rostedt
  2010-10-27 14:09     ` wu zhangjin
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2010-10-27 13:32 UTC (permalink / raw)
  To: Wu Zhangjin
  Cc: Ralf Baechle, linux-mips, linux-kernel, John Reiser,
	Maciej W. Rozycki, David Daney

On Wed, 2010-10-27 at 18:59 +0800, Wu Zhangjin wrote:
> From: Wu Zhangjin <wuzhangjin@gmail.com>

The From above states that you wrote this. Did you modify what John and
Maciej did? If so, please state clearly what each author has done, and
how you modified it. If you did not write this, please change the From
to the correct author.

If you did modify it, then state something like this in the change log.

Original version written by John Reiser <jreiser@BitWagon.com>

Usage of "union mips_r_info" and the functions MIPS64_r_sym() and
MIPS64_r_info() written by Maciej W. Rozycki <macro@linux-mips.org>

Then state the changes you made.

> 
> MIPS64 has 'weird' Elf64_Rel.r_info[1,2], which must be used instead of
> the generic Elf64_Rel.r_info, otherwise, the C version of recordmcount
> will not work for "segmentation fault".
> 
> ----
> [1] http://techpubs.sgi.com/library/manuals/4000/007-4658-001/pdf/007-4658-001.pdf
> [2] arch/mips/include/asm/module.h
> 
> Signed-off-by: John Reiser <jreiser@BitWagon.com>
> Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
> Tested-by: Wu Zhangjin <wuzhangjin@gmail.com>

Again, if you made changes, you need a SoB yourself.

Thanks!

-- Steve


> ---
>  scripts/recordmcount.c |   41 +++++++++++++++++++++++++++++++++++++++++
>  scripts/recordmcount.h |   34 ++++++++++++++++++++++++++++++----
>  2 files changed, 71 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
> index 26e1271..2d32b9c 100644
> --- a/scripts/recordmcount.c
> +++ b/scripts/recordmcount.c
> @@ -217,6 +217,39 @@ is_mcounted_section_name(char const *const txtname)
>  #define RECORD_MCOUNT_64
>  #include "recordmcount.h"
>  
> +/* 64-bit EM_MIPS has weird ELF64_Rela.r_info.
> + * http://techpubs.sgi.com/library/manuals/4000/007-4658-001/pdf/007-4658-001.pdf
> + * We interpret Table 29 Relocation Operation (Elf64_Rel, Elf64_Rela) [p.40]
> + * to imply the order of the members; the spec does not say so.
> + *	typedef unsigned char Elf64_Byte;
> + * fails on MIPS64 because their <elf.h> already has it!
> + */
> +
> +typedef uint8_t myElf64_Byte;		/* Type for a 8-bit quantity.  */
> +
> +union mips_r_info {
> +	Elf64_Xword r_info;
> +	struct {
> +		Elf64_Word r_sym;		/* Symbol index.  */
> +		myElf64_Byte r_ssym;		/* Special symbol.  */
> +		myElf64_Byte r_type3;		/* Third relocation.  */
> +		myElf64_Byte r_type2;		/* Second relocation.  */
> +		myElf64_Byte r_type;		/* First relocation.  */
> +	} r_mips;
> +};
> +
> +static uint64_t MIPS64_r_sym(Elf64_Rel const *rp)
> +{
> +	return w(((union mips_r_info){ .r_info = rp->r_info }).r_mips.r_sym);
> +}
> +
> +static void MIPS64_r_info(Elf64_Rel *const rp, unsigned sym, unsigned type)
> +{
> +	rp->r_info = ((union mips_r_info){
> +		.r_mips = { .r_sym = w(sym), .r_type = type }
> +	}).r_info;
> +}
> +
>  static void
>  do_file(char const *const fname)
>  {
> @@ -268,6 +301,7 @@ do_file(char const *const fname)
>  	case EM_386:	 reltype = R_386_32;                   break;
>  	case EM_ARM:	 reltype = R_ARM_ABS32;                break;
>  	case EM_IA_64:	 reltype = R_IA64_IMM64;   gpfx = '_'; break;
> +	case EM_MIPS:	 /* reltype: e_class    */ gpfx = '_'; break;
>  	case EM_PPC:	 reltype = R_PPC_ADDR32;   gpfx = '_'; break;
>  	case EM_PPC64:	 reltype = R_PPC64_ADDR64; gpfx = '_'; break;
>  	case EM_S390:    /* reltype: e_class    */ gpfx = '_'; break;
> @@ -291,6 +325,8 @@ do_file(char const *const fname)
>  		}
>  		if (EM_S390 == w2(ehdr->e_machine))
>  			reltype = R_390_32;
> +		if (EM_MIPS == w2(ehdr->e_machine))
> +			reltype = R_MIPS_32;
>  		do32(ehdr, fname, reltype);
>  	} break;
>  	case ELFCLASS64: {
> @@ -303,6 +339,11 @@ do_file(char const *const fname)
>  		}
>  		if (EM_S390 == w2(ghdr->e_machine))
>  			reltype = R_390_64;
> +		if (EM_MIPS == w2(ghdr->e_machine)) {
> +			reltype = R_MIPS_64;
> +			Elf64_r_sym = MIPS64_r_sym;
> +			Elf64_r_info = MIPS64_r_info;
> +		}
>  		do64(ghdr, fname, reltype);
>  	} break;
>  	}  /* end switch */
> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> index 7f39d09..190fd18 100644
> --- a/scripts/recordmcount.h
> +++ b/scripts/recordmcount.h
> @@ -31,8 +31,12 @@
>  #undef Elf_Rela
>  #undef Elf_Sym
>  #undef ELF_R_SYM
> +#undef Elf_r_sym
>  #undef ELF_R_INFO
> +#undef Elf_r_info
>  #undef ELF_ST_BIND
> +#undef fn_ELF_R_SYM
> +#undef fn_ELF_R_INFO
>  #undef uint_t
>  #undef _w
>  #undef _align
> @@ -52,8 +56,12 @@
>  # define Elf_Rela		Elf64_Rela
>  # define Elf_Sym		Elf64_Sym
>  # define ELF_R_SYM		ELF64_R_SYM
> +# define Elf_r_sym		Elf64_r_sym
>  # define ELF_R_INFO		ELF64_R_INFO
> +# define Elf_r_info		Elf64_r_info
>  # define ELF_ST_BIND		ELF64_ST_BIND
> +# define fn_ELF_R_SYM		fn_ELF64_R_SYM
> +# define fn_ELF_R_INFO		fn_ELF64_R_INFO
>  # define uint_t			uint64_t
>  # define _w			w8
>  # define _align			7u
> @@ -72,14 +80,32 @@
>  # define Elf_Rela		Elf32_Rela
>  # define Elf_Sym		Elf32_Sym
>  # define ELF_R_SYM		ELF32_R_SYM
> +# define Elf_r_sym		Elf32_r_sym
>  # define ELF_R_INFO		ELF32_R_INFO
> +# define Elf_r_info		Elf32_r_info
>  # define ELF_ST_BIND		ELF32_ST_BIND
> +# define fn_ELF_R_SYM		fn_ELF32_R_SYM
> +# define fn_ELF_R_INFO		fn_ELF32_R_INFO
>  # define uint_t			uint32_t
>  # define _w			w
>  # define _align			3u
>  # define _size			4
>  #endif
>  
> +/* Functions and pointers that 64-bit EM_MIPS can override. */
> +static uint_t fn_ELF_R_SYM(Elf_Rel const *rp)
> +{
> +	return ELF_R_SYM(_w(rp->r_info));
> +}
> +static uint_t (*Elf_r_sym)(Elf_Rel const *rp) = fn_ELF_R_SYM;
> +
> +static void fn_ELF_R_INFO(Elf_Rel *const rp, unsigned sym, unsigned type)
> +{
> +	rp->r_info = ELF_R_INFO(sym, type);
> +}
> +static void (*Elf_r_info)(Elf_Rel *const rp, unsigned sym, unsigned type) = fn_ELF_R_INFO;
> +
> +
>  /* Append the new shstrtab, Elf_Shdr[], __mcount_loc and its relocations. */
>  static void append_func(Elf_Ehdr *const ehdr,
>  			Elf_Shdr *const shstr,
> @@ -197,22 +223,22 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
>  	for (t = nrel; t; --t) {
>  		if (!mcountsym) {
>  			Elf_Sym const *const symp =
> -				&sym0[ELF_R_SYM(_w(relp->r_info))];
> +				&sym0[Elf_r_sym(relp)];
>  			char const *symname = &str0[w(symp->st_name)];
>  
>  			if ('.' == symname[0])
>  				++symname;  /* ppc64 hack */
>  			if (0 == strcmp((('_' == gpfx) ? "_mcount" : "mcount"),
>  					symname))
> -				mcountsym = ELF_R_SYM(_w(relp->r_info));
> +				mcountsym = Elf_r_sym(relp);
>  		}
>  
> -		if (mcountsym == ELF_R_SYM(_w(relp->r_info))) {
> +		if (mcountsym == Elf_r_sym(relp)) {
>  			uint_t const addend = _w(_w(relp->r_offset) - recval);
>  
>  			mrelp->r_offset = _w(offbase
>  				+ ((void *)mlocp - (void *)mloc0));
> -			mrelp->r_info = _w(ELF_R_INFO(recsym, reltype));
> +			Elf_r_info(mrelp, recsym, reltype);
>  			if (sizeof(Elf_Rela) == rel_entsize) {
>  				((Elf_Rela *)mrelp)->r_addend = addend;
>  				*mlocp++ = 0;



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] ftrace/MIPS: Enable C Version of recordmcount
  2010-10-27 10:59 ` [PATCH 3/3] ftrace/MIPS: Enable C Version " Wu Zhangjin
@ 2010-10-27 13:36   ` Steven Rostedt
  2010-10-30 13:28     ` Maciej W. Rozycki
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2010-10-27 13:36 UTC (permalink / raw)
  To: Wu Zhangjin
  Cc: Ralf Baechle, linux-mips, linux-kernel, John Reiser,
	Maciej W. Rozycki, David Daney

On Wed, 2010-10-27 at 18:59 +0800, Wu Zhangjin wrote:
> From: Wu Zhangjin <wuzhangjin@gmail.com>
> 
> Selects HAVE_C_RECORDMCOUNT to use the C version of the recordmcount
> intead of the old Perl Version of recordmcount.
> 
> Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>

I'd like to get an Acked-by from Ralf and Maciej on this.

Thanks,

-- Steve

> ---
>  arch/mips/Kconfig |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
> index 46cae2b..144e4b3 100644
> --- a/arch/mips/Kconfig
> +++ b/arch/mips/Kconfig
> @@ -9,6 +9,7 @@ config MIPS
>  	select HAVE_FUNCTION_TRACE_MCOUNT_TEST
>  	select HAVE_DYNAMIC_FTRACE
>  	select HAVE_FTRACE_MCOUNT_RECORD
> +	select HAVE_C_RECORDMCOUNT
>  	select HAVE_FUNCTION_GRAPH_TRACER
>  	select HAVE_KPROBES
>  	select HAVE_KRETPROBES



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] ftrace/MIPS: Add MIPS64 support for C version of recordmcount
  2010-10-27 13:32   ` Steven Rostedt
@ 2010-10-27 14:09     ` wu zhangjin
  2010-10-27 14:38       ` Steven Rostedt
  0 siblings, 1 reply; 11+ messages in thread
From: wu zhangjin @ 2010-10-27 14:09 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Ralf Baechle, linux-mips, linux-kernel, John Reiser,
	Maciej W. Rozycki, David Daney

On Wed, Oct 27, 2010 at 9:32 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 2010-10-27 at 18:59 +0800, Wu Zhangjin wrote:
>> From: Wu Zhangjin <wuzhangjin@gmail.com>
>
> The From above states that you wrote this. Did you modify what John and
> Maciej did? If so, please state clearly what each author has done, and
> how you modified it. If you did not write this, please change the From
> to the correct author.
>
> If you did modify it, then state something like this in the change log.
>
> Original version written by John Reiser <jreiser@BitWagon.com>
>
> Usage of "union mips_r_info" and the functions MIPS64_r_sym() and
> MIPS64_r_info() written by Maciej W. Rozycki <macro@linux-mips.org>
>

yeah, that is true above.

> Then state the changes you made.

I didn't change it, just added the comments in the patch log and tested it.

Should I resend the patch with the change log information about John and Maciej?

Regards,
Wu Zhangjin

>
>>
>> MIPS64 has 'weird' Elf64_Rel.r_info[1,2], which must be used instead of
>> the generic Elf64_Rel.r_info, otherwise, the C version of recordmcount
>> will not work for "segmentation fault".
>>
>> ----
>> [1] http://techpubs.sgi.com/library/manuals/4000/007-4658-001/pdf/007-4658-001.pdf
>> [2] arch/mips/include/asm/module.h
>>
>> Signed-off-by: John Reiser <jreiser@BitWagon.com>
>> Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
>> Tested-by: Wu Zhangjin <wuzhangjin@gmail.com>
>
> Again, if you made changes, you need a SoB yourself.
>
> Thanks!
>
> -- Steve
>
>
>> ---
>>  scripts/recordmcount.c |   41 +++++++++++++++++++++++++++++++++++++++++
>>  scripts/recordmcount.h |   34 ++++++++++++++++++++++++++++++----
>>  2 files changed, 71 insertions(+), 4 deletions(-)
>>
>> diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
>> index 26e1271..2d32b9c 100644
>> --- a/scripts/recordmcount.c
>> +++ b/scripts/recordmcount.c
>> @@ -217,6 +217,39 @@ is_mcounted_section_name(char const *const txtname)
>>  #define RECORD_MCOUNT_64
>>  #include "recordmcount.h"
>>
>> +/* 64-bit EM_MIPS has weird ELF64_Rela.r_info.
>> + * http://techpubs.sgi.com/library/manuals/4000/007-4658-001/pdf/007-4658-001.pdf
>> + * We interpret Table 29 Relocation Operation (Elf64_Rel, Elf64_Rela) [p.40]
>> + * to imply the order of the members; the spec does not say so.
>> + *   typedef unsigned char Elf64_Byte;
>> + * fails on MIPS64 because their <elf.h> already has it!
>> + */
>> +
>> +typedef uint8_t myElf64_Byte;                /* Type for a 8-bit quantity.  */
>> +
>> +union mips_r_info {
>> +     Elf64_Xword r_info;
>> +     struct {
>> +             Elf64_Word r_sym;               /* Symbol index.  */
>> +             myElf64_Byte r_ssym;            /* Special symbol.  */
>> +             myElf64_Byte r_type3;           /* Third relocation.  */
>> +             myElf64_Byte r_type2;           /* Second relocation.  */
>> +             myElf64_Byte r_type;            /* First relocation.  */
>> +     } r_mips;
>> +};
>> +
>> +static uint64_t MIPS64_r_sym(Elf64_Rel const *rp)
>> +{
>> +     return w(((union mips_r_info){ .r_info = rp->r_info }).r_mips.r_sym);
>> +}
>> +
>> +static void MIPS64_r_info(Elf64_Rel *const rp, unsigned sym, unsigned type)
>> +{
>> +     rp->r_info = ((union mips_r_info){
>> +             .r_mips = { .r_sym = w(sym), .r_type = type }
>> +     }).r_info;
>> +}
>> +
>>  static void
>>  do_file(char const *const fname)
>>  {
>> @@ -268,6 +301,7 @@ do_file(char const *const fname)
>>       case EM_386:     reltype = R_386_32;                   break;
>>       case EM_ARM:     reltype = R_ARM_ABS32;                break;
>>       case EM_IA_64:   reltype = R_IA64_IMM64;   gpfx = '_'; break;
>> +     case EM_MIPS:    /* reltype: e_class    */ gpfx = '_'; break;
>>       case EM_PPC:     reltype = R_PPC_ADDR32;   gpfx = '_'; break;
>>       case EM_PPC64:   reltype = R_PPC64_ADDR64; gpfx = '_'; break;
>>       case EM_S390:    /* reltype: e_class    */ gpfx = '_'; break;
>> @@ -291,6 +325,8 @@ do_file(char const *const fname)
>>               }
>>               if (EM_S390 == w2(ehdr->e_machine))
>>                       reltype = R_390_32;
>> +             if (EM_MIPS == w2(ehdr->e_machine))
>> +                     reltype = R_MIPS_32;
>>               do32(ehdr, fname, reltype);
>>       } break;
>>       case ELFCLASS64: {
>> @@ -303,6 +339,11 @@ do_file(char const *const fname)
>>               }
>>               if (EM_S390 == w2(ghdr->e_machine))
>>                       reltype = R_390_64;
>> +             if (EM_MIPS == w2(ghdr->e_machine)) {
>> +                     reltype = R_MIPS_64;
>> +                     Elf64_r_sym = MIPS64_r_sym;
>> +                     Elf64_r_info = MIPS64_r_info;
>> +             }
>>               do64(ghdr, fname, reltype);
>>       } break;
>>       }  /* end switch */
>> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
>> index 7f39d09..190fd18 100644
>> --- a/scripts/recordmcount.h
>> +++ b/scripts/recordmcount.h
>> @@ -31,8 +31,12 @@
>>  #undef Elf_Rela
>>  #undef Elf_Sym
>>  #undef ELF_R_SYM
>> +#undef Elf_r_sym
>>  #undef ELF_R_INFO
>> +#undef Elf_r_info
>>  #undef ELF_ST_BIND
>> +#undef fn_ELF_R_SYM
>> +#undef fn_ELF_R_INFO
>>  #undef uint_t
>>  #undef _w
>>  #undef _align
>> @@ -52,8 +56,12 @@
>>  # define Elf_Rela            Elf64_Rela
>>  # define Elf_Sym             Elf64_Sym
>>  # define ELF_R_SYM           ELF64_R_SYM
>> +# define Elf_r_sym           Elf64_r_sym
>>  # define ELF_R_INFO          ELF64_R_INFO
>> +# define Elf_r_info          Elf64_r_info
>>  # define ELF_ST_BIND         ELF64_ST_BIND
>> +# define fn_ELF_R_SYM                fn_ELF64_R_SYM
>> +# define fn_ELF_R_INFO               fn_ELF64_R_INFO
>>  # define uint_t                      uint64_t
>>  # define _w                  w8
>>  # define _align                      7u
>> @@ -72,14 +80,32 @@
>>  # define Elf_Rela            Elf32_Rela
>>  # define Elf_Sym             Elf32_Sym
>>  # define ELF_R_SYM           ELF32_R_SYM
>> +# define Elf_r_sym           Elf32_r_sym
>>  # define ELF_R_INFO          ELF32_R_INFO
>> +# define Elf_r_info          Elf32_r_info
>>  # define ELF_ST_BIND         ELF32_ST_BIND
>> +# define fn_ELF_R_SYM                fn_ELF32_R_SYM
>> +# define fn_ELF_R_INFO               fn_ELF32_R_INFO
>>  # define uint_t                      uint32_t
>>  # define _w                  w
>>  # define _align                      3u
>>  # define _size                       4
>>  #endif
>>
>> +/* Functions and pointers that 64-bit EM_MIPS can override. */
>> +static uint_t fn_ELF_R_SYM(Elf_Rel const *rp)
>> +{
>> +     return ELF_R_SYM(_w(rp->r_info));
>> +}
>> +static uint_t (*Elf_r_sym)(Elf_Rel const *rp) = fn_ELF_R_SYM;
>> +
>> +static void fn_ELF_R_INFO(Elf_Rel *const rp, unsigned sym, unsigned type)
>> +{
>> +     rp->r_info = ELF_R_INFO(sym, type);
>> +}
>> +static void (*Elf_r_info)(Elf_Rel *const rp, unsigned sym, unsigned type) = fn_ELF_R_INFO;
>> +
>> +
>>  /* Append the new shstrtab, Elf_Shdr[], __mcount_loc and its relocations. */
>>  static void append_func(Elf_Ehdr *const ehdr,
>>                       Elf_Shdr *const shstr,
>> @@ -197,22 +223,22 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
>>       for (t = nrel; t; --t) {
>>               if (!mcountsym) {
>>                       Elf_Sym const *const symp =
>> -                             &sym0[ELF_R_SYM(_w(relp->r_info))];
>> +                             &sym0[Elf_r_sym(relp)];
>>                       char const *symname = &str0[w(symp->st_name)];
>>
>>                       if ('.' == symname[0])
>>                               ++symname;  /* ppc64 hack */
>>                       if (0 == strcmp((('_' == gpfx) ? "_mcount" : "mcount"),
>>                                       symname))
>> -                             mcountsym = ELF_R_SYM(_w(relp->r_info));
>> +                             mcountsym = Elf_r_sym(relp);
>>               }
>>
>> -             if (mcountsym == ELF_R_SYM(_w(relp->r_info))) {
>> +             if (mcountsym == Elf_r_sym(relp)) {
>>                       uint_t const addend = _w(_w(relp->r_offset) - recval);
>>
>>                       mrelp->r_offset = _w(offbase
>>                               + ((void *)mlocp - (void *)mloc0));
>> -                     mrelp->r_info = _w(ELF_R_INFO(recsym, reltype));
>> +                     Elf_r_info(mrelp, recsym, reltype);
>>                       if (sizeof(Elf_Rela) == rel_entsize) {
>>                               ((Elf_Rela *)mrelp)->r_addend = addend;
>>                               *mlocp++ = 0;
>
>
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] ftrace/MIPS: Add MIPS64 support for C version of recordmcount
  2010-10-27 14:09     ` wu zhangjin
@ 2010-10-27 14:38       ` Steven Rostedt
  2010-10-27 15:59         ` wu zhangjin
  0 siblings, 1 reply; 11+ messages in thread
From: Steven Rostedt @ 2010-10-27 14:38 UTC (permalink / raw)
  To: wu zhangjin
  Cc: Ralf Baechle, linux-mips, linux-kernel, John Reiser,
	Maciej W. Rozycki, David Daney

On Wed, 2010-10-27 at 22:09 +0800, wu zhangjin wrote:
> On Wed, Oct 27, 2010 at 9:32 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> > On Wed, 2010-10-27 at 18:59 +0800, Wu Zhangjin wrote:
> >> From: Wu Zhangjin <wuzhangjin@gmail.com>
> >
> > The From above states that you wrote this. Did you modify what John and
> > Maciej did? If so, please state clearly what each author has done, and
> > how you modified it. If you did not write this, please change the From
> > to the correct author.
> >
> > If you did modify it, then state something like this in the change log.
> >
> > Original version written by John Reiser <jreiser@BitWagon.com>
> >
> > Usage of "union mips_r_info" and the functions MIPS64_r_sym() and
> > MIPS64_r_info() written by Maciej W. Rozycki <macro@linux-mips.org>
> >
> 
> yeah, that is true above.
> 
> > Then state the changes you made.
> 
> I didn't change it, just added the comments in the patch log and tested it.
> 
> Should I resend the patch with the change log information about John and Maciej?
> 

I can make the changes.

Then the patch is originally John's, right?

I'll make him the author, with the comment about Maciej, and keep you as
the Tested-by.

-- Steve



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] ftrace/MIPS: Add MIPS64 support for C version of recordmcount
  2010-10-27 14:38       ` Steven Rostedt
@ 2010-10-27 15:59         ` wu zhangjin
  0 siblings, 0 replies; 11+ messages in thread
From: wu zhangjin @ 2010-10-27 15:59 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Ralf Baechle, linux-mips, linux-kernel, John Reiser,
	Maciej W. Rozycki, David Daney

On Wed, Oct 27, 2010 at 10:38 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Wed, 2010-10-27 at 22:09 +0800, wu zhangjin wrote:
>> On Wed, Oct 27, 2010 at 9:32 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
>> > On Wed, 2010-10-27 at 18:59 +0800, Wu Zhangjin wrote:
>> >> From: Wu Zhangjin <wuzhangjin@gmail.com>
>> >
>> > The From above states that you wrote this. Did you modify what John and
>> > Maciej did? If so, please state clearly what each author has done, and
>> > how you modified it. If you did not write this, please change the From
>> > to the correct author.
>> >
>> > If you did modify it, then state something like this in the change log.
>> >
>> > Original version written by John Reiser <jreiser@BitWagon.com>
>> >
>> > Usage of "union mips_r_info" and the functions MIPS64_r_sym() and
>> > MIPS64_r_info() written by Maciej W. Rozycki <macro@linux-mips.org>
>> >
>>
>> yeah, that is true above.
>>
>> > Then state the changes you made.
>>
>> I didn't change it, just added the comments in the patch log and tested it.
>>
>> Should I resend the patch with the change log information about John and Maciej?
>>
>
> I can make the changes.
>

ok, thanks.

> Then the patch is originally John's, right?
>

Yes, John write it.

> I'll make him the author, with the comment about Maciej, and keep you as
> the Tested-by.

ok, Macjej has simplified the r_info of MIPS there, so perhaps we need
to add SoB for him and he told me to add that line.

For me, Tested-by: is ok.

Regards,
Wu Zhangjin

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] ftrace/MIPS: Enable C Version of recordmcount
  2010-10-27 13:36   ` Steven Rostedt
@ 2010-10-30 13:28     ` Maciej W. Rozycki
  0 siblings, 0 replies; 11+ messages in thread
From: Maciej W. Rozycki @ 2010-10-30 13:28 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: Wu Zhangjin, Ralf Baechle, linux-mips, linux-kernel, John Reiser,
	David Daney

On Wed, 27 Oct 2010, Steven Rostedt wrote:

> On Wed, 2010-10-27 at 18:59 +0800, Wu Zhangjin wrote:
> > From: Wu Zhangjin <wuzhangjin@gmail.com>
> > 
> > Selects HAVE_C_RECORDMCOUNT to use the C version of the recordmcount
> > intead of the old Perl Version of recordmcount.
> > 
> > Signed-off-by: Wu Zhangjin <wuzhangjin@gmail.com>
> 
> I'd like to get an Acked-by from Ralf and Maciej on this.

Acked-by: Maciej W. Rozycki <macro@linux-mips.org>

 I have looked through it and spotted nothing obviously wrong, but I can't 
afford any further testing, especially at run time, sorry.

 One point to note -- it seems to me the code currently assumes a 32-bit 
model, i.e. the use of the -msym32 GCC option suitable for a 64-bit kernel 
loaded to a CKSEG0 address (cf. KBUILD_SYM32 in arch/mips/Makefile), tools 
support permitting.  That means it does not (correctly) support kernels 
loaded to an XPHYS address as required for some platforms (or otherwise 
chosen for testing such a configuration; modulo some processor errata and 
bootloader limitations, it is generally OK to run the kernel from XPHYS on 
64-bit chips even if the entire RAM fits into CKSEG0).

 For the avoidance of doubt -- I'm just mentioning it to emphasise a 
possible future direction for improvement of this code -- not an objection 
against this submission, that is certainly a good foundation for future 
development.

 Thanks to everybody involved.

  Maciej

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/3] ftrace/MIPS: Add MIPS64 support for C version of recordmcount
  2010-10-28 14:05 [PATCH 0/3] [GIT PULL] ftrace: recordmcount.c updates for MIPS Steven Rostedt
@ 2010-10-28 14:05 ` Steven Rostedt
  0 siblings, 0 replies; 11+ messages in thread
From: Steven Rostedt @ 2010-10-28 14:05 UTC (permalink / raw)
  To: linux-kernel
  Cc: Ingo Molnar, Andrew Morton, Frederic Weisbecker, Wu Zhangjin,
	Ralf Baechle, John Reiser, Maciej W. Rozycki

[-- Attachment #1: 0001-ftrace-MIPS-Add-MIPS64-support-for-C-version-of-reco.patch --]
[-- Type: text/plain, Size: 6291 bytes --]

From: John Reiser <jreiser@BitWagon.com>

MIPS64 has 'weird' Elf64_Rel.r_info[1,2], which must be used instead of
the generic Elf64_Rel.r_info, otherwise, the C version of recordmcount
will not work for "segmentation fault".

Usage of "union mips_r_info" and the functions MIPS64_r_sym() and
MIPS64_r_info() written by Maciej W. Rozycki <macro@linux-mips.org>

----
[1] http://techpubs.sgi.com/library/manuals/4000/007-4658-001/pdf/007-4658-001.pdf
[2] arch/mips/include/asm/module.h

Tested-by: Wu Zhangjin <wuzhangjin@gmail.com>
Acked-by: Ralf Baechle <ralf@linux-mips.org>
Signed-off-by: John Reiser <jreiser@BitWagon.com>
Signed-off-by: Maciej W. Rozycki <macro@linux-mips.org>
LKML-Reference: <AANLkTinwXjLAYACUfhLYaocHD_vBbiErLN3NjwN8JqSy@mail.gmail.com>
LKML-Reference: <910dc2d5ae1ed042df4f96815fe4a433078d1c2a.1288176026.git.wuzhangjin@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.c |   41 +++++++++++++++++++++++++++++++++++++++++
 scripts/recordmcount.h |   34 ++++++++++++++++++++++++++++++----
 2 files changed, 71 insertions(+), 4 deletions(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 26e1271..2d32b9c 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -217,6 +217,39 @@ is_mcounted_section_name(char const *const txtname)
 #define RECORD_MCOUNT_64
 #include "recordmcount.h"
 
+/* 64-bit EM_MIPS has weird ELF64_Rela.r_info.
+ * http://techpubs.sgi.com/library/manuals/4000/007-4658-001/pdf/007-4658-001.pdf
+ * We interpret Table 29 Relocation Operation (Elf64_Rel, Elf64_Rela) [p.40]
+ * to imply the order of the members; the spec does not say so.
+ *	typedef unsigned char Elf64_Byte;
+ * fails on MIPS64 because their <elf.h> already has it!
+ */
+
+typedef uint8_t myElf64_Byte;		/* Type for a 8-bit quantity.  */
+
+union mips_r_info {
+	Elf64_Xword r_info;
+	struct {
+		Elf64_Word r_sym;		/* Symbol index.  */
+		myElf64_Byte r_ssym;		/* Special symbol.  */
+		myElf64_Byte r_type3;		/* Third relocation.  */
+		myElf64_Byte r_type2;		/* Second relocation.  */
+		myElf64_Byte r_type;		/* First relocation.  */
+	} r_mips;
+};
+
+static uint64_t MIPS64_r_sym(Elf64_Rel const *rp)
+{
+	return w(((union mips_r_info){ .r_info = rp->r_info }).r_mips.r_sym);
+}
+
+static void MIPS64_r_info(Elf64_Rel *const rp, unsigned sym, unsigned type)
+{
+	rp->r_info = ((union mips_r_info){
+		.r_mips = { .r_sym = w(sym), .r_type = type }
+	}).r_info;
+}
+
 static void
 do_file(char const *const fname)
 {
@@ -268,6 +301,7 @@ do_file(char const *const fname)
 	case EM_386:	 reltype = R_386_32;                   break;
 	case EM_ARM:	 reltype = R_ARM_ABS32;                break;
 	case EM_IA_64:	 reltype = R_IA64_IMM64;   gpfx = '_'; break;
+	case EM_MIPS:	 /* reltype: e_class    */ gpfx = '_'; break;
 	case EM_PPC:	 reltype = R_PPC_ADDR32;   gpfx = '_'; break;
 	case EM_PPC64:	 reltype = R_PPC64_ADDR64; gpfx = '_'; break;
 	case EM_S390:    /* reltype: e_class    */ gpfx = '_'; break;
@@ -291,6 +325,8 @@ do_file(char const *const fname)
 		}
 		if (EM_S390 == w2(ehdr->e_machine))
 			reltype = R_390_32;
+		if (EM_MIPS == w2(ehdr->e_machine))
+			reltype = R_MIPS_32;
 		do32(ehdr, fname, reltype);
 	} break;
 	case ELFCLASS64: {
@@ -303,6 +339,11 @@ do_file(char const *const fname)
 		}
 		if (EM_S390 == w2(ghdr->e_machine))
 			reltype = R_390_64;
+		if (EM_MIPS == w2(ghdr->e_machine)) {
+			reltype = R_MIPS_64;
+			Elf64_r_sym = MIPS64_r_sym;
+			Elf64_r_info = MIPS64_r_info;
+		}
 		do64(ghdr, fname, reltype);
 	} break;
 	}  /* end switch */
diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 7f39d09..190fd18 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -31,8 +31,12 @@
 #undef Elf_Rela
 #undef Elf_Sym
 #undef ELF_R_SYM
+#undef Elf_r_sym
 #undef ELF_R_INFO
+#undef Elf_r_info
 #undef ELF_ST_BIND
+#undef fn_ELF_R_SYM
+#undef fn_ELF_R_INFO
 #undef uint_t
 #undef _w
 #undef _align
@@ -52,8 +56,12 @@
 # define Elf_Rela		Elf64_Rela
 # define Elf_Sym		Elf64_Sym
 # define ELF_R_SYM		ELF64_R_SYM
+# define Elf_r_sym		Elf64_r_sym
 # define ELF_R_INFO		ELF64_R_INFO
+# define Elf_r_info		Elf64_r_info
 # define ELF_ST_BIND		ELF64_ST_BIND
+# define fn_ELF_R_SYM		fn_ELF64_R_SYM
+# define fn_ELF_R_INFO		fn_ELF64_R_INFO
 # define uint_t			uint64_t
 # define _w			w8
 # define _align			7u
@@ -72,14 +80,32 @@
 # define Elf_Rela		Elf32_Rela
 # define Elf_Sym		Elf32_Sym
 # define ELF_R_SYM		ELF32_R_SYM
+# define Elf_r_sym		Elf32_r_sym
 # define ELF_R_INFO		ELF32_R_INFO
+# define Elf_r_info		Elf32_r_info
 # define ELF_ST_BIND		ELF32_ST_BIND
+# define fn_ELF_R_SYM		fn_ELF32_R_SYM
+# define fn_ELF_R_INFO		fn_ELF32_R_INFO
 # define uint_t			uint32_t
 # define _w			w
 # define _align			3u
 # define _size			4
 #endif
 
+/* Functions and pointers that 64-bit EM_MIPS can override. */
+static uint_t fn_ELF_R_SYM(Elf_Rel const *rp)
+{
+	return ELF_R_SYM(_w(rp->r_info));
+}
+static uint_t (*Elf_r_sym)(Elf_Rel const *rp) = fn_ELF_R_SYM;
+
+static void fn_ELF_R_INFO(Elf_Rel *const rp, unsigned sym, unsigned type)
+{
+	rp->r_info = ELF_R_INFO(sym, type);
+}
+static void (*Elf_r_info)(Elf_Rel *const rp, unsigned sym, unsigned type) = fn_ELF_R_INFO;
+
+
 /* Append the new shstrtab, Elf_Shdr[], __mcount_loc and its relocations. */
 static void append_func(Elf_Ehdr *const ehdr,
 			Elf_Shdr *const shstr,
@@ -197,22 +223,22 @@ static uint_t *sift_rel_mcount(uint_t *mlocp,
 	for (t = nrel; t; --t) {
 		if (!mcountsym) {
 			Elf_Sym const *const symp =
-				&sym0[ELF_R_SYM(_w(relp->r_info))];
+				&sym0[Elf_r_sym(relp)];
 			char const *symname = &str0[w(symp->st_name)];
 
 			if ('.' == symname[0])
 				++symname;  /* ppc64 hack */
 			if (0 == strcmp((('_' == gpfx) ? "_mcount" : "mcount"),
 					symname))
-				mcountsym = ELF_R_SYM(_w(relp->r_info));
+				mcountsym = Elf_r_sym(relp);
 		}
 
-		if (mcountsym == ELF_R_SYM(_w(relp->r_info))) {
+		if (mcountsym == Elf_r_sym(relp)) {
 			uint_t const addend = _w(_w(relp->r_offset) - recval);
 
 			mrelp->r_offset = _w(offbase
 				+ ((void *)mlocp - (void *)mloc0));
-			mrelp->r_info = _w(ELF_R_INFO(recsym, reltype));
+			Elf_r_info(mrelp, recsym, reltype);
 			if (sizeof(Elf_Rela) == rel_entsize) {
 				((Elf_Rela *)mrelp)->r_addend = addend;
 				*mlocp++ = 0;
-- 
1.7.1



^ permalink raw reply related	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2010-10-30 13:28 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-27 10:59 [PATCH 0/3] Add C version of recordmcount for MIPS Wu Zhangjin
2010-10-27 10:59 ` [PATCH 1/3] ftrace/MIPS: Add MIPS64 support for C version of recordmcount Wu Zhangjin
2010-10-27 13:32   ` Steven Rostedt
2010-10-27 14:09     ` wu zhangjin
2010-10-27 14:38       ` Steven Rostedt
2010-10-27 15:59         ` wu zhangjin
2010-10-27 10:59 ` [PATCH 2/3] ftrace/MIPS: Add module " Wu Zhangjin
2010-10-27 10:59 ` [PATCH 3/3] ftrace/MIPS: Enable C Version " Wu Zhangjin
2010-10-27 13:36   ` Steven Rostedt
2010-10-30 13:28     ` Maciej W. Rozycki
2010-10-28 14:05 [PATCH 0/3] [GIT PULL] ftrace: recordmcount.c updates for MIPS Steven Rostedt
2010-10-28 14:05 ` [PATCH 1/3] ftrace/MIPS: Add MIPS64 support for C version of recordmcount Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).