From: Jessica Yu <jeyu@kernel.org>
To: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Matthias Maennich <maennich@google.com>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Arnd Bergmann <arnd@arndb.de>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Subject: Re: [PATCH v2] export.h: reduce __ksymtab_strings string duplication by using "MS" section flags
Date: Tue, 26 Nov 2019 10:55:10 +0100 [thread overview]
Message-ID: <20191126095510.GA30181@linux-8ccs> (raw)
In-Reply-To: <CAK7LNASU9YysYNXuBKSU4WeUyE=2itfLDYzCupXL-49GUZuGnQ@mail.gmail.com>
+++ Masahiro Yamada [26/11/19 17:32 +0900]:
>On Tue, Nov 26, 2019 at 12:42 AM Jessica Yu <jeyu@kernel.org> wrote:
>>
>> Commit c3a6cf19e695 ("export: avoid code duplication in
>> include/linux/export.h") refactors export.h quite nicely, but introduces
>> a slight increase in memory usage due to using the empty string ""
>> instead of NULL to indicate that an exported symbol has no namespace. As
>> mentioned in that commit, this meant an increase of 1 byte per exported
>> symbol without a namespace. For example, if a kernel configuration has
>> about 10k exported symbols, this would mean that the size of
>> __ksymtab_strings would increase by roughly 10kB.
>>
>> We can alleviate this situation by utilizing the SHF_MERGE and
>> SHF_STRING section flags. SHF_MERGE|SHF_STRING indicate to the linker
>> that the data in the section are null-terminated strings that can be
>> merged to eliminate duplication. More specifically, from the binutils
>> documentation - "for sections with both M and S, a string which is a
>> suffix of a larger string is considered a duplicate. Thus "def" will be
>> merged with "abcdef"; A reference to the first "def" will be changed to
>> a reference to "abcdef"+3". Thus, all the empty strings would be merged
>> as well as any strings that can be merged according to the cited method
>> above. For example, "memset" and "__memset" would be merged to just
>> "__memset" in __ksymtab_strings.
>>
>> As of v5.4-rc5, the following statistics were gathered with x86
>> defconfig with approximately 10.7k exported symbols.
>>
>> Size of __ksymtab_strings in vmlinux:
>> -------------------------------------
>> v5.4-rc5: 213834 bytes
>> v5.4-rc5 with commit c3a6cf19e695: 224455 bytes
>> v5.4-rc5 with this patch: 205759 bytes
>>
>> So, we already see memory savings of ~8kB compared to vanilla -rc5 and
>> savings of nearly 18.7kB compared to -rc5 with commit c3a6cf19e695 on top.
>>
>> Unfortunately, as of this writing, strings will not get deduplicated for
>> kernel modules, as ld does not do the deduplication for
>> SHF_MERGE|SHF_STRINGS sections for relocatable files (ld -r), which
>> kernel modules are. A patch for ld is currently being worked on to
>> hopefully allow for string deduplication in relocatable files in the
>> future.
>>
>> Suggested-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> Signed-off-by: Jessica Yu <jeyu@kernel.org>
>> ---
>>
>> v2: use %progbits throughout and document the oddity in a comment.
>>
>> include/asm-generic/export.h | 8 +++++---
>> include/linux/export.h | 27 +++++++++++++++++++++------
>> 2 files changed, 26 insertions(+), 9 deletions(-)
>>
>> diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
>> index fa577978fbbd..23bc98e97a66 100644
>> --- a/include/asm-generic/export.h
>> +++ b/include/asm-generic/export.h
>> @@ -26,9 +26,11 @@
>> .endm
>>
>> /*
>> - * note on .section use: @progbits vs %progbits nastiness doesn't matter,
>> - * since we immediately emit into those sections anyway.
>> + * note on .section use: we specify progbits since usage of the "M" (SHF_MERGE)
>> + * section flag requires it. Use '%progbits' instead of '@progbits' since the
>> + * former apparently works on all arches according to the binutils source.
>> */
>> +
>> .macro ___EXPORT_SYMBOL name,val,sec
>> #ifdef CONFIG_MODULES
>> .globl __ksymtab_\name
>> @@ -37,7 +39,7 @@
>> __ksymtab_\name:
>> __put \val, __kstrtab_\name
>> .previous
>> - .section __ksymtab_strings,"a"
>> + .section __ksymtab_strings,"aMS",%progbits,1
>> __kstrtab_\name:
>> .asciz "\name"
>> .previous
>> diff --git a/include/linux/export.h b/include/linux/export.h
>> index 201262793369..3d835ca34d33 100644
>> --- a/include/linux/export.h
>> +++ b/include/linux/export.h
>> @@ -81,16 +81,31 @@ struct kernel_symbol {
>>
>> #else
>>
>> +/*
>> + * note on .section use: we specify progbits since usage of the "M" (SHF_MERGE)
>> + * section flag requires it. Use '%progbits' instead of '@progbits' since the
>> + * former apparently works on all arches according to the binutils source.
>> + */
>> +#define __KSTRTAB_ENTRY(sym) \
>> + asm(" .section \"__ksymtab_strings\",\"aMS\",%progbits,1 \n" \
>> + "__kstrtab_" #sym ": \n" \
>> + " .asciz \"" #sym "\" \n" \
>> + " .previous \n")
>> +
>> +#define __KSTRTAB_NS_ENTRY(sym, ns) \
>> + asm(" .section \"__ksymtab_strings\",\"aMS\",%progbits,1 \n" \
>> + "__kstrtabns_" #sym ": \n" \
>> + " .asciz " #ns " \n" \
>
>
>Hmm, it took some time for me to how this code works.
>
>ns is already a C string, then you added # to it,
>then I was confused.
>
>Personally, I prefer this code:
>" .asciz \"" ns "\" \n"
>
>so it looks in the same way as __KSTRTAB_ENTRY().
>
>
>
>BTW, you duplicated \"aMS\",%progbits,1" and ".previous"
>
>
>I would write it shorter, like this:
>
>
>#define ___EXPORT_SYMBOL(sym, sec, ns) \
> extern typeof(sym) sym; \
> extern const char __kstrtab_##sym[]; \
> extern const char __kstrtabns_##sym[]; \
> __CRC_SYMBOL(sym, sec); \
> asm(" .section \"__ksymtab_strings\",\"aMS\",%progbits,1\n" \
> "__kstrtab_" #sym ": \n" \
> " .asciz \"" #sym "\" \n" \
> "__kstrtabns_" #sym ": \n" \
> " .asciz \"" ns "\" \n" \
> " .previous \n"); \
> __KSYMTAB_ENTRY(sym, sec)
>
Sure, I can change that. Just thought it'd be easier to read with the
separate macros. Thanks for your comments!
next prev parent reply other threads:[~2019-11-26 9:55 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-20 14:51 [PATCH] export.h: reduce __ksymtab_strings string duplication by using "MS" section flags Jessica Yu
2019-11-20 15:02 ` Jessica Yu
2019-11-21 10:51 ` Rasmus Villemoes
2019-11-21 16:09 ` Jessica Yu
2019-11-21 16:53 ` Will Deacon
2019-11-22 11:44 ` Masahiro Yamada
2019-11-25 9:29 ` Rasmus Villemoes
2019-11-25 9:45 ` Jessica Yu
2019-11-25 15:42 ` [PATCH v2] " Jessica Yu
2019-11-26 8:32 ` Masahiro Yamada
2019-11-26 9:55 ` Jessica Yu [this message]
2019-11-26 13:56 ` Matthias Maennich
2019-11-26 15:31 ` Jessica Yu
2019-11-26 16:02 ` Matthias Maennich
2019-11-26 16:12 ` Masahiro Yamada
2019-11-26 16:48 ` Jessica Yu
2019-12-05 16:42 ` Matthias Maennich
2019-12-06 12:41 ` [PATCH v3] " Jessica Yu
2019-12-12 6:22 ` Masahiro Yamada
2019-12-12 10:36 ` Jessica Yu
2019-12-12 14:16 ` [PATCH v4] " Jessica Yu
2019-12-12 14:29 ` Matthias Maennich
2019-12-16 9:41 ` Jessica Yu
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=20191126095510.GA30181@linux-8ccs \
--to=jeyu@kernel.org \
--cc=arnd@arndb.de \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=maennich@google.com \
--cc=yamada.masahiro@socionext.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).