[patch 0/4] [RFC] mcount address adjustment

[patch 0/4] [RFC] mcount address adjustment

[Martin Schwidefsky]

This series of patches aims to simplify the mcount address adjustment for
ftrace. The frace code wants to know the start address of each function,
not the address of the relocation against the mcount symbol which have
been recorded to the __mcount_loc section. The ftrace_call_adjust function
is used to calculate the start address of the function from the mcount
relocation at runtime.

For x86, ia64 and s390 the adjustment is a constant offset. There is no
need to do the adjustment at runtime, it can be done by recordmcount at
compile time. Blackfin already does this with the mcount_adjust variable
in the pearl version of recordmcount. After teaching the C version of
recordmcount about mcount_adjust x86, ia64 and s390 can be converted to
a nop ftrace_call_adjust function.

That leaves arm as the last remaining architecture with a non trivial
ftrace_call_adjust function. There the least significant bit is removed
from the address with an and operation. The comment says this is done
for Thumb-2. This implies that for Thumb-1 the offset is 0 and for
Thumb-2 the offset is -1, correct? If there is a way to distinguish
the two targets in recordmcount at compile time we could convert arm
as well. Which would allow us to remove the ftrace_call_adjust function.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

[patch 0/4] [RFC] mcount address adjustment

[Martin Schwidefsky]

This series of patches aims to simplify the mcount address adjustment for
ftrace. The frace code wants to know the start address of each function,
not the address of the relocation against the mcount symbol which have
been recorded to the __mcount_loc section. The ftrace_call_adjust function
is used to calculate the start address of the function from the mcount
relocation at runtime.

For x86, ia64 and s390 the adjustment is a constant offset. There is no
need to do the adjustment at runtime, it can be done by recordmcount at
compile time. Blackfin already does this with the mcount_adjust variable
in the pearl version of recordmcount. After teaching the C version of
recordmcount about mcount_adjust x86, ia64 and s390 can be converted to
a nop ftrace_call_adjust function.

That leaves arm as the last remaining architecture with a non trivial
ftrace_call_adjust function. There the least significant bit is removed
from the address with an and operation. The comment says this is done
for Thumb-2. This implies that for Thumb-1 the offset is 0 and for
Thumb-2 the offset is -1, correct? If there is a way to distinguish
the two targets in recordmcount at compile time we could convert arm
as well. Which would allow us to remove the ftrace_call_adjust function.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

[patch 1/4] recordmcount mcount address adjustment

[Martin Schwidefsky]

[-- Attachment #1: 300-mcount-adjust.diff --]
[-- Type: text/plain, Size: 2222 bytes --]

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Introduce mcount_adjust{,_32,_64} to the C implementation of
recordmcount analog to $mcount_adjust in the perl script.
The adjustment is added to the address of the relocations
against the mcount symbol. If this adjustment is done by
recordmcount at compile time the ftrace_call_adjust function
can be turned into a nop.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---


 scripts/recordmcount.h |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Index: test-2.6/scripts/recordmcount.h
===================================================================
--- test-2.6.orig/scripts/recordmcount.h	2011-01-07 09:17:19.000000000 +0100
+++ test-2.6/scripts/recordmcount.h	2011-05-03 09:38:56.781445980 +0200
@@ -22,6 +22,7 @@
 #undef is_fake_mcount
 #undef fn_is_fake_mcount
 #undef MIPS_is_fake_mcount
+#undef mcount_adjust
 #undef sift_rel_mcount
 #undef find_secsym_ndx
 #undef __has_rel_mcount
@@ -57,6 +58,7 @@
 # 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 mcount_adjust		mcount_adjust_64
 # define Elf_Addr		Elf64_Addr
 # define Elf_Ehdr		Elf64_Ehdr
 # define Elf_Shdr		Elf64_Shdr
@@ -85,6 +87,7 @@
 # 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 mcount_adjust		mcount_adjust_32
 # define Elf_Addr		Elf32_Addr
 # define Elf_Ehdr		Elf32_Ehdr
 # define Elf_Shdr		Elf32_Shdr
@@ -123,6 +126,8 @@
 }
 static void (*Elf_r_info)(Elf_Rel *const rp, unsigned sym, unsigned type) = fn_ELF_R_INFO;
 
+static int mcount_adjust = 0;
+
 /*
  * 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
@@ -285,8 +290,8 @@
 		}
 
 		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
-			uint_t const addend = _w(_w(relp->r_offset) - recval);
-
+			uint_t const addend =
+				_w(_w(relp->r_offset) - recval + mcount_adjust);
 			mrelp->r_offset = _w(offbase
 				+ ((void *)mlocp - (void *)mloc0));
 			Elf_r_info(mrelp, recsym, reltype);

[patch 1/4] recordmcount mcount address adjustment

[Martin Schwidefsky]

[-- Attachment #1: 300-mcount-adjust.diff --]
[-- Type: text/plain, Size: 2223 bytes --]

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Introduce mcount_adjust{,_32,_64} to the C implementation of
recordmcount analog to $mcount_adjust in the perl script.
The adjustment is added to the address of the relocations
against the mcount symbol. If this adjustment is done by
recordmcount at compile time the ftrace_call_adjust function
can be turned into a nop.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---


 scripts/recordmcount.h |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)
Index: test-2.6/scripts/recordmcount.h
===================================================================
--- test-2.6.orig/scripts/recordmcount.h	2011-01-07 09:17:19.000000000 +0100
+++ test-2.6/scripts/recordmcount.h	2011-05-03 09:38:56.781445980 +0200
@@ -22,6 +22,7 @@
 #undef is_fake_mcount
 #undef fn_is_fake_mcount
 #undef MIPS_is_fake_mcount
+#undef mcount_adjust
 #undef sift_rel_mcount
 #undef find_secsym_ndx
 #undef __has_rel_mcount
@@ -57,6 +58,7 @@
 # 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 mcount_adjust		mcount_adjust_64
 # define Elf_Addr		Elf64_Addr
 # define Elf_Ehdr		Elf64_Ehdr
 # define Elf_Shdr		Elf64_Shdr
@@ -85,6 +87,7 @@
 # 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 mcount_adjust		mcount_adjust_32
 # define Elf_Addr		Elf32_Addr
 # define Elf_Ehdr		Elf32_Ehdr
 # define Elf_Shdr		Elf32_Shdr
@@ -123,6 +126,8 @@
 }
 static void (*Elf_r_info)(Elf_Rel *const rp, unsigned sym, unsigned type) = fn_ELF_R_INFO;
 
+static int mcount_adjust = 0;
+
 /*
  * 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
@@ -285,8 +290,8 @@
 		}
 
 		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
-			uint_t const addend = _w(_w(relp->r_offset) - recval);
-
+			uint_t const addend =
+				_w(_w(relp->r_offset) - recval + mcount_adjust);
 			mrelp->r_offset = _w(offbase
 				+ ((void *)mlocp - (void *)mloc0));
 			Elf_r_info(mrelp, recsym, reltype);

[patch 2/4] x86 mcount offset calculation

[Martin Schwidefsky]

[-- Attachment #1: 301-mcount-adjust-x86.diff --]
[-- Type: text/plain, Size: 2790 bytes --]

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Do the mcount offset adjustment in the recordmcount.pl/recordmcount.[ch]
at compile time and not in ftrace_call_adjust at run time.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---


 arch/x86/include/asm/ftrace.h |    7 +++----
 scripts/recordmcount.c        |   10 ++++++++--
 scripts/recordmcount.pl       |    2 ++
 3 files changed, 13 insertions(+), 6 deletions(-)
Index: test-2.6/arch/x86/include/asm/ftrace.h
===================================================================
--- test-2.6.orig/arch/x86/include/asm/ftrace.h	2009-09-12 09:47:14.000000000 +0200
+++ test-2.6/arch/x86/include/asm/ftrace.h	2011-05-03 09:39:02.145472579 +0200
@@ -38,11 +38,10 @@
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
 	/*
-	 * call mcount is "e8 <4 byte offset>"
-	 * The addr points to the 4 byte offset and the caller of this
-	 * function wants the pointer to e8. Simply subtract one.
+	 * addr is the address of the mcount call instruction.
+	 * recordmcount does the necessary offset calculation.
 	 */
