From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net [217.70.183.195]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 693C2262A6; Mon, 21 Apr 2025 06:06:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.70.183.195 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1745215583; cv=none; b=dxzahRMM9rZo9RB6JaVgTmXkSdXymQlQjmgCgprn9zm7FIQEwgfhixmkVOvsGcsV1nWF+FBwdWmMcRpOKJiDgxG4KTSKaKd0e7tiNz0OlyymV7nAiFVUGOvJQIiKNR+308Cc5n6Ed+PXQ64Z4r4LkHPsPyxVXedqK+Int8iCUhU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1745215583; c=relaxed/simple; bh=hxUYmdprB8whlKzdlCmOqGWaVVwqb45yoD9TgT0gIq4=; h=Message-ID:Date:MIME-Version:Subject:From:To:Cc:References: In-Reply-To:Content-Type; b=NpWIc+V8sAmbvx424Q8/FtqxgOufNKyfhZ/zoJDDM5XQ+J2Idz9X4tLDyY6XRFWkhSyaflhSrh9mmu63tBQJEXOT/BQL/C9AbW6J5/J5OzIOiCfoEWbCSrzoSFHyEBpQ6LhFaOr0+cNwyDngKCV2hVPvJ6Ry3gAM/BEeeleTnr4= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ghiti.fr; spf=pass smtp.mailfrom=ghiti.fr; arc=none smtp.client-ip=217.70.183.195 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ghiti.fr Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=ghiti.fr Received: by mail.gandi.net (Postfix) with ESMTPSA id 7A1601FCE7; Mon, 21 Apr 2025 06:06:06 +0000 (UTC) Message-ID: Date: Mon, 21 Apr 2025 08:06:05 +0200 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] kbuild: Use --strip-unneeded with INSTALL_MOD_STRIP Content-Language: en-US From: Alexandre Ghiti To: Masahiro Yamada , Charlie Jenkins Cc: Nathan Chancellor , Nicolas Schier , Paul Walmsley , Palmer Dabbelt , Albert Ou , linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org References: <20250122-strip_unneeded-v1-1-ac29a726cb41@rivosinc.com> <20250131035245.GA47826@ax162> <28018be2-eabc-4d6f-9bb1-913a1f0515db@ghiti.fr> In-Reply-To: <28018be2-eabc-4d6f-9bb1-913a1f0515db@ghiti.fr> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-GND-State: clean X-GND-Score: -100 X-GND-Cause: gggruggvucftvghtrhhoucdtuddrgeefvddrtddtgddvgedttdejucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuifetpfffkfdpucggtfgfnhhsuhgsshgtrhhisggvnecuuegrihhlohhuthemuceftddunecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfjughrpefkffggfgfuhffvvehfjggtgfesthekredttddvjeenucfhrhhomheptehlvgigrghnughrvgcuifhhihhtihcuoegrlhgvgiesghhhihhtihdrfhhrqeenucggtffrrghtthgvrhhnpeetudeugffhhefgvddvleehvedvfeetueekieegteeguddvveelfffhfeetheehffenucfkphepvddttddumeekiedumeeffeekvdemvghfledtmedufhehjeemvgdutggumedukegutdemtgdtvdeknecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepvddttddumeekiedumeeffeekvdemvghfledtmedufhehjeemvgdutggumedukegutdemtgdtvdekpdhhvghloheplgfkrfggieemvddttddumeekiedumeeffeekvdemvghfledtmedufhehjeemvgdutggumedukegutdemtgdtvdekngdpmhgrihhlfhhrohhmpegrlhgvgiesghhhihhtihdrfhhrpdhnsggprhgtphhtthhopedutddprhgtphhtthhopehmrghsrghhihhrohihsehkvghrnhgvlhdrohhrghdprhgtphhtthhopegthhgrrhhlihgvsehrihhvohhsihhntgdrtghomhdprhgtphhtthhopehnrghthhgrnheskhgvrhhnvghlrdhorhhgpdhrtghpt hhtohepnhhitgholhgrshesfhhjrghslhgvrdgvuhdprhgtphhtthhopehprghulhdrfigrlhhmshhlvgihsehsihhfihhvvgdrtghomhdprhgtphhtthhopehprghlmhgvrhesuggrsggsvghlthdrtghomhdprhgtphhtthhopegrohhusegvvggtshdrsggvrhhkvghlvgihrdgvughupdhrtghpthhtoheplhhinhhugidqkhgsuhhilhgusehvghgvrhdrkhgvrhhnvghlrdhorhhg X-GND-Sasl: alex@ghiti.fr Hi Masahiro, On 03/04/2025 17:07, Alexandre Ghiti wrote: > Hi Masahiro, > > On 05/02/2025 16:00, Masahiro Yamada wrote: >> On Wed, Feb 5, 2025 at 3:29 AM Charlie Jenkins >> 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 >>>> 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 >>>>>> 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 >>>>>>>> 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 >>>>>>> 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 Any thoughts on this? I'd really like to merge this for riscv. Thanks, Alex > >