Skip to contentSkip to conversation

powerpc: port of dynamic ftrace

PATCH In review Steven Rostedt 2 participants 14 messages Nov 19, 2008 – Nov 20, 2008

Subsystems (4)
lore
Share & tools
lore

Steven Rostedt

author maintainer
#0 lore raw

cover powerpc: port of dynamic ftrace

Paul,

Here are the patches that include the changes suggested by both you and Milton. This series includes the back port of three commits from tip that are needed for the PowerPC port.

I also made a git branch called "ppc/ftrace-disable" that does not include two of the three commits. It adds a patch to keep dynamic ftrace from being enabled by PowerPC architectures.

As I stated above, both branches include one commit from tip:

ftrace: align __mcount_loc sections

This is because one of the PowerPC patches will not apply without it. That commit was a clean cherry pick into mainline, so I'm not worried about it. Still, the only commits that should go to mainline from the PowerPC git repo are the ones that start with "powerpc".

In the ppc/ftrace-hack branch, I folded the other two commits from tip, that are needed for the port, into a single commit. This commit is called:

NOT FOR MAINLINE ftrace: pass module struct to arch dynamic ftrace functions

This is only to let you test the rest of the patches. I've booted this branch and ran it on both my PPC64 and my PP32 boxes.

Again:

The working branch is: ppc/ftrace-hack

The disabling dynamic ftrace branch is: ppc/ftrace-disable

Only the commits starting with "powerpc" should be pushed to mainline by you. Those patches are the same in both of the above branches.

Also, I have not tested that "NOT FOR MAINLINE" patch on x86. It may break that arch. Which is another reason not to push it.

The following patches are in:

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

branch: ppc/ftrace-hack

Matt Fleming (1):
ftrace: align __mcount_loc sections

Steven Rostedt (8):
NOT FOR MAINLINE ftrace: pass module struct to arch dynamic ftrace functions
powerpc: ftrace, do not latency trace idle
powerpc: ftrace, convert to new dynamic ftrace arch API powerpc/ppc64: ftrace, mcount record powerpc port
powerpc: ftrace, use probe_kernel API to modify code powerpc/ppc64: ftrace, handle module trampolines for dyn ftrace powerpc/ppc32: ftrace, enabled dynamic ftrace
powerpc/ppc32: ftrace, dynamic ftrace to handle modules

----

 arch/powerpc/Kconfig              |    2 +
 arch/powerpc/include/asm/ftrace.h |   14 +-
 arch/powerpc/include/asm/module.h |   16 ++-
 arch/powerpc/kernel/ftrace.c      |  473 +++++++++++++++++++++++++++++++++---
 arch/powerpc/kernel/idle.c        |    5 +
 arch/powerpc/kernel/module_32.c   |   10 +
 arch/powerpc/kernel/module_64.c   |   13 +
 arch/x86/include/asm/ftrace.h     |    9 +-
 arch/x86/kernel/ftrace.c          |  168 +++++++++++++-
 include/linux/ftrace.h            |   51 ++++-
 kernel/module.c                   |    2 +-
 kernel/trace/ftrace.c             |  137 ++++++------
 scripts/recordmcount.pl           |   20 ++-
 13 files changed, 790 insertions(+), 130 deletions(-)
author maintainer
#1 lore raw

PATCH 1/9 ftrace: align __mcount_loc sections

Impact: add alignment option for recordmcount.pl script

Align the __mcount_loc sections so that architectures with strict alignment requirements need not worry about performing unaligned accesses.

This fixes an issue where I was seeing unaligned accesses, which are not supported on our architecture (the results of an unaligned access are undefined).

---
 scripts/recordmcount.pl |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
scripts/recordmcount.pl +4 -0
index 6b9fe3e..eeac71c 100755
@@ -134,6 +134,7 @@ my $section_regex;	# Find the start of a section
 my $function_regex;	# Find the name of a function
 			#    (return offset and func name)
 my $mcount_regex;	# Find the call site to mcount (return offset)