-	return addr - 1;
+	return addr;
 }
 
 #ifdef CONFIG_DYNAMIC_FTRACE
Index: test-2.6/scripts/recordmcount.c
===================================================================
--- test-2.6.orig/scripts/recordmcount.c	2011-03-16 09:26:43.006913981 +0100
+++ test-2.6/scripts/recordmcount.c	2011-05-03 09:39:02.145472579 +0200
@@ -300,7 +300,10 @@
 			w2(ehdr->e_machine), fname);
 		fail_file();
 	} break;
-	case EM_386:	 reltype = R_386_32;                   break;
+	case EM_386:
+		reltype = R_386_32;
+		mcount_adjust_32 = -1;
+		break;
 	case EM_ARM:	 reltype = R_ARM_ABS32;
 			 altmcount = "__gnu_mcount_nc";
 			 break;
@@ -311,7 +314,10 @@
 	case EM_S390:    /* reltype: e_class    */ gpfx = '_'; break;
 	case EM_SH:	 reltype = R_SH_DIR32;                 break;
 	case EM_SPARCV9: reltype = R_SPARC_64;     gpfx = '_'; break;
-	case EM_X86_64:	 reltype = R_X86_64_64;                break;
+	case EM_X86_64:
+		reltype = R_X86_64_64;
+		mcount_adjust_64 = -1;
+		break;
 	}  /* end switch */
 
 	switch (ehdr->e_ident[EI_CLASS]) {
Index: test-2.6/scripts/recordmcount.pl
===================================================================
--- test-2.6.orig/scripts/recordmcount.pl	2011-03-16 09:26:43.006913981 +0100
+++ test-2.6/scripts/recordmcount.pl	2011-05-03 09:39:02.145472579 +0200
@@ -222,6 +222,7 @@
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$";
     $type = ".quad";
     $alignment = 8;
+    $mcount_adjust = -1;
 
     # force flags for this arch
     $ld .= " -m elf_x86_64";
@@ -231,6 +232,7 @@
 
 } elsif ($arch eq "i386") {
     $alignment = 4;
+    $mcount_adjust = -1;
 
     # force flags for this arch
     $ld .= " -m elf_i386";

[patch 2/4] x86 mcount offset calculation

[Martin Schwidefsky]

[-- Attachment #1: 301-mcount-adjust-x86.diff --]
[-- Type: text/plain, Size: 2791 bytes --]

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Do the mcount offset adjustment in the recordmcount.pl/recordmcount.[ch]
at compile time and not in ftrace_call_adjust at run time.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---


 arch/x86/include/asm/ftrace.h |    7 +++----
 scripts/recordmcount.c        |   10 ++++++++--
 scripts/recordmcount.pl       |    2 ++
 3 files changed, 13 insertions(+), 6 deletions(-)
Index: test-2.6/arch/x86/include/asm/ftrace.h
===================================================================
--- test-2.6.orig/arch/x86/include/asm/ftrace.h	2009-09-12 09:47:14.000000000 +0200
+++ test-2.6/arch/x86/include/asm/ftrace.h	2011-05-03 09:39:02.145472579 +0200
@@ -38,11 +38,10 @@
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
 	/*
-	 * call mcount is "e8 <4 byte offset>"
-	 * The addr points to the 4 byte offset and the caller of this
-	 * function wants the pointer to e8. Simply subtract one.
+	 * addr is the address of the mcount call instruction.
+	 * recordmcount does the necessary offset calculation.
 	 */
-	return addr - 1;
+	return addr;
 }
 
 #ifdef CONFIG_DYNAMIC_FTRACE
Index: test-2.6/scripts/recordmcount.c
===================================================================
--- test-2.6.orig/scripts/recordmcount.c	2011-03-16 09:26:43.006913981 +0100
+++ test-2.6/scripts/recordmcount.c	2011-05-03 09:39:02.145472579 +0200
@@ -300,7 +300,10 @@
 			w2(ehdr->e_machine), fname);
 		fail_file();
 	} break;
-	case EM_386:	 reltype = R_386_32;                   break;
+	case EM_386:
+		reltype = R_386_32;
+		mcount_adjust_32 = -1;
+		break;
 	case EM_ARM:	 reltype = R_ARM_ABS32;
 			 altmcount = "__gnu_mcount_nc";
 			 break;
@@ -311,7 +314,10 @@
 	case EM_S390:    /* reltype: e_class    */ gpfx = '_'; break;
 	case EM_SH:	 reltype = R_SH_DIR32;                 break;
 	case EM_SPARCV9: reltype = R_SPARC_64;     gpfx = '_'; break;
-	case EM_X86_64:	 reltype = R_X86_64_64;                break;
+	case EM_X86_64:
+		reltype = R_X86_64_64;
+		mcount_adjust_64 = -1;
+		break;
 	}  /* end switch */
 
 	switch (ehdr->e_ident[EI_CLASS]) {
Index: test-2.6/scripts/recordmcount.pl
===================================================================
--- test-2.6.orig/scripts/recordmcount.pl	2011-03-16 09:26:43.006913981 +0100
+++ test-2.6/scripts/recordmcount.pl	2011-05-03 09:39:02.145472579 +0200
@@ -222,6 +222,7 @@
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$";
     $type = ".quad";
     $alignment = 8;
+    $mcount_adjust = -1;
 
     # force flags for this arch
     $ld .= " -m elf_x86_64";
@@ -231,6 +232,7 @@
 
 } elsif ($arch eq "i386") {
     $alignment = 4;
+    $mcount_adjust = -1;
 
     # force flags for this arch
     $ld .= " -m elf_i386";

[patch 3/4] ia64 mcount offset calculation

[Martin Schwidefsky]

[-- Attachment #1: 302-mcount-adjust-ia64.diff --]
[-- Type: text/plain, Size: 2241 bytes --]

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Do the mcount offset adjustment in the recordmcount.pl/recordmcount.[ch]
at compile time and not in ftrace_call_adjust at run time.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---


 arch/ia64/include/asm/ftrace.h |    7 +++++--
 scripts/recordmcount.c         |    7 ++++++-
 scripts/recordmcount.pl        |    1 +
 3 files changed, 12 insertions(+), 3 deletions(-)
Index: test-2.6/arch/ia64/include/asm/ftrace.h
===================================================================
--- test-2.6.orig/arch/ia64/include/asm/ftrace.h	2010-01-11 10:58:31.000000000 +0100
+++ test-2.6/arch/ia64/include/asm/ftrace.h	2011-05-03 09:39:04.929486388 +0200
@@ -14,8 +14,11 @@
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
-	/* second bundle, insn 2 */
-	return addr - 0x12;
+	/*
+	 * addr is the address of the mcount call instruction.
+	 * recordmcount does the necessary offset calculation.
+	 */
+	return addr;
 }
 
 struct dyn_arch_ftrace {
Index: test-2.6/scripts/recordmcount.c
===================================================================
--- test-2.6.orig/scripts/recordmcount.c	2011-05-03 09:39:02.145472579 +0200
+++ test-2.6/scripts/recordmcount.c	2011-05-03 09:39:04.929486388 +0200
@@ -307,7 +307,12 @@
 	case EM_ARM:	 reltype = R_ARM_ABS32;
 			 altmcount = "__gnu_mcount_nc";
 			 break;
-	case EM_IA_64:	 reltype = R_IA64_IMM64;   gpfx = '_'; break;
+	case EM_IA_64:
+		reltype = R_IA64_IMM64;
+		/* Adjust relocation to second bundle, insn 2 */
+		mcount_adjust_32 = -18;
+		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;
Index: test-2.6/scripts/recordmcount.pl
===================================================================
--- test-2.6.orig/scripts/recordmcount.pl	2011-05-03 09:39:02.145472579 +0200
+++ test-2.6/scripts/recordmcount.pl	2011-05-03 09:39:04.929486388 +0200
@@ -278,6 +278,7 @@
 
 } elsif ($arch eq "ia64") {
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s_mcount\$";
+    $mcount_adjust = -18;
     $type = "data8";
 
     if ($is_module eq "0") {

[patch 3/4] ia64 mcount offset calculation

[Martin Schwidefsky]

[-- Attachment #1: 302-mcount-adjust-ia64.diff --]
[-- Type: text/plain, Size: 2242 bytes --]

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Do the mcount offset adjustment in the recordmcount.pl/recordmcount.[ch]
at compile time and not in ftrace_call_adjust at run time.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---


 arch/ia64/include/asm/ftrace.h |    7 +++++--
 scripts/recordmcount.c         |    7 ++++++-
 scripts/recordmcount.pl        |    1 +
 3 files changed, 12 insertions(+), 3 deletions(-)
Index: test-2.6/arch/ia64/include/asm/ftrace.h
===================================================================
--- test-2.6.orig/arch/ia64/include/asm/ftrace.h	2010-01-11 10:58:31.000000000 +0100
+++ test-2.6/arch/ia64/include/asm/ftrace.h	2011-05-03 09:39:04.929486388 +0200
@@ -14,8 +14,11 @@
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
-	/* second bundle, insn 2 */
-	return addr - 0x12;
+	/*
+	 * addr is the address of the mcount call instruction.
+	 * recordmcount does the necessary offset calculation.
+	 */
+	return addr;
 }
 
 struct dyn_arch_ftrace {
Index: test-2.6/scripts/recordmcount.c
===================================================================
--- test-2.6.orig/scripts/recordmcount.c	2011-05-03 09:39:02.145472579 +0200
+++ test-2.6/scripts/recordmcount.c	2011-05-03 09:39:04.929486388 +0200
@@ -307,7 +307,12 @@
 	case EM_ARM:	 reltype = R_ARM_ABS32;
 			 altmcount = "__gnu_mcount_nc";
 			 break;
-	case EM_IA_64:	 reltype = R_IA64_IMM64;   gpfx = '_'; break;
+	case EM_IA_64:
+		reltype = R_IA64_IMM64;
+		/* Adjust relocation to second bundle, insn 2 */
+		mcount_adjust_32 = -18;
+		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;
Index: test-2.6/scripts/recordmcount.pl
===================================================================
--- test-2.6.orig/scripts/recordmcount.pl	2011-05-03 09:39:02.145472579 +0200
+++ test-2.6/scripts/recordmcount.pl	2011-05-03 09:39:04.929486388 +0200
@@ -278,6 +278,7 @@
 
 } elsif ($arch eq "ia64") {
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s_mcount\$";
+    $mcount_adjust = -18;
     $type = "data8";
 
     if ($is_module eq "0") {

[patch 4/4] s390 mcount offset calculation

[Martin Schwidefsky]

[-- Attachment #1: 303-mcount-adjust-s390.diff --]
[-- Type: text/plain, Size: 2681 bytes --]

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Do the mcount offset adjustment in the recordmcount.pl/recordmcount.[ch]
at compile time and not in ftrace_call_adjust at run time.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---


 arch/s390/include/asm/ftrace.h |    4 +---
 scripts/recordmcount.c         |    8 ++++++--
 scripts/recordmcount.pl        |    2 ++
 3 files changed, 9 insertions(+), 5 deletions(-)
Index: test-2.6/arch/s390/include/asm/ftrace.h
===================================================================
--- test-2.6.orig/arch/s390/include/asm/ftrace.h	2011-01-10 10:32:42.825504709 +0100
+++ test-2.6/arch/s390/include/asm/ftrace.h	2011-05-03 09:39:07.757500412 +0200
@@ -11,15 +11,13 @@
 
 #ifdef CONFIG_64BIT
 #define MCOUNT_INSN_SIZE  12
-#define MCOUNT_OFFSET	   8
 #else
 #define MCOUNT_INSN_SIZE  20
-#define MCOUNT_OFFSET	   4
 #endif
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
-	return addr - MCOUNT_OFFSET;
+	return addr;
 }
 
 #endif /* __ASSEMBLY__ */
Index: test-2.6/scripts/recordmcount.c
===================================================================
--- test-2.6.orig/scripts/recordmcount.c	2011-05-03 09:39:04.929486388 +0200
+++ test-2.6/scripts/recordmcount.c	2011-05-03 09:39:07.757500412 +0200
@@ -338,8 +338,10 @@
 				"unrecognized ET_REL file: %s\n", fname);
 			fail_file();
 		}
-		if (EM_S390 == w2(ehdr->e_machine))
+		if (EM_S390 == w2(ehdr->e_machine)) {
 			reltype = R_390_32;
+			mcount_adjust_32 = -4;
+		}
 		if (EM_MIPS == w2(ehdr->e_machine)) {
 			reltype = R_MIPS_32;
 			is_fake_mcount32 = MIPS32_is_fake_mcount;
@@ -354,8 +356,10 @@
 				"unrecognized ET_REL file: %s\n", fname);
 			fail_file();
 		}
-		if (EM_S390 == w2(ghdr->e_machine))
+		if (EM_S390 == w2(ghdr->e_machine)) {
 			reltype = R_390_64;
+			mcount_adjust_64 = -8;
+		}
 		if (EM_MIPS == w2(ghdr->e_machine)) {
 			reltype = R_MIPS_64;
 			Elf64_r_sym = MIPS64_r_sym;
Index: test-2.6/scripts/recordmcount.pl
===================================================================
--- test-2.6.orig/scripts/recordmcount.pl	2011-05-03 09:39:04.929486388 +0200
+++ test-2.6/scripts/recordmcount.pl	2011-05-03 09:39:07.757500412 +0200
@@ -242,12 +242,14 @@
 
 } elsif ($arch eq "s390" && $bits == 32) {
     $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*R_390_32\\s+_mcount\$";
+    $mcount_adjust = -4;
     $alignment = 4;
     $ld .= " -m elf_s390";
     $cc .= " -m31";
 
 } elsif ($arch eq "s390" && $bits == 64) {
     $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*R_390_(PC|PLT)32DBL\\s+_mcount\\+0x2\$";
+    $mcount_adjust = -8;
     $alignment = 8;
     $type = ".quad";
     $ld .= " -m elf64_s390";

[patch 4/4] s390 mcount offset calculation

[Martin Schwidefsky]

[-- Attachment #1: 303-mcount-adjust-s390.diff --]
[-- Type: text/plain, Size: 2682 bytes --]

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Do the mcount offset adjustment in the recordmcount.pl/recordmcount.[ch]
at compile time and not in ftrace_call_adjust at run time.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---


 arch/s390/include/asm/ftrace.h |    4 +---
 scripts/recordmcount.c         |    8 ++++++--
 scripts/recordmcount.pl        |    2 ++
 3 files changed, 9 insertions(+), 5 deletions(-)
Index: test-2.6/arch/s390/include/asm/ftrace.h
===================================================================
--- test-2.6.orig/arch/s390/include/asm/ftrace.h	2011-01-10 10:32:42.825504709 +0100
+++ test-2.6/arch/s390/include/asm/ftrace.h	2011-05-03 09:39:07.757500412 +0200
@@ -11,15 +11,13 @@
 
 #ifdef CONFIG_64BIT
 #define MCOUNT_INSN_SIZE  12
-#define MCOUNT_OFFSET	   8
 #else
 #define MCOUNT_INSN_SIZE  20
-#define MCOUNT_OFFSET	   4
 #endif
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
-	return addr - MCOUNT_OFFSET;
+	return addr;
 }
 
 #endif /* __ASSEMBLY__ */
Index: test-2.6/scripts/recordmcount.c
===================================================================
--- test-2.6.orig/scripts/recordmcount.c	2011-05-03 09:39:04.929486388 +0200
+++ test-2.6/scripts/recordmcount.c	2011-05-03 09:39:07.757500412 +0200
@@ -338,8 +338,10 @@
 				"unrecognized ET_REL file: %s\n", fname);
 			fail_file();
 		}
-		if (EM_S390 == w2(ehdr->e_machine))
+		if (EM_S390 == w2(ehdr->e_machine)) {
 			reltype = R_390_32;
+			mcount_adjust_32 = -4;
+		}
 		if (EM_MIPS == w2(ehdr->e_machine)) {
 			reltype = R_MIPS_32;
 			is_fake_mcount32 = MIPS32_is_fake_mcount;
@@ -354,8 +356,10 @@
 				"unrecognized ET_REL file: %s\n", fname);
 			fail_file();
 		}
