scripts/Makefile.modinst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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@rivosinc.com>
---
scripts/Makefile.modinst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst
index f97c9926ed31b2b14601ff7773a2ea48b225628b..c22f35f6b9db3cac3923b9e787b219f752570642 100644
--- a/scripts/Makefile.modinst
+++ b/scripts/Makefile.modinst
@@ -80,7 +80,7 @@ quiet_cmd_install = INSTALL $@
ifdef INSTALL_MOD_STRIP
ifeq ($(INSTALL_MOD_STRIP),1)
-strip-option := --strip-debug
+strip-option := --strip-unneeded
else
strip-option := $(INSTALL_MOD_STRIP)
endif
---
base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04
change-id: 20250122-strip_unneeded-cab729310056
--
- CharlieOn 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@rivosinc.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. Regardless: Reviewed-by: Nathan Chancellor <nathan@kernel.org> > --- > scripts/Makefile.modinst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst > index f97c9926ed31b2b14601ff7773a2ea48b225628b..c22f35f6b9db3cac3923b9e787b219f752570642 100644 > --- a/scripts/Makefile.modinst > +++ b/scripts/Makefile.modinst > @@ -80,7 +80,7 @@ quiet_cmd_install = INSTALL $@ > ifdef INSTALL_MOD_STRIP > > ifeq ($(INSTALL_MOD_STRIP),1) > -strip-option := --strip-debug > +strip-option := --strip-unneeded > else > strip-option := $(INSTALL_MOD_STRIP) > endif > > --- > base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04 > change-id: 20250122-strip_unneeded-cab729310056 > -- > - Charlie >
On Fri, Jan 31, 2025 at 12:52 PM Nathan Chancellor <nathan@kernel.org> 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@rivosinc.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 think INSTALL_MOD_STRIP=2 is worth supporting because it is equivalent to INSTALL_MOD_STRIP=--strip-unneeded Documentation/kbuild/kbuild.rst explains that any value other than '1' is passed to the strip command. INSTALL_MOD_STRIP ----------------- INSTALL_MOD_STRIP, if defined, will cause modules to be stripped after they 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. RISCV users can pass INSTALL_MOD_STRIP=--strip-unneeded. So, the question is, is it better to change the default option of INSTALL_MOD_STRIP=1 ? > > Regardless: > > Reviewed-by: Nathan Chancellor <nathan@kernel.org> > > > --- > > scripts/Makefile.modinst | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst > > index f97c9926ed31b2b14601ff7773a2ea48b225628b..c22f35f6b9db3cac3923b9e787b219f752570642 100644 > > --- a/scripts/Makefile.modinst > > +++ b/scripts/Makefile.modinst > > @@ -80,7 +80,7 @@ quiet_cmd_install = INSTALL $@ > > ifdef INSTALL_MOD_STRIP > > > > ifeq ($(INSTALL_MOD_STRIP),1) > > -strip-option := --strip-debug > > +strip-option := --strip-unneeded > > else > > strip-option := $(INSTALL_MOD_STRIP) > > endif > > > > --- > > base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04 > > change-id: 20250122-strip_unneeded-cab729310056 > > -- > > - Charlie > > -- Best Regards Masahiro Yamada
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@rivosinc.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@kernel.org> Thanks! - Charlie > > > --- > > scripts/Makefile.modinst | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/scripts/Makefile.modinst b/scripts/Makefile.modinst > > index f97c9926ed31b2b14601ff7773a2ea48b225628b..c22f35f6b9db3cac3923b9e787b219f752570642 100644 > > --- a/scripts/Makefile.modinst > > +++ b/scripts/Makefile.modinst > > @@ -80,7 +80,7 @@ quiet_cmd_install = INSTALL $@ > > ifdef INSTALL_MOD_STRIP > > > > ifeq ($(INSTALL_MOD_STRIP),1) > > -strip-option := --strip-debug > > +strip-option := --strip-unneeded > > else > > strip-option := $(INSTALL_MOD_STRIP) > > endif > > > > --- > > base-commit: ffd294d346d185b70e28b1a28abe367bbfe53c04 > > change-id: 20250122-strip_unneeded-cab729310056 > > -- > > - Charlie > >
On Fri, Jan 31, 2025 at 3:54 PM Charlie Jenkins <charlie@rivosinc.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@rivosinc.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@kernel.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. $ riscv64-linux-gnu-nm -n /tmp/strip-unneeded/lib/modules/6.13.0-09760-g69e858e0b8b2/kernel/drivers/gpu/drm/amd/amdgpu/amdgpu.ko > /tmp/symbol-with-strip-unneeded $ riscv64-linux-gnu-nm -n /tmp/strip-debug/lib/modules/6.13.0-09760-g69e858e0b8b2/kernel/drivers/gpu/drm/amd/amdgpu/amdgpu.ko > /tmp/symbol-with-strip-debug $ diff -u /tmp/symbol-with-strip-debug /tmp/symbol-with-strip-unneeded [ snip ] 00000000001676cc t uvd_v6_0_ring_test_ring 0000000000167802 t uvd_v6_0_ring_emit_pipeline_sync 0000000000167a02 t uvd_v6_0_ring_emit_fence -0000000000167b58 r CSWTCH.2 -0000000000167b68 r abm_settings -0000000000167b80 r abm_config -0000000000167b90 r min_reduction_table_v_2_2 -0000000000167ba0 r max_reduction_table_v_2_2 -0000000000167bb0 r min_reduction_table -0000000000167bc0 r max_reduction_table -0000000000167bd0 r custom_backlight_curve0 0000000000167c38 r abm_settings_config2 0000000000167c70 r abm_settings_config1 0000000000167ca8 r abm_settings_config0 -- Best Regards Masahiro Yamada
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@rivosinc.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@rivosinc.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@kernel.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. I believe these symbols are only useful for debugging, is there a usecase for them to be available when the user has modules compiled with INSTALL_MOD_STRIP=1? - Charlie > > > $ riscv64-linux-gnu-nm -n > /tmp/strip-unneeded/lib/modules/6.13.0-09760-g69e858e0b8b2/kernel/drivers/gpu/drm/amd/amdgpu/amdgpu.ko > > /tmp/symbol-with-strip-unneeded > $ riscv64-linux-gnu-nm -n > /tmp/strip-debug/lib/modules/6.13.0-09760-g69e858e0b8b2/kernel/drivers/gpu/drm/amd/amdgpu/amdgpu.ko > > /tmp/symbol-with-strip-debug > > $ diff -u /tmp/symbol-with-strip-debug /tmp/symbol-with-strip-unneeded > [ snip ] > 00000000001676cc t uvd_v6_0_ring_test_ring > 0000000000167802 t uvd_v6_0_ring_emit_pipeline_sync > 0000000000167a02 t uvd_v6_0_ring_emit_fence > -0000000000167b58 r CSWTCH.2 > -0000000000167b68 r abm_settings > -0000000000167b80 r abm_config > -0000000000167b90 r min_reduction_table_v_2_2 > -0000000000167ba0 r max_reduction_table_v_2_2 > -0000000000167bb0 r min_reduction_table > -0000000000167bc0 r max_reduction_table > -0000000000167bd0 r custom_backlight_curve0 > 0000000000167c38 r abm_settings_config2 > 0000000000167c70 r abm_settings_config1 > 0000000000167ca8 r abm_settings_config0 > > > > > -- > Best Regards > Masahiro Yamada
On Sat, Feb 1, 2025 at 6:33 AM Charlie Jenkins <charlie@rivosinc.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@rivosinc.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@rivosinc.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@kernel.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. > > I believe these symbols are only useful for debugging, is there a > usecase for them to be available when the user has modules compiled with > INSTALL_MOD_STRIP=1? > > - Charlie > > > > > > > $ riscv64-linux-gnu-nm -n > > /tmp/strip-unneeded/lib/modules/6.13.0-09760-g69e858e0b8b2/kernel/drivers/gpu/drm/amd/amdgpu/amdgpu.ko > > > /tmp/symbol-with-strip-unneeded > > $ riscv64-linux-gnu-nm -n > > /tmp/strip-debug/lib/modules/6.13.0-09760-g69e858e0b8b2/kernel/drivers/gpu/drm/amd/amdgpu/amdgpu.ko > > > /tmp/symbol-with-strip-debug > > > > $ diff -u /tmp/symbol-with-strip-debug /tmp/symbol-with-strip-unneeded > > [ snip ] > > 00000000001676cc t uvd_v6_0_ring_test_ring > > 0000000000167802 t uvd_v6_0_ring_emit_pipeline_sync > > 0000000000167a02 t uvd_v6_0_ring_emit_fence > > -0000000000167b58 r CSWTCH.2 > > -0000000000167b68 r abm_settings > > -0000000000167b80 r abm_config > > -0000000000167b90 r min_reduction_table_v_2_2 > > -0000000000167ba0 r max_reduction_table_v_2_2 > > -0000000000167bb0 r min_reduction_table > > -0000000000167bc0 r max_reduction_table > > -0000000000167bd0 r custom_backlight_curve0 > > 0000000000167c38 r abm_settings_config2 > > 0000000000167c70 r abm_settings_config1 > > 0000000000167ca8 r abm_settings_config0 > > > > > > > > > > -- > > Best Regards > > Masahiro Yamada -- Best Regards Masahiro Yamada
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@rivosinc.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@rivosinc.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@rivosinc.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@kernel.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? - Charlie > > > > > > > > > > > > > I believe these symbols are only useful for debugging, is there a > > usecase for them to be available when the user has modules compiled with > > INSTALL_MOD_STRIP=1? > > > > - Charlie > > > > > > > > > > > $ riscv64-linux-gnu-nm -n > > > /tmp/strip-unneeded/lib/modules/6.13.0-09760-g69e858e0b8b2/kernel/drivers/gpu/drm/amd/amdgpu/amdgpu.ko > > > > /tmp/symbol-with-strip-unneeded > > > $ riscv64-linux-gnu-nm -n > > > /tmp/strip-debug/lib/modules/6.13.0-09760-g69e858e0b8b2/kernel/drivers/gpu/drm/amd/amdgpu/amdgpu.ko > > > > /tmp/symbol-with-strip-debug > > > > > > $ diff -u /tmp/symbol-with-strip-debug /tmp/symbol-with-strip-unneeded > > > [ snip ] > > > 00000000001676cc t uvd_v6_0_ring_test_ring > > > 0000000000167802 t uvd_v6_0_ring_emit_pipeline_sync > > > 0000000000167a02 t uvd_v6_0_ring_emit_fence > > > -0000000000167b58 r CSWTCH.2 > > > -0000000000167b68 r abm_settings > > > -0000000000167b80 r abm_config > > > -0000000000167b90 r min_reduction_table_v_2_2 > > > -0000000000167ba0 r max_reduction_table_v_2_2 > > > -0000000000167bb0 r min_reduction_table > > > -0000000000167bc0 r max_reduction_table > > > -0000000000167bd0 r custom_backlight_curve0 > > > 0000000000167c38 r abm_settings_config2 > > > 0000000000167c70 r abm_settings_config1 > > > 0000000000167ca8 r abm_settings_config0 > > > > > > > > > > > > > > > -- > > > Best Regards > > > Masahiro Yamada > > > > -- > Best Regards > Masahiro Yamada
On Wed, Feb 5, 2025 at 3:29 AM Charlie Jenkins <charlie@rivosinc.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@rivosinc.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@rivosinc.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@rivosinc.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@kernel.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. -- Best Regards Masahiro Yamada
Hi Masahiro, On 05/02/2025 16:00, Masahiro Yamada wrote: > On Wed, Feb 5, 2025 at 3:29 AM Charlie Jenkins <charlie@rivosinc.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@rivosinc.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@rivosinc.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@rivosinc.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@kernel.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
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 <charlie@rivosinc.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@rivosinc.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@rivosinc.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@rivosinc.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@kernel.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 Any thoughts on this? I'd really like to merge this for riscv. Thanks, Alex > >
© 2016 - 2025 Red Hat, Inc.