All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jason Baron <jbaron@akamai.com>
To: Joe Perches <joe@perches.com>, <akpm@linux-foundation.org>
Cc: <peterz@infradead.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 7/7] dynamic_debug: add jump label support
Date: Thu, 7 Jul 2016 15:10:03 -0400	[thread overview]
Message-ID: <577EA90B.2060601@akamai.com> (raw)
In-Reply-To: <1467845547.8360.43.camel@perches.com>



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

  reply	other threads:[~2016-07-07 19:11 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-07-06 21:42 [PATCH v3 0/7] convert dynamic_debug to use jump labels Jason Baron
2016-07-06 21:42 ` [PATCH v3 1/7] powerpc: add explicit #include <asm/asm-compat.h> for jump label Jason Baron
2016-07-08 13:41   ` [v3, " Michael Ellerman
2016-07-06 21:42 ` [PATCH v3 2/7] s390: add explicit <linux/stringify.h> " Jason Baron
2016-07-06 21:42 ` [PATCH v3 3/7] sparc: support static_key usage in non-module __exit sections Jason Baron
2016-07-06 21:42   ` Jason Baron
2016-07-07  1:49   ` David Miller
2016-07-07  1:49     ` David Miller
2016-07-06 21:42 ` [PATCH v3 4/7] tile: " Jason Baron
2016-07-06 21:42 ` [PATCH v3 5/7] arm: jump label may reference text in __exit Jason Baron
2016-07-06 21:42   ` Jason Baron
2016-07-06 21:42 ` [PATCH v3 6/7] jump_label: remove bug.h, atomic.h dependencies for HAVE_JUMP_LABEL Jason Baron
2016-07-06 21:42 ` [PATCH v3 7/7] dynamic_debug: add jump label support Jason Baron
2016-07-06 22:52   ` Joe Perches
2016-07-07 19:10     ` Jason Baron [this message]
2016-07-08 21:41   ` Andrew Morton
2016-07-11 13:18     ` Jason Baron
2016-07-11 19:23       ` Andrew Morton
2016-07-11 19:52         ` Jason Baron

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=577EA90B.2060601@akamai.com \
    --to=jbaron@akamai.com \
    --cc=akpm@linux-foundation.org \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.