-		if (EM_S390 == w2(ghdr->e_machine))
+		if (EM_S390 == w2(ghdr->e_machine)) {
 			reltype = R_390_64;
+			mcount_adjust_64 = -8;
+		}
 		if (EM_MIPS == w2(ghdr->e_machine)) {
 			reltype = R_MIPS_64;
 			Elf64_r_sym = MIPS64_r_sym;
Index: test-2.6/scripts/recordmcount.pl
===================================================================
--- test-2.6.orig/scripts/recordmcount.pl	2011-05-03 09:39:04.929486388 +0200
+++ test-2.6/scripts/recordmcount.pl	2011-05-03 09:39:07.757500412 +0200
@@ -242,12 +242,14 @@
 
 } elsif ($arch eq "s390" && $bits == 32) {
     $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*R_390_32\\s+_mcount\$";
+    $mcount_adjust = -4;
     $alignment = 4;
     $ld .= " -m elf_s390";
     $cc .= " -m31";
 
 } elsif ($arch eq "s390" && $bits == 64) {
     $mcount_regex = "^\\s*([0-9a-fA-F]+):\\s*R_390_(PC|PLT)32DBL\\s+_mcount\\+0x2\$";
+    $mcount_adjust = -8;
     $alignment = 8;
     $type = ".quad";
     $ld .= " -m elf64_s390";

Re: [patch 0/4] [RFC] mcount address adjustment

[Rabin Vincent]

On Tue, May 10, 2011 at 13:40, Martin Schwidefsky
<schwidefsky@de.ibm.com> wrote:
> That leaves arm as the last remaining architecture with a non trivial
> ftrace_call_adjust function. There the least significant bit is removed
> from the address with an and operation. The comment says this is done
> for Thumb-2. This implies that for Thumb-1 the offset is 0 and for
> Thumb-2 the offset is -1, correct?

ARM supports building the kernel using either the ARM instruction set or
the Thumb-2 instruction set.  The kernel cannot be built with the
"Thumb-1" instruction set (btw usually referred to as just Thumb).

Thumb-2 via recordmcount.pl needs the clearing of the lsb because the
relocation (R_ARM_ABS32) that gets used for the assembly file
that recordmcount.pl generates and assembles dictates that the lsb be
set if the target symbol is Thumb/Thumb-2 function.  mcount_adjust would
not help here since the ORing is done later, when the relocation is
applied.

Thumb-2 via recordmcount.c does not need the clearing of the lsb in
ftrace_call_adjust.

Building with the ARM instruction set also does not need the clearing
of the lsb.

> Thumb-2 the offset is -1, correct? If there is a way to distinguish
> the two targets in recordmcount at compile time we could convert arm
> as well. Which would allow us to remove the ftrace_call_adjust function.

To remove ftrace_call_adjust, we could either deprecate the
recordmcount.pl usage for ARM (you already have to edit the Kconfig to
use it) or modify it to generate specific relocations explicitly instead
of using the assembler data directives.

[patch 0/4] [RFC] mcount address adjustment

[Rabin Vincent]

On Tue, May 10, 2011 at 13:40, Martin Schwidefsky
<schwidefsky@de.ibm.com> wrote:
> That leaves arm as the last remaining architecture with a non trivial
> ftrace_call_adjust function. There the least significant bit is removed
> from the address with an and operation. The comment says this is done
> for Thumb-2. This implies that for Thumb-1 the offset is 0 and for
> Thumb-2 the offset is -1, correct?

ARM supports building the kernel using either the ARM instruction set or
the Thumb-2 instruction set.  The kernel cannot be built with the
"Thumb-1" instruction set (btw usually referred to as just Thumb).

Thumb-2 via recordmcount.pl needs the clearing of the lsb because the
relocation (R_ARM_ABS32) that gets used for the assembly file
that recordmcount.pl generates and assembles dictates that the lsb be
set if the target symbol is Thumb/Thumb-2 function.  mcount_adjust would
not help here since the ORing is done later, when the relocation is
applied.

Thumb-2 via recordmcount.c does not need the clearing of the lsb in
ftrace_call_adjust.

Building with the ARM instruction set also does not need the clearing
of the lsb.

> Thumb-2 the offset is -1, correct? If there is a way to distinguish
> the two targets in recordmcount at compile time we could convert arm
> as well. Which would allow us to remove the ftrace_call_adjust function.

To remove ftrace_call_adjust, we could either deprecate the
recordmcount.pl usage for ARM (you already have to edit the Kconfig to
use it) or modify it to generate specific relocations explicitly instead
of using the assembler data directives.

Re: [patch 0/4] [RFC] mcount address adjustment

[Martin Schwidefsky]

On Wed, 11 May 2011 22:53:55 +0530
Rabin Vincent <rabin@rab.in> wrote:

> On Tue, May 10, 2011 at 13:40, Martin Schwidefsky
> <schwidefsky@de.ibm.com> wrote:
> > That leaves arm as the last remaining architecture with a non trivial
> > ftrace_call_adjust function. There the least significant bit is removed
> > from the address with an and operation. The comment says this is done
> > for Thumb-2. This implies that for Thumb-1 the offset is 0 and for
> > Thumb-2 the offset is -1, correct?
> 
> ARM supports building the kernel using either the ARM instruction set or
> the Thumb-2 instruction set.  The kernel cannot be built with the
> "Thumb-1" instruction set (btw usually referred to as just Thumb).
> 
> Thumb-2 via recordmcount.pl needs the clearing of the lsb because the
> relocation (R_ARM_ABS32) that gets used for the assembly file
> that recordmcount.pl generates and assembles dictates that the lsb be
> set if the target symbol is Thumb/Thumb-2 function.  mcount_adjust would
> not help here since the ORing is done later, when the relocation is
> applied.

Hmm, from what I can make out the C version of recordmcount uses R_ARM_ABS32
as well. 
 
> Thumb-2 via recordmcount.c does not need the clearing of the lsb in
> ftrace_call_adjust.

So the clearing of the lsb is only required if the recordmcount.pl script
is used?

> Building with the ARM instruction set also does not need the clearing
> of the lsb.

Who does the ORing? I can't find anything in recordmount.pl/recordmcount.c
which looks like doing an OR, does the assembler do that based on the
symbol type?

> > Thumb-2 the offset is -1, correct? If there is a way to distinguish
> > the two targets in recordmcount at compile time we could convert arm
> > as well. Which would allow us to remove the ftrace_call_adjust function.
> 
> To remove ftrace_call_adjust, we could either deprecate the
> recordmcount.pl usage for ARM (you already have to edit the Kconfig to
> use it) or modify it to generate specific relocations explicitly instead
> of using the assembler data directives.

Hmm, it would be a desirable property if the C version and the pearl
version of recordmcount would do the same. Or we could remove the arm
support from the pearl script, the C version is faster anyway.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

[patch 0/4] [RFC] mcount address adjustment

[Martin Schwidefsky]

On Wed, 11 May 2011 22:53:55 +0530
Rabin Vincent <rabin@rab.in> wrote:

> On Tue, May 10, 2011 at 13:40, Martin Schwidefsky
> <schwidefsky@de.ibm.com> wrote:
> > That leaves arm as the last remaining architecture with a non trivial
> > ftrace_call_adjust function. There the least significant bit is removed
> > from the address with an and operation. The comment says this is done
> > for Thumb-2. This implies that for Thumb-1 the offset is 0 and for
> > Thumb-2 the offset is -1, correct?
> 
> ARM supports building the kernel using either the ARM instruction set or
> the Thumb-2 instruction set.  The kernel cannot be built with the
> "Thumb-1" instruction set (btw usually referred to as just Thumb).
> 
> Thumb-2 via recordmcount.pl needs the clearing of the lsb because the
> relocation (R_ARM_ABS32) that gets used for the assembly file
> that recordmcount.pl generates and assembles dictates that the lsb be
> set if the target symbol is Thumb/Thumb-2 function.  mcount_adjust would
> not help here since the ORing is done later, when the relocation is
> applied.

Hmm, from what I can make out the C version of recordmcount uses R_ARM_ABS32
as well. 
 
> Thumb-2 via recordmcount.c does not need the clearing of the lsb in
> ftrace_call_adjust.

So the clearing of the lsb is only required if the recordmcount.pl script
is used?

> Building with the ARM instruction set also does not need the clearing
> of the lsb.

Who does the ORing? I can't find anything in recordmount.pl/recordmcount.c
which looks like doing an OR, does the assembler do that based on the
symbol type?

> > Thumb-2 the offset is -1, correct? If there is a way to distinguish
> > the two targets in recordmcount at compile time we could convert arm
> > as well. Which would allow us to remove the ftrace_call_adjust function.
> 
> To remove ftrace_call_adjust, we could either deprecate the
> recordmcount.pl usage for ARM (you already have to edit the Kconfig to
> use it) or modify it to generate specific relocations explicitly instead
> of using the assembler data directives.

Hmm, it would be a desirable property if the C version and the pearl
version of recordmcount would do the same. Or we could remove the arm
support from the pearl script, the C version is faster anyway.

-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [patch 0/4] [RFC] mcount address adjustment

[Rabin Vincent]

On Thu, May 12, 2011 at 14:54, Martin Schwidefsky
<schwidefsky@de.ibm.com> wrote:
> On Wed, 11 May 2011 22:53:55 +0530 Rabin Vincent <rabin@rab.in> wrote:
>> On Tue, May 10, 2011 at 13:40, Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
>> Thumb-2 via recordmcount.pl needs the clearing of the lsb because the
>> relocation (R_ARM_ABS32) that gets used for the assembly file
>> that recordmcount.pl generates and assembles dictates that the lsb be
>> set if the target symbol is Thumb/Thumb-2 function.  mcount_adjust would
>> not help here since the ORing is done later, when the relocation is
>> applied.
>
> Hmm, from what I can make out the C version of recordmcount uses R_ARM_ABS32
> as well.

Right.  It worked when I initially implemented ARM support there because
recordmcount.c always found the STT_SECTION symbol as a base and not a
STT_FUNC symbol.  However, I noticed yesterday that this does not happen
in some cases, so I sent a patch to avoid STT_FUNC symbol as bases on
ARM, not because of this relocation, but because of a slightly different
oddity of Thumb symbols:

http://lkml.org/lkml/2011/5/11/304

(The relocation problem alone could be solved by using R_ARM_ABS32_NOI
 instead.)

>
>> Thumb-2 via recordmcount.c does not need the clearing of the lsb in
>> ftrace_call_adjust.
>
> So the clearing of the lsb is only required if the recordmcount.pl script
> is used?

Yes.

>> Building with the ARM instruction set also does not need the clearing
>> of the lsb.
>
> Who does the ORing? I can't find anything in recordmount.pl/recordmcount.c
> which looks like doing an OR, does the assembler do that based on the
> symbol type?

The lsb is set to 1 by the linker, when it applies the relocations as it
links vmlinux.

>
>> > Thumb-2 the offset is -1, correct? If there is a way to distinguish
>> > the two targets in recordmcount at compile time we could convert arm
>> > as well. Which would allow us to remove the ftrace_call_adjust function.
>>
>> To remove ftrace_call_adjust, we could either deprecate the
>> recordmcount.pl usage for ARM (you already have to edit the Kconfig to
>> use it) or modify it to generate specific relocations explicitly instead
>> of using the assembler data directives.
>
> Hmm, it would be a desirable property if the C version and the pearl
> version of recordmcount would do the same. Or we could remove the arm
> support from the pearl script, the C version is faster anyway.

I'm OK with removing the ARM support from recordmcount.pl; it doesn't
seem needed to make significant modifications to it for ARM when we
don't use it anyway.

[patch 0/4] [RFC] mcount address adjustment

[Rabin Vincent]

On Thu, May 12, 2011 at 14:54, Martin Schwidefsky
<schwidefsky@de.ibm.com> wrote:
> On Wed, 11 May 2011 22:53:55 +0530 Rabin Vincent <rabin@rab.in> wrote:
>> On Tue, May 10, 2011 at 13:40, Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
>> Thumb-2 via recordmcount.pl needs the clearing of the lsb because the
>> relocation (R_ARM_ABS32) that gets used for the assembly file
>> that recordmcount.pl generates and assembles dictates that the lsb be
>> set if the target symbol is Thumb/Thumb-2 function. ?mcount_adjust would
>> not help here since the ORing is done later, when the relocation is
>> applied.
>
> Hmm, from what I can make out the C version of recordmcount uses R_ARM_ABS32
> as well.

Right.  It worked when I initially implemented ARM support there because
recordmcount.c always found the STT_SECTION symbol as a base and not a
STT_FUNC symbol.  However, I noticed yesterday that this does not happen
in some cases, so I sent a patch to avoid STT_FUNC symbol as bases on
ARM, not because of this relocation, but because of a slightly different
oddity of Thumb symbols:

http://lkml.org/lkml/2011/5/11/304

(The relocation problem alone could be solved by using R_ARM_ABS32_NOI
 instead.)

>
>> Thumb-2 via recordmcount.c does not need the clearing of the lsb in
>> ftrace_call_adjust.
>
> So the clearing of the lsb is only required if the recordmcount.pl script
> is used?

Yes.

>> Building with the ARM instruction set also does not need the clearing
>> of the lsb.
>
> Who does the ORing? I can't find anything in recordmount.pl/recordmcount.c
> which looks like doing an OR, does the assembler do that based on the
> symbol type?

The lsb is set to 1 by the linker, when it applies the relocations as it
links vmlinux.

>
>> > Thumb-2 the offset is -1, correct? If there is a way to distinguish
>> > the two targets in recordmcount at compile time we could convert arm
>> > as well. Which would allow us to remove the ftrace_call_adjust function.
>>
>> To remove ftrace_call_adjust, we could either deprecate the
>> recordmcount.pl usage for ARM (you already have to edit the Kconfig to
>> use it) or modify it to generate specific relocations explicitly instead
>> of using the assembler data directives.
>
> Hmm, it would be a desirable property if the C version and the pearl
> version of recordmcount would do the same. Or we could remove the arm
> support from the pearl script, the C version is faster anyway.

I'm OK with removing the ARM support from recordmcount.pl; it doesn't
seem needed to make significant modifications to it for ARM when we
don't use it anyway.

Re: [patch 0/4] [RFC] mcount address adjustment

[Dave Martin]

On Thu, May 12, 2011 at 07:00:13PM +0530, Rabin Vincent wrote:
> On Thu, May 12, 2011 at 14:54, Martin Schwidefsky
> <schwidefsky@de.ibm.com> wrote:
> > On Wed, 11 May 2011 22:53:55 +0530 Rabin Vincent <rabin@rab.in> wrote:
> >> On Tue, May 10, 2011 at 13:40, Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> >> Thumb-2 via recordmcount.pl needs the clearing of the lsb because the
> >> relocation (R_ARM_ABS32) that gets used for the assembly file
> >> that recordmcount.pl generates and assembles dictates that the lsb be
> >> set if the target symbol is Thumb/Thumb-2 function.  mcount_adjust would
> >> not help here since the ORing is done later, when the relocation is
> >> applied.
> >
> > Hmm, from what I can make out the C version of recordmcount uses R_ARM_ABS32
> > as well.
> 
> Right.  It worked when I initially implemented ARM support there because
> recordmcount.c always found the STT_SECTION symbol as a base and not a
> STT_FUNC symbol.  However, I noticed yesterday that this does not happen
> in some cases, so I sent a patch to avoid STT_FUNC symbol as bases on
> ARM, not because of this relocation, but because of a slightly different
> oddity of Thumb symbols:
> 
> http://lkml.org/lkml/2011/5/11/304
> 
> (The relocation problem alone could be solved by using R_ARM_ABS32_NOI
>  instead.)
> 
> >
> >> Thumb-2 via recordmcount.c does not need the clearing of the lsb in
> >> ftrace_call_adjust.
> >
> > So the clearing of the lsb is only required if the recordmcount.pl script
> > is used?
> 
> Yes.
> 
> >> Building with the ARM instruction set also does not need the clearing
> >> of the lsb.
> >
> > Who does the ORing? I can't find anything in recordmount.pl/recordmcount.c
> > which looks like doing an OR, does the assembler do that based on the
> > symbol type?
> 
> The lsb is set to 1 by the linker, when it applies the relocations as it
> links vmlinux.
> 
> >
> >> > Thumb-2 the offset is -1, correct? If there is a way to distinguish
> >> > the two targets in recordmcount at compile time we could convert arm
> >> > as well. Which would allow us to remove the ftrace_call_adjust function.
> >>
> >> To remove ftrace_call_adjust, we could either deprecate the
> >> recordmcount.pl usage for ARM (you already have to edit the Kconfig to
> >> use it) or modify it to generate specific relocations explicitly instead
> >> of using the assembler data directives.
> >
> > Hmm, it would be a desirable property if the C version and the pearl
> > version of recordmcount would do the same. Or we could remove the arm
> > support from the pearl script, the C version is faster anyway.
> 
> I'm OK with removing the ARM support from recordmcount.pl; it doesn't
> seem needed to make significant modifications to it for ARM when we
> don't use it anyway.

Is there any reason why the recordmcount.pl would ever be used now that the
C implementation exists?

I notice that arch/arm/Kconfig has:

config ARM
...
        select HAVE_C_RECORDMCOUNT

so deprecating ARM support from recordmcount.pl seems unlikely to hurt
anyone.

The C implementation seems to have worked fine when I was testing dynamic
ftrace with Thumb-2 recently.

Cheers
---Dave

[patch 0/4] [RFC] mcount address adjustment

[Dave Martin]

On Thu, May 12, 2011 at 07:00:13PM +0530, Rabin Vincent wrote:
> On Thu, May 12, 2011 at 14:54, Martin Schwidefsky
> <schwidefsky@de.ibm.com> wrote:
> > On Wed, 11 May 2011 22:53:55 +0530 Rabin Vincent <rabin@rab.in> wrote:
> >> On Tue, May 10, 2011 at 13:40, Martin Schwidefsky <schwidefsky@de.ibm.com> wrote:
> >> Thumb-2 via recordmcount.pl needs the clearing of the lsb because the
> >> relocation (R_ARM_ABS32) that gets used for the assembly file
> >> that recordmcount.pl generates and assembles dictates that the lsb be
> >> set if the target symbol is Thumb/Thumb-2 function. ?mcount_adjust would
> >> not help here since the ORing is done later, when the relocation is
> >> applied.
> >
> > Hmm, from what I can make out the C version of recordmcount uses R_ARM_ABS32
> > as well.
> 
> Right.  It worked when I initially implemented ARM support there because
> recordmcount.c always found the STT_SECTION symbol as a base and not a
> STT_FUNC symbol.  However, I noticed yesterday that this does not happen
> in some cases, so I sent a patch to avoid STT_FUNC symbol as bases on
> ARM, not because of this relocation, but because of a slightly different
> oddity of Thumb symbols:
> 
> http://lkml.org/lkml/2011/5/11/304
> 
> (The relocation problem alone could be solved by using R_ARM_ABS32_NOI
>  instead.)
> 
> >
> >> Thumb-2 via recordmcount.c does not need the clearing of the lsb in
> >> ftrace_call_adjust.
> >
> > So the clearing of the lsb is only required if the recordmcount.pl script
> > is used?
> 
> Yes.
> 
> >> Building with the ARM instruction set also does not need the clearing
> >> of the lsb.
> >
> > Who does the ORing? I can't find anything in recordmount.pl/recordmcount.c
> > which looks like doing an OR, does the assembler do that based on the
> > symbol type?
> 
> The lsb is set to 1 by the linker, when it applies the relocations as it
> links vmlinux.
> 
> >
> >> > Thumb-2 the offset is -1, correct? If there is a way to distinguish
> >> > the two targets in recordmcount at compile time we could convert arm
> >> > as well. Which would allow us to remove the ftrace_call_adjust function.
> >>
> >> To remove ftrace_call_adjust, we could either deprecate the
> >> recordmcount.pl usage for ARM (you already have to edit the Kconfig to
> >> use it) or modify it to generate specific relocations explicitly instead
> >> of using the assembler data directives.
> >
> > Hmm, it would be a desirable property if the C version and the pearl
> > version of recordmcount would do the same. Or we could remove the arm
> > support from the pearl script, the C version is faster anyway.
> 
> I'm OK with removing the ARM support from recordmcount.pl; it doesn't
> seem needed to make significant modifications to it for ARM when we
> don't use it anyway.

Is there any reason why the recordmcount.pl would ever be used now that the
C implementation exists?

I notice that arch/arm/Kconfig has:

config ARM
...
        select HAVE_C_RECORDMCOUNT

so deprecating ARM support from recordmcount.pl seems unlikely to hurt
anyone.

The C implementation seems to have worked fine when I was testing dynamic
ftrace with Thumb-2 recently.

Cheers
---Dave

Re: [patch 0/4] [RFC] mcount address adjustment

[Steven Rostedt]

On Mon, 2011-05-16 at 13:57 +0100, Dave Martin wrote:

> Is there any reason why the recordmcount.pl would ever be used now that the
> C implementation exists?
> 
> I notice that arch/arm/Kconfig has:
> 
> config ARM
> ...
>         select HAVE_C_RECORDMCOUNT
> 
> so deprecating ARM support from recordmcount.pl seems unlikely to hurt
> anyone.
> 
> The C implementation seems to have worked fine when I was testing dynamic
> ftrace with Thumb-2 recently.

It's there only as a backup. But you're right. I may as well start
removing the recordmcount.pl support from those that have the
HAVE_C_RECORDMCOUNT set, and then all users are gone (which may now be
the case) either keep it around as a backup for testing against
recordmcount.c, or remove it completely.

-- Steve

[patch 0/4] [RFC] mcount address adjustment

[Steven Rostedt]

On Mon, 2011-05-16 at 13:57 +0100, Dave Martin wrote:

> Is there any reason why the recordmcount.pl would ever be used now that the
> C implementation exists?
> 
> I notice that arch/arm/Kconfig has:
> 
> config ARM
> ...
>         select HAVE_C_RECORDMCOUNT
> 
> so deprecating ARM support from recordmcount.pl seems unlikely to hurt
> anyone.
> 
> The C implementation seems to have worked fine when I was testing dynamic
> ftrace with Thumb-2 recently.

It's there only as a backup. But you're right. I may as well start
removing the recordmcount.pl support from those that have the
HAVE_C_RECORDMCOUNT set, and then all users are gone (which may now be
the case) either keep it around as a backup for testing against
recordmcount.c, or remove it completely.

-- Steve

Re: [patch 3/4] ia64 mcount offset calculation

[Steven Rostedt]

Tony,

Can you give me an Acked-by for this patch?

Thanks,

-- Steve


On Tue, 2011-05-10 at 10:10 +0200, Martin Schwidefsky wrote:
> plain text document attachment (302-mcount-adjust-ia64.diff)
> From: Martin Schwidefsky <schwidefsky@de.ibm.com>
> 
> Do the mcount offset adjustment in the recordmcount.pl/recordmcount.[ch]
> at compile time and not in ftrace_call_adjust at run time.
> 
> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
> ---
> 
> 
>  arch/ia64/include/asm/ftrace.h |    7 +++++--
>  scripts/recordmcount.c         |    7 ++++++-
>  scripts/recordmcount.pl        |    1 +
>  3 files changed, 12 insertions(+), 3 deletions(-)
> Index: test-2.6/arch/ia64/include/asm/ftrace.h
> ===================================================================
> --- test-2.6.orig/arch/ia64/include/asm/ftrace.h	2010-01-11 10:58:31.000000000 +0100
> +++ test-2.6/arch/ia64/include/asm/ftrace.h	2011-05-03 09:39:04.929486388 +0200
> @@ -14,8 +14,11 @@
>  
>  static inline unsigned long ftrace_call_adjust(unsigned long addr)
>  {
> -	/* second bundle, insn 2 */
> -	return addr - 0x12;
> +	/*
> +	 * addr is the address of the mcount call instruction.
> +	 * recordmcount does the necessary offset calculation.
> +	 */
> +	return addr;
>  }
>  
>  struct dyn_arch_ftrace {
> Index: test-2.6/scripts/recordmcount.c
> ===================================================================
> --- test-2.6.orig/scripts/recordmcount.c	2011-05-03 09:39:02.145472579 +0200
> +++ test-2.6/scripts/recordmcount.c	2011-05-03 09:39:04.929486388 +0200
> @@ -307,7 +307,12 @@
>  	case EM_ARM:	 reltype = R_ARM_ABS32;
>  			 altmcount = "__gnu_mcount_nc";
>  			 break;
> -	case EM_IA_64:	 reltype = R_IA64_IMM64;   gpfx = '_'; break;
> +	case EM_IA_64:
> +		reltype = R_IA64_IMM64;
> +		/* Adjust relocation to second bundle, insn 2 */
> +		mcount_adjust_32 = -18;
> +		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;
> Index: test-2.6/scripts/recordmcount.pl
> ===================================================================
> --- test-2.6.orig/scripts/recordmcount.pl	2011-05-03 09:39:02.145472579 +0200
> +++ test-2.6/scripts/recordmcount.pl	2011-05-03 09:39:04.929486388 +0200
> @@ -278,6 +278,7 @@
>  
>  } elsif ($arch eq "ia64") {
>      $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s_mcount\$";
> +    $mcount_adjust = -18;
>      $type = "data8";
>  
>      if ($is_module eq "0") {

RE: [patch 3/4] ia64 mcount offset calculation

[Luck, Tony]

I'm not sure how to test it - but it looks somewhat suspect.

The difficulty is in the way that ia64 tools report program addresses within bundles.

ia64 crams 3 41-bit instructions (and a 5-bit template) into a 16-byte "bundle".

Some tools report consecutive addresses like this:

0x000
0x001
0x002
0x010
0x011
0x012

where the low nibble is the instruction number within the bundle.  Others go
for:

0x000
0x006
0x00c
0x010
0x016
0x01c

with the illusion that instructions take 6-byte, 6bytes, 4-bytes.

This patch does some fast and loose translation from hex (0x12) in the kernel
to decimal (18) in the user tools ... which doesn't look like it fits with
either of these conventions.

-Tony

-----Original Message-----
From: linux-arch-owner@vger.kernel.org [mailto:linux-arch-owner@vger.kernel.org] On Behalf Of Steven Rostedt
Sent: Monday, May 16, 2011 11:58 AM
To: Luck, Tony
Cc: Martin Schwidefsky; linux-arch@vger.kernel.org
Subject: Re: [patch 3/4] ia64 mcount offset calculation

Tony,

Can you give me an Acked-by for this patch?

Thanks,

-- Steve


On Tue, 2011-05-10 at 10:10 +0200, Martin Schwidefsky wrote:
> plain text document attachment (302-mcount-adjust-ia64.diff)
> From: Martin Schwidefsky <schwidefsky@de.ibm.com>
> 
> Do the mcount offset adjustment in the recordmcount.pl/recordmcount.[ch]
> at compile time and not in ftrace_call_adjust at run time.
> 
> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
> ---
> 
> 
>  arch/ia64/include/asm/ftrace.h |    7 +++++--
>  scripts/recordmcount.c         |    7 ++++++-
>  scripts/recordmcount.pl        |    1 +
>  3 files changed, 12 insertions(+), 3 deletions(-)
> Index: test-2.6/arch/ia64/include/asm/ftrace.h
> ===================================================================
> --- test-2.6.orig/arch/ia64/include/asm/ftrace.h	2010-01-11 10:58:31.000000000 +0100
> +++ test-2.6/arch/ia64/include/asm/ftrace.h	2011-05-03 09:39:04.929486388 +0200
> @@ -14,8 +14,11 @@
>  
>  static inline unsigned long ftrace_call_adjust(unsigned long addr)
>  {
> -	/* second bundle, insn 2 */
> -	return addr - 0x12;
> +	/*
> +	 * addr is the address of the mcount call instruction.
> +	 * recordmcount does the necessary offset calculation.
> +	 */
> +	return addr;
>  }
>  
>  struct dyn_arch_ftrace {
> Index: test-2.6/scripts/recordmcount.c
> ===================================================================
> --- test-2.6.orig/scripts/recordmcount.c	2011-05-03 09:39:02.145472579 +0200
> +++ test-2.6/scripts/recordmcount.c	2011-05-03 09:39:04.929486388 +0200
> @@ -307,7 +307,12 @@
>  	case EM_ARM:	 reltype = R_ARM_ABS32;
>  			 altmcount = "__gnu_mcount_nc";
>  			 break;
> -	case EM_IA_64:	 reltype = R_IA64_IMM64;   gpfx = '_'; break;
> +	case EM_IA_64:
> +		reltype = R_IA64_IMM64;
> +		/* Adjust relocation to second bundle, insn 2 */
> +		mcount_adjust_32 = -18;
> +		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;
> Index: test-2.6/scripts/recordmcount.pl
> ===================================================================
> --- test-2.6.orig/scripts/recordmcount.pl	2011-05-03 09:39:02.145472579 +0200
> +++ test-2.6/scripts/recordmcount.pl	2011-05-03 09:39:04.929486388 +0200
> @@ -278,6 +278,7 @@
>  
>  } elsif ($arch eq "ia64") {
>      $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s_mcount\$";
> +    $mcount_adjust = -18;
>      $type = "data8";
>  
>      if ($is_module eq "0") {

RE: [patch 3/4] ia64 mcount offset calculation

[Luck, Tony]

I'm not sure how to test it - but it looks somewhat suspect.

The difficulty is in the way that ia64 tools report program addresses within bundles.

ia64 crams 3 41-bit instructions (and a 5-bit template) into a 16-byte "bundle".

Some tools report consecutive addresses like this:

0x000
0x001
0x002
0x010
0x011
0x012

where the low nibble is the instruction number within the bundle.  Others go
for:

0x000
0x006
0x00c
0x010
0x016
0x01c

with the illusion that instructions take 6-byte, 6bytes, 4-bytes.

This patch does some fast and loose translation from hex (0x12) in the kernel
to decimal (18) in the user tools ... which doesn't look like it fits with
either of these conventions.

-Tony

-----Original Message-----
From: linux-arch-owner@vger.kernel.org [mailto:linux-arch-owner@vger.kernel.org] On Behalf Of Steven Rostedt
Sent: Monday, May 16, 2011 11:58 AM
To: Luck, Tony
Cc: Martin Schwidefsky; linux-arch@vger.kernel.org
Subject: Re: [patch 3/4] ia64 mcount offset calculation

Tony,

Can you give me an Acked-by for this patch?

Thanks,

-- Steve


On Tue, 2011-05-10 at 10:10 +0200, Martin Schwidefsky wrote:
> plain text document attachment (302-mcount-adjust-ia64.diff)
> From: Martin Schwidefsky <schwidefsky@de.ibm.com>
> 
> Do the mcount offset adjustment in the recordmcount.pl/recordmcount.[ch]
> at compile time and not in ftrace_call_adjust at run time.
> 
> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
> ---
> 
> 
>  arch/ia64/include/asm/ftrace.h |    7 +++++--
>  scripts/recordmcount.c         |    7 ++++++-
>  scripts/recordmcount.pl        |    1 +
>  3 files changed, 12 insertions(+), 3 deletions(-)
> Index: test-2.6/arch/ia64/include/asm/ftrace.h
> ===================================================================
> --- test-2.6.orig/arch/ia64/include/asm/ftrace.h	2010-01-11 10:58:31.000000000 +0100
> +++ test-2.6/arch/ia64/include/asm/ftrace.h	2011-05-03 09:39:04.929486388 +0200
> @@ -14,8 +14,11 @@
>  
>  static inline unsigned long ftrace_call_adjust(unsigned long addr)
>  {
> -	/* second bundle, insn 2 */
> -	return addr - 0x12;
> +	/*
> +	 * addr is the address of the mcount call instruction.
> +	 * recordmcount does the necessary offset calculation.
> +	 */
> +	return addr;
>  }
>  
>  struct dyn_arch_ftrace {
> Index: test-2.6/scripts/recordmcount.c
> ===================================================================
> --- test-2.6.orig/scripts/recordmcount.c	2011-05-03 09:39:02.145472579 +0200
> +++ test-2.6/scripts/recordmcount.c	2011-05-03 09:39:04.929486388 +0200
> @@ -307,7 +307,12 @@
>  	case EM_ARM:	 reltype = R_ARM_ABS32;
>  			 altmcount = "__gnu_mcount_nc";
>  			 break;
> -	case EM_IA_64:	 reltype = R_IA64_IMM64;   gpfx = '_'; break;
> +	case EM_IA_64:
> +		reltype = R_IA64_IMM64;
> +		/* Adjust relocation to second bundle, insn 2 */
> +		mcount_adjust_32 = -18;
> +		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;
> Index: test-2.6/scripts/recordmcount.pl
> ===================================================================
> --- test-2.6.orig/scripts/recordmcount.pl	2011-05-03 09:39:02.145472579 +0200
> +++ test-2.6/scripts/recordmcount.pl	2011-05-03 09:39:04.929486388 +0200
> @@ -278,6 +278,7 @@
>  
>  } elsif ($arch eq "ia64") {
>      $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s_mcount\$";
> +    $mcount_adjust = -18;
>      $type = "data8";
>  
>      if ($is_module eq "0") {


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

RE: [patch 3/4] ia64 mcount offset calculation

[Steven Rostedt]

On Mon, 2011-05-16 at 12:17 -0700, Luck, Tony wrote:
> I'm not sure how to test it - but it looks somewhat suspect.

You could apply the first patch and this patch and see if ftrace still
works :)

> 
> The difficulty is in the way that ia64 tools report program addresses within bundles.
> 
> ia64 crams 3 41-bit instructions (and a 5-bit template) into a 16-byte "bundle".
> 
> Some tools report consecutive addresses like this:
> 
> 0x000
> 0x001
> 0x002
> 0x010
> 0x011
> 0x012
> 
> where the low nibble is the instruction number within the bundle.  Others go
> for:
> 
> 0x000
> 0x006
> 0x00c
> 0x010
> 0x016
> 0x01c
> 
> with the illusion that instructions take 6-byte, 6bytes, 4-bytes.

And I'm amazed that this wonderful architecture didn't become the
defacto for all users!


> 
> This patch does some fast and loose translation from hex (0x12) in the kernel
> to decimal (18) in the user tools ... which doesn't look like it fits with
> either of these conventions.

Could be broken. I'll leave it out of my next push.

Thanks,

-- Steve

Re: [patch 3/4] ia64 mcount offset calculation

[Martin Schwidefsky]

On Mon, 16 May 2011 12:17:15 -0700
"Luck, Tony" <tony.luck@intel.com> wrote:

> This patch does some fast and loose translation from hex (0x12) in the kernel
> to decimal (18) in the user tools ... which doesn't look like it fits with
> either of these conventions.

All the patch does is to move an address calculation from the kernel "addr-0x12"
to user space by subtracting the constant offset from the addend value for the
relocation: "addend = _w(_w(relp->r_offset) - recval + mcount_adjust)".

The offset for ia64 is -18. I'd be very surprised if the outcome of the move
would yield a different end result. But as I took another look at the patch
I noticed a bug, for EM_IA64 we need to set mcount_adjust_64 and not
mcount_adjust_32 as it is a 64-bit architecture. New patch below.

I tried to do a test compile with/without the patch but with the latest git
kernels TRACING_SUPPORT depends on TRACE_IRQFLAGS_SUPPORT which is missing
for ia64. No luck ..

---
Subject: [PATCH] ia64 mcount offset calculation

From: Martin Schwidefsky <schwidefsky@de.ibm.com>

Do the mcount offset adjustment in the recordmcount.pl/recordmcount.[ch]
at compile time and not in ftrace_call_adjust at run time.

Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---

 arch/ia64/include/asm/ftrace.h |    7 +++++--
 scripts/recordmcount.c         |    7 ++++++-
 scripts/recordmcount.pl        |    1 +
 3 files changed, 12 insertions(+), 3 deletions(-)

diff -urpN linux-2.6/arch/ia64/include/asm/ftrace.h linux-2.6-patched/arch/ia64/include/asm/ftrace.h
--- linux-2.6/arch/ia64/include/asm/ftrace.h	2011-03-15 02:20:32.000000000 +0100
+++ linux-2.6-patched/arch/ia64/include/asm/ftrace.h	2011-05-16 10:16:20.000000000 +0200
@@ -14,8 +14,11 @@ extern void _mcount(unsigned long pfs, u
 
 static inline unsigned long ftrace_call_adjust(unsigned long addr)
 {
-	/* second bundle, insn 2 */
-	return addr - 0x12;
+	/*
+	 * addr is the address of the mcount call instruction.
+	 * recordmcount does the necessary offset calculation.
+	 */
+	return addr;
 }
 
 struct dyn_arch_ftrace {
diff -urpN linux-2.6/scripts/recordmcount.c linux-2.6-patched/scripts/recordmcount.c
--- linux-2.6/scripts/recordmcount.c	2011-05-16 10:16:20.000000000 +0200
+++ linux-2.6-patched/scripts/recordmcount.c	2011-05-16 10:16:20.000000000 +0200
@@ -307,7 +307,12 @@ do_file(char const *const fname)
 	case EM_ARM:	 reltype = R_ARM_ABS32;
 			 altmcount = "__gnu_mcount_nc";
 			 break;
-	case EM_IA_64:	 reltype = R_IA64_IMM64;   gpfx = '_'; break;
+	case EM_IA_64:
+		reltype = R_IA64_IMM64;
+		/* Adjust relocation to second bundle, insn 2 */
+		mcount_adjust_64 = -18;
+		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;
diff -urpN linux-2.6/scripts/recordmcount.pl linux-2.6-patched/scripts/recordmcount.pl
--- linux-2.6/scripts/recordmcount.pl	2011-05-16 10:16:20.000000000 +0200
+++ linux-2.6-patched/scripts/recordmcount.pl	2011-05-16 10:16:20.000000000 +0200
@@ -278,6 +278,7 @@ if ($arch eq "x86_64") {
 
 } elsif ($arch eq "ia64") {
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s_mcount\$";
+    $mcount_adjust = -18;
     $type = "data8";
 
     if ($is_module eq "0") {


-- 
blue skies,
   Martin.

"Reality continues to ruin my life." - Calvin.

Re: [patch 3/4] ia64 mcount offset calculation

[Steven Rostedt]

On Tue, 2011-05-17 at 10:04 +0200, Martin Schwidefsky wrote:
> On Mon, 16 May 2011 12:17:15 -0700

> I tried to do a test compile with/without the patch but with the latest git
> kernels TRACING_SUPPORT depends on TRACE_IRQFLAGS_SUPPORT which is missing
> for ia64. No luck ..

How hard would it be to add TRACE_IRQFLAGS_SUPPORT. That should allow
for including not only tracing, but also lockdep.

Although, I may be able to make function tracing work without it.

-- Steve