+my $alignment;         # The .align value to use for $mcount_section
 
 if ($arch eq "x86") {
     if ($bits == 64) {
@@ -148,6 +149,7 @@ if ($arch eq "x86_64") {
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$";
     $type = ".quad";
+    $alignment = 8;
 
     # force flags for this arch
     $ld .= " -m elf_x86_64";
@@ -160,6 +162,7 @@ if ($arch eq "x86_64") {
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
     $type = ".long";
+    $alignment = 4;
 
     # force flags for this arch
     $ld .= " -m elf_i386";
@@ -288,6 +291,7 @@ sub update_funcs
 	    open(FILE, ">$mcount_s") || die "can't create $mcount_s\n";
 	    $opened = 1;
 	    print FILE "\t.section $mcount_section,\"a\",\@progbits\n";
+	    print FILE "\t.align $alignment\n";
 	}
 	printf FILE "\t%s %s + %d\n", $type, $ref_func, $offsets[$i] - $offset;
     }
Signature · 3 lines
1.5.6.5

-- 
author maintainer
#2 lore raw

Re: [PATCH 1/9] ftrace: align __mcount_loc sections

I need to change my scripts to parse out the first line of all patches, so quilt can send out the proper owner. This patch has the following header:

Quoted 1 line

From 626f82959cd00ca804b12543cad86714e74264da Mon Sep 17 00:00:00 2001

From: Matt Fleming <[email protected]>
Date: Fri, 7 Nov 2008 13:26:25 +0000
Subject: [PATCH] ftrace: align __mcount_loc sections

If you pull from my repo, it will all work out. But still, I do not want to take credit for someone else's work.

-- Steve

author maintainer
#3 lore raw

PATCH 2/9 NOT FOR MAINLINE ftrace: pass module struct to arch dynamic ftrace functions

In reply to Steven Rostedt

Impact: allow archs more flexibility on dynamic ftrace implementations

Dynamic ftrace has largly been developed on x86. Since x86 does not have the same limitations as other architectures, the ftrace interaction between the generic code and the architecture specific code was not flexible enough to handle some of the issues that other architectures have.

Most notably, module trampolines. Due to the limited branch distance that archs make in calling kernel core code from modules, the module load code must create a trampoline to jump to what will make the larger jump into core kernel code.

The problem arises when this happens to a call to mcount. Ftrace checks all code before modifying it and makes sure the current code is what it expects. Right now, there is not enough information to handle modifying module trampolines.

This patch changes the API between generic dynamic ftrace code and the arch dependent code. There is now two functions for modifying code:

ftrace_make_nop(mod, rec, addr) - convert the code at rec->ip into
a nop, where the original text is calling addr. (mod is the module struct if called by module init)

ftrace_make_caller(rec, addr) - convert the code rec->ip that should
be a nop into a caller to addr.

The record "rec" now has a new field called "arch" where the architecture can add any special attributes to each call site record.

Conflicts:

arch/x86/include/asm/ftrace.h
arch/x86/kernel/ftrace.c
include/linux/ftrace.h
kernel/trace/ftrace.c

ftrace: allow NULL pointers in mcount_loc

Impact: let archs insert NULL pointers in mcount_loc section

Due to the way different architecture linkers combine the data sections of the mcount_loc (the section that lists all the locations that call mcount), there may be zeros added in that section. This is usually due to strange alignments that the linker performs, that pads in zeros.

This patch makes the conversion code to nops skip any pointer in the mcount_loc section that is NULL.

---
 arch/x86/include/asm/ftrace.h |    9 ++-
 arch/x86/kernel/ftrace.c      |  168 +++++++++++++++++++++++++++++++++++++++--
 include/linux/ftrace.h        |   51 ++++++++++---
 kernel/module.c               |    2 +-
 kernel/trace/ftrace.c         |  137 +++++++++++++++++-----------------
 5 files changed, 280 insertions(+), 87 deletions(-)
arch/x86/include/asm/ftrace.h +8 -1
index 9e8bc29..1d8d16f 100644
@@ -17,8 +17,15 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
 	 */
 	return addr - 1;
 }
-#endif
 
+#ifdef CONFIG_DYNAMIC_FTRACE
+
+struct dyn_arch_ftrace {
+	/* No extra data needed for x86 */
+};
+
+#endif /*  CONFIG_DYNAMIC_FTRACE */
+#endif /* __ASSEMBLY__ */
 #endif /* CONFIG_FUNCTION_TRACER */
 
 #endif /* _ASM_X86_FTRACE_H */
arch/x86/kernel/ftrace.c +161 -7

186 lines not on this page · raw message

include/linux/ftrace.h +41 -10

96 lines not on this page · raw message

kernel/module.c +1 -1
index 1f4cc00..6979127 100644
@@ -2201,7 +2201,7 @@ static noinline struct module *load_module(void __user *umod,
 	/* sechdrs[0].sh_size is always zero */
 	mseg = section_objs(hdr, sechdrs, secstrings, "__mcount_loc",
 			    sizeof(*mseg), &num_mcount);
-	ftrace_init_module(mseg, mseg + num_mcount);
+	ftrace_init_module(mod, mseg, mseg + num_mcount);
 
 	err = module_finalize(hdr, sechdrs, mod);
 	if (err < 0)
kernel/trace/ftrace.c +69 -68

242 lines not on this page · raw message

Signature · 3 lines
1.5.6.5

-- 
author maintainer
#4 lore raw

PATCH 3/9 powerpc: ftrace, do not latency trace idle

In reply to Steven Rostedt

Impact: fix for irq off latency tracer

When idle is called, interrupts are disabled, but the idle function will still wake up on an interrupt. The problem is that the interrupt disabled latency tracer will take this call to idle as a latency.

This patch disables the latency tracing when going into idle.

---
 arch/powerpc/kernel/idle.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)
arch/powerpc/kernel/idle.c +5 -0
index 31982d0..88d9c1d 100644
@@ -69,10 +69,15 @@ void cpu_idle(void)
 				smp_mb();
 				local_irq_disable();
 
+				/* Don't trace irqs off for idle */
+				stop_critical_timings();
+
 				/* check again after disabling irqs */
 				if (!need_resched() && !cpu_should_die())
 					ppc_md.power_save();
 
+				start_critical_timings();
+
 				local_irq_enable();
 				set_thread_flag(TIF_POLLING_NRFLAG);
 
Signature · 3 lines
1.5.6.5

-- 
author maintainer
#5 lore raw

PATCH 4/9 powerpc: ftrace, convert to new dynamic ftrace arch API

In reply to Steven Rostedt

Impact: update to PowerPC ftrace arch API

This patch converts PowerPC to use the new dynamic ftrace arch API.

Thanks to Paul Mackennas for pointing out the mistakes of my original test_24bit_addr function.

---
 arch/powerpc/include/asm/ftrace.h |   14 +++++++-
 arch/powerpc/kernel/ftrace.c      |   67 ++++++++++++++++++++++++++++++++++---
 2 files changed, 75 insertions(+), 6 deletions(-)
arch/powerpc/include/asm/ftrace.h +13 -1
index b298f7a..17efecc 100644
@@ -7,7 +7,19 @@
 
 #ifndef __ASSEMBLY__
 extern void _mcount(void);
-#endif
+
+#ifdef CONFIG_DYNAMIC_FTRACE
+static inline unsigned long ftrace_call_adjust(unsigned long addr)
+{
+       /* reloction of mcount call site is the same as the address */
+       return addr;
+}
+
+struct dyn_arch_ftrace {
+	/* nothing yet */
+};
+#endif /*  CONFIG_DYNAMIC_FTRACE */
+#endif /* __ASSEMBLY__ */
 
 #endif
 
arch/powerpc/kernel/ftrace.c +62 -5

96 lines not on this page · raw message

Signature · 3 lines
1.5.6.5

-- 
author maintainer
#6 lore raw

PATCH 5/9 powerpc/ppc64: ftrace, mcount record powerpc port

In reply to Steven Rostedt

Impact: 64 bit PowerPC port of dynamic ftrace

This patch converts 64 bit PowerPC to use the mcount location section.

Currently, modules will be ignored by the converter.

---
 arch/powerpc/Kconfig    |    2 ++
 scripts/recordmcount.pl |   13 +++++++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)
