[PATCH 1/9] tracing: recordmcount.pl Amend the documentation according to the implementation

[PATCH 1/9] tracing: recordmcount.pl Amend the documentation according to the implementation

[Li Hong]

In documentation, we says we will use the first function in a section as a
reference. Actually, the algorithm is: choose the first global function we
meet as a reference. If there is none, choose the first local one. So let the
documentation consistent to the code.

Also add several clarifications.

Signed-off-by: Li Hong <lihong.hi@gmail.com>

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 090d300..ac908b3 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -6,73 +6,88 @@
 #                   all the offsets to the calls to mcount.
 #
 #
-# What we want to end up with is a section in vmlinux called
-# __mcount_loc that contains a list of pointers to all the
-# call sites in the kernel that call mcount. Later on boot up, the kernel
-# will read this list, save the locations and turn them into nops.
-# When tracing or profiling is later enabled, these locations will then
-# be converted back to pointers to some function.
+# What we want to end up with this is a data area in .init.data in vmlinux 
+# that contains a list of pointers to all the call sites in the kernel that
+# call mcount. Later on boot up, the kernel will read this list, save the 
+# locations and turn them into nops. When tracing or profiling is later 
+# enabled, these locations will then be converted back to pointers to 
+# some function.
 #
 # This is no easy feat. This script is called just after the original
 # object is compiled and before it is linked.
 #
-# The references to the call sites are offsets from the section of text
-# that the call site is in. Hence, all functions in a section that
-# has a call site to mcount, will have the offset from the beginning of
-# the section and not the beginning of the function.
+# When parse this object file using 'objdump', the references to the call 
+# sites are offsets from the section that the call site is in. Hence, all 
+# functions in a section that  has a call site to mcount, will have the 
+# offset from the beginning of the section and not the beginning of the 
+# function.
 #
-# The trick is to find a way to record the beginning of the section.
-# The way we do this is to look at the first function in the section
-# which will also be the location of that section after final link.
+# But where this section will reside finally in vmlinx is undetermined at 
+# this point. So we can't use this kind of offsets to record the final
+# address of this call site.
+#
+# The trick is to change the call offset refering the start of a section to
+# refering a function symbol in this section. During the link step, 'ld' will
+# compute the final address according to the information we record.
+# 
 # e.g.
 #
 #  .section ".sched.text", "ax"
-#  .globl my_func
-#  my_func:
 #        [...]
-#        call mcount  (offset: 0x5)
+#  func1:
+#        [...]
+#        call mcount  (offset: 0x10)
+#        [...]
+#        ret
+#  .globl fun2
+#  func2:             (offset: 0x20)
+#        [...]
 #        [...]
 #        ret
-#  other_func:
+#  func3:
 #        [...]
-#        call mcount (offset: 0x1b)
+#        call mcount (offset: 0x30)
 #        [...]
 #
 # Both relocation offsets for the mcounts in the above example will be
-# offset from .sched.text. If we make another file called tmp.s with:
+# offset from .sched.text. If we choose global symbol func2 as a reference and
+# make another file called tmp.s with the new offsets:
 #
 #  .section __mcount_loc
-#  .quad  my_func + 0x5
-#  .quad  my_func + 0x1b
+#  .quad  func2 - 0x10
+#  .quad  func2 + 0x10
 #
-# We can then compile this tmp.s into tmp.o, and link it to the original
+# We can then compile this tmp.s into tmp.o, and link it back to the original
 # object.
 #
-# But this gets hard if my_func is not globl (a static function).
-# In such a case we have:
+# In our algorithm, we will choose the first global function we meet in this 
+# section as the reference. But this gets hard if there is no global functions 
+# in this section. In such a case we have to select a local one. E.g. func1:
 #
 #  .section ".sched.text", "ax"
-#  my_func:
+#  func1:
 #        [...]
-#        call mcount  (offset: 0x5)
+#        call mcount  (offset: 0x10)
 #        [...]
 #        ret
-#  other_func:
+#  func2:
 #        [...]
-#        call mcount (offset: 0x1b)
+#        call mcount (offset: 0x20)
 #        [...]
+#  .section "other.section"
 #
 # If we make the tmp.s the same as above, when we link together with
-# the original object, we will end up with two symbols for my_func:
+# the original object, we will end up with two symbols for func1:
 # one local, one global.  After final compile, we will end up with
-# an undefined reference to my_func.
+# an undefined reference to func1 or a wrong reference to another global
+# func1 in other files.
 #
 # Since local objects can reference local variables, we need to find
 # a way to make tmp.o reference the local objects of the original object
-# file after it is linked together. To do this, we convert the my_func
+# file after it is linked together. To do this, we convert func1
 # into a global symbol before linking tmp.o. Then after we link tmp.o
-# we will only have a single symbol for my_func that is global.
-# We can convert my_func back into a local symbol and we are done.
+# we will only have a single symbol for func1 that is global.
+# We can convert func1 back into a local symbol and we are done.
 #
 # Here are the steps we take:
 #
@@ -86,10 +101,8 @@
 # 6) Link together this new object with the list object.
 # 7) Convert the local functions back to local symbols and rename
 #    the result as the original object.
