Message-ID: <28018be2-eabc-4d6f-9bb1-913a1f0515db@ghiti.fr>
Date: Thu, 3 Apr 2025 17:07:09 +0200
From: Alexandre Ghiti <alex@...ti.fr>
To: Masahiro Yamada <masahiroy@...nel.org>,
 Charlie Jenkins <charlie@...osinc.com>
Cc: Nathan Chancellor <nathan@...nel.org>, Nicolas Schier
 <nicolas@...sle.eu>, Paul Walmsley <paul.walmsley@...ive.com>,
 Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
 linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-riscv@...ts.infradead.org
Subject: Re: [PATCH] kbuild: Use --strip-unneeded with INSTALL_MOD_STRIP

Hi Masahiro,

On 05/02/2025 16:00, Masahiro Yamada wrote:
> On Wed, Feb 5, 2025 at 3:29 AM Charlie Jenkins <charlie@...osinc.com> wrote:
>> On Tue, Feb 04, 2025 at 01:04:26PM +0900, Masahiro Yamada wrote:
>>> On Sat, Feb 1, 2025 at 6:33 AM Charlie Jenkins <charlie@...osinc.com> wrote:
>>>> On Sat, Feb 01, 2025 at 12:10:02AM +0900, Masahiro Yamada wrote:
>>>>> On Fri, Jan 31, 2025 at 3:54 PM Charlie Jenkins <charlie@...osinc.com> wrote:
>>>>>> On Thu, Jan 30, 2025 at 08:52:45PM -0700, Nathan Chancellor wrote:
>>>>>>> On Wed, Jan 22, 2025 at 07:17:26PM -0800, Charlie Jenkins wrote:
>>>>>>>> On riscv, kernel modules end up with a significant number of local
>>>>>>>> symbols. This becomes apparent when compiling modules with debug symbols
>>>>>>>> enabled. Using amdgpu.ko as an example of a large module, on riscv the
>>>>>>>> size is 754MB (no stripping), 53MB (--strip-debug), and 21MB
>>>>>>>> (--strip-unneeded). ON x86, amdgpu.ko is 482MB (no stripping), 21MB
>>>>>>>> (--strip-debug), and 20MB (--strip-unneeded).
>>>>>>>>
>>>>>>>> Use --strip-unneeded instead of --strip-debug to strip modules so
>>>>>>>> decrease the size of the resulting modules. This is particularly
>>>>>>>> relevant for riscv, but also marginally aids other architectures.
>>>>>>>>
>>>>>>>> Signed-off-by: Charlie Jenkins <charlie@...osinc.com>
>>>>>>> Is there any sort of regression risk with this patch? If so, another
>>>>>>> option may be to give another level to INSTALL_MOD_STRIP like 2 so that
>>>>>>> INSTALL_MOD_STRIP=1 continues to behave as before but people can easily
>>>>>>> opt into this option. No strong opinion because I am not sure but was
>>>>>>> not sure if it was considered.
>>>>>> I do not believe this would cause regressions. The description on gnu
>>>>>> strip is:
>>>>>>
>>>>>> "Remove all symbols that are not needed for relocation processing in
>>>>>> addition to debugging symbols and sections stripped by --strip-debug."
>>>>>>
>>>>>> The description on llvm-strip is:
>>>>>>
>>>>>> "Remove from the output all local or undefined symbols that are not
>>>>>> required by relocations. Also remove all debug sections."
>>>>>>
>>>>>> gnu strip --strip-unneeded strips slightly more aggressively but it does
>>>>>> not appear this causes any issues.
>>>>>>
>>>>>>> Regardless:
>>>>>>>
>>>>>>> Reviewed-by: Nathan Chancellor <nathan@...nel.org>
>>>>>> Thanks!
>>>>>>
>>>>>
>>>>> It is true --strip-unneeded drops a lot of compiler-generated symbols, but
>>>>> it also drops real symbols that originate in the source code.
>>>>>
>>>>> So, this would give user-visible changes for kallsyms at least.
>>>> Adding INSTALL_MOD_STRIP="--strip-unneeded" would be sufficient for
>>>> riscv. However, this has the downside that riscv will require different
>>>> flags than other architectures to get reasonably sized modules.
>>> You can use INSTALL_MOD_STRIP=--strip-unneeded for all architecture if you like.
>>>
>>> I assume this is a riscv issue. Specifically, riscv gcc.
>>> With LLVM=1, I see much smaller riscv modules using INSTALL_MOD_STRIP=1.
>>>
>>> --strip-unneeded is needlessly aggressive for other architectures,
>>> and I do not see a good reason to change the default.
>> Yes it is primarily an issue with riscv GCC. I was hoping for something
>> more standardized so that other people using riscv GCC wouldn't
>> encounter this. Would it be reasonable to add this flag by default only
>> for the riscv architecture, or do you think it's just better to leave it
>> up to the user's choice?
> The latter.
>
> INSTALL_MOD_STRIP=1 passes --strip-debug.
> This is clearly documented in Documentation/kbuild/makefiles.rst
> and it has worked like that for many years, with no exception.
>
> If users want it to work differently, the flexibility is already there.
>
> If INSTALL_MOD_STRIP=1 worked differently, silently only for riscv,
> I would regard it as insane behavior.
>
>

I find it a bit "harsh" for users to know that on riscv, they need to 
set INSTALL_MOD_STRIP to have modules with a "normal" size.

So I'd rather have it set by default for riscv, would the following be 
acceptable for you?

diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 13fbc0f942387..c49b9dda20cd1 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -6,6 +6,8 @@
  # for more details.
  #

+INSTALL_MOD_STRIP := --strip-unneeded
+
  LDFLAGS_vmlinux := -z norelro
  ifeq ($(CONFIG_RELOCATABLE),y)
         LDFLAGS_vmlinux += -shared -Bsymbolic -z notext --emit-relocs
diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index 1628198f3e830..e60895761b73b 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -77,6 +77,8 @@ quiet_cmd_install = INSTALL $@
  # are installed. If INSTALL_MOD_STRIP is '1', then the default option
  # --strip-debug will be used. Otherwise, INSTALL_MOD_STRIP value will 
be used
  # as the options to the strip command.
+# Read arch-specific Makefile to set INSTALL_MOD_STRIP as needed.
+include $(srctree)/arch/$(SRCARCH)/Makefile
  ifdef INSTALL_MOD_STRIP

  ifeq ($(INSTALL_MOD_STRIP),1)


Thanks,

Alex

Powered by blists - more mailing lists