Re: [PATCH v3 7/7] dynamic_debug: add jump label support

From: Jason Baron
Date: Thu Jul 07 2016 - 15:11:25 EST




On 07/06/2016 06:52 PM, Joe Perches wrote:
> On Wed, 2016-07-06 at 17:42 -0400, Jason Baron 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.
>>
>> text data bss dec hex filename
>> 8194852 4879776 925696 14000324 d5a0c4 vmlinux.pre
>> 8187337 4960224 925696 14073257 d6bda9 vmlinux.post
>
> Maybe to get some of that space back on the 32 bit builds
> the 8 byte alignment can be relaxed.
>
>> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> []
>> @@ -33,6 +37,12 @@ struct _ddebug {
>> #define _DPRINTK_FLAGS_DEFAULT 0
>> #endif
>> unsigned int flags:8;
>> +#ifdef HAVE_JUMP_LABEL
>> + union {
>> + struct static_key_true dd_key_true;
>> + struct static_key_false dd_key_false;
>> + } key;
>> +#endif
>> } __attribute__((aligned(8)));
>
> Couldn't this be:
>
> } __aligned(__alignof__(void *));
>
>> @@ -60,7 +70,7 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
>> const struct net_device *dev,
>> const char *fmt, ...);
>>
>> -#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \
>> +#define DEFINE_DYNAMIC_DEBUG_METADATA_KEY(name, fmt, key, init) \
>> static struct _ddebug __aligned(8) \
>> __attribute__((section("__verbose"))) name = { \
>
> And this shouldn't need the __aligned as it's already
> in the structure definition.
>
> Maybe:
>
> static struct _ddebug __section(__verbose) name = {
>
>> .modname = KBUILD_MODNAME, \
>


hmmm...dropping that last bit increases the size of 'data', and
results in an oops on boot, I think b/c the __verbose section
adds gaps that the dynamic debug init code is not expecting. We
could revisit this for 32-bit, but this is somewhat fragile...

I think we could reduce the size of 'struct static_key' by
removing the 'next' field and using the 'entries' when there
are multiple modules associated with a key. This will increase
the memory usage slightly when there are multiple modules
associated with a key, but this is probably not the common case,
and certainly for dynamic debug we only have one patch target
for each key. That should reduce the 'data' increase by around
~20% and would reduce the size for other users. I can try something
along those lines in a re-post...

Thanks,

-Jason