tracing: fix unaligned event arrays
Subsystems (5)
Steven Rostedt
maintainerIngo,
It was discovered that the alignment of the trace event sections break sparc64. The trace events are created by linker magic, so gcc does not realize that the structures being allocated will end up in an array. Without adding any alignment, x86_64 would create the trace event arrays with holes that would break reading the events in serial. The fix for that was to force a align(4) to keep gcc and the linker from spacing the event structures at various alignments. But recent changes have caused this change to break sparc due to bad alignment in the structures, caused by the 4 byte alignment.
After some discussions, Mathieu came up with the following patch set to correctly fix the alignment issues with the trace event structures.
Please pull the latest tip/perf/urgent tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/perf/urgent
Mathieu Desnoyers (3):
Introduce __u64_aligned and U64_ALIGN() for structure alignment in custom sections
tracing: Fix sparc64 alignment crash with __u64_aligned/U64_ALIGN()
tracepoints: Use __u64_aligned/U64_ALIGN()
----
include/asm-generic/vmlinux.lds.h | 27 +++++++++++------- include/linux/align-section.h | 54 +++++++++++++++++++++++++++++++++++++ include/linux/compiler.h | 8 +++-- include/linux/ftrace_event.h | 2 +- include/linux/syscalls.h | 12 ++++---- include/linux/tracepoint.h | 12 +++----- include/trace/ftrace.h | 8 +++--- include/trace/syscall.h | 2 +- kernel/trace/trace.h | 2 +- kernel/trace/trace_export.c | 2 +- 10 files changed, 94 insertions(+), 35 deletions(-)
Steven Rostedt
maintainerPATCH 1/3 Introduce __u64_aligned and U64_ALIGN() for structure alignment in custom sections
From: Mathieu Desnoyers <[email protected]>
Problem description:
gcc happily align on 32-byte structures defined statically. Ftrace trace events and Tracepoints both statically define structures into custom sections (using the "section" attribute), to then assign these to symbols with the linker scripts to iterate these sections as an array.
However, gcc uses different alignments for these structures when they are defined statically than when they are globally visible and/or in an array. Therefore iteration on these arrays sees "holes" of padding. gcc is within its rights to increase the alignment of the statically defined structures because, normally, there should be no other accesses to them than in the local object. We are actually iterating on the generated structures as if they were an array without letting gcc know anything about it.
This patch introduces __u64_aligned to force gcc to use the u64 type and variable alignment, up-aligning or down-aligning the target type if necessary. The memory accesses to the target structure are efficient (does not require bytewise memory accesses) and the atomic pointer update guarantees required by RCU are kept. u64 is considered as the largest type that can generate a trap for unaligned accesses (u64 on sparc32 needs to be aligned on 64-bit).
This alignment should be used for both structure definitions and declarations (as *both* the type and variable attribute) when using the "section" attribute to generate arrays of structures. Given that gcc only uses the type attribute "aligned" as a lower-bound for alignment, the structures should not contain types which require alignment larger than that of u64. The "aligned" variable attribute, on the other hand, forces gcc to use exactly the specified alignment.
Also introduce the linker script U64_ALIGN() macro for specification of custom section alignment that matches that of __u64_aligned.
Changelog since v2:
- Drop the "packed" type attribute, because it causes gcc to drop the padding between consecutive "int" and "pointer"/"long" fields, which leads to unaligned accesses on sparc64.
LKML-Reference: <[email protected]>
--- include/asm-generic/vmlinux.lds.h | 6 ++++ include/linux/align-section.h | 54 +++++++++++++++++++++++++++++++++++++ include/linux/compiler.h | 2 + 3 files changed, 62 insertions(+), 0 deletions(-) create mode 100644 include/linux/align-section.h
include/asm-generic/vmlinux.lds.h +6 -0
@@ -70,6 +70,12 @@#define STRUCT_ALIGNMENT 32#define STRUCT_ALIGN () . = ALIGN(STRUCT_ALIGNMENT) +/* +* Align to a 8 byte boundary. For use with custom section made from structures +* declared and defined with __u64_aligned. +*/ +#define U64_ALIGN () . = ALIGN(8) +/* The actual configuration determine if the init/exit sections * are handled as text/data or they can be discarded (which * often happens at runtime)
include/linux/align-section.h new file +54 -0
54 lines not on this page · raw message
include/linux/compiler.h +2 -0
@@ -57,6 +57,8 @@ extern void __chk_io_ptr(const volatile void __iomem *);# include <linux/compiler-intel.h> #endif +#include <linux/align-section.h> +/* * Generic compiler-dependent macros required for kernel * build go below this comment. Actual compiler/compiler version
Signature · 1 line
1.7.2.3
Steven Rostedt
maintainerPATCH 2/3 tracing: Fix sparc64 alignment crash with __u64_aligned/U64_ALIGN()
In reply to Steven RostedtFrom: Mathieu Desnoyers <[email protected]>
Problem description:
gcc happily align structures defined statically on 32-byte. Ftrace trace events and Tracepoints both statically define structures into sections (using the "section" attribute), to then assign these to symbols with the linker scripts to iterate the these sections as an array.
However, gcc uses different alignments for these structures when they are defined statically and when they are globally visible and/or in an array. Therefore iteration on these arrays sees "holes" of padding.
Use the __u64_aligned for type declarations and variable definitions to ensure that gcc:
a) iterates on the correctly aligned type. (type attribute)
b) generates the definitions within the sections with the appropriate alignment. (variable attribute)
The Ftrace code introduced the "aligned(4)" variable attribute in commit 1473e4417c79f12d91ef91a469699bfa911f510f to try to work around this problem that showed up on x86_64, but it causes unaligned accesses on sparc64, and is generally a bad idea on 64-bit if RCU pointers are contained within the structure. Moreover, it did not use the same attribute as type attribute, which could cause the iteration on the extern array structure not to match the variable definitions for some structure sizes.
We should also ensure proper alignment of each Ftrace section in include/asm-generic/vmlinux.lds.h.
Moving all STRUCT_ALIGN() for FTRACE_EVENTS() and TRACE_SYSCALLS() into the definitions, so the alignment is only done if these infrastructures are configured in. Use U64_ALIGN instead of STRUCT_ALIGN.
Also align TRACE_PRINTKS on U64_ALIGN to make sure the beginning of the section is aligned on pointer size.
LKML-Reference: <[email protected]>
--- include/asm-generic/vmlinux.lds.h | 19 ++++++++++--------- include/linux/compiler.h | 6 +++--- include/linux/ftrace_event.h | 2 +- include/linux/syscalls.h | 12 ++++++------ include/trace/ftrace.h | 8 ++++---- include/trace/syscall.h | 2 +- kernel/trace/trace.h | 2 +- kernel/trace/trace_export.c | 2 +- 8 files changed, 27 insertions(+), 26 deletions(-)
include/asm-generic/vmlinux.lds.h +10 -9
@@ -114,7 +114,8 @@#endif #ifdef CONFIG_TRACE_BRANCH_PROFILING -#define LIKELY_PROFILE () VMLINUX_SYMBOL(__start_annotated_branch_profile) = .; \ +#define LIKELY_PROFILE () U64_ALIGN(); \ + VMLINUX_SYMBOL(__start_annotated_branch_profile) = .; \ *(_ftrace_annotated_branch) \ VMLINUX_SYMBOL(__stop_annotated_branch_profile) = .;#else
@@ -122,7 +123,8 @@#endif #ifdef CONFIG_PROFILE_ALL_BRANCHES -#define BRANCH_PROFILE () VMLINUX_SYMBOL(__start_branch_profile) = .; \ +#define BRANCH_PROFILE () U64_ALIGN(); \ + VMLINUX_SYMBOL(__start_branch_profile) = .; \ *(_ftrace_branch) \ VMLINUX_SYMBOL(__stop_branch_profile) = .;#else
@@ -130,7 +132,8 @@#endif #ifdef CONFIG_EVENT_TRACING -#define FTRACE_EVENTS () VMLINUX_SYMBOL(__start_ftrace_events) = .; \ +#define FTRACE_EVENTS () U64_ALIGN(); \ + VMLINUX_SYMBOL(__start_ftrace_events) = .; \ *(_ftrace_events) \ VMLINUX_SYMBOL(__stop_ftrace_events) = .;#else
@@ -138,7 +141,8 @@#endif #ifdef CONFIG_TRACING -#define TRACE_PRINTKS () VMLINUX_SYMBOL(__start___trace_bprintk_fmt) = .; \ +#define TRACE_PRINTKS () U64_ALIGN(); \ + VMLINUX_SYMBOL(__start___trace_bprintk_fmt) = .; \ *(__trace_printk_fmt)/* Trace_printk fmt' pointer */ \ VMLINUX_SYMBOL(__stop___trace_bprintk_fmt) = .;#else
@@ -146,7 +150,8 @@#endif #ifdef CONFIG_FTRACE_SYSCALLS -#define TRACE_SYSCALLS () VMLINUX_SYMBOL(__start_syscalls_metadata) = .; \ +#define TRACE_SYSCALLS () U64_ALIGN(); \ + VMLINUX_SYMBOL(__start_syscalls_metadata) = .; \ *(__syscalls_metadata) \ VMLINUX_SYMBOL(__stop_syscalls_metadata) = .;#else
@@ -183,11 +188,7 @@ LIKELY_PROFILE() \ BRANCH_PROFILE() \ TRACE_PRINTKS() \ - \ -STRUCT_ALIGN (); \ FTRACE_EVENTS() \ - \ -STRUCT_ALIGN (); \TRACE_SYSCALLS ()/*
include/linux/compiler.h +3 -3
@@ -80,7 +80,7 @@ struct ftrace_branch_data { };unsigned long miss_hit[2 ]; }; -}; +} __u64_aligned;/* * Note: DISABLE_BRANCH_PROFILING can be used by special lowlevel code
@@ -96,7 +96,7 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect);#define __branch_check__ (x, expect) ({ \ int ______r; \ static struct ftrace_branch_data \ - __attribute__((__aligned__(4))) \ + __u64_aligned \ __attribute__((section("_ftrace_annotated_branch"))) \ ______f = { \ .func = __func__, \
@@ -131,7 +131,7 @@ void ftrace_likely_update(struct ftrace_branch_data *f, int val, int expect); ({ \ int ______r; \ static struct ftrace_branch_data \ - __attribute__((__aligned__(4))) \ + __u64_aligned \ __attribute__((section("_ftrace_branch"))) \ ______f = { \ .func = __func__, \
include/linux/ftrace_event.h +1 -1
@@ -196,7 +196,7 @@ struct ftrace_event_call {int perf_refcount;struct hlist_head __percpu *perf_events;#endif -}; +} __u64_aligned;#define __TRACE_EVENT_FLAGS (name, value) \ static int __init trace_init_flags_##name(void) \
include/linux/syscalls.h +6 -6
@@ -126,9 +126,9 @@ extern struct trace_event_functions exit_syscall_print_funcs;#define SYSCALL_TRACE_ENTER_EVENT (sname) \ static struct syscall_metadata \ - __attribute__((__aligned__(4))) __syscall_meta_##sname; \ + __u64_aligned __syscall_meta_##sname; \ static struct ftrace_event_call __used \ - __attribute__((__aligned__(4))) \ + __u64_aligned \ __attribute__((section("_ftrace_events"))) \ event_enter_##sname = { \ .name = "sys_enter"#sname, \
@@ -140,9 +140,9 @@ extern struct trace_event_functions exit_syscall_print_funcs;#define SYSCALL_TRACE_EXIT_EVENT (sname) \ static struct syscall_metadata \ - __attribute__((__aligned__(4))) __syscall_meta_##sname; \ + __u64_aligned __syscall_meta_##sname; \ static struct ftrace_event_call __used \ - __attribute__((__aligned__(4))) \ + __u64_aligned \ __attribute__((section("_ftrace_events"))) \ event_exit_##sname = { \ .name = "sys_exit"#sname, \
@@ -156,7 +156,7 @@ extern struct trace_event_functions exit_syscall_print_funcs;SYSCALL_TRACE_ENTER_EVENT (sname); \SYSCALL_TRACE_EXIT_EVENT (sname); \ static struct syscall_metadata __used \ - __attribute__((__aligned__(4))) \ + __u64_aligned \ __attribute__((section("__syscalls_metadata"))) \ __syscall_meta_##sname = { \ .name = "sys"#sname, \
@@ -172,7 +172,7 @@ extern struct trace_event_functions exit_syscall_print_funcs; SYSCALL_TRACE_ENTER_EVENT(_##sname); \ SYSCALL_TRACE_EXIT_EVENT(_##sname); \ static struct syscall_metadata __used \ - __attribute__((__aligned__(4))) \ + __u64_aligned \ __attribute__((section("__syscalls_metadata"))) \ __syscall_meta__##sname = { \ .name = "sys_"#sname, \
include/trace/ftrace.h +4 -4
@@ -69,7 +69,7 @@#undef DEFINE_EVENT#define DEFINE_EVENT (template, name, proto, args) \ static struct ftrace_event_call __used \ - __attribute__((__aligned__(4))) event_##name + __u64_aligned event_##name;#undef DEFINE_EVENT_PRINT#define DEFINE_EVENT_PRINT (template, name, proto, args, print) \
@@ -447,7 +447,7 @@ static inline notrace int ftrace_get_offsets_##call( \* }; * * static struct ftrace_event_call __used -* __attribute__((__aligned__(4))) +* __u64_aligned * __attribute__((section("_ftrace_events"))) event_<call> = { * .name = "<call>", * .class = event_class_<template>,
@@ -580,7 +580,7 @@ static struct ftrace_event_class __used event_class_##call = { \#define DEFINE_EVENT (template, call, proto, args) \ \ static struct ftrace_event_call __used \ -__attribute__((__aligned__(4))) \ +__u64_aligned \ __attribute__((section("_ftrace_events"))) event_##call = { \ .name = #call, \ .class = &event_class_##template, \
@@ -594,7 +594,7 @@ __attribute__((section("_ftrace_events"))) event_##call = { \ static const char print_fmt_##call[] = print; \ \ static struct ftrace_event_call __used \ -__attribute__((__aligned__(4))) \ +__u64_aligned \ __attribute__((section("_ftrace_events"))) event_##call = { \ .name = #call, \ .class = &event_class_##template, \
include/trace/syscall.h +1 -1
@@ -29,7 +29,7 @@ struct syscall_metadata {struct ftrace_event_call *enter_event;struct ftrace_event_call *exit_event; -}; +} __u64_aligned;#ifdef CONFIG_FTRACE_SYSCALLS extern unsigned long arch_syscall_addr (int nr);
kernel/trace/trace.h +1 -1
@@ -749,7 +749,7 @@ extern const char *__stop___trace_bprintk_fmt[];#undef FTRACE_ENTRY#define FTRACE_ENTRY (call, struct_name, id, tstruct, print) \ extern struct ftrace_event_call \ - __attribute__((__aligned__(4))) event_##call; + __u64_aligned event_##call;#undef FTRACE_ENTRY_DUP#define FTRACE_ENTRY_DUP (call, struct_name, id, tstruct, print) \ FTRACE_ENTRY(call, struct_name, id, PARAMS(tstruct), PARAMS(print))
kernel/trace/trace_export.c +1 -1
@@ -162,7 +162,7 @@ struct ftrace_event_class event_class_ftrace_##call = { \ }; \ \ struct ftrace_event_call __used \ -__attribute__((__aligned__(4))) \ +__u64_aligned \ __attribute__((section("_ftrace_events"))) event_##call = { \ .name = #call, \ .event.type = etype, \
Signature · 1 line
1.7.2.3
Steven Rostedt
maintainerPATCH 3/3 tracepoints: Use __u64_aligned/U64_ALIGN()
In reply to Steven RostedtFrom: Mathieu Desnoyers <[email protected]>
commit 7e066fb870fcd1025ec3ba7bbde5d541094f4ce1 added the aligned(32) type and variable attribute to the tracepoint structures to deal with gcc happily aligning statically defined structures on 32-byte multiples.
Working on issues within Ftrace, we came up with __64_aligned, which deals with this issue more elegantly by forcing an 8-byte alignment to both the type declaration and variable definition.
This therefore saves space, bringing down the size of struct tracepoint from 64 bytes to 38 on 64-bit architectures.
Updating:
- The type attribute (for iteration over the struct tracepoint array)
- Added the variable attribute to the extern definition (needed to force gcc to consider this alignment for the following definition)
- The definition variable attribute (to force gcc to this specific alignment for the static definitions)
- The linker script (8-byte alignment can now replace the previous 32-byte alignment for the custom tracepoint section)
LKML-Reference: <[email protected]>
--- include/asm-generic/vmlinux.lds.h | 2 +- include/linux/tracepoint.h | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-)
include/asm-generic/vmlinux.lds.h +1 -1
@@ -176,7 +176,7 @@ CPU_KEEP(exit.data) \ MEM_KEEP(init.data) \ MEM_KEEP(exit.data) \ - . = ALIGN(32); \ +U64_ALIGN (); \ VMLINUX_SYMBOL(__start___tracepoints) = .; \ *(__tracepoints ) \ VMLINUX_SYMBOL(__stop___tracepoints) = .; \
include/linux/tracepoint.h +4 -8
@@ -33,12 +33,7 @@ struct tracepoint {void (*regfunc)(void );void (*unregfunc)(void );struct tracepoint_func __rcu *funcs; -} __attribute__((aligned(32))); /* -* Aligned on 32 bytes because it is -* globally visible and gcc happily -* align these on the structure size. -* Keep in sync with vmlinux.lds.h. -*/ +} __u64_aligned;/* * Connect a probe to a tracepoint.
@@ -146,7 +141,7 @@ static inline void tracepoint_update_probe_range(struct tracepoint *begin,* structure. Force alignment to the same alignment as the section start. */ #define __DECLARE_TRACE (name, proto, args, cond, data_proto, data_args) \ - extern struct tracepoint __tracepoint_##name; \ + extern struct tracepoint __u64_aligned __tracepoint_##name; \ static inline void trace_##name(proto) \ { \ JUMP_LABEL(&__tracepoint_##name.state, do_trace); \
@@ -178,7 +173,8 @@ do_trace: \ static const char __tpstrtab_##name[] \ __attribute__((section("__tracepoints_strings"))) = #name; \ struct tracepoint __tracepoint_##name \ - __attribute__((section("__tracepoints"), aligned(32))) = \ + __u64_aligned \ + __attribute__((section("__tracepoints"))) = \ { __tpstrtab_##name, 0, reg, unreg, NULL }#define DEFINE_TRACE (name) \
Signature · 1 line
1.7.2.3
Re: [PATCH 3/3] tracepoints: Use __u64_aligned/U64_ALIGN()
Quoted 85 lines · Steven Rostedt
From: Mathieu Desnoyers <[email protected]>
commit
7e066fb870fcd1025ec3ba7bbde5d541094f4ce1added the aligned(32) type and variable attribute to the tracepoint structures to deal with gcc happily aligning statically defined structures on 32-byte multiples.Working on issues within Ftrace, we came up with
__64_aligned, which deals with this issue more elegantly by forcing an 8-byte alignment to both the type declaration and variable definition.This therefore saves space, bringing down the size of struct tracepoint from 64 bytes to 38 on 64-bit architectures.
Updating:
- The type attribute (for iteration over the struct tracepoint array)
- Added the variable attribute to the extern definition (needed to force gcc to consider this alignment for the following definition)
- The definition variable attribute (to force gcc to this specific alignment for the static definitions)
- The linker script (8-byte alignment can now replace the previous 32-byte alignment for the custom tracepoint section)Signed-off-by Mathieu Desnoyers <[email protected]>LKML-Reference: <[email protected]>
Acked-by David Miller <[email protected]>CC Frederic Weisbecker <[email protected]>CC Ingo Molnar <[email protected]>Signed-off-by Steven Rostedt <[email protected]>--- include/asm-generic/vmlinux.lds.h | 2 +- include/linux/tracepoint.h | 12 ++++-------- 2 files changed, 5 insertions(+), 9 deletions(-)include/asm-generic/vmlinux.lds.h +1
-1@@ -176,7 +176,7 @@ CPU_KEEP(exit.data) \ MEM_KEEP(init.data) \ MEM_KEEP(exit.data) \ - . = ALIGN(32); \ +U64_ALIGN (); \ VMLINUX_SYMBOL(__start___tracepoints) = .; \ *(__tracepoints ) \ VMLINUX_SYMBOL(__stop___tracepoints) = .; \include/linux/tracepoint.h +4
-8@@ -33,12 +33,7 @@ struct tracepoint {void (*regfunc)(void );void (*unregfunc)(void );struct tracepoint_func __rcu *funcs; -} __attribute__((aligned(32))); /* -* Aligned on 32 bytes because it is -* globally visible and gcc happily -* align these on the structure size. -* Keep in sync with vmlinux.lds.h. -*/ +} __u64_aligned;/* * Connect a probe to a tracepoint. @@ -146,7 +141,7 @@ static inline void tracepoint_update_probe_range(struct tracepoint *begin,* structure. Force alignment to the same alignment as the section start. */ #define __DECLARE_TRACE (name, proto, args, cond, data_proto, data_args) \ - extern struct tracepoint __tracepoint_##name; \ + extern struct tracepoint __u64_aligned __tracepoint_##name; \ static inline void trace_##name(proto) \ { \ JUMP_LABEL(&__tracepoint_##name.state, do_trace); \@@ -178,7 +173,8 @@ do_trace: \ static const char __tpstrtab_##name[] \ __attribute__((section("__tracepoints_strings"))) = #name; \ struct tracepoint __tracepoint_##name \ - __attribute__((section("__tracepoints"), aligned(32))) = \ + __u64_aligned \ + __attribute__((section("__tracepoints"))) = \ { __tpstrtab_##name, 0, reg, unreg, NULL }#define DEFINE_TRACE (name) \Signature · 1 line
1.7.2.3
hi,
this patch breaks tracepoint section iterating for me when trying to enable any event I get:
[ 69.119483] BUG: unable to handle kernel NULL pointer dereference at (null) [ 69.119868] IP: [<ffffffff8121c2c2>] strlen+0x2/0x30 [ 69.120104] PGD 77b84067 PUD 75dd6067 PMD 0 [ 69.120104] Oops: 0000 [#1] SMP [ 69.120104] last sysfs file: /sys/devices/virtual/vc/vcs5/uevent [ 69.120104] CPU 1 [ 69.120104] Modules linked in: [ 69.120104] [ 69.120104] Pid: 1119, comm: bash Not tainted 2.6.38-rc2-tip+ #325 To be filled by O.E.M./Montevina platform [ 69.120104] RIP: 0010:[<ffffffff8121c2c2>] [<ffffffff8121c2c2>] strlen+0x2/0x30 [ 69.120104] RSP: 0018:ffff8800754fbd60 EFLAGS: 00010246 [ 69.120104] RAX: 0000000000000000 RBX: ffffffff8180e530 RCX: 0000000073ed290b [ 69.120104] RDX: 0000000002687d93 RSI: ffffffff8173387c RDI: 0000000000000000 [ 69.120104] RBP: ffff8800754fbd88 R08: 0000000000000000 R09: 0000000000000000 [ 69.120104] R10: ffff8800779dbad0 R11: 0000000000000000 R12: 0000000000000000 [ 69.120104] R13: 0000000000000000 R14: ffffffff8180fa48 R15: 0000000000000000 [ 69.120104] FS: 00007fd6c90ae720(0000) GS:ffff88007b480000(0000) knlGS:0000000000000000 [ 69.120104] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 69.120104] CR2: 0000000000000000 CR3: 0000000075bec000 CR4: 00000000000406e0 [ 69.120104] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 69.120104] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400 [ 69.120104] Process bash (pid: 1119, threadinfo ffff8800754fa000, task ffff88007bdc2640) [ 69.120104] Stack: [ 69.120104] ffffffff8109b9fa ffff8800779dbb00 ffffffff8180e530 0000000000000000 [ 69.120104] 0000000000000001 ffff8800754fbdc8 ffffffff8109c4b6 ffffffff81a1d3e8 [ 69.120104] 0000000000000000 0000000000000000 0000000000000000 ffff880076ed3c00 [ 69.120104] Call Trace: [ 69.120104] [<ffffffff8109b9fa>] ? get_tracepoint+0x1a/0x1a0 [ 69.120104] [<ffffffff8109c4b6>] tracepoint_update_probe_range+0xe6/0x1d0 [ 69.120104] [<ffffffff8109c5fc>] tracepoint_update_probes+0x1c/0x30 [ 69.120104] [<ffffffff8109c855>] tracepoint_probe_register+0x85/0xb0 [ 69.120104] [<ffffffff810ad4dd>] tracing_start_sched_switch+0x4d/0xf0 [ 69.120104] [<ffffffff810ad5f9>] tracing_start_cmdline_record+0x9/0x10 [ 69.120104] [<ffffffff810b5955>] ftrace_event_enable_disable+0xc5/0xf0 [ 69.120104] [<ffffffff810b6648>] event_enable_write+0xf8/0x130 [ 69.120104] [<ffffffff8110e564>] ? rw_verify_area+0xf4/0x1b0 [ 69.120104] [<ffffffff8110f0d9>] vfs_write+0x119/0x1c0 [ 69.120104] [<ffffffff8110f281>] sys_write+0x51/0x90 [ 69.120104] [<ffffffff8100316b>] system_call_fastpath+0x16/0x1b [ 69.120104] Code: 11 eb 1b 66 0f 1f 44 00 00 48 83 ea 01 48 39 c2 72 0c 0f b6 0a f6 81 a0 c4 55 81 20 75 eb c6 42 01 00 c9 c3 0f 1f 44 00 00 31 c0 <80> 3f 00 55 48 89 fa 48 89 e5 74 11 66 90 48 83 c2 01 80 3a 00 [ 69.120104] RIP [<ffffffff8121c2c2>] strlen+0x2/0x30 [ 69.120104] RSP <ffff8800754fbd60> [ 69.120104] CR2: 0000000000000000
tracepoint_update_probe_range will oops due to the tracepoint name being null
I found the elements in "__tracepoints" sections might have different size after above patch is applied:
with:
nm vmlinux | grep __tracepoint_ | sort
I got:
...ffffffff8180e4b8 D __tracepoint_clock_disable ffffffff8180e4e0 D __tracepoint_clock_set_rate ffffffff8180e508 D __tracepoint_power_domain_target ffffffff8180e540 D __tracepoint_mm_vmscan_kswapd_sleep ffffffff8180e568 D __tracepoint_mm_vmscan_kswapd_wake
...
addr(__tracepoint_mm_vmscan_kswapd_sleep) - addr(__tracepoint_power_domain_target) = 0x38
while size of "struct tracepoints" seems to be 0x28
and the listing has several places like that
At least the code "tracepoint_update_probe_range" relies on that, while iterating the section.
please let me know if you need more info,
not sure what's going on.. :)
thanks,
jirka
[root@dhcp-26-214 ~]# uname -a
Linux dhcp-26-214.brq.redhat.com 2.6.38-rc2-tip+ #325 SMP Thu Jan 27 19:11:56 CET 2011 x86_64 x86_64 x86_64 GNU/Linux
[jolsa@krava1 linux-2.6-tip]$ gcc --version
gcc (GCC) 4.4.4 20100726 (Red Hat 4.4.4-13)
Copyright (C) 2010 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Steven Rostedt
maintainerRe: [PATCH 3/3] tracepoints: Use __u64_aligned/U64_ALIGN()
Quoted 2 lines · Jiri Olsa
this patch breaks tracepoint section iterating for me when trying to enable any event I get:
Yep we know. We are currently going through and reverting this with something else.
Thanks!
-- Steve
Re: [PATCH 3/3] tracepoints: Use __u64_aligned/U64_ALIGN()
Quoted 7 lines · Steven Rostedt
Quoted 2 lines · Jiri Olsa
this patch breaks tracepoint section iterating for me when trying to enable any event I get:
Yep we know. We are currently going through and reverting this with something else.
For now i've unrolled it from -tip to give this stuff some more time to cook - it's clearly not ready yet.
Thanks,
Ingo
Steven Rostedt
maintainerRe: [PATCH 3/3] tracepoints: Use __u64_aligned/U64_ALIGN()
Quoted 4 lines · Ingo Molnar
Quoted 2 lines · Steven Rostedt
On Thu, 2011-01-27 at 19:19 +0100, Jiri Olsa wrote:
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
this patch breaks tracepoint section iterating for me when trying to enable any event I get:
Yep we know. We are currently going through and reverting this with something else.
For now i've unrolled it from -tip to give this stuff some more time to cook - it's clearly not ready yet.
OK, good, because I was going to revert that anyway :)
I'll make sure the new patches roll on top of what you have in tip.
Thanks!
-- Steve
Quoted 28 lines · Steven Rostedt
Ingo,
It was discovered that the alignment of the trace event sections break sparc64. The trace events are created by linker magic, so gcc does not realize that the structures being allocated will end up in an array. Without adding any alignment,
x86_64would create the trace event arrays with holes that would break reading the events in serial. The fix for that was to force a align(4) to keep gcc and the linker from spacing the event structures at various alignments. But recent changes have caused this change to break sparc due to bad alignment in the structures, caused by the 4 byte alignment.After some discussions, Mathieu came up with the following patch set to correctly fix the alignment issues with the trace event structures.
Please pull the latest tip/perf/urgent tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-2.6-trace.git
tip/perf/urgentMathieu Desnoyers (3):
Introduce__u64_alignedandU64_ALIGN()for structure alignment in custom sections
tracing: Fix sparc64 alignment crash with __u64_aligned/U64_ALIGN()
tracepoints: Use __u64_aligned/U64_ALIGN()----
Quoted 11 lines
include/asm-generic/vmlinux.lds.h | 27 +++++++++++------- include/linux/align-section.h | 54 +++++++++++++++++++++++++++++++++++++ include/linux/compiler.h | 8 +++-- include/linux/ftrace_event.h | 2 +- include/linux/syscalls.h | 12 ++++---- include/linux/tracepoint.h | 12 +++----- include/trace/ftrace.h | 8 +++--- include/trace/syscall.h | 2 +- kernel/trace/trace.h | 2 +- kernel/trace/trace_export.c | 2 +- 10 files changed, 94 insertions(+), 35 deletions(-)
Pulled, thanks a lot Steve!
Ingo
Steven Rostedt
maintainerQuoted 9 lines · Ingo Molnar
Quoted 5 lines
Mathieu Desnoyers (3):
Introduce__u64_alignedandU64_ALIGN()for structure alignment in custom sections
tracing: Fix sparc64 alignment crash with __u64_aligned/U64_ALIGN()
tracepoints: Use __u64_aligned/U64_ALIGN()Pulled, thanks a lot Steve!
Ingo
Hi Ingo,
We (well Peter really) just discovered that this can cause alignment issues with gcc 4.4.4 and 4.4.5 (I unfortunately only tested it with gcc 4.5.1). We are currently looking into fixing it.
Letting you know so that you don't push this off to Linus ;-)
-- Steve
Quoted 13 lines · Steven Rostedt
Quoted 9 lines · Ingo Molnar
Quoted 5 lines
Mathieu Desnoyers (3):
Introduce__u64_alignedandU64_ALIGN()for structure alignment in custom sections
tracing: Fix sparc64 alignment crash with __u64_aligned/U64_ALIGN()
tracepoints: Use __u64_aligned/U64_ALIGN()Pulled, thanks a lot Steve!
Ingo
Quoted 7 lines
Hi Ingo,
We (well Peter really) just discovered that this can cause alignment issues with gcc 4.4.4 and 4.4.5 (I unfortunately only tested it with gcc 4.5.1). We are currently looking into fixing it.
Letting you know so that you don't push this off to Linus ;-)
Well, i found and reported that 'perf stat -e irq:irq_handler_entry -a sleep 1' crash to Peter (i first thought it's due to his patch) so yes i know about it :-)
Thanks,
Ingo