Skip to contentSkip to conversation

ftrace: updates to recordmcount.pl script

PATCH In review Steven Rostedt 5 participants 10 messages Nov 20, 2008 – Nov 21, 2008

Subsystems (2)
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

cover ftrace: updates to recordmcount.pl script

Ingo,

I took Matt's patch and stripped out all the sh changes, leaving only the update to recordmcount.pl. Hopefully, this will disappear when it is pulled into linux-next.

The second patch is the changes needed for PowerPC in the recordmcount.pl script.

The thrird patch is the cleanup.

All of this is based off of tip/master.

The following patches are in:

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

branch: tip/devel

Matt Fleming (1):
sh: dynamic ftrace support.

Steven Rostedt (2):
ftrace: add support for powerpc to recordmcount.pl script
ftrace: create default variables for archs in recordmcount.pl

----

 scripts/recordmcount.pl |   37 +++++++++++++++++++++++++++++--------
 1 files changed, 29 insertions(+), 8 deletions(-)
author maintainer
#1 lore raw

PATCH 1/3 sh: dynamic ftrace support.

From: Matt Fleming <[email protected]>

First cut at dynamic ftrace support.

[
Steven Rostedt - only updated the recordmcount.pl file.
There are updates for PowerPC that will conflict with this, and we need to base off of these changes.
]

---
 scripts/recordmcount.pl |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)
scripts/recordmcount.pl +11 -0
index eeac71c..9f75438 100755
@@ -170,6 +170,17 @@ if ($arch eq "x86_64") {
     $objcopy .= " -O elf32-i386";
     $cc .= " -m32";
 
+} elsif ($arch eq "sh") {
+    $section_regex = "Disassembly of section\\s+(\\S+):";
+    $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
+    $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
+    $type = ".long";
+
+    # force flags for this arch
+    $ld .= " -m shlelf_linux";
+    $objcopy .= " -O elf32-sh-linux";
+    $cc .= " -m32";
+
 } else {
     die "Arch $arch is not supported with CONFIG_FTRACE_MCOUNT_RECORD";
 }
Signature · 3 lines
1.5.6.5

-- 
#2 lore raw

Re: [PATCH 1/3] sh: dynamic ftrace support.