-#    End.
 # 8) Link the object with the list object.
 # 9) Move the result back to the original object.
-#    End.
 #
 
 use strict;
-- 
1.6.0.4

[PATCH 2/9] tracing: recordmcount.pl Correct the check on the number of parameters

[Li Hong]

>From 2ee0d7914a95461a19f174085a081bad345cddd5 Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 12:27:32 +0800
Subject: [PATCH] tracing: recordmcount.pl Correct the check on the number of parameters

Signed-off-by: Li Hong <lihong.hi@gmail.com>

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index ac908b3..0850b83 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -112,7 +112,7 @@ $P =~ s@.*/@@g;
 
 my $V = '0.1';
 
-if ($#ARGV < 7) {
+if ($#ARGV != 10) {
 	print "usage: $P arch bits objdump objcopy cc ld nm rm mv is_module inputfile\n";
 	print "version: $V\n";
 	exit(1);
-- 
1.6.0.4

Re: [PATCH 3/9] tracing: recordmcount.pl Support absolute path check on $inputfile

[Li Hong]

>From fdf89709ddf4a6ff2f2d21ff8a964e949a069fe4 Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 12:32:18 +0800
Subject: [PATCH] tracing: recordmcount.pl Support absolute path check on $inputfile

Signed-off-by: Li Hong <lihong.hi@gmail.com>

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 0850b83..94daf9e 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -122,9 +122,7 @@ my ($arch, $bits, $objdump, $objcopy, $cc,
     $ld, $nm, $rm, $mv, $is_module, $inputfile) = @ARGV;
 
 # This file refers to mcount and shouldn't be ftraced, so lets' ignore it
-if ($inputfile eq "kernel/trace/ftrace.o") {
-    exit(0);
-}
+exit (0) if ($inputfile =~ "kernel/trace/ftrace.o");
 
 # Acceptable sections to record.
 my %text_sections = (
-- 
1.6.0.4

[PATCH 4/9] tracing: recordmcount.pl Objcopy check should disable local reference correctly

[Li Hong]

>From 580ea035d04d4ca58300423db3dd5b4c73c8d61c Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 12:41:22 +0800
Subject: [PATCH] tracing: recordmcount.pl Objcopy check should disable local reference correctly

Use a function to do objcopy version check. Disable the local reference if
copy can't support it. Remove some unused variables.

Signed-off-by: Li Hong <lihong.hi@gmail.com>

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 94daf9e..970c6d6 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -155,6 +155,28 @@ my $function_regex;	# Find the name of a function
 my $mcount_regex;	# Find the call site to mcount (return offset)
 my $alignment;		# The .align value to use for $mcount_section
 my $section_type;	# Section header plus possible alignment command
+my $can_use_local = 0; 	# If we can use local function references
+
+##
+# check_objcopy - whether objcopy supports --globalize-symbols
+#
+#  --globalize-symbols came out in 2.17, we must test the version
+#  of objcopy, and if it is less than 2.17, then we can not
+#  record local functions.
+sub check_objcopy
+{
+    open (IN, "$objcopy --version |") or die "error running $objcopy";
+    while (<IN>) {
+        if (/objcopy.*\s(\d+)\.(\d+)/) {
+            $can_use_local = 1 if ($1 > 2 || ($1 == 2 && $2 >= 17)); 
+            last;
+        }
+    }
+    close (IN);
+
+    print STDERR "WARNING: could not find objcopy version or version is less than 2.17.\n" .
+	"\tLocal function references is disabled.\n" if !$can_use_local; 
+}
 
 if ($arch eq "x86") {
     if ($bits == 64) {
@@ -289,34 +311,7 @@ if ($filename =~ m,^(.*)(\.\S),) {
 my $mcount_s = $dirname . "/.tmp_mc_" . $prefix . ".s";
 my $mcount_o = $dirname . "/.tmp_mc_" . $prefix . ".o";
 
-#
-# --globalize-symbols came out in 2.17, we must test the version
-# of objcopy, and if it is less than 2.17, then we can not
-# record local functions.
-my $use_locals = 01;
-my $local_warn_once = 0;
-my $found_version = 0;
-
-open (IN, "$objcopy --version |") || die "error running $objcopy";
-while (<IN>) {
-    if (/objcopy.*\s(\d+)\.(\d+)/) {
-	my $major = $1;
-	my $minor = $2;
-
-	$found_version = 1;
-	if ($major < 2 ||
-	    ($major == 2 && $minor < 17)) {
-	    $use_locals = 0;
-	}
-	last;
-    }
-}
-close (IN);
-
-if (!$found_version) {
-    print STDERR "WARNING: could not find objcopy version.\n" .
-	"\tDisabling local function references.\n";
-}
+check_objcopy();
 
 #
 # Step 1: find all the local (static functions) and weak symbols.
@@ -363,7 +358,7 @@ sub update_funcs
     if (defined $locals{$ref_func}) {
 
 	# only use locals if objcopy supports globalize-symbols
-	if (!$use_locals) {
+	if (!$can_use_local) {
 	    return;
 	}
 	$convert{$ref_func} = 1;
-- 
1.6.0.4

[PATCH 5/9] tracing: recordmcount.pl Clarify the logic on mcount section check

[Li Hong]

>From eb8a2d53bc2484be223e4fa0df8804389e969b72 Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 12:53:52 +0800
Subject: [PATCH] tracing: recordmcount.pl Clarify the logic on mcount section check

Move the mcount section check to the beginning of the objdump read loop.
It is clearer, because mcount section check uses headers dump part of objdump
to identify a mcount section, which goes before the section parts.

Signed-off-by: Li Hong <lihong.hi@gmail.com>

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 970c6d6..a6585b6 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -384,9 +384,26 @@ open(IN, "$objdump -hdr $inputfile|") || die "error running $objdump";
 
 my $text;
 
-my $read_headers = 1;
 
 while (<IN>) {
+    # read headers first
+    my $read_headers = 1;
+    
+    if ($read_headers && /$mcount_section/) {
+	#
+	# Somehow the make process can execute this script on an
+	# object twice. If it does, we would duplicate the mcount
+	# section and it will cause the function tracer self test
+	# to fail. Check if the mcount section exists, and if it does,
+	# warn and exit.
+	#
+	print STDERR "ERROR: $mcount_section already in $inputfile\n" .
+	    "\tThis may be an indication that your build is corrupted.\n" .
+	    "\tDelete $inputfile and try again. If the same object file\n" .
+	    "\tstill causes an issue, then disable CONFIG_DYNAMIC_FTRACE.\n";
+	exit(-1);
+    }
+
     # is it a section?
     if (/$section_regex/) {
 	$read_headers = 0;
@@ -427,21 +444,7 @@ while (<IN>) {
 		$offset = hex $1;
 	    }
 	}
-    } elsif ($read_headers && /$mcount_section/) {
-	#
-	# Somehow the make process can execute this script on an
-	# object twice. If it does, we would duplicate the mcount
-	# section and it will cause the function tracer self test
-	# to fail. Check if the mcount section exists, and if it does,
-	# warn and exit.
-	#
-	print STDERR "ERROR: $mcount_section already in $inputfile\n" .
-	    "\tThis may be an indication that your build is corrupted.\n" .
-	    "\tDelete $inputfile and try again. If the same object file\n" .
-	    "\tstill causes an issue, then disable CONFIG_DYNAMIC_FTRACE.\n";
-	exit(-1);
-    }
-
+    } 
     # is this a call site to mcount? If so, record it to print later
     if ($text_found && /$mcount_regex/) {
 	$offsets[$#offsets + 1] = hex $1;
-- 
1.6.0.4

[PATCH 6/9] tracing: recordmcount.pl Exit early if no work to do

[Li Hong]

>From 1589563d8113db41fa77dd657459b563dcecd389 Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 13:13:37 +0800
Subject: [PATCH] tracing: recordmcount.pl Exit early if no work to do

Also keep the global symbols and use it to check if no work to do, exit
early.

Signed-off-by: Li Hong <lihong.hi@gmail.com>

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index a6585b6..d750da8 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -91,7 +91,7 @@
 #
 # Here are the steps we take:
 #
-# 1) Record all the local symbols by using 'nm'
+# 1) Record all the global, local and weak symbols by using 'nm'
 # 2) Use objdump to find all the call site offsets and sections for
 #    mcount.
 # 3) Compile the list into its own object.
@@ -143,12 +143,15 @@ $mv = "mv" if ((length $mv) == 0);
 #print STDERR "running: $P '$arch' '$objdump' '$objcopy' '$cc' '$ld' " .
 #    "'$nm' '$rm' '$mv' '$inputfile'\n";
 
+my %globals;		# List of global functions
 my %locals;		# List of local (static) functions
 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 $global_regex;	# Match a global function (return function)
+my $local_regex;	# Match a local function (return function)
+my $weak_regex; 	# Match a weak function (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)
@@ -190,7 +193,9 @@ 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+)";
+$global_regex = "^[0-9a-fA-F]+\\s+T\\s+(\\S+)";
+$local_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
+$weak_regex = "^[0-9a-fA-F]+\\s+([wW])\\s+(\\S+)";
 $section_regex = "Disassembly of section\\s+(\\S+):";
 $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
 $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
@@ -239,7 +244,8 @@ if ($arch eq "x86_64") {
     $cc .= " -m32";
 
 } elsif ($arch eq "powerpc") {
-    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\.?\\S+)";
+    $global_regex = "^[0-9a-fA-F]+\\s+T\\s+(\\.?\\S+)";
+    $local_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\.?\\S+)";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(\\.?.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s\\.?_mcount\$";
 
@@ -314,19 +320,24 @@ my $mcount_o = $dirname . "/.tmp_mc_" . $prefix . ".o";
 check_objcopy();
 
 #
-# Step 1: find all the local (static functions) and weak symbols.
-#        't' is local, 'w/W' is weak (we never use a weak function)
+# Step 1: find all the global and local (static functions) and weak symbols.
+#        'T' is global 't' is local, 'w/W' is weak
 #
 open (IN, "$nm $inputfile|") || die "error running $nm";
 while (<IN>) {
-    if (/$nm_regex/) {
+    if (/$global_regex/) {
+        $globals{$1} = 1;
+    } elsif (/$local_regex/) {
 	$locals{$1} = 1;
-    } elsif (/^[0-9a-fA-F]+\s+([wW])\s+(\S+)/) {
+    } elsif (/$weak_regex/) {
 	$weak{$2} = $1;
     }
 }
 close(IN);
 
+# Exit early if no work to do 
+exit(0) unless (%globals or (%locals and $can_use_local)); 
+
 my @offsets;		# Array of offsets of mcount callers
 my $ref_func;		# reference function to use for offsets
 my $offset = 0;		# offset of ref_func to section beginning
-- 
1.6.0.4

[PATCH 7/9] tracing: recordmcount.pl Combine the condition validation in update_funcs

[Li Hong]

>From d0aa9b99e4ffd13fada96bfe1651055b459e9988 Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 13:20:48 +0800
Subject: [PATCH] tracing: recordmcount.pl Combine the condition validation in update_funcs

Move all the condition validation into function update_funcs. Also
update_funcs shouldn't die if $ref_func is undefined for there may be
more than one valid sections in an object file.

Signed-off-by: Li Hong <lihong.hi@gmail.com>

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index d750da8..490b4cd 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -350,9 +350,7 @@ my $offset = 0;		# offset of ref_func to section beginning
 #
 sub update_funcs
 {
-    return if ($#offsets < 0);
-
-    defined($ref_func) || die "No function to reference";
+    return unless ($ref_func and @offsets);
 
     # A section only had a weak function, to represent it.
     # Unfortunately, a weak function may be overwritten by another
@@ -426,7 +424,7 @@ while (<IN>) {
 	    $read_function = 0;
 	}
 	# print out any recorded offsets
-	update_funcs() if (defined($ref_func));
+	update_funcs();
 
 	# reset all markers and arrays
 	$text_found = 0;
@@ -463,7 +461,7 @@ while (<IN>) {
 }
 
 # dump out anymore offsets that may have been found
-update_funcs() if (defined($ref_func));
+update_funcs();
 
 # If we did not find any mcount callers, we are done (do nothing).
 if (!$opened) {
-- 
1.6.0.4

[PATCH 8/9] tracing: recordmcount.pl We won't use weak function as reference, remove the check

[Li Hong]

>From 4433c78ca3c0319fd75d1fd6d64929fa101e38a4 Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 13:25:39 +0800
Subject: [PATCH] tracing: recordmcount.pl We won't use weak function as reference, remove the check

Signed-off-by: Li Hong <lihong.hi@gmail.com>

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 490b4cd..867c24a 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -352,17 +352,6 @@ sub update_funcs
 {
     return unless ($ref_func and @offsets);
 
-    # A section only had a weak function, to represent it.
-    # Unfortunately, a weak function may be overwritten by another
-    # function of the same name, making all these offsets incorrect.
-    # To be safe, we simply print a warning and bail.
-    if (defined $weak{$ref_func}) {
-	print STDERR
-	    "$inputfile: WARNING: referencing weak function" .
-	    " $ref_func for mcount\n";
-	return;
-    }
-
     # is this function static? If so, note this fact.
     if (defined $locals{$ref_func}) {
 
-- 
1.6.0.4

[PATCH 9/9] tracing: recordmcount.pl Remove the redundant code

[Li Hong]

>From d0aa71d523b6315fd3ea0ee66fddb020a625856f Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 13:28:53 +0800
Subject: [PATCH] tracing: recordmcount.pl Remove the redundant code

If an object file has some local symbols and objcopy doesn't support local
reference, we have exited before the real work starts. So remove the redundant
check in update_funcs.

Signed-off-by: Li Hong <lihong.hi@gmail.com>

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 867c24a..ff3a9eb 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -353,14 +353,7 @@ sub update_funcs
     return unless ($ref_func and @offsets);
 
     # is this function static? If so, note this fact.
-    if (defined $locals{$ref_func}) {
-
-	# only use locals if objcopy supports globalize-symbols
-	if (!$can_use_local) {
-	    return;
-	}
-	$convert{$ref_func} = 1;
-    }
+    $convert{$ref_func} = 1 if $locals{$ref_func}; 
 
     # Loop through all the mcount caller offsets and print a reference
     # to the caller based from the ref_func.
-- 
1.6.0.4

Re: [PATCH 1/9] tracing: recordmcount.pl Amend the documentation according to the implementation

[Steven Rostedt]

On Tue, 2009-10-27 at 14:54 +0800, Li Hong wrote:
> In documentation, we says we will use the first function in a section as a
> reference. Actually, the algorithm is: choose the first global function we
> meet as a reference. If there is none, choose the first local one. So let the
> documentation consistent to the code.
> 
> Also add several clarifications.
> 
> Signed-off-by: Li Hong <lihong.hi@gmail.com>
> 
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index 090d300..ac908b3 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -6,73 +6,88 @@
>  #                   all the offsets to the calls to mcount.
>  #
>  #
> -# What we want to end up with is a section in vmlinux called
> -# __mcount_loc that contains a list of pointers to all the
> -# call sites in the kernel that call mcount. Later on boot up, the kernel
> -# will read this list, save the locations and turn them into nops.
> -# When tracing or profiling is later enabled, these locations will then
> -# be converted back to pointers to some function.
> +# What we want to end up with this is a data area in .init.data in vmlinux 
> +# that contains a list of pointers to all the call sites in the kernel that
> +# call mcount. Later on boot up, the kernel will read this list, save the 
> +# locations and turn them into nops. When tracing or profiling is later 
> +# enabled, these locations will then be converted back to pointers to 
> +# some function.

Yes it is in .init.data, but the point is that we called it __mcount_loc
in the object files. I don't want to blindly throw that name away.

Something to the lines of this.

Each object file will have a section called __mcount_loc that will hold
the list of pointers to mcount callers. After final linking, the vmlinux
will have within .init.data the list of all callers to mcount between
_start_mcount_loc and __stop_mcount_loc.


>  #
>  # This is no easy feat. This script is called just after the original
>  # object is compiled and before it is linked.
>  #
> -# The references to the call sites are offsets from the section of text
> -# that the call site is in. Hence, all functions in a section that
> -# has a call site to mcount, will have the offset from the beginning of
> -# the section and not the beginning of the function.
> +# When parse this object file using 'objdump', the references to the call 
> +# sites are offsets from the section that the call site is in. Hence, all 
> +# functions in a section that  has a call site to mcount, will have the 
                                ^^
                            double space

> +# offset from the beginning of the section and not the beginning of the 
> +# function.
>  #
> -# The trick is to find a way to record the beginning of the section.
> -# The way we do this is to look at the first function in the section
> -# which will also be the location of that section after final link.
> +# But where this section will reside finally in vmlinx is undetermined at 
> +# this point. So we can't use this kind of offsets to record the final
> +# address of this call site.
> +#
> +# The trick is to change the call offset refering the start of a section to
                                         referring

> +# refering a function symbol in this section. During the link step, 'ld' will
     referring


The rest looks good.

-- Steve

> +# compute the final address according to the information we record.
> +# 
>  # e.g.
>  #
>  #  .section ".sched.text", "ax"
> -#

Re: [PATCH 3/9] tracing: recordmcount.pl Support absolute path check on $inputfile

[Steven Rostedt]

On Tue, 2009-10-27 at 14:58 +0800, Li Hong wrote:
> >From fdf89709ddf4a6ff2f2d21ff8a964e949a069fe4 Mon Sep 17 00:00:00 2001
> From: Li Hong <lihong.hi@gmail.com>
> Date: Tue, 27 Oct 2009 12:32:18 +0800
> Subject: [PATCH] tracing: recordmcount.pl Support absolute path check on $inputfile
> 
> Signed-off-by: Li Hong <lihong.hi@gmail.com>
> 
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index 0850b83..94daf9e 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -122,9 +122,7 @@ my ($arch, $bits, $objdump, $objcopy, $cc,
>      $ld, $nm, $rm, $mv, $is_module, $inputfile) = @ARGV;
>  
>  # This file refers to mcount and shouldn't be ftraced, so lets' ignore it
> -if ($inputfile eq "kernel/trace/ftrace.o") {
> -    exit(0);
> -}
> +exit (0) if ($inputfile =~ "kernel/trace/ftrace.o");

I prefer to keep the C like notation for error paths. Less to scare C
programers from perl the better.

Also, if you are using a regular expression to match, might as well do
it fully:

if ($inputfile =~ m,kernel/tracel/ftrace\.o$,) {
	exit(0);
}

-- Steve

Re: [PATCH 4/9] tracing: recordmcount.pl Objcopy check should disable local reference correctly

[Steven Rostedt]

On Tue, 2009-10-27 at 15:00 +0800, Li Hong wrote:
> >From 580ea035d04d4ca58300423db3dd5b4c73c8d61c Mon Sep 17 00:00:00 2001
> From: Li Hong <lihong.hi@gmail.com>
> Date: Tue, 27 Oct 2009 12:41:22 +0800
> Subject: [PATCH] tracing: recordmcount.pl Objcopy check should disable local reference correctly
> 
> Use a function to do objcopy version check. Disable the local reference if
> copy can't support it. Remove some unused variables.
> 
> Signed-off-by: Li Hong <lihong.hi@gmail.com>
> 
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index 94daf9e..970c6d6 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -155,6 +155,28 @@ my $function_regex;	# Find the name of a function
>  my $mcount_regex;	# Find the call site to mcount (return offset)
>  my $alignment;		# The .align value to use for $mcount_section
>  my $section_type;	# Section header plus possible alignment command
> +my $can_use_local = 0; 	# If we can use local function references
> +
> +##
> +# check_objcopy - whether objcopy supports --globalize-symbols
> +#
> +#  --globalize-symbols came out in 2.17, we must test the version
> +#  of objcopy, and if it is less than 2.17, then we can not
> +#  record local functions.
> +sub check_objcopy
> +{
> +    open (IN, "$objcopy --version |") or die "error running $objcopy";
> +    while (<IN>) {
> +        if (/objcopy.*\s(\d+)\.(\d+)/) {
> +            $can_use_local = 1 if ($1 > 2 || ($1 == 2 && $2 >= 17)); 
> +            last;
> +        }
> +    }
> +    close (IN);
> +
> +    print STDERR "WARNING: could not find objcopy version or version is less than 2.17.\n" .
> +	"\tLocal function references is disabled.\n" if !$can_use_local; 

Please use C like if's. This tricked even me. I looked at it, and said
to myself, "Wait, this prints an error every time... oh wait!".

There are some cases where I don't mind the perl if style, and used it
for assigning defaults. But this just makes it more difficult to
understand.

The rest of the patch looks good.

-- Steve

> +}
>  
>  if ($arch eq "x86") {
>      if ($bits == 64) {
> @@ -289,34 +311,7 @@ if ($filename =~ m,^(.*)(\.\S),) {
>  my $mcount_s = $dirname . "/.tmp_mc_" . $prefix . ".s";
>  my $mcount_o = $dirname . "/.tmp_mc_" . $prefix . ".o";
>  
> -#
> -# --globalize-symbols came out in 2.17, we must test the version
> -# of objcopy, and if it is less than 2.17, then we can not
> -# record local functions.
> -my $use_locals = 01;
> -my $local_warn_once = 0;
> -my $found_version = 0;
> -
> -open (IN, "$objcopy --version |") || die "error running $objcopy";
> -while (<IN>) {
> -    if (/objcopy.*\s(\d+)\.(\d+)/) {
> -	my $major = $1;
> -	my $minor = $2;
> -
> -	$found_version = 1;
> -	if ($major < 2 ||
> -	    ($major == 2 && $minor < 17)) {
> -	    $use_locals = 0;
> -	}
> -	last;
> -    }
> -}
> -close (IN);
> -
> -if (!$found_version) {
> -    print STDERR "WARNING: could not find objcopy version.\n" .
> -	"\tDisabling local function references.\n";
> -}
> +check_objcopy();
>  
>  #
>  # Step 1: find all the local (static functions) and weak symbols.
> @@ -363,7 +358,7 @@ sub update_funcs
>      if (defined $locals{$ref_func}) {
>  
>  	# only use locals if objcopy supports globalize-symbols
> -	if (!$use_locals) {
> +	if (!$can_use_local) {
>  	    return;
>  	}
>  	$convert{$ref_func} = 1;

Re: [PATCH 5/9] tracing: recordmcount.pl Clarify the logic on mcount section check

[Steven Rostedt]

On Tue, 2009-10-27 at 15:01 +0800, Li Hong wrote:
> >From eb8a2d53bc2484be223e4fa0df8804389e969b72 Mon Sep 17 00:00:00 2001
> From: Li Hong <lihong.hi@gmail.com>
> Date: Tue, 27 Oct 2009 12:53:52 +0800
> Subject: [PATCH] tracing: recordmcount.pl Clarify the logic on mcount section check
> 
> Move the mcount section check to the beginning of the objdump read loop.
> It is clearer, because mcount section check uses headers dump part of objdump
> to identify a mcount section, which goes before the section parts.
> 
> Signed-off-by: Li Hong <lihong.hi@gmail.com>
> 
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index 970c6d6..a6585b6 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -384,9 +384,26 @@ open(IN, "$objdump -hdr $inputfile|") || die "error running $objdump";
>  
>  my $text;
>  
> -my $read_headers = 1;
>  
>  while (<IN>) {
> +    # read headers first
> +    my $read_headers = 1;
> +    

Please keep the $read_headers initialization outside of the while loop.
It makes it easier for C programers to understand. Otherwise it looks
like it gets reset every time.

-- Steve

> +    if ($read_headers && /$mcount_section/) {
> +	#
> +	# Somehow the make process can execute this script on an
> +	# object twice. If it does, we would duplicate the mcount
> +	# section and it will cause the function tracer self test
> +	# to fail. Check if the mcount section exists, and if it does,
> +	# warn and exit.
> +	#
> +	print STDERR "ERROR: $mcount_section already in $inputfile\n" .
> +	    "\tThis may be an indication that your build is corrupted.\n" .
> +	    "\tDelete $inputfile and try again. If the same object file\n" .
> +	    "\tstill causes an issue, then disable CONFIG_DYNAMIC_FTRACE.\n";
> +	exit(-1);
> +    }
> +
>      # is it a section?
>      if (/$section_regex/) {
>  	$read_headers = 0;
> @@ -427,21 +444,7 @@ while (<IN>) {
>  		$offset = hex $1;
>  	    }
>  	}
> -    } elsif ($read_headers && /$mcount_section/) {
> -	#
> -	# Somehow the make process can execute this script on an
> -	# object twice. If it does, we would duplicate the mcount
> -	# section and it will cause the function tracer self test
> -	# to fail. Check if the mcount section exists, and if it does,
> -	# warn and exit.
> -	#
> -	print STDERR "ERROR: $mcount_section already in $inputfile\n" .
> -	    "\tThis may be an indication that your build is corrupted.\n" .
> -	    "\tDelete $inputfile and try again. If the same object file\n" .
> -	    "\tstill causes an issue, then disable CONFIG_DYNAMIC_FTRACE.\n";
> -	exit(-1);
> -    }
> -
> +    } 
>      # is this a call site to mcount? If so, record it to print later
>      if ($text_found && /$mcount_regex/) {
>  	$offsets[$#offsets + 1] = hex $1;

Re: [PATCH 6/9] tracing: recordmcount.pl Exit early if no work to do

[Steven Rostedt]

On Tue, 2009-10-27 at 15:02 +0800, Li Hong wrote:
> >From 1589563d8113db41fa77dd657459b563dcecd389 Mon Sep 17 00:00:00 2001
> From: Li Hong <lihong.hi@gmail.com>
> Date: Tue, 27 Oct 2009 13:13:37 +0800
> Subject: [PATCH] tracing: recordmcount.pl Exit early if no work to do
> 
> Also keep the global symbols and use it to check if no work to do, exit
> early.

How many files does this fall out of?  This looks like added complexity
for no real gain.

-- Steve

> 
> Signed-off-by: Li Hong <lihong.hi@gmail.com>
> 
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index a6585b6..d750da8 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -91,7 +91,7 @@
>  #
>  # Here are the steps we take:
>  #
> -# 1) Record all the local symbols by using 'nm'
> +# 1) Record all the global, local and weak symbols by using 'nm'
>  # 2) Use objdump to find all the call site offsets and sections for
>  #    mcount.
>  # 3) Compile the list into its own object.
> @@ -143,12 +143,15 @@ $mv = "mv" if ((length $mv) == 0);
>  #print STDERR "running: $P '$arch' '$objdump' '$objcopy' '$cc' '$ld' " .
>  #    "'$nm' '$rm' '$mv' '$inputfile'\n";
>  
> +my %globals;		# List of global functions
>  my %locals;		# List of local (static) functions
>  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 $global_regex;	# Match a global function (return function)
> +my $local_regex;	# Match a local function (return function)
> +my $weak_regex; 	# Match a weak function (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)
> @@ -190,7 +193,9 @@ 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+)";
> +$global_regex = "^[0-9a-fA-F]+\\s+T\\s+(\\S+)";
> +$local_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
> +$weak_regex = "^[0-9a-fA-F]+\\s+([wW])\\s+(\\S+)";
>  $section_regex = "Disassembly of section\\s+(\\S+):";
>  $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
>  $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
> @@ -239,7 +244,8 @@ if ($arch eq "x86_64") {
>      $cc .= " -m32";
>  
>  } elsif ($arch eq "powerpc") {
> -    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\.?\\S+)";
> +    $global_regex = "^[0-9a-fA-F]+\\s+T\\s+(\\.?\\S+)";
> +    $local_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\.?\\S+)";
>      $function_regex = "^([0-9a-fA-F]+)\\s+<(\\.?.*?)>:";
>      $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s\\.?_mcount\$";
>  
> @@ -314,19 +320,24 @@ my $mcount_o = $dirname . "/.tmp_mc_" . $prefix . ".o";
>  check_objcopy();
>  
>  #
> -# Step 1: find all the local (static functions) and weak symbols.
> -#        't' is local, 'w/W' is weak (we never use a weak function)
> +# Step 1: find all the global and local (static functions) and weak symbols.
> +#        'T' is global 't' is local, 'w/W' is weak
>  #
>  open (IN, "$nm $inputfile|") || die "error running $nm";
>  while (<IN>) {
> -    if (/$nm_regex/) {
> +    if (/$global_regex/) {
> +        $globals{$1} = 1;
> +    } elsif (/$local_regex/) {
>  	$locals{$1} = 1;
> -    } elsif (/^[0-9a-fA-F]+\s+([wW])\s+(\S+)/) {
> +    } elsif (/$weak_regex/) {
>  	$weak{$2} = $1;
>      }
>  }
>  close(IN);
>  
> +# Exit early if no work to do 
> +exit(0) unless (%globals or (%locals and $can_use_local)); 
> +
>  my @offsets;		# Array of offsets of mcount callers
>  my $ref_func;		# reference function to use for offsets
>  my $offset = 0;		# offset of ref_func to section beginning

Re: [PATCH 7/9] tracing: recordmcount.pl Combine the condition validation in update_funcs

[Steven Rostedt]

On Tue, 2009-10-27 at 15:03 +0800, Li Hong wrote:
> >From d0aa9b99e4ffd13fada96bfe1651055b459e9988 Mon Sep 17 00:00:00 2001
> From: Li Hong <lihong.hi@gmail.com>
> Date: Tue, 27 Oct 2009 13:20:48 +0800
> Subject: [PATCH] tracing: recordmcount.pl Combine the condition validation in update_funcs
> 
> Move all the condition validation into function update_funcs. Also
> update_funcs shouldn't die if $ref_func is undefined for there may be
> more than one valid sections in an object file.
> 

Nice clean up!

-- Steve

> Signed-off-by: Li Hong <lihong.hi@gmail.com>
> 
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index d750da8..490b4cd 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -350,9 +350,7 @@ my $offset = 0;		# offset of ref_func to section beginning
>  #
>  sub update_funcs
>  {
> -    return if ($#offsets < 0);
> -
> -    defined($ref_func) || die "No function to reference";
> +    return unless ($ref_func and @offsets);
>  
>      # A section only had a weak function, to represent it.
>      # Unfortunately, a weak function may be overwritten by another
> @@ -426,7 +424,7 @@ while (<IN>) {
>  	    $read_function = 0;
>  	}
>  	# print out any recorded offsets
> -	update_funcs() if (defined($ref_func));
> +	update_funcs();
>  
>  	# reset all markers and arrays
>  	$text_found = 0;
> @@ -463,7 +461,7 @@ while (<IN>) {
>  }
>  
>  # dump out anymore offsets that may have been found
> -update_funcs() if (defined($ref_func));
> +update_funcs();
>  
>  # If we did not find any mcount callers, we are done (do nothing).
>  if (!$opened) {

Re: [PATCH 8/9] tracing: recordmcount.pl We won't use weak function as reference, remove the check

[Steven Rostedt]

On Tue, 2009-10-27 at 15:04 +0800, Li Hong wrote:
> >From 4433c78ca3c0319fd75d1fd6d64929fa101e38a4 Mon Sep 17 00:00:00 2001
> From: Li Hong <lihong.hi@gmail.com>
> Date: Tue, 27 Oct 2009 13:25:39 +0800
> Subject: [PATCH] tracing: recordmcount.pl We won't use weak function as reference, remove the check
> 
> Signed-off-by: Li Hong <lihong.hi@gmail.com>
> 
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index 490b4cd..867c24a 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -352,17 +352,6 @@ sub update_funcs
>  {
>      return unless ($ref_func and @offsets);
>  
> -    # A section only had a weak function, to represent it.
> -    # Unfortunately, a weak function may be overwritten by another
> -    # function of the same name, making all these offsets incorrect.
> -    # To be safe, we simply print a warning and bail.
> -    if (defined $weak{$ref_func}) {
> -	print STDERR
> -	    "$inputfile: WARNING: referencing weak function" .
> -	    " $ref_func for mcount\n";
> -	return;
> -    }
> -

I consider this a sanity check. It may be triggered if someone modifies
the rest of the script and somehow a weak function gets through. I'd
like to keep it. It does not hurt to have it. Maybe we should make it
"die" instead of just returning.

-- Steve

>      # is this function static? If so, note this fact.
>      if (defined $locals{$ref_func}) {
>  

Re: [PATCH 9/9] tracing: recordmcount.pl Remove the redundant code

[Steven Rostedt]

On Tue, 2009-10-27 at 15:05 +0800, Li Hong wrote:
> >From d0aa71d523b6315fd3ea0ee66fddb020a625856f Mon Sep 17 00:00:00 2001
> From: Li Hong <lihong.hi@gmail.com>
> Date: Tue, 27 Oct 2009 13:28:53 +0800
> Subject: [PATCH] tracing: recordmcount.pl Remove the redundant code
> 
> If an object file has some local symbols and objcopy doesn't support local
> reference, we have exited before the real work starts. So remove the redundant
> check in update_funcs.

Are you sure? Where does it exit? Even if I add patch 6, you can still
have a section that does not have any globals in it.

-- Steve

> 
> Signed-off-by: Li Hong <lihong.hi@gmail.com>
> 
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index 867c24a..ff3a9eb 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -353,14 +353,7 @@ sub update_funcs
>      return unless ($ref_func and @offsets);
>  
>      # is this function static? If so, note this fact.
> -    if (defined $locals{$ref_func}) {
> -
> -	# only use locals if objcopy supports globalize-symbols
> -	if (!$can_use_local) {
> -	    return;
> -	}
> -	$convert{$ref_func} = 1;
> -    }
> +    $convert{$ref_func} = 1 if $locals{$ref_func}; 
>  
>      # Loop through all the mcount caller offsets and print a reference
>      # to the caller based from the ref_func.

[tip:tracing/core] tracing: Correct the check for number of arguments in recordmcount.pl

[tip-bot for Li Hong]

Commit-ID:  e2d753fac5b3954a3b6001f98479f0435fe7c868
Gitweb:     http://git.kernel.org/tip/e2d753fac5b3954a3b6001f98479f0435fe7c868
Author:     Li Hong <lihong.hi@gmail.com>
AuthorDate: Tue, 27 Oct 2009 14:57:33 +0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 29 Oct 2009 15:11:41 -0400

tracing: Correct the check for number of arguments in recordmcount.pl

The number of arguments passed into recordmcount.pl is 10, but the code
checks if only 7 are passed in.

Signed-off-by: Li Hong <lihong.hi@gmail.com>
LKML-Reference: <20091027065733.GB22032@uhli>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.pl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index a569be7..a512af1 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -113,7 +113,7 @@ $P =~ s@.*/@@g;
 
 my $V = '0.1';
 
-if ($#ARGV < 7) {
+if ($#ARGV != 10) {
 	print "usage: $P arch bits objdump objcopy cc ld nm rm mv is_module inputfile\n";
 	print "version: $V\n";
 	exit(1);