Skip to contentSkip to conversation

ftrace: clean ups and fixes

PATCH Superseded Steven Rostedt 5 participants 37 messages Oct 22, 2008 – Oct 31, 2008

Subsystems (6)
Arm (32-bit) x86 PowerPC Kbuild +2 more subsystems

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

cover ftrace: clean ups and fixes

This is a series of patches to make ftrace more robust and clean ups.

The first couple of patches fix the recordmount.pl script and changes it to only record the .text section functions. This means that the init sections will not be processed.

I still have a patch to add notrace to the init sections, and not for safety reasons, but for perfomance. Since the init sections will not be processed, they will still call mcount. Note, mcount is just a ret, but why have the init code waste CPU cycles to call a stub function?

A FTRACE_WARN_ON is added to change all WARN_ONS to not only print a warning, but also to disable ftrace as well.

The later patches are a bit more drastic. Since the daemon is error prone, I stripped it out. In doing so, I have to disable dynamic ftrace from all archs that use it. The archs can get dynamic ftrace reenabled when they are ported to the recordmcount.pl method. (Arch maintainers, please contact me if you want help. I can do it with some information about your arch).

Since the hash was created to work with the daemon, that too was stripped out, making the remaining code smaller and cleaner. The kprobe hooks in ftrace may need to be reworked.

-- Steve

author maintainer
#1 lore raw

PATCH 1/11 ftrace: handle generic arch calls

The recordmcount script requires that the actual arch is passed in. This works well when ARCH=i386 or ARCH=x86_64 but does not handle the case of ARCH=x86.

This patch adds a parameter to the function to pass in the number of bits of the architecture. So that it can determine if x86 should be run for x86_64 or i386 archs.

---
 scripts/Makefile.build  |   10 ++++++++--
 scripts/recordmcount.pl |   11 ++++++++++-
 2 files changed, 18 insertions(+), 3 deletions(-)
scripts/Makefile.build +8 -2
Index: linux-compile.git/scripts/Makefile.build
===================================================================
@@ -198,10 +198,16 @@ cmd_modversions =							\
 	fi;
 endif
 
+ifdef CONFIG_64BIT
+arch_bits = 64
+else
+arch_bits = 32
+endif
+
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
 cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl \
-	"$(ARCH)" "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" "$(NM)" "$(RM)" \
-	"$(MV)" "$(@)";
+	"$(ARCH)" "$(arch_bits)" "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" \
+	"$(NM)" "$(RM)" "$(MV)" "$(@)";
 endif
 
 define rule_cc_o_c
scripts/recordmcount.pl +10 -1
Index: linux-compile.git/scripts/recordmcount.pl
===================================================================
@@ -106,7 +106,8 @@ if ($#ARGV < 6) {
 	exit(1);
 }
 
-my ($arch, $objdump, $objcopy, $cc, $ld, $nm, $rm, $mv, $inputfile) = @ARGV;
+my ($arch, $bits, $objdump, $objcopy, $cc,
+    $ld, $nm, $rm, $mv, $inputfile) = @ARGV;
 
 $objdump = "objdump" if ((length $objdump) == 0);
 $objcopy = "objcopy" if ((length $objcopy) == 0);
@@ -129,6 +130,14 @@ my $function_regex;	# Find the name of a
 			#    (return offset and func name)
 my $mcount_regex;	# Find the call site to mcount (return offset)
 
