On Mon, 11 Jul 2016 09:18:21 -0400 Jason Baron <jba...@akamai.com> wrote:
> On 07/08/2016 05:41 PM, Andrew Morton wrote:
> > On Wed, 6 Jul 2016 17:42:36 -0400 Jason Baron <jba...@akamai.com> wrote:
> >
> >> Although dynamic debug is often only used for debug builds, sometimes its
> >> enabled for production builds as well. Minimize its impact by using jump
> >> labels. This reduces the text section by 7000+ bytes in the kernel image
> >> below. It does increase data, but this should only be referenced when
> >> changing the direction of the branches, and hence usually not in cache.
> >>
> >> ...
> >>
> >> +#ifdef HAVE_JUMP_LABEL
> >> +
> >> +#define dd_key_init(key, init) key = (init)
> >>
> >> ...
> >>
> >> +#else
> >> +
> >> +#define dd_key_init(key, init)
> >> +
> > umm, lazy. One is an lval and returns a value and the other does
> > neither. Lack of parenthesization in the first version doubtless
> > exposes various horrors.
> >
> > Care to do something more robust and conventional here? Presumably use
> > symmetrical do{}while(0) things, neither of which is an lval, both of
> > which don't return anything.
> >
>
> Hi,
>
> The 'dd_key_init()' macro is being used here to help initialize
> the 'key' field in the 'struct _ddebug', and its not being used as a
> statement.
>
> In the 'HAVE_JUMP_LABEL' case, we are initializing the 'key' field, while in
> the not-'HAVE_JUMP_LABEL' case, the 'key' field is simply not present
> in the structure (to conserve space).
Well yeah. And it's doing it wrongly, isn't it?
: @@ -68,13 +78,51 @@ void __dynamic_netdev_dbg(struct _ddebug
: .filename = __FILE__, \
: .format = (fmt), \
: .lineno = __LINE__, \
: - .flags = _DPRINTK_FLAGS_DEFAULT, \
: + .flags = _DPRINTK_FLAGS_DEFAULT, \
: + dd_key_init(key, init) \
: }
:
: +#ifdef HAVE_JUMP_LABEL
: +
: +#define dd_key_init(key, init) key = (init)
Shouldn't it be ".key = (init)"?
Anyway, it's odd-looking. I guess something like
#define DD_KEY_INIT(init) .key = (init)
would be more idiomatic.
