From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752746AbcGGTLS (ORCPT ); Thu, 7 Jul 2016 15:11:18 -0400 Received: from prod-mail-xrelay05.akamai.com ([23.79.238.179]:51732 "EHLO prod-mail-xrelay05.akamai.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750865AbcGGTLA (ORCPT ); Thu, 7 Jul 2016 15:11:00 -0400 Subject: Re: [PATCH v3 7/7] dynamic_debug: add jump label support To: Joe Perches , References: <1467845547.8360.43.camel@perches.com> CC: , From: Jason Baron X-Enigmail-Draft-Status: N1110 Message-ID: <577EA90B.2060601@akamai.com> Date: Thu, 7 Jul 2016 15:10:03 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.8.0 MIME-Version: 1.0 In-Reply-To: <1467845547.8360.43.camel@perches.com> Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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