arch/powerpc/Kconfig +2 -0
index 525c13a..9675e95 100644
@@ -108,6 +108,8 @@ config ARCH_NO_VIRT_TO_BUS
 config PPC
 	bool
 	default y
+	select HAVE_FTRACE_MCOUNT_RECORD if PPC64
+	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FUNCTION_TRACER
 	select ARCH_WANT_OPTIONAL_GPIOLIB
 	select HAVE_IDE
scripts/recordmcount.pl +11 -2
index eeac71c..7acbe17 100755
@@ -130,6 +130,7 @@ my %weak;		# List of weak functions
 my %convert;		# List of local functions used that needs conversion
 
 my $type;
+my $nm_regex;		# Find the local functions (return function)
 my $section_regex;	# Find the start of a section
 my $function_regex;	# Find the name of a function
 			#    (return offset and func name)
@@ -145,6 +146,7 @@ if ($arch eq "x86") {
 }
 
 if ($arch eq "x86_64") {
+    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
     $section_regex = "Disassembly of section\\s+(\\S+):";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount([+-]0x[0-9a-zA-Z]+)?\$";
@@ -158,6 +160,7 @@ if ($arch eq "x86_64") {
     $cc .= " -m64";
 
 } elsif ($arch eq "i386") {
+    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
     $section_regex = "Disassembly of section\\s+(\\S+):";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
@@ -170,6 +173,12 @@ if ($arch eq "x86_64") {
     $objcopy .= " -O elf32-i386";
     $cc .= " -m32";
 
+} elsif ($arch eq "powerpc") {
+    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\.?\\S+)";
+    $section_regex = "Disassembly of section\\s+(\\S+):";
+    $function_regex = "^([0-9a-fA-F]+)\\s+<(\\.?.*?)>:";
+    $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s\\.?_mcount\$";
+    $type = ".quad";
 } else {
     die "Arch $arch is not supported with CONFIG_FTRACE_MCOUNT_RECORD";
 }
@@ -239,7 +248,7 @@ if (!$found_version) {
 #
 open (IN, "$nm $inputfile|") || die "error running $nm";
 while (<IN>) {
-    if (/^[0-9a-fA-F]+\s+t\s+(\S+)/) {
+    if (/$nm_regex/) {
 	$locals{$1} = 1;
     } elsif (/^[0-9a-fA-F]+\s+([wW])\s+(\S+)/) {
 	$weak{$2} = $1;
@@ -291,7 +300,7 @@ sub update_funcs
 	    open(FILE, ">$mcount_s") || die "can't create $mcount_s\n";
 	    $opened = 1;
 	    print FILE "\t.section $mcount_section,\"a\",\@progbits\n";
-	    print FILE "\t.align $alignment\n";
+	    print FILE "\t.align $alignment\n" if (defined($alignment));
 	}
 	printf FILE "\t%s %s + %d\n", $type, $ref_func, $offsets[$i] - $offset;
     }
Signature · 3 lines
1.5.6.5

-- 
author maintainer
#7 lore raw

PATCH 6/9 powerpc: ftrace, use probe_kernel API to modify code

In reply to Steven Rostedt

Impact: use cleaner probe_kernel API over assembly

Using probe_kernel_read/write interface is a much cleaner approach than the current assembly version.

---
 arch/powerpc/kernel/ftrace.c |   53 ++++++++++++++++-------------------------
 1 files changed, 21 insertions(+), 32 deletions(-)
arch/powerpc/kernel/ftrace.c +21 -32
index 24c023a..1adfbb2 100644
@@ -9,6 +9,7 @@
 
 #include <linux/spinlock.h>
 #include <linux/hardirq.h>
+#include <linux/uaccess.h>
 #include <linux/ftrace.h>
 #include <linux/percpu.h>
 #include <linux/init.h>
@@ -72,45 +73,33 @@ static int
 ftrace_modify_code(unsigned long ip, unsigned char *old_code,
 		   unsigned char *new_code)
 {
-	unsigned replaced;
-	unsigned old = *(unsigned *)old_code;
-	unsigned new = *(unsigned *)new_code;
-	int faulted = 0;
+	unsigned char replaced[MCOUNT_INSN_SIZE];
 
 	/*
 	 * 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
+	 *  probe_kernel_* functions.
 	 *
 	 * No real locking needed, this code is run through
-	 * kstop_machine.
+	 * kstop_machine, or before SMP starts.
 	 */
-	asm volatile (
-		"1: lwz		%1, 0(%2)\n"
-		"   cmpw	%1, %5\n"
-		"   bne		2f\n"
-		"   stwu	%3, 0(%2)\n"
-		"2:\n"
-		".section .fixup, \"ax\"\n"
-		"3:	li %0, 1\n"
-		"	b 2b\n"
-		".previous\n"
-		".section __ex_table,\"a\"\n"
-		_ASM_ALIGN "\n"
-		_ASM_PTR "1b, 3b\n"
-		".previous"
-		: "=r"(faulted), "=r"(replaced)
-		: "r"(ip), "r"(new),
-		  "0"(faulted), "r"(old)
-		: "memory");
-
-	if (replaced != old && replaced != new)
-		faulted = 2;
-
-	if (!faulted)
-		flush_icache_range(ip, ip + 8);
-
-	return faulted;
+
+	/* read the text we want to modify */
+	if (probe_kernel_read(replaced, (void *)ip, MCOUNT_INSN_SIZE))
+		return -EFAULT;
+
+	/* Make sure it is what we expect it to be */
+	if (memcmp(replaced, old_code, MCOUNT_INSN_SIZE) != 0)
+		return -EINVAL;
+
+	/* replace the text with the new text */
+	if (probe_kernel_write((void *)ip, new_code, MCOUNT_INSN_SIZE))
+		return -EPERM;
+
+	flush_icache_range(ip, ip + 8);
+
+	return 0;
 }
 
 static int test_24bit_addr(unsigned long ip, unsigned long addr)
Signature · 3 lines
1.5.6.5

-- 
author maintainer
#8 lore raw

PATCH 7/9 powerpc/ppc64: ftrace, handle module trampolines for dyn ftrace

In reply to Steven Rostedt

Impact: Allow 64 bit PowerPC to trace modules with dynamic ftrace

This adds code to handle the PPC64 module trampolines, and allows for PPC64 to use dynamic ftrace.

Thanks to Paul Mackerras for these updates:

- fix the mod and rec->arch.mod NULL checks.
- fix to is_bl_op compare.

Thanks to Milton Miller for:

- finding the nasty race with using two nops, and recommending instead that I use a branch 8 forward.

---
 arch/powerpc/include/asm/ftrace.h |    2 +-
 arch/powerpc/include/asm/module.h |   11 ++
 arch/powerpc/kernel/ftrace.c      |  278 +++++++++++++++++++++++++++++++++++--
 arch/powerpc/kernel/module_64.c   |   13 ++
 4 files changed, 293 insertions(+), 11 deletions(-)
arch/powerpc/include/asm/ftrace.h +1 -1
index 17efecc..e5f2ae8 100644
@@ -16,7 +16,7 @@ static inline unsigned long ftrace_call_adjust(unsigned long addr)
 }
 
 struct dyn_arch_ftrace {
-	/* nothing yet */
+	struct module *mod;
 };
 #endif /*  CONFIG_DYNAMIC_FTRACE */
 #endif /* __ASSEMBLY__ */
arch/powerpc/include/asm/module.h +11 -0
index e5f14b1..340bc69 100644
@@ -34,6 +34,11 @@ struct mod_arch_specific {
 #ifdef __powerpc64__
 	unsigned int stubs_section;	/* Index of stubs section in module */
 	unsigned int toc_section;	/* What section is the TOC? */
+#ifdef CONFIG_DYNAMIC_FTRACE
+	unsigned long toc;
+	unsigned long tramp;
+#endif
+
 #else
 	/* Indices of PLT sections within module. */
 	unsigned int core_plt_section;
@@ -68,6 +73,12 @@ struct mod_arch_specific {
 #    endif	/* MODULE */
 #endif
 
+#ifdef CONFIG_DYNAMIC_FTRACE
+#    ifdef MODULE
+	asm(".section .ftrace.tramp,\"ax\",@nobits; .align 3; .previous");
+#    endif	/* MODULE */
+#endif
+
 
 struct exception_table_entry;
 void sort_ex_table(struct exception_table_entry *start,
arch/powerpc/kernel/ftrace.c +268 -10

343 lines not on this page · raw message

arch/powerpc/kernel/module_64.c +13 -0
index 1af2377..8992b03 100644
@@ -20,6 +20,7 @@
 #include <linux/moduleloader.h>
 #include <linux/err.h>
 #include <linux/vmalloc.h>
+#include <linux/ftrace.h>
 #include <linux/bug.h>
 #include <asm/module.h>
 #include <asm/firmware.h>
@@ -163,6 +164,11 @@ static unsigned long get_stubs_size(const Elf64_Ehdr *hdr,
 		}
 	}
 
+#ifdef CONFIG_DYNAMIC_FTRACE
+	/* make the trampoline to the ftrace_caller */
+	relocs++;
+#endif
+
 	DEBUGP("Looks like a total of %lu stubs, max\n", relocs);
 	return relocs * sizeof(struct ppc64_stub_entry);
 }
@@ -441,5 +447,12 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 		}
 	}
 
+#ifdef CONFIG_DYNAMIC_FTRACE
+	me->arch.toc = my_r2(sechdrs, me);
+	me->arch.tramp = stub_for_addr(sechdrs,
+				       (unsigned long)ftrace_caller,
+				       me);
+#endif
+
 	return 0;
 }
