[PATCH v2 0/6] ftrace: to kill a daemon (small updates)

[PATCH v2 0/6] ftrace: to kill a daemon (small updates)

[Steven Rostedt]

[
  Changes since v1:

    regex fix in x86_64 recordmcount.pl. Now it can handle all
      mcount+0x... mcount-0x... and mcount, where as the original
      only handled mcount+0x...

    Made mcount on start-up to simply return. The current mcount
      is set up to be replaced with a call to ftrace_record_ip.
      This is no longer necessary.

  Note: This patch series is focusing on how calls to mcount in
    the kernel are converted to nops. It does not address what
    kind of nop is used. That is a different topic, and should
    be in a different patch series.

  Note 2: I have found that the changes here are more stable than
    the current daemon method, and these patches should be used.
    It also solves the resume from suspend to ram bug that was
    reported:

      http://lkml.org/lkml/2008/8/12/234
       and
      http://lkml.org/lkml/2008/8/12/451

  Note 3: I have already ported this to PowerPC64, but I am waiting
    for this to be accepted first before submitting those changes.
]

One of the things that bothered me about the latest ftrace code was
this annoying daemon that would wake up once a second to see if it
had work to do. If it did not, it would go to sleep, otherwise it would
do its work and then go to sleep.

You see, the reason for this is that for ftrace to maintain performance
when configured in but disabled, it would need to change all the
locations that called "mcount" (enabled with the gcc -pg option) into
nops.  The "-pg" option in gcc sets up a function profiler to call this
function called "mcount". If you simply have "mcount" return, it will
still add 15 to 18% overhead in performance. Changing all the calls to
nops moved the overhead into noise.

To get rid of this, I had the mcount code record the location that called
it. Later, the "ftraced" daemon would wake up and look to see if
any new functions were recorded. If so, it would call kstop_machine
and convert the calls to "nops". We needed kstop_machine because bad
things happen on SMP if you modify code that happens to be in the
instruction cache of another CPU.

This "ftraced" kernel thread would be a happy little worker, but it caused
some pains. One, is that it woke up once a second, and Ted Tso got mad
at me because it would show up on PowerTop. I could easily make the
default 5 seconds, and even have it runtime configurable, with a trivial
patch. I have not got around to doing that yet.

The other annoying thing, and this one bothers me the most, is that we
can not have this enabled on a production -rt kernel.  The latency caused
by the kstop_machine when doing work is lost in the noise on a non-rt
kernel, but it can be up to 800 microseconds, and that would kill
the -rt kernel.  The reason this bothered me the most, is that -rt is
where it came from, and ftraced was not treating its motherland very well.

Along came Gregory Haskins, who was bickering about having ftrace enabled
on a production -rt kernel. I told him the reasons that this would be bad
and then he started thinking out loud, and suggesting wild ideas, like
patching gcc!

Since I have recently seen "The Dark Knight", Gregory's comments put me
into an "evil" mood.  I then thought of the idea about using the
relocation entries of the mcount call sites, in a prelinked object file,
and create a separate section with a list of these sites. On boot up,
record them and change them into nops.

That's it! No kstop_machine for turning them into nops. We would only need
stop_machine to enable or disable tracing, but a user not tracing will not have
to deal with this annoying "ftraced" kernel thread waking up every second
or ever running kstop_machine.

What's more, this means we can enable it on a production -rt kernel!

Now, this was no easy task. We needed to add a section to every object
file with a list of pointers to the call sites to mcount. The idea I came
up with was to make a tmp.s file for every object just after it is compiled.
This tmp.s would then be compiled and relinked into the original object.
The tmp.s file would have something like:

  .section __mcount_loc,"a",@progbits
  .quad location_of_mcount1
  .quad location_of_mcount2
  (etc)

By running objdump on the object file we can find the offsets into the
sections that the functions are called.

For example, looking at hrtimer.o:

Disassembly of section .text:

0000000000000000 <hrtimer_init_sleeper>:
       0:       55                      push   %rbp
       1:       48 89 e5                mov    %rsp,%rbp
       4:       e8 00 00 00 00          callq  9 <hrtimer_init_sleeper+0x9>
                        5: R_X86_64_PC32        mcount+0xfffffffffffffffc
[...]

the '5' in the '5: R_X86_64_PC32' is the offset that the mcount relocation
is to be done for the call site. This offset is from the .text section,
and not necessarily, from the function. If we look further we see:

000000000000001e <ktime_add_safe>:
      1e:       55                      push   %rbp
      1f:       48 89 e5                mov    %rsp,%rbp
      22:       e8 00 00 00 00          callq  27 <ktime_add_safe+0x9>
                        23: R_X86_64_PC32       mcount+0xfffffffffffffffc


This mcount call site is 0x23 from the .text section, and obviously
not from the ktime_add_safe.

If we make a tmp.s that has the following:

  .section __mcount_loc,"a",@progbits
  .quad hrtimer_init_sleeper + 0x5
  .quad hrtimer_init_sleeper + 0x23

We have a section with the locations of these two call sites. After the final
linking, they will point to the actual address used.

All that would need to be done is:

gcc -c tmp.s -o tmp.o
ld -r tmp.o hrtimer.o -o tmp_hrtime.o
mv tmp_hrtimer.o hrtimer.o

Easy as that! Not quite.  What happens if that first function in the
section is a static function? That is, the symbol for the function
is local to the object. If for some reason hrtimer_init_sleeper is static,
the tmp_hrtimer.o would have two symbols for hrtimer_init_sleeper.
One local and one global.

But we can be even more evil with this idea. We can do crazy things
with objcopy to solve it for us.

  objcopy --globalize-symbol hrtimer_init_sleeper hrtimer.o tmp_hrtimer.o

Now the hrtimer_init_sleeper would be global for linking.

  ld -r tmp_hrtimer.o tmp.o -o tmp2_hrtimer.o

Now the tmp.o could use the same global hrtimer_init_sleeper symbol.
But we have tmp2_hritmer.o that has the tmp.o and tmp_hrtimer.o symbols,
but we cant just blindly convert local symbols to globals.

The solution is simply put it back to local.

  objcopy --localize-symbol hrtimer_init_sleeper tmp2_hrtimer.o hrtimer.o

Now our hrtimer.o file has our __mcount_loc section and the
reference to hrtimer_init_sleeper will be resolved.

