Re: [PATCH 09/11 re-post take #2] dynamic_debug: consolidaterepetitive struct _ddebug descriptor definitions

[Jason Baron]

On Mon, Aug 15, 2011 at 04:12:27PM -0700, Joe Perches wrote:
> On Mon, 2011-08-15 at 16:44 -0400, Jason Baron wrote:
> > Replace the repetitive  struct _ddebug descriptor definitions with
> > a new DECLARE_DYNAMIC_DEBUG_META_DATA(name, fmt) macro.
> []
> > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> []
> > +#define DECLARE_DYNAMIC_DEBUG_METADATA(name, fmt)		\
> > +	static struct _ddebug __used __aligned(8)		\
> > +	__attribute__((section("__verbose"))) name = {		\
> []
> 
> Jason, just one more thing...
> 
> Because the original struct _ddebug definition
> above this uses __attribute__((aligned(8))),
> 
> struct _ddebug {
> 	[...]
> } __attribute__((aligned(8)));
> 
> ( and I suppose that should be __aligned(8) instead )
> 
> the __aligned(8) use in DECLARE_DYNAMIC_DEBUG_METADATA
> is not necessary.
> 
> cheers, Joe
> 

Hi Joe,

hmmm....what we probably need here, is an extra array of pointers into
the dynamic debug structures. The reason being is that 'aligned' only
specifies the minimum alignment, so the dynamic debug structures could
be aligned to higher multiples, thus leaving extra padding. Since the
dynamic debug code relies on a contiguous array this could lead to NULl
pointer exceptions. This has been implemented for tracepoints, see: 
https://lkml.org/lkml/2011/1/26/463. I haven't seen this issue in
practice though...

So I'm going to leave the extra alignment specifications for now (they
can't hurt), and at some point we probably need to look at the approach
mentioned above.

Thanks,

-Jason
--
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/