Messages in this thread | | | Date | Mon, 11 Jul 2016 12:23:28 -0700 | From | Andrew Morton <> | Subject | Re: [PATCH v3 7/7] dynamic_debug: add jump label support |
| |
On Mon, 11 Jul 2016 09:18:21 -0400 Jason Baron <jbaron@akamai.com> wrote:
> On 07/08/2016 05:41 PM, Andrew Morton wrote: > > On Wed, 6 Jul 2016 17:42:36 -0400 Jason Baron <jbaron@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.
| |