Signature · 3 lines
1.5.6.5

-- 
author maintainer
#9 lore raw

PATCH 8/9 powerpc/ppc32: ftrace, enabled dynamic ftrace

In reply to Steven Rostedt

Impact: Port 32 bit PowerPC dynamic ftrace

This patch adds the necessary hooks to get PPC32 dynamic ftrace working. It does not handle modules. They are ignored by this patch.

---
 arch/powerpc/Kconfig    |    2 +-
 scripts/recordmcount.pl |    7 ++++++-
 2 files changed, 7 insertions(+), 2 deletions(-)
arch/powerpc/Kconfig +1 -1
index 9675e95..d64b629 100644
@@ -108,7 +108,7 @@ config ARCH_NO_VIRT_TO_BUS
 config PPC
 	bool
 	default y
-	select HAVE_FTRACE_MCOUNT_RECORD if PPC64
+	select HAVE_FTRACE_MCOUNT_RECORD
 	select HAVE_DYNAMIC_FTRACE
 	select HAVE_FUNCTION_TRACER
 	select ARCH_WANT_OPTIONAL_GPIOLIB
scripts/recordmcount.pl +6 -1
index 7acbe17..48609e9 100755
@@ -178,7 +178,12 @@ if ($arch eq "x86_64") {
     $section_regex = "Disassembly of section\\s+(\\S+):";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(\\.?.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s\\.?_mcount\$";
-    $type = ".quad";
+    if ($bits == 64) {
+	$type = ".quad";
+    } else {
+	$type = ".long";
+    }
+
 } else {
     die "Arch $arch is not supported with CONFIG_FTRACE_MCOUNT_RECORD";
 }
Signature · 3 lines
1.5.6.5

-- 
author maintainer
#10 lore raw

PATCH 9/9 powerpc/ppc32: ftrace, dynamic ftrace to handle modules

In reply to Steven Rostedt

Impact: add ability to trace modules on 32 bit PowerPC

This patch performs the necessary trampoline calls to handle modules with dynamic ftrace on 32 bit PowerPC.

---
 arch/powerpc/include/asm/module.h |    5 ++-
 arch/powerpc/kernel/ftrace.c      |  101 ++++++++++++++++++++++++++++++++++--
 arch/powerpc/kernel/module_32.c   |   10 ++++
 3 files changed, 109 insertions(+), 7 deletions(-)
arch/powerpc/include/asm/module.h +4 -1
index 340bc69..0845488 100644
@@ -39,11 +39,14 @@ struct mod_arch_specific {
 	unsigned long tramp;
 #endif
 
-#else
+#else /* powerpc64 */
 	/* Indices of PLT sections within module. */
 	unsigned int core_plt_section;
 	unsigned int init_plt_section;
+#ifdef CONFIG_DYNAMIC_FTRACE
+	unsigned long tramp;
 #endif
+#endif /* powerpc64 */
 
 	/* List of BUG addresses, source line numbers and filenames */
 	struct list_head bug_list;
arch/powerpc/kernel/ftrace.c +95 -6

137 lines not on this page · raw message

arch/powerpc/kernel/module_32.c +10 -0
index 2df91a0..f832773 100644
@@ -22,6 +22,7 @@
 #include <linux/fs.h>
 #include <linux/string.h>
 #include <linux/kernel.h>
+#include <linux/ftrace.h>
 #include <linux/cache.h>
 #include <linux/bug.h>
 #include <linux/sort.h>
@@ -53,6 +54,9 @@ static unsigned int count_relocs(const Elf32_Rela *rela, unsigned int num)
 			r_addend = rela[i].r_addend;
 		}
 
+#ifdef CONFIG_DYNAMIC_FTRACE
+	_count_relocs++;	/* add one for ftrace_caller */
+#endif
 	return _count_relocs;
 }
 