scripts/recordmcount.pl if ($arch eq "x86_64") { · line 183

Quoted 11 lines · Steven Rostedt
scripts/recordmcount.pl +11 -0
+} elsif ($arch eq "sh") {
+    $section_regex = "Disassembly of section\\s+(\\S+):";
+    $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
+    $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
+    $type = ".long";
+
+    # force flags for this arch
+    $ld .= " -m shlelf_linux";
+    $objcopy .= " -O elf32-sh-linux";
+    $cc .= " -m32";
+

Note that if the $alignment changes are in this version, you will also need to add an $alignment = 2; for sh. That was in Matt's original patch, but I stripped the alignment line out in the commit given that it was not yet present in the mainline version of the script.

#3 lore raw

Re: [PATCH 1/3] sh: dynamic ftrace support.

Quoted 17 lines · Paul Mundt

scripts/recordmcount.pl if ($arch eq "x86_64") { · line 183

Quoted 11 lines · Steven Rostedt
scripts/recordmcount.pl +11 -0
+} elsif ($arch eq "sh") {
+    $section_regex = "Disassembly of section\\s+(\\S+):";
+    $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
+    $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
+    $type = ".long";
+
+    # force flags for this arch
+    $ld .= " -m shlelf_linux";
+    $objcopy .= " -O elf32-sh-linux";
+    $cc .= " -m32";
+

Note that if the $alignment changes are in this version, you will also need to add an $alignment = 2; for sh. That was in Matt's original patch, but I stripped the alignment line out in the commit given that it was not yet present in the mainline version of the script.

I've sent a patch to steven for his tip/devel branch that adds this line.

author maintainer
#4 lore raw

PATCH 2/3 ftrace: add support for powerpc to recordmcount.pl script

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

Impact: Add PowerPC port to recordmcount.pl script

This patch updates the recordmcount.pl script to process PowerPC.

---
 scripts/recordmcount.pl |   19 +++++++++++++++++--
 1 files changed, 17 insertions(+), 2 deletions(-)
scripts/recordmcount.pl +17 -2
index 9f75438..7ec032e 100755
@@ -130,6 +130,7 @@ my %weak;		# List of weak functions
 my %convert;		# List of local functions used that needs conversion
 
 my $type;
+my $nm_regex;		# Find the local functions (return function)
 my $section_regex;	# Find the start of a section
 my $function_regex;	# Find the name of a function
 			#    (return offset and func name)
@@ -145,6 +146,7 @@ if ($arch eq "x86") {
 }
 
 if ($arch eq "x86_64") {
+    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
     $section_regex = "Disassembly of section\\s+(\\S+):";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$";
@@ -158,6 +160,7 @@ if ($arch eq "x86_64") {
     $cc .= " -m64";
 
 } elsif ($arch eq "i386") {
+    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
     $section_regex = "Disassembly of section\\s+(\\S+):";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
@@ -171,6 +174,7 @@ if ($arch eq "x86_64") {
     $cc .= " -m32";
 
 } elsif ($arch eq "sh") {
+    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
     $section_regex = "Disassembly of section\\s+(\\S+):";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
@@ -181,6 +185,17 @@ if ($arch eq "x86_64") {
     $objcopy .= " -O elf32-sh-linux";
     $cc .= " -m32";
 
+} elsif ($arch eq "powerpc") {
+    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\.?\\S+)";
+    $section_regex = "Disassembly of section\\s+(\\S+):";
+    $function_regex = "^([0-9a-fA-F]+)\\s+<(\\.?.*?)>:";
+    $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s\\.?_mcount\$";
+    if ($bits == 64) {
+	$type = ".quad";
+    } else {
+	$type = ".long";
+    }
+
 } else {
     die "Arch $arch is not supported with CONFIG_FTRACE_MCOUNT_RECORD";
 }
@@ -250,7 +265,7 @@ if (!$found_version) {
 #
 open (IN, "$nm $inputfile|") || die "error running $nm";
 while (<IN>) {
-    if (/^[0-9a-fA-F]+\s+t\s+(\S+)/) {
+    if (/$nm_regex/) {
 	$locals{$1} = 1;
     } elsif (/^[0-9a-fA-F]+\s+([wW])\s+(\S+)/) {
 	$weak{$2} = $1;
@@ -302,7 +317,7 @@ sub update_funcs
 	    open(FILE, ">$mcount_s") || die "can't create $mcount_s\n";
 	    $opened = 1;
 	    print FILE "\t.section $mcount_section,\"a\",\@progbits\n";
-	    print FILE "\t.align $alignment\n";
+	    print FILE "\t.align $alignment\n" if (defined($alignment));
 	}
 	printf FILE "\t%s %s + %d\n", $type, $ref_func, $offsets[$i] - $offset;
     }
Signature · 3 lines
1.5.6.5

-- 
author maintainer
#5 lore raw

PATCH 3/3 ftrace: create default variables for archs in recordmcount.pl

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

Impact: cleanup of recordmcount.pl

Now that more architectures are being ported to the MCOUNT_RECORD method, there is no reason to have each declare their own arch specific variable if most of them share the same value. This patch creates a set of default values for the arch specific variables based off of i386.

---
 scripts/recordmcount.pl |   27 +++++++++++----------------
 1 files changed, 11 insertions(+), 16 deletions(-)
scripts/recordmcount.pl +11 -16
index 7ec032e..c5c58ac 100755
@@ -145,10 +145,17 @@ if ($arch eq "x86") {
     }
 }
 
+#
+# We base the defaults off of i386, the other archs may
+# feel free to change them in the below if statements.
+#
+$nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
+$section_regex = "Disassembly of section\\s+(\\S+):";
+$function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
+$mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
+$type = ".long";
+
 if ($arch eq "x86_64") {
-    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
-    $section_regex = "Disassembly of section\\s+(\\S+):";
-    $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$";
     $type = ".quad";
     $alignment = 8;
@@ -160,11 +167,6 @@ if ($arch eq "x86_64") {
     $cc .= " -m64";
 
 } elsif ($arch eq "i386") {
-    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
-    $section_regex = "Disassembly of section\\s+(\\S+):";
-    $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
-    $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
-    $type = ".long";
     $alignment = 4;
 
     # force flags for this arch
@@ -174,11 +176,6 @@ if ($arch eq "x86_64") {
     $cc .= " -m32";
 
 } elsif ($arch eq "sh") {
-    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
-    $section_regex = "Disassembly of section\\s+(\\S+):";
-    $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
-    $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
-    $type = ".long";
 
     # force flags for this arch
     $ld .= " -m shlelf_linux";
@@ -187,13 +184,11 @@ if ($arch eq "x86_64") {
 
 } elsif ($arch eq "powerpc") {
     $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\.?\\S+)";
-    $section_regex = "Disassembly of section\\s+(\\S+):";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(\\.?.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s\\.?_mcount\$";
+
     if ($bits == 64) {
 	$type = ".quad";
-    } else {
-	$type = ".long";
     }
 
 } else {
Signature · 3 lines
1.5.6.5

-- 
#6 lore raw

PATCH 1/1 ftrace: mcountrecord.pl for arm

In reply to Steven Rostedt

Ingo and Steven,

Here is a patch on top of Steven's for recordmcount.pl on arm.

-Jim

---
From: Jim Radford <[email protected]>
Subject: ftrace: mcountrecord.pl for arm

scripts/recordmcount.pl +7 -1
index c5c58ac..3d16795 100755
@@ -135,6 +135,7 @@ my $section_regex;	# Find the start of a section
 my $function_regex;	# Find the name of a function
 			#    (return offset and func name)
 my $mcount_regex;	# Find the call site to mcount (return offset)
+my $section_type;	# Section header plus possible alignment command
 my $alignment;         # The .align value to use for $mcount_section
 
 if ($arch eq "x86") {
@@ -153,6 +154,7 @@ $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
 $section_regex = "Disassembly of section\\s+(\\S+):";
 $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
 $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
+$section_type = '@progbits';
 $type = ".long";
 
 if ($arch eq "x86_64") {
@@ -191,6 +193,10 @@ if ($arch eq "x86_64") {
 	$type = ".quad";
     }
 
+} elsif ($arch eq "arm") {
+    $alignment = 2;
+    $section_type = '%progbits';
+
 } else {
     die "Arch $arch is not supported with CONFIG_FTRACE_MCOUNT_RECORD";
 }
@@ -311,7 +317,7 @@ sub update_funcs
 	if (!$opened) {
 	    open(FILE, ">$mcount_s") || die "can't create $mcount_s\n";
 	    $opened = 1;
-	    print FILE "\t.section $mcount_section,\"a\",\@progbits\n";
+	    print FILE "\t.section $mcount_section,\"a\",$section_type\n";
 	    print FILE "\t.align $alignment\n" if (defined($alignment));
 	}
 	printf FILE "\t%s %s + %d\n", $type, $ref_func, $offsets[$i] - $offset;
#7 lore raw

Re: [PATCH] ftrace: mcountrecord.pl for arm

Ingo and Steven,

Here's an updated version of the arch/arm changes for dynamic ftrace based on top of your latest tip/master.

-Jim

---
From: Jim Radford <[email protected]>
Subject: ftrace: enable dynamic ftrace for arm

Update to the latest api, syncing functions with the x86 versions.

arch/arm/Kconfig +3 -1
Index: linux-2.6/arch/arm/Kconfig
===================================================================
@@ -16,7 +16,9 @@ config ARM
 	select HAVE_ARCH_KGDB
 	select HAVE_KPROBES if (!XIP_KERNEL)
 	select HAVE_KRETPROBES if (HAVE_KPROBES)
-	select HAVE_FUNCTION_TRACER if (!XIP_KERNEL)
+	select HAVE_FTRACE_MCOUNT_RECORD
+	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL)
+	select HAVE_FUNCTION_TRACER
 	select HAVE_GENERIC_DMA_COHERENT
 	help
 	  The ARM series is a line of low-power-consumption RISC chip designs
arch/arm/include/asm/ftrace.h +13 -0
Index: linux-2.6/arch/arm/include/asm/ftrace.h
===================================================================
@@ -7,6 +7,19 @@
 
 #ifndef __ASSEMBLY__
 extern void mcount(void);
+
+static inline unsigned long ftrace_call_adjust(unsigned long addr)
+{
+	return addr;
+}
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+
+struct dyn_arch_ftrace {
+	/* No extra data needed for x86 */
+};
+
+#endif /*  CONFIG_DYNAMIC_FTRACE */
 #endif
 
 #endif
arch/arm/kernel/entry-common.S +1 -8
Index: linux-2.6/arch/arm/kernel/entry-common.S
===================================================================
@@ -104,14 +104,7 @@ ENDPROC(ret_from_fork)
 #ifdef CONFIG_FUNCTION_TRACER
 #ifdef CONFIG_DYNAMIC_FTRACE
 ENTRY(mcount)
-	stmdb sp!, {r0-r3, lr}
-	mov r0, lr
-	sub r0, r0, #MCOUNT_INSN_SIZE
-
-	.globl mcount_call
-mcount_call:
-	bl ftrace_stub
-	ldmia sp!, {r0-r3, pc}
+	mov pc, lr
 
 ENTRY(ftrace_caller)
 	stmdb sp!, {r0-r3, lr}
arch/arm/kernel/ftrace.c +34 -10

76 lines not on this page · raw message

#8 lore raw

Re: [PATCH] ftrace: mcountrecord.pl for arm

Quoted 4 lines · Jim Radford

Ingo and Steven,

Here's an updated version of the arch/arm changes for dynamic ftrace based on top of your latest tip/master.

Excuse me if I'm rather confused, but...

When ftrace for ARM was originally merged, neither linux-arm-kernel nor myself were copied with the patches. Now, I'm being sent updates to code that I've no understanding of and haven't seen before.

I mean, yes, it's nice to be copied with patches which are relevent. It would've been even nicer to have been copied with the patches adding ftrace in the first place, so people knew something about it and were aware of the changes.

It seems to me like there's been a total breakdown of communication when ftrace was initially merged...

So, questions: has ftrace actually been tested on ARM at all? Has it been reviewed? Which ARM platforms has it been tried on? How stable is it? How has it been implemented on ARM? Does it rely on any CPU specific behaviour?

Looking at the git history, ftrace was merged via Ingo, so I assume that Ingo has some understanding of this code. So, for the time being if these are urgent updates, I suggest that updates go through Ingo's tree rather than mine.

And looking at arch/arm/kernel/ftrace.c, it's incompatible with Thumb2 which we've been working towards supporting. What about SMP? ARM is a SMP capable architecture now, and I see no locking in there - what I do see is static data with pointers to it being returned to other code... Yuck.

author maintainer
#9 lore raw

Re: [PATCH] ftrace: mcountrecord.pl for arm

Quoted 12 lines · Russell King - ARM Linux
Quoted 4 lines · Jim Radford

Ingo and Steven,

Here's an updated version of the arch/arm changes for dynamic ftrace based on top of your latest tip/master.

Excuse me if I'm rather confused, but...

When ftrace for ARM was originally merged, neither linux-arm-kernel nor myself were copied with the patches. Now, I'm being sent updates to code that I've no understanding of and haven't seen before.

Quoted 7 lines

I mean, yes, it's nice to be copied with patches which are relevent. It would've been even nicer to have been copied with the patches adding ftrace in the first place, so people knew something about it and were aware of the changes.

It seems to me like there's been a total breakdown of communication when ftrace was initially merged...

Yes I totally agree that in the beginning there was a breakdown of communication. I myself just learned of the ARM port.

Quoted 10 lines

So, questions: has ftrace actually been tested on ARM at all? Has it been reviewed? Which ARM platforms has it been tried on? How stable is it? How has it been implemented on ARM? Does it rely on any CPU specific behaviour?

Looking at the git history, ftrace was merged via Ingo, so I assume that Ingo has some understanding of this code. So, for the time being if these are urgent updates, I suggest that updates go through Ingo's tree rather than mine.

I would suggest that they at least get an ACK from you. The original code should have too.

Quoted 6 lines

And looking at arch/arm/kernel/ftrace.c, it's incompatible with Thumb2 which we've been working towards supporting. What about SMP? ARM is a SMP capable architecture now, and I see no locking in there - what I do see is static data with pointers to it being returned to other code... Yuck.

Some of this code will be redesigned in 29. But as for the locking, this code is run under kstop_machine. Which means that even on SMP architectures, this acts like a UP box. Some of the code can be run outside of kstop_machine, but it is protected by locks in the module code. I'll take a look at the ftrace.c arm code and see if there's any problems with it. I wrote the x86 version as well as the coming PowerPC port.

Thanks,

-- Steve