+if ($arch eq "x86") {
+    if ($bits == 64) {
+	$arch = "x86_64";
+    } else {
+	$arch = "xi386";
+    }
+}
+
 if ($arch eq "x86_64") {
     $section_regex = "Disassembly of section";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
#2 lore raw

Re: [PATCH 01/11] ftrace: handle generic arch calls

scripts/recordmcount.pl my $function_regex; # Find the name of a · line 137

Quoted 5 lines · On Wed, 22 Oct 2008 14:43:14 -0400 Steven Rostedt <[email protected]> wrote:
scripts/recordmcount.pl +5 -0
+if ($arch eq "x86") {
+    if ($bits == 64) {
+	$arch = "x86_64";
+    } else {
+	$arch = "xi386";

?

Quoted 1 line

+ }

(how well was this tested?)

author maintainer
#3 lore raw

Re: [PATCH 01/11] ftrace: handle generic arch calls

Quoted 11 lines · Andrew Morton

scripts/recordmcount.pl my $function_regex; # Find the name of a · line 137

Quoted 5 lines · On Wed, 22 Oct 2008 14:43:14 -0400 Steven Rostedt <[email protected]> wrote:
scripts/recordmcount.pl +5 -0
+if ($arch eq "x86") {
+    if ($bits == 64) {
+	$arch = "x86_64";
+    } else {
+	$arch = "xi386";

?

Quoted 3 lines
Quoted 1 line

+ }

(how well was this tested?)

I've tested this with !CONFIG_FTRACE, CONFIG_FTRACE & !CONFIG_DYNAMIC_FTRACE, and CONFIG_FTRACE & CONFIG_DYNAMIC_FTRACE. I did only test this on x86_64, so that explains why the i386 was broken :-(

I'll boot up my i386 box, and start running the tests there too.

-- Steve

author maintainer
#4 lore raw

Re: [PATCH 01/11] ftrace: handle generic arch calls

In reply to Steven Rostedt

[
Resending patch.

Sam, can you Ack this?

-- Steve
]

From: Steven Rostedt <[email protected]>
Subject: ftrace: handle generic arch calls

The recordmcount script requires that the actual arch is passed in. This works well when ARCH=i386 or ARCH=x86_64 but does not handle the case of ARCH=x86.

This patch adds a parameter to the function to pass in the number of bits of the architecture. So that it can determine if x86 should be run for x86_64 or i386 archs.

---
 scripts/Makefile.build  |   10 ++++++++--
 scripts/recordmcount.pl |   11 ++++++++++-
 2 files changed, 18 insertions(+), 3 deletions(-)
scripts/Makefile.build +8 -2
Index: linux-compile.git/scripts/Makefile.build
===================================================================
@@ -198,10 +198,16 @@ cmd_modversions =							\
 	fi;
 endif
 
+ifdef CONFIG_64BIT
+arch_bits = 64
+else
+arch_bits = 32
+endif
+
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
 cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl \
-	"$(ARCH)" "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" "$(NM)" "$(RM)" \
-	"$(MV)" "$(@)";
+	"$(ARCH)" "$(arch_bits)" "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" \
+	"$(NM)" "$(RM)" "$(MV)" "$(@)";
 endif
 
 define rule_cc_o_c
scripts/recordmcount.pl +10 -1
Index: linux-compile.git/scripts/recordmcount.pl
===================================================================
@@ -106,7 +106,8 @@ if ($#ARGV < 6) {
 	exit(1);
 }
 
-my ($arch, $objdump, $objcopy, $cc, $ld, $nm, $rm, $mv, $inputfile) = @ARGV;
+my ($arch, $bits, $objdump, $objcopy, $cc,
+    $ld, $nm, $rm, $mv, $inputfile) = @ARGV;
 
 $objdump = "objdump" if ((length $objdump) == 0);
 $objcopy = "objcopy" if ((length $objcopy) == 0);
@@ -129,6 +130,14 @@ my $function_regex;	# Find the name of a
 			#    (return offset and func name)
 my $mcount_regex;	# Find the call site to mcount (return offset)
 
+if ($arch eq "x86") {
+    if ($bits == 64) {
+	$arch = "x86_64";
+    } else {
+	$arch = "i386";
+    }
+}
+
 if ($arch eq "x86_64") {
     $section_regex = "Disassembly of section";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
#5 lore raw

Re: [PATCH 01/11] ftrace: handle generic arch calls

Quoted 37 lines · Steven Rostedt

[
Resending patch.

Sam, can you Ack this?

-- Steve
]

From: Steven Rostedt <[email protected]>
Subject: ftrace: handle generic arch calls

The recordmcount script requires that the actual arch is passed in. This works well when ARCH=i386 or ARCH=x86_64 but does not handle the case of ARCH=x86.

This patch adds a parameter to the function to pass in the number of bits of the architecture. So that it can determine if x86 should be run for x86_64 or i386 archs.

---
 scripts/Makefile.build  |   10 ++++++++--
 scripts/recordmcount.pl |   11 ++++++++++-
 2 files changed, 18 insertions(+), 3 deletions(-)

Index: linux-compile.git/scripts/Makefile.build
===================================================================

scripts/Makefile.build +3 -0
@@ -198,10 +198,16 @@ cmd_modversions =							\
 	fi;
 endif
 
+ifdef CONFIG_64BIT
+arch_bits = 64
+else

scripts/Makefile.build cmd_modversions = \ · line 211

Quoted 10 lines
scripts/Makefile.build +5 -2
+arch_bits = 32
+endif
+
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
 cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl \
-	"$(ARCH)" "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" "$(NM)" "$(RM)" \
-	"$(MV)" "$(@)";
+	"$(ARCH)" "$(arch_bits)" "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" \
+	"$(NM)" "$(RM)" "$(MV)" "$(@)";
 endif

A simple $(if $(CONFIG_64BIT),64,32) in the command would be more dense.

Quoted 22 lines
scripts/Makefile.build +0 -0
 define rule_cc_o_c

Index: linux-compile.git/scripts/recordmcount.pl
===================================================================

scripts/recordmcount.pl +3 -1
@@ -106,7 +106,8 @@ if ($#ARGV < 6) {
 	exit(1);
 }
 
-my ($arch, $objdump, $objcopy, $cc, $ld, $nm, $rm, $mv, $inputfile) = @ARGV;
+my ($arch, $bits, $objdump, $objcopy, $cc,
+    $ld, $nm, $rm, $mv, $inputfile) = @ARGV;
 
 $objdump = "objdump" if ((length $objdump) == 0);
 $objcopy = "objcopy" if ((length $objcopy) == 0);
@@ -129,6 +130,14 @@ my $function_regex;	# Find the name of a
 			#    (return offset and func name)
 my $mcount_regex;	# Find the call site to mcount (return offset)
 
+if ($arch eq "x86") {

scripts/recordmcount.pl my $function_regex; # Find the name of a · line 143

Quoted 10 lines
scripts/recordmcount.pl +7 -0
+    if ($bits == 64) {
+	$arch = "x86_64";
+    } else {
+	$arch = "i386";
+    }
+}
+
 if ($arch eq "x86_64") {
     $section_regex = "Disassembly of section";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";

This looks strange to my eyes.
Why not do the more obvious:
if ($arch eq "x86" && $bits == 64) {

The change above is like trying to stick to the old i386/x86_64 notation.

Sam

author maintainer
#6 lore raw

Re: [PATCH 01/11] ftrace: handle generic arch calls

Quoted 35 lines · Sam Ravnborg
Quoted 34 lines · Steven Rostedt

[
Resending patch.

Sam, can you Ack this?

-- Steve
]

From: Steven Rostedt <[email protected]>
Subject: ftrace: handle generic arch calls

The recordmcount script requires that the actual arch is passed in. This works well when ARCH=i386 or ARCH=x86_64 but does not handle the case of ARCH=x86.

This patch adds a parameter to the function to pass in the number of bits of the architecture. So that it can determine if x86 should be run for x86_64 or i386 archs.

---
 scripts/Makefile.build  |   10 ++++++++--
 scripts/recordmcount.pl |   11 ++++++++++-
 2 files changed, 18 insertions(+), 3 deletions(-)

Index: linux-compile.git/scripts/Makefile.build
===================================================================

scripts/Makefile.build +0 -0
@@ -198,10 +198,16 @@ cmd_modversions =							\
 	fi;
 endif
 
Quoted 15 lines

scripts/Makefile.build cmd_modversions = \ · line 211

Quoted 13 lines
scripts/Makefile.build +8 -2
+ifdef CONFIG_64BIT
+arch_bits = 64
+else
+arch_bits = 32
+endif
+
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
 cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl \
-	"$(ARCH)" "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" "$(NM)" "$(RM)" \
-	"$(MV)" "$(@)";
+	"$(ARCH)" "$(arch_bits)" "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" \
+	"$(NM)" "$(RM)" "$(MV)" "$(@)";
 endif

A simple $(if $(CONFIG_64BIT),64,32) in the command would be more dense.

OK, will fix.

Quoted 38 lines
Quoted 23 lines
scripts/Makefile.build +0 -0
 define rule_cc_o_c

Index: linux-compile.git/scripts/recordmcount.pl
===================================================================

scripts/recordmcount.pl +3 -1
@@ -106,7 +106,8 @@ if ($#ARGV < 6) {
 	exit(1);
 }
 
-my ($arch, $objdump, $objcopy, $cc, $ld, $nm, $rm, $mv, $inputfile) = @ARGV;
+my ($arch, $bits, $objdump, $objcopy, $cc,
+    $ld, $nm, $rm, $mv, $inputfile) = @ARGV;
 
 $objdump = "objdump" if ((length $objdump) == 0);
 $objcopy = "objcopy" if ((length $objcopy) == 0);
@@ -129,6 +130,14 @@ my $function_regex;	# Find the name of a
 			#    (return offset and func name)
 my $mcount_regex;	# Find the call site to mcount (return offset)
 
+if ($arch eq "x86") {

scripts/recordmcount.pl my $function_regex; # Find the name of a · line 143

Quoted 10 lines
scripts/recordmcount.pl +7 -0
+    if ($bits == 64) {
+	$arch = "x86_64";
+    } else {
+	$arch = "i386";
+    }
+}
+
 if ($arch eq "x86_64") {
     $section_regex = "Disassembly of section";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";

This looks strange to my eyes.
Why not do the more obvious:
if ($arch eq "x86" && $bits == 64) {

Quoted 2 lines

The change above is like trying to stick to the old i386/x86_64 notation.

Heh, you're right. This is from writing an update without wanting to modify the original code too much. But it makes the end result ugly.

I'll create another patch to go on top of this one. We needed this one in because without it, it broke "make ARCH=x86"

Thanks,

-- Steve

author maintainer
#7 lore raw

Re: [PATCH 01/11] ftrace: handle generic arch calls

In reply to Sam Ravnborg
Quoted 18 lines · Sam Ravnborg

scripts/recordmcount.pl my $function_regex; # Find the name of a · line 143

Quoted 13 lines
scripts/recordmcount.pl +8 -0
+if ($arch eq "x86") {
+    if ($bits == 64) {
+	$arch = "x86_64";
+    } else {
+	$arch = "i386";
+    }
+}
+
 if ($arch eq "x86_64") {
     $section_regex = "Disassembly of section";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";

This looks strange to my eyes.
Why not do the more obvious:
if ($arch eq "x86" && $bits == 64) {

Quoted 2 lines

The change above is like trying to stick to the old i386/x86_64 notation.

Trying to fix it tells me my answer to why I did it his way ;-)

I have queued patches that will support other archs so x86 is not the only arch that can be used here. But x86 is special, it seems to be the only arch (that I know of, correct me if I'm wrong) that can compile with multiple archs defined: make ARCH=x86_64, make ARCH=i386, or make ARCH=x86. All are legit.

Now how do we handle this. I've been fine for all my testing to do just x86_64 and i386 because a normal make of x86 will use automatically set ARCH to i386 or x86_64 depending on the build.

But then Adrian Bunk pointed out that "make ARCH=x86" fails. Now I need to add a case for x86, but still allow for x86_64 or i386 being passed in.

Since x86 is the ambiguous case, I made it the one that would be converted to i386 or x86_64 since those could be passed in directly.

Seems that the best approach is still what is there :-/

-- Steve

#8 lore raw

Re: [PATCH 01/11] ftrace: handle generic arch calls

Quoted 35 lines · Steven Rostedt
Quoted 18 lines · Sam Ravnborg

scripts/recordmcount.pl my $function_regex; # Find the name of a · line 143

Quoted 13 lines
scripts/recordmcount.pl +8 -0
+if ($arch eq "x86") {
+    if ($bits == 64) {
+	$arch = "x86_64";
+    } else {
+	$arch = "i386";
+    }
+}
+
 if ($arch eq "x86_64") {
     $section_regex = "Disassembly of section";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";

This looks strange to my eyes.
Why not do the more obvious:
if ($arch eq "x86" && $bits == 64) {

Quoted 2 lines

The change above is like trying to stick to the old i386/x86_64 notation.

Trying to fix it tells me my answer to why I did it his way ;-)

I have queued patches that will support other archs so x86 is not the only arch that can be used here. But x86 is special, it seems to be the only arch (that I know of, correct me if I'm wrong) that can compile with multiple archs defined: make ARCH=x86_64, make ARCH=i386, or make ARCH=x86. All are legit.

Now how do we handle this. I've been fine for all my testing to do just x86_64 and i386 because a normal make of x86 will use automatically set ARCH to i386 or x86_64 depending on the build.

Quoted 5 lines

But then Adrian Bunk pointed out that "make ARCH=x86" fails. Now I need to add a case for x86, but still allow for x86_64 or i386 being passed in.

Since x86 is the ambiguous case, I made it the one that would be converted to i386 or x86_64 since those could be passed in directly.

The trick is usually to replace use of ARCH with SRCARCH.

Sam

#9 lore raw

Re: [PATCH 01/11] ftrace: handle generic arch calls

Quoted 36 lines · Sam Ravnborg
Quoted 35 lines · Steven Rostedt
Quoted 18 lines · Sam Ravnborg

scripts/recordmcount.pl my $function_regex; # Find the name of a · line 143

Quoted 13 lines
scripts/recordmcount.pl +8 -0
+if ($arch eq "x86") {
+    if ($bits == 64) {
+	$arch = "x86_64";
+    } else {
+	$arch = "i386";
+    }
+}
+
 if ($arch eq "x86_64") {
     $section_regex = "Disassembly of section";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";

This looks strange to my eyes.
Why not do the more obvious:
if ($arch eq "x86" && $bits == 64) {

Quoted 2 lines

The change above is like trying to stick to the old i386/x86_64 notation.

Trying to fix it tells me my answer to why I did it his way ;-)

I have queued patches that will support other archs so x86 is not the only arch that can be used here. But x86 is special, it seems to be the only arch (that I know of, correct me if I'm wrong) that can compile with multiple archs defined: make ARCH=x86_64, make ARCH=i386, or make ARCH=x86. All are legit.

Now how do we handle this. I've been fine for all my testing to do just x86_64 and i386 because a normal make of x86 will use automatically set ARCH to i386 or x86_64 depending on the build.

Quoted 6 lines
Quoted 5 lines

But then Adrian Bunk pointed out that "make ARCH=x86" fails. Now I need to add a case for x86, but still allow for x86_64 or i386 being passed in.

Since x86 is the ambiguous case, I made it the one that would be converted to i386 or x86_64 since those could be passed in directly.

The trick is usually to replace use of ARCH with SRCARCH.

That won't help - what he wants to know is whether he's on 32bit or on 64bit.

Quoted 1 line

Sam

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
author maintainer
#10 lore raw

Re: [PATCH 01/11] ftrace: handle generic arch calls

Quoted 7 lines · Adrian Bunk
Quoted 4 lines
Quoted 3 lines

Since x86 is the ambiguous case, I made it the one that would be converted to i386 or x86_64 since those could be passed in directly.

The trick is usually to replace use of ARCH with SRCARCH.

That won't help - what he wants to know is whether he's on 32bit or on 64bit.

I will still get the "bits" parameter. The issue that we are talking about here is that I have this ugly if statement that deals with the fact that we can get "x86", "x86_64" or "i386" passed in as the ARCH. I'm guessing if we switch ARCH to SRCARCH then we can just test against x86 and bits?

-- Steve

#11 lore raw

Re: [PATCH 01/11] ftrace: handle generic arch calls

Quoted 11 lines · Steven Rostedt
Quoted 7 lines · Adrian Bunk
Quoted 4 lines
Quoted 3 lines

Since x86 is the ambiguous case, I made it the one that would be converted to i386 or x86_64 since those could be passed in directly.

The trick is usually to replace use of ARCH with SRCARCH.

That won't help - what he wants to know is whether he's on 32bit or on 64bit.

I will still get the "bits" parameter.

Ah, I didn't get this bit from your email.

Quoted 4 lines

The issue that we are talking about
here is that I have this ugly if statement that deals with the fact that we can get "x86", "x86_64" or "i386" passed in as the ARCH. I'm guessing if we switch ARCH to SRCARCH then we can just test against x86 and bits?

Yes.

And as a bonus you will automatically get the correct output when you'll add support for UML.

Quoted 1 line

-- Steve

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
#12 lore raw

Re: [PATCH 01/11] ftrace: handle generic arch calls

In reply to Steven Rostedt
Quoted 23 lines · Steven Rostedt
Quoted 18 lines · Sam Ravnborg

scripts/recordmcount.pl my $function_regex; # Find the name of a · line 143

Quoted 13 lines
scripts/recordmcount.pl +8 -0
+if ($arch eq "x86") {
+    if ($bits == 64) {
+	$arch = "x86_64";
+    } else {
+	$arch = "i386";
+    }
+}
+
 if ($arch eq "x86_64") {
     $section_regex = "Disassembly of section";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";

This looks strange to my eyes.
Why not do the more obvious:
if ($arch eq "x86" && $bits == 64) {

Quoted 2 lines

The change above is like trying to stick to the old i386/x86_64 notation.

Quoted 8 lines

Trying to fix it tells me my answer to why I did it his way ;-)

I have queued patches that will support other archs so x86 is not the only arch that can be used here. But x86 is special, it seems to be the only arch (that I know of, correct me if I'm wrong) that can compile with multiple archs defined: make ARCH=x86_64, make ARCH=i386, or make ARCH=x86. All are legit.
...

The multiple ARCH settings with different semantics for them are an x86 curiosity.

But the MIPS, PowerPC and s390 architectures also have unified 32bit/64bit architectures, so whatever you do will most likely also be required on these architectures.

Quoted 1 line

-- Steve

cu
Adrian

Signature · 4 lines
       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed
author maintainer
#13 lore raw

PATCH 1/1 ftrace, kbuild: condense recordmcount.pl parameter code

In reply to Sam Ravnborg

Sam Ravnborg pointed out that I could condense the code for the parameters of recordmcount.pl by using an $(if ...) condition.

---
 scripts/Makefile.build |   12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)
scripts/Makefile.build +3 -9
Index: linux-tip.git/scripts/Makefile.build
===================================================================
@@ -198,16 +198,10 @@ cmd_modversions =							\
 	fi;
 endif
 
-ifdef CONFIG_64BIT
-arch_bits = 64
-else
-arch_bits = 32
-endif
-
 ifdef CONFIG_FTRACE_MCOUNT_RECORD
-cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl \
-	"$(ARCH)" "$(arch_bits)" "$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" \
-	"$(NM)" "$(RM)" "$(MV)" "$(@)";
+cmd_record_mcount = perl $(srctree)/scripts/recordmcount.pl "$(ARCH)" \
+	"$(if $(CONFIG_64BIT),64,32)" \
+	"$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" "$(NM)" "$(RM)" "$(MV)" "$(@)";
 endif
 
 define rule_cc_o_c
#14 lore raw

Re: [PATCH] ftrace, kbuild: condense recordmcount.pl parameter code

Quoted 8 lines · Steven Rostedt

Sam Ravnborg pointed out that I could condense the code for the parameters of recordmcount.pl by using an $(if ...) condition.

---
 scripts/Makefile.build |   12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

Sam, would you like to see this cleanup in v2.6.28, or can it wait until v2.6.29?

Ingo

#15 lore raw

Re: [PATCH] ftrace, kbuild: condense recordmcount.pl parameter code

Quoted 7 lines · Ingo Molnar
Quoted 4 lines · Steven Rostedt

Sam Ravnborg pointed out that I could condense the code for the parameters of recordmcount.pl by using an $(if ...) condition.

Quoted 7 lines
Quoted 4 lines
---
 scripts/Makefile.build |   12 +++---------
 1 file changed, 3 insertions(+), 9 deletions(-)

Sam, would you like to see this cleanup in v2.6.28, or can it wait until v2.6.29?

This is pure cleanup so it can/shall wait.

Sam

author maintainer
#16 lore raw

PATCH 2/11 ftrace: dynamic ftrace process only text section

In reply to Steven Rostedt

The text section stays in memory without ever leaving. With the exception of modules, but modules know how to handle that case. With the dynamic ftrace tracer, we need to make sure that it does not try to modify code that no longer exists. The only safe section is .text.

This patch changes the recordmcount script to only record the mcount calls in the .text sections.

---
 scripts/recordmcount.pl |   17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)
scripts/recordmcount.pl +14 -3
Index: linux-compile.git/scripts/recordmcount.pl
===================================================================
@@ -109,6 +109,11 @@ if ($#ARGV < 6) {
 my ($arch, $bits, $objdump, $objcopy, $cc,
     $ld, $nm, $rm, $mv, $inputfile) = @ARGV;
 
+# Acceptible sections to record.
+my %text_sections = (
+     ".text" => 1,
+);
+
 $objdump = "objdump" if ((length $objdump) == 0);
 $objcopy = "objcopy" if ((length $objcopy) == 0);
 $cc = "gcc" if ((length $cc) == 0);
@@ -139,7 +144,7 @@ if ($arch eq "x86") {
 }
 
 if ($arch eq "x86_64") {
-    $section_regex = "Disassembly of section";
+    $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";
@@ -151,7 +156,7 @@ if ($arch eq "x86_64") {
     $cc .= " -m64";
 
 } elsif ($arch eq "i386") {
-    $section_regex = "Disassembly of section";
+    $section_regex = "Disassembly of section\\s+(\\S+):";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
     $type = ".long";
@@ -298,7 +303,13 @@ my $text;
 while (<IN>) {
     # is it a section?
     if (/$section_regex/) {
-	$read_function = 1;
+
+	# Only record text sections that we know are safe
+	if (defined($text_sections{$1})) {
+	    $read_function = 1;
+	} else {
+	    $read_function = 0;
+	}
 	# print out any recorded offsets
 	update_funcs() if ($text_found);
 
author maintainer
#17 lore raw

PATCH 3/11 ftrace: return error on failed modified text.

In reply to Steven Rostedt

Return -1 on failed modified text.

---
 arch/x86/kernel/ftrace.c |   12 +++++++-----
 kernel/trace/ftrace.c    |   29 ++++++++++-------------------
 2 files changed, 17 insertions(+), 24 deletions(-)
arch/x86/kernel/ftrace.c +7 -5
Index: linux-compile.git/arch/x86/kernel/ftrace.c
===================================================================
@@ -71,14 +71,16 @@ ftrace_modify_code(unsigned long ip, uns
 	 * No real locking needed, this code is run through
 	 * kstop_machine, or before SMP starts.
 	 */
-	if (__copy_from_user_inatomic(replaced, (char __user *)ip, MCOUNT_INSN_SIZE))
-		return 1;
+	if (__copy_from_user_inatomic(replaced, (char __user *)ip,
+				      MCOUNT_INSN_SIZE))
+		return -1;
 
 	if (memcmp(replaced, old_code, MCOUNT_INSN_SIZE) != 0)
-		return 2;
+		return -1;
 
-	WARN_ON_ONCE(__copy_to_user_inatomic((char __user *)ip, new_code,
-				    MCOUNT_INSN_SIZE));
+	if (__copy_to_user_inatomic((char __user *)ip, new_code,
+				    MCOUNT_INSN_SIZE))
+		return -1;
 
 	sync_core();
 
kernel/trace/ftrace.c +10 -19
Index: linux-compile.git/kernel/trace/ftrace.c
===================================================================
@@ -591,31 +591,22 @@ ftrace_code_disable(struct dyn_ftrace *r
 {
 	unsigned long ip;
 	unsigned char *nop, *call;
-	int failed;
+	int ret;
 
 	ip = rec->ip;
 
 	nop = ftrace_nop_replace();
 	call = ftrace_call_replace(ip, mcount_addr);
 
-	failed = ftrace_modify_code(ip, call, nop);
-	if (failed) {
-		switch (failed) {
-		case 1:
-			WARN_ON_ONCE(1);
-			pr_info("ftrace faulted on modifying ");
-			print_ip_sym(ip);
-			break;
-		case 2:
-			WARN_ON_ONCE(1);
-			pr_info("ftrace failed to modify ");
-			print_ip_sym(ip);
-			print_ip_ins(" expected: ", call);
-			print_ip_ins(" actual: ", (unsigned char *)ip);
-			print_ip_ins(" replace: ", nop);
-			printk(KERN_CONT "\n");
-			break;
-		}
+	ret = ftrace_modify_code(ip, call, nop);
+	if (ret) {
+		WARN_ON_ONCE(1);
+		pr_info("ftrace failed to modify ");
+		print_ip_sym(ip);
+		print_ip_ins(" expected: ", call);
+		print_ip_ins(" replace: ", nop);
+		printk(KERN_CONT "\n");
+		break;
 
 		rec->flags |= FTRACE_FL_FAILED;
 		return 0;
author maintainer
#18 lore raw

Re: [PATCH 03/11] ftrace: return error on failed modified text.

kernel/trace/ftrace.c ftrace_code_disable(struct dyn_ftrace *r · line 609

Quoted 11 lines · Steven Rostedt
kernel/trace/ftrace.c +9 -2
-			break;
-		}
+	ret = ftrace_modify_code(ip, call, nop);
+	if (ret) {
+		WARN_ON_ONCE(1);
+		pr_info("ftrace failed to modify ");
+		print_ip_sym(ip);
+		print_ip_ins(" expected: ", call);
+		print_ip_ins(" replace: ", nop);
+		printk(KERN_CONT "\n");
+		break;

This break is an error. I refreshed it in patch 5/11 by mistake.

-- Steve

Quoted 7 lines
kernel/trace/ftrace.c +0 -0
 		rec->flags |= FTRACE_FL_FAILED;
 		return 0;
Signature · 1 line
-- 
#19 lore raw

Re: [PATCH 03/11] ftrace: return error on failed modified text.

In reply to Steven Rostedt
Quoted 2 lines · On Wed, 22 Oct 2008 14:43:16 -0400 Steven Rostedt <[email protected]> wrote:

Return -1 on failed modified text.

changelog fails to explain the reason for the change.

arch/x86/kernel/ftrace.c ftrace_modify_code(unsigned long ip, uns · line 83

Quoted 3 lines
arch/x86/kernel/ftrace.c +3 -0
+	if (__copy_to_user_inatomic((char __user *)ip, new_code,
+				    MCOUNT_INSN_SIZE))
+		return -1;

why not -EFAULT?

author maintainer
#20 lore raw

Re: [PATCH 03/11] ftrace: return error on failed modified text.

Quoted 6 lines · Andrew Morton
Quoted 2 lines · On Wed, 22 Oct 2008 14:43:16 -0400 Steven Rostedt <[email protected]> wrote:

Return -1 on failed modified text.

Quoted 7 lines

changelog fails to explain the reason for the change.

arch/x86/kernel/ftrace.c ftrace_modify_code(unsigned long ip, uns · line 83

Quoted 3 lines
arch/x86/kernel/ftrace.c +3 -0
+	if (__copy_to_user_inatomic((char __user *)ip, new_code,
+				    MCOUNT_INSN_SIZE))
+		return -1;

why not -EFAULT?

Indeed, why not. I'll update it in v2.

-- Steve

author maintainer
#21 lore raw

PATCH 4/11 ftrace: comment arch ftrace code

In reply to Steven Rostedt

Add comments to explain what is happening in the x86 arch ftrace code.

---
 arch/x86/kernel/ftrace.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
arch/x86/kernel/ftrace.c +6 -1
Index: linux-compile.git/arch/x86/kernel/ftrace.c
===================================================================
@@ -66,18 +66,23 @@ ftrace_modify_code(unsigned long ip, uns
 	/*
 	 * Note: Due to modules and __init, code can
 	 *  disappear and change, we need to protect against faulting
-	 *  as well as code changing.
+	 *  as well as code changing. We do this by using the
+	 *  __copy_*_user functions.
 	 *
 	 * No real locking needed, this code is run through
 	 * kstop_machine, or before SMP starts.
 	 */
+
+	/* read the text we want to modify */
 	if (__copy_from_user_inatomic(replaced, (char __user *)ip,
 				      MCOUNT_INSN_SIZE))
 		return -1;
 
+	/* Make sure it is what we expect it to be */
 	if (memcmp(replaced, old_code, MCOUNT_INSN_SIZE) != 0)
 		return -1;
 
+	/* replace the text with the new text */
 	if (__copy_to_user_inatomic((char __user *)ip, new_code,
 				    MCOUNT_INSN_SIZE))
 		return -1;
#22 lore raw

Re: [PATCH 04/11] ftrace: comment arch ftrace code

Quoted 27 lines · On Wed, 22 Oct 2008 14:43:17 -0400 Steven Rostedt <[email protected]> wrote:

Add comments to explain what is happening in the x86 arch ftrace code.

---
 arch/x86/kernel/ftrace.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Index: linux-compile.git/arch/x86/kernel/ftrace.c
===================================================================

arch/x86/kernel/ftrace.c +4 -1
@@ -66,18 +66,23 @@ ftrace_modify_code(unsigned long ip, uns
 	/*
 	 * Note: Due to modules and __init, code can
 	 *  disappear and change, we need to protect against faulting
-	 *  as well as code changing.
+	 *  as well as code changing. We do this by using the
+	 *  __copy_*_user functions.
 	 *
 	 * No real locking needed, this code is run through
 	 * kstop_machine, or before SMP starts.
 	 */
+
+	/* read the text we want to modify */
 	if (__copy_from_user_inatomic(replaced, (char __user *)ip,
 				      MCOUNT_INSN_SIZE))

arch/x86/kernel/ftrace.c ftrace_modify_code(unsigned long ip, uns · line 88

Quoted 10 lines
arch/x86/kernel/ftrace.c +2 -0
 		return -1;
 
+	/* Make sure it is what we expect it to be */
 	if (memcmp(replaced, old_code, MCOUNT_INSN_SIZE) != 0)
 		return -1;
 
+	/* replace the text with the new text */
 	if (__copy_to_user_inatomic((char __user *)ip, new_code,
 				    MCOUNT_INSN_SIZE))
 		return -1;

I dunno.

__copy_to_user_inatomic() is for "copying memory from userspace while in an atomic context".

But what you're doing here is "modifying some kernel text which might generate a fault". It seems somewhat interface-abusive to use a userspace access function for that just because it happens right now to do the right thing.

I'd suggest that for clarity and for future-safety, you create some new interface function which does that thing. Right now it can be a simple wrapper around __copy_from_user_inatomic().

<looks>

oh, someone added one - probe_kernel_write(). Why not use that?

<wonders why he doesn't know what's going on any more>

Also, I hope that the above code is called from within a pagefault_disable()d region? Or are relying upon some magical side-effect of something which happens to do the same thing as pagefault_disable()? IOW: by what means does the above code ensure that do_page_fault() will see in_atomic()==true?

author maintainer
#23 lore raw

Re: [PATCH 04/11] ftrace: comment arch ftrace code

Quoted 11 lines · Andrew Morton

I dunno.

__copy_to_user_inatomic() is for "copying memory from userspace while in an atomic context".

But what you're doing here is "modifying some kernel text which might generate a fault". It seems somewhat interface-abusive to use a userspace access function for that just because it happens right now to do the right thing.

Quoted 7 lines

I'd suggest that for clarity and for future-safety, you create some new interface function which does that thing. Right now it can be a simple wrapper around __copy_from_user_inatomic().

<looks>

oh, someone added one - probe_kernel_write(). Why not use that?

I didn't know about that code. That is what I want.

-- Steve

Quoted 9 lines

<wonders why he doesn't know what's going on any more>

Also, I hope that the above code is called from within a pagefault_disable()d region? Or are relying upon some magical side-effect of something which happens to do the same thing as pagefault_disable()? IOW: by what means does the above code ensure that do_page_fault() will see in_atomic()==true?

This code is called from kstop_machine, or simply has interrupts disabled.

-- Steve

#24 lore raw

Re: [PATCH 04/11] ftrace: comment arch ftrace code

Quoted 7 lines · On Wed, 22 Oct 2008 15:16:33 -0400 (EDT) Steven Rostedt <[email protected]>…
Quoted 5 lines

Also, I hope that the above code is called from within a pagefault_disable()d region? Or are relying upon some magical side-effect of something which happens to do the same thing as pagefault_disable()? IOW: by what means does the above code ensure that do_page_fault() will see in_atomic()==true?

This code is called from kstop_machine, or simply has interrupts disabled.

in_atomic() doesn't test irqs_disabled()!

Still, probe_kernel_write() correctly handles the
secret-argument-passing to do_page_fault().

author maintainer
#25 lore raw

PATCH 5/11 ftrace: only have ftrace_kill atomic

In reply to Steven Rostedt

Only have a way to completely disable ftrace in atomic sections.

---
 include/linux/ftrace.h |    3 +--
 kernel/trace/ftrace.c  |   43 ++-----------------------------------------
 kernel/trace/trace.c   |    2 +-
 3 files changed, 4 insertions(+), 44 deletions(-)
include/linux/ftrace.h +1 -2
Index: linux-compile.git/include/linux/ftrace.h
===================================================================
@@ -40,7 +40,7 @@ extern void ftrace_stub(unsigned long a0
 # define register_ftrace_function(ops) do { } while (0)
 # define unregister_ftrace_function(ops) do { } while (0)
 # define clear_ftrace_function(ops) do { } while (0)
-static inline void ftrace_kill_atomic(void) { }
+static inline void ftrace_kill(void) { }
 #endif /* CONFIG_FTRACE */
 
 #ifdef CONFIG_DYNAMIC_FTRACE
@@ -97,7 +97,6 @@ static inline void ftrace_release(void *
 
 /* totally disable ftrace - can not re-enable after this */
 void ftrace_kill(void);
-void ftrace_kill_atomic(void);
 
 static inline void tracer_disable(void)
 {
kernel/trace/ftrace.c +2 -41

75 lines not on this page · raw message

kernel/trace/trace.c +1 -1
Index: linux-compile.git/kernel/trace/trace.c
===================================================================
@@ -3097,7 +3097,7 @@ void ftrace_dump(void)
 	dump_ran = 1;
 
 	/* No turning back! */
-	ftrace_kill_atomic();
+	ftrace_kill();
 
 	for_each_tracing_cpu(cpu) {
 		atomic_inc(&global_trace.data[cpu]->disabled);
#26 lore raw

Re: [PATCH 05/11] ftrace: only have ftrace_kill atomic

Quoted 2 lines · On Wed, 22 Oct 2008 14:43:18 -0400 Steven Rostedt <[email protected]> wrote:

Only have a way to completely disable ftrace in atomic sections.

I have NFI what that means.

More importantly there is no explanation for *why* this change is being made.

author maintainer
#27 lore raw

Re: [PATCH 05/11] ftrace: only have ftrace_kill atomic

Quoted 10 lines · Andrew Morton
Quoted 2 lines · On Wed, 22 Oct 2008 14:43:18 -0400 Steven Rostedt <[email protected]> wrote:

Only have a way to completely disable ftrace in atomic sections.

I have NFI what that means.

More importantly there is no explanation for *why* this change is being made.

Sorry, I'll explain it better in the next release. This was just something that Ingo wanted cleaned up. He prefers the name "ftrace_turn_off". I really don't care what the name is, but the "atomic" part was something that had to go.

What's your preference fo a name of a function that should completely disable ftrace on anomaly detection? Unfortunately, ftrace_stop, ftrace_disable, and ftrace_shutdown are already in use.

-- Steve

#28 lore raw

Re: [PATCH 05/11] ftrace: only have ftrace_kill atomic

Quoted 3 lines · On Wed, 22 Oct 2008 15:18:39 -0400 (EDT) Steven Rostedt <[email protected]>…

What's your preference fo a name of a function that should completely disable ftrace on anomaly detection? Unfortunately, ftrace_stop, ftrace_disable, and ftrace_shutdown are already in use.

ftrace_kill()? ftrace_akpm()?

author maintainer
#29 lore raw

PATCH 6/11 ftrace: add ftrace warn on to disable ftrace

In reply to Steven Rostedt

Add ftrace warn on to disable ftrace as well as report a warning.

---
 kernel/trace/ftrace.c |   29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)
kernel/trace/ftrace.c +22 -7
Index: linux-compile.git/kernel/trace/ftrace.c
===================================================================
@@ -32,6 +32,24 @@
 
 #include "trace.h"
 
+#define FTRACE_WARN_ON(cond)			\
+	do {					\
+		if (unlikely(cond)) {		\
+			WARN_ON(1);		\
+			ftrace_kill();		\
+		}				\
+	} while (0)
+
+#define FTRACE_WARN_ON_ONCE(cond)		\
+	do {					\
+		static int once;		\
+		if (unlikely(cond) && !once) {	\
+			once++;			\
+			WARN_ON(1);		\
+			ftrace_kill();		\
+		}				\
+	} while (0)
+
 /* ftrace_enabled is a method to turn ftrace on or off */
 int ftrace_enabled __read_mostly;
 static int last_ftrace_enabled;
@@ -358,10 +376,8 @@ static struct dyn_ftrace *ftrace_alloc_d
 		rec = ftrace_free_records;
 
 		if (unlikely(!(rec->flags & FTRACE_FL_FREE))) {
-			WARN_ON_ONCE(1);
+			FTRACE_WARN_ON_ONCE(1);
 			ftrace_free_records = NULL;
-			ftrace_disabled = 1;
-			ftrace_enabled = 0;
 			return NULL;
 		}
 
@@ -410,7 +426,7 @@ ftrace_record_ip(unsigned long ip)
 
 	key = hash_long(ip, FTRACE_HASHBITS);
 
-	WARN_ON_ONCE(key >= FTRACE_HASHSIZE);
+	FTRACE_WARN_ON_ONCE(key >= FTRACE_HASHSIZE);
 
 	if (ftrace_ip_in_hash(ip, key))
 		goto out;
@@ -600,7 +616,7 @@ ftrace_code_disable(struct dyn_ftrace *r
 
 	ret = ftrace_modify_code(ip, call, nop);
 	if (ret) {
-		WARN_ON_ONCE(1);
+		FTRACE_WARN_ON_ONCE(1);
 		pr_info("ftrace failed to modify ");
 		print_ip_sym(ip);
 		print_ip_ins(" expected: ", call);
@@ -1660,8 +1676,7 @@ static int ftraced(void *ignore)
 					ftrace_update_cnt != 1 ? "s" : "",
 					ftrace_update_tot_cnt,
 					usecs, usecs != 1 ? "s" : "");
-				ftrace_disabled = 1;
-				WARN_ON_ONCE(1);
+				FTRACE_WARN_ON_ONCE(1);
 			}
 		}
 		mutex_unlock(&ftraced_lock);
#30 lore raw

Re: [PATCH 06/11] ftrace: add ftrace warn on to disable ftrace

Quoted 12 lines · On Wed, 22 Oct 2008 14:43:19 -0400 Steven Rostedt <[email protected]> wrote:

Add ftrace warn on to disable ftrace as well as report a warning.

---
 kernel/trace/ftrace.c |   29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

Index: linux-compile.git/kernel/trace/ftrace.c
===================================================================

kernel/trace/ftrace.c line 41

Quoted 10 lines
kernel/trace/ftrace.c +7 -0
@@ -32,6 +32,24 @@
 
 #include "trace.h"
 
+#define FTRACE_WARN_ON(cond)			\
+	do {					\
+		if (unlikely(cond)) {		\
+			WARN_ON(1);		\
+			ftrace_kill();		\
+		}				\
+	} while (0)

This could be coded as

	if (WARN_ON(cond))
		ftrace_kill();

kernel/trace/ftrace.c line 51

Quoted 9 lines
kernel/trace/ftrace.c +9 -0
+#define FTRACE_WARN_ON_ONCE(cond)		\
+	do {					\
+		static int once;		\
+		if (unlikely(cond) && !once) {	\
+			once++;			\
+			WARN_ON(1);		\
+			ftrace_kill();		\
+		}				\
+	} while (0)
	if (WARN_ON_ONCE(cond))
		ftrace_kill();
author maintainer
#31 lore raw

Re: [PATCH 06/11] ftrace: add ftrace warn on to disable ftrace

Quoted 18 lines · Andrew Morton
Quoted 5 lines

Index: linux-compile.git/kernel/trace/ftrace.c
===================================================================

kernel/trace/ftrace.c line 41

Quoted 10 lines
kernel/trace/ftrace.c +7 -0
@@ -32,6 +32,24 @@
 
 #include "trace.h"
 
+#define FTRACE_WARN_ON(cond)			\
+	do {					\
+		if (unlikely(cond)) {		\
+			WARN_ON(1);		\
+			ftrace_kill();		\
+		}				\
+	} while (0)

This could be coded as

Quoted 2 lines
	if (WARN_ON(cond))
		ftrace_kill();

Ah, I didn't realize that WARN_ON returned the value of the condition. Will update in v2.

-- Steve

Quoted 18 lines

kernel/trace/ftrace.c line 51

Quoted 10 lines
kernel/trace/ftrace.c +9 -0
+#define FTRACE_WARN_ON_ONCE(cond)		\
+	do {					\
+		static int once;		\
+		if (unlikely(cond) && !once) {	\
+			once++;			\
+			WARN_ON(1);		\
+			ftrace_kill();		\
+		}				\
+	} while (0)
	if (WARN_ON_ONCE(cond))
		ftrace_kill();
author maintainer
#32 lore raw

PATCH 7/11 ftrace: do not trace init sections

In reply to Steven Rostedt

The recordmcount script is now robust enough not to process any sections but the .text section. But the gcc compiler still adds a call to mcount.

Note: The function mcount looks like:

ENTRY(mcount)
ret
END(mcount)

Which means the overhead is just a return.

This patch adds notrace to the init sections to not even bother calling mcount (which simply returns).

---
 include/linux/init.h |   10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)
include/linux/init.h +5 -5
Index: linux-compile.git/include/linux/init.h
===================================================================
@@ -75,15 +75,15 @@
 
 
 #ifdef MODULE
-#define __exitused
+#define __exitused notrace
 #else
-#define __exitused  __used
+#define __exitused  __used notrace
 #endif
 
 #define __exit          __section(.exit.text) __exitused __cold
 
 /* Used for HOTPLUG */
-#define __devinit        __section(.devinit.text) __cold
+#define __devinit        __section(.devinit.text) __cold notrace
 #define __devinitdata    __section(.devinit.data)
 #define __devinitconst   __section(.devinit.rodata)
 #define __devexit        __section(.devexit.text) __exitused __cold
@@ -91,7 +91,7 @@
 #define __devexitconst   __section(.devexit.rodata)
 
 /* Used for HOTPLUG_CPU */
-#define __cpuinit        __section(.cpuinit.text) __cold
+#define __cpuinit        __section(.cpuinit.text) __cold notrace
 #define __cpuinitdata    __section(.cpuinit.data)
 #define __cpuinitconst   __section(.cpuinit.rodata)
 #define __cpuexit        __section(.cpuexit.text) __exitused __cold
@@ -99,7 +99,7 @@
 #define __cpuexitconst   __section(.cpuexit.rodata)
 
 /* Used for MEMORY_HOTPLUG */
-#define __meminit        __section(.meminit.text) __cold
+#define __meminit        __section(.meminit.text) __cold notrace
 #define __meminitdata    __section(.meminit.data)
 #define __meminitconst   __section(.meminit.rodata)
 #define __memexit        __section(.memexit.text) __exitused __cold
author maintainer
#33 lore raw

PATCH 8/11 ftrace: disable dynamic ftrace for all archs that use daemon

In reply to Steven Rostedt

The ftrace daemon is complex and can cause nasty races if something goes wrong. Since it affects all of the kernel, this patch disables dynamic ftrace from any arch that depends on the daemon. Until the archs are ported over to the new MCOUNT_RECORD method, I am disabling dynamic ftrace from them.

Note: I am leaving in the arch/<arch>/kernel/ftrace.c code alone since that can be used when the arch is ported to MCOUNT_RECORD. To port the arch to MCOUNT_RECORD, the scripts/recordmcount.pl needs to be updated. I will make that easier to do for 2.6.29. For 28, we will keep the archs disabled.

---
 arch/arm/Kconfig     |    1 -
 arch/powerpc/Kconfig |    1 -
 arch/sparc64/Kconfig |    1 -
 3 files changed, 3 deletions(-)
arch/arm/Kconfig +0 -1
Index: linux-compile.git/arch/arm/Kconfig
===================================================================
@@ -17,7 +17,6 @@ config ARM
 	select HAVE_KPROBES if (!XIP_KERNEL)
 	select HAVE_KRETPROBES if (HAVE_KPROBES)
 	select HAVE_FTRACE if (!XIP_KERNEL)
-	select HAVE_DYNAMIC_FTRACE if (HAVE_FTRACE)
 	select HAVE_GENERIC_DMA_COHERENT
 	help
 	  The ARM series is a line of low-power-consumption RISC chip designs
arch/powerpc/Kconfig +0 -1
Index: linux-compile.git/arch/powerpc/Kconfig
===================================================================
@@ -111,7 +111,6 @@ config ARCH_NO_VIRT_TO_BUS
 config PPC
 	bool
 	default y
-	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select HAVE_IDE
arch/sparc64/Kconfig +0 -1
Index: linux-compile.git/arch/sparc64/Kconfig
===================================================================
@@ -11,7 +11,6 @@ config SPARC
 config SPARC64
 	bool
 	default y
-	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE
 	select HAVE_IDE
 	select HAVE_LMB
author maintainer
#34 lore raw

PATCH 9/11 ftrace: remove daemon

In reply to Steven Rostedt

The ftrace daemon is complex and error prone. This patch strips it out of the code.

---
 kernel/trace/ftrace.c         |  280 ++++--------------------------------------
 kernel/trace/trace_selftest.c |   14 --
 2 files changed, 28 insertions(+), 266 deletions(-)
kernel/trace/ftrace.c +28 -252
Index: linux-compile.git/kernel/trace/ftrace.c
===================================================================
@@ -171,21 +171,8 @@ static int __unregister_ftrace_function(
 }
 
 #ifdef CONFIG_DYNAMIC_FTRACE
-
 #ifndef CONFIG_FTRACE_MCOUNT_RECORD
-/*
- * The hash lock is only needed when the recording of the mcount
- * callers are dynamic. That is, by the caller themselves and
- * not recorded via the compilation.
- */
-static DEFINE_SPINLOCK(ftrace_hash_lock);
-#define ftrace_hash_lock(flags)	  spin_lock_irqsave(&ftrace_hash_lock, flags)
-#define ftrace_hash_unlock(flags) \
-			spin_unlock_irqrestore(&ftrace_hash_lock, flags)
-#else
-/* This is protected via the ftrace_lock with MCOUNT_RECORD. */
-#define ftrace_hash_lock(flags)   do { (void)(flags); } while (0)
-#define ftrace_hash_unlock(flags) do { } while(0)
+# error Dynamic ftrace depends on MCOUNT_RECORD
 #endif
 
 /*
@@ -196,8 +183,6 @@ static DEFINE_SPINLOCK(ftrace_hash_lock)
  */
 static unsigned long mcount_addr = MCOUNT_ADDR;
 
-static struct task_struct *ftraced_task;
-
 enum {
 	FTRACE_ENABLE_CALLS		= (1 << 0),
 	FTRACE_DISABLE_CALLS		= (1 << 1),
@@ -214,7 +199,6 @@ static struct hlist_head ftrace_hash[FTR
 
 static DEFINE_PER_CPU(int, ftrace_shutdown_disable_cpu);
 
-static DEFINE_MUTEX(ftraced_lock);
 static DEFINE_MUTEX(ftrace_regex_lock);
 
 struct ftrace_page {
@@ -232,10 +216,6 @@ struct ftrace_page {
 static struct ftrace_page	*ftrace_pages_start;
 static struct ftrace_page	*ftrace_pages;
 
-static int ftraced_trigger;
-static int ftraced_suspend;
-static int ftraced_stop;
-
 static int ftrace_record_suspend;
 
 static struct dyn_ftrace *ftrace_free_records;
@@ -399,7 +379,6 @@ static void
 ftrace_record_ip(unsigned long ip)
 {
 	struct dyn_ftrace *node;
-	unsigned long flags;
 	unsigned long key;
 	int resched;
 	int cpu;
@@ -431,24 +410,18 @@ ftrace_record_ip(unsigned long ip)
 	if (ftrace_ip_in_hash(ip, key))
 		goto out;
 
-	ftrace_hash_lock(flags);
-
 	/* This ip may have hit the hash before the lock */
 	if (ftrace_ip_in_hash(ip, key))
-		goto out_unlock;
+		goto out;
 
 	node = ftrace_alloc_dyn_node(ip);
 	if (!node)
-		goto out_unlock;
+		goto out;
 
 	node->ip = ip;
 
 	ftrace_add_hash(node, key);
 
-	ftraced_trigger = 1;
-
- out_unlock:
-	ftrace_hash_unlock(flags);
  out:
 	per_cpu(ftrace_shutdown_disable_cpu, cpu)--;
 
@@ -629,7 +602,7 @@ ftrace_code_disable(struct dyn_ftrace *r
 	return 1;
 }
 
-static int __ftrace_update_code(void *ignore);
+static int ftrace_update_code(void *ignore);
 
 static int __ftrace_modify_code(void *data)
 {
@@ -641,7 +614,7 @@ static int __ftrace_modify_code(void *da
 		 * Update any recorded ips now that we have the
 		 * machine stopped
 		 */
-		__ftrace_update_code(NULL);
+		ftrace_update_code(NULL);
 		ftrace_replace_code(1);
 		tracing_on = 1;
 	} else if (*command & FTRACE_DISABLE_CALLS) {
@@ -668,26 +641,9 @@ static void ftrace_run_update_code(int c
 	stop_machine(__ftrace_modify_code, &command, NULL);
 }
 
-void ftrace_disable_daemon(void)
-{
-	/* Stop the daemon from calling kstop_machine */
-	mutex_lock(&ftraced_lock);
-	ftraced_stop = 1;
-	mutex_unlock(&ftraced_lock);
-
-	ftrace_force_update();
-}
-
-void ftrace_enable_daemon(void)
-{
-	mutex_lock(&ftraced_lock);
-	ftraced_stop = 0;
-	mutex_unlock(&ftraced_lock);
-
-	ftrace_force_update();
-}
-
 static ftrace_func_t saved_ftrace_func;
+static int ftrace_start;
+static DEFINE_MUTEX(ftrace_start_lock);
 
 static void ftrace_startup(void)
 {
@@ -696,9 +652,9 @@ static void ftrace_startup(void)
 	if (unlikely(ftrace_disabled))
 		return;
 
-	mutex_lock(&ftraced_lock);
-	ftraced_suspend++;
-	if (ftraced_suspend == 1)
+	mutex_lock(&ftrace_start_lock);
+	ftrace_start++;
+	if (ftrace_start == 1)
 		command |= FTRACE_ENABLE_CALLS;
 
 	if (saved_ftrace_func != ftrace_trace_function) {
@@ -711,7 +667,7 @@ static void ftrace_startup(void)
 
 	ftrace_run_update_code(command);
  out:
-	mutex_unlock(&ftraced_lock);
+	mutex_unlock(&ftrace_start_lock);
 }
 
 static void ftrace_shutdown(void)
@@ -721,9 +677,9 @@ static void ftrace_shutdown(void)
 	if (unlikely(ftrace_disabled))
 		return;
 
-	mutex_lock(&ftraced_lock);
-	ftraced_suspend--;
-	if (!ftraced_suspend)
+	mutex_lock(&ftrace_start_lock);
+	ftrace_start--;
+	if (!ftrace_start)
 		command |= FTRACE_DISABLE_CALLS;
 
 	if (saved_ftrace_func != ftrace_trace_function) {
@@ -736,7 +692,7 @@ static void ftrace_shutdown(void)
 
 	ftrace_run_update_code(command);
  out:
-	mutex_unlock(&ftraced_lock);
+	mutex_unlock(&ftrace_start_lock);
 }
 
 static void ftrace_startup_sysctl(void)
@@ -746,15 +702,15 @@ static void ftrace_startup_sysctl(void)
 	if (unlikely(ftrace_disabled))
 		return;
 
-	mutex_lock(&ftraced_lock);
+	mutex_lock(&ftrace_start_lock);
 	/* Force update next time */
 	saved_ftrace_func = NULL;
-	/* ftraced_suspend is true if we want ftrace running */
-	if (ftraced_suspend)
+	/* ftrace_start is true if we want ftrace running */
+	if (ftrace_start)
 		command |= FTRACE_ENABLE_CALLS;
 
 	ftrace_run_update_code(command);
-	mutex_unlock(&ftraced_lock);
+	mutex_unlock(&ftrace_start_lock);
 }
 
 static void ftrace_shutdown_sysctl(void)
@@ -764,20 +720,20 @@ static void ftrace_shutdown_sysctl(void)
 	if (unlikely(ftrace_disabled))
 		return;
 
-	mutex_lock(&ftraced_lock);
-	/* ftraced_suspend is true if ftrace is running */
-	if (ftraced_suspend)
+	mutex_lock(&ftrace_start_lock);
+	/* ftrace_start is true if ftrace is running */
+	if (ftrace_start)
 		command |= FTRACE_DISABLE_CALLS;
 
 	ftrace_run_update_code(command);
-	mutex_unlock(&ftraced_lock);
+	mutex_unlock(&ftrace_start_lock);
 }
 
 static cycle_t		ftrace_update_time;
 static unsigned long	ftrace_update_cnt;
 unsigned long		ftrace_update_tot_cnt;
 
-static int __ftrace_update_code(void *ignore)
+static int ftrace_update_code(void *ignore)
 {
 	int i, save_ftrace_enabled;
 	cycle_t start, stop;
@@ -851,7 +807,6 @@ static int __ftrace_update_code(void *ig
 	stop = ftrace_now(raw_smp_processor_id());
 	ftrace_update_time = stop - start;
 	ftrace_update_tot_cnt += ftrace_update_cnt;
-	ftraced_trigger = 0;
 
 	ftrace_enabled = save_ftrace_enabled;
 	ftrace_record_suspend--;
@@ -859,17 +814,6 @@ static int __ftrace_update_code(void *ig
 	return 0;
 }
 
-static int ftrace_update_code(void)
-{
-	if (unlikely(ftrace_disabled) ||
-	    !ftrace_enabled || !ftraced_trigger)
-		return 0;
-
-	stop_machine(__ftrace_update_code, NULL, NULL);
-
-	return 1;
-}
-
 static int __init ftrace_dyn_table_alloc(unsigned long num_to_init)
 {
 	struct ftrace_page *pg;
@@ -1407,10 +1351,10 @@ ftrace_regex_release(struct inode *inode
 	}
 
 	mutex_lock(&ftrace_sysctl_lock);
-	mutex_lock(&ftraced_lock);
-	if (iter->filtered && ftraced_suspend && ftrace_enabled)
+	mutex_lock(&ftrace_start_lock);
+	if (iter->filtered && ftrace_start && ftrace_enabled)
 		ftrace_run_update_code(FTRACE_ENABLE_CALLS);
-	mutex_unlock(&ftraced_lock);
+	mutex_unlock(&ftrace_start_lock);
 	mutex_unlock(&ftrace_sysctl_lock);
 
 	kfree(iter);
@@ -1430,55 +1374,6 @@ ftrace_notrace_release(struct inode *ino
 	return ftrace_regex_release(inode, file, 0);
 }
 
-static ssize_t
-ftraced_read(struct file *filp, char __user *ubuf,
-		     size_t cnt, loff_t *ppos)
-{
-	/* don't worry about races */
-	char *buf = ftraced_stop ? "disabled\n" : "enabled\n";
-	int r = strlen(buf);
-
-	return simple_read_from_buffer(ubuf, cnt, ppos, buf, r);
-}
-
-static ssize_t
-ftraced_write(struct file *filp, const char __user *ubuf,
-		      size_t cnt, loff_t *ppos)
-{
-	char buf[64];
-	long val;
-	int ret;
-
-	if (cnt >= sizeof(buf))
-		return -EINVAL;
-
-	if (copy_from_user(&buf, ubuf, cnt))
-		return -EFAULT;
-
-	if (strncmp(buf, "enable", 6) == 0)
-		val = 1;
-	else if (strncmp(buf, "disable", 7) == 0)
-		val = 0;
-	else {
-		buf[cnt] = 0;
-
-		ret = strict_strtoul(buf, 10, &val);
-		if (ret < 0)
-			return ret;
-
-		val = !!val;
-	}
-
-	if (val)
-		ftrace_enable_daemon();
-	else
-		ftrace_disable_daemon();
-
-	filp->f_pos += cnt;
-
-	return cnt;
-}
-
 static struct file_operations ftrace_avail_fops = {
 	.open = ftrace_avail_open,
 	.read = seq_read,
@@ -1509,38 +1404,6 @@ static struct file_operations ftrace_not
 	.release = ftrace_notrace_release,
 };
 
-static struct file_operations ftraced_fops = {
-	.open = tracing_open_generic,
-	.read = ftraced_read,
-	.write = ftraced_write,
-};
-
-/**
- * ftrace_force_update - force an update to all recording ftrace functions
- */
-int ftrace_force_update(void)
-{
-	int ret = 0;
-
-	if (unlikely(ftrace_disabled))
-		return -ENODEV;
-
-	mutex_lock(&ftrace_sysctl_lock);
-	mutex_lock(&ftraced_lock);
-
-	/*
-	 * If ftraced_trigger is not set, then there is nothing
-	 * to update.
-	 */
-	if (ftraced_trigger && !ftrace_update_code())
-		ret = -EBUSY;
-
-	mutex_unlock(&ftraced_lock);
-	mutex_unlock(&ftrace_sysctl_lock);
-
-	return ret;
-}
-
 static __init int ftrace_init_debugfs(void)
 {
 	struct dentry *d_tracer;
@@ -1571,17 +1434,11 @@ static __init int ftrace_init_debugfs(vo
 		pr_warning("Could not create debugfs "
 			   "'set_ftrace_notrace' entry\n");
 
-	entry = debugfs_create_file("ftraced_enabled", 0644, d_tracer,
-				    NULL, &ftraced_fops);
-	if (!entry)
-		pr_warning("Could not create debugfs "
-			   "'ftraced_enabled' entry\n");
 	return 0;
 }
 
 fs_initcall(ftrace_init_debugfs);
 
-#ifdef CONFIG_FTRACE_MCOUNT_RECORD
 static int ftrace_convert_nops(unsigned long *start,
 			       unsigned long *end)
 {
@@ -1601,7 +1458,7 @@ static int ftrace_convert_nops(unsigned 
 
 	/* p is ignored */
 	local_irq_save(flags);
-	__ftrace_update_code(p);
+	ftrace_update_code(p);
 	local_irq_restore(flags);
 
 	return 0;
@@ -1648,84 +1505,6 @@ void __init ftrace_init(void)
  failed:
 	ftrace_disabled = 1;
 }
-#else /* CONFIG_FTRACE_MCOUNT_RECORD */
-static int ftraced(void *ignore)
-{
-	unsigned long usecs;
-
-	while (!kthread_should_stop()) {
-
-		set_current_state(TASK_INTERRUPTIBLE);
-
-		/* check once a second */
-		schedule_timeout(HZ);
-
-		if (unlikely(ftrace_disabled))
-			continue;
-
-		mutex_lock(&ftrace_sysctl_lock);
-		mutex_lock(&ftraced_lock);
-		if (!ftraced_suspend && !ftraced_stop &&
-		    ftrace_update_code()) {
-			usecs = nsecs_to_usecs(ftrace_update_time);
-			if (ftrace_update_tot_cnt > 100000) {
-				ftrace_update_tot_cnt = 0;
-				pr_info("hm, dftrace overflow: %lu change%s"
-					" (%lu total) in %lu usec%s\n",
-					ftrace_update_cnt,
-					ftrace_update_cnt != 1 ? "s" : "",
-					ftrace_update_tot_cnt,
-					usecs, usecs != 1 ? "s" : "");
-				FTRACE_WARN_ON_ONCE(1);
-			}
-		}
-		mutex_unlock(&ftraced_lock);
-		mutex_unlock(&ftrace_sysctl_lock);
-
-		ftrace_shutdown_replenish();
-	}
-	__set_current_state(TASK_RUNNING);
-	return 0;
-}
-
-static int __init ftrace_dynamic_init(void)
-{
-	struct task_struct *p;
-	unsigned long addr;
-	int ret;
-
-	addr = (unsigned long)ftrace_record_ip;
-
-	stop_machine(ftrace_dyn_arch_init, &addr, NULL);
-
-	/* ftrace_dyn_arch_init places the return code in addr */
-	if (addr) {
-		ret = (int)addr;
-		goto failed;
-	}
-
-	ret = ftrace_dyn_table_alloc(NR_TO_INIT);
-	if (ret)
-		goto failed;
-
-	p = kthread_run(ftraced, NULL, "ftraced");
-	if (IS_ERR(p)) {
-		ret = -1;
-		goto failed;
-	}
-
-	last_ftrace_enabled = ftrace_enabled = 1;
-	ftraced_task = p;
-
-	return 0;
-
- failed:
-	ftrace_disabled = 1;
-	return ret;
-}
-
-core_initcall(ftrace_dynamic_init);
-#endif /* CONFIG_FTRACE_MCOUNT_RECORD */
 
 #else
 # define ftrace_startup()		do { } while (0)
@@ -1745,9 +1524,6 @@ void ftrace_kill(void)
 {
 	ftrace_disabled = 1;
 	ftrace_enabled = 0;
-#ifdef CONFIG_DYNAMIC_FTRACE
-	ftraced_suspend = -1;
-#endif
 	clear_ftrace_function();
 }
 
kernel/trace/trace_selftest.c +0 -14
Index: linux-compile.git/kernel/trace/trace_selftest.c
===================================================================
@@ -99,13 +99,6 @@ int trace_selftest_startup_dynamic_traci
 	/* passed in by parameter to fool gcc from optimizing */
 	func();
 
-	/* update the records */
-	ret = ftrace_force_update();
-	if (ret) {
-		printk(KERN_CONT ".. ftraced failed .. ");
-		return ret;
-	}
-
 	/*
 	 * Some archs *cough*PowerPC*cough* add charachters to the
 	 * start of the function names. We simply put a '*' to
@@ -183,13 +176,6 @@ trace_selftest_startup_function(struct t
 	/* make sure msleep has been recorded */
 	msleep(1);
 
-	/* force the recorded functions to be traced */
-	ret = ftrace_force_update();
-	if (ret) {
-		printk(KERN_CONT ".. ftraced failed .. ");
-		return ret;
-	}
-
 	/* start the tracing */
 	ftrace_enabled = 1;
 	tracer_enabled = 1;
author maintainer
#35 lore raw

PATCH 10/11 ftrace: remove mcount set

In reply to Steven Rostedt

The arch dependent function ftrace_mcount_set was only used by the daemon start up code. This patch removes it.

---
 arch/arm/kernel/ftrace.c     |   13 -------------
 arch/powerpc/kernel/ftrace.c |   17 -----------------
 arch/x86/kernel/ftrace.c     |    7 -------
 include/linux/ftrace.h       |    1 -
 kernel/trace/ftrace.c        |    9 ---------
 5 files changed, 47 deletions(-)
arch/arm/kernel/ftrace.c +0 -13
Index: linux-compile.git/arch/arm/kernel/ftrace.c
===================================================================
@@ -95,19 +95,6 @@ int ftrace_update_ftrace_func(ftrace_fun
 	return ret;
 }
 
-int ftrace_mcount_set(unsigned long *data)
-{
-	unsigned long pc, old;
-	unsigned long *addr = data;
-	unsigned char *new;
-
-	pc = (unsigned long)&mcount_call;
-	memcpy(&old, &mcount_call, MCOUNT_INSN_SIZE);
-	new = ftrace_call_replace(pc, *addr);
-	*addr = ftrace_modify_code(pc, (unsigned char *)&old, new);
-	return 0;
-}
-
 /* run from kstop_machine */
 int __init ftrace_dyn_arch_init(void *data)
 {
arch/powerpc/kernel/ftrace.c +0 -17
Index: linux-compile.git/arch/powerpc/kernel/ftrace.c
===================================================================
@@ -126,23 +126,6 @@ notrace int ftrace_update_ftrace_func(ft
 	return ret;
 }
 
-notrace int ftrace_mcount_set(unsigned long *data)
-{
-	unsigned long ip = (long)(&mcount_call);
-	unsigned long *addr = data;
-	unsigned char old[MCOUNT_INSN_SIZE], *new;
-
-	/*
-	 * Replace the mcount stub with a pointer to the
-	 * ip recorder function.
-	 */
-	memcpy(old, &mcount_call, MCOUNT_INSN_SIZE);
-	new = ftrace_call_replace(ip, *addr);
-	*addr = ftrace_modify_code(ip, old, new);
-
-	return 0;
-}
-
 int __init ftrace_dyn_arch_init(void *data)
 {
 	/* This is running in kstop_machine */
arch/x86/kernel/ftrace.c +0 -7
Index: linux-compile.git/arch/x86/kernel/ftrace.c
===================================================================
@@ -105,13 +105,6 @@ notrace int ftrace_update_ftrace_func(ft
 	return ret;
 }
 
-notrace int ftrace_mcount_set(unsigned long *data)
-{
-	/* mcount is initialized as a nop */
-	*data = 0;
-	return 0;
-}
-
 int __init ftrace_dyn_arch_init(void *data)
 {
 	extern const unsigned char ftrace_test_p6nop[];
include/linux/ftrace.h +0 -1
Index: linux-compile.git/include/linux/ftrace.h
===================================================================
@@ -71,7 +71,6 @@ extern int ftrace_ip_converted(unsigned 
 extern unsigned char *ftrace_nop_replace(void);
 extern unsigned char *ftrace_call_replace(unsigned long ip, unsigned long addr);
 extern int ftrace_dyn_arch_init(void *data);
-extern int ftrace_mcount_set(unsigned long *data);
 extern int ftrace_modify_code(unsigned long ip, unsigned char *old_code,
 			      unsigned char *new_code);
 extern int ftrace_update_ftrace_func(ftrace_func_t func);
kernel/trace/ftrace.c +0 -9
Index: linux-compile.git/kernel/trace/ftrace.c
===================================================================
@@ -606,7 +606,6 @@ static int ftrace_update_code(void *igno
 
 static int __ftrace_modify_code(void *data)
 {
-	unsigned long addr;
 	int *command = data;
 
 	if (*command & FTRACE_ENABLE_CALLS) {
@@ -625,14 +624,6 @@ static int __ftrace_modify_code(void *da
 	if (*command & FTRACE_UPDATE_TRACE_FUNC)
 		ftrace_update_ftrace_func(ftrace_trace_function);
 
-	if (*command & FTRACE_ENABLE_MCOUNT) {
-		addr = (unsigned long)ftrace_record_ip;
-		ftrace_mcount_set(&addr);
-	} else if (*command & FTRACE_DISABLE_MCOUNT) {
-		addr = (unsigned long)ftrace_stub;
-		ftrace_mcount_set(&addr);
-	}
-
 	return 0;
 }
 
author maintainer
#36 lore raw

PATCH 11/11 ftrace: remove ftrace hash

In reply to Steven Rostedt

The ftrace hash was used by the ftrace_daemon code. The record ip function would place the calling address (ip) into the hash. The daemon would later read the hash and modify that code.

The hash complicates the code. This patch removes it.

---
 include/linux/ftrace.h |    8 -
 kernel/trace/ftrace.c  |  247 +++++++------------------------------------------
 kernel/trace/trace.c   |    3 
 3 files changed, 40 insertions(+), 218 deletions(-)
include/linux/ftrace.h +3 -5
Index: linux-compile.git/include/linux/ftrace.h
===================================================================
@@ -44,8 +44,6 @@ static inline void ftrace_kill(void) { }
 #endif /* CONFIG_FTRACE */
 
 #ifdef CONFIG_DYNAMIC_FTRACE
-# define FTRACE_HASHBITS	10
-# define FTRACE_HASHSIZE	(1<<FTRACE_HASHBITS)
 
 enum {
 	FTRACE_FL_FREE		= (1 << 0),
@@ -58,9 +56,9 @@ enum {
 };
 
 struct dyn_ftrace {
-	struct hlist_node node;
-	unsigned long	  ip; /* address of mcount call-site */
-	unsigned long	  flags;
+	struct list_head	list;
+	unsigned long		ip; /* address of mcount call-site */
+	unsigned long		flags;
 };
 
 int ftrace_force_update(void);
kernel/trace/ftrace.c +37 -210

353 lines not on this page · raw message

kernel/trace/trace.c +0 -3
Index: linux-compile.git/kernel/trace/trace.c
===================================================================
@@ -865,9 +865,6 @@ function_trace_call(unsigned long ip, un
 	if (unlikely(!ftrace_function_enabled))
 		return;
 
-	if (skip_trace(ip))
-		return;
-
 	pc = preempt_count();
 	resched = need_resched();
 	preempt_disable_notrace();