@@ -306,5 +310,11 @@ int apply_relocate_add(Elf32_Shdr *sechdrs,
 			return -ENOEXEC;
 		}
 	}
+#ifdef CONFIG_DYNAMIC_FTRACE
+	module->arch.tramp =
+		do_plt_call(module->module_core,
+			    (unsigned long)ftrace_caller,
+			    sechdrs, module);
+#endif
 	return 0;
 }
Signature · 3 lines
1.5.6.5

-- 
#11 lore raw
In reply to Steven Rostedt
Quoted 15 lines · Steven Rostedt

----

 arch/powerpc/Kconfig              |    2 +
 arch/powerpc/include/asm/ftrace.h |   14 +-
 arch/powerpc/include/asm/module.h |   16 ++-
 arch/powerpc/kernel/ftrace.c      |  473 +++++++++++++++++++++++++++++++++---
 arch/powerpc/kernel/idle.c        |    5 +
 arch/powerpc/kernel/module_32.c   |   10 +
 arch/powerpc/kernel/module_64.c   |   13 +
 arch/x86/include/asm/ftrace.h     |    9 +-
 arch/x86/kernel/ftrace.c          |  168 +++++++++++++-
 include/linux/ftrace.h            |   51 ++++-
 kernel/module.c                   |    2 +-
 kernel/trace/ftrace.c             |  137 ++++++------
 scripts/recordmcount.pl           |   20 ++-
 13 files changed, 790 insertions(+), 130 deletions(-)