This is a bit complex to do in shell scripting and Makefiles, so I wrote
a well documented recordmcount.pl perl script, that will do the above
all in one place.

With this new update, we can work to kill that kernel thread "ftraced"!

This patch set ports to x86_64 and i386, the other archs will still use
the daemon until they are converted over.

I tested this on both x86_64 and i386 with and without CONFIG_RELOCATE
set.

[PATCH v2 1/6] ftrace: create __mcount_loc section

[Steven Rostedt]

[-- Attachment #1: ftrace-record-mcount.patch --]
[-- Type: text/plain, Size: 15695 bytes --]

[
 Note: after this patch is applied, you need to run:

   chmod +x scripts/recordmcount.pl
]


This patch creates a section in the kernel called "__mcount_loc".
This will hold a list of pointers to the mcount relocation for
each call site of mcount.

For example:

objdump -dr init/main.o
[...]
Disassembly of section .text:

0000000000000000 <do_one_initcall>:
   0:   55                      push   %rbp
[...]
000000000000017b <init_post>:
 17b:   55                      push   %rbp
 17c:   48 89 e5                mov    %rsp,%rbp
 17f:   53                      push   %rbx
 180:   48 83 ec 08             sub    $0x8,%rsp
 184:   e8 00 00 00 00          callq  189 <init_post+0xe>
                        185: R_X86_64_PC32      mcount+0xfffffffffffffffc
[...]

We will add a section to point to each function call.

   .section __mcount_loc,"a",@progbits
[...]
   .quad .text + 0x185
[...]

The offset to of the mcount call site in init_post is an offset from
the start of the section, and not the start of the function init_post.
The mcount relocation is at the call site 0x185 from the start of the
.text section.

  .text + 0x185  == init_post + 0xa

We need a way to add this __mcount_loc section in a way that we do not
lose the relocations after final link.  The .text section here will
be attached to all other .text sections after final link and the
offsets will be meaningless.  We need to keep track of where these
.text sections are.

To do this, we use the start of the first function in the section.
do_one_initcall.  We can make a tmp.s file with this function as a reference
to the start of the .text section.

   .section __mcount_loc,"a",@progbits
[...]
   .quad do_one_initcall + 0x185
[...]

Then we can compile the tmp.s into a tmp.o

  gcc -c tmp.s -o tmp.o

And link it into back into main.o.

  ld -r main.o tmp.o -o tmp_main.o
  mv tmp_main.o main.o

But we have a problem.  What happens if the first function in a section
is not exported, and is a static function. The linker will not let
the tmp.o use it.  This case exists in main.o as well.

Disassembly of section .init.text:

0000000000000000 <set_reset_devices>:
   0:   55                      push   %rbp
   1:   48 89 e5                mov    %rsp,%rbp
   4:   e8 00 00 00 00          callq  9 <set_reset_devices+0x9>
                        5: R_X86_64_PC32        mcount+0xfffffffffffffffc

The first function in .init.text is a static function.

#  nm init/main.o | grep set_reset_devices
00000000000000a8 t __setup_set_reset_devices
000000000000105f t __setup_str_set_reset_devices
0000000000000000 t set_reset_devices

The lowercase 't' means that set_reset_devices is local and is not exported.
If we simply try to link the tmp.o with the set_reset_devices we end
up with two symbols: one local and one global.

# cat tmp.s
 .section __mcount_loc,"a",@progbits
 .quad set_reset_devices + 0x10

# gcc -c tmp.s -o tmp.o
# ld -r main.o tmp.o -o tmp_main.o
# nm tmp_main.o | grep set_reset_devices
00000000000000a8 t __setup_set_reset_devices
000000000000105f t __setup_str_set_reset_devices
0000000000000000 t set_reset_devices
                 U set_reset_devices

We still have an undefined reference to set_reset_devices, and if we try
to compile the kernel, we will end up with an undefined reference to
set_reset_devices, or even worst, it could be exported someplace else,
and then we will have a reference to the wrong location.

To handle this case, we make an intermediate step using objcopy.
We convert set_reset_devices into a global exported symbol before linking
it with tmp.o and set it back afterwards.

# objcopy --globalize-symbol set_reset_devices main.o tmp_main.o
# nm tmp_main.o | grep set_reset_devices
00000000000000a8 t __setup_set_reset_devices
000000000000105f t __setup_str_set_reset_devices
0000000000000000 T set_reset_devices

# ld -r tmp_main.o tmp.o -o tmp2_main.o
# nm tmp2_main.o | grep set_reset_devices
00000000000000a8 t __setup_set_reset_devices
000000000000105f t __setup_str_set_reset_devices
0000000000000000 T set_reset_devices

# objcopy --localize-symbol set_reset_devices tmp2_main.o main.o
# nm main.o | grep set_reset_devices
00000000000000a8 t __setup_set_reset_devices
000000000000105f t __setup_str_set_reset_devices
0000000000000000 t set_reset_devices

Now we have a section in main.o called __mcount_loc that we can place
somewhere in the kernel using vmlinux.ld.S and access it to convert
all these locations that call mcount into nops before starting SMP
and thus, eliminating the need to do this with kstop_machine.

Note, A well documented perl script (scripts/recordmcount.pl) is used
to do all this in one location.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 include/asm-generic/vmlinux.lds.h |    8 +
 kernel/trace/Kconfig              |    8 +
 scripts/Makefile.build            |    6 
 scripts/recordmcount.pl           |  280 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 302 insertions(+)

Index: linux-tip.git/include/asm-generic/vmlinux.lds.h
===================================================================
--- linux-tip.git.orig/include/asm-generic/vmlinux.lds.h	2008-08-14 13:55:11.000000000 -0400
+++ linux-tip.git/include/asm-generic/vmlinux.lds.h	2008-08-14 13:57:06.000000000 -0400
@@ -37,6 +37,13 @@
 #define MEM_DISCARD(sec) *(.mem##sec)
 #endif
 
+#ifdef CONFIG_FTRACE_MCOUNT_RECORD
+#define MCOUNT_REC()	VMLINUX_SYMBOL(__start_mcount_loc) = .; \
+			*(__mcount_loc)				\
+			VMLINUX_SYMBOL(__stop_mcount_loc) = .;
+#else
+#define MCOUNT_REC()
+#endif
 
 /* .data section */
 #define DATA_DATA							\
@@ -192,6 +199,7 @@
 	/* __*init sections */						\
 	__init_rodata : AT(ADDR(__init_rodata) - LOAD_OFFSET) {		\
 		*(.ref.rodata)						\
+		MCOUNT_REC()						\
 		DEV_KEEP(init.rodata)					\
 		DEV_KEEP(exit.rodata)					\
 		CPU_KEEP(init.rodata)					\
Index: linux-tip.git/scripts/Makefile.build
===================================================================
--- linux-tip.git.orig/scripts/Makefile.build	2008-08-14 13:55:16.000000000 -0400
+++ linux-tip.git/scripts/Makefile.build	2008-08-14 13:57:06.000000000 -0400
@@ -198,10 +198,16 @@ cmd_modversions =							\
 	fi;
 endif
 
+ifdef CONFIG_FTRACE_MCOUNT_RECORD
+cmd_record_mcount = scripts/recordmcount.pl "$(ARCH)" \
+	"$(OBJDUMP)" "$(OBJCOPY)" "$(CC)" "$(LD)" "$(NM)" "$(RM)" "$(MV)" "$(@)";
+endif
+
 define rule_cc_o_c
 	$(call echo-cmd,checksrc) $(cmd_checksrc)			  \
 	$(call echo-cmd,cc_o_c) $(cmd_cc_o_c);				  \
 	$(cmd_modversions)						  \
+	$(cmd_record_mcount)						  \
 	scripts/basic/fixdep $(depfile) $@ '$(call make-cmd,cc_o_c)' >    \
 	                                              $(dot-target).tmp;  \
 	rm -f $(depfile);						  \
Index: linux-tip.git/scripts/recordmcount.pl
===================================================================
--- /dev/null	1970-01-01 00:00:00.000000000 +0000
+++ linux-tip.git/scripts/recordmcount.pl	2008-08-14 13:57:39.000000000 -0400
@@ -0,0 +1,280 @@
+#!/usr/bin/perl -w
+# (c) 2008, Steven Rostedt <srostedt@redhat.com>
+# Licensed under the terms of the GNU GPL License version 2
+#
+# recordmcount.pl - makes a section called __mcount_loc that holds
+#                   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.
+#
+# 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.
+#
+# 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.
+# e.g.
+#
+#  .section ".text.sched"
+#  .globl my_func
+#  my_func:
+#        [...]
+#        call mcount  (offset: 0x5)
+#        [...]
+#        ret
+#  other_func:
+#        [...]
+#        call mcount (offset: 0x1b)
+#        [...]
+#
+# Both relocation offsets for the mcounts in the above example will be
+# offset from .text.sched. If we make another file called tmp.s with:
+#
+#  .section __mcount_loc
+#  .quad  my_func + 0x5
+#  .quad  my_func + 0x1b
+#
+# We can then compile this tmp.s into tmp.o, and link it to the original
+# object.
+#
+# But this gets hard if my_func is not globl (a static function).
+# In such a case we have:
+#
+#  .section ".text.sched"
+#  my_func:
+#        [...]
+#        call mcount  (offset: 0x5)
+#        [...]
+#        ret
+#  .globl my_func
+#  other_func:
+#        [...]
+#        call mcount (offset: 0x1b)
+#        [...]
+#
+# 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:
+# one local, one global.  After final compile, we will end up with
+# an undefined reference to my_func.
+#
+# 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
+# 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.
+#
+# Here are the steps we take:
+#
+# 1) Record all the local 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.
+# 4) Do we have to deal with local functions? If not, go to step 8.
+# 5) Make an object that converts these local functions to global symbols
+#    with objcopy.
+# 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;
+
+my $P = $0;
+$P =~ s@.*/@@g;
+
+my $V = '0.1';
+
+if ($#ARGV < 6) {
+	print "usage: $P arch objdump objcopy cc ld nm rm mv inputfile\n";
+	print "version: $V\n";
+	exit(1);
+}
+
+my ($arch, $objdump, $objcopy, $cc, $ld, $nm, $rm, $mv, $inputfile) = @ARGV;
+
+$objdump = "objdump" if ((length $objdump) == 0);
+$objcopy = "objcopy" if ((length $objcopy) == 0);
+$cc = "gcc" if ((length $cc) == 0);
+$ld = "ld" if ((length $ld) == 0);
+$nm = "nm" if ((length $nm) == 0);
+$rm = "rm" if ((length $rm) == 0);
+$mv = "mv" if ((length $mv) == 0);
+
+#print STDERR "running: $P '$arch' '$objdump' '$objcopy' '$cc' '$ld' " .
+#    "'$nm' '$rm' '$mv' '$inputfile'\n";
+
+my %locals;
+my %convert;
+
+my $type;
+my $section_regex;	# Find the start of a section
+my $function_regex;	# Find the name of a function (return func name)
+my $mcount_regex;	# Find the call site to mcount (return offset)
+
+if ($arch eq "x86_64") {
+    $section_regex = "Disassembly of section";
+    $function_regex = "<(.*?)>:";
+    $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$";
+    $type = ".quad";
+} elsif ($arch eq "i386") {
+    $section_regex = "Disassembly of section";
+    $function_regex = "<(.*?)>:";
+    $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
+    $type = ".long";
+} else {
+    die "Arch $arch is not supported with CONFIG_FTRACE_MCOUNT_RECORD";
+}
+
+my $text_found = 0;
+my $read_function = 0;
+my $opened = 0;
+my $text = "";
+my $mcount_section = "__mcount_loc";
+
+my $dirname;
+my $filename;
+my $prefix;
+my $ext;
+
+if ($inputfile =~ m,^(.*)/([^/]*)$,) {
+    $dirname = $1;
+    $filename = $2;
+} else {
+    $dirname = ".";
+    $filename = $inputfile;
+}
+
+if ($filename =~ m,^(.*)(\.\S),) {
+    $prefix = $1;
+    $ext = $2;
+} else {
+    $prefix = $filename;
+    $ext = "";
+}
+
+my $mcount_s = $dirname . "/.tmp_mc_" . $prefix . ".s";
+my $mcount_o = $dirname . "/.tmp_mc_" . $prefix . ".o";
+
+#
+# Step 1: find all the local symbols (static functions).
+#
+open (IN, "$nm $inputfile|") || die "error running $nm";
+while (<IN>) {
+    if (/^[0-9a-fA-F]+\s+t\s+(\S+)/) {
+	$locals{$1} = 1;
+    }
+}
+close(IN);
+
+#
+# Step 2: find the sections and mcount call sites
+#
+open(IN, "$objdump -dr $inputfile|") || die "error running $objdump";
+
+while (<IN>) {
+    # is it a section?
+    if (/$section_regex/) {
+	$read_function = 1;
+	$text_found = 0;
+    # section found, now is this a start of a function?
+    } elsif ($read_function && /$function_regex/) {
+	$read_function = 0;
+	$text_found = 1;
+	$text = $1;
+	# is this function static? If so, note this fact.
+	if (defined $locals{$text}) {
+	    $convert{$text} = 1;
+	}
+    # is this a call site to mcount? If so, print the offset from the section
+    } elsif ($text_found && /$mcount_regex/) {
+	if (!$opened) {
+	    open(FILE, ">$mcount_s") || die "can't create $mcount_s\n";
+	    $opened = 1;
+	    print FILE "\t.section $mcount_section,\"a\",\@progbits\n";
+	}
+	print FILE "\t$type $text + 0x$1\n";
+    }
+}
+
+# If we did not find any mcount callers, we are done (do nothing).
+if (!$opened) {
+    exit(0);
+}
+
+close(FILE);
+
+#
+# Step 3: Compile the file that holds the list of call sites to mcount.
+#
+`$cc -o $mcount_o -c $mcount_s`;
+
+my @converts = keys %convert;
+
+#
+# Step 4: Do we have sections that started with local functions?
+#
+if ($#converts >= 0) {
+    my $globallist = "";
+    my $locallist = "";
+
+    foreach my $con (@converts) {
+	$globallist .= " --globalize-symbol $con";
+	$locallist .= " --localize-symbol $con";
+    }
+
+    my $globalobj = $dirname . "/.tmp_gl_" . $filename;
+    my $globalmix = $dirname . "/.tmp_mx_" . $filename;
+
+    #
+    # Step 5: set up each local function as a global
+    #
+    `$objcopy $globallist $inputfile $globalobj`;
+
+    #
+    # Step 6: Link the global version to our list.
+    #
+    `$ld -r $globalobj $mcount_o -o $globalmix`;
+
+    #
+    # Step 7: Convert the local functions back into local symbols
+    #
+    `$objcopy $locallist $globalmix $inputfile`;
+
+    # Remove the temp files
+    `$rm $globalobj $globalmix`;
+
+} else {
+
+    my $mix = $dirname . "/.tmp_mx_" . $filename;
+
+    #
+    # Step 8: Link the object with our list of call sites object.
+    #
+    `$ld -r $inputfile $mcount_o -o $mix`;
+
+    #
+    # Step 9: Move the result back to the original object.
+    #
+    `$mv $mix $inputfile`;
+}
+
+# Clean up the temp files
+`$rm $mcount_o $mcount_s`;
+
+exit(0);
Index: linux-tip.git/kernel/trace/Kconfig
===================================================================
--- linux-tip.git.orig/kernel/trace/Kconfig	2008-08-14 13:55:13.000000000 -0400
+++ linux-tip.git/kernel/trace/Kconfig	2008-08-14 13:57:06.000000000 -0400
@@ -7,6 +7,9 @@ config HAVE_FTRACE
 config HAVE_DYNAMIC_FTRACE
 	bool
 
+config HAVE_FTRACE_MCOUNT_RECORD
+	bool
+
 config TRACER_MAX_TRACE
 	bool
 
@@ -122,6 +125,11 @@ config DYNAMIC_FTRACE
 	 were made. If so, it runs stop_machine (stops all CPUS)
 	 and modifies the code to jump over the call to ftrace.
 
+config FTRACE_MCOUNT_RECORD
+	def_bool y
+	depends on DYNAMIC_FTRACE
+	depends on HAVE_FTRACE_MCOUNT_RECORD
+
 config FTRACE_SELFTEST
 	bool
 

-- 

[PATCH v2 2/6] ftrace: mcount call site on boot nops core

[Steven Rostedt]

[-- Attachment #1: ftrace-convert-recorded-mcount.patch --]
[-- Type: text/plain, Size: 7313 bytes --]

This is the infrastructure to the converting the mcount call sites
recorded by the __mcount_loc section into nops on boot. It also allows
for using these sites to enable tracing as normal. When the __mcount_loc
section is used, the "ftraced" kernel thread is disabled.

This uses the current infrastructure to record the mcount call sites
as well as convert them to nops. The mcount function is kept as a stub
on boot up and not converted to the ftrace_record_ip function. We use the
ftrace_record_ip to only record from the table.

This patch does not handle modules. That comes with a later patch.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 include/asm-x86/ftrace.h |   10 +++
 include/linux/ftrace.h   |    6 +
 init/main.c              |    3 
 kernel/trace/ftrace.c    |  148 +++++++++++++++++++++++++++++++++--------------
 4 files changed, 124 insertions(+), 43 deletions(-)

Index: linux-tip.git/include/asm-x86/ftrace.h
===================================================================
--- linux-tip.git.orig/include/asm-x86/ftrace.h	2008-08-14 13:55:12.000000000 -0400
+++ linux-tip.git/include/asm-x86/ftrace.h	2008-08-14 13:59:19.000000000 -0400
@@ -7,6 +7,16 @@
 
 #ifndef __ASSEMBLY__
 extern void mcount(void);
+
+static inline unsigned long ftrace_call_adjust(unsigned long addr)
+{
+	/*
+	 * call mcount is "e8 <4 byte offset>"
+	 * The addr points to the 4 byte offset and the caller of this
+	 * function wants the pointer to e8. Simply subtract one.
+	 */
+	return addr - 1;
+}
 #endif
 
 #endif /* CONFIG_FTRACE */
Index: linux-tip.git/include/linux/ftrace.h
===================================================================
--- linux-tip.git.orig/include/linux/ftrace.h	2008-08-14 13:55:12.000000000 -0400
+++ linux-tip.git/include/linux/ftrace.h	2008-08-14 13:59:19.000000000 -0400
@@ -141,4 +141,10 @@ static inline void
 ftrace_special(unsigned long arg1, unsigned long arg2, unsigned long arg3) { }
 #endif
 
+#ifdef CONFIG_FTRACE_MCOUNT_RECORD
+extern void ftrace_init(void);
+#else
+static inline void ftrace_init(void) { }
+#endif
+
 #endif /* _LINUX_FTRACE_H */
Index: linux-tip.git/init/main.c
===================================================================
--- linux-tip.git.orig/init/main.c	2008-08-14 13:55:13.000000000 -0400
+++ linux-tip.git/init/main.c	2008-08-14 13:59:19.000000000 -0400
@@ -62,6 +62,7 @@
 #include <linux/signal.h>
 #include <linux/idr.h>
 #include <linux/kmemcheck.h>
+#include <linux/ftrace.h>
 
 #include <asm/io.h>
 #include <asm/bugs.h>
@@ -820,6 +821,8 @@ asmlinkage void __init start_kernel(void
 
 	acpi_early_init(); /* before LAPIC and SMP init */
 
+	ftrace_init();
+
 	/* Do the rest non-__init'ed, we're now alive */
 	rest_init();
 }
Index: linux-tip.git/kernel/trace/ftrace.c
===================================================================
--- linux-tip.git.orig/kernel/trace/ftrace.c	2008-08-14 13:55:13.000000000 -0400
+++ linux-tip.git/kernel/trace/ftrace.c	2008-08-14 13:59:19.000000000 -0400
@@ -799,47 +799,7 @@ static int ftrace_update_code(void)
 	return 1;
 }
 
-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_disabled = 1;
-				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_dyn_table_alloc(void)
+static int __init ftrace_dyn_table_alloc(unsigned long num_to_init)
 {
 	struct ftrace_page *pg;
 	int cnt;
@@ -866,7 +826,9 @@ static int __init ftrace_dyn_table_alloc
 
 	pg = ftrace_pages = ftrace_pages_start;
 
-	cnt = NR_TO_INIT / ENTRIES_PER_PAGE;
+	cnt = num_to_init / ENTRIES_PER_PAGE;
+	pr_info("ftrace: allocating %ld hash entries in %d pages\n",
+		num_to_init, cnt);
 
 	for (i = 0; i < cnt; i++) {
 		pg->next = (void *)get_zeroed_page(GFP_KERNEL);
@@ -1563,6 +1525,104 @@ static __init int ftrace_init_debugfs(vo
 
 fs_initcall(ftrace_init_debugfs);
 
+#ifdef CONFIG_FTRACE_MCOUNT_RECORD
+static int ftrace_convert_nops(unsigned long *start,
+			       unsigned long *end)
+{
+	unsigned long *p;
+	unsigned long addr;
+	unsigned long flags;
+
+	p = start;
+	while (p < end) {
+		addr = ftrace_call_adjust(*p++);
+		ftrace_record_ip(addr);
+		ftrace_shutdown_replenish();
+	}
+
+	/* p is ignored */
+	local_irq_save(flags);
+	__ftrace_update_code(p);
+	local_irq_restore(flags);
+
+	return 0;
+}
+
+extern unsigned long __start_mcount_loc[];
+extern unsigned long __stop_mcount_loc[];
+
+void __init ftrace_init(void)
+{
+	unsigned long count, addr, flags;
+	int ret;
+
+	/* Keep the ftrace pointer to the stub */
+	addr = (unsigned long)ftrace_stub;
+
+	local_irq_save(flags);
+	ftrace_dyn_arch_init(&addr);
+	local_irq_restore(flags);
+
+	/* ftrace_dyn_arch_init places the return code in addr */
+	if (addr)
+		goto failed;
+
+	count = __stop_mcount_loc - __start_mcount_loc;
+
+	ret = ftrace_dyn_table_alloc(count);
+	if (ret)
+		goto failed;
+
+	last_ftrace_enabled = ftrace_enabled = 1;
+
+	ret = ftrace_convert_nops(__start_mcount_loc,
+				  __stop_mcount_loc);
+
+	return;
+ 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_disabled = 1;
+				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;
@@ -1579,7 +1639,7 @@ static int __init ftrace_dynamic_init(vo
 		goto failed;
 	}
 
-	ret = ftrace_dyn_table_alloc();
+	ret = ftrace_dyn_table_alloc(NR_TO_INIT);
 	if (ret)
 		goto failed;
 
@@ -1600,6 +1660,8 @@ static int __init ftrace_dynamic_init(vo
 }
 
 core_initcall(ftrace_dynamic_init);
+#endif /* CONFIG_FTRACE_MCOUNT_RECORD */
+
 #else
 # define ftrace_startup()		do { } while (0)
 # define ftrace_shutdown()		do { } while (0)

-- 

[PATCH v2 3/6] ftrace: enable mcount recording for modules

[Steven Rostedt]

[-- Attachment #1: ftrace-mcount-loc-modules.patch --]
[-- Type: text/plain, Size: 3102 bytes --]

This patch enables the loading of the __mcount_section of modules and
changing all the callers of mcount into nops.

The modification is done before the init_module function is called, so
again, we do not need to use kstop_machine to make these changes.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 include/linux/ftrace.h |    3 +++
 kernel/module.c        |   11 +++++++++++
 kernel/trace/ftrace.c  |    5 +++++
 3 files changed, 19 insertions(+)

Index: linux-tip.git/include/linux/ftrace.h
===================================================================
--- linux-tip.git.orig/include/linux/ftrace.h	2008-08-14 13:59:19.000000000 -0400
+++ linux-tip.git/include/linux/ftrace.h	2008-08-14 14:03:44.000000000 -0400
@@ -143,8 +143,11 @@ ftrace_special(unsigned long arg1, unsig
 
 #ifdef CONFIG_FTRACE_MCOUNT_RECORD
 extern void ftrace_init(void);
+extern void ftrace_init_module(unsigned long *start, unsigned long *end);
 #else
 static inline void ftrace_init(void) { }
+static inline void
+ftrace_init_module(unsigned long *start, unsigned long *end) { }
 #endif
 
 #endif /* _LINUX_FTRACE_H */
Index: linux-tip.git/kernel/module.c
===================================================================
--- linux-tip.git.orig/kernel/module.c	2008-08-14 13:55:13.000000000 -0400
+++ linux-tip.git/kernel/module.c	2008-08-14 14:03:44.000000000 -0400
@@ -48,6 +48,7 @@
 #include <linux/license.h>
 #include <asm/sections.h>
 #include <linux/tracepoint.h>
+#include <linux/ftrace.h>
 
 #if 0
 #define DEBUGP printk
@@ -1835,6 +1836,7 @@ static struct module *load_module(void _
 	unsigned int markersstringsindex;
 	unsigned int tracepointsindex;
 	unsigned int tracepointsstringsindex;
+	unsigned int mcountindex;
 	struct module *mod;
 	long err = 0;
 	void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */
@@ -2125,6 +2127,9 @@ static struct module *load_module(void _
 	tracepointsstringsindex = find_sec(hdr, sechdrs, secstrings,
 					"__tracepoints_strings");
 
+	mcountindex = find_sec(hdr, sechdrs, secstrings,
+			       "__mcount_loc");
+
 	/* Now do relocations. */
 	for (i = 1; i < hdr->e_shnum; i++) {
 		const char *strtab = (char *)sechdrs[strindex].sh_addr;
@@ -2185,6 +2190,12 @@ static struct module *load_module(void _
 			mod->tracepoints + mod->num_tracepoints);
 #endif
 	}
+
+	if (mcountindex) {
+		void *mseg = (void *)sechdrs[mcountindex].sh_addr;
+		ftrace_init_module(mseg, mseg + sechdrs[mcountindex].sh_size);
+	}
+
 	err = module_finalize(hdr, sechdrs, mod);
 	if (err < 0)
 		goto cleanup;
Index: linux-tip.git/kernel/trace/ftrace.c
===================================================================
--- linux-tip.git.orig/kernel/trace/ftrace.c	2008-08-14 13:59:19.000000000 -0400
+++ linux-tip.git/kernel/trace/ftrace.c	2008-08-14 14:03:44.000000000 -0400
@@ -1548,6 +1548,11 @@ static int ftrace_convert_nops(unsigned 
 	return 0;
 }
 
+void ftrace_init_module(unsigned long *start, unsigned long *end)
+{
+	ftrace_convert_nops(start, end);
+}
+
 extern unsigned long __start_mcount_loc[];
 extern unsigned long __stop_mcount_loc[];
 

-- 

Re: [PATCH v2 3/6] ftrace: enable mcount recording for modules

[Rusty Russell]

On Friday 15 August 2008 05:45:09 Steven Rostedt wrote:
> This patch enables the loading of the __mcount_section of modules and
> changing all the callers of mcount into nops.
...
> +	if (mcountindex) {
> +		void *mseg = (void *)sechdrs[mcountindex].sh_addr;
> +		ftrace_init_module(mseg, mseg + sechdrs[mcountindex].sh_size);

You don't actually need the if.  The size of section 0 is 0.

Rusty.

Re: [PATCH v2 3/6] ftrace: enable mcount recording for modules

[Steven Rostedt]

Rusty Russell wrote:
> On Friday 15 August 2008 05:45:09 Steven Rostedt wrote:
>   
>> This patch enables the loading of the __mcount_section of modules and
>> changing all the callers of mcount into nops.
>>     
> ...
>   
>> +	if (mcountindex) {
>> +		void *mseg = (void *)sechdrs[mcountindex].sh_addr;
>> +		ftrace_init_module(mseg, mseg + sechdrs[mcountindex].sh_size);
>>     
>
> You don't actually need the if.  The size of section 0 is 0.
>   

Thanks,

But I find the if is a bit more clear. It may confuse code reviewers if 
we expect them to know that sechdrs[0].sh_size is zero. This isn't a hot 
path.

-- Steve

[PATCH v2 4/6] ftrace: rebuild everything on change to FTRACE_MCOUNT_RECORD

[Steven Rostedt]

[-- Attachment #1: ftrace-compile-all-for-mcount-change.patch --]
[-- Type: text/plain, Size: 767 bytes --]

When enabling or disabling CONFIG_FTRACE_MCOUNT_RECORD, we want a full
kernel compile to handle the adding of the __mcount_loc sections.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 include/linux/kernel.h |    5 +++++
 1 file changed, 5 insertions(+)

Index: linux-tip.git/include/linux/kernel.h
===================================================================
--- linux-tip.git.orig/include/linux/kernel.h	2008-08-14 13:55:12.000000000 -0400
+++ linux-tip.git/include/linux/kernel.h	2008-08-14 14:04:40.000000000 -0400
@@ -489,4 +489,9 @@ struct sysinfo {
 #define NUMA_BUILD 0
 #endif
 
+/* Rebuild everything on CONFIG_FTRACE_MCOUNT_RECORD */
+#ifdef CONFIG_FTRACE_MCOUNT_RECORD
+# define REBUILD_DUE_TO_FTRACE_MCOUNT_RECORD
+#endif
+
 #endif

-- 

[PATCH v2 5/6] ftrace: enable using mcount recording on x86

[Steven Rostedt]

[-- Attachment #1: ftrace-x86-enable-mcount-record.patch --]
[-- Type: text/plain, Size: 728 bytes --]

Enable the use of the __mcount_loc infrastructure on x86_64 and i386.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 arch/x86/Kconfig |    1 +
 1 file changed, 1 insertion(+)

Index: linux-tip.git/arch/x86/Kconfig
===================================================================
--- linux-tip.git.orig/arch/x86/Kconfig	2008-08-14 13:55:03.000000000 -0400
+++ linux-tip.git/arch/x86/Kconfig	2008-08-14 14:05:05.000000000 -0400
@@ -25,6 +25,7 @@ config X86
 	select HAVE_KPROBES
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select HAVE_KRETPROBES
+	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FTRACE
 	select HAVE_KVM if ((X86_32 && !X86_VOYAGER && !X86_VISWS && !X86_NUMAQ) || X86_64)

-- 

[PATCH v2 6/6] ftrace: x86 mcount stub

[Steven Rostedt]

[-- Attachment #1: ftrace-x86-mcount-stub.patch --]
[-- Type: text/plain, Size: 2880 bytes --]

x86 now sets up the mcount locations through the build and no longer
needs to record the ip when the function is executed. This patch changes
the initial mcount to simply return. There's no need to do any other work.
If the ftrace start up test fails, the original mcount will be what everything
will use, so having this as fast as possible is a good thing.


Signed-off-by: Steven Rostedt <srostedt@redhat.com>
---
 arch/x86/kernel/entry_32.S |   14 --------------
 arch/x86/kernel/entry_64.S |   26 --------------------------
 arch/x86/kernel/ftrace.c   |   14 ++------------
 3 files changed, 2 insertions(+), 52 deletions(-)

Index: linux-tip.git/arch/x86/kernel/entry_32.S
===================================================================
--- linux-tip.git.orig/arch/x86/kernel/entry_32.S	2008-08-14 13:55:03.000000000 -0400
+++ linux-tip.git/arch/x86/kernel/entry_32.S	2008-08-14 14:06:00.000000000 -0400
@@ -1189,20 +1189,6 @@ ENDPROC(xen_failsafe_callback)
 #ifdef CONFIG_DYNAMIC_FTRACE
 
 ENTRY(mcount)
-	pushl %eax
-	pushl %ecx
-	pushl %edx
-	movl 0xc(%esp), %eax
-	subl $MCOUNT_INSN_SIZE, %eax
-
-.globl mcount_call
-mcount_call:
-	call ftrace_stub
-
-	popl %edx
-	popl %ecx
-	popl %eax
-
 	ret
 END(mcount)
 
Index: linux-tip.git/arch/x86/kernel/entry_64.S
===================================================================
--- linux-tip.git.orig/arch/x86/kernel/entry_64.S	2008-08-14 13:55:03.000000000 -0400
+++ linux-tip.git/arch/x86/kernel/entry_64.S	2008-08-14 14:06:10.000000000 -0400
@@ -64,32 +64,6 @@
 #ifdef CONFIG_FTRACE
 #ifdef CONFIG_DYNAMIC_FTRACE
 ENTRY(mcount)
-
-	subq $0x38, %rsp
-	movq %rax, (%rsp)
-	movq %rcx, 8(%rsp)
-	movq %rdx, 16(%rsp)
-	movq %rsi, 24(%rsp)
-	movq %rdi, 32(%rsp)
-	movq %r8, 40(%rsp)
-	movq %r9, 48(%rsp)
-
-	movq 0x38(%rsp), %rdi
-	subq $MCOUNT_INSN_SIZE, %rdi
-
-.globl mcount_call
-mcount_call:
-	call ftrace_stub
-
-	movq 48(%rsp), %r9
-	movq 40(%rsp), %r8
-	movq 32(%rsp), %rdi
-	movq 24(%rsp), %rsi
-	movq 16(%rsp), %rdx
-	movq 8(%rsp), %rcx
-	movq (%rsp), %rax
-	addq $0x38, %rsp
-
 	retq
 END(mcount)
 
Index: linux-tip.git/arch/x86/kernel/ftrace.c
===================================================================
--- linux-tip.git.orig/arch/x86/kernel/ftrace.c	2008-08-14 13:55:03.000000000 -0400
+++ linux-tip.git/arch/x86/kernel/ftrace.c	2008-08-14 14:18:21.000000000 -0400
@@ -112,18 +112,8 @@ notrace int ftrace_update_ftrace_func(ft
 
 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);
-
+	/* mcount is initialized as a nop */
+	*data = 0;
 	return 0;
 }
 

-- 

Re: [PATCH v2 0/6] ftrace: to kill a daemon (small updates)

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> [
>   Changes since v1:
> 
>     regex fix in x86_64 recordmcount.pl. Now it can handle all
>       mcount+0x... mcount-0x... and mcount, where as the original
>       only handled mcount+0x...
> 
>     Made mcount on start-up to simply return. The current mcount
>       is set up to be replaced with a call to ftrace_record_ip.
>       This is no longer necessary.
> 
>   Note: This patch series is focusing on how calls to mcount in
>     the kernel are converted to nops. It does not address what
>     kind of nop is used. That is a different topic, and should
>     be in a different patch series.
> 
>   Note 2: I have found that the changes here are more stable than
>     the current daemon method, and these patches should be used.
>     It also solves the resume from suspend to ram bug that was
>     reported:
> 
>       http://lkml.org/lkml/2008/8/12/234
>        and
>       http://lkml.org/lkml/2008/8/12/451
> 
>   Note 3: I have already ported this to PowerPC64, but I am waiting
>     for this to be accepted first before submitting those changes.
> ]
> 
> One of the things that bothered me about the latest ftrace code was 
> this annoying daemon that would wake up once a second to see if it had 
> work to do. If it did not, it would go to sleep, otherwise it would do 
> its work and then go to sleep.

i like this concept alot - i've applied the whole lot to 
tip/tracing/ftrace.

Eventually gcc should be extended to provide a separate section for 
instrumentation patch sites, instead of us having to disassemble the 
object code. That would also make the ftrace build faster. Any gcc folks 
interested in that?

	Ingo

Re: [PATCH v2 0/6] ftrace: to kill a daemon (small updates)

[Gregory Haskins]

[-- Attachment #1: Type: text/plain, Size: 1307 bytes --]

(From the original mail)

Steven Rostedt wrote:
>
> Along came Gregory Haskins, who was bickering about having ftrace enabled
> on a production -rt kernel. I told him the reasons that this would be bad
> and then he started thinking out loud, and suggesting wild ideas, like
> patching gcc!
>   
^^^^^^^^^^

Ingo Molnar wrote:
> Eventually gcc should be extended to provide a separate section for 
> instrumentation patch sites, instead of us having to disassemble the 
> object code. 

:)

I obviously agree with this, so +1

Though, tbh, at the time I suggested it I didn't think of Steve's idea 
to post-process which was quite clever. But I do agree that having gcc 
do it will probably save some build time since it will probably be 
trivial for it to do this when already processing -pg. It would have the 
added benefit of letting the arch specific toolchain do the arch 
specific work (though I think Steve's solution capitalizes on the 
toolchain extensively as it is).

The biggest downside is that we would have an external dependency on gcc 
for the feature, but I guess the kernel already has some of those anyway 
(e.g. the stack overflow guard feature, etc). We could always fall back 
on Steve's post-processing if the toolchain lacks the feature.

-Greg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [PATCH v2 0/6] ftrace: to kill a daemon (small updates)

[Marcin Slusarz]

On Thu, Aug 14, 2008 at 03:45:06PM -0400, Steven Rostedt wrote:
>   Note 2: I have found that the changes here are more stable than
>     the current daemon method, and these patches should be used.
>     It also solves the resume from suspend to ram bug that was
>     reported:
> 
>       http://lkml.org/lkml/2008/8/12/234
>        and
>       http://lkml.org/lkml/2008/8/12/451

I don't remember testing any patch for my bug... ;)

For which tree this patchset is?
I tried to apply it on rc3, rc3+tip/tip and rc3+tip/master and all failed.

Marcin

Re: [PATCH v2 0/6] ftrace: to kill a daemon (small updates)

[Steven Rostedt]

On Fri, 15 Aug 2008, Marcin Slusarz wrote:

> On Thu, Aug 14, 2008 at 03:45:06PM -0400, Steven Rostedt wrote:
> >   Note 2: I have found that the changes here are more stable than
> >     the current daemon method, and these patches should be used.
> >     It also solves the resume from suspend to ram bug that was
> >     reported:
> > 
> >       http://lkml.org/lkml/2008/8/12/234
> >        and
> >       http://lkml.org/lkml/2008/8/12/451
> 
> I don't remember testing any patch for my bug... ;)
> 
> For which tree this patchset is?
> I tried to apply it on rc3, rc3+tip/tip and rc3+tip/master and all failed.
> 

Ah, Bruce reported the same error, and said this patch series fixed it. 
I assumed that this would fix your bug too. But if it does not, then I
need to look deeper into what you are reporting.

Could you recheck-out the latest tip/master and see if it works for you.

Thanks,

-- Steve

Re: [PATCH v2 0/6] ftrace: to kill a daemon (small updates)

[Marcin Slusarz]

On Fri, Aug 15, 2008 at 08:50:57AM -0400, Steven Rostedt wrote:
> 
> On Fri, 15 Aug 2008, Marcin Slusarz wrote:
> 
> > On Thu, Aug 14, 2008 at 03:45:06PM -0400, Steven Rostedt wrote:
> > >   Note 2: I have found that the changes here are more stable than
> > >     the current daemon method, and these patches should be used.
> > >     It also solves the resume from suspend to ram bug that was
> > >     reported:
> > > 
> > >       http://lkml.org/lkml/2008/8/12/234
> > >        and
> > >       http://lkml.org/lkml/2008/8/12/451
> > 
> > I don't remember testing any patch for my bug... ;)
> > 
> > For which tree this patchset is?
> > I tried to apply it on rc3, rc3+tip/tip and rc3+tip/master and all failed.
> > 
> 
> Ah, Bruce reported the same error, and said this patch series fixed it. 
> I assumed that this would fix your bug too. But if it does not, then I
> need to look deeper into what you are reporting.
> 
> Could you recheck-out the latest tip/master and see if it works for you.

Ok, I tried tip/master and resume from suspend to ram works again.
Thanks!

Marcin

Re: [PATCH v2 0/6] ftrace: to kill a daemon (small updates)

[Marcin Slusarz]

On Fri, Aug 15, 2008 at 03:35:22PM +0200, Marcin Slusarz wrote:
> On Fri, Aug 15, 2008 at 08:50:57AM -0400, Steven Rostedt wrote:
> > 
> > On Fri, 15 Aug 2008, Marcin Slusarz wrote:
> > 
> > > On Thu, Aug 14, 2008 at 03:45:06PM -0400, Steven Rostedt wrote:
> > > >   Note 2: I have found that the changes here are more stable than
> > > >     the current daemon method, and these patches should be used.
> > > >     It also solves the resume from suspend to ram bug that was
> > > >     reported:
> > > > 
> > > >       http://lkml.org/lkml/2008/8/12/234
> > > >        and
> > > >       http://lkml.org/lkml/2008/8/12/451
> > > 
> > > I don't remember testing any patch for my bug... ;)
> > > 
> > > For which tree this patchset is?
> > > I tried to apply it on rc3, rc3+tip/tip and rc3+tip/master and all failed.
> > > 
> > 
> > Ah, Bruce reported the same error, and said this patch series fixed it. 
> > I assumed that this would fix your bug too. But if it does not, then I
> > need to look deeper into what you are reporting.
> > 
> > Could you recheck-out the latest tip/master and see if it works for you.
> 
> Ok, I tried tip/master and resume from suspend to ram works again.
> Thanks!

I assume this patchset won't go into .27. So is there any plan to fix
it in .27?

Marcin

Re: [PATCH v2 0/6] ftrace: to kill a daemon (small updates)

[Steven Rostedt]

On Wed, 20 Aug 2008, Marcin Slusarz wrote:

> On Fri, Aug 15, 2008 at 03:35:22PM +0200, Marcin Slusarz wrote:
> > On Fri, Aug 15, 2008 at 08:50:57AM -0400, Steven Rostedt wrote:
> > > 
> > > On Fri, 15 Aug 2008, Marcin Slusarz wrote:
> > > 
> > > > On Thu, Aug 14, 2008 at 03:45:06PM -0400, Steven Rostedt wrote:
> > > > >   Note 2: I have found that the changes here are more stable than
> > > > >     the current daemon method, and these patches should be used.
> > > > >     It also solves the resume from suspend to ram bug that was
> > > > >     reported:
> > > > > 
> > > > >       http://lkml.org/lkml/2008/8/12/234
> > > > >        and
> > > > >       http://lkml.org/lkml/2008/8/12/451
> > > > 
> > > > I don't remember testing any patch for my bug... ;)
> > > > 
> > > > For which tree this patchset is?
> > > > I tried to apply it on rc3, rc3+tip/tip and rc3+tip/master and all failed.
> > > > 
> > > 
> > > Ah, Bruce reported the same error, and said this patch series fixed it. 
> > > I assumed that this would fix your bug too. But if it does not, then I
> > > need to look deeper into what you are reporting.
> > > 
> > > Could you recheck-out the latest tip/master and see if it works for you.
> > 
> > Ok, I tried tip/master and resume from suspend to ram works again.
> > Thanks!
> 
> I assume this patchset won't go into .27. So is there any plan to fix
> it in .27?

I'm hoping that this does go into 27, since it is more reliable than the 
current daemon approach.

-- Steve

Re: [PATCH v2 0/6] ftrace: to kill a daemon (small updates)

[Ingo Molnar]

* Steven Rostedt <rostedt@goodmis.org> wrote:

> > I assume this patchset won't go into .27. So is there any plan to 
> > fix it in .27?
> 
> I'm hoping that this does go into 27, since it is more reliable than 
> the current daemon approach.

it cannot really go into .27 - there's a lot of ftrace stuff piled up in 
tip/tracing/ftrace. (the script is also pretty hacky and unproven. All 
this stuff was written after the merge window.)

	Ingo