Re: [PATCH 2/3] DEBUGFS: Add per cpu counters

[Thomas Gleixner <tglx@linutronix.de>]

On Tue, 13 Dec 2011, Andi Kleen wrote:
> On Tue, Dec 13, 2011 at 11:43:16PM +0100, Thomas Gleixner wrote:
> > On Tue, 13 Dec 2011, Andi Kleen wrote:
> > >  									\
> > > +	__debugfs	: AT(ADDR(__debugfs) - LOAD_OFFSET) {		\
> > > +		VMLINUX_SYMBOL(__start___debugfs) = .;			\
> > > +		*(__debugfs)						\
> > > +		VMLINUX_SYMBOL(__stop___debugfs) = .;			\
> > > +	}								\
> > > +									\
> > 
> > ....
> >   
> > > +struct debugfs_counter {
> > > +	unsigned __percpu *ptr;
> > > +	const char *fn;
> > > +	const char *name;
> > > +} __attribute__((aligned(sizeof(char *))));
> > > +
> > > +/* Note: static doesn't work unlike DEFINE_PERCPU. Sorry. */
> > > +#define DEFINE_DEBUGFS_COUNTER(name_, file)				\
> > > +	DEFINE_PER_CPU(unsigned, name_ ## _counter);			\
> > > +	struct debugfs_counter name_ ## _pcpu_counter __used		\
> > > +	__attribute__((aligned(sizeof(char *)),section("__debugfs"),unused)) \
> > > +	= { .ptr = &name_ ## _counter, .fn = file, .name = #name_ }; \
> > 
> > Sigh, we had that section forms an array problem more than once
> > already. Why do you invent another variant and think that it will not
> > explode?
> 
> I did three or four different sections like this in the past and as far
> as I know none of them has exploded so far in production use.
> 
> Can you be more specific? Where exactly do you think this will
> not work?

Can you actually read and understand what I wrote below?
 
> > Your alignment magic does not guarantee at all that the structs will
> > form an array. The "aligned" attribute guarantees only the _MINIMUM_
> > alignment for a structure, but the compiler and the linker are free to
> > align on larger multiples.
> 
> > 
> > See commit 654986462 for details.
> 
> Doesn't give a lot of details actually.

There are enough details to figure it out. http://www.jfgit.com might
help.

>  Which target?

The target is irrelevant.

> Note that my structure only has pointers, so there is not a lot 
> of potential for "evil" alignment.

That does NOT matter at ALL. Also whether you did that in the past or
not is completely irrelevant. Its also totally irrelevant whether your
structure only has pointers or not.

There is NO GUARANTEE that this will end up in a linear array with a
stride of the struct size. Period.

Just get it.

Thanks,

	tglx