Hm, something like this shouldnt be pulled into the powerpc tree: it touches the core kernel, x86 code and ftrace code as well.

Please do the suggestion i outlined and which Paul agreed with:
prepare a branch that touches _only_ powerpc files and gets these changes there, without actually breaking the build on powerpc. And please do not name the branch as "hack" - we dont want Paul to pull such a branch name - it will show up in the upstream git logs.

Those changes should only touch powerpc files. Do not try to shoe-horn already applied ftrace commits into a separate branch with different sha1's. Yes, ftrace wont be enable-able on powerpc when that is pulled, but it will only be for a brief period shortly before the merge window. (and it will all just work fine when integrated together)

Once this branch is done, and once Paul agrees that it looks OK-ish to him, we can put it into ftrace-next straight away [but still keep it in a separate topic tree in tip/tracing/powerpc - so it can all be reconsidered reversibly if it causes too much merge trouble]. Paul will then be able to pull it in a few weeks, in the runup to the v2.6.29 merge window.

The other option is to go the slow route of 2-3 kernel releases to pull this all off.

ok?

Ingo

author maintainer
#12 lore raw
Quoted 10 lines · Ingo Molnar

Hm, something like this shouldnt be pulled into the powerpc tree: it touches the core kernel, x86 code and ftrace code as well.

