Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Thu, 21 Nov 2019 17:09:20 +0100
From:   Jessica Yu <jeyu@kernel.org>
To:     Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc:     linux-kernel@vger.kernel.org,
        Matthias Maennich <maennich@google.com>,
        Masahiro Yamada <yamada.masahiro@socionext.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Will Deacon <will@kernel.org>,
        Catalin Marinas <catalin.marinas@arm.com>
Subject: Re: [PATCH] export.h: reduce __ksymtab_strings string duplication by
 using "MS" section flags
Message-ID: <20191121160919.GB22213@linux-8ccs>
References: <20191120145110.8397-1-jeyu@kernel.org>
 <93d3936d-0bc4-9639-7544-42a324f01ac1@rasmusvillemoes.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <93d3936d-0bc4-9639-7544-42a324f01ac1@rasmusvillemoes.dk>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

+++ Rasmus Villemoes [21/11/19 11:51 +0100]:
>On 20/11/2019 15.51, Jessica Yu wrote:
>>
>> 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
>
>[pet peeve: nul, not null.]

Ah, right you are!

>> 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.
>
>Yippee :) Thanks for picking up the ball and sending this.

And thanks for the idea in the first place :-)

>>  /*
>> - * 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 vs %progbits since usage of
>> + * "M" (SHF_MERGE) section flag requires it.
>>   */
>> +
>> +#ifdef CONFIG_ARM
>> +#define ARCH_PROGBITS %progbits
>> +#else
>> +#define ARCH_PROGBITS @progbits
>> +#endif
>
>Did you figure out a way to determine if ARM is the only odd one? I was
>just going by gas' documentation which mentions ARM as an example, but
>doesn't really provide a way to know what each arch uses. I suppose 0day
>will tell us shortly.

I *think* so. At least, I was going off of
drivers/base/firmware_loader/builtin/Makefile and
scripts/recordmcount.pl, which were the only other places that I found
that reference the %progbits vs @progbits oddity. They only use
%progbits in the case of CONFIG_ARM and @progbits for other
arches. I wasn't sure about arm64, but I looked at the assembly files
gcc produced and it looked like @progbits was used there. Added some
arm64 people to CC since they would know :-)

>If we want to avoid arch-ifdefs in these headers (and that could become
>unwieldy if ARM is not the only one) I think one could add a
>asm-generic/progbits.h, add progbits.h to mandatory-y in
>include/asm-generic/Kbuild, and then just provide a progbits.h on ARM
>(and the other exceptions) - then I think the kbuild logic automatically
>makes "#include <asm/progbits.h>" pick up the right one. And the header
>could define ARCH_PROGBITS with or without the double quotes depending
>on __ASSEMBLY__.

I think this would work, and it feels like the more correct solution
especially if arm isn't the only one with the odd progbits char. It
might be overkill if it's just arm that's affected though. I leave it
to Masahiro to see what he prefers.

Thanks!

Jessica