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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 7 Jul 2016 15:10:03 -0400
From:	Jason Baron <jbaron@...mai.com>
To:	Joe Perches <joe@...ches.com>, <akpm@...ux-foundation.org>
CC:	<peterz@...radead.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 7/7] dynamic_debug: add jump label support



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