Please do the suggestion i outlined and which Paul agreed with:
prepare a branch that touches _only_ powerpc files and gets these changes there, without actually breaking the build on powerpc. And please do not name the branch as "hack" - we dont want Paul to pull such a branch name - it will show up in the upstream git logs.

Quoted 6 lines

Those changes should only touch powerpc files. Do not try to shoe-horn already applied ftrace commits into a separate branch with different sha1's. Yes, ftrace wont be enable-able on powerpc when that is pulled, but it will only be for a brief period shortly before the merge window. (and it will all just work fine when integrated together)

As stated in the original post, I also had a separate branch called ppc/ftrace-disable. Here's the generate posting if I were to send it:

The following patches are in:

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

branch: ppc/ftrace-disable

Matt Fleming (1):
ftrace: align __mcount_loc sections

Steven Rostedt (8):
ftrace: disable dynamic ftrace from PowerPC
powerpc: ftrace, do not latency trace idle
powerpc: ftrace, convert to new dynamic ftrace arch API powerpc/ppc64: ftrace, mcount record powerpc port
powerpc: ftrace, use probe_kernel API to modify code powerpc/ppc64: ftrace, handle module trampolines for dyn ftrace powerpc/ppc32: ftrace, enabled dynamic ftrace powerpc/ppc32: ftrace, dynamic ftrace to handle modules

----

 arch/powerpc/Kconfig              |    2 +
 arch/powerpc/include/asm/ftrace.h |   14 +-
 arch/powerpc/include/asm/module.h |   16 ++-
 arch/powerpc/kernel/ftrace.c      |  473 +++++++++++++++++++++++++++++++++---
 arch/powerpc/kernel/idle.c        |    5 +
 arch/powerpc/kernel/module_32.c   |   10 +
 arch/powerpc/kernel/module_64.c   |   13 +
 kernel/trace/Kconfig              |    1 +
 scripts/recordmcount.pl           |   20 ++-
 9 files changed, 511 insertions(+), 43 deletions(-)

There is a dependency on Matt's commit for one of the PPC commits to apply. I could hack that commit to work without Matt's patch, but then that just postpones conflicts later on.

Matt's change is this:

 scripts/recordmcount.pl           |   20 ++-

The commit to disable PPC is this:

kernel/trace/Kconfig +1 -0
index 33dbefd..d9127f4 100644
@@ -162,6 +162,7 @@ config DYNAMIC_FTRACE
        depends on FUNCTION_TRACER
        depends on HAVE_DYNAMIC_FTRACE
        depends on DEBUG_KERNEL
+       depends on !PPC
        default y
        help
          This option will modify all the calls to ftrace dynamically

This does touch generic code, but this is to handle the chicken and the egg problem. I would like the disabling of PowerPC code in a separate commit (one easy to revert). But I can not disable the PowerPC code before the code is there. If I disable it after the code is there, then there exists a time that PowerPC will not compile due to this not being disabled.

Quoted 7 lines

Once this branch is done, and once Paul agrees that it looks OK-ish to him, we can put it into ftrace-next straight away [but still keep it in a separate topic tree in tip/tracing/powerpc - so it can all be reconsidered reversibly if it causes too much merge trouble]. Paul will then be able to pull it in a few weeks, in the runup to the v2.6.29 merge window.

I guess the question comes down to, do I work around Matt's patch?

Quoted 3 lines

The other option is to go the slow route of 2-3 kernel releases to pull this all off.

2 or 3 kernel releases seems a bit extreme. But this is Paul's call.

Paul, any suggestions?

-- Steve

Quoted 6 lines

ok?

Ingo

#13 lore raw
Quoted 28 lines · Steven Rostedt

branch: ppc/ftrace-disable

Matt Fleming (1):
ftrace: align __mcount_loc sections

Steven Rostedt (8):
ftrace: disable dynamic ftrace from PowerPC
powerpc: ftrace, do not latency trace idle
powerpc: ftrace, convert to new dynamic ftrace arch API powerpc/ppc64: ftrace, mcount record powerpc port
powerpc: ftrace, use probe_kernel API to modify code powerpc/ppc64: ftrace, handle module trampolines for dyn ftrace powerpc/ppc32: ftrace, enabled dynamic ftrace powerpc/ppc32: ftrace, dynamic ftrace to handle modules

----

 arch/powerpc/Kconfig              |    2 +
 arch/powerpc/include/asm/ftrace.h |   14 +-
 arch/powerpc/include/asm/module.h |   16 ++-
 arch/powerpc/kernel/ftrace.c      |  473 +++++++++++++++++++++++++++++++++---
 arch/powerpc/kernel/idle.c        |    5 +
 arch/powerpc/kernel/module_32.c   |   10 +
 arch/powerpc/kernel/module_64.c   |   13 +
 kernel/trace/Kconfig              |    1 +
 scripts/recordmcount.pl           |   20 ++-
 9 files changed, 511 insertions(+), 43 deletions(-)
Quoted 7 lines

There is a dependency on Matt's commit for one of the PPC commits to apply. I could hack that commit to work without Matt's patch, but then that just postpones conflicts later on.

Matt's change is this:

 scripts/recordmcount.pl           |   20 ++-

looks much better and the (minimal) generic impact is OK i think - as long as Paul Acks the powerpc changes we could freeze that branch (so that Paul can pull it in a few weeks) and base your ftrace-powerpc work on that.

( The branch should still be named something constructive like
powerpc-ftrace-v29 or so - not ppc/ftrace-disable - but that's a detail. )

Ingo