* Steven Rostedt (rostedt@xxxxxxxxxxx) wrote:
> [ Added Mathieu on Cc, since he likes alignments ;-) ]
Oh yes, alignments are so much fun! (for some definitions of fun) ;)
>
> On Sun, 2011-01-16 at 11:39 -0800, David Miller wrote:
> > From: Richard Mortimer <richm@xxxxxxxxxxxxxxx>
> > Date: Sun, 16 Jan 2011 14:17:49 +0000
> >
> > > I'm wondering if gcc is just getting better at honouring the source
> > > code. The DEFINE_EVENT macros in include/trace/ftrace.h have a
> > > __aligned__(4) attribute in them. Maybe that should be 8 on sparc64
> > > systems.
> > > The aligned 4 seems to be unchanged since include/trace/ftrace.h was
> > > created in f42c85e74faa422cf0bc747ed808681145448f88 in April 2009.
> >
> > That needs to be at least "8" on 64-bit systems. Why is this aligned
> > directive there at all?
>
> IIRC, the problem showed up in 64-bit systems. OK, x86-64 (but of
> course ;-).
>
> The problem comes when the linker puts these sections together. We read
> all the sections as one big array. If the linker puts in holes, then
> this breaks the array, and the kernel crashes while reading the section.
>
> I guess one solution is to remove the alignment at the allocation and
> place it at the structure. This will mean all accesses to this structure
> will need to be on an alignment.
The problem with these alignments is that they are just a hint to gcc, telling
it what the minimum alignment of a type should be. gcc is free to align on a
larger boundary if it wants to.
But the following test program is very instructive:
#include <stdio.h>
struct test {
void *a;
void *b;
void *c;
void *d;
void *e;
void *f;
void *g;
void *h;
void *i;
void *j;
void *k;
void *l;
void *m;
void *n;
void *o;
void *p;
void *q;
};
int main()
{
struct test __attribute__((aligned(4))) v;
printf("%d\n", __alignof__(v));
return 0;
}
(on x86_64, with gcc 4.5.1 and gcc 4.4.4)
if we put the "__attribute__((aligned(4)))" at the v definition (variable
attribute), the program returns an alignment of 4. If we move it after struct
test declaration (type attribute), the program returns an alignment of 8 (thus
taking the max between the attribute alignment and the largest field).
But that's a real problem, because in include/trace/ftrace.h, we have an
alignment of 4 forced on the definition, but there is a mismatch with
trace_events.c:
extern struct ftrace_event_call __start_ftrace_events[];
extern struct ftrace_event_call __stop_ftrace_events[];
for which the alignment attribute is missing (so an alignment of 8 will be
used there).
So it all worked as long as the size of struct ftrace_event_call was a multiple
of 8 bytes (struct ftrace_event_call constains 2 integers if we exclude the perf
fields), but the new fields added by perf contain a supplementary 4-byte
integer, which seems to be causing the breakage: the structures are appended one
next to another when defined, but the iteration on these structures thinks they
are 8-byte aligned.
Steven, what were you trying to fix in the first place when you added the
aligned(4) to the definition ? It might have just been that the _ftrace_events
section needed to be aligned on at least 8 bytes in the linker scripts, but was
only aligned on 4-bytes. Forcing the definition alignment down to 4 possibly
fixed the problem you experienced on x86_64, but seems to be causing other
problems.
I would recommend to:
- Keep the linker script _ftrace_events alignment as it is now (aligned on 32
bytes).
- Remove the aligned(4) attributes from all struct ftrace_event_call
definitions.
And see how this works. The only problem that might come up is if gcc decides to
align struct ftrace_event_call (which is about 136 bytes in size) on an
alignment larger than 32 bytes, which would be really surprising.
Mathieu
>
> -- Steve
>
>
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
