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

[Thomas Gleixner]

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?

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. That has been discussed to death and
your solution wont work either across all the various compiler
incarnations we have and will have.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/