[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20111213231523.GB19390@Krystal>
Date: Tue, 13 Dec 2011 18:15:23 -0500
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: Andi Kleen <ak@...ux.intel.com>,
Thomas Gleixner <tglx@...utronix.de>,
Andi Kleen <andi@...stfloor.org>, Greg KH <greg@...ah.com>,
LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/3] DEBUGFS: Add per cpu counters
* Steven Rostedt (rostedt@...dmis.org) wrote:
> On Tue, 2011-12-13 at 14:56 -0800, Andi Kleen wrote:
> >
> > > > +struct debugfs_counter {
> > > > + unsigned __percpu *ptr;
> > > > + const char *fn;
> > > > + const char *name;
> > > > +} __attribute__((aligned(sizeof(char *))));
> > > > +
>
> > >
> > > See commit 654986462 for details.
> >
> > Doesn't give a lot of details actually. Which target?
> >
> > Note that my structure only has pointers, so there is not a lot
> > of potential for "evil" alignment.
>
> We hit this with trace events as well, and your structure does have an
> evil alignment, it's 3 pointers, which is not a base 2 number. Thus,
> there were some compilers that liked to add padding to make the
> alignment a power of 2. That is, between two sections of files we ended
> up with something like:
>
>
> .section file1
> .ptra1
> .ptra2
> .ptra3
> .ptrb1
> .ptrb2
> .ptrb3
> .ptrc1
> .ptrc2
> .ptrc3
> .ptrd1
> .ptrd2
> .ptrd3
> .ptre1
> .ptre2
> .ptre3
> <space>
> .section file2
> .ptrf1
> .ptrf2
> .ptrf3
> [...]
>
>
>
> It didn't happen often, heck, trace events and tracepoints were like
> this for sometime before this blew up in our faces.
>
> From what I've seen is that 1 or 2 longs will pack nicely, but anything
> else (except for maybe 4, 8, 16, etc) will run a risk of crashing.
Yep. We changed our implementations to use an array of pointers to the
elements rather than creating a section of these elements per se to make
sure we would not trigger this problem anymore. Unless my memory fails
me, it started blowing up in our faces with new gcc versions. The thing
is that gcc is within its right to happily over-align those structures.
__attribute__((aligned())) is just a hint providing a minimum alignment
to respect, but the compiler can choose a higher alignment value as it
sees fit.
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 linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists